Fix ECH on Visual Studio #449
Closed
Conversation
This file contains 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
suppress warning on Xcode
* it works * import / export API (tls12 only atm) * transplant other properties retained by picotls * send alerts correctly * add support for chachapoly * fix alert type * to avoid nonce reuse, start from what OpenSSL used * record if session was reused * update cifra, bcrypt backends * fix MSVC compile error * add `ptls_get_protocol_version` * promote to public value * ci: use Ubuntu 22.04 to test picotls with OpenSSL 3.0 * concept * add ptlslog_set_fd(int) to set the fd the cli support -j=file and PTLSLOG=file to use ptlslog_set_fd() * revert the switch to writev(2) discussion: #406 (comment) * let PTLSLOG_CONN respect ptls_skip_tracing() * ptlslog: add "module" field to provide who emits the event * do not emit secret in PTLSLOG * add PTLS_HEXDUMP macro as an easy-to-use handle for ptls_hexdump() * introduce PTLSLOG_ELEMENT_UNSAFESTR() * ptlslog: put JSON's null if a string value is NULL * Revert "ptlslog: put JSON's null if a string value is NULL" This reverts commit a163989. * inline function must be declared as static * do not use PTLS_HEXDUMP; it is not available for VC++ * avoid errors in VC++ * disable wrarnings in VC++ for write(2) * VC++ does not support VLAs * invalidate ptlslog_fd when write(2) fails * fix unsafe string handling in ptlslog * remove debugging code * add PTLSLOG_ELEMENT_HEXDUMP instead of PTLS_ELEMENT that depends on a GCC extesions * allow ptlslog to set multiple fds (up to 8) * remove a redundant ptlslog interface fromn cli.c * reorder options * close fd before invalidating it * fix the return value of ptlslog_add_fd * ptlslog: records num of lost events * Fix compile warnings about unchecked returns in t/cli * Fix Visual Studio warnings when using OpenSSL 3.0 * symbols defined in util.h do not have `ptls_` prefix * first argument is an expression * fix the image to use ubuntu 22.04 * Add ticket functions which use OpenSSL v3 EVP_MAC_CTX This change adds the following new ticket functions which use OpenSSL v3 EVP_MAC_CTX. HMAC APIs are deprecated in OpenSSL v3. EVP_MAC_CTX is its replacement. - ptls_openssl_encrypt_ticket_evp - ptls_openssl_decrypt_ticket_evp * add failing test * copy correct amount of data _and_ generate new vectors * msvc requires an element * clear GHASH vectors before calling `free` * clang-format * ptlslog: remove the limitation of PTLSLOG_MAXCONN * do not use magic numbers * ptlslog: check ptlslog is active or not at the beginning of PTLSLOG_CONN * inlinize ptlslog_is_active() * No need to call free() because realloc(ptr, 0) frees ptr and returns 0 * move the condition out of PTLSLOG_CONN * add missing `inline` keyword * revert using a label & goto because the label name must not be duplicated in a function * comment * rewrite escape_json_unsafe_string based on picojson.h https://github.com/kazuho/picojson/blob/master/picojson.h#L531 * include pthread.h for Windows * format * ptlslog: introduce bool, and some internal funcs for optimizations * fix comments * use INT32_MIN value * let ptlslog optional (enabled by default on Linux and macOS) * suppress errors for Windows * merge ptlslog.h into picotls.h (like those in pembase64.c), determine availability rather than let it be configurable (like `PTLS_OPENSSL_HAVE_*`) * [xcode] add log.c * do not expose the internals * symbols use `ptls_` as prefix * picotls checks allocation failure; returns PTLS_ERROR_* * partial write is loss * rename before refactor * realloc(0) may return non-NULL, fd_index should not be incremented when removing fd * let's use constant expressions * rather than exposing an internal API used for building strings, promote JSON escape function to a public API * revert changes to wincompat.h as they should be unnecessary now that logging is disabled on windows * this is also unnecessary * eaisest way to maintain compatibility is to not have new files; log.c is small anyways * no need to define as a block (the macro cannot be used as an ordinary statement anyways) * concede to using `snprintf` to support compilers that complain without having the wisdom to check the correctness * [github actions] increase timeout * use the result returned rather than recalculate * better use `PTLS_LOG` assuming that we'd have `QUICLY_LOG` and `H2O_LOG` rather than `QUICLYLOG`, `H2OLOG` * oops * add tls12 cipher awareness * add tls12 ciphers to supported cipher suites instead of using mapping table * leave minicrypto alone * remove new accessors * rename tls12 ciphers but remove them from tls13 list * iana_id is not needed * export `find_cipher_suite` externally as `ptls_find_cipher_suite` in addition export supported tls12 ciphers from openssl backend * make PTLS_OPENSSL_HAVE_CHACHA20_POLY1305 checks consistent Co-authored-by: Kazuho Oku <kazuhooku@gmail.com> * ptls_find_cipher_suite should accept cipher list instead of context * provide separate knob for including / excluding sensitive data * revert unecessary change around alloca() * s/ptlslog_/ptls_log_/ for (semi-)public functions * check `ptls_log.is_active` first, but only once * move `tls12_cipher_suites` to the end to improve source compatibility with existing code that uses positional initialization of a struct * clang-format * declare IANA IDs in the header file, fixes incorrect IANA ID (0x009d) used for `ptls_openssl_tls12_dhe_rsa_aes128gcmsha256` which results in incorrect cipher suite being returned due the value being added to `ptls_openssl_tls12_cipher_suites` prior to `ptls_openssl_tls12_rsa_aes256gcmsha384` * and `.name`s also * fix cipher name (amends #429) * remove ones that are not going to be meaningful as performance optimization * add callback for loading raw public keys (at the moment X25519-only) * implement HPKE (basic mode, x25519-only) * raw private key cannot be loaded in OpenSSL 1.x * some deployments do not have x25519 * `load` should be available on both sides * remove redundant code * [xcode] add files * update bcrypt backend following API changes * MSVC does not like `{}` * add missing include * maybe these are the right names * add casts to suppress unneeded compiler warnings from MSVC * Kazuho/hpke as of 2022/11/08 (#4) * it works * import / export API (tls12 only atm) * transplant other properties retained by picotls * send alerts correctly * add support for chachapoly * fix alert type * to avoid nonce reuse, start from what OpenSSL used * record if session was reused * update cifra, bcrypt backends * fix MSVC compile error * add `ptls_get_protocol_version` * promote to public value * ci: use Ubuntu 22.04 to test picotls with OpenSSL 3.0 * concept * add ptlslog_set_fd(int) to set the fd the cli support -j=file and PTLSLOG=file to use ptlslog_set_fd() * revert the switch to writev(2) discussion: #406 (comment) * let PTLSLOG_CONN respect ptls_skip_tracing() * ptlslog: add "module" field to provide who emits the event * do not emit secret in PTLSLOG * add PTLS_HEXDUMP macro as an easy-to-use handle for ptls_hexdump() * introduce PTLSLOG_ELEMENT_UNSAFESTR() * ptlslog: put JSON's null if a string value is NULL * Revert "ptlslog: put JSON's null if a string value is NULL" This reverts commit a163989. * inline function must be declared as static * do not use PTLS_HEXDUMP; it is not available for VC++ * avoid errors in VC++ * disable wrarnings in VC++ for write(2) * VC++ does not support VLAs * invalidate ptlslog_fd when write(2) fails * fix unsafe string handling in ptlslog * remove debugging code * add PTLSLOG_ELEMENT_HEXDUMP instead of PTLS_ELEMENT that depends on a GCC extesions * allow ptlslog to set multiple fds (up to 8) * remove a redundant ptlslog interface fromn cli.c * reorder options * close fd before invalidating it * fix the return value of ptlslog_add_fd * ptlslog: records num of lost events * Fix compile warnings about unchecked returns in t/cli * Fix Visual Studio warnings when using OpenSSL 3.0 * symbols defined in util.h do not have `ptls_` prefix * first argument is an expression * fix the image to use ubuntu 22.04 * Add ticket functions which use OpenSSL v3 EVP_MAC_CTX This change adds the following new ticket functions which use OpenSSL v3 EVP_MAC_CTX. HMAC APIs are deprecated in OpenSSL v3. EVP_MAC_CTX is its replacement. - ptls_openssl_encrypt_ticket_evp - ptls_openssl_decrypt_ticket_evp * add failing test * copy correct amount of data _and_ generate new vectors * msvc requires an element * clear GHASH vectors before calling `free` * clang-format * ptlslog: remove the limitation of PTLSLOG_MAXCONN * do not use magic numbers * ptlslog: check ptlslog is active or not at the beginning of PTLSLOG_CONN * inlinize ptlslog_is_active() * No need to call free() because realloc(ptr, 0) frees ptr and returns 0 * move the condition out of PTLSLOG_CONN * add missing `inline` keyword * revert using a label & goto because the label name must not be duplicated in a function * comment * rewrite escape_json_unsafe_string based on picojson.h https://github.com/kazuho/picojson/blob/master/picojson.h#L531 * include pthread.h for Windows * format * ptlslog: introduce bool, and some internal funcs for optimizations * fix comments * use INT32_MIN value * let ptlslog optional (enabled by default on Linux and macOS) * suppress errors for Windows * merge ptlslog.h into picotls.h (like those in pembase64.c), determine availability rather than let it be configurable (like `PTLS_OPENSSL_HAVE_*`) * [xcode] add log.c * do not expose the internals * symbols use `ptls_` as prefix * picotls checks allocation failure; returns PTLS_ERROR_* * partial write is loss * rename before refactor * realloc(0) may return non-NULL, fd_index should not be incremented when removing fd * let's use constant expressions * rather than exposing an internal API used for building strings, promote JSON escape function to a public API * revert changes to wincompat.h as they should be unnecessary now that logging is disabled on windows * this is also unnecessary * eaisest way to maintain compatibility is to not have new files; log.c is small anyways * no need to define as a block (the macro cannot be used as an ordinary statement anyways) * concede to using `snprintf` to support compilers that complain without having the wisdom to check the correctness * [github actions] increase timeout * use the result returned rather than recalculate * better use `PTLS_LOG` assuming that we'd have `QUICLY_LOG` and `H2O_LOG` rather than `QUICLYLOG`, `H2OLOG` * oops * add tls12 cipher awareness * add tls12 ciphers to supported cipher suites instead of using mapping table * leave minicrypto alone * remove new accessors * rename tls12 ciphers but remove them from tls13 list * iana_id is not needed * export `find_cipher_suite` externally as `ptls_find_cipher_suite` in addition export supported tls12 ciphers from openssl backend * make PTLS_OPENSSL_HAVE_CHACHA20_POLY1305 checks consistent Co-authored-by: Kazuho Oku <kazuhooku@gmail.com> * ptls_find_cipher_suite should accept cipher list instead of context * provide separate knob for including / excluding sensitive data * revert unecessary change around alloca() * s/ptlslog_/ptls_log_/ for (semi-)public functions * check `ptls_log.is_active` first, but only once * move `tls12_cipher_suites` to the end to improve source compatibility with existing code that uses positional initialization of a struct * clang-format * declare IANA IDs in the header file, fixes incorrect IANA ID (0x009d) used for `ptls_openssl_tls12_dhe_rsa_aes128gcmsha256` which results in incorrect cipher suite being returned due the value being added to `ptls_openssl_tls12_cipher_suites` prior to `ptls_openssl_tls12_rsa_aes256gcmsha384` * and `.name`s also * fix cipher name (amends #429) * remove ones that are not going to be meaningful as performance optimization * add callback for loading raw public keys (at the moment X25519-only) * implement HPKE (basic mode, x25519-only) * raw private key cannot be loaded in OpenSSL 1.x * some deployments do not have x25519 * `load` should be available on both sides * remove redundant code * [xcode] add files * update bcrypt backend following API changes * MSVC does not like `{}` * add missing include * maybe these are the right names * add casts to suppress unneeded compiler warnings from MSVC Co-authored-by: Kazuho Oku <kazuhooku@gmail.com> Co-authored-by: Goro Fuji <goro@fastly.com> Co-authored-by: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com> Co-authored-by: Joe Calderon <jcalderon@fastly.com> Co-authored-by: Joe Calderon <sleepybishop@users.noreply.github.com> * Compile with visual studio * use #define and sizeof consistency * hpke definitions cannot depend on picotls.h, because ECH which is part of ptls_context_t has to depend on hpke * add sha512 implementation * add tests * oops * HKDF used for DH and AEAD can be different * now that we have cipher-suite type for HPKE, revert changes to hash and aead * let's not change the API just for testing; we can load key from PEM files * add test vectors of p256, sha512 * typo * let backends provide list, hpke test known combinations * RFC 9180 uses the same encoding for NIST curves as RFC 8446 does (i.e., uncompressed form) * dedicated type for ID, omit `h` as that is internal * id itself does not have to be const * it works for NIST curves too * size of shared secret is `kem->hash->digest_size` (RFC 9180 section 4) * do not clear sender pubkey when succeeding * clang-format * go back to the old way hard-coding the prefix * ci: add CIFuzz Github action Signed-off-by: David Korczynski <david@adalogics.com> * add test vectors for multi-shot AEAD * [xcode] suppress build warning * `ptls_decode8` for consistency * here also * switch to a table * split SH and HRR of the table, so as to align with that of RFC 8446 * we support one more extension * suppress warning on Xcode Signed-off-by: David Korczynski <david@adalogics.com> Co-authored-by: Kazuho Oku <kazuhooku@gmail.com> Co-authored-by: Goro Fuji <goro@fastly.com> Co-authored-by: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com> Co-authored-by: Joe Calderon <jcalderon@fastly.com> Co-authored-by: Joe Calderon <sleepybishop@users.noreply.github.com> Co-authored-by: David Korczynski <david@adalogics.com>
* 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>
* 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>
|
Sorry for the bad merges. I was using the built-in editor to fix merge conflicts, and doing so I managed to erase a continuation mark in the ECH TEST macro. Fixed now. Really sorry for the mixup. |
kazuho
added a commit
that referenced
this pull request
Dec 4, 2022
|
Thank you for the PR. Merged in the commit above. |
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.
Fix compile on Visual studio: