quic: part 1... quic-modified openssl #32377
Conversation
nodejs-github-bot
added
the
openssl
|
I don't feel qualified to review the technical aspects of this but that first commit looks like a rather large one to float on top of openssl and I'm concerned may impair our ability to take openssl updates in a timely fashion. The in-progress 1.1.1e update, for example, has changed some of the header file locations. The current maintaining openssl guide removes and replaces the cc @sam-github |
Yes, that's a valid concern. The patch itself is actually quite easy to maintain. The third PR in this set (#32379) includes the entirely of the patch file and instructions for updating. I still need to update it to 1.1.1e but that should not take long at all. But yes, since the OpenSSL OMC has taken a bit of an obstinate point of view on this it's not ideal. I don't think, however, that we should block on that. |
|
I'm concerned about the size of the patch as well, but as long as how to maintain it is clear, it should be OK. Also, the feature has to remain experimental until QUIC support lands officially in OpenSSL, and someone has to commit to maintaining the patches, because if the QUIC patches don't cherry-pick clean onto a new OpenSSL release, we might have to drop them. To avoid that outcome, can you make these updates:
The process of a maintainer updating OpenSSL should be as mechanical as necessary. I rebuilt the config, it didn't change the whitespace comment, so that's OK. Unfortunately, I attempted to cherrypick ce0fdeb onto #32328, it conflicted with deps/openssl/openssl/test/sslapitest.c, which was discouraging, and I didn't find a https://github.com/akamai/openssl/commits/OpenSSL_1_1_1e-quic |
|
I've just about got the updated version of the patch for 1.1.1e ready to go. |
|
@sam-github ... yes absolutely. The instructions for maintaining are actually added in the third PR in the batch but I'll be moving them in here. The patch file itself will be checked in separately and will be kept up to date. The quic support is currently disabled and not built by default and would remain so for at least some period of time and the folks at akamai have indicated that they will be keeping the patches up to date as much as possible -- they need to because they have infrastructure that depends on it. I will get the commit that adds the maintenance details in to this PR soonish. |
Done. Once the 1.1.1e update lands and I'll update the nodejs/quic repo and then update the PR |
jasnell
force-pushed
the
initial-quic-part-1
branch
from
718a2b7
to
00bfcd9
Compare
|
Updated for openssl 1.1.1e with maintenance instructions included. |
|
CI is good on this. |
jasnell
added
review wanted
|
Putting all three of the PRs here on the tsc-agenda to queue up discussion on how we want to get these landed. |
jasnell
added
the
quic
|
TODO: Will update this to 1.1.1f once #32583 lands |
jasnell
removed
the
tsc-agenda
jasnell
force-pushed
the
initial-quic-part-1
branch
from
00bfcd9
to
7afa9a8
Compare
|
nodejs/quic master was rebased and sync'd with nodejs/master earlier today. The commits in the PR have been updated accordingly. |
jasnell
force-pushed
the
initial-quic-part-1
branch
from
7afa9a8
to
846b216
Compare
Those whitespace changes are part of the openssl build process. We see exactly the same thing in the other openssl updates when configuration updates happen. Unfortunately there's not much we can do about it. |
|
Weird. Not a blocker for me, just an observation that it makes it kind of a pain to review. 😬 |
jasnell
force-pushed
the
initial-quic-part-1
branch
from
846b216
to
0612442
Compare
Acquired from: https://github.com/akamai/openssl/tree/OpenSSL_1_1_1f-quic Squashed: * akamai/openssl@2ef7c58 * akamai/openssl@3f8eda3 * akamai/openssl@b37f665 * akamai/openssl@6b23589 * akamai/openssl@3a793e0 --- This is a cherry-pick of 2a4b03a306439307e0b822b17eda3bdabddfbb68 on the master-quic-support2 branch (2019-10-07) Which was a rebase/squash of master-quic-support: * 5aa62ce Add support for more secrets - Todd Short/Todd Short (master-quic-support) * 58e0643 Tweeks to quic_change_cipher_state() - Todd Short/Todd Short * 8169702 Move QUIC code out of tls13_change_cipher_state() - Todd Short/Todd Short * a08cfe6 Correctly disable middlebox compat - Todd Short/Todd Short * 3a9eabf Add OPENSSL_NO_QUIC wrapper - Todd Short/Todd Short * f550eca Add client early traffic secret storage - Todd Short/Todd Short * 1b787ae Quick fix: s2c to c2s for early secret - Todd Short/Todd Short * f97e6a9 Don't process an incomplete message - Todd Short/Todd Short * 81f0ce2 Reset init state in SSL_process_quic_post_handshake() - Todd Short/Todd Short * 5d59cf9 Fix quic_transport constructors/parsers - Todd Short/Todd Short * 5e5f91c Fix INSTALL nit. - Todd Short/Todd Short * bd290ab Fix duplicate word in docs - Todd Short/Todd Short * 699590b fixup! Handle partial handshake messages - Todd Short/Todd Short * a472a8d Handle partial handshake messages - Todd Short/Todd Short * 363cf3d fixup! Use proper secrets for handshake - Todd Short/Todd Short * b03fee6 Use proper secrets for handshake - Todd Short/Todd Short * 2ab1aa0 Move QUIC transport params to encrypted extensions - Todd Short/Todd Short * 0d16af9 Make temp secret names less confusing - Todd Short/Todd Short * abb6f39 New method to get QUIC secret length - Todd Short/Todd Short * 05fdae9 Add support for BoringSSL QUIC APIs - Todd Short/Todd Short 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 New method to get QUIC secret length Make temp secret names less confusing Move QUIC transport params to encrypted extensions Use proper secrets for handshake fixup! Use proper secrets for handshake Handle partial handshake messages fixup! Handle partial handshake messages Fix duplicate word in docs Fix INSTALL nit. Fix quic_transport constructors/parsers Reset init state in SSL_process_quic_post_handshake() Don't process an incomplete message Quick fix: s2c to c2s for early secret Add client early traffic secret storage Add OPENSSL_NO_QUIC wrapper Correctly disable middlebox compat Move QUIC code out of tls13_change_cipher_state() Create quic_change_cipher_state() that does the minimal required to generate the QUIC secrets. (e.g. encryption contexts are not initialized). Tweeks to quic_change_cipher_state() Add support for more secrets Fix resumption secret (cherry picked from commit 16fafdf4e0ec6cddd5705f407e5dca26cb30914d) QUIC: Handle EndOfEarlyData and MaxEarlyData QUIC: Increase HKDF_MAXBUF to 2048 Fall-through for 0RTT
After an OpenSSL source update, all the config files need to be regenerated and committed by: $ make -C deps/openssl/config $ git add deps/openssl/config/archs $ git add deps/openssl/openssl/include/crypto/bn_conf.h $ git add deps/openssl/openssl/include/crypto/dso_conf.h $ git add deps/openssl/openssl/include/openssl/opensslconf.h $ git commit
Signed-off-by: James M Snell <jasnell@gmail.com>
jasnell
force-pushed
the
initial-quic-part-1
branch
from
0612442
to
e96bea6
Compare
This is one of three separate PRs that would need to be landed together. They have been split to allow easier reviewing. This PR covers the Openssl modifications necessary to support the QUIC implementation.
NOTE: This PR merges into the
masterbranchThe other two PRs in this batch are:
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes