Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Openssl 3.1.4+quic #149

Merged
merged 65 commits into from
Oct 26, 2023
Merged

Conversation

tmshort
Copy link
Member

@tmshort tmshort commented Oct 24, 2023

Checklist
  • documentation is added or updated
  • tests are added or updated

tmshort and others added 30 commits October 24, 2023 16:47
This adds a compatible API for BoringSSL's QUIC support, based
on the current |draft-ietf-quic-tls|.

Based on BoringSSL commit 3c034b2cf386b3131f75520705491871a2e0cafe
Based on BoringSSL commit c8e0f90f83b9ec38ea833deb86b5a41360b62b6a
Based on BoringSSL commit 3cbb0299a28a8bd0136257251a78b91a96c5eec8
Based on BoringSSL commit cc9d935256539af2d3b7f831abf57c0d685ffd81
Based on BoringSSL commit e6eef1ca16a022e476bbaedffef044597cfc8f4b
Based on BoringSSL commit 6f733791148cf8a076bf0e95498235aadbe5926d
Based on BoringSSL commit 384d0eaf1930af1ebc47eda751f0c78dfcba1c03
Based on BoringSSL commit a0373182eb5cc7b81d49f434596b473c7801c942
Based on BoringSSL commit b1b76aee3cb43ce11889403c5334283d951ebd37
Create quic_change_cipher_state() that does the minimal required
to generate the QUIC secrets. (e.g. encryption contexts are not
initialized).
Try to reduce unneeded whitespace changes and wrap new code to 80 columns.
Reword documentation to attempt to improve clarity.
Add some more sanity checks and clarifying comments to the code.
Update referenced I-D versions.
QUIC does not use the TLS KeyUpdate message/mechanism, and indeed
it is an error to generate or receive such a message.  Add the
necessary checks (noting that the check for receipt should be
redundant since SSL_provide_quic_data() is the only way to provide
input to the TLS layer for a QUIC connection).
For now, just test that we don't generate any, since we don't really
expose the mechanics for encrypting one and the QUIC API is not
integrated into the TLSProxy setup.
Make all data supplied via SSL_provide_quic_data() pass through an
internal buffer, so that we can handle data supplied with arbitrary
framing and only parse complete TLS records onto the list of QUIC_DATA
managed by quic_input_data_head/quic_input_data_tail.

This lets us remove the concept of "incomplete" QUIC_DATA structures,
and the 'offset' field needed to support them.

However, we've already moved the provided data onto the buffer by
the time we can check for KeyUpdate messages, so defer that check
to quic_get_message() (where it is adjacent to the preexisting
ChangeCipherSpec check).

To avoid extra memory copies, we also make the QUIC_DATA structures
just store offsets into the consolidated buffer instead of having copies
of the TLS handshake messages themselves.
The QUIC-TLS spec requires that TLS handshake messages do not cross
encryption level boundaries, but we were not previously enforcing this.
Prefix the shared library version with 17 (for 'Q'), to allow this
version to be used alongside a standard OpenSSL distribution

Add +quic to the version (i.e. build metadata)
kaduk and others added 16 commits October 24, 2023 16:48
We now let you call this function outside of the handshake, to provide
post-handshake QUIC data.

We also no longer have the limitation that the application must provide
the TLS handshake message header in a single call.
The limit on the amount of queued data is to avoid being an amplification
vector, specifically.
The QUIC APIs have no need to interact with TLS ciphers, since
QUIC records use different cryptographic protections than TLS ciphers.
Fixes openssl#55
Had to fixup tests because SSL_accept() eventually calls SSL_clear() and
it was removing the inital ClientHello sent via SSL_provide_quic_data()
from the server SSL.
Undo SSL_clear() changes in test
Break apart SSL_clear() into SSL_clear_quic() and SSL_clear_not_quic()
In SSL_clear(), call both functions
In SSL_accept(), call SSL_clear_not_quic()
Don't make the new functions public.
Add link to OMCs plans.
OpenSSL 3.0 is released, update tense.
Fix some typos.
Make relative URLs absolute.
@tmshort
Copy link
Member Author

tmshort commented Oct 25, 2023

Not worried about Fuzz

@richsalz
Copy link
Member

Not worried about Fuzz

Is this something we should care about? I'm probably fine with either answer, but would like to know why it's an issue.

@kaduk
Copy link
Member

kaduk commented Oct 25, 2023

Not worried about Fuzz

Is this something we should care about? I'm probably fine with either answer, but would like to know why it's an issue.

clang-15: error: unsupported option '--with-fuzzer-lib=/usr/lib/libFuzzingEngine' seems to say that the clang available doesn't support libfuzzer. But that's a pretty modern clang, and libfuzzer is also a LLVM project, so that's kind of weird. Googling for that flag doesn't find much other than "how to fuzz openssl" postings, but I do wonder if perhaps it needs the C++ driver rather than the C driver.
Anyway, to answer the question, we would probably prefer to have the fuzzers running, but openssl itself is also supposed to be running them, and IIRC we haven't updated the fuzzers' test recipes to attempt to engage any of our QUIC code, so we wouldn't be getting particular benefit from them other than testing stock openssl's code. So, we should care, but not very urgently.

@@ -37,8 +37,11 @@ IF[{- !$disabled{'deprecated-3.0'} -}]
SHARED_SOURCE[../libssl]=s3_cbc.c
SOURCE[../libssl]=ssl_rsa_legacy.c
ENDIF

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one looks a little funny

Copy link
Member Author

@tmshort tmshort Oct 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not incorrect, but strange how it was removed.

@baparham
Copy link

Are tags going to be created for these new versions once they are merged? what about existing missing tags like 3.0.11 that have been merged for a few weeks?

@tmshort
Copy link
Member Author

tmshort commented Oct 26, 2023

Are tags going to be created for these new versions once they are merged? what about existing missing tags like 3.0.11 that have been merged for a few weeks?

I will do it soon (possibly this week).

@tmshort tmshort merged commit 330feef into quictls:openssl-3.1.4+quic Oct 26, 2023
72 of 73 checks passed
@tmshort tmshort deleted the openssl-3.1.4+quic branch October 26, 2023 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants