-
Notifications
You must be signed in to change notification settings - Fork 159
move to draft-17 #7
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kazuho
added a commit
that referenced
this pull request
Oct 26, 2016
huitema
added a commit
that referenced
this pull request
Aug 22, 2017
Align with recent changes in h2o/gettls/master
deweerdt
added a commit
to deweerdt/picotls
that referenced
this pull request
Dec 20, 2018
ASAN finding: ``` ================================================================= ==24799==ERROR: AddressSanitizer: global-buffer-overflow on address 0x55841ae13761 at pc 0x55841aced3ec bp 0x7ffca51cae30 sp 0x7ffca51ca5e0 READ of size 48 at 0x55841ae13761 thread T0 #0 0x55841aced3eb in __asan_memcpy (/home/def/p/floss/picotls/test-openssl.t+0x1603eb) h2o#1 0x55841ad882db in ptls_hmac_create /home/def/p/floss/picotls/t/../lib/picotls.c:4680:5 h2o#2 0x55841ad899e3 in ptls_hkdf_expand /home/def/p/floss/picotls/t/../lib/picotls.c:4709:25 h2o#3 0x55841ad87dcd in hkdf_expand_label /home/def/p/floss/picotls/t/../lib/picotls.c:4751:11 h2o#4 0x55841ad8a500 in ptls_hkdf_expand_label /home/def/p/floss/picotls/t/../lib/picotls.c:4764:12 h2o#5 0x55841ad8a500 in get_traffic_key /home/def/p/floss/picotls/t/../lib/picotls.c:1090 h2o#6 0x55841ad8a500 in new_aead /home/def/p/floss/picotls/t/../lib/picotls.c:4798 h2o#7 0x55841add8597 in ptls_aead_new /home/def/p/floss/picotls/t/../lib/picotls.c:4818:12 h2o#8 0x55841add8597 in test_ciphersuite /home/def/p/floss/picotls/t/picotls.c:122 h2o#9 0x55841ad9a4ed in test_aes256gcm /home/def/p/floss/picotls/t/picotls.c:241:9 h2o#10 0x55841ad69d3f in subtest /home/def/p/floss/picotls/deps/picotest/picotest.c:96:5 h2o#11 0x55841ad99615 in test_picotls /home/def/p/floss/picotls/t/picotls.c:1161:5 h2o#12 0x55841ad69d3f in subtest /home/def/p/floss/picotls/deps/picotest/picotest.c:96:5 h2o#13 0x55841ade8e5b in main /home/def/p/floss/picotls/t/openssl.c:277:5 h2o#14 0x7faf59057222 in __libc_start_main (/usr/lib/libc.so.6+0x24222) h2o#15 0x55841ac1b7cd in _start (/home/def/p/floss/picotls/test-openssl.t+0x8e7cd) 0x55841ae13761 is located 63 bytes to the left of global variable '<string literal>' defined in '/home/def/p/floss/picotls/t/picotls.c:116:78' (0x55841ae137a0) of size 12 '<string literal>' is ascii string 'hello world' 0x55841ae13761 is located 0 bytes to the right of global variable '<string literal>' defined in '/home/def/p/floss/picotls/t/picotls.c:116:34' (0x55841ae13740) of size 33 '<string literal>' is ascii string '01234567890123456789012345678901' SUMMARY: AddressSanitizer: global-buffer-overflow (/home/def/p/floss/picotls/test-openssl.t+0x1603eb) in __asan_memcpy Shadow bytes around the buggy address: 0x0ab1035ba690: f9 f9 f9 f9 00 06 f9 f9 f9 f9 f9 f9 00 00 05 f9 ```
huitema
added a commit
that referenced
this pull request
Dec 2, 2022
* remove ESNI stuff * be certain `tls` is immutable when decoding CH * encode CH without touching `ptls_t` directly, decode ECHConfigList * send ech extension / send fake psk in outer * emit ECH * it works (for the most straight-forward case) * signal if ech is used * remove esni command line tool * [cli] -E and -K options to handle ECH (it works) * check existence of the extension (and the error code) * Update include/picotls.h * `ech` will always be non-NULL with modes other than INNER * less magic numbers * add and recognize padding * restore msghash_off, it points mid-message when resuming * emit and check ECH accept confirmation hash * run two hashes for CHInner and CHOuter, choose the right one * refactor as a preparation * generate HRR.ECH (and we can roll the key schedule when sending stateless retry) * "confirm" implies acceptance * [ECH] handle HRR correctly * check ECH.type always (as well as concentrating the logic) * ServerHello.ECH can exist unless when the server responds to inner CH * add I/F to obtain the type of the handshake * fix the encoded order * HKDF-Expand-Label being used is that of RFC 8446, hence uses the "tls13 " prefix * use const-time op * key-schedule uses the transcript with confirmation hash * CHinner MUST NOT offer tls 1.2 or below * [ECH] test variations, e.g., retry * use wrapper function so as to not miss setting fields * we can say that ECH is used whenever ECH AEAD context is available * ciphers given significance, as it is the only attribute used on both sides * test configuration mismatch * send / receive retry_configs * add FIXME * oops * [ECH] do not touch key_schedule when determining acceptance * remove ESNI stuff * replay entire ECH extension when ECH is rejected via HRR * upon ech config mismatch, report retry_config to the application iff it is applicable * split ECH config applicability testing (ignore upon failure) vs. ECH instatiation error (reported) * send ECH_REQUIRED alert if rejected, saving retry_configs correctly * it's a MISmatch * p256 might be the only algorithm that we support * dispose state when AEAD decryption fails, otherwise `ptls_is_ech_handshake` returns true * clarify the contract * make it simple * consistent naming convention * add comment * better to rename "select_one" now that we have `select_outer` that selects "one" of the CH * move the condition out, add comment * unless the client offered ECH, reject EE.ECH * outer- and inner-random have to be identical unless ECH is used * retain innerCH.random separately * [ECH] add I/F to obtain kem/cipher being used * send retry_config only when we are capable of accepting ECH * in PSK mode, CertificateRequest is rejected by the state machine (and when ECH is rejected, we send ECH_REQUIRED alert right above) * clang-format * add note that we are not following the spec * do not use ECH even when config is provided, unless server name is a DNS name * merge the struct * pass server-name as argument as it can be ECH.public_name * public_name is at least one byte * report error code * create helper * enc is at least one byte * use `ptls_decode8` * ignore ECHConfig that have IP address as public name * oops * payload is at least one byte * reorder and clarify the logic * rely on the decode function * use constant, state check in `decode_server_hello` * add new extensions to the table, rely on that * dispose of ECH AEAD context during handshake, decryption failure of inner CH in 2nd CH is fatal * use the existing function to discard ECH state after Hello exchange * track known extensions rather than the smallest 64 (otherwise we cannot track the draft codepoints of ECH extensions) * clear remaining ECH state even when HRR is used * when ECH exchange is complete reduce the number of hashes too * no need to write after duplicate * add test for rebuilding inner CH * rebuild error is ILLEGAL_PARAMETER * encrypted_client_hello extension cannot be referred to by ech_outer_extensions Co-authored-by: Kazuho Oku <kazuhooku@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Among the wire-level changes, picotls (at the moment) is affected by:
picotls is unaffected by the following changes, since it does not (yet) implement the features being mentioned: