Update to upstream ae88f19 #100
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
The output of ASN1_generate_v3 is *mostly* linear with the input, except SEQ and SET reference config sections. Sections can be referenced multiple times, and so the structure grows exponentially. Cap the total output size to mitigate this. As before, we don't consider these functions safe to use with untrusted inputs, but unbounded growth will frustrate fuzzing. This CL sets the limit to 64K, which should be enough for anyone. (This is the size of a single X.509 extension, whereas certificates themselves should not get that large.) While not strictly necessary, this also rearranges the ASN1_mbstring_copy call to pass in a maximum output. This portion does scale linearly with the output, so it's fine, but the fuzzer discovered an input with a 700K-byte input, which, with fuzzer instrumentation and sanitizers, seems to be a bit slow. This change should help the fuzzer get past those cases faster. Update-Note: The stringly-typed API for constructing X.509 extensions now has a maximum output size. If anyone was constructing an extension larger than 64K, this will break. This is unlikely and should be caught by unit tests; if a project hits this outside of tests, that means they are passing untrusted input into this function, which is a security vulnerability in itself, and means they especially need this change to avoid a DoS. Bug: oss-fuzz:55725 Change-Id: Ibb65854293f44bf48ed5855016ef7cd46d2fae77 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57125 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> Auto-Submit: David Benjamin <[email protected]>
This change exposes the necessary APIs for rust-openssl's HKDF wrappers in [PKeyCtxRef] (https://docs.rs/openssl/latest/openssl/pkey_ctx/struct.PkeyCtxRef.html#method.set_hkdf_mode). Diff of the generated bindings: https://gist.github.com/mauricelam/51602bf17d4a5f9023a27cbe5f9ff705 Change-Id: Ic8df3d5308cfee5964ceadd3c2524b77e12512fb Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57066 Reviewed-by: Alex Gaynor <[email protected]> Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]>
Especially when they were named "2214" instead of "chromium-2214", I've seen papers and other projects treat them as releases. Add a note to make it clear they are not releases. Change-Id: Ie820b800de3d25a31d3083d4ceff75e1d7f74a06 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57145 Reviewed-by: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]>
This includes an internal version which allows a flag to specify the use of system malloc, or OPENSSL_malloc - this in turn allows us to use this function in the ERR family of functions and allow for ERR to not call OPENSSL_malloc with a circular dependency. Bug: 564 Change-Id: Ifd02d062fda9695cddbb0dbef2e1c1db0802a486 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57005 Auto-Submit: Bob Beck <[email protected]> Reviewed-by: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]>
This will let us call ERR and thread_local from OPENSSL_malloc without creating a circular dependency. We also make ERR_get_error_line_data add ERR_FLAG_MALLOCED to the returned flags value, since some projects appear to be making assumptions about it being there. Bug: 564 Update-Note: Any recent documentation (in all OpenSSL forks) for the ERR functions cautions against freeing the returned ERR "data" strings, as freeing them is handled by the error library. This change can make an existing double free bug more obvious by being more likely to cause a crash with the double free. Change-Id: Ie30bd3aee0b506473988b90675c48510969db31a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57045 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]> Auto-Submit: Bob Beck <[email protected]>
Decoding from decimal takes quadratic time, and BN_dec2bn will happily decode however large of input you pass in. This frustrates fuzzers. I've added a cap to the input length in s2i_ASN1_INTEGER for now, rather than BN_dec2bn, because we've seen people use BN for surprisingly large calculator operations, and BN generally doesn't cap inputs to quadratic (or worse) algorithms beyond memory limits. (We generally rely on cryptography using fixed parameter sizes, though RSA, DSA, and DH were misstandardized and need ad-hoc limits everywhere.) Update-Note: The stringly-typed API for constructing X.509 extensions now has (very generous) maximum input length for decimal integers of 8,192 digits. If anyone was relying on a higher input, this will break. This is unlikely and should be caught by unit tests; if a project hits this outside of tests, that means they are passing untrusted input into this function, which is a security vulnerability in itself, and means they especially need this change to avoid a DoS. Bug: chromium:1415108 Change-Id: I138249d23ca6b1996f8437dba98633349bb3042b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57205 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]>
Remove all the other ERR_R_MALLOC_FAILURES from the codebase. Also changes cbb to push to the error stack, to correctly report cbb failures instead of now only reporting malloc failures. Previously it turned all cbb failures into a malloc failure Bug: 564 Change-Id: Ic13208bf9d9aaa470e83b2f15782fc94946bbc7b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57046 Auto-Submit: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> Reviewed-by: David Benjamin <[email protected]>
The const bool doesn't do anything. While I'm here, make the methods const. Change-Id: Id8c31d5fcda6d8bc244c64b02b1d758e4eff6849 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57185 Auto-Submit: David Benjamin <[email protected]> Commit-Queue: Steven Valdez <[email protected]> Reviewed-by: Steven Valdez <[email protected]>
draft-07 to draft-16 is mostly editorial, but there were a few notable changes: - Empty DST values are forbidden. - The sample implementation for map_to_curve_simple_swu has completely changed. The new formulation has the same performance (if not a smidge faster), and aligning with the spec seems generally useful. - P-384 is now paired with SHA-384, not SHA-512. As this would be a breaking change for the trust tokens code, I've left that in. A follow-up CL will add implementations of draft-16, which is expected to match the final draft. Before: Did 77000 hash-to-curve P384_XMD:SHA-512_SSWU_RO_ operations in 4025677us (19127.2 ops/sec) Did 7156000 hash-to-scalar P384_XMD:SHA-512 operations in 4000385us (1788827.8 ops/sec) After: Did 77000 hash-to-curve P384_XMD:SHA-512_SSWU_RO_ operations in 4009708us (19203.4 ops/sec) [+0.4%] Did 7327000 hash-to-scalar P384_XMD:SHA-512 operations in 4000477us (1831531.6 ops/sec) [+2.4%] Bug: 1414562 Change-Id: Ic3c37061e325250d5d8723fd9aa263930c6023cf Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57146 Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Steven Valdez <[email protected]> Commit-Queue: Steven Valdez <[email protected]>
Also add public APIs for this, now that the specification is no longer expected to change, and because a project external to the library wishes to use it. For now, I've kept the P-256 version using the generic felem_exp, but we should update that to use the specialized field arithmetic. Trust Tokens will presumably move to this later and, in the meantime, another team wants this. Bug: chromium:1414562 Change-Id: Ie38203b4439ff55659c4fb2070f45d524c55aa2a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57147 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Steven Valdez <[email protected]>
Compilers are fine at inlining functions nowadays. We can hide the BN_ULLONG vs. manual carry extraction inside an inline function. I've patterned the type signatures intentionally after Clang's builtins, in case we want to use them in the future. (Previously I wrote in https://boringssl-review.googlesource.com/c/boringssl/+/56966 that the builtins weren't good on aarch64. This wasn't quite right. Rather, they were bad on both x86_64 and aarch64 in LLVM 13, but they're fine on both in LLVM 14. My machine's Xcode was just a little old.) Change-Id: I666466dce7a146d5e49e94ff372ea018b610ef34 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57245 Commit-Queue: David Benjamin <[email protected]> Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
It's unclear to me whether doing it target-by-target is an improvement in crypto/fipsmodule, but this otherwise does seem a bit tidier. This aligns with CMake's documentation and "modern CMake" which prefers this pattern. Change-Id: I36c81842bff8b36eeaaf5dd3e0695fb45f3376c9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56585 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
Also stick the very verbose default install directory in a variable so we don't have to repeat it everywhere. Change-Id: I1a6a85c4d42d3a6e766e52b2d0ecd8e81c6ed4e3 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56607 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
This was added with the generated symbol-prefixing header. But it seems to be sufficient for crypto to have a dependency on the generated header, along with some of the stray bits of delocate. It's a little unclear from CMake documentation how these are processed; normally .o files can be built before libraries are built or linked, only the link step depends on. But, empirically, if A links B, and B has a dependency on C, then CMake seems to run C before building any of A. I tested this by making a small project where the generation step slept for three seconds and running with enough parallelism that we'd have tripped. Interestingly, in the Makefile output, the individual object file targets didn't have that dependency, but the target itself did. But this was true on both A and B, so I think that just might not work. Also fix the dependency in the custom target. The old formulation broke when using an absolute path to the symbols file. Change-Id: I2053d44949f907d465da403a5ec69c191740268f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56928 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
Nothing uses this, and the code is somewhat decrepit. Instead of fixing it and continuing to maintain it as attack surface, we send this off to the farm where it can run and play all day with the other unused X.509 extensions. Update-note: This removes the proxy certificate extension from the recognized certificate extensions. Previously by default a certificate with a critical proxy certificate extension would have been rejected with "proxy certificate not allowed", but will now be rejected with an unrecognized critical extension error. Fixed: 568 Change-Id: I5f838d69c59517254b4fa83f6e2abe6057fa66c7 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57265 Commit-Queue: Bob Beck <[email protected]> Reviewed-by: David Benjamin <[email protected]> Auto-Submit: Bob Beck <[email protected]>
Change-Id: I0b1ba546374aa8b0fe79528f56e19f261536e565 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57305 Commit-Queue: Bob Beck <[email protected]> Reviewed-by: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]>
bindgen, by default, will bind every random symbol in the libc, which is clearly unreasonable. Now that --allowlist-file exists, we can switch to doing what it should have done from the beginning. This produces a pretty large diff in the bindgen output, but it's all to exclude miscellaneous bits of libc. Change-Id: I9a35fda10ff6f1b82449919f9fcc2ea86ad5b802 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57325 Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Alex Gaynor <[email protected]> Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Bob Beck <[email protected]>
Change-Id: I5682358b5564dbaa734c5910c11a8274e88ca67a Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57186 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]>
Prior to 3.12 (which we won't be requiring until July), OBJECT libraries cannot be used with target_link_libraries. That means they cannot pick up INTERFACE_INCLUDE_DIRECTORIES, which makes them pretty unusable in the "modern CMake" style. Just switch it to a static library to unbreak the build in CMake 3.10. For some link ordering reason I don't understand, this also requires explicitly linking boringssl_gtest to libcxx when we USE_CUSTOM_LIBCXX is set. Change-Id: Ia9d8351551f5da060248aa3ca73fe04473bf62aa Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57345 Commit-Queue: Bob Beck <[email protected]> Reviewed-by: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]>
While hiding 'type' isn't such a huge deal, accessing 'pkey' without a type check is very dangerous. The accessors are type-checked and avoid this problem. It also gets us slightly closer to not needing to utter CRYPTO_refcount_t in public headers, as we're currently not quite declaring it right. And it allows us to remove another union: https://boringssl-review.googlesource.com/c/boringssl/+/57106 This matches what upstream did in OpenSSL 1.1.0. Update-Note: Code that reaches into the EVP_PKEY struct will no longer compile, like in OpenSSL. I believe I've fixed all the cases. If I missed any, the fix is to switch code to accessors. EVP_PKEY_id(pkey) for pkey->type is the most common fix. Change-Id: Ibe8d6b6cb8fbd141ea1cef0d02dc1ae3703e9469 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57105 Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
The union isn't actually providing type-safety: nothing checks that we access the correct arm of the union, and it has a void* arm anyway. Instead, it's just adding some strict aliasing risk by relying on type-punning: we usually write to the pointer as void*, via EVP_PKEY_assign, but then we read from it as the underlying type. This is allowed in C, but not in C++. And even in C, while that is allowed, if we ever wrote &pkey->pkey.rsa, it would suddenly be a strict aliasing violation. Just use a void*, which means we don't type-pun pointer types against each other. While I'm here, I made the free callbacks for EVP_PKEYs also NULL the pointer. The one caller also NULLs it, so its fine, but some did and some didn't do it, and this seems prudent. Bug: 301 Change-Id: I74c76ed3984527df66f64bb2d397af44f63920bd Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57106 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
This convention seems to break with some other tooling we have. Until we figure out how to resolve that, remove the lines. This partially reverts 54b04fd but keeps the fixes to the license header comments. Change-Id: I4f08a9f3daf65d17b4c78ac6f4ac3de234ec3436 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57366 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Bob Beck <[email protected]> Commit-Queue: David Benjamin <[email protected]> Auto-Submit: David Benjamin <[email protected]>
Change-Id: I7dea47e73cbfbfa48a8924f3a633215c06730b22 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57425 Reviewed-by: Adam Langley <[email protected]> Reviewed-by: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]>
This extends the support for execute-only memory in AArch64 assembly and uses adrp and add instead of adr. Change-Id: I388a13ec754e7f179d7a234516f1bb4ff6a5c919 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57446 Reviewed-by: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
Ran go get -u all, followed by go mod tidy. Some tools are flagging CVE-2021-43565 and CVE-2022-27191 in some of the Go packages. Our uses of x/crypto are x/net are not impacted by either bug, but update anyway to silence the tools. Change-Id: Ia0e2757625b58d964aedd4217f21b72f293b910b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57485 Commit-Queue: Bob Beck <[email protected]> Reviewed-by: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
It happened that it used to be possible to run `acvptool` with `-json` and not need a config file. That stopped working as of 02397c7 but the test runner needs to be updated accordingly. Change-Id: I54cf0fc7420d18749e93c3d85201ba24d0a59e15 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57465 Commit-Queue: Adam Langley <[email protected]> Auto-Submit: Adam Langley <[email protected]> Reviewed-by: David Benjamin <[email protected]>
Go has a bzip2 decoder but not an encoder (because I only needed a decoder when I wrote the package). Thus when updated ACVP tests are written they aren't compressed. This change removes the `.bz2` suffix from paths when writing updated tests, which makes it easier to compress them. Change-Id: I9de6a1845cdf1cddca2e5d253b97262b023393f6 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57466 Auto-Submit: Adam Langley <[email protected]> Reviewed-by: David Benjamin <[email protected]>
1740ff9 added an "algorithm" key to the output. The test expectations need to be updated accordingly. Also drops 3DES tests since they were removed from the regcap in 8241345. Change-Id: Ibd23f6166111be361511c2f7974348400f7151e2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57467 Reviewed-by: David Benjamin <[email protected]> Auto-Submit: Adam Langley <[email protected]>
After https://chromium-review.googlesource.com/c/chromium/tools/build/+/4277422, the old ANDROID_NATIVE_API_LEVEL values are no longer in CMakeCache.txt. Change-Id: If0c724081ef99bf3bc2e714fe84b6d16925bd116 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57507 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
OpenSSL's APIs are full of empty objects that aren't captured in the type system. A DSA object may represent any of nothing, a group, a public key, or a private key. Since the type system doesn't capture this, better to check which type we've got and NULL-check the fields we need for the operation. Change-Id: I32b09ebaad58fcb2a0bfc9ad56d381f95099bf7b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/57225 Reviewed-by: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]>
That NULL+0 is forbidden is still an awful language bug in C (fixed in C++), but this particular instance could have been written without pointer arithmetic. While I'm here, tidy pointers a bit: - No need to cast pointers to char* when we're writing to void* anyway - BIO_C_GET_BUF_MEM_PTR is technically a strict aliasing violation. The pointer points to a BUF_MEM*, not a char*, so we should not write to it as a char**. - C casts from void* freely and we've usually omitted the cast in that case. (Though if we ever move libcrypto to C++, that'll all have to change.) Bug: b:286384999 Change-Id: I16d7da675d61f726f259fc9a3cc4a6fce2d6d1fd Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60605 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]>
This probably needs a few iterations, but fix the stuff I noticed on a first pass: - I don't think it's useful to say this is the BoringSSL implementation of something. That's all implicit from this crate anyway. - Rust seems to prefer "Returns ..." rather than "Return ..." - Algorithm names like "hkdf" or "hmac" should be written "HKDF" or "HMAC" when referring to the algorithms. Also BoringSSL is styled "BoringSSL" rather than "boringssl". - Given Rust overall doesn't try to handle allocation failure, let's just write that we don't believe in allocation failure once in the README rather than marking which functions do and don't panic. Change-Id: I48501717dd0b063a2fa4106c4c140d76a7ef69a9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60546 Reviewed-by: Nabil Wadih <[email protected]> Commit-Queue: Bob Beck <[email protected]> Reviewed-by: Bob Beck <[email protected]>
These bits need more work (and possibly some removal) as they're very, very far from thread-safe, but rust-openssl relies on them being const-correct when targetting OpenSSL 1.1.x. Change-Id: I60531c7e90dbdbcb79c09fc440bd7c6b474172df Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60607 Auto-Submit: David Benjamin <[email protected]> Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Bob Beck <[email protected]>
We added SSL_CIPHER_get_prf_nid to match the other SSL_CIPHER_get_*_nid functions, but OpenSSL went with returning the EVP_MD instead. rust-openssl uses this function, and all the callers of SSL_CIPHER_get_prf_nid then call EVP_get_digestbynid anyway, so this version is preferable. Update-Note: This change is backwards-compatible, but we should update the QUIC code to use this new function when OPENSSL_API_VERSION is high enough. It has the benefit of not pulling in random other hash functions like MD4. Change-Id: Ied66a6f0adbd5d7d86257d9349c40a2830e3c7e8 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60606 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]>
We had to allow this when parsing certs to remain compatible with some misissued certificates, but there's no reason to allow it when making new values. Update-Note: ASN1_UTCTIME_set_string and ASN1_TIME_set_string will no longer accept times with timezone offsets, which is forbidden by RFC 5280. These functions are used when minting new certificates, rather than parsing them. The parsing behavior is unchanged by this CL. Change-Id: I0860deb44a49e99ce477f8cde847d20edfd29ed9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60608 Auto-Submit: David Benjamin <[email protected]> Commit-Queue: Bob Beck <[email protected]> Reviewed-by: Bob Beck <[email protected]>
rust-openssl uses this function when targetting OpenSSL 1.1.x. Change-Id: Ifeb1b65be9976358f9ee636ed23c1a931e03b275 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60609 Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Bob Beck <[email protected]>
__builtin_ia32_addcarryx_u64 is, strictly speaking, an ADX intrinsic.
GCC and newer Clang seem to actually implement it without ADX, but
Clang 7 and older will actually try to generate ADX code with it. But
since the caller is not marked target("adx"), this fails to build.
Manually add ADX and BMI2 target attributes to all these functions. The
compiler should be free to use those instructions as these functions all
call into an ADX+BMI2 assembly function anyway. (Though it doesn't do
much with this.)
Note we cannot just annotate fiat_addcarryx_u64. Clang and GCC won't
inline across incompatible targets, so if we tag fiat_addcarryx_u64, we
need to tag the callers up the chain until we're willing to stop
inlining.
Change-Id: I855bb88fea666d92997984836e664292d90df5be
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60612
Reviewed-by: Adam Langley <[email protected]>
Commit-Queue: Adam Langley <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Three years of updating calling code are finally complete! Update-Note: Accessing the RSA struct directly is no longer supported. Use accessors instead. Bug: 316, 325 Change-Id: I27b4c9899cb96f5807075b8fe351eaf72a9a9d44 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60610 Reviewed-by: Adam Langley <[email protected]> Reviewed-by: Bob Beck <[email protected]> Auto-Submit: David Benjamin <[email protected]> Commit-Queue: Adam Langley <[email protected]>
We no longer need to define CRYPTO_MUTEX in public headers. This simplifies a pile of things. First, we can now use pthread_rwlock_t without any fuss, rather than trying to guess the size on glibc. As a result, CRYPTO_MUTEX and CRYPTO_STATIC_MUTEX can be merged into one type. We can almost do this to CRYPTO_refcount_t too. BIO is the one straggler remaining. Fixed: 325 Change-Id: Ie93c9f553c0f02ce594b959c041b00fc15ba51d2 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60611 Commit-Queue: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]>
Bug: 285222580 Change-Id: I14715c50c3b5b0425443c191f4bf2e3ef7d665ae Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60266 Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Bob Beck <[email protected]>
While EVP_PKEY_RSA, EVP_PKEY_DSA, and EVP_PKEY_EC have publicly-exposed internaly representations, other EVP_PKEY types to not. EVP_PKEY_assign should not allow callers to manipulate those representations. As part of this, teach EVP_PKEY_assign_RSA, etc. to find their method tables directly, rather than indirecting through an integer. This makes those EVP APIs static-linker-friendly. Bug: 618, 497 Change-Id: Ic45a7514e9a3adc505759f2327129f13faf03a65 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60645 Auto-Submit: David Benjamin <[email protected]> Reviewed-by: Bob Beck <[email protected]> Commit-Queue: Bob Beck <[email protected]>
Chromium actually has a checker for unexpected exported symbols, which caught this. Change-Id: If1af4c26a3326eea185725f9e84c17f5d9f0f58e Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60665 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: Adam Langley <[email protected]> Auto-Submit: David Benjamin <[email protected]>
|
Seems there's a bug about ClientHelloPadding Any ideas? |
|
Great to see you working on this, @Raytonne -- Thanks! Would you be willing to answer positively to this, then: open-quantum-safe/oqs-demos#204 (comment) (i.e., take on maintenance for PQ-boringssl/chromium)? |
Yes - I can help to maintain the boringssl & chromium fork. |
|
The However, our @xvzcf Could you review this PR? |
baentsch
reviewed
Jun 15, 2023
There was a problem hiding this comment.
I increased the size to 10045; 10045 is the size corresponding to SSL_CURVE_P256_FRODO640AES
I'm not sure I understand: Where did you do this increase and how did you compute it? And why did you choose that algorithm and not the other (BIKE) hybrid?
Last question: There is an interop test with the interop test server at test.openquantumsafe.org in the oqs-scripts directory that for some reason is not part of CI. Did you run this as part of your testing (and does it pass)?
Please check the last commit 78ceb4a. We changed it in
The
We may compute any SSL_GROUP_* by forcing the
We've tested with test.openquantumsafe.org and our quantum safe demo server. We tried bikel, kyber, hqc together with dilithium, falcon, and sphincsshake. You may also do more tests with it. |
baentsch
approved these changes
Jun 15, 2023
There was a problem hiding this comment.
Please check the last commit 78ceb4a. We changed it in /ssl/test/runner/runner.go and added a comment there
I see -- it's for this test only, so has no reading on the actual operation as per:
We've tested with test.openquantumsafe.org and our quantum safe demo server. We tried bikel, kyber, hqc together with dilithium, falcon, and sphincsshake
Great to hear. As you're more keenly interested in ensuring this runs properly, take my enquiry only as a word of advice.
I'm then fine with this PR. Thanks again for taking this on!
sync with oqs-provider v0.5.0 and liboqs v0.8.0
Does this mean, this code now interoperates correctly with all ports at test.openquantumsafe.org? |
We tested these SIG|KEX: https://pastebin.pl/view/raw/b3bf2701 , basically all SIG|KEX combinations supported by the oqs-boringssl These ports failed (as expected because the correct curve should be p384_kyber768; don't know why they are listed on the test.openquantumsafe.org): |
|
Thanks for this re-confirmation.
Me neither :-) Seriously: IANA has standardized these hybrids so we just added them for completeness to oqsprovider/openssl/nginx and I think they are used by Google (chromium beta), Cloudflare and PQShield, respectively. Edit/Add: p384_kyber768 should also be there (and work). |
|
Thanks again for the contribution! Added #96 (comment) to keep track of the limitation(s) of |
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.
Hope it will work with the latest Chromium.
We'll try to build an OQS-Chromium based on this update on Windows next week; after that we'll update the Chromium tag in README.md