| commit | 5b7171f2da10d5dad52d0a2e4426fbc71d4ff146 | [log] [tgz] |
|---|---|---|
| author | David Benjamin <davidben@google.com> | Fri Aug 15 15:14:56 2025 -0400 |
| committer | Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> | Fri Aug 22 11:29:50 2025 -0700 |
| tree | 9a1948e0ef38ea812aa64d66069ab488e241a67d | |
| parent | 9b602f2d911db5c528bb9832256477bf63584260 [diff] |
Make some more half-empty EVP_PKEY states impossible
EVP_PKEY_{assign,set1}_FOO would check for NULL, return an error, but
still leave the EVP_PKEY assigned to that type on failure. Check for
NULL first, so that we don't leave it in that state.
I originally did this with a slightly more ambitious goal of also
banning EVP_PKEY_set1_EC_KEY if the EC_KEY has no parameters. That was
so that, in the happy future where we have EVP_PKEY_ALGs for P-256 and
P-384, EVP_PKEY_ASN1_METHOD would simply be renamed EVP_PKEY_ALG and
every key would have an associated EVP_PKEY_ALG.
For that to work, EVP_PKEY_set1_EC_KEY must never be ambiguous about
which EVP_PKEY_ALG to associate with the EVP_PKEY.
However, the existence of custom EC_GROUPs throws a spanner in that.
We need to support EVP_PKEY_set1_EC_KEY with an custom EC_GROUP (at
least until we manage to get Conscrypt to stop using this function). So,
at least for now, I'm thinking we say that EVP_PKEY_ALGs point to
EVP_PKEY_ASN1_METHODs but you can't go from EVP_PKEY back to
EVP_PKEY_ALG, and we'll see how irksome of an API that becomes.
(We can always go back to this idea later. The custom EC_GROUPs thing
isn't fatal if EC_KEYs with funny EC_GROUPs map to some goofy private
EVP_PKEY_ALG that can't parse anything.)
Still, half-empty states are generally bad, so I'm going to keep this
change on the branch and see if we can get it to stick.
Update-Note: Some half-empty, invalid EVP_PKEY states are now
impossible. Running through tests, no callers were tripping this. There
seems to be no legitimate reason to do this.
Bug: 42290409
Change-Id: I0211a38ab62268a05e3ff1d138a092e4feec10b1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81549
Reviewed-by: Lily Chen <chlily@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
BoringSSL is a fork of OpenSSL that is designed to meet Google's needs.
Although BoringSSL is an open source project, it is not intended for general use, as OpenSSL is. We don't recommend that third parties depend upon it. Doing so is likely to be frustrating because there are no guarantees of API or ABI stability.
Programs ship their own copies of BoringSSL when they use it and we update everything as needed when deciding to make API changes. This allows us to mostly avoid compromises in the name of compatibility. It works for us, but it may not work for you.
BoringSSL arose because Google used OpenSSL for many years in various ways and, over time, built up a large number of patches that were maintained while tracking upstream OpenSSL. As Google's product portfolio became more complex, more copies of OpenSSL sprung up and the effort involved in maintaining all these patches in multiple places was growing steadily.
Currently BoringSSL is the SSL library in Chrome/Chromium, Android (but it's not part of the NDK) and a number of other apps/programs.
Project links:
To file a security issue, use the Chromium process and mention in the report this is for BoringSSL. You can ignore the parts of the process that are specific to Chromium/Chrome.
There are other files in this directory which might be helpful: