From 9e87594e11a89338795e4b72513193a83359d2fe Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 27 Feb 2023 16:36:24 -0500 Subject: [PATCH 01/15] Fix ninja run_tests https://boringssl-review.googlesource.com/c/boringssl/+/57645 wasn't quite right. The cd to run ssl/test/runner affects the subsequent commands. Fix this by running the Go tests first. They're very fast compared to the others anyway. Change-Id: Id5ea54a9787173eb3ed80e9db2c9ecfe064a93b0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57688 Auto-Submit: David Benjamin Commit-Queue: Bob Beck Commit-Queue: David Benjamin Reviewed-by: Bob Beck (cherry picked from commit e3a5face899e16183f1d207d7327baac57454935) --- CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2d6de163b4..8e3ed51ce8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -952,18 +952,18 @@ if(BUILD_TESTING) add_custom_target( run_tests + COMMAND ${GO_EXECUTABLE} test ${GO_TESTS} COMMAND ${GO_EXECUTABLE} run util/all_tests.go -build-dir ${PROJECT_BINARY_DIR} - COMMAND ${GO_EXECUTABLE} test ${GO_TESTS} WORKING_DIRECTORY ${PROJECT_SOURCE_DIR} DEPENDS all_tests run_ssl_runner_tests ${MAYBE_USES_TERMINAL}) else() add_custom_target( run_tests + COMMAND ${GO_EXECUTABLE} test ${GO_TESTS} COMMAND ${GO_EXECUTABLE} run util/all_tests.go -build-dir ${PROJECT_BINARY_DIR} -ssl-tests=false - COMMAND ${GO_EXECUTABLE} test ${GO_TESTS} WORKING_DIRECTORY ${PROJECT_SOURCE_DIR} DEPENDS all_tests fips_specific_tests_if_any ${MAYBE_USES_TERMINAL} From 1df42cc56e33b8dbedfb164afe6412c0799cd85c Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 27 Feb 2023 15:28:30 -0500 Subject: [PATCH 02/15] Use the common location for CMake in the bots Follow-up to https://chromium-review.googlesource.com/c/chromium/tools/build/+/4296194/ But mostly I've since added a lot more configurations to CI and want to make sure I haven't broken anything. Change-Id: I627711356004bf2244bd729b6055e9e0e619724e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57691 Commit-Queue: Bob Beck Auto-Submit: David Benjamin Reviewed-by: Bob Beck (cherry picked from commit f88b7c83979d128fa83eb5f9102be56cc4bec33c) --- .gitignore | 4 +--- util/bot/DEPS | 25 +------------------------ 2 files changed, 2 insertions(+), 27 deletions(-) diff --git a/.gitignore b/.gitignore index 26b63cccb2..0754f0a585 100644 --- a/.gitignore +++ b/.gitignore @@ -17,9 +17,7 @@ bindings/rust/generate/target util/bot/android_ndk util/bot/android_sdk/public -util/bot/cmake-linux64 -util/bot/cmake-mac -util/bot/cmake-win32 +util/bot/cmake util/bot/golang util/bot/libFuzzer util/bot/libcxx diff --git a/util/bot/DEPS b/util/bot/DEPS index 20eed07eda..777f7c301a 100644 --- a/util/bot/DEPS +++ b/util/bot/DEPS @@ -56,34 +56,11 @@ deps = { 'dep_type': 'cipd', }, - # TODO(davidben): Merge the three CMake directories. Now that we use CIPD, - # which supports a ${{platform}} marker, there is nothing platform-specific - # about this anymore. However, the recipe still expects CMake to be found at - # these directories, so this needs to be coordinated with a change there. - 'boringssl/util/bot/cmake-linux64': { + 'boringssl/util/bot/cmake': { 'packages': [{ 'package': 'infra/3pp/tools/cmake/${{platform}}', 'version': Var('cmake_version'), }], - 'condition': 'host_os == "linux"', - 'dep_type': 'cipd', - }, - - 'boringssl/util/bot/cmake-mac': { - 'packages': [{ - 'package': 'infra/3pp/tools/cmake/${{platform}}', - 'version': Var('cmake_version'), - }], - 'condition': 'host_os == "mac"', - 'dep_type': 'cipd', - }, - - 'boringssl/util/bot/cmake-win32': { - 'packages': [{ - 'package': 'infra/3pp/tools/cmake/${{platform}}', - 'version': Var('cmake_version'), - }], - 'condition': 'host_os == "win"', 'dep_type': 'cipd', }, From 8341f23ef106e9f2731cbf2b5d8a1b46a0e363bb Mon Sep 17 00:00:00 2001 From: Bob Beck Date: Thu, 2 Mar 2023 09:02:54 -0700 Subject: [PATCH 03/15] Fix use of unitialized cbb on failure case. This made fido2's fuzzer angry: https://buganizer.corp.google.com/issues/271220905 Change-Id: Ib1b909be10f230df2daea3942f35cba0a81dcedb Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57765 Commit-Queue: Bob Beck Commit-Queue: David Benjamin Auto-Submit: Bob Beck Reviewed-by: David Benjamin (cherry picked from commit e06f172bf22c098719d0d9b970f839b39dcd41ce) --- crypto/asn1/a_mbstr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crypto/asn1/a_mbstr.c b/crypto/asn1/a_mbstr.c index 80cfac44e0..85a7b98a74 100644 --- a/crypto/asn1/a_mbstr.c +++ b/crypto/asn1/a_mbstr.c @@ -222,6 +222,8 @@ int ASN1_mbstring_ncopy(ASN1_STRING **out, const unsigned char *in, int len, } } + CBB cbb; + CBB_zero(&cbb); // If both the same type just copy across if (inform == outform) { if (!ASN1_STRING_set(dest, in, len)) { @@ -231,8 +233,6 @@ int ASN1_mbstring_ncopy(ASN1_STRING **out, const unsigned char *in, int len, *out = dest; return str_type; } - - CBB cbb; if (!CBB_init(&cbb, size_estimate + 1)) { goto err; } From f81204a85412721a320e2f2ee5da7b046b22c031 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 2 Mar 2023 12:24:58 -0500 Subject: [PATCH 04/15] Add a few more assertions to compiler_test.cc. Also turn assertions into static_assert where we can. These should be no-ops with existing assertions. The int assertion is tighter, but we already assert this in constant_time_declassify_int. We cannot support 64-bit int because it messes up integer promotion rules. Change-Id: I628d2d7decdfa8bc01d8c6013bc7c20f927d63b1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57785 Reviewed-by: Adam Langley Auto-Submit: David Benjamin Commit-Queue: Adam Langley (cherry picked from commit 93e8d4463d59d671e9c5c6171226341f04b07907) --- crypto/compiler_test.cc | 46 ++++++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/crypto/compiler_test.cc b/crypto/compiler_test.cc index adef5ee747..2c81462d87 100644 --- a/crypto/compiler_test.cc +++ b/crypto/compiler_test.cc @@ -58,27 +58,49 @@ static void CheckRepresentation(T value) { } TEST(CompilerTest, IntegerRepresentation) { - EXPECT_EQ(8, CHAR_BIT); - EXPECT_EQ(0xff, static_cast(UCHAR_MAX)); + static_assert(CHAR_BIT == 8, "BoringSSL only supports 8-bit chars"); + static_assert(UCHAR_MAX == 0xff, "BoringSSL only supports 8-bit chars"); - // uint8_t is assumed to be unsigned char. I.e., casting to uint8_t should be - // as good as unsigned char for strict aliasing purposes. + // Require that |unsigned char| and |uint8_t| be the same type. We require + // that type-punning through |uint8_t| is not a strict aliasing violation. In + // principle, type-punning should be done with |memcpy|, which would make this + // moot. + // + // However, C made too many historical mistakes with the types and signedness + // of character strings. As a result, aliasing between all variations on 8-bit + // chars are a practical necessity for all real C code. We do not support + // toolchains that break this assumption. + static_assert( + std::is_same::value, + "BoringSSL requires uint8_t and unsigned char be the same type"); uint8_t u8 = 0; unsigned char *ptr = &u8; (void)ptr; // Sized integers have the expected size. - EXPECT_EQ(1u, sizeof(uint8_t)); - EXPECT_EQ(2u, sizeof(uint16_t)); - EXPECT_EQ(4u, sizeof(uint32_t)); - EXPECT_EQ(8u, sizeof(uint64_t)); + static_assert(sizeof(uint8_t) == 1u, "uint8_t has the wrong size"); + static_assert(sizeof(uint16_t) == 2u, "uint16_t has the wrong size"); + static_assert(sizeof(uint32_t) == 4u, "uint32_t has the wrong size"); + static_assert(sizeof(uint64_t) == 8u, "uint64_t has the wrong size"); // size_t does not exceed uint64_t. - EXPECT_LE(sizeof(size_t), 8u); + static_assert(sizeof(size_t) <= 8u, "size_t must not exceed uint64_t"); - // int must be 32-bit or larger. - EXPECT_LE(0x7fffffff, INT_MAX); - EXPECT_LE(0xffffffffu, UINT_MAX); + // Require that |int| be exactly 32 bits. OpenSSL historically mixed up + // |unsigned| and |uint32_t|, so we require it be at least 32 bits. Requiring + // at most 32-bits is a bit more subtle. C promotes arithemetic operands to + // |int| when they fit. But this means, if |int| is 2N bits wide, multiplying + // two maximum-sized |uintN_t|s is undefined by integer overflow! + // + // We attempt to handle this for |uint16_t|, assuming a 32-bit |int|, but we + // make no attempts to correct for this with |uint32_t| for a 64-bit |int|. + // Thus BoringSSL does not support ILP64 platforms. + // + // This test is on |INT_MAX| and |INT32_MAX| rather than sizeof because it is + // theoretically allowed for sizeof(int) to be 4 but include padding bits. + static_assert(INT_MAX == INT32_MAX, "BoringSSL requires int be 32-bit"); + static_assert(UINT_MAX == UINT32_MAX, + "BoringSSL requires unsigned be 32-bit"); CheckRepresentation(static_cast(127)); CheckRepresentation(static_cast(1)); From 2f55cf36eda864ab9562bf495170915d3a56474f Mon Sep 17 00:00:00 2001 From: Theo Buehler Date: Sun, 5 Mar 2023 23:41:50 +0100 Subject: [PATCH 05/15] Plug a leak in ASN1_item_i2d() ASN1_item_ex_i2d() does not take ownership of the memory pointed at by *out, so it's the caller's responsibility to free it on error. Change-Id: Id8cb70e50f280944418629a32b53fd4ca248b0bd Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57805 Commit-Queue: David Benjamin Reviewed-by: David Benjamin (cherry picked from commit 3a7dfdb984434a4b4beef947b2e49602c557c0de) --- crypto/asn1/tasn_enc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/crypto/asn1/tasn_enc.c b/crypto/asn1/tasn_enc.c index 4dd0653555..8b1a0e3a11 100644 --- a/crypto/asn1/tasn_enc.c +++ b/crypto/asn1/tasn_enc.c @@ -97,6 +97,7 @@ int ASN1_item_i2d(ASN1_VALUE *val, unsigned char **out, const ASN1_ITEM *it) { p = buf; int len2 = ASN1_item_ex_i2d(&val, &p, it, /*tag=*/-1, /*aclass=*/0); if (len2 <= 0) { + OPENSSL_free(buf); return len2; } assert(len == len2); From de14e2f762138ff106c2dc0e65eb1ab7a1a339b7 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 6 Mar 2023 14:25:39 -0500 Subject: [PATCH 06/15] Work around a NASM bug I did not have "find a bug in the assembler" on my bingo card today, but here we are. NASM 2.15, prior to 2.15.04, has a bug where, if a section that already exists is referenced again with alignment qualifiers, it incorrect adds padding and mangles the output. See https://bugzilla.nasm.us/show_bug.cgi?id=3392701. Work around this by suppressing the perlasm-emitted qualifiers the second time a section is emitted. We likely don't need these qualifiers because, for all sections we care about, NASM's defaults are fine, but perlasm tries to align .text more aggressively than the default, so let it do that. Bug: chromium:1422018 Change-Id: Iade5702c139b70772d4957a83c8f9be86c8af97c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57825 Reviewed-by: Adam Langley Commit-Queue: David Benjamin (cherry picked from commit abb9af83bc223eca0ffffce246ed551f2fcd11e1) --- crypto/perlasm/x86_64-xlate.pl | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/crypto/perlasm/x86_64-xlate.pl b/crypto/perlasm/x86_64-xlate.pl index b8802c586a..16a7846835 100755 --- a/crypto/perlasm/x86_64-xlate.pl +++ b/crypto/perlasm/x86_64-xlate.pl @@ -1029,6 +1029,27 @@ } } { package directive; # pick up directives, which start with . + my %sections; + sub nasm_section { + my ($name, $qualifiers) = @_; + my $ret = "section\t$name"; + if (exists $sections{$name}) { + # Work around https://bugzilla.nasm.us/show_bug.cgi?id=3392701. Only + # emit section qualifiers the first time a section is referenced. + # For all subsequent references, require the qualifiers match and + # omit them. + # + # See also https://crbug.com/1422018 and b/270643835. + my $old = $sections{$name}; + die "Inconsistent qualifiers: $qualifiers vs $old" if ($qualifiers ne "" && $qualifiers ne $old); + } else { + $sections{$name} = $qualifiers; + if ($qualifiers ne "") { + $ret .= " $qualifiers"; + } + } + return $ret; + } sub re { my ($class, $line) = @_; my $self = {}; @@ -1137,7 +1158,7 @@ SWITCH: for ($dir) { /\.text/ && do { my $v=undef; if ($nasm) { - $v="section .text code align=64\n"; + $v=nasm_section(".text", "code align=64")."\n"; } else { $v="$current_segment\tENDS\n" if ($current_segment); $current_segment = ".text\$"; @@ -1150,7 +1171,7 @@ }; /\.data/ && do { my $v=undef; if ($nasm) { - $v="section .data data align=8\n"; + $v=nasm_section(".data", "data align=8")."\n"; } else { $v="$current_segment\tENDS\n" if ($current_segment); $current_segment = "_DATA"; @@ -1164,13 +1185,14 @@ $$line = ".CRT\$XCU" if ($$line eq ".init"); $$line = ".rdata" if ($$line eq ".rodata"); if ($nasm) { - $v="section $$line"; + my $qualifiers = ""; if ($$line=~/\.([prx])data/) { - $v.=" rdata align="; - $v.=$1 eq "p"? 4 : 8; + $qualifiers = "rdata align="; + $qualifiers .= $1 eq "p"? 4 : 8; } elsif ($$line=~/\.CRT\$/i) { - $v.=" rdata align=8"; + $qualifiers = "rdata align=8"; } + $v = nasm_section($$line, $qualifiers); } else { $v="$current_segment\tENDS\n" if ($current_segment); $v.="$$line\tSEGMENT"; From 129ee9e2f73d94b8e2ea10bd5e6be8c42ebd4fb2 Mon Sep 17 00:00:00 2001 From: Pete Bentley Date: Thu, 25 Aug 2022 15:32:33 +0100 Subject: [PATCH 07/15] test_fips: flush stdout before aborting on failure. When running on a device via `adb shell`, stdout will be a pipe and so is block buffered, leading to lost output if abort() is called before flushing. Change-Id: Ica67132fb8b2b1e7967df89fa3d0a9a793d8cbbf Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/54025 Reviewed-by: Adam Langley Reviewed-by: Bob Beck Commit-Queue: Bob Beck (cherry picked from commit 082e953a134ad423a00b8859f9daf5708e729260) --- util/fipstools/test_fips.c | 1 + 1 file changed, 1 insertion(+) diff --git a/util/fipstools/test_fips.c b/util/fipstools/test_fips.c index 0664267088..a05d19d32a 100644 --- a/util/fipstools/test_fips.c +++ b/util/fipstools/test_fips.c @@ -447,5 +447,6 @@ int main(int argc, char **argv) { err: printf("FAIL\n"); + fflush(stdout); abort(); } From 64afe9b41b7c41112e7849344c5b4a926a015668 Mon Sep 17 00:00:00 2001 From: Ilya Tokar Date: Mon, 6 Mar 2023 16:20:44 -0500 Subject: [PATCH 08/15] Add prefetch to sha1_block_data_order_shaext MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Similar idea to https://boringssl-review.googlesource.com/c/boringssl/+/55466 Results are pretty close to the current state, e.g. tool speed goes from Did 74000 SHA-1 (16384 bytes) operations in 1004094us (73698.3 ops/sec): 1207.5 MB/s to Did 75000 SHA-1 (16384 bytes) operations in 1004022us (74699.6 ops/sec): 1223.9 MB/s But on AMD with prefetchers disabled and large enough data size, to force cache misses this gives ~3x improvement: name old time/op new time/op delta BM_SHA1Hash/2 141ns ± 1% 143ns ± 2% ~ (p=0.421 n=5+5) BM_SHA1Hash/4 143ns ± 2% 143ns ± 3% ~ (p=0.841 n=5+5) BM_SHA1Hash/8 141ns ± 1% 141ns ± 2% ~ (p=1.000 n=5+5) BM_SHA1Hash/16 141ns ± 1% 141ns ± 1% ~ (p=0.841 n=5+5) BM_SHA1Hash/32 143ns ± 2% 143ns ± 1% ~ (p=0.690 n=5+5) BM_SHA1Hash/64 178ns ± 1% 179ns ± 1% ~ (p=0.151 n=5+5) BM_SHA1Hash/512 454ns ± 1% 454ns ± 1% ~ (p=0.841 n=5+5) BM_SHA1Hash/4k 2.66µs ± 1% 2.65µs ± 1% ~ (p=1.000 n=5+5) BM_SHA1Hash/32k 20.3µs ± 1% 20.3µs ± 2% ~ (p=1.000 n=5+5) BM_SHA1Hash/256k 162µs ± 1% 161µs ± 1% ~ (p=0.548 n=5+5) BM_SHA1Hash/1M 644µs ± 1% 645µs ± 1% ~ (p=0.841 n=5+5) BM_SHA1Hash/2M 1.29ms ± 1% 1.29ms ± 2% ~ (p=0.690 n=5+5) BM_SHA1Hash/4M 2.58ms ± 1% 2.58ms ± 1% ~ (p=0.841 n=5+5) BM_SHA1Hash/8M 5.14ms ± 0% 5.15ms ± 1% ~ (p=0.286 n=4+5) BM_SHA1Hash/16M 11.4ms ± 3% 10.3ms ± 1% -9.04% (p=0.016 n=4+5) BM_SHA1Hash/128M 249ms ± 0% 83ms ± 1% -66.73% (p=0.008 n=5+5) Change-Id: I7cae746b6d8a705d6bf2d5c5df6a2dca6d44791a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57826 Commit-Queue: Adam Langley Reviewed-by: Adam Langley (cherry picked from commit ecb722aeeb7ec6fcd2d6c60d177b9e952eab51f8) --- crypto/fipsmodule/sha/asm/sha1-x86_64.pl | 1 + 1 file changed, 1 insertion(+) diff --git a/crypto/fipsmodule/sha/asm/sha1-x86_64.pl b/crypto/fipsmodule/sha/asm/sha1-x86_64.pl index 368e3fb1a1..60891143ce 100755 --- a/crypto/fipsmodule/sha/asm/sha1-x86_64.pl +++ b/crypto/fipsmodule/sha/asm/sha1-x86_64.pl @@ -393,6 +393,7 @@ sub BODY_40_59 { lea 0x40($inp),%r8 # next input block paddd @MSG[0],$E cmovne %r8,$inp + prefetcht0 512($inp) movdqa $ABCD,$ABCD_SAVE # offload $ABCD ___ for($i=0;$i<20-4;$i+=2) { From c82490992722fa5435c65c7a830736dc0e47c501 Mon Sep 17 00:00:00 2001 From: Robert Liu Date: Thu, 9 Mar 2023 15:50:41 +0000 Subject: [PATCH 09/15] Add OPENSSL_EXPORT to BN_mod_inverse_blinded Change-Id: Ie7543047c6f489ae849b3c27703948f0977c33fd Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57905 Commit-Queue: David Benjamin Reviewed-by: David Benjamin (cherry picked from commit 8aa51ddfcf1fbf2e5f976762657e21c7aee2f922) --- include/openssl/bn.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/openssl/bn.h b/include/openssl/bn.h index acde58d297..1934a29d45 100644 --- a/include/openssl/bn.h +++ b/include/openssl/bn.h @@ -846,8 +846,9 @@ OPENSSL_EXPORT BIGNUM *BN_mod_inverse(BIGNUM *out, const BIGNUM *a, // Note this function may incorrectly report |a| has no inverse if the random // blinding value has no inverse. It should only be used when |n| has few // non-invertible elements, such as an RSA modulus. -int BN_mod_inverse_blinded(BIGNUM *out, int *out_no_inverse, const BIGNUM *a, - const BN_MONT_CTX *mont, BN_CTX *ctx); +OPENSSL_EXPORT int BN_mod_inverse_blinded(BIGNUM *out, int *out_no_inverse, + const BIGNUM *a, + const BN_MONT_CTX *mont, BN_CTX *ctx); // BN_mod_inverse_odd sets |out| equal to |a|^-1, mod |n|. |a| must be // non-negative and must be less than |n|. |n| must be odd. This function From 1acfe0d28c7d9ab1235c6c279de80bbed4c6bff0 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 14 Mar 2023 17:42:04 -0400 Subject: [PATCH 10/15] Test that X509_NAMEs sort their RDNs when encoding. RDNs are a SET OF attributes which means they should be sorted by DER encoding length, then lexicographically. We didn't have any test coverage for this. Bug: 548 Change-Id: I542196aae26984aeee4f1c6774878b121675b0dc Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58025 Commit-Queue: Bob Beck Reviewed-by: Bob Beck Auto-Submit: David Benjamin (cherry picked from commit d0cff637a25b8323578729a01575b62001967bc8) --- crypto/x509/x509_test.cc | 80 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc index 609dde5397..91499b7aca 100644 --- a/crypto/x509/x509_test.cc +++ b/crypto/x509/x509_test.cc @@ -6093,3 +6093,83 @@ TEST(X509Test, AddUnserializableExtension) { ASSERT_TRUE(X509_EXTENSION_set_object(ext.get(), OBJ_nid2obj(NID_undef))); EXPECT_FALSE(X509_add_ext(x509.get(), ext.get(), /*loc=*/-1)); } + +// Test that, when constructing an |X509_NAME|, names are sorted by DER order. +TEST(X509Test, SortRDN) { + bssl::UniquePtr name(X509_NAME_new()); + ASSERT_TRUE(name); + + auto append_entry_new_rdn = [&](const char *str) { + return X509_NAME_add_entry_by_NID(name.get(), NID_commonName, MBSTRING_ASC, + reinterpret_cast(str), + strlen(str), /*loc=*/-1, /*set=*/0); + }; + auto append_entry_prev_rdn = [&](const char *str) { + return X509_NAME_add_entry_by_NID(name.get(), NID_commonName, MBSTRING_ASC, + reinterpret_cast(str), + strlen(str), /*loc=*/-1, /*set=*/-1); + }; + + // This is the sort order to expect. + ASSERT_TRUE(append_entry_new_rdn("A")); + ASSERT_TRUE(append_entry_prev_rdn("B")); + ASSERT_TRUE(append_entry_prev_rdn("AA")); + ASSERT_TRUE(append_entry_prev_rdn("AB")); + + // The same RDN, with entries added in a different order. + ASSERT_TRUE(append_entry_new_rdn("AB")); + ASSERT_TRUE(append_entry_prev_rdn("AA")); + ASSERT_TRUE(append_entry_prev_rdn("B")); + ASSERT_TRUE(append_entry_prev_rdn("A")); + + // The same RDN, with entries added in a different order. + ASSERT_TRUE(append_entry_new_rdn("A")); + ASSERT_TRUE(append_entry_prev_rdn("AA")); + ASSERT_TRUE(append_entry_prev_rdn("B")); + ASSERT_TRUE(append_entry_prev_rdn("AB")); + + uint8_t *der = nullptr; + int der_len = i2d_X509_NAME(name.get(), &der); + ASSERT_GT(der_len, 0); + bssl::UniquePtr free_der(der); + + // SEQUENCE { + // SET { + // SEQUENCE { + // # commonName + // OBJECT_IDENTIFIER { 2.5.4.3 } + // UTF8String { "A" } + // } + // SEQUENCE { + // # commonName + // OBJECT_IDENTIFIER { 2.5.4.3 } + // UTF8String { "B" } + // } + // SEQUENCE { + // # commonName + // OBJECT_IDENTIFIER { 2.5.4.3 } + // UTF8String { "AA" } + // } + // SEQUENCE { + // # commonName + // OBJECT_IDENTIFIER { 2.5.4.3 } + // UTF8String { "AB" } + // } + // } + // ...two more copies of the above SET... + // } + static uint8_t kExpected[] = { + 0x30, 0x81, 0x84, 0x31, 0x2a, 0x30, 0x08, 0x06, 0x03, 0x55, 0x04, 0x03, + 0x0c, 0x01, 0x41, 0x30, 0x08, 0x06, 0x03, 0x55, 0x04, 0x03, 0x0c, 0x01, + 0x42, 0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x03, 0x0c, 0x02, 0x41, 0x41, + 0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x03, 0x0c, 0x02, 0x41, 0x42, 0x31, + 0x2a, 0x30, 0x08, 0x06, 0x03, 0x55, 0x04, 0x03, 0x0c, 0x01, 0x41, 0x30, + 0x08, 0x06, 0x03, 0x55, 0x04, 0x03, 0x0c, 0x01, 0x42, 0x30, 0x09, 0x06, + 0x03, 0x55, 0x04, 0x03, 0x0c, 0x02, 0x41, 0x41, 0x30, 0x09, 0x06, 0x03, + 0x55, 0x04, 0x03, 0x0c, 0x02, 0x41, 0x42, 0x31, 0x2a, 0x30, 0x08, 0x06, + 0x03, 0x55, 0x04, 0x03, 0x0c, 0x01, 0x41, 0x30, 0x08, 0x06, 0x03, 0x55, + 0x04, 0x03, 0x0c, 0x01, 0x42, 0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x03, + 0x0c, 0x02, 0x41, 0x41, 0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x03, 0x0c, + 0x02, 0x41, 0x42}; + EXPECT_EQ(Bytes(kExpected), Bytes(der, der_len)); +} From 3445f4c81a4cdbf92b2ba9f0d6d6f701f8f11e72 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 14 Mar 2023 17:42:16 -0400 Subject: [PATCH 11/15] Const-correct and document a few functions in x509v3.h. Change-Id: I59bcacf10a59ffdf9709785727f5f8b73c992f6e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58026 Auto-Submit: David Benjamin Commit-Queue: Bob Beck Reviewed-by: Bob Beck (cherry picked from commit 8c7aa6bfcd7573d7b904fde6acb4f3652a3ebecc) --- crypto/x509v3/v3_alt.c | 17 ++++++++--------- crypto/x509v3/v3_genn.c | 2 +- include/openssl/x509v3.h | 18 ++++++++++++++---- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/crypto/x509v3/v3_alt.c b/crypto/x509v3/v3_alt.c index 858ef4d13b..ddd112a2eb 100644 --- a/crypto/x509v3/v3_alt.c +++ b/crypto/x509v3/v3_alt.c @@ -97,11 +97,11 @@ const X509V3_EXT_METHOD v3_alt[] = { }; STACK_OF(CONF_VALUE) *i2v_GENERAL_NAMES(const X509V3_EXT_METHOD *method, - GENERAL_NAMES *gens, + const GENERAL_NAMES *gens, STACK_OF(CONF_VALUE) *ret) { int ret_was_null = ret == NULL; for (size_t i = 0; i < sk_GENERAL_NAME_num(gens); i++) { - GENERAL_NAME *gen = sk_GENERAL_NAME_value(gens, i); + const GENERAL_NAME *gen = sk_GENERAL_NAME_value(gens, i); STACK_OF(CONF_VALUE) *tmp = i2v_GENERAL_NAME(method, gen, ret); if (tmp == NULL) { if (ret_was_null) { @@ -118,7 +118,7 @@ STACK_OF(CONF_VALUE) *i2v_GENERAL_NAMES(const X509V3_EXT_METHOD *method, } STACK_OF(CONF_VALUE) *i2v_GENERAL_NAME(const X509V3_EXT_METHOD *method, - GENERAL_NAME *gen, + const GENERAL_NAME *gen, STACK_OF(CONF_VALUE) *ret) { // Note the error-handling for this function relies on there being at most // one |X509V3_add_value| call. If there were two and the second failed, we @@ -207,9 +207,7 @@ STACK_OF(CONF_VALUE) *i2v_GENERAL_NAME(const X509V3_EXT_METHOD *method, return ret; } -int GENERAL_NAME_print(BIO *out, GENERAL_NAME *gen) { - unsigned char *p; - int i; +int GENERAL_NAME_print(BIO *out, const GENERAL_NAME *gen) { switch (gen->type) { case GEN_OTHERNAME: BIO_printf(out, "othername:"); @@ -244,13 +242,13 @@ int GENERAL_NAME_print(BIO *out, GENERAL_NAME *gen) { X509_NAME_print_ex(out, gen->d.dirn, 0, XN_FLAG_ONELINE); break; - case GEN_IPADD: - p = gen->d.ip->data; + case GEN_IPADD: { + const unsigned char *p = gen->d.ip->data; if (gen->d.ip->length == 4) { BIO_printf(out, "IP Address:%d.%d.%d.%d", p[0], p[1], p[2], p[3]); } else if (gen->d.ip->length == 16) { BIO_printf(out, "IP Address"); - for (i = 0; i < 8; i++) { + for (int i = 0; i < 8; i++) { uint16_t v = ((uint16_t)p[0] << 8) | p[1]; BIO_printf(out, ":%X", v); p += 2; @@ -261,6 +259,7 @@ int GENERAL_NAME_print(BIO *out, GENERAL_NAME *gen) { break; } break; + } case GEN_RID: BIO_printf(out, "Registered ID"); diff --git a/crypto/x509v3/v3_genn.c b/crypto/x509v3/v3_genn.c index d593727b0d..609c5dae4f 100644 --- a/crypto/x509v3/v3_genn.c +++ b/crypto/x509v3/v3_genn.c @@ -124,7 +124,7 @@ static int edipartyname_cmp(const EDIPARTYNAME *a, const EDIPARTYNAME *b) { } // Returns 0 if they are equal, != 0 otherwise. -static int othername_cmp(OTHERNAME *a, OTHERNAME *b) { +static int othername_cmp(const OTHERNAME *a, const OTHERNAME *b) { int result = -1; if (!a || !b) { diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h index bab5ee998f..73cdd75070 100644 --- a/include/openssl/x509v3.h +++ b/include/openssl/x509v3.h @@ -421,9 +421,15 @@ OPENSSL_EXPORT GENERAL_NAME *GENERAL_NAME_dup(GENERAL_NAME *a); // human-readable print functions. If extracting a SAN list from a certificate, // look at |gen| directly. OPENSSL_EXPORT STACK_OF(CONF_VALUE) *i2v_GENERAL_NAME( - const X509V3_EXT_METHOD *method, GENERAL_NAME *gen, + const X509V3_EXT_METHOD *method, const GENERAL_NAME *gen, STACK_OF(CONF_VALUE) *ret); -OPENSSL_EXPORT int GENERAL_NAME_print(BIO *out, GENERAL_NAME *gen); + +// GENERAL_NAME_print prints a human-readable representation of |gen| to |out|. +// It returns one on success and zero on error. +// +// TODO(davidben): Actually, it just returns one and doesn't check for I/O or +// allocation errors. But it should return zero on error. +OPENSSL_EXPORT int GENERAL_NAME_print(BIO *out, const GENERAL_NAME *gen); // TODO(https://crbug.com/boringssl/407): This is not const because it contains // an |X509_NAME|. @@ -439,7 +445,7 @@ DECLARE_ASN1_FUNCTIONS(GENERAL_NAMES) // human-readable print functions. If extracting a SAN list from a certificate, // look at |gen| directly. OPENSSL_EXPORT STACK_OF(CONF_VALUE) *i2v_GENERAL_NAMES( - const X509V3_EXT_METHOD *method, GENERAL_NAMES *gen, + const X509V3_EXT_METHOD *method, const GENERAL_NAMES *gen, STACK_OF(CONF_VALUE) *extlist); OPENSSL_EXPORT GENERAL_NAMES *v2i_GENERAL_NAMES( const X509V3_EXT_METHOD *method, const X509V3_CTX *ctx, @@ -457,8 +463,12 @@ OPENSSL_EXPORT int GENERAL_NAME_get0_otherName(const GENERAL_NAME *gen, ASN1_OBJECT **poid, ASN1_TYPE **pvalue); +// i2s_ASN1_OCTET_STRING returns a human-readable representation of |oct| as a +// newly-allocated, NUL-terminated string, or NULL on error. |method| is +// ignored. The caller must release the result with |OPENSSL_free| when done. OPENSSL_EXPORT char *i2s_ASN1_OCTET_STRING(const X509V3_EXT_METHOD *method, - const ASN1_OCTET_STRING *ia5); + const ASN1_OCTET_STRING *oct); + OPENSSL_EXPORT ASN1_OCTET_STRING *s2i_ASN1_OCTET_STRING( const X509V3_EXT_METHOD *method, const X509V3_CTX *ctx, const char *str); From ad737f9b62277e629eea8c1226aed8358c080265 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Wed, 15 Mar 2023 18:41:09 -0400 Subject: [PATCH 12/15] Add CTRDBG_STATE to bssl::UniquePtr Change-Id: I18596751776262be09d8ba09ed258e1f66d90654 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58046 Reviewed-by: Adam Langley Commit-Queue: David Benjamin (cherry picked from commit 74646566e93de7551bfdfc5f49de7462f13d1d05) --- crypto/fipsmodule/rand/ctrdrbg_test.cc | 6 ++---- include/openssl/ctrdrbg.h | 6 ++++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/crypto/fipsmodule/rand/ctrdrbg_test.cc b/crypto/fipsmodule/rand/ctrdrbg_test.cc index e6ebbca2eb..50f42973f0 100644 --- a/crypto/fipsmodule/rand/ctrdrbg_test.cc +++ b/crypto/fipsmodule/rand/ctrdrbg_test.cc @@ -63,13 +63,11 @@ TEST(CTRDRBGTest, Basic) { TEST(CTRDRBGTest, Allocated) { const uint8_t kSeed[CTR_DRBG_ENTROPY_LEN] = {0}; - CTR_DRBG_STATE *allocated = CTR_DRBG_new(kSeed, nullptr, 0); + bssl::UniquePtr allocated(CTR_DRBG_new(kSeed, nullptr, 0)); ASSERT_TRUE(allocated); - CTR_DRBG_free(allocated); - allocated = CTR_DRBG_new(kSeed, nullptr, 1<<20); + allocated.reset(CTR_DRBG_new(kSeed, nullptr, 1<<20)); ASSERT_FALSE(allocated); - CTR_DRBG_free(allocated); } TEST(CTRDRBGTest, Large) { diff --git a/include/openssl/ctrdrbg.h b/include/openssl/ctrdrbg.h index 62afe0c18e..5440fb4d14 100644 --- a/include/openssl/ctrdrbg.h +++ b/include/openssl/ctrdrbg.h @@ -71,6 +71,12 @@ OPENSSL_EXPORT void CTR_DRBG_clear(CTR_DRBG_STATE *drbg); #if defined(__cplusplus) } // extern C + +extern "C++" { +BSSL_NAMESPACE_BEGIN +BORINGSSL_MAKE_DELETER(CTR_DRBG_STATE, CTR_DRBG_free) +BSSL_NAMESPACE_END +} // extern C++ #endif #endif // OPENSSL_HEADER_CTRDRBG_H From c67c1aca3d1fc07ca04c8db0875b99c46dd73b4e Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sun, 12 Mar 2023 13:10:29 -0400 Subject: [PATCH 13/15] Add a test for OPTIONAL CHOICE values An in-progress rewrite of tasn_dec.c accidentally broke this, so add a regression test. Bug: 548 Change-Id: Iac6a23acbc08459187c96a2f6471f0aa97d445a1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58125 Auto-Submit: David Benjamin Reviewed-by: Bob Beck Commit-Queue: Bob Beck (cherry picked from commit 6a231e5c6e2b299da0be16e1edbb0195265afec3) --- crypto/asn1/asn1_test.cc | 62 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/crypto/asn1/asn1_test.cc b/crypto/asn1/asn1_test.cc index 985098aaad..1aebc78452 100644 --- a/crypto/asn1/asn1_test.cc +++ b/crypto/asn1/asn1_test.cc @@ -2575,4 +2575,66 @@ TEST(ASN1Test, DoublyTagged) { TestSerialize(obj.get(), i2d_DOUBLY_TAGGED, kTrueEmpty); } +#define CHOICE_TYPE_OCT 0 +#define CHOICE_TYPE_BOOL 1 + +struct CHOICE_TYPE { + int type; + union { + ASN1_OCTET_STRING *oct; + ASN1_BOOLEAN b; + } value; +}; + +DECLARE_ASN1_FUNCTIONS(CHOICE_TYPE) +ASN1_CHOICE(CHOICE_TYPE) = { + ASN1_SIMPLE(CHOICE_TYPE, value.oct, ASN1_OCTET_STRING), + ASN1_SIMPLE(CHOICE_TYPE, value.b, ASN1_BOOLEAN), +} ASN1_CHOICE_END(CHOICE_TYPE) +IMPLEMENT_ASN1_FUNCTIONS(CHOICE_TYPE) + +struct OPTIONAL_CHOICE { + CHOICE_TYPE *choice; +}; + +DECLARE_ASN1_FUNCTIONS(OPTIONAL_CHOICE) +ASN1_SEQUENCE(OPTIONAL_CHOICE) = { + ASN1_OPT(OPTIONAL_CHOICE, choice, CHOICE_TYPE), +} ASN1_SEQUENCE_END(OPTIONAL_CHOICE) +IMPLEMENT_ASN1_FUNCTIONS(OPTIONAL_CHOICE) + +TEST(ASN1Test, OptionalChoice) { + std::unique_ptr obj( + nullptr, OPTIONAL_CHOICE_free); + + // Value omitted. + static const uint8_t kOmitted[] = {0x30, 0x00}; + const uint8_t *inp = kOmitted; + obj.reset(d2i_OPTIONAL_CHOICE(nullptr, &inp, sizeof(kOmitted))); + ASSERT_TRUE(obj); + EXPECT_FALSE(obj->choice); + TestSerialize(obj.get(), i2d_OPTIONAL_CHOICE, kOmitted); + + // Value is present as an OCTET STRING. + static const uint8_t kOct[] = {0x30, 0x02, 0x04, 0x00}; + inp = kOct; + obj.reset(d2i_OPTIONAL_CHOICE(nullptr, &inp, sizeof(kOct))); + ASSERT_TRUE(obj); + ASSERT_TRUE(obj->choice); + ASSERT_EQ(obj->choice->type, CHOICE_TYPE_OCT); + ASSERT_TRUE(obj->choice->value.oct); + EXPECT_EQ(ASN1_STRING_length(obj->choice->value.oct), 0); + TestSerialize(obj.get(), i2d_OPTIONAL_CHOICE, kOct); + + // Value is present as TRUE. + static const uint8_t kTrue[] = {0x30, 0x03, 0x01, 0x01, 0xff}; + inp = kTrue; + obj.reset(d2i_OPTIONAL_CHOICE(nullptr, &inp, sizeof(kTrue))); + ASSERT_TRUE(obj); + ASSERT_TRUE(obj->choice); + ASSERT_EQ(obj->choice->type, CHOICE_TYPE_BOOL); + EXPECT_EQ(obj->choice->value.b, ASN1_BOOLEAN_TRUE); + TestSerialize(obj.get(), i2d_OPTIONAL_CHOICE, kTrue); +} + #endif // !WINDOWS || !SHARED_LIBRARY From 7c2b740c2b6ae4cdee26d33fe8c89250f34c3081 Mon Sep 17 00:00:00 2001 From: samuel40791765 Date: Fri, 5 May 2023 22:07:36 +0000 Subject: [PATCH 14/15] Update build files in generated-src --- generated-src/linux-x86_64/crypto/fipsmodule/sha1-x86_64.S | 1 + generated-src/mac-x86_64/crypto/fipsmodule/sha1-x86_64.S | 1 + generated-src/win-x86_64/crypto/chacha/chacha-x86_64.asm | 2 +- .../crypto/cipher_extra/chacha20_poly1305_x86_64.asm | 2 +- .../win-x86_64/crypto/fipsmodule/aesni-gcm-x86_64.asm | 2 +- generated-src/win-x86_64/crypto/fipsmodule/aesni-x86_64.asm | 2 +- .../win-x86_64/crypto/fipsmodule/ghash-ssse3-x86_64.asm | 2 +- generated-src/win-x86_64/crypto/fipsmodule/ghash-x86_64.asm | 2 +- generated-src/win-x86_64/crypto/fipsmodule/p256-x86_64-asm.asm | 2 +- generated-src/win-x86_64/crypto/fipsmodule/rsaz-avx2.asm | 2 +- generated-src/win-x86_64/crypto/fipsmodule/sha1-x86_64.asm | 3 ++- generated-src/win-x86_64/crypto/fipsmodule/sha256-x86_64.asm | 2 +- generated-src/win-x86_64/crypto/fipsmodule/sha512-x86_64.asm | 2 +- generated-src/win-x86_64/crypto/fipsmodule/vpaes-x86_64.asm | 2 +- generated-src/win-x86_64/crypto/fipsmodule/x86_64-mont5.asm | 2 +- 15 files changed, 16 insertions(+), 13 deletions(-) diff --git a/generated-src/linux-x86_64/crypto/fipsmodule/sha1-x86_64.S b/generated-src/linux-x86_64/crypto/fipsmodule/sha1-x86_64.S index a8674bc705..f9c7250de4 100644 --- a/generated-src/linux-x86_64/crypto/fipsmodule/sha1-x86_64.S +++ b/generated-src/linux-x86_64/crypto/fipsmodule/sha1-x86_64.S @@ -1299,6 +1299,7 @@ _shaext_shortcut: leaq 64(%rsi),%r8 paddd %xmm4,%xmm1 cmovneq %r8,%rsi + prefetcht0 512(%rsi) movdqa %xmm0,%xmm8 .byte 15,56,201,229 movdqa %xmm0,%xmm2 diff --git a/generated-src/mac-x86_64/crypto/fipsmodule/sha1-x86_64.S b/generated-src/mac-x86_64/crypto/fipsmodule/sha1-x86_64.S index 872d87973a..2a6a692872 100644 --- a/generated-src/mac-x86_64/crypto/fipsmodule/sha1-x86_64.S +++ b/generated-src/mac-x86_64/crypto/fipsmodule/sha1-x86_64.S @@ -1298,6 +1298,7 @@ L$oop_shaext: leaq 64(%rsi),%r8 paddd %xmm4,%xmm1 cmovneq %r8,%rsi + prefetcht0 512(%rsi) movdqa %xmm0,%xmm8 .byte 15,56,201,229 movdqa %xmm0,%xmm2 diff --git a/generated-src/win-x86_64/crypto/chacha/chacha-x86_64.asm b/generated-src/win-x86_64/crypto/chacha/chacha-x86_64.asm index 62d82f60c4..994b787fb2 100644 --- a/generated-src/win-x86_64/crypto/chacha/chacha-x86_64.asm +++ b/generated-src/win-x86_64/crypto/chacha/chacha-x86_64.asm @@ -49,7 +49,7 @@ $L$sixteen: DB 95,54,52,44,32,67,82,89,80,84,79,71,65,77,83,32 DB 98,121,32,60,97,112,112,114,111,64,111,112,101,110,115,115 DB 108,46,111,114,103,62,0 -section .text code align=64 +section .text global ChaCha20_ctr32 diff --git a/generated-src/win-x86_64/crypto/cipher_extra/chacha20_poly1305_x86_64.asm b/generated-src/win-x86_64/crypto/cipher_extra/chacha20_poly1305_x86_64.asm index b97b03b85b..527762a5ba 100644 --- a/generated-src/win-x86_64/crypto/cipher_extra/chacha20_poly1305_x86_64.asm +++ b/generated-src/win-x86_64/crypto/cipher_extra/chacha20_poly1305_x86_64.asm @@ -54,7 +54,7 @@ $L$and_masks: DB 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x00,0x00 DB 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x00 DB 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff -section .text code align=64 +section .text diff --git a/generated-src/win-x86_64/crypto/fipsmodule/aesni-gcm-x86_64.asm b/generated-src/win-x86_64/crypto/fipsmodule/aesni-gcm-x86_64.asm index bdbdd7f9ea..67c43c9406 100644 --- a/generated-src/win-x86_64/crypto/fipsmodule/aesni-gcm-x86_64.asm +++ b/generated-src/win-x86_64/crypto/fipsmodule/aesni-gcm-x86_64.asm @@ -953,7 +953,7 @@ $L$one_lsb: DB 89,80,84,79,71,65,77,83,32,98,121,32,60,97,112,112 DB 114,111,64,111,112,101,110,115,115,108,46,111,114,103,62,0 ALIGN 64 -section .text code align=64 +section .text section .pdata rdata align=4 ALIGN 4 diff --git a/generated-src/win-x86_64/crypto/fipsmodule/aesni-x86_64.asm b/generated-src/win-x86_64/crypto/fipsmodule/aesni-x86_64.asm index 891101019c..2f410fea07 100644 --- a/generated-src/win-x86_64/crypto/fipsmodule/aesni-x86_64.asm +++ b/generated-src/win-x86_64/crypto/fipsmodule/aesni-x86_64.asm @@ -3648,7 +3648,7 @@ $L$key_rcon1b: DB 32,98,121,32,60,97,112,112,114,111,64,111,112,101,110,115 DB 115,108,46,111,114,103,62,0 ALIGN 64 -section .text code align=64 +section .text EXTERN __imp_RtlVirtualUnwind diff --git a/generated-src/win-x86_64/crypto/fipsmodule/ghash-ssse3-x86_64.asm b/generated-src/win-x86_64/crypto/fipsmodule/ghash-ssse3-x86_64.asm index dc9be6d01f..b3d54e6ae1 100644 --- a/generated-src/win-x86_64/crypto/fipsmodule/ghash-ssse3-x86_64.asm +++ b/generated-src/win-x86_64/crypto/fipsmodule/ghash-ssse3-x86_64.asm @@ -443,7 +443,7 @@ $L$reverse_bytes: $L$low4_mask: DQ 0x0f0f0f0f0f0f0f0f,0x0f0f0f0f0f0f0f0f -section .text code align=64 +section .text section .pdata rdata align=4 ALIGN 4 diff --git a/generated-src/win-x86_64/crypto/fipsmodule/ghash-x86_64.asm b/generated-src/win-x86_64/crypto/fipsmodule/ghash-x86_64.asm index 511ab24117..b2323ee628 100644 --- a/generated-src/win-x86_64/crypto/fipsmodule/ghash-x86_64.asm +++ b/generated-src/win-x86_64/crypto/fipsmodule/ghash-x86_64.asm @@ -1208,7 +1208,7 @@ ALIGN 64 DB 60,97,112,112,114,111,64,111,112,101,110,115,115,108,46,111 DB 114,103,62,0 ALIGN 64 -section .text code align=64 +section .text section .pdata rdata align=4 ALIGN 4 diff --git a/generated-src/win-x86_64/crypto/fipsmodule/p256-x86_64-asm.asm b/generated-src/win-x86_64/crypto/fipsmodule/p256-x86_64-asm.asm index b8e84bd535..73f536ea35 100644 --- a/generated-src/win-x86_64/crypto/fipsmodule/p256-x86_64-asm.asm +++ b/generated-src/win-x86_64/crypto/fipsmodule/p256-x86_64-asm.asm @@ -34,7 +34,7 @@ $L$ord: DQ 0xf3b9cac2fc632551,0xbce6faada7179e84,0xffffffffffffffff,0xffffffff00000000 $L$ordK: DQ 0xccd1c8aaee00bc4f -section .text code align=64 +section .text diff --git a/generated-src/win-x86_64/crypto/fipsmodule/rsaz-avx2.asm b/generated-src/win-x86_64/crypto/fipsmodule/rsaz-avx2.asm index f012cb0ab0..fd41206e65 100644 --- a/generated-src/win-x86_64/crypto/fipsmodule/rsaz-avx2.asm +++ b/generated-src/win-x86_64/crypto/fipsmodule/rsaz-avx2.asm @@ -1837,7 +1837,7 @@ $L$inc: DD 2,2,2,2,3,3,3,3 DD 4,4,4,4,4,4,4,4 ALIGN 64 -section .text code align=64 +section .text EXTERN __imp_RtlVirtualUnwind diff --git a/generated-src/win-x86_64/crypto/fipsmodule/sha1-x86_64.asm b/generated-src/win-x86_64/crypto/fipsmodule/sha1-x86_64.asm index d94e83eca9..c842eee83c 100644 --- a/generated-src/win-x86_64/crypto/fipsmodule/sha1-x86_64.asm +++ b/generated-src/win-x86_64/crypto/fipsmodule/sha1-x86_64.asm @@ -1323,6 +1323,7 @@ $L$oop_shaext: lea r8,[64+rsi] paddd xmm1,xmm4 cmovne rsi,r8 + prefetcht0 [512+rsi] movdqa xmm8,xmm0 DB 15,56,201,229 movdqa xmm2,xmm0 @@ -5574,7 +5575,7 @@ K_XX_XX: DB 97,112,112,114,111,64,111,112,101,110,115,115,108,46,111,114 DB 103,62,0 ALIGN 64 -section .text code align=64 +section .text EXTERN __imp_RtlVirtualUnwind diff --git a/generated-src/win-x86_64/crypto/fipsmodule/sha256-x86_64.asm b/generated-src/win-x86_64/crypto/fipsmodule/sha256-x86_64.asm index 78436934a3..fc7c3ff99e 100644 --- a/generated-src/win-x86_64/crypto/fipsmodule/sha256-x86_64.asm +++ b/generated-src/win-x86_64/crypto/fipsmodule/sha256-x86_64.asm @@ -1797,7 +1797,7 @@ K256: DB 52,44,32,67,82,89,80,84,79,71,65,77,83,32,98,121 DB 32,60,97,112,112,114,111,64,111,112,101,110,115,115,108,46 DB 111,114,103,62,0 -section .text code align=64 +section .text ALIGN 64 diff --git a/generated-src/win-x86_64/crypto/fipsmodule/sha512-x86_64.asm b/generated-src/win-x86_64/crypto/fipsmodule/sha512-x86_64.asm index 6e6b24ccdc..a85813c0c8 100644 --- a/generated-src/win-x86_64/crypto/fipsmodule/sha512-x86_64.asm +++ b/generated-src/win-x86_64/crypto/fipsmodule/sha512-x86_64.asm @@ -1837,7 +1837,7 @@ K512: DB 52,44,32,67,82,89,80,84,79,71,65,77,83,32,98,121 DB 32,60,97,112,112,114,111,64,111,112,101,110,115,115,108,46 DB 111,114,103,62,0 -section .text code align=64 +section .text ALIGN 64 diff --git a/generated-src/win-x86_64/crypto/fipsmodule/vpaes-x86_64.asm b/generated-src/win-x86_64/crypto/fipsmodule/vpaes-x86_64.asm index 06f408cad1..593ab402dc 100644 --- a/generated-src/win-x86_64/crypto/fipsmodule/vpaes-x86_64.asm +++ b/generated-src/win-x86_64/crypto/fipsmodule/vpaes-x86_64.asm @@ -1341,7 +1341,7 @@ $L$ctr_add_two: DB 85,110,105,118,101,114,115,105,116,121,41,0 ALIGN 64 -section .text code align=64 +section .text EXTERN __imp_RtlVirtualUnwind diff --git a/generated-src/win-x86_64/crypto/fipsmodule/x86_64-mont5.asm b/generated-src/win-x86_64/crypto/fipsmodule/x86_64-mont5.asm index 83a4701f80..8583bda113 100644 --- a/generated-src/win-x86_64/crypto/fipsmodule/x86_64-mont5.asm +++ b/generated-src/win-x86_64/crypto/fipsmodule/x86_64-mont5.asm @@ -3690,7 +3690,7 @@ $L$inc: DB 114,32,120,56,54,95,54,52,44,32,67,82,89,80,84,79 DB 71,65,77,83,32,98,121,32,60,97,112,112,114,111,64,111 DB 112,101,110,115,115,108,46,111,114,103,62,0 -section .text code align=64 +section .text EXTERN __imp_RtlVirtualUnwind From 5671f657895a69b0e98aee88ad3798878480c1ea Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 7 Mar 2023 15:14:00 -0500 Subject: [PATCH 15/15] Remove EVP_PKEY_ASN1_METHOD and EVP_PKEY_METHOD from public headers With EVP_PKEY and EVP_PKEY_CTX opaque, these symbols don't appear in any public APIs anymore. Make them internal, which also opens the door to renaming them: - EVP_PKEY_METHOD is more accurately EVP_PKEY_CTX_METHOD - EVP_PKEY_ASN1_METHOD is more accurately EVP_PKEY_METHOD Or perhaps the split doesn't mean much and we should fold them together. Change-Id: I8a0f7c2e07445dc981c7cef697263e59dba7784e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57885 Commit-Queue: David Benjamin Auto-Submit: David Benjamin Reviewed-by: Bob Beck Commit-Queue: Bob Beck (cherry picked from commit a925c220c123af0bdd49be3a8a84a506584c1fb2) --- crypto/evp_extra/internal.h | 1 + crypto/fipsmodule/evp/internal.h | 3 +++ include/openssl/base.h | 2 -- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/crypto/evp_extra/internal.h b/crypto/evp_extra/internal.h index 6d82267b1a..26c823b40d 100644 --- a/crypto/evp_extra/internal.h +++ b/crypto/evp_extra/internal.h @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 OR ISC #include +#include "../fipsmodule/evp/internal.h" typedef struct { // key is the concatenation of the private seed and public key. It is stored diff --git a/crypto/fipsmodule/evp/internal.h b/crypto/fipsmodule/evp/internal.h index ba18ee644e..1e7abb8277 100644 --- a/crypto/fipsmodule/evp/internal.h +++ b/crypto/fipsmodule/evp/internal.h @@ -70,6 +70,9 @@ extern "C" { // |EVP_MD_CTX_set_pkey_ctx|. #define EVP_MD_CTX_FLAG_KEEP_PKEY_CTX 0x0400 +typedef struct evp_pkey_asn1_method_st EVP_PKEY_ASN1_METHOD; +typedef struct evp_pkey_method_st EVP_PKEY_METHOD; + struct evp_pkey_asn1_method_st { int pkey_id; uint8_t oid[11]; diff --git a/include/openssl/base.h b/include/openssl/base.h index 1421ed3279..250578b69b 100644 --- a/include/openssl/base.h +++ b/include/openssl/base.h @@ -431,9 +431,7 @@ typedef struct evp_hpke_kem_st EVP_HPKE_KEM; typedef struct evp_hpke_key_st EVP_HPKE_KEY; typedef struct evp_kem_st EVP_KEM; typedef struct kem_key_st KEM_KEY; -typedef struct evp_pkey_asn1_method_st EVP_PKEY_ASN1_METHOD; typedef struct evp_pkey_ctx_st EVP_PKEY_CTX; -typedef struct evp_pkey_method_st EVP_PKEY_METHOD; typedef struct evp_pkey_st EVP_PKEY; typedef struct hmac_ctx_st HMAC_CTX; typedef struct md4_state_st MD4_CTX;