aboutsummaryrefslogtreecommitdiff
path: root/crypto
AgeCommit message (Collapse)AuthorFilesLines
4 daysEnsure CRYPTO_needs_hwcap2_workaround works without CRYPTO_library_initDavid Benjamin1-1/+4
There's one place in Chromium that relied on CRYPTO_library_init to have initialized things. We can fix this by simply initializing inside the accessor. Bug: 40644931 Change-Id: Id6c849d350f3db16103dd118659a48710c3d05a3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/69607 Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com>
7 daysWork around GCC's broken -Warray-bounds warningDavid Benjamin1-2/+14
When building with a sufficiently high -march that GCC can use YMM registers, its -Warray-bounds warning misfires on 32-byte arrays that are initialized directly. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114826 Work around this by using a less safe pattern and initializing the array with memset. Change-Id: I3bcbdca2b749fd7a3093d5194515a04c034e7bf9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/69567 Reviewed-by: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
7 daysRewrite CBS_get_asn1_int64 slightlyDavid Benjamin1-5/+3
GCC 13.2.0 has a false positive in -Wstringop-overflow. Oddly, I can only reproduce it with -O2 -march=native. The old code was also correct, but this version seems to do a better job of avoiding the warning. Instead of reversing the variable-length string while sign-extending, we assemble a big-endian sign-extended version and then do a fixed-width byte swap at the end. Fixed: 42290598 Change-Id: I6d5de1e1d6d117f6b5947d3a2155e794764eb472 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/69547 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com>
8 daysMake BoringSSL initialization-lessDavid Benjamin7-86/+26
Now that we don't depend on external CRYPTO_library_init calls or the static initializer to initialize CPU capabilities, we can drop a ton of code. This makes CRYPTO_library_init, and all its wrappers, into no-ops and drops the (non-FIPS) static initializer. I've added an internal OPENSSL_init_cpuid function for the places where the library actually needs to initialize the CPU vector. Note this slightly changes the default, previously static-initializer-full build: previously, CRYPTO_library_init was a no-op and we relied on the static initializer. Now we uniformly use CRYPTO_once. This should be an atomic read in the steady state and essentially free. We can restore the static initializer by default if this ends up being a problem, but having only one mode is more straightforward. This also avoids problems if an application calls into BoringSSL during its own static initializer. Static initializers are not coherently ordered. Update-Note: The BORINGSSL_NO_STATIC_INITIALIZER build option and CRYPTO_library_init are now unnecessary. Once updating past this revision, those options can now be cleaned up from downstream projects. Fixed: 40644931 Change-Id: Idc2e6ea7a73d6352e0360fd886c46d88dba3568c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/69508 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
8 daysCall CRYPTO_library_init before ChaCha20 and P-256 assemblyDavid Benjamin2-0/+12
We really should remove the ia32cap references from those files, but now that we're down to two files, let's go ahead and remove the CRYPTO_library_init requirement from our callers and close out the initialization hole. Notably, use of bssl-crypto in Chromium is slightly shaky without this. Although I think, prior to this CL, we'd already gotten to benign races being all that are possible because these two remaining spots don't change any in-memory representations. (Unlike C/C++, benign races from assembly are actually well-defined and truly benign.) But no sense in relying on this when we can just fix it directly. This CL just adds some explicit CRYPTO_library_init calls. A subsequent one will update the docs and clean up all the remnants of our messy initialization story. Bug: 40644931 Change-Id: Ife288a4817b930473210f43a2680a60b040bf9a0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/69507 Commit-Queue: David Benjamin <davidben@google.com> Commit-Queue: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2024-06-12Fix pointers in CONSTTIME_DECLASSIFY.Thomas Holenstein1-1/+1
Tested by compiling with valgrind. Before: $ cmake -GNinja -B build -D CONSTANT_TIME_VALIDATION=1 $ ninja -C build [...] /home/tholenst/boringssl/boringssl/crypto/dilithium/dilithium.c:1199:25: error: ‘pub’ undeclared (first use in this function) 1199 | CONSTTIME_DECLASSIFY(&pub.t1, sizeof(pub.t1)); After: $ ninja -C build ninja: Entering directory `build' [440/440] Linking CXX executable ssl/test/handshaker Change-Id: I26481d69e9d033b0c23b7486970991b673132d20 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/69267 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2024-06-11Remove OPENSSL_ia32cap_P references from AES-NI assemblyDavid Benjamin6-299/+392
The AES-NI key schedule functions have two versions, dating to OpenSSL's 23f6eec71dbd472044db7dc854599f1de14a1f48. This cites RT#3576. Unfortunately, OpenSSL purged their old RT bugs, without any archives, so this context is now lost. Some archives of openssl-dev discussion (also predating OpenSSL's archives) give most of the context: https://groups.google.com/g/mailing.openssl.dev/c/OuFXwW4NfO8/m/7d2ZXVjkxVkJ Broadly, although AES-NI has an aeskeygenassist instruction for the key schedule, apparently it's overall faster to ignore it and use aesenclast instead. But it's slower on older processors, so the assembly would check for AVX && !XOP as a proxy. (Note we always set XOP to false, even though this likely wasn't a capability check but a proxy for pre-Xen AMD chips.) It is unclear if the aeskeygenassist version is still worthwhile. However, the aesenclast version requires SSSE3. SSSE3 long predates AES-NI, but it's not clear if AES-NI implies SSSE3. In OpenSSL, the CCM AES-NI assembly seems to assume it does. For now, I've preserved the pair of them. There are now only two assembly files with OPENSSL_ia32cap_P references! Bug: 673 Change-Id: I990b1393d780db4caf074c184ce8bbd182da6e29 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68690 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2024-06-10Remove Knights Landing and Knights Mill logicDavid Benjamin2-20/+5
Intel has removed support for Knights Landing and Knights Mill chips (Xeon Phi) from GCC, LLVM, and SDE. The last of which means we can no longer test the special-case to optimize for them. These special cases date to OpenSSL's 64d92d74985ebb3d0be58a9718f9e080a14a8e7f, which describes it as a "salvage operation" because they have Silvermont-like performance issues and "ADCX/ADOX instructions are effectively mishandled at decode time". In principle, this is not very much code to continue "salvaging" them, but we don't like having code we cannot test, so follow Intel's lead in removing all support. With this change, Xeon Phi chips should keep working (assuming their CPUID reports capabilities accurately), but will likely perform much worse. This should unbreak the SDE builders on CI. Change-Id: I00c3c435222fc53c1a6c9fddf961146f837dee7d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/69187 Commit-Queue: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com>
2024-06-08Extract the AES-NI encrypt -> decrypt assembly conversionDavid Benjamin5-60/+58
aes_hw_set_decrypt_key calls aes_hw_set_encrypt_key and then does a conversion, all in assembly. On x86(_64), aes_hw_set_encrypt_key internally checks OPENSSL_ia32cap_P to call one of two variants. In preparation for splitting those variants into separate functions, get the in-asm function call out o f the day by extracting an aes_hw_encrypt_key_to_decrypt_key function. Bug: 673 Change-Id: I23eefc00bdc8cb1f20e17fb6716974e91f1c32c4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68689 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2024-06-04Mark kyber as experimentalBob Beck2-0/+2
Kyber will be changing once the standardization of ML_KEM is finalized by NIST. Update-Note: The use of Kyber functions from <openssl/experimental/kyber.h> will not compile unless the build using them defines an appropriate preprocessor define. As this interface will change once NIST finalizes the standardization of ML_KEM. Users of this code must be aware, and be prepared to make changes when this happens Change-Id: I159bae38e58dd059b3fcccf69ae9f3d5fb03bd46 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66868 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com>
2024-05-31Tidy up dilithium_test.ccDavid Benjamin1-117/+86
Some missing includes, include order is a bit off, switch C-style casts to C++-style casts (or avoid them in the first place). Also use std::vector<uint8_t> instead of std::unique_ptr<uint8_t[]>. It's a bit shorter and since the vector knows its size, it can be better integrated into our more convenient span-like APIs. Ideally we would have something like Rust Box<[u8]>, but unique_ptr<uint8_t[]> isn't quite that because it loses track of the size. Change-Id: I9ed5b1f84bfba7e112f208c9749ddcb98b92fc81 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68927 Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com>
2024-05-31Move GCMTest.ByteSwap to crypto_test.ccDavid Benjamin2-6/+9
This isn't testing part of the low-level GCM code anymore, but a common utility function. Change-Id: I5a8d2aefc31a7297256dbe53ec2360160d561ba5 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68867 Commit-Queue: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com>
2024-05-29Revert "Move unit tests out of bcm/fipsmodule"Bob Beck16-47/+55
This reverts commit e09fcf8302f75dc50afcfe40f0d59a92b40a3c2e. Change-Id: Ib15912481ac25fd60e7e4806d9b6bd5be8e62db8 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68809 Auto-Submit: Bob Beck <bbe@google.com> Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2024-05-29Revert "Also extract the test data files from bcm"Bob Beck30-21/+22
This reverts commit 5326e94dd188beba0a5e536b1d2723aee65bd85d. Change-Id: Ieb7fafaf9ca553330620f3f5c30fb69d54bdb8aa Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68808 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Auto-Submit: Bob Beck <bbe@google.com>
2024-05-29Also extract the test data files from bcmBob Beck30-22/+21
Put them in the same places as the tests moved to. Bug: 772 Change-Id: Id89de2e7418edcf3349e33ec1880ae8d2285312d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68827 Commit-Queue: Bob Beck <bbe@google.com> Auto-Submit: Bob Beck <bbe@google.com> Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2024-05-28Move unit tests out of bcm/fipsmoduleBob Beck16-55/+47
Strictly speaking this does not change any of the bcm/fipsmodule code, it moves the tests out into libcrypto so that once we move to an api boundary where bcm does not call libcrypto functions directly, these tests still can do so. Bug: 722 Change-Id: I9defc70a9e523e52dda2d53ab4bd155a4b44fc02 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68787 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Bob Beck <bbe@google.com>
2024-05-24Fix some enum issues in the test-only BORINGSSL_FIPS_COUNTERS buildDavid Benjamin2-11/+9
This fixes b/342634459. enums are weird. They're neither tight sum types, nor ints with designated values but something in between. By default (i.e. if you do not say `enum Foo : int`, which isn't in C until very new versions), an enum's range of valid values is the smallest hypothetical-bit-width signed or unsigned integer that can fit all the values. So, the following enum can only hold 0, 1, 2, or 3, and all values outside that range are UB. enum E { A = 0, B = 2, } Meanwhile, enum is signed and can hold values -2, -1, 0, 1: enum E { A = -1, B = 1, } This is incredibly bizarre. This has two consequences. First, clang emits a warning like the following: .../boringssl/crypto/fipsmodule/self_check/fips.c:75:15: error: result of comparison of unsigned enum expression < 0 is always false [-Werror,-Wtautological-unsigned-enum-zero -compare] 75 | if (counter < 0 || counter > fips_counter_max) { | ~~~~~~~ ^ ~ Second, this loop over enum values in the unit test trips UBSan. [----------] 1 test from CryptoTest [ RUN ] CryptoTest.FIPSCountersEVP_AEAD .../boringssl/crypto/crypto_test.cc:51:8: runtime error: load of value 4, which is not a valid value for type 'fips_counter_t' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/local/google/home/davidben/boringssl/crypto/crypto_test.cc:51:8 To avoid this, just do our arithemetic in integer space and only move to enums at the edges. Change-Id: Icbd0e41604ce2aa67be8d394b03c7de50062199c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68768 Commit-Queue: David Benjamin <davidben@google.com> Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> Reviewed-by: Adam Langley <agl@google.com>
2024-05-23Don't bother checking for NULL pointers in AES key schedule assemblyDavid Benjamin3-20/+0
Some of the AES implementations tried to cleanly check for NULL input and output pointers, but others did not, so callers could not rely on this. (If we end up needing to check this for some reason, we should do it in the C wrapper.) Change-Id: I495e5b3689837242b5c51bf01840997845190754 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68688 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2024-05-22Add edge-case tests for Dilithium, and fix a bug that sneaked in withGuillaume Endignoux4-8/+1244
the constant-time transformation. Change-Id: If8e9af65f4f02d695212ce449a2de82ec4a4e7c5 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68668 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2024-05-21Use SEH directives for aes_hw_set_encrypt_key and aes_hw_set_decrypt_keyDavid Benjamin2-17/+12
These functions don't use the calling convention conversion, so we can use the automatic things. This will make it a little easier, in the commit, to split these into a few functions. Note this only works because aes_hw_set_encrypt_key and aes_hw_set_decrypt_key are the last two functions in this file. We cannot interleave automatic and handwritten SEH tables. This also lets us remove some hand-encoded instructions. When OpenSSL handwrites SEH tables, they had to hand-encode instructions just in case the assembler picked a different length encoding. The synthesized tables fill in computed offsets. Bug: 259 Change-Id: Ic94cdceeab1378ef7afb217de7643a6bb75ae1a2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68687 Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com>
2024-05-21Check DSA size limits in a couple more placesDavid Benjamin4-5/+36
This change was prompted by CVE-2024-4603, but is *not* part of it. BoringSSL is not affected by CVE-2024-4603. When applications use DSA they are expected to use known-valid domain parameters (as required by FIPS 186-4, section 4.3). If the application instead allows the attacker to supply domain parameters, DSA is no longer cryptographically sound, and algorithms that were otherwise correct now become DoS attack surfaces. Alas, incorrect uses of DSA, like Diffie-Hellman (see CVE-2023-3446 and CVE-2023-3817) are rampant, so it is useful to bound our operations, so we at least can say something coherent about DoS in this scenario, even though we cannot avoid the application being cryptographically unsound. To that end, we already check DSA sizes before using the key, however, we missed a couple of operations: - DSA_generate_parameters_ex - DSA_generate_key The CL adds those too. I would not expect either to have any security impact as it's quite unlikely for an application to allow attacker control of the inputs to either function. Still, we ought to check for completeness' sake. There's no sense in generating parameters or a key that we know we'll reject. Ultimately, the problem here is that DSA was a badly-designed primitive. with a continuum of domain parameters. Experience has shown that such systems are confusing and applications will often incorrectly treat domain parameters as part of the key. Modern primitives use a small set of discrete, named parameter sets, which is far easier to get right. For this reason, we consider DSA a deprecated, legacy algorithm. Change-Id: Ib3b5dece32bcb0ac9a795f8222c1c530d9dd91a0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68707 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com>
2024-05-21The FIPS hash is only 32 bytes, not 64 bytesDavid Benjamin1-6/+3
fips_shared_support.c was reserving 64 bytes, but only 32 were used. This is a holdover from before https://boringssl-review.googlesource.com/c/boringssl/+/52165, when sometimes it was HMAC-SHA512. Change-Id: Idbaa02e91db90f6f5ead236236cd536f75b45e19 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68607 Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com>
2024-05-20Disable DilithiumTest.BitFlips testDavid Benjamin1-1/+2
This test is very slow. Even BoringSSL's CI got noticeably slower after it was enabled. It should hopefully be redundant with the test vectors, supposing the test vectors were generated well. (The test is still available to be run manually.) Bug: chromium:341639202 Change-Id: Ica248c6643458988e45e50d5ec80718b5ff4bb5f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68627 Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: Adam Langley <agl@google.com>
2024-05-20Expose the FIPS module hash at build- and run-time.Adam Langley1-0/+5
In order to provide evidence that a given build is being used when testing the module (as part of validation), this change prints the module hash during the build process and makes it available for logging at run time. Change-Id: Ib128858cc429655e86444ee86dd04f1467abc735 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68528 Reviewed-by: David Benjamin <davidben@google.com> Auto-Submit: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2024-05-17Reject invalid IPv4 addresses in ipv4_from_ascDavid Benjamin2-16/+160
The old scanf-based parser accepted all kinds of invalid inputs like: "1.2.3.4.5" "1.2.3.4 " "1.2.3. 4" " 1.2.3.4" "1.2.3.4." "1.2.3.+4" "1.2.3.4.example.test" "1.2.3.01" "1.2.3.0x1" Thanks to Amir Mohamadi for pointing this out in https://boringssl-review.googlesource.com/c/boringssl/+/68167. This is a different implementation since patching sscanf doesn't quite catch all the cases. Add a bunch of tests, some imported from Amr's patch to OpenSSL upstream, plus a bunch of my own. (IPv6 parsing is complicated!) Update-Note: The deprecated (and dangerous) string-based APIs for configuring X.509 extensions will no longer silently misinterpret some invalid inputs as IPv4 addresses. This was run through TGP internally without any issue. Change-Id: I66e223a466cc3e74df9f9ddc8aef3b6b6c790f7e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68567 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com> Auto-Submit: David Benjamin <davidben@google.com>
2024-05-16Don't define CRYPTO_addc_* and CRYPTO_subc_* in C++David Benjamin1-0/+7
GCC does not support C11 _Generic in C++ mode. Change-Id: I974a0b04bbe4900419736044d0d8050f2b856d56 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68507 Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
2024-05-15Namespace crypto/test/file_util.hDavid Benjamin5-13/+21
We've historically not wrapped the test bits in namespaces, since it's not general library code or anything. (Possibly we should?) Unfortunately Android's libbase ended up leaving a TemporaryFile class in the root namespace. That, which actually is general library code, really should have been namespaced, but moving it will take a while, so namespace ours. Change-Id: I600e2dcf347e4cc6b4b81be5a26659698fe80064 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68427 Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com>
2024-05-15Fix alignment of generated UNWIND_INFO structuresDavid Benjamin1-0/+10
I missed a few parts of the Windows documentation: > The UNWIND_INFO structure must be DWORD aligned in memory. > For alignment purposes, this array [unwind codes] always has an even > number of entries, and the final entry is potentially unused. In that > case, the array is one longer than indicated by the count of unwind > codes field. https://learn.microsoft.com/en-us/cpp/build/exception-handling-x64?view=msvc-170 This didn't seem to have any practical effect (unwinding tests worked as-is), but I noticed this while rewriting some handwritten codes. Bug: 259 Change-Id: I655f3a7f3a907797e7665a276f4926a31a1e1639 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68407 Reviewed-by: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Adam Langley <agl@google.com>
2024-05-15Document and test stance on non-canonical base64Theo Buehler2-4/+27
From RFC 4648: 3.5. Canonical Encoding The padding step in base 64 and base 32 encoding can, if improperly implemented, lead to non-significant alterations of the encoded data. For example, if the input is only one octet for a base 64 encoding, then all six bits of the first symbol are used, but only the first two bits of the next symbol are used. These pad bits MUST be set to zero by conforming encoders, which is described in the descriptions on padding below. If this property do not hold, there is no canonical representation of base-encoded data, and multiple base- encoded strings can be decoded to the same binary data. If this property (and others discussed in this document) holds, a canonical encoding is guaranteed. In some environments, the alteration is critical and therefore decoders MAY chose to reject an encoding if the pad bits have not been set to zero. The specification referring to this may mandate a specific behaviour. OpenSSL's decoder has always accepted non-canonical encodings and it still appears to be the prevalent practice in 2024. In particular Go's encoding/base64 package requires you to opt into strict mode (which encoding/pem does not use). Also, Bouncy Castle and NSS accept such encodings. So add a comment to the code that this is a deliberate, if perhaps begrudging, choice and encode this in regress with a few test cases that are more obviously of a degenerate nature than the current non-canonical forms. Also, group the test vectors straight from RFC 4648 section 10 together. Change-Id: Ibcc22b7feed86fe1cb0fd51a1d61ec0c60dc8672 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68247 Auto-Submit: Theo Buehler <theorbuehler@gmail.com> Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2024-05-11Align perlasm SEH directives with gas/clang-assemblerDavid Benjamin5-87/+115
perlasm broadly uses gas syntax. gas and clang-assembler already have SEH directives. From what I can tell, no one ever properly documented this, but this mail describes this. LLVM's test data also has examples. https://sourceware.org/legacy-ml/binutils/2009-08/msg00193.html First, we named ours based on the MASM directives and prepended ".seh_". gas says "endprologue" instead of "endprolog", "savexmm" instead of "savexmm128", and "stackalloc" instead of "allocstack". Since perlasm mostly looks like gas, I've switched to the gas spellings. Second, we made .seh_endprologue implicit because it's always immediately after the last directive. Both MASM and clang-assembler make it explicit. Synthesizing an .seh_endprologue for those syntaxes would require buffering the up the whole function, so just require it be explicit in the source. The last difference is that gas says ".seh_proc name_of_function". I've not aligned on that one because MASM actually integrates it into the PROC line. You add the FRAME keyword or not depending on whether it's a frame function. To make the MASM output easier, I think we need to diverge from both gas and what we've currently done. I'll resolve that in a follow-up change. Along the way, fix a couple of instances where the _CET_ENDBR got put on the wrong side of the SEH directive. I don't think that macro works on Windows anyway, so it's moot. But if it did emit anything, it should be included in the prologue. Bug: 571 Change-Id: I39701a952a654afe6bfc8b3b908ca8fe65d6f1a1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68292 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com>
2024-05-10Remove no-op register move from ChaCha20_ctr32_ssse3_4xDavid Benjamin1-1/+0
r11 does not have anything in it yet, and r10 is never read. This is a remnant of the ia32cap dispatch that we missed in f5e0c8f92a22679b0cd8d24d0d670769c1cc07f3. Bug: 673 Change-Id: I5cc179401418aee6479b4a38ae1c09f68ec40238 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68291 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com>
2024-05-10Replace conf_def.h with straightforward functionsDavid Benjamin2-162/+63
We aren't trying to support multiple syntaxes, and most of these table lookups recognize one or two characters. As the table is unreadable anyway, this CL was made based on crypto/conf/keysets.pl in upstream OpenSSL. (See how @V_def is populated in that script.) Change-Id: Ie38339edabc1dacc49f3efb2bee51d2a026f21fa Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68289 Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com>
2024-05-10Remove X509_STORE_set_get_crl and X509_STORE_set_check_crlDavid Benjamin3-31/+7
gRPC is no longer using these, so remove them. They were impossible to use correctly and are the cause of weird statefulness around ctx->error_depth. Once this CL sticks, we can follow up and clean up this a code a bit. Update-Note: Some unused (and unusable) callbacks were removed. Bug: 674 Change-Id: I8109dd6555d2ca056447c1b4f0aa28abe7af81b9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68387 Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2024-05-09Make Dilithium pass constant-time validationDavid Benjamin2-15/+56
Tested with GCC[0] and Clang[1] on x86_64 in release builds: - Declassify the signature before outputting it - Declassify the public key before outputting it - Some asserts need to be declassify_assert because they act on secret data. - Rejection sampling is not actually vartime (good because it's run with secret inputs + outputs), but does need declassifications. - Rejecting the signature is an intentional declassification. But also compute all the intermediate values with constant time functions and a value barrier (hidden inside the declassify call) because the compiler will otherwise leak which arm of the || fired. - SampleInBall is... unclear. Declassify it for now, because the algorithm is only viable if this is safe to leak, but leave a TODO because we will need to follow-up with the Dilithium authors. [0] gcc (Debian 13.2.0-10) 13.2.0 [1] clang version 19.0.0git (https://chromium.googlesource.com/a/external/github.com/llvm/llvm-project 315c88c5fbdb2b27cebf23c87fb502f7a567d84b) Change-Id: I362e69bd3d1ea59fb0dbf35574e654c371061af6 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67747 Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com>
2024-05-09bn: Move dispatching logic from x86_64-mont5.pl to C.David Benjamin4-81/+140
CL originally uploaded by Brian Smith at https://boringssl-review.googlesource.com/c/boringssl/+/65569 Bug: 673 Change-Id: If84d34cae1c44cc883fc292dd048542e2b341f41 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68347 Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com>
2024-05-08Re-apply dilithium and make it work with a limited stackBob Beck4-0/+4027
This reverts commit a56407d27da6ebf63ae9817dc19587a0ae98ef4a. On top of this it then makes changes to make dilithium work without consuming excessive stack space.. This makes DILITHIUM_sign and DILITHIUM_generate_key and friends fallible for malloc failures. It removes the abort() calls that were previously present if CBB allocations did not work and rolls them into the malloc failure case. Change-Id: Ibcd70a98bb500c76df8885c0a7d06f8e9f5b18c3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67027 Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: Adam Langley <agl@google.com> Reviewed-by: Guillaume Endignoux <guillaumee@google.com> Reviewed-by: Sophie Schmieg <sschmieg@google.com>
2024-05-07Add tests for some odd escaping behavior in the CONF parserDavid Benjamin1-0/+37
Honestly, these are probably bugs, but add tests for them so, if we change the behavior, we do so intentionally. The escape processing logic has checks for hitting the end of the string early. At first, I had a hard time reaching this case because this is normally processed as a line continuation. However, the line continuation logic is not escape-aware. It just assumes if your line ends "...\\\\", that the last backslash is not a continuation. I.e. it doesn't correctly count escapes. This is almost certainly a bug, but means the escape + EOF behavior is reachable. Even more interesting is that it seems to intentionally terminate quote handling. Add tests for these cases. Change-Id: I9e058ec2b1ce3e20d87eab28a64c79880e3e5cae Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68288 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2024-05-07Test some more CONF edge casesDavid Benjamin2-3/+53
Ensure that, by rejecting "$foo", we didn't make it impossible to embed "$" in a config file. Also test every allowed punctuation character in CONF, non-ASCII characters, and empty values. Change-Id: I55c3c02b357c6017adadf0deebe95f52244ac9d2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68287 Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com>
2024-05-03Move dispatch from sha512-586.pl to CDavid Benjamin3-20/+62
Bug: 673 Change-Id: I93b839674704175f8dd85eb0fb838c1caacc4a10 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68208 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2024-05-02Remove SSE2 checks in 32-bit x86 assemblyDavid Benjamin3-666/+3
We've made crypto/internal.h require SSE2 support for a few months now without much fuss. Finish the job and remove the fallback paths. We've never tested any of these paths, and this removes a slew of OPENSSL_ia32cap_P references from the assembly. Bug: 673 Change-Id: I446a033d132af5038ab427b8560cbf20c1d97335 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68207 Commit-Queue: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com>
2024-04-29Rename function pointers to avoid shadowing global declarationPatryk Duda1-12/+12
BoringSSL compilation with -Wshadow fails with following error error: declaration of 'write' shadows a global declaration [-Werror=shadow] 673 | int (*write)(BIO *, const char *, int)) { | ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Renamed function pointers to fix this error. Bug: b:321092852 Change-Id: I94f988e5c12da5eae93f16e6666bc7ab5ca0fc06 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68107 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2024-04-26Add a standalone Bazel buildDavid Benjamin2-0/+24
Now that we can generate build files, we can actually maintain a Bazel build in HEAD, without synthesizing a separate "-with-bazel" branch. (Though we'll still need both for a transition, and until all the other build modes have migrated.) Note this has a slightly different directory structure than the old -with-bazel branch. But the targets are the same and #include <openssl/whatever.h> still works, so hopefully it's compatible. For now, I'm only setting this up with the new bzlmod scheme. Also since pulling in googletest is much less tedious with bzlmod, I've wired up tests and everything. https://bazel.build/external/overview#bzlmod To build, run commands like: bazelisk build ... bazelisk test ... bazelisk run :bssl The thinking is we can also go add this to CI and whatnot. This is also intended to replace the boringssl module in the bazel-central-registry which someone else set up already. To ease the transition, I've seeded the compatibility_level value with theirs. (I think we want to never bump it. Although we don't do SemVer, I think bzlmod's MVS version selection scheme is generally reasonable. Bumping it just introduces hiccups into the update process where projects need to coordinate updates, and we do not want that.) I wasn't clear on what to put in the version field in the tree, so I just went with 0.0.0-dev so we don't have to change it, but it's still vaguely analogous to the versions already in there. As part of this, I've added support for Bazel's runfiles mechanism in crypto/test/test_data.cc. This is completely undocumented and I had to figure out how it works by guessing, but I believe this is the officially supported way to make cc_test(data = ...) work? The official documentation says to use devtools_build::GetDataDependencyFilepath, but that API does not exist in the first place. I've also made it build with C++17 for now, so we can build libpki, but C++14 consumers should still be able to use this module, just not build libpki. To that end, one difference between this and the old module is that we no longer specify the C++ version in the build. From what I can tell, Bazel does *not* want libraries to set the C/C++ version and prefers it only come from the root. Unfortunately, they provide zero tools to effectively manage this. I've followed this pattern for C++ versions, as we can assume that Bazel projects are very aware of their C++ version, but I've explicitly ignored it for the C version. Projects tend not to change ABIs by C version, so it should be fine to set it internally. For context when reviewing, the unreadable MODULE.bazel.lock is automatically generated. I think the idea is that subsequent diffs will be more readable?? Bug: 542 Change-Id: I88f68b2602a75f58cc6d18832a956f01dc054f58 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67301 Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: Bob Beck <bbe@google.com>
2024-04-12Disable fork detection for Zephyr and CrOS ECPatryk Duda1-1/+2
Zephyr and CrOS EC targets embedded devices without MMU unit (no virtual memory). It means that they don't support any address space duplication like fork() or clone(). BUG=b/321092852 Change-Id: Icdf8be888ba87cd164cffb35f1accbc14f1a6887 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67807 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com>
2024-04-11Remove unnecessary NULL checksTheo Buehler1-12/+8
These NULL checks are redundant with sk_num()'s semantics. Change-Id: I9871bd97c3188fa67f8701ba3eb12395d955d162 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67388 Commit-Queue: Adam Langley <agl@google.com> Auto-Submit: Theo Buehler <theorbuehler@gmail.com> Reviewed-by: Adam Langley <agl@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2024-04-10Avoid strdup in crypto/err/err.cDavid Benjamin2-17/+15
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 <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2024-04-01Set service indicator for TLS 1.3 KDF.Adam Langley2-0/+22
Change-Id: Ia6fffb4c1fbe9edc62a4c22b45408e41ac6ae086 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67547 Reviewed-by: David Benjamin <davidben@google.com> Auto-Submit: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com>
2024-03-29Document that our Unicode APIs reject noncharactersDavid Benjamin1-2/+3
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 <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2024-03-29Switch EVP_CIPHERs to C99 initializersDavid Benjamin4-93/+83
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 <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com>
2024-03-29Reflect latest FIPS updates, including 186-5.Adam Langley4-23/+30
Change-Id: Iaa166136b4b7700e59c3a7643ec1b4aacf43c647 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66747 Auto-Submit: Adam Langley <agl@google.com> Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
2024-03-23Flatten crypto/CMakeLists.txt into the top-levelDavid Benjamin1-137/+0
This avoids needing to rebase the source lists. It also means that libcrypto.a and libssl.a end up directly in the build directory, which makes it a bit easier to pass it to, say, gcc -L when testing things. That file is, alas, getting a bit large. delocate is a pretty large amount of code. I tried to abstract things into functions to toss into a cmake/delocate.cmake, but CMake is really bad at making abstractions. Bug: 542 Change-Id: I084d7a6bdd4c21ac27859b8b0c9d7a84829f2823 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/67298 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
generated by cgit v1.1 at 2024-06-29 00:52:43 +0000