-
Notifications
You must be signed in to change notification settings - Fork 850
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
Ech hello retry request #6805
base: master
Are you sure you want to change the base?
Ech hello retry request #6805
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Would like to see customer feedback. Assigned to PR cap wolfssl-bot.
AArch64 test failed, possibly from network hiccup? Retest this please Jenkins
|
Looks like CI is flagging a valgrind test. Please also rebase onto master rather than merge in, seems like github has issues when merging in the master branch as seen here with lots of extra commits showing. |
10be95d
to
73a6424
Compare
to an hrr, fix bad getSize for hrr ech, fix using the wrong transcript hash for hrr ech, add new hrr test for ech to api.c
blows away the ech and it's current hpke context, causing the hrr handling to fail
allocated if an hrr happened
which are not restarted and the inner hsHashes which are restared on HRR. also send empty string with 0 encLen when sending clientHelloInner2. setup works wolfssl->wolfssl but fails to match acceptance for first HRR message when talking to an openssl server, does still work without HRR when talking to cloudflare's server without HRR.
the unmodified hrr and sh in the digest
0ed604c
to
36c89cc
Compare
@jpbland1 please resolve merge conflicts. |
@@ -8435,6 +8435,13 @@ void wolfSSL_ResourceFree(WOLFSSL* ssl) | |||
} | |||
FreeSuites(ssl); | |||
FreeHandshakeHashes(ssl); | |||
#ifdef HAVE_ECH | |||
/* try to free the ech hashes in case we errored out */ | |||
ssl->hsHashes = ssl->hsHashesEch; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assigning to be freed seems silly.
Make a function that takes a HsHashes* and frees it.
Either name it FreeHanshakeHashes and change callers or have FreeHanshakeHashes call the new function.
Description
Refactor Hpke to allow multiple uses of a context instead of just one shot mode. Refactor ECH to handle hello retry request with a different ech acceptance scheme and to re-use the hpke context after a hello retry request when sending the second client hello
Fixes #6802
Testing
Tested the regular ECH through unit tests, need to figure out how to trigger an HRR
Checklist