The extensions list in a certificate, CRL, and CRL entry is defined as:
... extensions [3] EXPLICIT Extensions OPTIONAL ...
... crlEntryExtensions Extensions OPTIONAL ...
... crlExtensions [0] EXPLICIT Extensions OPTIONAL ...
Extensions ::= SEQUENCE SIZE (1..MAX) OF Extension
This means that a present but empty extensions list is actually invalid.
Rather, if you have no extensions to encode, you are meant to omit the
list altogether. Fix the delete_ext functions to handle this correctly.
Bug: 442221114
Change-Id: I92af89d3e7120e06489359b3c6e2af499ecf5b85
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81751
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
These functions only need to pull in the algorithms they support. The
new API is slightly less friendly in this context because it expects the
caller to have found the end first, but it's easy enough to to write a
small wrapper.
Bug: 42290364
Change-Id: Ibdc44f399182cd5722700917aec6c151221f4674
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81747
Reviewed-by: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Update-Note: ASN1_TIME, DIRECTORYSTRING, and DISPLAYTEXT are no longer
usable in custom macro-based ASN.1 types using <openssl/asn1t.h>. There
do not seem to be any external callers that depend on this.
If this sticks then, after all the built-in types are rewritten, we may
be able to remove MSTRING from tasn_* altogether!
Bug: 42290417
Change-Id: Ia12d8767bcada8eda75550e13bbab5a4a572382c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81729
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
EXTERNAL, EMBEDDED PDV, and CHARACTER STRING are actually constructed!
They're encoded as some implicitly-tagged SEQUENCE. I couldn't be
bothered to fill in valid contents thought.
While I'm here, make RELATIVE-OID non-empty. That is the most likely new
type for us to pay attention to here, just because we borrowed them for
trust anchor IDs. (I'm not totally clear on whether the empty relative
OID is a valid relative OID. Meh.)
Change-Id: I193e5fb800d317cad8711a09078ac15adf80d76a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81728
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We should be able to parse a variety of valid types, and also reject
syntax errors in any types that we recognize. Also double-check that
nothing went wrong in the translation from i2d_ASN1_TYPE to X509_ALGOR
(basically the only use of ASN1_TYPE in the library).
Change-Id: I825f44c35a7e8ea6374641cf5e0ea5daaf471ad7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81727
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Keeping it to X25519/Ed25519 for now, but arguably we could pick "raw"
options for most of our keys. The problem is just when there's a couple
different options. (ECPrivateKey or just the serialized scalar for EC
keys, compressed vs uncompressed points.)
Bug: 42290364
Change-Id: I1e1a0a3fa971f27a962946301091ebe11bac2725
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81667
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
In the STL, <iterator> has a std::size for arrays. Some of these could
also just be ranged for loops. One static_assert could not use
std::size(out->whatever) because out was not a compile-time value, but
std::extent_v<decltype(out->whatever)> works instead.
Change-Id: I28007c79f5583e09167b81a34a447e205ee6dd9b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81658
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
While, in principle, PSS is better than PKCS#1 v1.5, and
algorithm-specific keys are better than mixing them up, RSASSA-PSS was
so badly mis-standardized in RFC 3447 and RFC 4055 that this is not
worth it. Any marginal benefits one might get from PSS is completely
overshadowed by the mountain of unforced errors those two RFCs made.
Applications are better off just using ECDSA.
Nonetheless, it is a thing we are now supporting. Add off-by-default
support for EVP_PKEY_RSA_PSS, only using the SHA-256 parameter set. In
OpenSSL's implementation, the underlying RSA object stores an
RSA_PSS_PARAMS, though the RSA-level APIs don't enforce the parameters,
only the EVP-level APIs do. For now, since the SHA-256 parameters are
the only ones we support, I have not bothered adding extra state to the
RSA object. If we need to add more parameters, we can store the
rsa_pss_params_t enum on the RSA object. (Preferably after we've split
the BCM and non-BCM halves of the RSA object.)
This support is off by default and must remain so. We have a bit of a
mess API-wise: OpenSSL made EVP_PKEY_get0_RSA work with
EVP_PKEY_RSA_PSS. This is plausible in that applications may want to
inspect RSA components and that is, for now, the API to do so. However,
existing callers generally assume a non-NULL EVP_PKEY_get0_RSA return
implies EVP_PKEY_RSA. Changing this will break those callers.
Thus the opt-in not only limits a badly-designed key type, but also
prevents existing callers from being exposed to this unexpected state.
These keys are not wired up to libssl and we have no plans to do so.
Bug: 384818542
Change-Id: I4d99be86ce1d891a2e50335ef097913707ede55a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81656
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
For now this is just used by the X.509 verification logic and supports
the three sets of parameters we accept. Later EVP_PKEY_RSA_PSS will use
the same parser.
Bug: 384818542
Change-Id: I4d9a83283cbf239268f37d814f00d0b6ec3b46f2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81655
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
For now this uses a hardcoded, but smaller, list of accepted algorithms.
Ideally we'd get the algorithms from the caller, via the same APIs that
limit acceptable keys.
Bug: 42290364
Change-Id: Ife60a841f836d1a67cf8daea30f5b961a3ba32ff
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81653
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
This adds new SPKI and PKCS#8 parser entrypoints that pass in the list
of allowed algorithms. Algorithms are expressed as EVP_PKEY_ALG
functions and roughly things that can create EVP_PKEYs in various ways.
(Right now just SPKI and PKCS#8 parsers, but I'm imagining that they'll
also be the handle for the "raw" public/private key parsers and things
like the list of curves for ECPrivateKey or X9.62 points.)
The immediate motivation is to give a handle for opting into
EVP_PKEY_RSA_PSS (we don't want most applications to be exposed to
those), but I'm hoping this can also:
- Let us add legacy or experimental algorithms for one caller without
worrying about exposing all callers to them.
- Let size-constrained applications use EVP without worrying about
binary size.
- Reduce the need for people to check the key type after parsing keys. I
would like to it eliminate the need, but RSA having a continuum of
sizes makes things tricky. For now, EVP_pkey_rsa() just accepts all
supported RSA sizes (512 through 8192, but hopefully we can clamp it
down).
While we're here, correct what, in hindsight, were I think some missteps
in the EVP_parse_public_key convention:
- Not saying the name of the format in the function name. Since I expect
we'll add EVP_PKEY_from_ec_private_key, EVP_PKEY_from_rsa_private_key,
etc., functions to start filling out EVP, having the name of the
format would have been nice.
- Taking an in/out CBS parameter. As an in/out parameter, we don't check
trailing data and just pass it to the caller to check. This optimized
for parsing an SPKI inside a structure, rather than standalone. In
practice, folks forget to check trailing data.
In comparison to OpenSSL 3.x, I think an EVP_PKEY_ALG is roughly an
OSSL_KEYMGMT, but that's a very confusing name. Maybe also with bits of
the OSSL_DECODER in there. Mostly I don't understand their very
complicated (and apparently quadratic-time) decoder system.
OpenSSL 3.x also has an OSSL_LIB_CTX which has been threaded into a
bunch of objects. Having BoringSSL's OSSL_LIB_CTX carry a list of these
things would be plausible, but different call sites within a single
application may support different algorithms, so let's start with the
explicit list approach and see where we go from there.
Bug: 42290364
Change-Id: I342603efdc167c8f587357d17800d7ad545d5908
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81651
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
They're internal for now; I'm thinking we'll just have the public
versions of these at the EVP layer for now and try to move past this
mess where there's two versions of every API.
The API is mildly annoying in that both this caller and the one added
next will want to distinguish UNKNOWN_GROUP from other errors, but the
recent helper function makes checking the error queue not tooooo bad.
Bug: 42290364
Change-Id: Ibb5e3e643a9180459cc09b4559ee40696cea2688
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81650
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Between the "raw" import and what'll soon be a few parsing entrypoints,
we'll have a few different ways to make keys. Rather than importing one
key and then checking properties, this function is now meant to describe
properties of a key and then:
- For each set of properties that is enough to construct a key,
construct that key.
- For each key that was constructed, check all properties apply to that
key.
Rather than having ExpectNo* attributes, let's just say that if you omit
the raw public/private key, the expectation is that you don't have one.
I've also dropped "Expect" on the positive cases because it is now both
a property to check and a way to construct the key.
We've since added a few properties of keys that aren't curently
well-tested, so go test those. This revealed that actually the DSA test
vectors were misnamed. The test public key didn't correspond to the test
public key and they were of different sizes.
I tried to test the EC point format getting but ran into
https://crbug.com/441087671, so I'm going to just ignore this for now.
Change-Id: I5a0432061e33fcc92f790023ad40019e8eaf27c1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81647
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Checking for an error on the error queue is very tedious. You have to
check both library and reason code separately, which often means you
need to save the error code to a local variable.
Make an ERR_equals function. I've gone ahead and just made it public API
because even within BoringSSL, it seems to come up everywhere.
Change-Id: Ia963c9ae743ac5c6939846f8f64bbc98b495ce0b
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81627
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
We internally half-initialize EVP_PKEYs everywhere, but there are very
few places where we actually need to half-initialize them.
This changes most of the EVP_PKEY_ASN1_METHOD callbacks so that output
EVP_PKEYs are not half-initialized with the method first. Rather, the
callback is expected to fill in the method and contents together.
EVP_PKEY_copy_parameters remains as a goofy exception because it's an
in-out parameter. In principle, it is possible to have a goofy
parameter-less, key-only DSA object, and we need to fill in the
parameters later. This was due to how DSA was embedded into X.509. But
we don't support DSA in X.509 and we removed this parameterless state
from the parser, so we probably can remove this now. (I've left it as-is
for now.)
Bug: 42290409
Change-Id: I2a576571d75ce755fd7e963be467aa5d94f20466
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81550
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Lily Chen <chlily@google.com>
Commit-Queue: Lily Chen <chlily@google.com>
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>
Per the docs, the only real use case is setting EVP_PKEY_X25519. Outside
BoringSSL, the one caller using other key types to make test keys has
since been fixed. One call to EVP_PKEY_set_type(EVP_PKEY_NONE) remains,
but this CL does not break it.
This removes one path to making this flavor of half-empty keys for all
but X25519, where we're forced to have such keys for OpenSSL
compatibility.
Update-Note: EVP_PKEY_set_type will now only succeed for
EVP_PKEY_X25519. EVP_PKEY_set_type(EVP_PKEY_NONE) will continue to clear
the pkey and then fail. Going through code search, there are not
expected to be any affected callers.
Bug: 42290409
Change-Id: I20ca762be71f71a628f5894045e5b66b7773c215
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81548
Reviewed-by: Lily Chen <chlily@google.com>
Commit-Queue: Lily Chen <chlily@google.com>
Conscrypt relies on both this working at the EC_KEY level, but also
EVP_PKEY. Make sure this keeps working, even as we mess around with
EVP_PKEY's EC bits to separate the curves out.
While I'm here, move the common HexToBIGNUM wrapper into test_util.h.
Bug: 42290364
Change-Id: I58fa5a8487b8a499000a798040a53fc851b0a732
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81547
Reviewed-by: Lily Chen <chlily@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
ec_pkey_meth, and thus every all of EC EVP_PKEY logic, currently depends
on all supported curves by way of EC_GROUP_new_by_curve_name. In
reality, the only call pattern which depends on every curve is
"paramgem", when callers do:
EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_EC, nullptr);
EVP_PKEY_paramgen_init(ctx);
EVP_PKEY_CTX_set_ec_paramgen_curve_nid(ctx, NID_X9_62_prime256v1);
EVP_PKEY_paramgen(ctx, &key);
This is a really roundabout way to get at a basically static object. EVP
is kinda cumbersome. The other pattern is keygen when you don't already
have an object that represents the curve, since OpenSSL does not provide
such a thing.
EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_EC, nullptr);
EVP_PKEY_keygen_init(ctx);
EVP_PKEY_CTX_set_ec_paramgen_curve_nid(ctx, NID_X9_62_prime256v1);
EVP_PKEY_keygen(ctx, &key);
That one is harder to avoid because EVP does not provide a better way to
do this. Ideally our API would look more like
EVP_generate_ec_key(EC_group_p256()), or perhaps
EVP_PKEY_generate(EVP_pkey_ec_p256()) or something.
Either way, we can lift the dependency to
EVP_PKEY_CTX_set_ec_paramgen_curve_nid which is the function that
actually pulls in all curves, and then folks who don't use that pattern
aren't impacted. The other thing I considered was to make
EVP_PKEY_CTX_new_id(EVP_PKEY_EC) use a different method table from
EVP_PKEY_CTX_new(pkey), but this was really easy.
(We can do this because our EVP_PKEY_CTRL_* constants are completely
internal. We don't have to implement the same hooks that upstream does.)
Bug: 42290364
Change-Id: Ib223b966d1a48527088e1bb13435ac6dc2c11749
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81509
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Lily Chen <chlily@google.com>
Commit-Queue: Lily Chen <chlily@google.com>
It's getting a bit large. rsa_tests.txt is unfortunately still kind of
large, but we can't easily split that up because the test vector
framework is slightly stateful. (The key parsing tests remember all
parsed keys so that they can be used later by the operation tests.)
Change-Id: I62b669814c04fbe5aba7a9e4f8614e39550a3d7a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81508
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Lily Chen <chlily@google.com>
Reviewed-by: Lily Chen <chlily@google.com>
In particular, EVP_parse_public_key was written to avoid checking for a
whole number of bytes until *after* we've recognized the OID. This is
almost certainly not worth the effort, but I suppose if we tried to do
that, we should test it.
Why X.509 decided SPKIs use BIT STRINGs instead of OCTET STRINGs, who
knows.
Change-Id: Ib18887c6bed2dbbd3407de262b4912ad38ccf9dd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81507
Reviewed-by: Lily Chen <chlily@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
We were simulating non-blocking writes, but not non-blocking flush.
Model it as consuming one byte in AsyncBio.
In doing so, fix an obscure corner case in DTLS: If flushing after ACK
blocked, the next try would write a new ACK to the transport. There's no
real harm in this (we're running over UDP), but our tests intentionally
check for exactly the right number of writes and this was easy to fix.
This completely doesn't matter as a non-blocking writes on UDP-like
sockets are not really a thing, much less buffered non-blocking writes
on them. I don't even concretely know of anyone relying on BIO_flush in
TCP-like BIOs anymore in libssl. But since we try to support it, we
should test that we get it right.
Fixed: 381906252
Change-Id: I5296fcb01ca409d3026ca1150d6bdeaccc868014
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81348
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Lily Chen <chlily@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
EC_KEY_KEY looks very silly. In OpenSSL, this is
EVP_R_EXPECTING_A_EC_KEY. It's a little odd that they say "a EC key"
instead of "an EC key", but ah well. (They do say "an RSA key".) May as
well match.
Update-Note: Code search finds no references to the BoringSSL name, so
this is not expected to impact anything.
Change-Id: I7563645fd269e4a62fbc46de2647f06bde00e1e4
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81428
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Lily Chen <chlily@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Lily Chen <chlily@google.com>
Given OpenSSL puts it all in one header, I don't think we can ever make
<openssl/evp.h> stop including <openssl/digest.h>. Best we can do is
move the public/private key stuff into its own header and make
<openssl/evp.h> be the compat one.
Change-Id: I60310630657513a1cb410cd401ae7e80b1c16913
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81408
Commit-Queue: Lily Chen <chlily@google.com>
Reviewed-by: Lily Chen <chlily@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
David Benjamin