Commit Graph

24 Commits

Author SHA1 Message Date
David Benjamin
e30a09e604 Implement new PKCS#8 parsers.
As with SPKI parsers, the intent is make EVP_PKEY capture the key's
constraints in full fidelity, so we'd have to add new types or store the
information in the underlying key object if people introduce variant key
types with weird constraints on them.

Note that because PKCS#8 has a space for arbitrary attributes, this
parser must admit a hole. I'm assuming for now that we don't need an API
that enforces no attributes and just ignore trailing data in the
structure for simplicity.

BUG=499653

Change-Id: I6fc641355e87136c7220f5d7693566d1144a68e8
Reviewed-on: https://boringssl-review.googlesource.com/6866
Reviewed-by: Adam Langley <agl@google.com>
2016-02-17 17:24:10 +00:00
David Benjamin
239a0abfd5 Slightly simplify and deprecate i2d_{Public,Private}Key.
There are all the type-specific serializations rather than something
tagged with a type. i2d_PrivateKey's PKCS#8 codepath was unreachable
because every EVP_PKEY type has an old_priv_encode function.

To prune EVP_PKEY_ASN1_METHOD further, replace i2d_PrivateKey into a
switch case so we don't need to keep old_priv_encode around. This cuts
down on a case of outside modules reaching into crypto/evp method
tables.

Change-Id: I30db2eed836d560056ba9d1425b960d0602c3cf2
Reviewed-on: https://boringssl-review.googlesource.com/6865
Reviewed-by: Adam Langley <agl@google.com>
2016-02-17 16:31:26 +00:00
David Benjamin
32fdc512ca Remove param_decode and param_encode EVP_PKEY hooks.
They're only used by a pair of PEM functions, which are never used.

BUG=499653

Change-Id: I89731485c66ca328c634efbdb7e182a917f2a963
Reviewed-on: https://boringssl-review.googlesource.com/6863
Reviewed-by: Adam Langley <agl@google.com>
2016-02-17 16:30:29 +00:00
David Benjamin
68772b31b0 Implement new SPKI parsers.
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>
2016-02-17 16:28:07 +00:00
Brian Smith
5ba06897be Don't cast |OPENSSL_malloc|/|OPENSSL_realloc| result.
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>
2016-02-11 22:07:56 +00:00
David Benjamin
4e3d17a7e7 Remove redundant logic to compute EC public key.
d2i_ECPrivateKey already computes it as of
9f5a314d35.

Change-Id: Ie48b2319ee7d96d09c8e4f13d99de38bfa89be76
Reviewed-on: https://boringssl-review.googlesource.com/6857
Reviewed-by: Adam Langley <agl@google.com>
2016-02-02 16:23:05 +00:00
David Benjamin
2cdf398773 Remove pkey_base_id.
This is never accessed.

Change-Id: I4cade5e907ad4c03e9de7634b53ef965f7240087
Reviewed-on: https://boringssl-review.googlesource.com/6864
Reviewed-by: Adam Langley <agl@google.com>
2016-01-28 15:55:24 +00:00
David Benjamin
0a2c9938a5 Don't allow the specifiedCurve form of ECParameters in SPKIs.
Although RFC 3279 allows both, per RFC 5912, keys must use a named curve
rather than spelling out the curve parameters. Although we do not allow
arbitrary curves, we do have to (pretty hackishly) recognize built-in
curves in ECPrivateKeys.

It seems the cause of this was that OpenSSL, unless you set asn1_flag on
the EC_GROUP, likes to encode keys by spelling out the parameters. This
is in violation of RFC 5915, though probably not in violation of one of
the other redundant ECC specifications. For more fun, it appears
asn1_flag defaults to *off* in the API and *on* in the command-line
tools.

I think the original cause was these defaults meant the pre-BoringSSL
Android/OpenSSL Chromium port wrote out Channel ID keys in this format.
By now this should no longer by an issue, but it'll warrant a bit more
investigation to be sure we can drop it.

For now, keep this logic out of SPKIs by not calling d2i_ECParameters.
d2i_ECParameters is a fairly pointless function when only named curves
are allowed. In testing other implementations, none of Firefox, Safari,
or IE11/Win will parse such certificates (i.e. the error is fatal and
unbypassable). Likewise, because Mac and Windows' underlying libraries
reject this, Chrome on Mac and Windows already rejects such things. Thus
this change should be compatible.

The following is the certificate and key I constructed to test with:

-----BEGIN CERTIFICATE-----
MIICwjCCAmqgAwIBAgIJANlMBNpJfb/rMAkGByqGSM49BAEwRTELMAkGA1UEBhMC
QVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdp
dHMgUHR5IEx0ZDAeFw0xNDA0MjMyMzIxNTdaFw0xNDA1MjMyMzIxNTdaMEUxCzAJ
BgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5l
dCBXaWRnaXRzIFB0eSBMdGQwggFLMIIBAwYHKoZIzj0CATCB9wIBATAsBgcqhkjO
PQEBAiEA/////wAAAAEAAAAAAAAAAAAAAAD///////////////8wWwQg/////wAA
AAEAAAAAAAAAAAAAAAD///////////////wEIFrGNdiqOpPns+u9VXaYhrxlHQaw
zFOw9jvOPD4n0mBLAxUAxJ02CIbnBJNqZnjhE50mt4GffpAEQQRrF9Hy4SxCR/i8
5uVjpEDydwN9gS3rM6D0oTlF2JjClk/jQuL+Gn+bjufrSnwPnhYrzjNXazFezsu2
QGg3v1H1AiEA/////wAAAAD//////////7zm+q2nF56E87nKwvxjJVECAQEDQgAE
5itp4r9ln5e+Lx4NlIpM1Zdrt6keDUb73ampHp3culoB59aXqAoY+cPEox5W4nyD
SNsWGhz1HX7xlC1Lz3IiwaNQME4wHQYDVR0OBBYEFKuE0qyrlfCCThZ4B1VXX+Qm
jYLRMB8GA1UdIwQYMBaAFKuE0qyrlfCCThZ4B1VXX+QmjYLRMAwGA1UdEwQFMAMB
Af8wCQYHKoZIzj0EAQNHADBEAiBATB6aVJxDD6YAxEM4vf6Sbg2Ty334ldXpkNwc
TF+SngIgZ/f59kgDLf6YA04iLw1fUv5Wf1nLYJWwgrRFON5+zvw=
-----END CERTIFICATE-----
-----BEGIN EC PARAMETERS-----
MIH3AgEBMCwGByqGSM49AQECIQD/////AAAAAQAAAAAAAAAAAAAAAP//////////
/////zBbBCD/////AAAAAQAAAAAAAAAAAAAAAP///////////////AQgWsY12Ko6
k+ez671VdpiGvGUdBrDMU7D2O848PifSYEsDFQDEnTYIhucEk2pmeOETnSa3gZ9+
kARBBGsX0fLhLEJH+Lzm5WOkQPJ3A32BLeszoPShOUXYmMKWT+NC4v4af5uO5+tK
fA+eFivOM1drMV7Oy7ZAaDe/UfUCIQD/////AAAAAP//////////vOb6racXnoTz
ucrC/GMlUQIBAQ==
-----END EC PARAMETERS-----
-----BEGIN EC PRIVATE KEY-----
MHcCAQEEIAcPCHJ61KBKnN1ZyU2JaHcItW/JXTB3DujRyc4Ki7RqoAoGCCqGSM49
AwEHoUQDQgAE5itp4r9ln5e+Lx4NlIpM1Zdrt6keDUb73ampHp3culoB59aXqAoY
+cPEox5W4nyDSNsWGhz1HX7xlC1Lz3IiwQ==
-----END EC PRIVATE KEY-----

BUG=522228

Change-Id: I3723411a633dc07c4640027de07500293f8f7913
Reviewed-on: https://boringssl-review.googlesource.com/6853
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:51:14 +00:00
Brian Smith
a3d9de05fb Add |EC_GROUP_get0_order| to replace |EC_GROUP_get_order|.
|EC_GROUP_get0_order| doesn't require any heap allocations and never
fails, so it is much more convenient and more efficient for callers to
call.

Change-Id: Ic60f768875e7bc8e74362dacdb5cbbc6957b05a6
Reviewed-on: https://boringssl-review.googlesource.com/6532
Reviewed-by: Adam Langley <agl@google.com>
2015-12-15 18:18:13 +00:00
Adam Langley
d9e817309a Fix several warnings that arise in Android.
Android is now using Ninja so it doesn't spew so much to the terminal
and thus any warnings in BoringSSL (which builds really early in the
process) and much more obvious.

Thus this change fixes a few warnings that appear in the Android build.

Change-Id: Id255ace90fece772a1c3a718c877559ce920b960
Reviewed-on: https://boringssl-review.googlesource.com/6400
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-10-30 21:11:48 +00:00
David Benjamin
3570d73bf1 Remove the func parameter to OPENSSL_PUT_ERROR.
Much of this was done automatically with
  find . -name '*.c' | xargs sed -E -i '' -e 's/(OPENSSL_PUT_ERROR\([a-zA-Z_0-9]+, )[a-zA-Z_0-9]+, ([a-zA-Z_0-9]+\);)/\1\2/'
  find . -name '*.c' | xargs sed -E -i '' -e 's/(OPENSSL_PUT_ERROR\([a-zA-Z_0-9]+, )[a-zA-Z_0-9]+,  ([a-zA-Z_0-9]+\);)/\1\2/'

BUG=468039

Change-Id: I4c75fd95dff85ab1d4a546b05e6aed1aeeb499d8
Reviewed-on: https://boringssl-review.googlesource.com/5276
Reviewed-by: Adam Langley <agl@google.com>
2015-07-16 02:02:37 +00:00
David Benjamin
2e40091dd4 PKCS8_pkey_set0 doesn't take ownership on error.
It carefully NULLs all references to the buffer, so the failure cases
still need to call OPENSSL_free.

Change-Id: Ia14341ecea81296f94e467325ab6eff1362e987e
Reviewed-on: https://boringssl-review.googlesource.com/5271
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 02:30:37 +00:00
David Benjamin
42ae3adcf6 Remove info field on EVP_PKEY_ASN1_METHOD.
Nothing ever reads it.

Change-Id: Id375c461aa2feb6877a14e19eb2daefec7a03f89
Reviewed-on: https://boringssl-review.googlesource.com/5345
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:11:46 +00:00
Matt Braithwaite
3bf1cca262 Don't report |ERR_R_MALLOC_FAILURE| on failure of |EC_KEY_new_by_curve_name|.
Change |EC_KEY_new_by_curve_name| to report |ERR_R_MALLOC_FAILURE|
itself, so that reporting of |EC_R_UNKNOWN_GROUP| is not confused by
the caller's addition of a spurious |ERR_R_MALLOC_FAILURE|.

Change-Id: Id3f5364f01eb8e3597bcddd6484bc03d5578befb
Reviewed-on: https://boringssl-review.googlesource.com/4690
Reviewed-by: Adam Langley <agl@google.com>
2015-05-09 00:05:30 +00:00
David Benjamin
cca4ba7611 Remove unnecessary NULL checks, part 3.
Finish up the e's.

Change-Id: Iabb8da000fbca6efee541edb469b90896f60d54b
Reviewed-on: https://boringssl-review.googlesource.com/4516
Reviewed-by: Adam Langley <agl@google.com>
2015-05-04 23:12:04 +00:00
David Benjamin
4f7783eaea Use EC_KEY_new_by_curve_name.
May as well use this convenience function when we can. A little tidier. Even
fixes a leak on malloc failure in eckey_type2param.

Change-Id: Ie48dd98f2fe03fa9911bd78db4423ab9faefc63d
Reviewed-on: https://boringssl-review.googlesource.com/3772
Reviewed-by: Adam Langley <agl@google.com>
2015-03-13 19:49:39 +00:00
Adam Langley
7c21925a10 EC_GROUP_cmp should return zero if the groups match.
(I got this wrong when reading the OpenSSL code.)

Change-Id: Ib289ef41d0ab5a3157ad8b9454d2de96d1f86c22
Reviewed-on: https://boringssl-review.googlesource.com/3620
Reviewed-by: Adam Langley <agl@google.com>
2015-02-25 21:13:02 +00:00
Adam Langley
93531bd70f Add the CTX parameter back to EC_GROUP_cmp.
It was a mistake to remove this in the first place.

Change-Id: Icd97b4db01e49151daa41dd892f9da573ddc2842
Reviewed-on: https://boringssl-review.googlesource.com/3541
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-02-20 23:33:55 +00:00
David Benjamin
6eb000dbee Add in missing curly braces part 3.
Everything else.

Change-Id: Iac02b144465b4e7b6d69ea22ff2aaf52695ae732
2015-02-11 15:14:46 -08:00
David Benjamin
c20febe177 Add EVP_PKEY_supports_digest.
This is intended for TLS client auth with Windows CAPI- and CNG-backed keys
which implement sign over sign_raw and do not support all hash functions. Only
plumbed through RSA for now.

Change-Id: Ica42e7fb026840f817a169da9372dda226f7d6fd
Reviewed-on: https://boringssl-review.googlesource.com/2250
Reviewed-by: Adam Langley <agl@google.com>
2014-11-18 22:18:36 +00:00
Adam Langley
0e7f89f96c Remove pkey_ctrl.
It only included ASN1_PKEY_CTRL_DEFAULT_MD_NID and that's unused in
BoringSSL.

Change-Id: Idfcbd0f26f6448ce307c53ddef334f2e63c85a64
2014-11-10 13:45:32 -08:00
David Benjamin
ecc0ce7e67 Introduce EVP_PKEY_is_opaque to replace RSA_METHOD_FLAG_NO_CHECK.
Custom RSA and ECDSA keys may not expose the key material. Plumb and "opaque"
bit out of the *_METHOD up to EVP_PKEY. Query that in ssl_rsa.c to skip the
sanity checks for certificate and key matching.

Change-Id: I362a2d5116bfd1803560dfca1d69a91153e895fc
Reviewed-on: https://boringssl-review.googlesource.com/1255
Reviewed-by: Adam Langley <agl@google.com>
2014-07-18 23:35:04 +00:00
Adam Langley
f71a27920a Fix EC crash.
This change saves several EC routines from crashing when an EC_KEY is
missing a public key. The public key is optional in the EC private key
format and, without this patch, running the following through `openssl
ec` causes a crash:

-----BEGIN EC PRIVATE KEY-----
MBkCAQEECAECAwQFBgcIoAoGCCqGSM49AwEH
-----END EC PRIVATE KEY-----
2014-06-20 13:17:34 -07:00
Adam Langley
95c29f3cd1 Inital import.
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).

(This change contains substantial changes from the original and
effectively starts a new history.)
2014-06-20 13:17:32 -07:00