From 669cb1e6595e6b855919fed407b1fe3fd98dd371 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Tue, 26 Mar 2024 09:30:02 -0400 Subject: [PATCH 1/7] Add a PrivacyInfo plist file See https://developer.apple.com/documentation/bundleresources/privacy_manifest_files Change-Id: I12e485ce294ead6a6cc16018e7e2adbb1efeddec Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67487 Auto-Submit: Adam Langley Reviewed-by: David Benjamin Commit-Queue: Adam Langley (cherry picked from commit a2ef200d79158613f5a312eb5a13ddceae518bfb) --- PrivacyInfo.xcprivacy | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 PrivacyInfo.xcprivacy diff --git a/PrivacyInfo.xcprivacy b/PrivacyInfo.xcprivacy new file mode 100644 index 0000000000..dc3a906b2f --- /dev/null +++ b/PrivacyInfo.xcprivacy @@ -0,0 +1,21 @@ + + + + + + + + NSPrivacyTracking + + NSPrivacyTrackingDomains + + NSPrivacyCollectedDataTypes + + NSPrivacyAccessedAPITypes + + + From aea3d7295dfddfc2b208bf2b4380531ab98d45e9 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 29 Mar 2024 17:01:25 -0400 Subject: [PATCH 2/7] Switch EVP_CIPHERs to C99 initializers We're using it in parts of EVP already and this is much more readable. Change-Id: I42f30b83331cafdabd4f5d995b61176458e906bc Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67567 Auto-Submit: David Benjamin Commit-Queue: David Benjamin Reviewed-by: Adam Langley Commit-Queue: Adam Langley (cherry picked from commit b171749734c438994526096c2150e9baf9d87313) --- crypto/cipher_extra/e_des.c | 108 +++++++++++++--------------- crypto/cipher_extra/e_null.c | 10 ++- crypto/cipher_extra/e_rc2.c | 46 ++++++------ crypto/cipher_extra/e_rc4.c | 12 ++-- crypto/decrepit/blowfish/blowfish.c | 39 ++++++---- crypto/decrepit/cast/cast.c | 26 ++++--- 6 files changed, 123 insertions(+), 118 deletions(-) diff --git a/crypto/cipher_extra/e_des.c b/crypto/cipher_extra/e_des.c index baf0be0b8e..62ac2a03a6 100644 --- a/crypto/cipher_extra/e_des.c +++ b/crypto/cipher_extra/e_des.c @@ -85,16 +85,14 @@ static int des_cbc_cipher(EVP_CIPHER_CTX *ctx, uint8_t *out, const uint8_t *in, } static const EVP_CIPHER evp_des_cbc = { - /* nid = */ NID_des_cbc, - /* block_size = */ 8, - /* key_len = */ 8, - /* iv_len = */ 8, - /* ctx_size = */ sizeof(EVP_DES_KEY), - /* flags = */ EVP_CIPH_CBC_MODE, - /* init = */ des_init_key, - /* cipher = */ des_cbc_cipher, - /* cleanup = */ NULL, - /* ctrl = */ NULL, + .nid = NID_des_cbc, + .block_size = 8, + .key_len = 8, + .iv_len = 8, + .ctx_size = sizeof(EVP_DES_KEY), + .flags = EVP_CIPH_CBC_MODE, + .init = des_init_key, + .cipher = des_cbc_cipher, }; const EVP_CIPHER *EVP_des_cbc(void) { return &evp_des_cbc; } @@ -114,16 +112,14 @@ static int des_ecb_cipher(EVP_CIPHER_CTX *ctx, uint8_t *out, const uint8_t *in, } static const EVP_CIPHER evp_des_ecb = { - /* nid = */ NID_des_ecb, - /* block_size = */ 8, - /* key_len = */ 8, - /* iv_len = */ 0, - /* ctx_size = */ sizeof(EVP_DES_KEY), - /* flags = */ EVP_CIPH_ECB_MODE, - /* init = */ des_init_key, - /* cipher = */ des_ecb_cipher, - /* cleanup = */ NULL, - /* ctrl = */ NULL, + .nid = NID_des_ecb, + .block_size = 8, + .key_len = 8, + .iv_len = 0, + .ctx_size = sizeof(EVP_DES_KEY), + .flags = EVP_CIPH_ECB_MODE, + .init = des_init_key, + .cipher = des_ecb_cipher, }; const EVP_CIPHER *EVP_des_ecb(void) { return &evp_des_ecb; } @@ -153,16 +149,14 @@ static int des_ede3_cbc_cipher(EVP_CIPHER_CTX *ctx, uint8_t *out, } static const EVP_CIPHER evp_des_ede3_cbc = { - /* nid = */ NID_des_ede3_cbc, - /* block_size = */ 8, - /* key_len = */ 24, - /* iv_len = */ 8, - /* ctx_size = */ sizeof(DES_EDE_KEY), - /* flags = */ EVP_CIPH_CBC_MODE, - /* init = */ des_ede3_init_key, - /* cipher = */ des_ede3_cbc_cipher, - /* cleanup = */ NULL, - /* ctrl = */ NULL, + .nid = NID_des_ede3_cbc, + .block_size = 8, + .key_len = 24, + .iv_len = 8, + .ctx_size = sizeof(DES_EDE_KEY), + .flags = EVP_CIPH_CBC_MODE, + .init = des_ede3_init_key, + .cipher = des_ede3_cbc_cipher, }; const EVP_CIPHER *EVP_des_ede3_cbc(void) { return &evp_des_ede3_cbc; } @@ -178,16 +172,14 @@ static int des_ede_init_key(EVP_CIPHER_CTX *ctx, const uint8_t *key, } static const EVP_CIPHER evp_des_ede_cbc = { - /* nid = */ NID_des_ede_cbc, - /* block_size = */ 8, - /* key_len = */ 16, - /* iv_len = */ 8, - /* ctx_size = */ sizeof(DES_EDE_KEY), - /* flags = */ EVP_CIPH_CBC_MODE, - /* init = */ des_ede_init_key, - /* cipher = */ des_ede3_cbc_cipher, - /* cleanup = */ NULL, - /* ctrl = */ NULL, + .nid = NID_des_ede_cbc, + .block_size = 8, + .key_len = 16, + .iv_len = 8, + .ctx_size = sizeof(DES_EDE_KEY), + .flags = EVP_CIPH_CBC_MODE, + .init = des_ede_init_key, + .cipher = des_ede3_cbc_cipher, }; const EVP_CIPHER *EVP_des_ede_cbc(void) { return &evp_des_ede_cbc; } @@ -208,31 +200,27 @@ static int des_ede_ecb_cipher(EVP_CIPHER_CTX *ctx, uint8_t *out, } static const EVP_CIPHER evp_des_ede = { - /* nid = */ NID_des_ede_ecb, - /* block_size = */ 8, - /* key_len = */ 16, - /* iv_len = */ 0, - /* ctx_size = */ sizeof(DES_EDE_KEY), - /* flags = */ EVP_CIPH_ECB_MODE, - /* init = */ des_ede_init_key, - /* cipher = */ des_ede_ecb_cipher, - /* cleanup = */ NULL, - /* ctrl = */ NULL, + .nid = NID_des_ede_ecb, + .block_size = 8, + .key_len = 16, + .iv_len = 0, + .ctx_size = sizeof(DES_EDE_KEY), + .flags = EVP_CIPH_ECB_MODE, + .init = des_ede_init_key, + .cipher = des_ede_ecb_cipher, }; const EVP_CIPHER *EVP_des_ede(void) { return &evp_des_ede; } static const EVP_CIPHER evp_des_ede3 = { - /* nid = */ NID_des_ede3_ecb, - /* block_size = */ 8, - /* key_len = */ 24, - /* iv_len = */ 0, - /* ctx_size = */ sizeof(DES_EDE_KEY), - /* flags = */ EVP_CIPH_ECB_MODE, - /* init = */ des_ede3_init_key, - /* cipher = */ des_ede_ecb_cipher, - /* cleanup = */ NULL, - /* ctrl = */ NULL, + .nid = NID_des_ede3_ecb, + .block_size = 8, + .key_len = 24, + .iv_len = 0, + .ctx_size = sizeof(DES_EDE_KEY), + .flags = EVP_CIPH_ECB_MODE, + .init = des_ede3_init_key, + .cipher = des_ede_ecb_cipher, }; const EVP_CIPHER *EVP_des_ede3(void) { return &evp_des_ede3; } diff --git a/crypto/cipher_extra/e_null.c b/crypto/cipher_extra/e_null.c index 10a5c269e2..ad99df924c 100644 --- a/crypto/cipher_extra/e_null.c +++ b/crypto/cipher_extra/e_null.c @@ -78,9 +78,13 @@ static int null_cipher(EVP_CIPHER_CTX *ctx, uint8_t *out, } static const EVP_CIPHER n_cipher = { - NID_undef, 1 /* block size */, 0 /* key_len */, 0 /* iv_len */, - 0 /* ctx_size */, 0 /* flags */, null_init_key, null_cipher, - NULL /* cleanup */, NULL /* ctrl */, + .nid = NID_undef, + .block_size = 1, + .key_len = 0, + .iv_len = 0, + .ctx_size = 0, + .init = null_init_key, + .cipher = null_cipher, }; const EVP_CIPHER *EVP_enc_null(void) { return &n_cipher; } diff --git a/crypto/cipher_extra/e_rc2.c b/crypto/cipher_extra/e_rc2.c index acc99ad8c1..c2a143ebac 100644 --- a/crypto/cipher_extra/e_rc2.c +++ b/crypto/cipher_extra/e_rc2.c @@ -427,35 +427,29 @@ static int rc2_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg, void *ptr) { } static const EVP_CIPHER rc2_40_cbc = { - NID_rc2_40_cbc, - 8 /* block size */, - 5 /* 40 bit */, - 8 /* iv len */, - sizeof(EVP_RC2_KEY), - EVP_CIPH_CBC_MODE | EVP_CIPH_VARIABLE_LENGTH | EVP_CIPH_CTRL_INIT, - rc2_init_key, - rc2_cbc_cipher, - NULL, - rc2_ctrl, + .nid = NID_rc2_40_cbc, + .block_size = 8, + .key_len = 5 /* 40 bit */, + .iv_len = 8, + .ctx_size = sizeof(EVP_RC2_KEY), + .flags = EVP_CIPH_CBC_MODE | EVP_CIPH_VARIABLE_LENGTH | EVP_CIPH_CTRL_INIT, + .init = rc2_init_key, + .cipher = rc2_cbc_cipher, + .ctrl = rc2_ctrl, }; -const EVP_CIPHER *EVP_rc2_40_cbc(void) { - return &rc2_40_cbc; -} +const EVP_CIPHER *EVP_rc2_40_cbc(void) { return &rc2_40_cbc; } static const EVP_CIPHER rc2_cbc = { - NID_rc2_cbc, - 8 /* block size */, - 16 /* 128 bit */, - 8 /* iv len */, - sizeof(EVP_RC2_KEY), - EVP_CIPH_CBC_MODE | EVP_CIPH_VARIABLE_LENGTH | EVP_CIPH_CTRL_INIT, - rc2_init_key, - rc2_cbc_cipher, - NULL, - rc2_ctrl, + .nid = NID_rc2_cbc, + .block_size = 8, + .key_len = 16 /* 128 bit */, + .iv_len = 8, + .ctx_size = sizeof(EVP_RC2_KEY), + .flags = EVP_CIPH_CBC_MODE | EVP_CIPH_VARIABLE_LENGTH | EVP_CIPH_CTRL_INIT, + .init = rc2_init_key, + .cipher = rc2_cbc_cipher, + .ctrl = rc2_ctrl, }; -const EVP_CIPHER *EVP_rc2_cbc(void) { - return &rc2_cbc; -} +const EVP_CIPHER *EVP_rc2_cbc(void) { return &rc2_cbc; } diff --git a/crypto/cipher_extra/e_rc4.c b/crypto/cipher_extra/e_rc4.c index e252ad8dcb..0c3404fd72 100644 --- a/crypto/cipher_extra/e_rc4.c +++ b/crypto/cipher_extra/e_rc4.c @@ -81,10 +81,14 @@ static int rc4_cipher(EVP_CIPHER_CTX *ctx, uint8_t *out, const uint8_t *in, } static const EVP_CIPHER rc4 = { - NID_rc4, 1 /* block_size */, 16 /* key_size */, - 0 /* iv_len */, sizeof(RC4_KEY), EVP_CIPH_VARIABLE_LENGTH, - rc4_init_key, rc4_cipher, NULL /* cleanup */, - NULL /* ctrl */, + .nid = NID_rc4, + .block_size = 1, + .key_len = 16, + .iv_len = 0, + .ctx_size = sizeof(RC4_KEY), + .flags = EVP_CIPH_VARIABLE_LENGTH, + .init = rc4_init_key, + .cipher = rc4_cipher, }; const EVP_CIPHER *EVP_rc4(void) { return &rc4; } diff --git a/crypto/decrepit/blowfish/blowfish.c b/crypto/decrepit/blowfish/blowfish.c index 4e5a00d533..124764e6a1 100644 --- a/crypto/decrepit/blowfish/blowfish.c +++ b/crypto/decrepit/blowfish/blowfish.c @@ -594,27 +594,36 @@ static int bf_cfb_cipher(EVP_CIPHER_CTX *ctx, uint8_t *out, const uint8_t *in, } static const EVP_CIPHER bf_ecb = { - NID_bf_ecb, BF_BLOCK /* block_size */, - 16 /* key_size */, BF_BLOCK /* iv_len */, - sizeof(BF_KEY), EVP_CIPH_ECB_MODE | EVP_CIPH_VARIABLE_LENGTH, - bf_init_key, bf_ecb_cipher, - NULL /* cleanup */, NULL /* ctrl */, + .nid = NID_bf_ecb, + .block_size = BF_BLOCK, + .key_len = 16, + .iv_len = BF_BLOCK, + .ctx_size = sizeof(BF_KEY), + .flags = EVP_CIPH_ECB_MODE | EVP_CIPH_VARIABLE_LENGTH, + .init = bf_init_key, + .cipher = bf_ecb_cipher, }; static const EVP_CIPHER bf_cbc = { - NID_bf_cbc, BF_BLOCK /* block_size */, - 16 /* key_size */, BF_BLOCK /* iv_len */, - sizeof(BF_KEY), EVP_CIPH_CBC_MODE | EVP_CIPH_VARIABLE_LENGTH, - bf_init_key, bf_cbc_cipher, - NULL /* cleanup */, NULL /* ctrl */, + .nid = NID_bf_cbc, + .block_size = BF_BLOCK, + .key_len = 16, + .iv_len = BF_BLOCK, + .ctx_size = sizeof(BF_KEY), + .flags = EVP_CIPH_CBC_MODE | EVP_CIPH_VARIABLE_LENGTH, + .init = bf_init_key, + .cipher = bf_cbc_cipher, }; static const EVP_CIPHER bf_cfb = { - NID_bf_cfb64, 1 /* block_size */, - 16 /* key_size */, BF_BLOCK /* iv_len */, - sizeof(BF_KEY), EVP_CIPH_CFB_MODE | EVP_CIPH_VARIABLE_LENGTH, - bf_init_key, bf_cfb_cipher, - NULL /* cleanup */, NULL /* ctrl */, + .nid = NID_bf_cfb64, + .block_size = 1, + .key_len = 16, + .iv_len = BF_BLOCK, + .ctx_size = sizeof(BF_KEY), + .flags = EVP_CIPH_CFB_MODE | EVP_CIPH_VARIABLE_LENGTH, + .init = bf_init_key, + .cipher = bf_cfb_cipher, }; const EVP_CIPHER *EVP_bf_ecb(void) { return &bf_ecb; } diff --git a/crypto/decrepit/cast/cast.c b/crypto/decrepit/cast/cast.c index 012283ae27..1b80b8f386 100644 --- a/crypto/decrepit/cast/cast.c +++ b/crypto/decrepit/cast/cast.c @@ -384,19 +384,25 @@ static int cast_cbc_cipher(EVP_CIPHER_CTX *ctx, uint8_t *out, const uint8_t *in, } static const EVP_CIPHER cast5_ecb = { - NID_cast5_ecb, CAST_BLOCK, - CAST_KEY_LENGTH, CAST_BLOCK /* iv_len */, - sizeof(CAST_KEY), EVP_CIPH_ECB_MODE | EVP_CIPH_VARIABLE_LENGTH, - cast_init_key, cast_ecb_cipher, - NULL /* cleanup */, NULL /* ctrl */, + .nid = NID_cast5_ecb, + .block_size = CAST_BLOCK, + .key_len = CAST_KEY_LENGTH, + .iv_len = CAST_BLOCK, + .ctx_size = sizeof(CAST_KEY), + .flags = EVP_CIPH_ECB_MODE | EVP_CIPH_VARIABLE_LENGTH, + .init = cast_init_key, + .cipher = cast_ecb_cipher, }; static const EVP_CIPHER cast5_cbc = { - NID_cast5_cbc, CAST_BLOCK, - CAST_KEY_LENGTH, CAST_BLOCK /* iv_len */, - sizeof(CAST_KEY), EVP_CIPH_CBC_MODE | EVP_CIPH_VARIABLE_LENGTH, - cast_init_key, cast_cbc_cipher, - NULL /* cleanup */, NULL /* ctrl */, + .nid = NID_cast5_cbc, + .block_size = CAST_BLOCK, + .key_len = CAST_KEY_LENGTH, + .iv_len = CAST_BLOCK, + .ctx_size = sizeof(CAST_KEY), + .flags = EVP_CIPH_CBC_MODE | EVP_CIPH_VARIABLE_LENGTH, + .init = cast_init_key, + .cipher = cast_cbc_cipher, }; const EVP_CIPHER *EVP_cast5_ecb(void) { return &cast5_ecb; } From ed4a8b0b4e2e7c2f23960b422d10770b286ce95d Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 28 Mar 2024 23:53:10 -0400 Subject: [PATCH 3/7] Document that our Unicode APIs reject noncharacters Noncharacters are weird. They're code points and generally expected to pass through string APIs and such, but they're also not meant to be used for "open interchange". We reject them, while most Unicode APIs accept them. They're public API nowadays, so document this. Change-Id: I56aa436ae954b591d9a00b6560617e1ad5c26d95 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67568 Auto-Submit: David Benjamin Reviewed-by: Adam Langley Commit-Queue: Adam Langley Commit-Queue: David Benjamin (cherry picked from commit 27e09a3277d17718902afca16cce7e2fb9a82ec2) --- crypto/bytestring/unicode.c | 5 +++-- include/openssl/bytestring.h | 27 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/crypto/bytestring/unicode.c b/crypto/bytestring/unicode.c index 6f9467f9c3..244839ea9e 100644 --- a/crypto/bytestring/unicode.c +++ b/crypto/bytestring/unicode.c @@ -18,11 +18,12 @@ static int is_valid_code_point(uint32_t v) { - // References in the following are to Unicode 9.0.0. + // References in the following are to Unicode 15.0.0. if (// The Unicode space runs from zero to 0x10ffff (3.4 D9). v > 0x10ffff || // Values 0x...fffe, 0x...ffff, and 0xfdd0-0xfdef are permanently reserved - // (3.4 D14) + // as noncharacters (3.4 D14). See also 23.7. As our APIs are intended for + // "open interchange", such as ASN.1, we reject them. (v & 0xfffe) == 0xfffe || (v >= 0xfdd0 && v <= 0xfdef) || // Surrogate code points are invalid (3.2 C1). diff --git a/include/openssl/bytestring.h b/include/openssl/bytestring.h index c59bb82dc5..688593b5a3 100644 --- a/include/openssl/bytestring.h +++ b/include/openssl/bytestring.h @@ -645,6 +645,33 @@ OPENSSL_EXPORT int CBB_add_asn1_oid_from_text(CBB *cbb, const char *text, OPENSSL_EXPORT int CBB_flush_asn1_set_of(CBB *cbb); +// Unicode utilities. +// +// These functions consider noncharacters (see section 23.7 from Unicode 15.0.0) +// to be invalid code points and will treat them as an error condition. + +// The following functions read one Unicode code point from |cbs| with the +// corresponding encoding and store it in |*out|. They return one on success and +// zero on error. +OPENSSL_EXPORT int CBS_get_utf8(CBS *cbs, uint32_t *out); +OPENSSL_EXPORT int CBS_get_latin1(CBS *cbs, uint32_t *out); +OPENSSL_EXPORT int CBS_get_ucs2_be(CBS *cbs, uint32_t *out); +OPENSSL_EXPORT int CBS_get_utf32_be(CBS *cbs, uint32_t *out); + +// CBB_get_utf8_len returns the number of bytes needed to represent |u| in +// UTF-8. +OPENSSL_EXPORT size_t CBB_get_utf8_len(uint32_t u); + +// The following functions encode |u| to |cbb| with the corresponding +// encoding. They return one on success and zero on error. Error conditions +// include |u| being an invalid code point, or |u| being unencodable in the +// specified encoding. +OPENSSL_EXPORT int CBB_add_utf8(CBB *cbb, uint32_t u); +OPENSSL_EXPORT int CBB_add_latin1(CBB *cbb, uint32_t u); +OPENSSL_EXPORT int CBB_add_ucs2_be(CBB *cbb, uint32_t u); +OPENSSL_EXPORT int CBB_add_utf32_be(CBB *cbb, uint32_t u); + + #if defined(__cplusplus) } // extern C From 4745fb430149df399f81d7e6c16172e18967dab1 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 29 Mar 2024 18:55:30 -0400 Subject: [PATCH 4/7] Rewrite RAND_enable_fork_unsafe_buffering documentation The docs still describe the old implementation, but our PRNG has changed drastically since then. Change-Id: I51c34833a364a1d6bd70cf5d3b6cfb87b4aa06e7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67569 Commit-Queue: Adam Langley Reviewed-by: Adam Langley Auto-Submit: David Benjamin Commit-Queue: David Benjamin (cherry picked from commit ec6cb3e3a016a8e7ffee42d589d423e6057f21bf) --- include/openssl/rand.h | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/include/openssl/rand.h b/include/openssl/rand.h index fddb890f0c..a450fb5f0e 100644 --- a/include/openssl/rand.h +++ b/include/openssl/rand.h @@ -37,15 +37,25 @@ OPENSSL_EXPORT int RAND_priv_bytes(uint8_t *buf, size_t len); // Obscure functions. #if !defined(OPENSSL_WINDOWS) -// RAND_enable_fork_unsafe_buffering enables efficient buffered reading of -// /dev/urandom. It adds an overhead of a few KB of memory per thread. It must -// be called before the first call to |RAND_bytes|. +// RAND_enable_fork_unsafe_buffering indicates that clones of the address space, +// e.g. via |fork|, will never call into BoringSSL. It may be used to disable +// BoringSSL's more expensive fork-safety measures. However, calling this +// function and then using BoringSSL across |fork| calls will leak secret keys. +// |fd| must be -1. // -// |fd| must be -1. We no longer support setting the file descriptor with this -// function. +// WARNING: This function affects BoringSSL for the entire address space. Thus +// this function should never be called by library code, only by code with +// global knowledge of the application's use of BoringSSL. // -// It has an unusual name because the buffer is unsafe across calls to |fork|. -// Hence, this function should never be called by libraries. +// Do not use this function unless a performance issue was measured with the +// default behavior. BoringSSL can efficiently detect forks on most platforms, +// in which case this function is a no-op and is unnecessary. In particular, +// Linux kernel versions 4.14 or later provide |MADV_WIPEONFORK|. Future +// versions of BoringSSL will remove this functionality when older kernels are +// sufficiently rare. +// +// This function has an unusual name because it historically controlled internal +// buffers, but no longer does. OPENSSL_EXPORT void RAND_enable_fork_unsafe_buffering(int fd); #endif From 7e62341fd34c1385ebdbfd0a6b41b58d4d0987b1 Mon Sep 17 00:00:00 2001 From: Brian Ledger Date: Tue, 2 Apr 2024 15:17:16 -0400 Subject: [PATCH 5/7] Disable `-Wcast-function-type-strict` for `BORINGSSL_DEFINE_STACK_OF_IMPL.` Change-Id: I8340cf9259de72792a01049ecc36997233887006 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67607 Commit-Queue: David Benjamin Reviewed-by: David Benjamin (cherry picked from commit 68c6fd8943ffba4e5054ff3a9befa8882b6b226a) --- include/openssl/base.h | 7 +++++++ include/openssl/stack.h | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/include/openssl/base.h b/include/openssl/base.h index d08bf1a95d..1d26ac9182 100644 --- a/include/openssl/base.h +++ b/include/openssl/base.h @@ -199,6 +199,13 @@ extern "C" { #define OPENSSL_PRINTF_FORMAT_FUNC(string_index, first_to_check) #endif +// OPENSSL_CLANG_PRAGMA emits a pragma on clang and nothing on other compilers. +#if defined(__clang__) +#define OPENSSL_CLANG_PRAGMA(arg) _Pragma(arg) +#else +#define OPENSSL_CLANG_PRAGMA(arg) +#endif + // OPENSSL_MSVC_PRAGMA emits a pragma on MSVC and nothing on other compilers. #if defined(_MSC_VER) #define OPENSSL_MSVC_PRAGMA(arg) __pragma(arg) diff --git a/include/openssl/stack.h b/include/openssl/stack.h index 7ff03fe48b..beb3c2dcce 100644 --- a/include/openssl/stack.h +++ b/include/openssl/stack.h @@ -416,6 +416,9 @@ BSSL_NAMESPACE_END * positive warning. */ \ OPENSSL_MSVC_PRAGMA(warning(push)) \ OPENSSL_MSVC_PRAGMA(warning(disable : 4191)) \ + OPENSSL_CLANG_PRAGMA("clang diagnostic push") \ + OPENSSL_CLANG_PRAGMA("clang diagnostic ignored \"-Wunknown-warning-option\"") \ + OPENSSL_CLANG_PRAGMA("clang diagnostic ignored \"-Wcast-function-type-strict\"") \ \ DECLARE_STACK_OF(name) \ \ @@ -569,6 +572,7 @@ BSSL_NAMESPACE_END (OPENSSL_sk_free_func)free_func); \ } \ \ + OPENSSL_CLANG_PRAGMA("clang diagnostic pop") \ OPENSSL_MSVC_PRAGMA(warning(pop)) From a41b35df6123179643715ee3bdd46984c1cb4c7e Mon Sep 17 00:00:00 2001 From: Nick Harper Date: Thu, 28 Mar 2024 14:35:17 -0400 Subject: [PATCH 6/7] Increase DTLS window size from 64 to 256 Bug: chromium:40925630 Change-Id: Ide72960600747f5ce9a9213a9103510fee3e3806 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67527 Reviewed-by: David Benjamin Commit-Queue: David Benjamin (cherry picked from commit f94f3ed3965ea033001fb9ae006084eee408b861) --- ssl/dtls_record.cc | 10 +++++----- ssl/internal.h | 7 ++++--- ssl/test/runner/runner.go | 10 +++++++--- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/ssl/dtls_record.cc b/ssl/dtls_record.cc index 992fb526b3..26819f165d 100644 --- a/ssl/dtls_record.cc +++ b/ssl/dtls_record.cc @@ -139,14 +139,14 @@ static uint64_t to_u64_be(const uint8_t in[8]) { // |bitmap| or is stale. Otherwise it returns zero. static bool dtls1_bitmap_should_discard(DTLS1_BITMAP *bitmap, const uint8_t seq_num[8]) { - const unsigned kWindowSize = sizeof(bitmap->map) * 8; + const size_t kWindowSize = bitmap->map.size(); uint64_t seq_num_u = to_u64_be(seq_num); if (seq_num_u > bitmap->max_seq_num) { return false; } uint64_t idx = bitmap->max_seq_num - seq_num_u; - return idx >= kWindowSize || (bitmap->map & (((uint64_t)1) << idx)); + return idx >= kWindowSize || bitmap->map[idx]; } // dtls1_bitmap_record updates |bitmap| to record receipt of sequence number @@ -154,14 +154,14 @@ static bool dtls1_bitmap_should_discard(DTLS1_BITMAP *bitmap, // this function on a stale sequence number. static void dtls1_bitmap_record(DTLS1_BITMAP *bitmap, const uint8_t seq_num[8]) { - const unsigned kWindowSize = sizeof(bitmap->map) * 8; + const size_t kWindowSize = bitmap->map.size(); uint64_t seq_num_u = to_u64_be(seq_num); // Shift the window if necessary. if (seq_num_u > bitmap->max_seq_num) { uint64_t shift = seq_num_u - bitmap->max_seq_num; if (shift >= kWindowSize) { - bitmap->map = 0; + bitmap->map.reset(); } else { bitmap->map <<= shift; } @@ -170,7 +170,7 @@ static void dtls1_bitmap_record(DTLS1_BITMAP *bitmap, uint64_t idx = bitmap->max_seq_num - seq_num_u; if (idx < kWindowSize) { - bitmap->map |= ((uint64_t)1) << idx; + bitmap->map[idx] = true; } } diff --git a/ssl/internal.h b/ssl/internal.h index 7e3b21fd57..717759a54c 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -147,6 +147,7 @@ #include #include +#include #include #include #include @@ -994,9 +995,9 @@ class SSLAEADContext { // DTLS1_BITMAP maintains a sliding window of 64 sequence numbers to detect // replayed packets. It should be initialized by zeroing every field. struct DTLS1_BITMAP { - // map is a bit mask of the last 64 sequence numbers. Bit - // |1< map; // max_seq_num is the largest sequence number seen so far as a 64-bit // integer. uint64_t max_seq_num = 0; diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index c5705c5e37..16d1fda2ec 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -10004,7 +10004,7 @@ func addDTLSReplayTests() { config: Config{ Bugs: ProtocolBugs{ SequenceNumberMapping: func(in uint64) uint64 { - return in * 127 + return in * 1023 }, }, }, @@ -10019,11 +10019,15 @@ func addDTLSReplayTests() { config: Config{ Bugs: ProtocolBugs{ SequenceNumberMapping: func(in uint64) uint64 { - return in ^ 31 + // This mapping has numbers counting backwards in groups + // of 256, and then jumping forwards 511 numbers. + return in ^ 255 }, }, }, - messageCount: 200, + // This messageCount is large enough to make sure that the SequenceNumberMapping + // will reach the point where it jumps forwards after stepping backwards. + messageCount: 500, replayWrites: true, }) } From f4a486465f1c5f5248baedd4cb7a5cb5141a7ddd Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 20 Nov 2023 15:11:02 -0500 Subject: [PATCH 7/7] Avoid strdup in crypto/err/err.c This makes me sad, but strdup may be more trouble than is worth it? Being not in C (until C23) and only a (by POSIX standards) recent addition to POSIX means a lot of folks seem to make it unnecessarily hard to use: - MSVC adds a deprecation warning that we have to suppress - glibc gates it on feature macros; we just don't notice because we already have to work around their bad behavior for pthread_rwlock - musl gates it on feature macros, which was one of the things that tripped cl/583161936 Given we only want to use strdup in one file (err.c, which wants to avoid OPENSSL_malloc), a small reimplementation is probably not the end of the world. While I'm here, we can actually make OPENSSL_strdup's implementation a little simpler. Change-Id: I4e6c743b3104a67357d7d527c178c615de6bc844 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64047 Reviewed-by: Bob Beck Commit-Queue: David Benjamin (cherry picked from commit 89f097740e6376521926eb56a61b25f639c473ac) --- crypto/err/err.c | 23 +++++++++++++---------- crypto/mem.c | 9 ++------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/crypto/err/err.c b/crypto/err/err.c index 8741402818..8687c738de 100644 --- a/crypto/err/err.c +++ b/crypto/err/err.c @@ -178,6 +178,17 @@ extern const uint32_t kOpenSSLReasonValues[]; extern const size_t kOpenSSLReasonValuesLen; extern const char kOpenSSLReasonStringData[]; +static char *strdup_libc_malloc(const char *str) { + // |strdup| is not in C until C23, so MSVC triggers deprecation warnings, and + // glibc and musl gate it on a feature macro. Reimplementing it is easier. + size_t len = strlen(str); + char *ret = malloc(len + 1); + if (ret != NULL) { + memcpy(ret, str, len + 1); + } + return ret; +} + // err_clear clears the given queued error. static void err_clear(struct err_error_st *error) { free(error->data); @@ -188,13 +199,9 @@ static void err_copy(struct err_error_st *dst, const struct err_error_st *src) { err_clear(dst); dst->file = src->file; if (src->data != NULL) { - // Disable deprecated functions on msvc so it doesn't complain about strdup. - OPENSSL_MSVC_PRAGMA(warning(push)) - OPENSSL_MSVC_PRAGMA(warning(disable : 4996)) // We can't use OPENSSL_strdup because we don't want to call OPENSSL_malloc, // which can affect the error stack. - dst->data = strdup(src->data); - OPENSSL_MSVC_PRAGMA(warning(pop)) + dst->data = strdup_libc_malloc(src->data); } dst->packed = src->packed; dst->line = src->line; @@ -766,13 +773,9 @@ void ERR_set_error_data(char *data, int flags) { assert(0); return; } - // Disable deprecated functions on msvc so it doesn't complain about strdup. - OPENSSL_MSVC_PRAGMA(warning(push)) - OPENSSL_MSVC_PRAGMA(warning(disable : 4996)) // We can not use OPENSSL_strdup because we don't want to call OPENSSL_malloc, // which can affect the error stack. - char *copy = strdup(data); - OPENSSL_MSVC_PRAGMA(warning(pop)) + char *copy = strdup_libc_malloc(data); if (copy != NULL) { err_set_error_data(copy); } diff --git a/crypto/mem.c b/crypto/mem.c index 6bf94151b3..2490358ce4 100644 --- a/crypto/mem.c +++ b/crypto/mem.c @@ -379,13 +379,8 @@ char *OPENSSL_strdup(const char *s) { if (s == NULL) { return NULL; } - const size_t len = strlen(s) + 1; - char *ret = OPENSSL_malloc(len); - if (ret == NULL) { - return NULL; - } - OPENSSL_memcpy(ret, s, len); - return ret; + // Copy the NUL terminator. + return OPENSSL_memdup(s, strlen(s) + 1); } int OPENSSL_isalpha(int c) {