-
Notifications
You must be signed in to change notification settings - Fork 704
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merge BoringSSL through 4c8bcf0da2951cacd8ed8eaa7fd2df4b22fca23b #1658
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Change-Id: I330ac0fa7f0b2c9984d12da831d8f34019ea2c49 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58526 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: Adam Langley <agl@google.com>
Change-Id: I6e53434246f3fef06d4f88924bfe1cbfad0543e8 Bug: chromium:1414562 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58205 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: Steven Valdez <svaldez@google.com>
Thanks to wangjiale3 for noticing a leak of ext_oct on malloc error in https://boringssl-review.googlesource.com/c/boringssl/+/58488. This is a fixed version of that CL, and also fixes a leak of ext_der on malloc failure. Change-Id: I2c7ece6b2950a9cb807d78e72b2fddc21897a019 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58686 Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com>
Bug: 586 Change-Id: I5bc8e6df3a5a14e6b218f41181d06406e835f9c1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58605 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Bob Beck <bbe@google.com>
This reverts a small portion of 8c8629b. The parsers for ANY remain unchanged, but we inadvertently changed a corner case of ASN1_PRINTABLE MSTRINGs. This is a huge mess. utype in these switch cases is usually the type of the ASN1_ITEM, but, with ANY and MSTRING, it is the tag of the value we found. (An MSTRING or "multi-string" is a CHOICE of string-like types.) When parsing ANY, this is moot because the is_supported_universal_type logic ensures we'll never pass in an invalid type. When encoding ANY, this only happens if you manually construct such an ASN1_TYPE. MSTRINGs *should* be similar because of the bitmask they apply on tag types. However, there is one MSTRING type whose bitmask, B_ASN1_PRINTABLE, includes B_ASN1_UNKNOWN. ASN1_tag2bit, arbitrarily maps eight unsupported tags to B_ASN1_UNKNOWN and instead of zero. These are: - ObjectDescriptor - EXTERNAL - REAL - EMBEDDED PDV - RELATIVE-OID - TIME (note this is not the same as the X.509 Time CHOICE type) - [UNIVERSAL 15], which is not even a defined type! - CHARACTER STRING (ENUMERATED is also mapped to B_ASN1_UNKNOWN, but it's supported.) These eight tags were previously accepted in d2i_X509_NAME but 8c8629b inadvertently started rejecting them. For now, restore the default in the switch/case so that we accept them again. Per https://crbug.com/boringssl/412, attribute values are ANY DEFINED BY types, so we actually should be accepting *all* types. We do not, because B_ASN1_PRINTABLE is completely incoherent. But because ANY is the correct type, going from the original incoherent set, to this new, smaller incoherent set is arguably a regression. This is a minimal fix. Long-term, we should handle that ANY correctly, and avoid unexpected ASN1_STRING type values, by mapping all unsupported types to V_ASN1_OTHER. This would allow us to support all types correctly. A follow-up change will do that. Update-Note: The X.509 name parser will go back to accepting a handful of universal tag types that were inadvertently rejected in 8c8629b. It is extremely unlikely that anyone uses these as they're unsupported, obscure types. This CL also makes our ASN1_TYPE encoder slightly more permissive again, if the caller manually constructs an legacy in-memory representation of an unsupported tag. But the follow-up change will restore the stricter behavior. Bug: 412, 561 Change-Id: Ia44a270f12f3021154761a1cd285707416d8787e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58705 Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: Bob Beck <bbe@google.com>
I don't believe these codepaths have ever been run. All the built-in X509V3_EXT_METHODs are ASN1_ITEM-based, as are all callers I found of X509V3_EXT_add and X509V3_EXT_add_list. Also document not to use those APIs because they're pointless and (for now) not even thread-safe. Making them thread-safe is doable, but it'd add rwlock contention in certificate verification, unless we first rework certificate verification to ignore custom registrations, because it never uses them anyway. But that only proves that this whole feature was pointless, so this time may be better spent trying to get rid of this API. Update-Note: Externally-installed X509V3_EXT_METHODs now must be ASN1_ITEM-based. I believe all existing ones already are. If there are any that aren't, let us know. We'll either revert this for now, or export a way to implement custom ASN1_ITEMs, or, most likely, try to move the caller off custom X509V3_EXT_METHODs altogether. Like most of OpenSSL's other global registration APIs, this one is unsafe (two callers may conflict) and there isn't much reason to register it with the library because OpenSSL doesn't do much with the registration anyway. Bug: 590 Change-Id: Ice0e246d50069e10e6cca8949f60fac474d0846c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58687 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
These are already unused, though add and add_alias will need more work. In doing so, simplify the X509V3_EXT_DYNAMIC business. I added some cleanup calls to https://boringssl-review.googlesource.com/2208, but that should have been in the error-handling path of X509V3_EXT_add_alias, the only case that cares about this. Update-Note: Removed unused API. Bug: 590 Change-Id: Idd97366d90d7aab0ca2e020c76a7c8065b3dd7ff Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58765 Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com>
…its original state keeping the same key as initially used Change-Id: Ie781e2a20da26b50b34f35ea0a5cfc578b64ee7f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58565 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com>
HRSS itself remains in libcrypto because there are some direct users of it. But this will let it be dropped by the linker in many cases. Change-Id: I870eda30c9ed1d08693c770e9e7df45a2711b7df Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58645 Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I7c5b0a24c26b83779cf889d890e2c18ae13187c3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58725 Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: I32a40a73f96e029ac9096af826d15b22d9dcad28 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58745 Auto-Submit: Adam Langley <agl@google.com> Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
…icer with other build systems like Soong and Gn. Change-Id: I42e40da22dd243796cd735e09a9821cc2d114200 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58785 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com>
Change-Id: Ib3372c2a4e0e6189402485e87963d8151ad29981 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58786 Reviewed-by: Adam Langley <agl@google.com> Reviewed-by: danakj <danakj@chromium.org> Commit-Queue: Bob Beck <bbe@google.com>
This relands https://boringssl-review.googlesource.com/c/boringssl/+/54606, which was temporarily reverted. Update-Note: By default, clients will now require RSA server certificates used in TLS 1.2 and earlier to include the keyEncipherment or digitalSignature bit. keyEncipherment is required if using RSA key exchange. digitalSignature is required if using ECDHE_RSA key exchange. If unsure, TLS RSA server signatures should include both, but some deployments may wish to include only one if separating keys, or simply disabling RSA key exchange. The latter is useful to mitigate either the Bleichenbacher attack (from 1998, most recently resurfaced in 2017 as ROBOT), or to strengthen TLS 1.3 downgrade protections, which is particularly important for enterprise environments using client certificates (aka "mTLS") because, prior to TLS 1.3, the TLS client certificate flow was insufficiently encrypted or authenticated. Without reflecting an RSA key exchange disable into key usage, and then the client checking it, an attacker can spoof a CertificateRequest as coming from some server. This aligns with standard security requirements for using X.509 certificates, specified in RFC 5280, section 4.2.1.3, and reiterated in TLS as early as TLS 1.0, RFC 2246, section 7.4.2, published 24 years ago on January 1999. Constraints on usage of keys are important to mitigate cross-protocol attacks, a class of cryptographic attacks that is well-studied in the literature. We already checked this for each of ECDSA, TLS 1.3, and servers verifying client certificates, so this just fills in the remaining hole. As a result, this change is also important for avoiding some weird behaviors when configuration changes transition a server in or out of this hole. (We've seen connection failures get misattributed to TLS 1.3 when it was really a certificate misconfiguration.) Chrome has also enforced this for some time with publicly-trusted certificates. As a temporary measure for callers that need more time, the SSL_set_enforce_rsa_key_usage API, added to BoringSSL in January 2019, still exists where we need to turn this off. Fixed: 519 Change-Id: I91bf2cfb04c92aec7875e640f90ba6f837146dc1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58805 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com>
NIST has deprecated the test that we were using and replaced it with the one that this change switches BoringSSL to using. Change-Id: Iff975cda33153f8db42d9c01457d104c502485b9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58787 Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: David Benjamin <davidben@google.com>
On Android, when running from an APK, |tmpfile| does not work. See b/36991167#comment8. Change-Id: I1415471907e61da5e8c8d1530a2b915fcd991d53 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58845 Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com>
(This change breaks running on Android. But it's close to what we used for non-Android FIPS testing so is useful to have on the record.) This change adds testing of the run-time break tests: the pair-wise consistency tests and the RNG test. It also switches to using two test_fips binaries: an unmodified one for showing a clean up and testing the integrity test, and a test_fips_break which makes integrity test failures non-fatal (for KAT testing) and which allows the run-time tests to be triggered. Change-Id: Id2787723059cfb17cc2d22013ad66b985ef86701 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53885 Reviewed-by: David Benjamin <davidben@google.com> Auto-Submit: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
Change-Id: Ibe428995fbc03669bf822296741574e2bbeb482d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58865 Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: David Benjamin <davidben@google.com>
Bug: 516 Change-Id: I3f374f05188bebe7aa4cbf45c81a6f945d3ce97c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58549 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com>
Change-Id: I2218807c6bfe445460a01f6c86712640915e87df Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57666 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
I had a branch lying around to rewrite X509_NAME_print(_ex) because those functions are a disaster, but it needs more work and probably isn't high priority. In the meantime, document what we've got. Also tidy up X509_print_ex slightly. m was completely unused and some variable declarations could be moved closer to their definition. Bug: 426 Change-Id: I24295048c36268c745f579ad66f34736cfe6830f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58925 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com> Auto-Submit: David Benjamin <davidben@google.com>
Also organize the low-level signature verification functions. I missed those in the first pass. Bug: 426 Change-Id: I9c93d643d8f0f77a35ee132f31377ba447f2f2f1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58926 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com>
They're not used anywhere, as X509_REQ doesn't expose the underlying STACK_OF(X509_ATTRIBUTE) anyway. They're also very thin wrappers over the stack functions, so just delete them and inline them into X509_REQ functions. While I'm here, I've tidied up the add1_attr_by_* functions to reduce an unnecessary copy. Change-Id: Iec002c83ab7ad7267314e98866d680d12a82e971 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58927 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
These probably don't need their own section. They're just thin wrappers over other ASN1_TIME functions. Bug: 426 Change-Id: I8672feb0ca7ba1cf69b56d02d2559de5b80a3ee7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58928 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
Change-Id: I54195f3fabbed788d3cf299d478d5151acfe2a4f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58929 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com>
This has been on by default since rust-lang/rust-bindgen@cc78b6f, and now removed from recent bindgen altogether. Change-Id: Iea4c2a7480fe8b138c375686ca6b36e6d68257b3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58965 Reviewed-by: Nabil Wadih <nwadih@google.com> Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com>
These appear to be unused. Some ones of note: - XN_FLAG_FN_ALIGN breaks with multi-attribute RDNs anyway - XN_FLAG_FN_NONE is completely pointless Update-Note: Some seemingly unused XN_FLAG_* values were removed. If some project fails to build, we can put them back but one shouldn't be using this function in the first place. Change-Id: I4d8472e1e31aeec623b4d4e2aea48da7b1ef6798 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58930 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
Newer CMakes support a -B parameter, which saves some fuss. Change-Id: Ifdbbb50b3720cdc42af098eb32941283692e9d99 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58966 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> Reviewed-by: Nabil Wadih <nwadih@google.com>
The allowlist is just a regex, which means bindgen leaves it to the user to resolve Windows vs POSIX filepath differences. We need to support both / and \. It's unclear why only some headers are broken, but it's probably something to do with whether the header is included directly or indirectly. Unfortunately, in doing so, we run into a mess of escaping issues, so the regex is more permissing than ideal. Bug: 595 Change-Id: I8b785aeaaeff162d9eb2aced89928f9602445903 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58967 Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com>
Change-Id: I6e0361a42b9612ba4294cc8806203ea445bc9257 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58945 Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com>
This can still be run like go run ../../util/convert_wycheproof. This is part of an attempt to reland 54b04fd, which ran into an issue with internal tooling that could not handle standalone Go files. Since the only such target we actually needed to run in that repository is convert_wycheproof, just promote it into its own package. Then we can stop trying to import util. Change-Id: I6237777dad09e5c81ad961816ce14a287ab2d46a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59185 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com>
This reverts commit bab2f96. This clears the sea of red in my editor. Change-Id: I600ef6c36556fb526da729f0f0d8bc69db5c5a08 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59186 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
This aligns the DLEQ proof portion of TRUST_TOKEN_pst_v1_voprf with draft-irtf-cfrg-voprf-21. The blind and finalize operations still differ. Additionally, as VOPRF doesn't include batched issuance, the issuance process around the DLEQ proof is adapted from draft-robert-privacypass-batched-tokens-01. Bug: chromium:1414562 Change-Id: If1c6de0f92089a826968a57279ae598ccf89ca3e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58906 Commit-Queue: Steven Valdez <svaldez@google.com> Reviewed-by: David Benjamin <davidben@google.com>
We were currently squeezing 3 bytes at a time. And because kyber.c and keccak.c are separate files, we've got an optimization barrier, at least in non-LTO builds, at a very inconvenient place. Instead, extract the full 168 bytes (SHAKE-128 rate) at a time. This is conveniently a multiple of three, so we don't need to worry about partial coefficients. We're still doing a copy due to the Keccak abstraction, but it should extend cleanly to either LTO or a different abstraction to read from the state directly. Even without that, it's a significant perf win. Benchmarks on an M1: Before: Did 87390 Kyber generate + decap operations in 4001590us (21838.8 ops/sec) Did 119000 Kyber parse + encap operations in 4009460us (29679.8 ops/sec) After: Did 96747 Kyber generate + decap operations in 4003687us (24164.5 ops/sec) [+10.6%] Did 152000 Kyber parse + encap operations in 4018360us (37826.4 ops/sec) [+27.4%] Change-Id: Iada393edf0c634b7410a34374fb90242a392a9d3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59265 Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
We previously used the functions from the cipher language to define it, but it's more straightforward to just list them out. Change-Id: I1467d6db47a93c8443a0a448ef974c827b1b3233 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59146 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
When building BCM sources individually, this gets missed. Change-Id: I58858da441daaeffc5e54b653f5436fe817c4178 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59306 Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: Adam Langley <agl@google.com>
Prior to https://boringssl-review.googlesource.com/c/boringssl/+/58548, ASN1_item_sign_ctx returned the length of the signature on success. It's unclear why anyone would ever want this, but some test was sensitive to it. (I think it was a typo.) Restore the old behavior. Change-Id: Ibf3e45331a339226744d51df703634d02b08a7c4 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59307 Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com>
The reason to make it a package was to avoid needing this, but I missed that git put it back. Change-Id: Idd6df275aa964083db525d4d5e300128b204d973 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59305 Commit-Queue: Bob Beck <bbe@google.com> Reviewed-by: Bob Beck <bbe@google.com> Auto-Submit: David Benjamin <davidben@google.com>
Instead, just have it look for the files it needs via a BORINGSSL_BUILD_DIR environment variable. This avoids hardcoding "../../build" anywhere that cannot be easily overriden by the user. Although this puts logic in a build.rs file, which is problematic for repositories with more coherent build stories like Android or Chromium, those are already driving the bindgen and link process themselves, without calling CMake. I.e. this file should already only be used for standalone development and testing and not directly impact them. (Though we'd like to keep it vaguely analogous to better predict without a change will impact downstream folks.) For now, I've kept bindgen generated from CMake, mostly in anticipation of using the inline functions feature. Building the synthesized C file from CMake seems less of a headache than Cargo. Additionally, calling bindgen from the command-line is closer to how those consumers will do it, so this forces us to stick to bindgen invocations that can be expressed via command-line arguments. (E.g. the mess that is regexes and escaping.) As part of this, I've removed the messy "find the first matching wrapper file" behavior in build.rs. Instead, it knows the expected TARGET and just finds the file with matching name. This means we'll be stricter about matching the two. (Otherwise there's no point in naming it by target name anyway.) Fixed: 598 Change-Id: I07fa74f7e5f5f008d6f0ceec648a2378df7d317a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59105 Reviewed-by: Matthew Maurer <mmaurer@google.com> Reviewed-by: Nabil Wadih <nwadih@google.com> Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com>
We have since added an implementation of a subset of the SSL BIO, but we don't implement all the features, notably some of the BIO_ctrl values. Remove them, so it doesn't look like they should work. Update-Note: I found no code using those symbols (that we build). If anything was, they most likely were broken. Now they'll fail to build and the brokenness will be more obvious. (If we find something needs it, we can always go back and implement them.) Fixed: 420 Change-Id: Iad03fa65f098023dca555a9b2ac0214ba4264546 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59125 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com>
Also fixes some copy-paste errors in earlier docs. Bug: 426 Change-Id: I330445477b6feb50f65a868130387804114f23a8 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59205 Commit-Queue: David Benjamin <davidben@google.com> Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
AllCurves interferes with cross-version handshake hint tests. If any curve is removed, the test breaks. This is a particular nuisance for signing tests, where we'd rather like to see cross-version hint compatibility. It's also a nuisance for writing test suppressions because the remove curve is not actually listed in the test name. The signing tests don't actually need to enable all curves. They just need to handle some TLS 1.2 ECDSA rules. Fix that by having the test know the expected curve and to configure it explicitly. Hopefully that'll reduce the exceptions needed in the future. Change-Id: I432e084c49a943afc92726ccf0b73658e7bd30b1 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59325 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com> Auto-Submit: David Benjamin <davidben@google.com>
It's unwise for organisations to try and define TLS profiles. As in this case, they usually make security worse. However, since this is already established and supported by Android, this change raises it to the level of a supported policy. Change-Id: Ic66d5eaa33d884e57fc6d8eb922d86882b621e9e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58626 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: Adam Langley <agl@google.com>
As part of getting a handle on RSA state, I would like for RSA keys created from parsing to come pre-"frozen". This reduces some contention on first use. But that potentially breaks an existing use case: today, you're allowed to parse a private key and then override one field without problems, because none of the cached state has materialized yet. If the caller modified the RSA key by reaching into the struct, it's hopeless. If they used the setters, we actually can handle it correctly, so go ahead and make this work. DH and DSA aren't of particular interest to us, but fix them while I'm at it. This also avoids having to later document that something doesn't work, just that it's a terrible API. Bug: 316 Change-Id: Idf02c777b932a62df9396e21de459381455950e0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59385 Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I49920d3917f0aebf1b9efbd45d0bcd944d6c8117 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59405 Auto-Submit: Adam Langley <agl@google.com> Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: Adam Langley <agl@google.com>
Per the bug discussion, FreeBSD seems to default to a cap of 1500 threads per process. Just turn the test off. But enable the test unconditionally if building with TSan. With TSan on, we only spawn two threads, which should be within everyone's bounds. Fixed: 603 Change-Id: Ic8c49e5ce7c3f2d09487debc72b7e4c54b04a77c Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59445 Reviewed-by: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
This adds APIs for constructing RSA keys given all the parameters. This is much less tedious than the set0 functions, and also avoids issues caused by deferred initialization. It doesn't quite finish initializing the keys on construction, as that is tied up in the rest of this mess. But later work, probably after Conscrypt is moved to these APIs, will do this. As part of this, add APIs that explicitly create RSA keys with no e. There is actually no way to do this right now without reaching into library internals, because RSA_set0_key refuses to accept an e-less private key. Handle this by adding a flag. I also had to add a path for Conscrypt to make an RSA_METHOD-backed key with n but no e. (They need n to compute RSA-PSS padding.) I think that all wants to be rethought but, for now, just add an API for it. This bumps BORINGSSL_API_VERSION so Conscrypt can have an easier time switching to the new APIs. Bug: 316 Change-Id: I81498a7d0690886842016c3680ea27d1ee0fa490 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59386 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
Satisfy a go lint. As of Go 1.18, any is the preferred spelling of interface{}. Also remove an instance of redundant types in util/fipstools/acvp/acvptool/acvp.go because my editor warned about it. (A []map[string]any{map[string]any{...}, map[string]any{...}} literal can omit the inner copy of the type because it's implicit from the outer one.) Change-Id: I2251b2285c16c19bc779fa41d1011f7fa1392563 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59465 Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com>
It is now RFC 8452. The final RFC also has a few more test vectors, so import those too. Change-Id: Ib7667802973df7733ba981f16ef6a129cb4f62e7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59485 Commit-Queue: David Benjamin <davidben@google.com> Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: Adam Langley <agl@google.com>
Relevant spec bits: https://www.rfc-editor.org/rfc/rfc9180.html#section-4.1 https://www.rfc-editor.org/rfc/rfc9180.html#section-5.1.3 Change-Id: Iddb151afc92f7a91beb9ca52caceec6cb5383206 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/59387 Reviewed-by: Adam Langley <agl@google.com> Auto-Submit: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.