Skip to content

Commit 8ac2d2d

Browse files
authored
BXB-2575 use our boringssl (#7)
* Fix beeu_mod_inverse_vartime CFI annotations and preamble. This was also caught by the in-progress unwind tester. There are two issues here. First, .cfi_endproc must come after ret to fully cover the function. More importantly, this function is confused about whether it has a frame pointer or not. It looks like it does (movq %rsp, %rbp), and annotates accordingly, but it does not actually use the frame pointer. It cannot. $y4 is rbp and gets clobbered immediately after the preamble! Remove this instruction and align the CFI annotations with a frame-pointer-less function. Bug: 181 Change-Id: I47b5f9798b3bcee1748e537b21c173d312a14b42 Reviewed-on: https://boringssl-review.googlesource.com/c/33947 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]> * Annotate leaf functions with .cfi_{startproc,endproc} While gdb can figure it out, libunwind requires CFI directives to unwind a leaf function, even though the directives are trivial. Adding them matches what GCC outputs, and likely gdb has many heuristics that less complex tools (e.g. profilers) may not. Bug: 181 Change-Id: I25c72152de33109a29710a828aeb99c608dd0470 Reviewed-on: https://boringssl-review.googlesource.com/c/33964 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Add EC_GROUP_order_bits for OpenSSL compatibility Change-Id: I37149fa4274357d84befff85728ce2337131afa7 Reviewed-on: https://boringssl-review.googlesource.com/c/33804 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Clarify build requirements. The minimum versions are largely bogus, since we do not continuously test them. Instead, we've been using Abseil's five year guidelines to decide when to rely on tooling improvements. Document this. Remove the note on how to build Ninja as that'll just get out of date. For instance, they appear to support Python 3 when building now. Explicitly call out that CMake 3.0 will be required next year (released June 2014). 3.0 is the minimum needed to distinguish Clang from AppleClang, without which version checks on Clang don't work. Also document that we require a C++11 compiler for more than just tests these days. Change-Id: I4e5766934edc1d69f7be01f48e855d400adfb5f2 Reviewed-on: https://boringssl-review.googlesource.com/c/33845 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Remove bundled copy of android-cmake. I don't believe we use this anymore. People using it should upgrade to a newer NDK (or, worst case, download android-cmake themselves). Change-Id: Ia99d7b19d6f2ec3f4ffe90795813b00480dc2d60 Reviewed-on: https://boringssl-review.googlesource.com/c/34004 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Add EC_KEY_key2buf for OpenSSL compatibility Change-Id: If45ef3a9bb757bd0c7f592f40ececaf4aa2f607d Reviewed-on: https://boringssl-review.googlesource.com/c/33824 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> * Remove pooling of PRNG state. Prior to 82639e6f we used thread-local data for the PRNG state. That change switched to using a mutex-protected pool instead in order to save memory in heavily-threaded applications. However, the pool mutex can get extremely hot in cases where the PRNG is heavily used. 8e8f2504 was a short-term work around, but supporting both modes is overly complex. This change moves back to the state of the prior to 82639e6f. The best way to review this is to diff the changed files against '82639e6f^' and note that the only difference is a comment added in rand.c: https://paste.googleplex.com/4997991748337664 Change-Id: I8febce089696fa6bc39f94f4a1e268127a8f78db Reviewed-on: https://boringssl-review.googlesource.com/c/34024 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]> * Update SDE and add the Windows version. Windows is sufficiently different from Linux that running tests under SDE for Windows, particularly with the new ABI tests, is worthwhile. Change-Id: I32c4f6de06b2e732ebb2c1492eb1766cae73c0e0 Reviewed-on: https://boringssl-review.googlesource.com/c/34064 Reviewed-by: Steven Valdez <[email protected]> Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> * Be less clever with CHECK_ABI. Unwind testing will make CHECK_ABI much slower. The original ptrace-based design is some 10,000x slower. I've found an alternate design that's a mere 1,000x slower, but this probably warrants being more straightforward. It also removes the weirdness where NDEBUG controlled which tests were run. While it does mean we need to write some extra tests for p256-x86_64.pl, we otherwise do not directly unit test our assembly anyway. Usually we test the public crypto APIs themselves. So, for most files, this isn't actually extra work. Bug: 181 Change-Id: I7cbb7f930c2ea6ae32a201da503dcd36844704f0 Reviewed-on: https://boringssl-review.googlesource.com/c/33965 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Add EVP_CIPHER support for Blowfish and CAST to decrepit. Postgres contains a “pqcrypto” module that showcases the worst of 90's crypto, including Blowfish and CAST5 in CFB, CBC, and ECB modes. (Also, 64-bit keys for both of those.) In order to minimise the patching needed to build Postgres, put these things in decrepit. Change-Id: I8390c5153dd7227eef07293a4363878d79df8b21 Reviewed-on: https://boringssl-review.googlesource.com/c/34044 Reviewed-by: Adam Langley <[email protected]> Reviewed-by: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]> * Fix some size_t to long casts. Maybe someday we'll be able to turn on that warning. (The EVP_CIPHER hooks take size_t while the functions took long.) Change-Id: Ic4da44efca9419a7f703e232d3f92638eb4ab37a Reviewed-on: https://boringssl-review.googlesource.com/c/34084 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Add a CFI tester to CHECK_ABI. This uses the x86 trap flag and libunwind to test CFI works at each instruction. For now, it just uses the system one out of pkg-config and disables unwind tests if unavailable. We'll probably want to stick a copy into //third_party and perhaps try the LLVM one later. This tester caught two bugs in P-256 CFI annotations already: I47b5f9798b3bcee1748e537b21c173d312a14b42 and I9f576d868850312d6c14d1386f8fbfa85021b347 An earlier design used PTRACE_SINGLESTEP with libunwind's remote unwinding features. ptrace is a mess around stop signals (see group-stop discussion in ptrace(2)) and this is 10x faster, so I went with it. The question of which is more future-proof is complex: - There are two libunwinds with the same API, https://www.nongnu.org/libunwind/ and LLVM's. This currently uses the system nongnu.org for convenience. In future, LLVM's should be easier to bundle (less complex build) and appears to even support Windows, but I haven't tested this. Moreover, setting the trap flag keeps the test single-process, which is less complex on Windows. That suggests the trap flag design and switching to LLVM later. However... - Not all architectures have a trap flag settable by userspace. As far as I can tell, ARMv8's PSTATE.SS can only be set from the kernel. If we stick with nongnu.org libunwind, we can use PTRACE_SINGLESTEP and remote unwinding. Or we implement it for LLVM. Another thought is for the ptracer to bounce SIGTRAP back into the process, to share the local unwinding code. - ARMv7 has no trap flag at all and PTRACE_SINGLESTEP fails. Debuggers single-step by injecting breakpoints instead. However, ARMv8's trap flag seems to work in both AArch32 and AArch64 modes, so we may be able to condition it on a 64-bit kernel. Sadly, neither strategy works with Intel SDE. Adding flags to cpucap vectors as we do with ARM would help, but it would not emulate CPUs newer than the host CPU. For now, I've just had SDE tests disable these. Annoyingly, CMake does not allow object libraries to have dependencies, so make test_support a proper static library. Rename the target to test_support_lib to avoid https://gitlab.kitware.com/cmake/cmake/issues/17785 Update-Note: This adds a new optional test dependency, but it's disabled by default (define BORINGSSL_HAVE_LIBUNWIND), so consumers do not need to do anything. We'll probably want to adjust this in the future. Bug: 181 Change-Id: I817263d7907aff0904a9cee83f8b26747262cc0c Reviewed-on: https://boringssl-review.googlesource.com/c/33966 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Set NIDs for Blowfish and CAST. I hadn't thought that we still had the NIDs for these, but it appears that we do. In which case, might as well set them. Change-Id: I0d459ecacda95298c7ef345b73639cc02c74914f Reviewed-on: https://boringssl-review.googlesource.com/c/34045 Commit-Queue: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]> * Add ABI tests for rdrand. This one is easy. For others we may wish to get in the habit of pulling assembly declarations into headers. Bug: 181 Change-Id: I24c774e3c9b1f983585b9828b0783ceddd08f0e7 Reviewed-on: https://boringssl-review.googlesource.com/c/33967 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Add DEPS rules to checkout Windows SDE. Change-Id: Ia2398fa822fef1ac79f2062a8401bdd3ec963727 Reviewed-on: https://boringssl-review.googlesource.com/c/34104 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> * Make pkg-config optional. Since libunwind, and therefore the CFI tests, are already optional, might as well make pkg-config optional too. (I'm not sure whether we actually want to support people using our development build, but gRPC appear to be trying to do so: https://github.com/grpc/grpc/issues/17638) Change-Id: I16b4c53bd8a66933bc19fba29aed0d79ce2670c2 Reviewed-on: https://boringssl-review.googlesource.com/c/34124 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]> * Add ABI tests for SHA*. Bug: 181 Change-Id: Ica9299613d7fd1b803533b7e489b9ba8fe816a24 Reviewed-on: https://boringssl-review.googlesource.com/c/33968 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Add ABI tests for bn_mul_mont. Bug: 181 Change-Id: Ibd606329278c6b727d95e762920a12b58bb8687a Reviewed-on: https://boringssl-review.googlesource.com/c/33969 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Use handshake parameters to decide if cert/key are available Whether the host has a valid certificate or private key may depend on the handshake parameters and not just its configuration. For example, negotiating the delegated credential extension (see https://tools.ietf.org/html/draft-ietf-tls-subcerts) requires an alternate private key for the handshake. Change-Id: I11cea1d11e731aa4018d980c010b8d8ebaa64c31 Reviewed-on: https://boringssl-review.googlesource.com/c/33664 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> * Fix protos_len size in SSL_set_alpn_protos and SSL_CTX_set_alpn_protos MakeConstSpan() takes size_t as the second argument, so protos_len ought to also be size_t. Bug: chromium:879657 Change-Id: I93089ea20ce4b9c2b9d4d954dce807feb5341482 Reviewed-on: https://boringssl-review.googlesource.com/c/34164 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]> * Update tools. Unfortunately, this requires partially reverting https://boringssl-review.googlesource.com/31324. This is a mess. While clang proper includes a fuzzer driver, Chromium doesn't use it. Chromium builds exclusively with fuzzer-no-link and links to its own copy of the fuzzer runtime[1]. As of [2], Chromium's clang (which we use on bots) no longer includes the driver, so we must mimic them. However, Chromium's setup is somewhat questionable because fuzzer-no-link pulls in libclang_rt.fuzzer_no_main which still includes most of libclang_rt.fuzzer, just not the one main function[3]. It appears Chromium is actually linking two copies of libclang_rt.fuzzer_no_main. Hopefully this mostly works out as Chromium's clang and libFuzzer should be relatively aligned, but it's not a good assumption for our build, which can take other Clangs too. Thus, if you pass -DFUZZ=1 as-is, we will assume you are using a "normal" Clang with all its relevant runtimes intact. If, however, you are using Chromium clang, you must drop the matching libFuzzer where the bots expected it and build with -DLIBFUZZER_FROM_DEPS=1. This involves no changes to the bots because we never actually unwound all the LIBFUZZER_FROM_DEPS bits before. [1] https://cs.chromium.org/chromium/src/testing/libfuzzer/BUILD.gn?rcl=d21c49585f262e851e2984f96f52905782706325&l=14 [2] https://chromium.googlesource.com/chromium/src/+/c79bf2ea4cf65431dccb57cb2a44528c284645a1 [3] https://github.com/llvm-mirror/compiler-rt/blob/8ebc3668b07fc5cca6010265cd4795443f1c1bea/lib/fuzzer/CMakeLists.txt#L93-L107 https://github.com/llvm-mirror/compiler-rt/blob/8ebc3668b07fc5cca6010265cd4795443f1c1bea/lib/fuzzer/FuzzerMain.cpp Change-Id: I946b3c821c3d7e6def7e07f1381f58241611ba3d Reviewed-on: https://boringssl-review.googlesource.com/c/34184 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Delete the variants/draft code. Change-Id: I84abfedc30e4c34e42285f3c366c2f504a3b9cf2 Reviewed-on: https://boringssl-review.googlesource.com/c/34144 Commit-Queue: Steven Valdez <[email protected]> Reviewed-by: David Benjamin <[email protected]> * Refresh fuzzer corpus. Change-Id: If5239e701f4e0a01758e17e58ede1ef6c00293b2 Reviewed-on: https://boringssl-review.googlesource.com/c/34204 Reviewed-by: Steven Valdez <[email protected]> Commit-Queue: Steven Valdez <[email protected]> * Add ABI tests for MD5. This does not actually matter, but writing new CFI directives with the tester seemed like fun. (It caught two typos, one intentional and one accidental.) Change-Id: Iff3e0358f2e56caa26079f658fa7a682772150a1 Reviewed-on: https://boringssl-review.googlesource.com/c/34185 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Add ABI tests for ChaCha20_ctr32. Change-Id: I1fad7f954284000474e5723c3fa59fedceb52ad4 Reviewed-on: https://boringssl-review.googlesource.com/c/34186 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Add direction flag checking to CHECK_ABI. Linux and Windows ABIs both require that the direction flag be cleared on function exit, so that functions can rely on it being cleared on entry. (Some OpenSSL assembly preserves it, which is stronger, but we only require what is specified by the ABI so CHECK_ABI works with C compiler output.) Change-Id: I1a320aed4371176b4b44fe672f1a90167b84160f Reviewed-on: https://boringssl-review.googlesource.com/c/34187 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Move aes_nohw, bsaes, and vpaes prototypes to aes/internal.h. This is in preparation for adding ABI tests to them. In doing so, update delocate.go so that OPENSSL_ia32cap_get is consistently callable outside the module. Right now it's callable both inside and outside normally, but not in FIPS mode because the function is generated. This is needed for tests and the module to share headers that touch OPENSSL_ia32cap_P. Change-Id: Idbc7d694acfb974e0b04adac907dab621e87de62 Reviewed-on: https://boringssl-review.googlesource.com/c/34188 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Add AES ABI tests. This involves fixing some bugs in aes_nohw_cbc_encrypt's annotations, and working around a libunwind bug. In doing so, support .cfi_remember_state and .cfi_restore_state in perlasm. Change-Id: Iaedfe691356b0468327a6be0958d034dafa760e5 Reviewed-on: https://boringssl-review.googlesource.com/c/34189 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> * Add ABI tests for HRSS assembly. The last instruction did not unwind correctly. Also add .type and .size annotations so that errors show up properly. Change-Id: Id18e12b4ed51bdabb90bd5ac66631fd989649eec Reviewed-on: https://boringssl-review.googlesource.com/c/34190 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> * Fix header file for _byteswap_ulong and _byteswap_uint64 from MSVC CRT _byteswap_ulong and _byteswap_uint64 are documented (see below link) as coming from stdlib.h. On some build configurations stdlib.h is pulled in by intrin.h but that is not guaranteed. In particular, this assumption causes build breaks when building Chromium for Windows ARM64 with clang-cl. This change switches the #include to use the documented header file, thus fixing Windows ARM64 with clang-cl. https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/byteswap-uint64-byteswap-ulong-byteswap-ushort Bug: chromium:893460 Change-Id: I738c7227a9e156c894c2be62b52228a5bbd88414 Reviewed-on: https://boringssl-review.googlesource.com/c/34244 Reviewed-by: David Benjamin <[email protected]> Reviewed-by: Bruce Dawson <[email protected]> Commit-Queue: David Benjamin <[email protected]> * Allow configuring QUIC method per-connection This allows sharing SSL_CTX between TCP and QUIC connections, such that common settings can be configured without having to duplicate the context. Change-Id: Ie920e7f2a772dd6c6c7b63fdac243914ac5b7b26 Reviewed-on: https://boringssl-review.googlesource.com/c/33904 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> * Fix RSAZ's OPENSSL_cleanse. https://boringssl-review.googlesource.com/28584 switched RSAZ's buffer to being externally-allocated, which means the OPENSSL_cleanse needs to be tweaked to match. Change-Id: I0a7307ac86aa10933d10d380ef652c355fed3ee9 Reviewed-on: https://boringssl-review.googlesource.com/c/34191 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]> * Ignore new fields in forthcoming Wycheproof tests. Change-Id: I95dd20bb71c18cecd4cae72bcdbd708ee5e92e77 Reviewed-on: https://boringssl-review.googlesource.com/c/34284 Commit-Queue: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]> * Remove pointer cast in P-256 table. We expect the table to have a slightly nested structure, so just generate it that way. Avoid risking strict aliasing problems. Thanks to Brian Smith for pointing this out. Change-Id: Ie21610c4afab07a610d914265079135dba17b3b7 Reviewed-on: https://boringssl-review.googlesource.com/c/34264 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Test CRYPTO_gcm128_tag in gcm_test.cc. CRYPTO_gcm128_encrypt should be paired with CRYPTO_gcm128_tag, not CRYPTO_gcm128_finish. Change-Id: Ia3023a196fe5b613e9309b5bac19ea849dbc33b7 Reviewed-on: https://boringssl-review.googlesource.com/c/34265 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Fix SSL_R_TOO_MUCH_READ_EARLY_DATA. https://boringssl-review.googlesource.com/15164 allocated a new error code by hand, rather than using the make_errors.go script, which caused it to clobber the error space reserved for alerts. Change-Id: Ife92c45da2c1d3c5506439bd5781ae91240d16d8 Reviewed-on: https://boringssl-review.googlesource.com/c/34307 Commit-Queue: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Add ABI tests for GCM. Change-Id: If28096e677104c6109e31e31a636fee82ef4ba11 Reviewed-on: https://boringssl-review.googlesource.com/c/34266 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Revert "Fix protos_len size in SSL_set_alpn_protos and SSL_CTX_set_alpn_protos" This reverts commit 35771ff8afc2201b5cf5b884db5b3889edde06bc. It breaks tcnetty, which is tcnetty's fault but we have a large backlog from Christmas to break with at the moment. Bug: chromium:879657 Change-Id: Iafe93b335d88722170ec2689a25e145969e19e73 Reviewed-on: https://boringssl-review.googlesource.com/c/34324 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]> * Mark some unmarked array sizes in curve25519.c. Change-Id: I92589f5d5e89c836cff3c26739b43eb65de67836 Reviewed-on: https://boringssl-review.googlesource.com/c/34304 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Don't look for libunwind if cross-compiling. pkg-config gets confused and doesn't know to look in, say, /usr/lib/i386-linux-gnu when building for 32-bit. Fortunately, CMake sets a CMAKE_CROSSCOMPILING variable whenever CMAKE_SYSTEM_NAME is set manually (as done in util/32-bit-toolchain.cmake). Change-Id: I638b4d54ea92ade4b2b5baa40a3c5e8c17914d46 Reviewed-on: https://boringssl-review.googlesource.com/c/34305 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]> * Switch to new fiat pipeline. This new version makes it much easier to tell which code is handwritten and which is verified. For some reason, it also is *dramatically* faster for 32-bit x86 GCC. Clang x86_64, however, does take a small hit. Benchmarks below. x86, GCC 7.3.0, OPENSSL_SMALL (For some reason, GCC used to be really bad at compiling the 32-bit curve25519 code. The new one fixes this. I'm not sure what changed.) Before: Did 17135 Ed25519 key generation operations in 10026402us (1709.0 ops/sec) Did 17170 Ed25519 signing operations in 10074192us (1704.4 ops/sec) Did 9180 Ed25519 verify operations in 10034025us (914.9 ops/sec) Did 17271 Curve25519 base-point multiplication operations in 10050837us (1718.4 ops/sec) Did 10605 Curve25519 arbitrary point multiplication operations in 10047714us (1055.5 ops/sec) Did 7800 ECDH P-256 operations in 10018331us (778.6 ops/sec) Did 24308 ECDSA P-256 signing operations in 10019241us (2426.1 ops/sec) Did 9191 ECDSA P-256 verify operations in 10081639us (911.7 ops/sec) After: Did 99873 Ed25519 key generation operations in 10021810us (9965.6 ops/sec) [+483.1%] Did 99960 Ed25519 signing operations in 10052236us (9944.1 ops/sec) [+483.4%] Did 53676 Ed25519 verify operations in 10009078us (5362.7 ops/sec) [+486.2%] Did 102000 Curve25519 base-point multiplication operations in 10039764us (10159.6 ops/sec) [+491.2%] Did 60802 Curve25519 arbitrary point multiplication operations in 10056897us (6045.8 ops/sec) [+472.8%] Did 7900 ECDH P-256 operations in 10054509us (785.7 ops/sec) [+0.9%] Did 24926 ECDSA P-256 signing operations in 10050919us (2480.0 ops/sec) [+2.2%] Did 9494 ECDSA P-256 verify operations in 10064659us (943.3 ops/sec) [+3.5%] x86, Clang 8.0.0 trunk 349417, OPENSSL_SMALL Before: Did 82750 Ed25519 key generation operations in 10051177us (8232.9 ops/sec) Did 82400 Ed25519 signing operations in 10035806us (8210.6 ops/sec) Did 41511 Ed25519 verify operations in 10048919us (4130.9 ops/sec) Did 83300 Curve25519 base-point multiplication operations in 10044283us (8293.3 ops/sec) Did 49700 Curve25519 arbitrary point multiplication operations in 10007005us (4966.5 ops/sec) Did 14039 ECDH P-256 operations in 10093929us (1390.8 ops/sec) Did 40950 ECDSA P-256 signing operations in 10006757us (4092.2 ops/sec) Did 16068 ECDSA P-256 verify operations in 10095996us (1591.5 ops/sec) After: Did 80476 Ed25519 key generation operations in 10048648us (8008.6 ops/sec) [-2.7%] Did 79050 Ed25519 signing operations in 10049180us (7866.3 ops/sec) [-4.2%] Did 40501 Ed25519 verify operations in 10048347us (4030.6 ops/sec) [-2.4%] Did 81300 Curve25519 base-point multiplication operations in 10017480us (8115.8 ops/sec) [-2.1%] Did 48278 Curve25519 arbitrary point multiplication operations in 10092500us (4783.6 ops/sec) [-3.7%] Did 15402 ECDH P-256 operations in 10096705us (1525.4 ops/sec) [+9.7%] Did 44200 ECDSA P-256 signing operations in 10037715us (4403.4 ops/sec) [+7.6%] Did 17000 ECDSA P-256 verify operations in 10008813us (1698.5 ops/sec) [+6.7%] x86_64, GCC 7.3.0 (Note these P-256 numbers are not affected by this change. Included to get a sense of noise.) Before: Did 557000 Ed25519 key generation operations in 10011721us (55634.8 ops/sec) Did 550000 Ed25519 signing operations in 10016449us (54909.7 ops/sec) Did 190000 Ed25519 verify operations in 10014565us (18972.4 ops/sec) Did 587000 Curve25519 base-point multiplication operations in 10015402us (58609.7 ops/sec) Did 230000 Curve25519 arbitrary point multiplication operations in 10023827us (22945.3 ops/sec) Did 179000 ECDH P-256 operations in 10016294us (17870.9 ops/sec) Did 557000 ECDSA P-256 signing operations in 10014158us (55621.3 ops/sec) Did 198000 ECDSA P-256 verify operations in 10036694us (19727.6 ops/sec) After: Did 569000 Ed25519 key generation operations in 10004965us (56871.8 ops/sec) [+2.2%] Did 563000 Ed25519 signing operations in 10000064us (56299.6 ops/sec) [+2.5%] Did 196000 Ed25519 verify operations in 10025650us (19549.9 ops/sec) [+3.0%] Did 596000 Curve25519 base-point multiplication operations in 10008666us (59548.4 ops/sec) [+1.6%] Did 229000 Curve25519 arbitrary point multiplication operations in 10028921us (22834.0 ops/sec) [-0.5%] Did 182910 ECDH P-256 operations in 10014905us (18263.8 ops/sec) [+2.2%] Did 562000 ECDSA P-256 signing operations in 10011944us (56133.0 ops/sec) [+0.9%] Did 202000 ECDSA P-256 verify operations in 10046901us (20105.7 ops/sec) [+1.9%] x86_64, GCC 7.3.0, OPENSSL_SMALL Before: Did 350000 Ed25519 key generation operations in 10002540us (34991.1 ops/sec) Did 344000 Ed25519 signing operations in 10010420us (34364.2 ops/sec) Did 197000 Ed25519 verify operations in 10030593us (19639.9 ops/sec) Did 362000 Curve25519 base-point multiplication operations in 10004615us (36183.3 ops/sec) Did 235000 Curve25519 arbitrary point multiplication operations in 10025951us (23439.2 ops/sec) Did 32032 ECDH P-256 operations in 10056486us (3185.2 ops/sec) Did 96354 ECDSA P-256 signing operations in 10007297us (9628.4 ops/sec) Did 37774 ECDSA P-256 verify operations in 10044892us (3760.5 ops/sec) After: Did 343000 Ed25519 key generation operations in 10025108us (34214.1 ops/sec) [-2.2%] Did 340000 Ed25519 signing operations in 10014870us (33949.5 ops/sec) [-1.2%] Did 192000 Ed25519 verify operations in 10025082us (19152.0 ops/sec) [-2.5%] Did 355000 Curve25519 base-point multiplication operations in 10013220us (35453.1 ops/sec) [-2.0%] Did 231000 Curve25519 arbitrary point multiplication operations in 10010775us (23075.1 ops/sec) [-1.6%] Did 31540 ECDH P-256 operations in 10009664us (3151.0 ops/sec) [-1.1%] Did 99012 ECDSA P-256 signing operations in 10090296us (9812.6 ops/sec) [+1.9%] Did 37695 ECDSA P-256 verify operations in 10092859us (3734.8 ops/sec) [-0.7%] x86_64, Clang 8.0.0 trunk 349417 (Note these P-256 numbers are not affected by this change. Included to get a sense of noise.) Before: Did 600000 Ed25519 key generation operations in 10000278us (59998.3 ops/sec) Did 595000 Ed25519 signing operations in 10010375us (59438.3 ops/sec) Did 184000 Ed25519 verify operations in 10013984us (18374.3 ops/sec) Did 636000 Curve25519 base-point multiplication operations in 10005250us (63566.6 ops/sec) Did 229000 Curve25519 arbitrary point multiplication operations in 10006059us (22886.1 ops/sec) Did 179250 ECDH P-256 operations in 10026354us (17877.9 ops/sec) Did 547000 ECDSA P-256 signing operations in 10017585us (54604.0 ops/sec) Did 197000 ECDSA P-256 verify operations in 10013020us (19674.4 ops/sec) After: Did 560000 Ed25519 key generation operations in 10009295us (55948.0 ops/sec) [-6.8%] Did 548000 Ed25519 signing operations in 10007912us (54756.7 ops/sec) [-7.9%] Did 170000 Ed25519 verify operations in 10056948us (16903.7 ops/sec) [-8.0%] Did 592000 Curve25519 base-point multiplication operations in 10016818us (59100.6 ops/sec) [-7.0%] Did 214000 Curve25519 arbitrary point multiplication operations in 10043918us (21306.4 ops/sec) [-6.9%] Did 180000 ECDH P-256 operations in 10026019us (17953.3 ops/sec) [+0.4%] Did 550000 ECDSA P-256 signing operations in 10004943us (54972.8 ops/sec) [+0.7%] Did 198000 ECDSA P-256 verify operations in 10021714us (19757.1 ops/sec) [+0.4%] x86_64, Clang 8.0.0 trunk 349417, OPENSSL_SMALL Before: Did 326000 Ed25519 key generation operations in 10003266us (32589.4 ops/sec) Did 322000 Ed25519 signing operations in 10026783us (32114.0 ops/sec) Did 181000 Ed25519 verify operations in 10015635us (18071.7 ops/sec) Did 335000 Curve25519 base-point multiplication operations in 10000359us (33498.8 ops/sec) Did 224000 Curve25519 arbitrary point multiplication operations in 10027245us (22339.1 ops/sec) Did 68552 ECDH P-256 operations in 10018900us (6842.3 ops/sec) Did 184000 ECDSA P-256 signing operations in 10014516us (18373.3 ops/sec) Did 76020 ECDSA P-256 verify operations in 10016891us (7589.2 ops/sec) After: Did 310000 Ed25519 key generation operations in 10022086us (30931.7 ops/sec) [-5.1%] Did 308000 Ed25519 signing operations in 10007543us (30776.8 ops/sec) [-4.2%] Did 173000 Ed25519 verify operations in 10005829us (17289.9 ops/sec) [-4.3%] Did 321000 Curve25519 base-point multiplication operations in 10027058us (32013.4 ops/sec) [-4.4%] Did 212000 Curve25519 arbitrary point multiplication operations in 10015203us (21167.8 ops/sec) [-5.2%] Did 64059 ECDH P-256 operations in 10042781us (6378.6 ops/sec) [-6.8%] Did 170000 ECDSA P-256 signing operations in 10030896us (16947.6 ops/sec) [-7.8%] Did 72176 ECDSA P-256 verify operations in 10075369us (7163.6 ops/sec) [-5.6%] Bug: 254 Change-Id: Ib04c773f01b542bcb8611cceb582466bfa6f6d52 Reviewed-on: https://boringssl-review.googlesource.com/c/34306 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Rename Fiat include files to end in .h Otherwise generate_build_files.py thinks that they're top-level source files. Fixes grpc/grpc#17780 Change-Id: I9f14a816a5045c1101841a2ef7ef9868abcd5d12 Reviewed-on: https://boringssl-review.googlesource.com/c/34364 Reviewed-by: Adam Langley <[email protected]> * Add SSL_OP_NO_RENEGOTIATION Since |ssl_renegotiate_never| is the default, this option is moot. However, OpenSSL defines and supports it so this helps code that wishes to support both. Change-Id: I3a2f6e93a078d39526d10f9cd0a990953bd45825 Reviewed-on: https://boringssl-review.googlesource.com/c/34384 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> * Simplify HRSS mod3 circuits. The multiplication and subtraction circuits were found by djb using GNU Superoptimizer, and the addition circuit is derived from the subtraction one by hand. They depend on a different representation: -1 is now (1, 1) rather than (1, 0), and the latter becomes undefined. The following Python program checks that the circuits work: values = [0, 1, -1] def toBits(v): if v == 0: return 0, 0 elif v == 1: return 0, 1 elif v == -1: return 1, 1 else: raise ValueError(v) def mul((s1, a1), (s2, a2)): return ((s1 ^ s2) & a1 & a2, a1 & a2) def add((s1, a1), (s2, a2)): t = s1 ^ a2 return (t & (s2 ^ a1), (a1 ^ a2) | (t ^ s2)) def sub((s1, a1), (s2, a2)): t = a1 ^ a2 return ((s1 ^ a2) & (t ^ s2), t | (s1 ^ s2)) def fromBits((s, a)): if s == 0 and a == 0: return 0 if s == 0 and a == 1: return 1 if s == 1 and a == 1: return -1 else: raise ValueError((s, a)) def wrap(v): if v == 2: return -1 elif v == -2: return 1 else: return v for v1 in values: for v2 in values: print v1, v2 result = fromBits(mul(toBits(v1), toBits(v2))) if result != v1 * v2: raise ValueError((v1, v2, result)) result = fromBits(add(toBits(v1), toBits(v2))) if result != wrap(v1 + v2): raise ValueError((v1, v2, result)) result = fromBits(sub(toBits(v1), toBits(v2))) if result != wrap(v1 - v2): raise ValueError((v1, v2, result)) Change-Id: Ie1a4ca5a82c2651057efc62330eca6fdd9878122 Reviewed-on: https://boringssl-review.googlesource.com/c/34344 Reviewed-by: David Benjamin <[email protected]> * Add test of assembly code dispatch. The first attempt involved using Linux's support for hardware breakpoints to detect when assembly code was run. However, this doesn't work with SDE, which is a problem. This version has the assembly code update a global flags variable when it's run, but only in non-FIPS and non-debug builds. Update-Note: Assembly files now pay attention to the NDEBUG preprocessor symbol. Ensure the build passes the symbol in. (If release builds fail to link due to missing BORINGSSL_function_hit, this is the cause.) Change-Id: I6b7ced442b7a77d0b4ae148b00c351f68af89a6e Reviewed-on: https://boringssl-review.googlesource.com/c/33384 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: David Benjamin <[email protected]> * HRSS: flatten sample distribution. With HRSS-SXY, the sampling algorithm now longer has to be the same between the two parties. Therefore we can change it at will (as long as it remains reasonably uniform) and thus take the opportunity to make the output distribution flatter. Change-Id: I74c667fcf919fe11ddcf2f4fb8a540b5112268bf Reviewed-on: https://boringssl-review.googlesource.com/c/34404 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: David Benjamin <[email protected]> * Fix undefined pointer casts in SHA-512 code. Casting an unaligned pointer to uint64_t* is undefined, even on platforms that support unaligned access. Additionally, dereferencing as uint64_t violates strict aliasing rules. Instead, use memcpys which we assume any sensible compiler can optimize. Also simplify the PULL64 business with the existing CRYPTO_bswap8. This also removes the need for the SHA512_BLOCK_CAN_MANAGE_UNALIGNED_DATA logic. The generic C code now handles unaligned data and the assembly already can as well. (The only problematic platform with assembly is old ARM, but sha512-armv4.pl already handles this via an __ARM_ARCH__ check. See also OpenSSL's version of this file which always defines SHA512_BLOCK_CAN_MANAGE_UNALIGNED_DATA if SHA512_ASM is defined.) Add unaligned tests to digest_test.cc, so we retain coverage of unaligned EVP_MD inputs. Change-Id: Idfd8586c64bab2a77292af2fa8eebbd193e57c7d Reviewed-on: https://boringssl-review.googlesource.com/c/34444 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Add an option to build with UBSan. Change-Id: I31d5660fa4792bbb1ef8a721bad4bdbdb0e56863 Reviewed-on: https://boringssl-review.googlesource.com/c/34445 Reviewed-by: Adam Langley <[email protected]> * Fix signed left-shifts in curve25519.c. Due to a language flaw in C, left-shifts on signed integers are undefined for negative numbers. This makes them all but useless. Cast to the unsigned type, left-shift, and cast back (casts are defined to wrap) to silence UBSan. Change-Id: I8fbe739aee1c99cf553462b675863e6d68c2b302 Reviewed-on: https://boringssl-review.googlesource.com/c/34446 Reviewed-by: Adam Langley <[email protected]> * Don't pass NULL,0 to qsort. qsort shares the same C language bug as mem*. Two of our calls may see zero-length lists. This trips UBSan. Change-Id: Id292dd277129881001eb57b1b2db78438cf4642e Reviewed-on: https://boringssl-review.googlesource.com/c/34447 Reviewed-by: Adam Langley <[email protected]> * Avoid unwind tests on libc functions. When built under UBSan, it gets confused inside a PLT stub. Change-Id: Ib082ecc076ba2111337ff5921e465e4beb99aab5 Reviewed-on: https://boringssl-review.googlesource.com/c/34448 Reviewed-by: Adam Langley <[email protected]> * Remove union from |SHA512_CTX|. With 2fe0360a4e1b988e7b0aa0b4348bf55805512c09, we no longer use the other member of this union so it can be removed. Change-Id: Ideb7c47a72df0b420eb1e7d8c718e1cacb2129f5 Reviewed-on: https://boringssl-review.googlesource.com/c/34449 Commit-Queue: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]> * Add a RelWithAsserts build configuration. On our bots, debug unit tests take around 2.5x as long to complete as release tests on Linux, 3x as long on macOS, and 6x as long on Windows. Our tests are fast, so this does not particularly matter, but SDE inflates a 13 second test run to 8 minutes. On Windows (MSVC), where we don't but would like to test with SDE, the difference between optimized and unoptimized is even larger, and test runs are slower in general. This suggests running SDE tests in release mode. Release mode tests, however, are less effective because they do not include asserts. Thus, add a RelWithAsserts option. (Chromium does something similar. I believe most of the test-running configurations on the critical path run is_debug = false and dcheck_always_on = true.) Change-Id: I273dd86ab8ea039f34eca431483827c87dc5c461 Reviewed-on: https://boringssl-review.googlesource.com/c/34464 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Update comments around JDK11 workaround. 11.0.2 has since been released, but we are now aware of several more bugs, so the workaround is unlikely to be removable for the foreseeable future. Change-Id: I8e7edcba2f002d0558a21e607306ddf9a205bfb3 Reviewed-on: https://boringssl-review.googlesource.com/c/34484 Commit-Queue: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Make 256-bit ciphers a preference for CECPQ2, not a requirement. If 256-bit ciphers are a requirement for CECPQ2 then that introduces a link between supported ciphers and supported groups: offering CECPQ2 without a 256-bit cipher is invalid. But that's a little weird since these things were otherwise independent. So, rather than require a 256-bit cipher for CECPQ2, just prefer them. Change-Id: I491749e41708cd9c5eeed5b4ae23c11e5c0b9725 Reviewed-on: https://boringssl-review.googlesource.com/c/34504 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]> * Tweak some slightly fragile tests. These tests failed when CECPQ2 was enabled by default. Even if we're not going to make CECPQ2 the default, it's worth fixing them to be more robust. Change-Id: Idef508bca9e17a4ef0e0a8a396755abd975f9908 Reviewed-on: https://boringssl-review.googlesource.com/c/34524 Commit-Queue: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]> * Add a constant-time pshufb-based GHASH implementation. We currently require clmul instructions for constant-time GHASH on x86_64. Otherwise, it falls back to a variable-time 4-bit table implementation. However, a significant proportion of clients lack these instructions. Inspired by vpaes, we can use pshufb and a slightly different order of incorporating the bits to make a constant-time GHASH. This requires SSSE3, which is very common. Benchmarking old machines we had on hand, it appears to be a no-op on Sandy Bridge and a small slowdown for Penryn. Sandy Bridge (Intel Pentium CPU 987 @ 1.50GHz): (Note: these numbers are before 16-byte-aligning the table. That was an improvement on Penryn, so it's possible Sandy Bridge is now better.) Before: Did 4244750 AES-128-GCM (16 bytes) seal operations in 4015000us (1057222.9 ops/sec): 16.9 MB/s Did 442000 AES-128-GCM (1350 bytes) seal operations in 4016000us (110059.8 ops/sec): 148.6 MB/s Did 84000 AES-128-GCM (8192 bytes) seal operations in 4015000us (20921.5 ops/sec): 171.4 MB/s Did 3349250 AES-256-GCM (16 bytes) seal operations in 4016000us (833976.6 ops/sec): 13.3 MB/s Did 343500 AES-256-GCM (1350 bytes) seal operations in 4016000us (85532.9 ops/sec): 115.5 MB/s Did 65250 AES-256-GCM (8192 bytes) seal operations in 4015000us (16251.6 ops/sec): 133.1 MB/s After: Did 4229250 AES-128-GCM (16 bytes) seal operations in 4016000us (1053100.1 ops/sec): 16.8 MB/s [-0.4%] Did 442250 AES-128-GCM (1350 bytes) seal operations in 4016000us (110122.0 ops/sec): 148.7 MB/s [+0.1%] Did 83500 AES-128-GCM (8192 bytes) seal operations in 4015000us (20797.0 ops/sec): 170.4 MB/s [-0.6%] Did 3286500 AES-256-GCM (16 bytes) seal operations in 4016000us (818351.6 ops/sec): 13.1 MB/s [-1.9%] Did 342750 AES-256-GCM (1350 bytes) seal operations in 4015000us (85367.4 ops/sec): 115.2 MB/s [-0.2%] Did 65250 AES-256-GCM (8192 bytes) seal operations in 4016000us (16247.5 ops/sec): 133.1 MB/s [-0.0%] Penryn (Intel Core 2 Duo CPU P8600 @ 2.40GHz): Before: Did 1179000 AES-128-GCM (16 bytes) seal operations in 1000139us (1178836.1 ops/sec): 18.9 MB/s Did 97000 AES-128-GCM (1350 bytes) seal operations in 1006347us (96388.2 ops/sec): 130.1 MB/s Did 18000 AES-128-GCM (8192 bytes) seal operations in 1028943us (17493.7 ops/sec): 143.3 MB/s Did 977000 AES-256-GCM (16 bytes) seal operations in 1000197us (976807.6 ops/sec): 15.6 MB/s Did 82000 AES-256-GCM (1350 bytes) seal operations in 1012434us (80992.9 ops/sec): 109.3 MB/s Did 15000 AES-256-GCM (8192 bytes) seal operations in 1006528us (14902.7 ops/sec): 122.1 MB/s After: Did 1306000 AES-128-GCM (16 bytes) seal operations in 1000153us (1305800.2 ops/sec): 20.9 MB/s [+10.8%] Did 94000 AES-128-GCM (1350 bytes) seal operations in 1009852us (93082.9 ops/sec): 125.7 MB/s [-3.4%] Did 17000 AES-128-GCM (8192 bytes) seal operations in 1012096us (16796.8 ops/sec): 137.6 MB/s [-4.0%] Did 1070000 AES-256-GCM (16 bytes) seal operations in 1000929us (1069006.9 ops/sec): 17.1 MB/s [+9.4%] Did 79000 AES-256-GCM (1350 bytes) seal operations in 1002209us (78825.9 ops/sec): 106.4 MB/s [-2.7%] Did 15000 AES-256-GCM (8192 bytes) seal operations in 1061489us (14131.1 ops/sec): 115.8 MB/s [-5.2%] Change-Id: I1c3760a77af7bee4aee3745d1c648d9e34594afb Reviewed-on: https://boringssl-review.googlesource.com/c/34267 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Implement server support for delegated credentials. This implements the server-side of delegated credentials, a proposed extension for TLS: https://tools.ietf.org/html/draft-ietf-tls-subcerts-02 Change-Id: I6a29cf1ead87b90aeca225335063aaf190a417ff Reviewed-on: https://boringssl-review.googlesource.com/c/33666 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> * Add some Node compatibility functions. This doesn't cover all the functions used by Node, but it's the easy bits. (EVP_PKEY_paramgen will be done separately as its a non-trivial bit of machinery.) Change-Id: I6501e99f9239ffcdcc57b961ebe85d0ad3965549 Reviewed-on: https://boringssl-review.googlesource.com/c/34544 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> * Add a very roundabout EC keygen API. OpenSSL's EVP-level EC API involves a separate "paramgen" operation, which is ultimately just a roundabout way to go from a NID to an EC_GROUP. But Node uses this, and it's the pattern used within OpenSSL these days, so this appears to be the official upstream recommendation. Also add a #define for OPENSSL_EC_EXPLICIT_CURVE, because Node uses it, but fail attempts to use it. Explicit curve encodings are forbidden by RFC 5480 and generally a bad idea. (Parsing such keys back into OpenSSL will cause it to lose the optimized path.) Change-Id: I5e97080e77cf90fc149f6cf6f2cc4900f573fc64 Reviewed-on: https://boringssl-review.googlesource.com/c/34565 Commit-Queue: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Add ABI testing for 32-bit x86. This is much less interesting (stack-based parameters, Windows and SysV match, no SEH concerns as far as I can tell) than x86_64, but it was easy to do and I'm more familiar with x86 than ARM, so it made a better second architecture to make sure all the architecture ifdefs worked out. Also fix a bug in the x86_64 direction flag code. It was shifting in the wrong direction, making give 0 or 1<<20 rather than 0 or 1. (Happily, x86_64 appears to be unique in having vastly different calling conventions between OSs. x86 is the same between SysV and Windows, and ARM had the good sense to specify a (mostly) common set of rules.) Since a lot of the assembly functions use the same names and the tests were written generically, merely dropping in a trampoline and CallerState implementation gives us a bunch of ABI tests for free. Change-Id: I15408c18d43e88cfa1c5c0634a8b268a150ed961 Reviewed-on: https://boringssl-review.googlesource.com/c/34624 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]> * Better document RSAZ and tidy up types. It's an assembly function, so types are a little meaningless, but everything is passed through as BN_ULONG, so be consistent. Also annotate all the RSAZ prototypes with sizes. Change-Id: I32e59e896da39e79c30ce9db52652fd645a033b4 Reviewed-on: https://boringssl-review.googlesource.com/c/34625 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]> * Add RSAZ ABI tests. As part of this, move the CPU checks to C. Change-Id: I17b701e1196c1ca116bbd23e0e669cf603ad464d Reviewed-on: https://boringssl-review.googlesource.com/c/34626 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]> * Test and fix an ABI issue with small parameters. Calling conventions must specify how to handle arguments smaller than a machine word. Should the caller pad them up to a machine word size with predictable values (zero/sign-extended), or should the callee tolerate an arbitrary bit pattern? Annoyingly, I found no text in either SysV or Win64 ABI documentation describing any of this and resorted to experiment. The short answer is that callees must tolerate an arbitrary bit pattern on x86_64, which means we must test this. See the comment in abi_test::internal::ToWord for the long answer. CHECK_ABI now, if the type of the parameter is smaller than crypto_word_t, fills the remaining bytes with 0xaa. This is so the number is out of bounds for code expecting either zero or sign extension. (Not that crypto assembly has any business seeing negative numbers.) Doing so reveals a bug in ecp_nistz256_ord_sqr_mont. The rep parameter is typed int, but the code expected uint64_t. In practice, the compiler will always compile this correctly because: - On both Win64 and SysV, rep is a register parameter. - The rep parameter is always a constant, so the compiler has no reason to leave garbage in the upper half. However, I was indeed able to get a bug out of GCC via: uint64_t foo = (1ull << 63) | 2; // Some global the compiler can't // prove constant. ecp_nistz256_ord_sqr_mont(res, a, foo >> 1); Were ecp_nistz256_ord_sqr_mont a true int-taking function, this would act like ecp_nistz256_ord_sqr_mont(res, a, 1). Instead, it hung. Fix this by having it take a full-width word. This mess has several consequences: - ABI testing now ideally needs a functional testing component to fully cover this case. A bad input might merely produce the wrong answer. Still, this is fairly effective as it will cause most code to either segfault or loop forever. (Not the enc parameter to AES however...) - We cannot freely change the type of assembly function prototypes. If the prototype says int or unsigned, it must be ignoring the upper half and thus "fixing" it to size_t cannot have handled the full range. (Unless it was simply wrong of the parameter is already bounded.) If the prototype says size_t, switching to int or unsigned will hit this type of bug. The former is a safer failure mode though. - The simplest path out of this mess: new assembly code should *only* ever take word-sized parameters. This is not a tall order as the bad parameters are usually ints that should have been size_t. Calling conventions are hard. Change-Id: If8254aff8953844679fbce4bd3e345e5e2fa5213 Reviewed-on: https://boringssl-review.googlesource.com/c/34627 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Avoid SCT/OCSP extensions in SH on {Omit|Empty}Extensions They were causing a "panic: ServerHello unexpectedly contained extensions" if the client unconditionally signals support for OCSP or SCTs. Change-Id: Ia60639431daf78679b269dfe337c1af171fd7d8b Reviewed-on: https://boringssl-review.googlesource.com/c/34644 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> * Remove infra/config folder in master branch. As of https://boringssl-review.googlesource.com/c/34584, the LUCI config has been consolidated on the infra/config branch. Change-Id: Idd9f38b99197b9ff324d98c4aecb5d8fe94a2f9e Reviewed-on: https://boringssl-review.googlesource.com/c/34684 Reviewed-by: Andrii Shyshkalov <[email protected]> Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> * Enforce key usage for RSA keys in TLS 1.2. For now, this is off by default and controlled by SSL_set_enforce_rsa_key_usage. This may be set as late as certificate verification so we may start by enforcing it for known roots. Generalizes ssl_cert_check_digital_signature_key_usage to check any part of the key_usage, and adds a new error KEY_USAGE_BIT_INCORRECT for the generalized method. Bug: chromium:795089 Change-Id: Ifa504c321bec3263a4e74f2dc48513e3b895d3ee Reviewed-on: https://boringssl-review.googlesource.com/c/34604 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> * Add instructions for debugging on Android with gdb. Android's official documentation seems to assume you're using the NDK build system or Android Studio. I extracted this from one of their scripts a while back. May as well put it somewhere we can easily find it. Change-Id: I259abc54e6935ab537956a7cbf9f80e924a60b7a Reviewed-on: https://boringssl-review.googlesource.com/c/34724 Reviewed-by: Adam Langley <[email protected]> * perlasm/x86_64-xlate.pl: refine symbol recognition in .xdata. Hexadecimals were erroneously recognized as symbols in .xdata. (Imported from upstream's b068a9b914887af5cc99895754412582fbb0e10b) Change-Id: I5d8e8e1969669a8961733802d9f034cf26c45552 Reviewed-on: https://boringssl-review.googlesource.com/c/34704 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]> * Don't use bsaes over vpaes for CTR-DRBG. RAND_bytes rarely uses large enough inputs for bsaes to be worth it. https://boringssl-review.googlesource.com/c/boringssl/+/33589 includes some rough benchmarks of various bits here. Some observations: - 8 blocks of bsaes costs roughly 6.5 blocks of vpaes. Note the comparison isn't quite accurate because I'm measuring bsaes_ctr32_encrypt_blocks against vpaes_encrypt and vpaes in CTR mode today must make do with a C loop. Even assuming a cutoff of 6 rather than 7 blocks, it's rare to ask for 96 bytes of entropy at a time. - CTR-DRBG performs some stray block operations (ctr_drbg_update), which bsaes is bad at without extra work to fold them into the CTR loop (not really worth it). - CTR-DRBG calculates a couple new key schedules every RAND_bytes call. We don't currently have a constant-time bsaes key schedule. Unfortunately, even plain vpaes loses to the current aes_nohw used by bsaes, but it's not constant-time. Also taking CTR-DRBG out of the bsaes equation - Machines without AES hardware (clients) are not going to be RNG-bound. It's mostly servers pushing way too many CBC IVs that care. This means bsaes's current side channel tradeoffs make even less sense here. I'm not sure yet what we should do for the rest of the bsaes mess, but it seems clear that we want to stick with vpaes for the RNG. Bug: 256 Change-Id: Iec8f13af232794afd007cb1065913e8117eeee24 Reviewed-on: https://boringssl-review.googlesource.com/c/34744 Reviewed-by: Adam Langley <[email protected]> * runner: Don't generate an RSA key on startup. RSA keygen isn't the fastest. Just use the existing one in rsaCertificate. Change-Id: Icd151232928e67e0a7d5becabf9dc96b0e9bfa22 Reviewed-on: https://boringssl-review.googlesource.com/c/34764 Commit-Queue: Steven Valdez <[email protected]> Reviewed-by: Steven Valdez <[email protected]> * Tolerate spaces when parsing .type directives. The .type foo, @abi-omnipotent lines weren't being parsed correctly. This doesn't change the generated files, but some internal state (used in-progress work on perlasm SEH directives) wasn't quite right. Change-Id: Id6aec79281a59f45b2eb2aea9f1fb8806b4c483e Reviewed-on: https://boringssl-review.googlesource.com/c/34786 Reviewed-by: Adam Langley <[email protected]> * Implement unwind testing for Windows. Unfortunately, due to most OpenSSL assembly using custom exception handlers to unwind, most of our assembly doesn't work with non-destructive unwind. For now, CHECK_ABI behaves like CHECK_ABI_NO_UNWIND on Windows, and CHECK_ABI_SEH will test unwinding on both platforms. The tests do, however, work with the unwind-code-based assembly we recently added, as well as the clmul-based GHASH which is also code-based. Remove the ad-hoc SEH tests which intentionally hit memory access exceptions, now that we can test unwind directly. Now that we can test it, the next step is to implement SEH directives in perlasm so writing these unwind codes is less of a chore. Bug: 259 Change-Id: I23a57a22c5dc9fa4513f575f18192335779678a5 Reviewed-on: https://boringssl-review.googlesource.com/c/34784 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Fix the order of Windows unwind codes. The unwind tester suggests Windows doesn't care, but the documentation says that unwind codes should be sorted in descending offset, which means the last instruction should be first. https://docs.microsoft.com/en-us/cpp/build/exception-handling-x64?view=vs-2017#struct-unwind_code Bug: 259 Change-Id: I21e54c362e18e0405f980005112cc3f7c417c70c Reviewed-on: https://boringssl-review.googlesource.com/c/34785 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Implement ABI testing for ARM. Update-Note: There's some chance this'll break iOS since I was unable to test it there. The iPad I have to test on is too new to run 32-bit code at all. Change-Id: I6593f91b67a5e8a82828237d3b69ed948b07922d Reviewed-on: https://boringssl-review.googlesource.com/c/34725 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Fix ABI error in bn_mul_mont on aarch64. This was caught by an aarch64 ABI tester. aarch64 has the same considerations around small arguments as x86_64 does. The aarch64 version of bn_mul_mont does not mask off the upper words of the argument. The x86_64 version does, so size_t is, strictly speaking, wrong for aarch64, but bn_mul_mont already has an implicit size limit to support its internal alloca, so this doesn't really make things worse than before. Change-Id: I39bffc8fdb2287e45a2d1f0d1b4bd5532bbf3868 Reviewed-on: https://boringssl-review.googlesource.com/c/34804 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Implement ABI testing for aarch64. This caught a bug in bn_mul_mont. Tested manually on iOS and Android. Change-Id: I1819fcd9ad34dbe3ba92bba952507d86dd12185a Reviewed-on: https://boringssl-review.googlesource.com/c/34805 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Enable all curves (inc CECPQ2) during fuzzing. Change-Id: I8083e841de135e9ec244609b1c20f0280ce20072 Reviewed-on: https://boringssl-review.googlesource.com/c/34664 Reviewed-by: David Benjamin <[email protected]> * Remove separate default group list for servers. It's the same as for clients, and we're probably not going to change that any time soon. Change-Id: Ic48cb640e98b0957d264267b97b5393f1977c6e6 Reviewed-on: https://boringssl-review.googlesource.com/c/34665 Reviewed-by: David Benjamin <[email protected]> * Remove stray semicolons. Thanks to Nico Weber for pointing this out. Change-Id: I763fd4a6f8fe467a027d5b249d9f76633ab4375a Reviewed-on: https://boringssl-review.googlesource.com/c/34824 Commit-Queue: David Benjamin <[email protected]> Commit-Queue: Steven Valdez <[email protected]> Reviewed-by: Steven Valdez <[email protected]> * Hyperlink DOI to preferred resolver Change-Id: Ib9983a74d5d2f8be7c96cedde17be5a4e9223d5e Reviewed-on: https://boringssl-review.googlesource.com/c/34844 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> * sync EVP_get_cipherbyname with EVP_do_all_sorted EVP_get_cipherbyname should work on everything that EVP_do_all_sorted lists, and conversely, there should be nothing that EVP_get_cipherbyname works on that EVP_do_all_sorted doesn't list. node.js uses these APIs to enumerate and instantiate ciphers. Change-Id: I87fcedce62d06774f7c6ee7acc898326276be089 Reviewed-on: https://boringssl-review.googlesource.com/c/33984 Reviewed-by: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> * Add ABI tests for x86_64-mont5.pl. Fix some missing CFI bits. Change-Id: I42114527f0ef8e03079d37a9f466d64a63a313f5 Reviewed-on: https://boringssl-review.googlesource.com/c/34864 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Add ABI tests for aesni-gcm-x86_64.pl. Change-Id: Ic23fc5fbec2c4f8df5d06f807c6bd2c5e1f0e99c Reviewed-on: https://boringssl-review.googlesource.com/c/34865 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Patch out unused aesni-x86_64 functions. This shrinks the bssl binary by about 8k. Change-Id: I571f258ccf7032ae34db3f20904ad9cc81cca839 Reviewed-on: https://boringssl-review.googlesource.com/c/34866 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Unwind RDRAND functions correctly on Windows. But for the ABI conversion bits, these are just leaf functions and don't even need unwind tables. Just renumber the registers on Windows to only used volatile ones. In doing so, this switches to writing rdrand explicitly. perlasm already knows how to manually encode it and our minimum assembler versions surely cover rdrand by now anyway. Also add the .size directive. I'm not sure what it's used for, but the other files have it. (This isn't a generally reusable technique. The more complex functions will need actual unwind codes.) Bug: 259 Change-Id: I1d5669bcf8b6e34939885d78aea6f60597be1528 Reviewed-on: https://boringssl-review.googlesource.com/c/34867 Commit-Queue: Adam Langley <[email protected]> Reviewed-by: Adam Langley <[email protected]> * Use Windows symbol APIs in the unwind tester. This should make things a bit easier to debug. Update-Note: Test binaries on Windows now link to dbghelp. Bug: 259 Change-Id: I9da1fc89d429080c5250238e4341445922b1dd8e Reviewed-on: https://boringssl-review.googlesource.com/c/34868 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]> * Update delegated credentials to draft-03 Change-Id: I0c648340ac7bb134fcda42c56a83f4815bbaa557 Reviewed-on: https://boringssl-review.googlesource.com/c/34884 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]> * Always define GHASH. There is a C implementation of gcm_ghash_4bit to pair with gcm_gmult_4bit. It's even slightly faster per the numbers below (x86_64 OPENSSL_NO_ASM build), but, more importantly, we trim down the combinatorial explosion of GCM implementations and free up complexity budget for potentially using bsaes better in the future. Old: Did 2557000 AES-128-GCM (16 bytes) seal operations in 1000057us (2556854.3 ops/sec): 40.9 MB/s Did 94000 AES-128-GCM (1350 bytes) seal operations in 1009613us (93105.0 ops/sec): 125.7 MB/s Did 17000 AES-128-GCM (8192 bytes) seal operations in 1024768us (16589.1 ops/sec): 135.9 MB/s Did 2511000 AES-256-GCM (16 bytes) seal operations in 1000196us (2510507.9 ops/sec): 40.2 MB/s Did 84000 AES-256-GCM (1350 bytes) seal operations in 1000412us (83965.4 ops/sec): 113.4 MB/s Did 15000 AES-256-GCM (8192 bytes) seal operations in 1046963us (14327.2 ops/sec): 117.4 MB/s New: Did 2739000 AES-128-GCM (16 bytes) seal operations in 1000322us (2738118.3 ops/sec): 43.8 MB/s Did 100000 AES-128-GCM (1350 bytes) seal operations in 1008190us (99187.7 ops/sec): 133.9 MB/s Did 17000 AES-128-GCM (8192 bytes) seal operations in 1006360us (16892.6 ops/sec): 138.4 MB/s Did 2546000 AES-256-GCM (16 bytes) seal operations in 1000150us (2545618.2 ops/sec): 40.7 MB/s Did 86000 AES-256-GCM (1350 bytes) seal operations in 1000970us (85916.7 ops/sec): 116.0 MB/s Did 14850 AES-256-GCM (8192 bytes) seal operations in 1023459us (14509.6 ops/sec): 118.9 MB/s While I'm here, tighten up some of the functions and align the ctr32 and non-ctr32 paths. Bug: 256 Change-Id: Id4df699cefc8630dd5a350d44f927900340f5e60 Reviewed-on: https://boringssl-review.googlesource.com/c/34869 Reviewed-by: Adam Langley <[email protected]> * Remove stray prototype. The function's since been renamed. Change-Id: Id1a9788dfeb5c46b3463611b08318b3f253d03df Reviewed-on: https://boringssl-review.googlesource.com/c/34870 Reviewed-by: Adam Langley <[email protected]> * Patch XTS out of ARMv7 bsaes too. Bug: 256 Change-Id: I822274bf05901d82b41dc9c9c4e6d0b5d622f3ff Reviewed-on: https://boringssl-review.googlesource.com/c/34871 Reviewed-by: Adam Langley <[email protected]> * Remove non-STRICT_ALIGNMENT code from xts.c. Independent of the underlying CPU architecture, casting unaligned pointers to uint64_t* is undefined. Just use a memcpy. The compiler should be able to optimize that itself. Change-Id: I39210871fca3eaf1f4b1d205b2bb0c337116d9cc Reviewed-on: https://boringssl-review.googlesource.com/c/34872 Reviewed-by: Adam Langley <[email protected]> * Remove STRICT_ALIGNMENT code from modes. STRICT_ALIGNMENT is a remnant of OpenSSL code would cast pointers to size_t* and load more than one byte at a time. Not all architectures support unaligned access, so it did an alignment check and only enterred this path if aligned or the underlying architecture didn't care. This is UB. Unaligned casts in C are undefined on all architectures, so we switch these to memcpy some time ago. Compilers can optimize memcpy to the unaligned accesses we wanted. That left our modes logic as: - If STRICT_ALIGNMENT is 1 and things are unaligned, work byte-by-byte. - Otherwise, use the memcpy-based word-by-word code, which now works independent of STRICT_ALIGNMENT. Remove the first check to simplify things. On x86, x86_64, and aarch64, STRICT_ALIGNMENT is zero and this is a no-op. ARM is more complex. Per [0], ARMv7 and up support unaligned access. ARMv5 do not. ARMv6 does, but can run in a mode where it looks more like ARMv5. For ARMv7 and up, STRICT_ALIGNMENT should have been zero, but was one. Thus this change should be an improvement for ARMv7 (right now unaligned inputs lose bsaes-…
1 parent 17a0b8f commit 8ac2d2d

File tree

5,736 files changed

+1001832
-169618
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

5,736 files changed

+1001832
-169618
lines changed

.clang-format

+3
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,6 @@ BasedOnStyle: Google
22
MaxEmptyLinesToKeep: 3
33
AllowShortIfStatementsOnASingleLine: false
44
AllowShortLoopsOnASingleLine: false
5+
DerivePointerAlignment: false
6+
PointerAlignment: Right
7+

.github/PULL_REQUEST_TEMPLATE

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Please do not send pull requests to the BoringSSL repository.
2+
3+
We do, however, take contributions gladly.
4+
5+
See https://boringssl.googlesource.com/boringssl/+/master/CONTRIBUTING.md
6+
7+
Thanks!

.gitignore

+24
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,30 @@
11
build/
22
ssl/test/runner/runner
3+
*.pyc
34
*.swp
45
*.swo
56
doc/*.html
67
doc/doc.css
8+
9+
util/bot/android_ndk
10+
util/bot/android_sdk/public
11+
util/bot/cmake-linux64
12+
util/bot/cmake-linux64.tar.gz
13+
util/bot/cmake-mac
14+
util/bot/cmake-mac.tar.gz
15+
util/bot/cmake-win32
16+
util/bot/cmake-win32.zip
17+
util/bot/golang
18+
util/bot/gyp
19+
util/bot/libcxx
20+
util/bot/libcxxabi
21+
util/bot/llvm-build
22+
util/bot/nasm-win32.exe
23+
util/bot/perl-win32
24+
util/bot/perl-win32.zip
25+
util/bot/sde-linux64
26+
util/bot/sde-linux64.tar.bz2
27+
util/bot/sde-win32
28+
util/bot/sde-win32.tar.bz2
29+
util/bot/win_toolchain.json
30+
util/bot/yasm-win32.exe

API-CONVENTIONS.md

+248
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,248 @@
1+
# BoringSSL API Conventions
2+
3+
This document describes conventions for BoringSSL APIs. The [style
4+
guide](/STYLE.md) also includes guidelines, but this document is targeted at
5+
both API consumers and developers.
6+
7+
8+
## Documentation
9+
10+
All supported public APIs are documented in the public header files, found in
11+
`include/openssl`. The API documentation is also available
12+
[online](https://commondatastorage.googleapis.com/chromium-boringssl-docs/headers.html).
13+
14+
Some headers lack documention comments. These are functions and structures from
15+
OpenSSL's legacy ASN.1, X.509, and PEM implementation. If possible, avoid using
16+
them. These are left largely unmodified from upstream and are retained only for
17+
compatibility with existing OpenSSL consumers.
18+
19+
20+
## Forward declarations
21+
22+
Do not write `typedef struct foo_st FOO` or try otherwise to define BoringSSL's
23+
types. Including `openssl/base.h` (or `openssl/ossl_typ.h` for consumers who
24+
wish to be OpenSSL-compatible) will forward-declare each type without importing
25+
the rest of the library or invasive macros.
26+
27+
28+
## Error-handling
29+
30+
Most functions in BoringSSL may fail, either due to allocation failures or input
31+
errors. Functions which return an `int` typically return one on success and zero
32+
on failure. Functions which return a pointer typically return `NULL` on failure.
33+
However, due to legacy constraints, some functions are more complex. Consult the
34+
API documentation before using a function.
35+
36+
On error, most functions also push errors on the error queue, an `errno`-like
37+
mechanism. See the documentation for
38+
[err.h](https://commondatastorage.googleapis.com/chromium-boringssl-docs/err.h.html)
39+
for more details.
40+
41+
As with `errno`, callers must test the function's return value, not the error
42+
queue to determine whether an operation failed. Some codepaths may not interact
43+
with the error queue, and the error queue may have state from a previous failed
44+
operation.
45+
46+
When ignoring a failed operation, it is recommended to call `ERR_clear_error` to
47+
avoid the state interacting with future operations. Failing to do so should not
48+
affect the actual behavior of any functions, but may result in errors from both
49+
operations being mixed in error logging. We hope to
50+
[improve](https://bugs.chromium.org/p/boringssl/issues/detail?id=38) this
51+
situation in the future.
52+
53+
Where possible, avoid conditioning on specific reason codes and limit usage to
54+
logging. The reason codes are very specific and may change over time.
55+
56+
57+
## Memory allocation
58+
59+
BoringSSL allocates memory via `OPENSSL_malloc`, found in `mem.h`. Use
60+
`OPENSSL_free`, found in the same header file, to release it. BoringSSL
61+
functions will fail gracefully on allocation error, but it is recommended to use
62+
a `malloc` implementation that `abort`s on failure.
63+
64+
65+
## Object initialization and cleanup
66+
67+
BoringSSL defines a number of structs for use in its APIs. It is a C library,
68+
so the caller is responsible for ensuring these structs are properly
69+
initialized and released. Consult the documentation for a module for the
70+
proper use of its types. Some general conventions are listed below.
71+
72+
73+
### Heap-allocated types
74+
75+
Some types, such as `RSA`, are heap-allocated. All instances will be allocated
76+
and returned from BoringSSL's APIs. It is an error to instantiate a heap-
77+
allocated type on the stack or embedded within another object.
78+
79+
Heap-allocated types may have functioned named like `RSA_new` which allocates a
80+
fresh blank `RSA`. Other functions may also return newly-allocated instances.
81+
For example, `RSA_parse_public_key` is documented to return a newly-allocated
82+
`RSA` object.
83+
84+
Heap-allocated objects must be released by the corresponding free function,
85+
named like `RSA_free`. Like C's `free` and C++'s `delete`, all free functions
86+
internally check for `NULL`. Consumers are not required to check for `NULL`
87+
before calling.
88+
89+
A heap-allocated type may be reference-counted. In this case, a function named
90+
like `RSA_up_ref` will be available to take an additional reference count. The
91+
free function must be called to decrement the reference count. It will only
92+
release resources when the final reference is released. For OpenSSL
93+
compatibility, these functions return `int`, but callers may assume they always
94+
successfully return one because reference counts use saturating arithmetic.
95+
96+
C++ consumers are recommended to use `bssl::UniquePtr` to manage heap-allocated
97+
objects. `bssl::UniquePtr<T>`, like other types, is forward-declared in
98+
`openssl/base.h`. Code that needs access to the free functions, such as code
99+
which destroys a `bssl::UniquePtr`, must include the corresponding module's
100+
header. (This matches `std::unique_ptr`'s relationship with forward
101+
declarations.) Note, despite the name, `bssl::UniquePtr` is also used with
102+
reference-counted types. It owns a single reference to the object. To take an
103+
additional reference, use the `bssl::UpRef` function, which will return a
104+
separate `bssl::UniquePtr`.
105+
106+
107+
### Stack-allocated types
108+
109+
Other types in BoringSSL are stack-allocated, such as `EVP_MD_CTX`. These
110+
types may be allocated on the stack or embedded within another object.
111+
However, they must still be initialized before use.
112+
113+
Every stack-allocated object in BoringSSL has a *zero state*, analogous to
114+
initializing a pointer to `NULL`. In this state, the object may not be
115+
completely initialized, but it is safe to call cleanup functions. Entering the
116+
zero state cannot fail. (It is usually `memset(0)`.)
117+
118+
The function to enter the zero state is named like `EVP_MD_CTX_init` or
119+
`CBB_zero` and will always return `void`. To release resources associated with
120+
the type, call the cleanup function, named like `EVP_MD_CTX_cleanup`. The
121+
cleanup function must be called on all codepaths, regardless of success or
122+
failure. For example:
123+
124+
uint8_t md[EVP_MAX_MD_SIZE];
125+
unsigned md_len;
126+
EVP_MD_CTX ctx;
127+
EVP_MD_CTX_init(&ctx); /* Enter the zero state. */
128+
int ok = EVP_DigestInit_ex(&ctx, EVP_sha256(), NULL) &&
129+
EVP_DigestUpdate(&ctx, "hello ", 6) &&
130+
EVP_DigestUpdate(&ctx, "world", 5) &&
131+
EVP_DigestFinal_ex(&ctx, md, &md_len);
132+
EVP_MD_CTX_cleanup(&ctx); /* Release |ctx|. */
133+
134+
Note that `EVP_MD_CTX_cleanup` is called whether or not the `EVP_Digest*`
135+
operations succeeded. More complex C functions may use the `goto err` pattern:
136+
137+
int ret = 0;
138+
EVP_MD_CTX ctx;
139+
EVP_MD_CTX_init(&ctx);
140+
141+
if (!some_other_operation()) {
142+
goto err;
143+
}
144+
145+
uint8_t md[EVP_MAX_MD_SIZE];
146+
unsigned md_len;
147+
if (!EVP_DigestInit_ex(&ctx, EVP_sha256(), NULL) ||
148+
!EVP_DigestUpdate(&ctx, "hello ", 6) ||
149+
!EVP_DigestUpdate(&ctx, "world", 5) ||
150+
!EVP_DigestFinal_ex(&ctx, md, &md_len) {
151+
goto err;
152+
}
153+
154+
ret = 1;
155+
156+
err:
157+
EVP_MD_CTX_cleanup(&ctx);
158+
return ret;
159+
160+
Note that, because `ctx` is set to the zero state before any failures,
161+
`EVP_MD_CTX_cleanup` is safe to call even if the first operation fails before
162+
`EVP_DigestInit_ex`. However, it would be illegal to move the `EVP_MD_CTX_init`
163+
below the `some_other_operation` call.
164+
165+
As a rule of thumb, enter the zero state of stack-allocated structs in the
166+
same place they are declared.
167+
168+
C++ consumers are recommended to use the wrappers named like
169+
`bssl::ScopedEVP_MD_CTX`, defined in the corresponding module's header. These
170+
wrappers are automatically initialized to the zero state and are automatically
171+
cleaned up.
172+
173+
174+
### Data-only types
175+
176+
A few types, such as `SHA_CTX`, are data-only types and do not require cleanup.
177+
These are usually for low-level cryptographic operations. These types may be
178+
used freely without special cleanup conventions.
179+
180+
181+
### Ownership and lifetime
182+
183+
When working with allocated objects, it is important to think about *ownership*
184+
of each object, or what code is responsible for releasing it. This matches the
185+
corresponding notion in higher-level languages like C++ and Rust.
186+
187+
Ownership applies to both uniquely-owned types and reference-counted types. For
188+
the latter, ownership means the code is responsible for releasing one
189+
reference. Note a *reference* in BoringSSL refers to an increment (and eventual
190+
decrement) of an object's reference count, not `T&` in C++. Thus, to "take a
191+
reference" means to increment the reference count and take ownership of
192+
decrementing it.
193+
194+
As BoringSSL's APIs are primarily in C, ownership and lifetime obligations are
195+
not rigorously annotated in the type signatures or checked at compile-time.
196+
Instead, they are described in
197+
[API documentation](https://commondatastorage.googleapis.com/chromium-boringssl-docs/headers.html).
198+
This section describes some conventions.
199+
200+
Unless otherwise documented, functions do not take ownership of pointer
201+
arguments. The pointer typically must remain valid for the duration of the
202+
function call. The function may internally copy information from the argument or
203+
take a reference, but the caller is free to release its copy or reference at any
204+
point after the call completes.
205+
206+
A function may instead be documented to *take* or *transfer* ownership of a
207+
pointer. The caller must own the object before the function call and, after
208+
transfer, no longer owns it. As a corollary, the caller may no longer reference
209+
the object without a separate guarantee on the lifetime. The function may even
210+
release the object before returning. Callers that wish to independently retain a
211+
transfered object must therefore take a reference or make a copy before
212+
transferring. Callers should also take note of whether the function is
213+
documented to transfer pointers unconditionally or only on success. Unlike C++
214+
and Rust, functions in BoringSSL typically only transfer on success.
215+
216+
Likewise, output pointers may be owning or non-owning. Unless otherwise
217+
documented, functions output non-owning pointers. The caller is not responsible
218+
for releasing the output pointer, but it must not use the pointer beyond its
219+
lifetime. The pointer may be released when the parent object is released or even
220+
sooner on state change in the parent object.
221+
222+
If documented to output a *newly-allocated* object or a *reference* or *copy* of
223+
one, the caller is responsible for releasing the object when it is done.
224+
225+
By convention, functions named `get0` return non-owning pointers. Functions
226+
named `new` or `get1` return owning pointers. Functions named `set0` take
227+
ownership of arguments. Functions named `set1` do not. They typically take a
228+
reference or make a copy internally. These names originally referred to the
229+
effect on a reference count, but the convention applies equally to
230+
non-reference-counted types.
231+
232+
API documentation may also describe more complex obligations. For instance, an
233+
object may borrow a pointer for longer than the duration of a single function
234+
call, in which case the caller must ensure the lifetime extends accordingly.
235+
236+
Memory errors are one of the most common and dangerous bugs in C and C++, so
237+
callers are encouraged to make use of tools such as
238+
[AddressSanitizer](https://clang.llvm.org/docs/AddressSanitizer.html) and
239+
higher-level languages.
240+
241+
242+
## Thread safety
243+
244+
BoringSSL is internally aware of the platform threading library and calls into
245+
it as needed. Consult the API documentation for the threading guarantees of
246+
particular objects. In general, stateless reference-counted objects like `RSA`
247+
or `EVP_PKEY` which represent keys may typically be used from multiple threads
248+
simultaneously, provided no thread mutates the key.

0 commit comments

Comments
 (0)