Many consumers need SPKI support (X.509, TLS, QUIC, WebCrypto), each
with different ways to set signature parameters. SPKIs themselves can
get complex with id-RSASSA-PSS keys which come with various constraints
in the key parameters. This suggests we want a common in-library
representation of an SPKI.
This adds two new functions EVP_parse_public_key and
EVP_marshal_public_key which converts EVP_PKEY to and from SPKI and
implements X509_PUBKEY functions with them. EVP_PKEY seems to have been
intended to be able to express the supported SPKI types with
full-fidelity, so these APIs will continue this.
This means future support for id-RSASSA-PSS would *not* repurpose
EVP_PKEY_RSA. I'm worried about code assuming EVP_PKEY_RSA implies
acting on the RSA* is legal. Instead, it'd add an EVP_PKEY_RSA_PSS and
the data pointer would be some (exposed, so the caller may still check
key size, etc.) RSA_PSS_KEY struct. Internally, the EVP_PKEY_CTX
implementation would enforce the key constraints. If RSA_PSS_KEY would
later need its own API, that code would move there, but that seems
unlikely.
Ideally we'd have a 1:1 correspondence with key OID, although we may
have to fudge things if mistakes happen in standardization. (Whether or
not X.509 reuses id-ecPublicKey for Ed25519, we'll give it a separate
EVP_PKEY type.)
DSA parsing hooks are still implemented, missing parameters and all for
now. This isn't any worse than before.
Decoupling from the giant crypto/obj OID table will be a later task.
BUG=522228
Change-Id: I0e3964edf20cb795a18b0991d17e5ca8bce3e28c
Reviewed-on: https://boringssl-review.googlesource.com/6861
Reviewed-by: Adam Langley <agl@google.com>
This imports upstream's ea6b07b54c1f8fc2275a121cdda071e2df7bd6c1 along
with a bugfix in 987157f6f63fa70dbeffca3c8bc62f26e9767ff2.
In an SPKI, a DSA key is only an INTEGER, with the group information in
the AlgorithmIdentifier. But a standalone DSAPublicKey is more complex
(and apparently made up by OpenSSL). OpenSSL implemented this with a
write_params boolean and making DSAPublicKey a CHOICE.
Instead, have p_dsa_asn1.c encode an INTEGER directly. d2i_DSAPublicKey
only parses the standalone form. (That code will be replaced later, but
first do this in preparation for rewriting the DSA ASN.1 code.)
Change-Id: I6fbe298d2723b9816806e9c196c724359b9ffd63
Reviewed-on: https://boringssl-review.googlesource.com/7021
Reviewed-by: Adam Langley <agl@google.com>
Functions which lose object reuse and need auditing:
- d2i_ECParameters
- d2i_ECPrivateKey
This adds a handful of bytestring-based APIs to handle EC key
serialization. Deprecate all the old serialization APIs. Notes:
- An EC_KEY has additional state that controls its encoding, enc_flags
and conv_form. conv_form is left alone, but enc_flags in the new API
is an explicit parameter.
- d2i_ECPrivateKey interpreted its T** argument unlike nearly every
other d2i function. This is an explicit EC_GROUP parameter in the new
function.
- The new specified curve code is much stricter and should parse enough
to uniquely identify the curve.
- I've not bothered with a new version of i2d_ECParameters. It just
writes an OID. This may change later when decoupling from the giant
OID table.
- Likewise, I've not bothered with new APIs for the public key since the
EC_POINT APIs should suffice.
- Previously, d2i_ECPrivateKey would not call EC_KEY_check_key and it
was possible for the imported public and private key to mismatch. It
now calls it.
BUG=499653
Change-Id: I30b4dd2841ae76c56ab0e1808360b2628dee0615
Reviewed-on: https://boringssl-review.googlesource.com/6859
Reviewed-by: Adam Langley <agl@google.com>
In c0d9484902, we had to add support for
recognizing specified versions of named curves. I believe the motivation
was an ECPrivateKey encoded by OpenSSL without the EC_KEY's asn1_flag
set to OPENSSL_EC_NAMED_CURVE. Annoyingly, it appears OpenSSL's API
defaulted to the specified form while the tool defaulted to the named
form.
Add tests for this at the ECPrivateKey and the PKCS#8 level. The latter
was taken from Chromium's ec_private_key_unittest.cc which was the
original impetus for this.
Change-Id: I53a80c842c3fc9598f2e0ee7bf2d86b2add9e6c4
Reviewed-on: https://boringssl-review.googlesource.com/7072
Reviewed-by: Adam Langley <agl@google.com>
The function |ge_frombytes_negate_vartime|, as the name suggests,
negates its output. This change converts it to |ge_frombytes_vartime|
and, instead, does the negation explicitly when verifying signatures.
The latter function is more generally useful.
Change-Id: I465f8bdf5edb101a80ab1835909ae0ff41d3e295
Reviewed-on: https://boringssl-review.googlesource.com/7142
Reviewed-by: Arnar Birgisson <arnarb@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
An i2d compatibility function is rather long, so add CBB_finish_i2d for
part of it. It takes a CBB as input so only a 'marshal' function is
needed, rather than a 'to_bytes' one.
Also replace the *inp d2i update pattern with a slightly shorter one.
Change-Id: Ibb41059c9532f6a8ce33460890cc1afe26adc97c
Reviewed-on: https://boringssl-review.googlesource.com/6868
Reviewed-by: Adam Langley <agl@google.com>
CBS_asn1_ber_to_der currently uses heuristics because implicitly-tagged
constructed strings in BER are ambiguous with implicitly-tagged sequences. It's
not possible to convert BER to DER without knowing the schema.
Fortunately, implicitly tagged strings don't appear often so instead split the
job up: CBS_asn1_ber_to_der fixes indefinite-length elements and constructed
strings it can see. Implicitly-tagged strings it leaves uncoverted, but they
will only nest one level down (because BER kindly allows one to nest
constructed strings arbitrarily!).
CBS_get_asn1_implicit_string then performs the final concatenation at parse
time. This isn't much more complex and lets us parse BER more accurately and
also reject a number of mis-encoded values (e.g. constructed INTEGERs are not a
thing) we'd previously let through. The downside is the post-conversion parsing
code must be aware of this limitation of CBS_asn1_ber_to_der. Fortunately,
there's only one implicitly-tagged string in our PKCS#12 code.
(In the category of things that really really don't matter, but I had spare
cycles and the old BER converter is weird.)
Change-Id: Iebdd13b08559fa158b308ef83a5bb07bfdf80ae8
Reviewed-on: https://boringssl-review.googlesource.com/7052
Reviewed-by: Adam Langley <agl@google.com>
This is significantly less of a nuisance than having to explicitly type out
kRule5, kExpected5.
Change-Id: I61820c26a159c71e09000fbe0bf91e30da42205e
Reviewed-on: https://boringssl-review.googlesource.com/7000
Reviewed-by: Adam Langley <agl@google.com>
C has implicit conversion of |void *| to other pointer types so these
casts are unnecessary. Clean them up to make the code easier to read
and to make it easier to find dangerous casts.
Change-Id: I26988a672e8ed4d69c75cfbb284413999b475464
Reviewed-on: https://boringssl-review.googlesource.com/7102
Reviewed-by: David Benjamin <davidben@google.com>
I guess the comment "just a reference" was intended to mean that the
|mod| member is a weak reference to a |BIGNUM| owned by something else.
However, it is actually owned by the |bn_blinding_st|, as one can see
by reading |BN_BLINDING_new| and |BN_BLINDING_free|.
Change-Id: If2a681fc9d9db536170e0efb11fdab93e4f0baba
Reviewed-on: https://boringssl-review.googlesource.com/7112
Reviewed-by: David Benjamin <davidben@google.com>
We'd manually marked some of them hidden, but missed some. Do it in the perlasm
driver instead since we will never expose an asm symbol directly. This reduces
some of our divergence from upstream on these files (and indeed we'd
accidentally lose some .hiddens at one point).
BUG=586141
Change-Id: Ie1bfc6f38ba73d33f5c56a8a40c2bf1668562e7e
Reviewed-on: https://boringssl-review.googlesource.com/7140
Reviewed-by: Adam Langley <agl@google.com>
Updating the Perl docs to describe behavior of Strawberry Perl and possible
interaction with CMake on Windows.
Also adding a few other links and instructions for using CMake/Ninja to build
release mode with position independent code, since this seems generally useful.
Change-Id: I616c0d267da749fe90673bc9e8bde9ec181fec25
Reviewed-on: https://boringssl-review.googlesource.com/7113
Reviewed-by: David Benjamin <davidben@google.com>
The |_ex| versions of these functions are unnecessary because when they
are used, they are always passed |NULL| for |r|, which is what the
non-|_ex| versions do. Just use the non-|_ex| versions instead and
remove the |_ex| versions.
Also, drop the unused flags mechanism.
Change-Id: Ida4cb5a2d4c89d9cd318e06f71867aea98408d0d
Reviewed-on: https://boringssl-review.googlesource.com/7110
Reviewed-by: David Benjamin <davidben@google.com>
It is always the case that either |BN_ULLONG| is defined or
|BN_UMULT_LOHI| is defined because |BN_ULLONG| is defined everywhere
except 64-bit MSVC, and BN_UMULT_LOHI is defined for 64-bit MSVC.
Change-Id: I85e5d621458562501af1af65d587c0b8d937ba3b
Reviewed-on: https://boringssl-review.googlesource.com/7044
Reviewed-by: David Benjamin <davidben@google.com>
|a_is_minus_3| is calculated in |ec_GFp_simple_group_set_curve|, so
the custom |group_init| functions are unnecessary. Just as in
commit 9f1f04f313, it is never the case
that custom parameters are passed to the |group_set_curve| method for
these curves.
Change-Id: I18a38b104bc332e44cc2053c465cf234f4c5163b
Reviewed-on: https://boringssl-review.googlesource.com/7090
Reviewed-by: David Benjamin <davidben@google.com>
mul.c is the only file that uses these values.
Change-Id: I50a685cbff0f26357229e742f42e014434e9cebe
Reviewed-on: https://boringssl-review.googlesource.com/7061
Reviewed-by: David Benjamin <davidben@google.com>
Notably, putting Strawberry Perl in %PATH% will usually end up putting a copy
of gcc in %PATH%, which trips up people trying to build on Windows.
This is arguably misusing the variable (normally set by the generator), but it
should work.
Change-Id: I13a011eb33688ae928a56cce266edd2759a3cb32
Reviewed-on: https://boringssl-review.googlesource.com/7070
Reviewed-by: Adam Langley <agl@google.com>
It is always the case that |BN_ULLONG| is defined or we're building for
64-bit MSVC. Lots of code is trying to handle impossible cases where
neither of those is true.
Change-Id: Ie337adda1dfb453843c6e0999807dfa1afb1ed89
Reviewed-on: https://boringssl-review.googlesource.com/7043
Reviewed-by: David Benjamin <davidben@google.com>
Windows build failures seem to have been a CMake statefulness problem. Recipes
were changed to do clean builds each run.
Change-Id: Id5aefa53aead7e82e095d7dccbf88ad89a678c62
This allows much code to be subsequently simplified and removed.
Change-Id: I0ac256957c6eae9f35a70508bd454cb44f3f8653
Reviewed-on: https://boringssl-review.googlesource.com/7042
Reviewed-by: David Benjamin <davidben@google.com>
On failure, CBB_finish doesn't call CBB_cleanup. Also chain more of the ||s
together now that CBB_cleanup after failed CBB_init is legal.
(I don't think this is actually reachable because the CBB is guaranteed to be
flushed by this point.)
Change-Id: Ib16a0a185f15e13675ac2550c5e8e0926ceb7957
Reviewed-on: https://boringssl-review.googlesource.com/7051
Reviewed-by: Adam Langley <agl@google.com>
There was a test for 512 bit orders but not one for 521-bit orders.
Test 521-bit orders instead.
Change-Id: I61a76d02637ca55d8ae21834085311dd84fd870f
Reviewed-on: https://boringssl-review.googlesource.com/7011
Reviewed-by: David Benjamin <davidben@google.com>
node.js uses a memory BIO in the wrong mode which, for now, we work
around. It also passes in NULL (rather than empty) strings and a
non-NULL out-arg for |d2i_PKCS12_bio|.
Change-Id: Ib565b4a202775bb32fdcb76db8a4e8c54268c052
Reviewed-on: https://boringssl-review.googlesource.com/7012
Reviewed-by: Adam Langley <agl@google.com>
Otherwise it still thinks this is an RFC 5114 prime and kicks in the (now
incorrect) validity check.
Change-Id: Ie78514211927f1f2d2549958621cb7896f68b5ce
Reviewed-on: https://boringssl-review.googlesource.com/7050
Reviewed-by: Adam Langley <agl@google.com>
This slightly simplifies the SSL_ECDH code and will be useful later on
in reimplementing the key parsing logic.
Change-Id: Ie41ea5fd3a9a734b3879b715fbf57bd991e23799
Reviewed-on: https://boringssl-review.googlesource.com/6858
Reviewed-by: Adam Langley <agl@google.com>
Change acb24518 renamed some functions, but there were some dangling
references in bn_test.c. Thanks to Brian Smith for noticing.
This change has no semantic effect.
Change-Id: Id149505090566583834be3abce2cee28b8c248e2
Reviewed-on: https://boringssl-review.googlesource.com/7040
Reviewed-by: David Benjamin <davidben@google.com>
This is CVE-2016-0701 for OpenSSL, reported by Antonio Sanso. It is a no-op for
us as we'd long removed SSL_OP_DH_SINGLE_USE and static DH cipher suites. (We
also do not parse or generate X9.42 DH parameters.)
However, we do still have the APIs which return RFC 5114 groups, so we should
perform the necessary checks in case later consumers reuse keys.
Unlike groups we generate, RFC 5114 groups do not use "safe primes" and have
many small subgroups. In those cases, the subprime q is available. Before using
a public key, ensure its order is q by checking y^q = 1 (mod p). (q is assumed
to be prime and the existing range checks ensure y is not 1.)
(Imported from upstream's 878e2c5b13010329c203f309ed0c8f2113f85648 and
75374adf8a6ff69d6718952121875a491ed2cd29, but with some bugs fixed. See
RT4278.)
Change-Id: Ib18c3e84819002fa36a127ac12ca00ee33ea018a
Reviewed-on: https://boringssl-review.googlesource.com/7001
Reviewed-by: Adam Langley <agl@google.com>
Add guards for the architecture and OPENSSL_NO_ASM to
the assembly-language files in crypto/curve25519/asm.
The Dart compilation of BoringSSL includes all files,
because the architecture is not known when gyp is run.
Change-Id: I66f5ae525266b63b0fe3a929012b771d545779b5
Reviewed-on: https://boringssl-review.googlesource.com/7030
Reviewed-by: Adam Langley <agl@google.com>
Take the mappings for MD5 and SHA-224 values out of the code altogether. This
aligns with the current TLS 1.3 draft.
For MD5, this is a no-op. It is not currently possible to configure accepted
signature algorithms, MD5 wasn't in the hardcoded list, and we already had a
test ensuring we enforced our preferences correctly. MD5 also wasn't in the
default list of hashes our keys could sign and no one overrides it with a
different hash.
For SHA-224, this is not quite a no-op. The hardcoded accepted signature
algorithms list included SHA-224, so this will break servers relying on that.
However, Chrome's metrics have zero data points of servers picking SHA-224 and
no other major browser includes it. Thus that should be safe.
SHA-224 was also in the default list of hashes we are willing to sign. For
client certificates, Chromium's abstractions already did not allow signing
SHA-224, so this is a no-op there. For servers, this will break any clients
which only accept SHA-224. But no major browsers do this and I am not aware of
any client implementation which does such ridiculous thing.
(SHA-1's still in there. Getting rid of that one is going to take more effort.)
Change-Id: I6a765fdeea9e19348e409d58a0eac770b318e599
Reviewed-on: https://boringssl-review.googlesource.com/7020
Reviewed-by: Adam Langley <agl@google.com>
The |inline| must appear before the type.
Change-Id: Iecebbcc50024a846d7804228a858acfc33d68efd
Reviewed-on: https://boringssl-review.googlesource.com/7010
Reviewed-by: David Benjamin <davidben@google.com>
Finally, we can stick ScopedFOO in containers.
Change-Id: I3ed166575822af9f182e8be8f4db723e1f08ea31
Reviewed-on: https://boringssl-review.googlesource.com/6553
Reviewed-by: Adam Langley <alangley@gmail.com>
