diff options
author | David Benjamin <davidben@chromium.org> | 2015-12-31 05:40:40 +0300 |
---|---|---|
committer | Adam Langley <agl@google.com> | 2016-02-17 19:28:07 +0300 |
commit | 68772b31b07827793827e45ea81e8035269774c1 (patch) | |
tree | f9f661c29a375dfd4586e847b2a5ece92733de5f /crypto/x509 | |
parent | 2dc469e06655be3a761ee25b1d89b4f6ebfc29a4 (diff) |
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>
Diffstat (limited to 'crypto/x509')
-rw-r--r-- | crypto/x509/x_pubkey.c | 84 |
1 files changed, 38 insertions, 46 deletions
diff --git a/crypto/x509/x_pubkey.c b/crypto/x509/x_pubkey.c index e8732a11..47f256c7 100644 --- a/crypto/x509/x_pubkey.c +++ b/crypto/x509/x_pubkey.c @@ -54,8 +54,11 @@ * copied and put under another distribution licence * [including the GNU Public Licence.] */ +#include <limits.h> + #include <openssl/asn1.h> #include <openssl/asn1t.h> +#include <openssl/bytestring.h> #include <openssl/err.h> #include <openssl/evp.h> #include <openssl/mem.h> @@ -63,7 +66,6 @@ #include <openssl/thread.h> #include <openssl/x509.h> -#include "../evp/internal.h" #include "../internal.h" /* Minor tweak to operation: free up EVP_PKEY */ @@ -87,51 +89,50 @@ IMPLEMENT_ASN1_FUNCTIONS(X509_PUBKEY) int X509_PUBKEY_set(X509_PUBKEY **x, EVP_PKEY *pkey) { X509_PUBKEY *pk = NULL; + uint8_t *spki = NULL; + size_t spki_len; if (x == NULL) return (0); - if ((pk = X509_PUBKEY_new()) == NULL) + CBB cbb; + if (!CBB_init(&cbb, 0) || + !EVP_marshal_public_key(&cbb, pkey) || + !CBB_finish(&cbb, &spki, &spki_len) || + spki_len > LONG_MAX) { + CBB_cleanup(&cbb); + OPENSSL_PUT_ERROR(X509, X509_R_PUBLIC_KEY_ENCODE_ERROR); goto error; + } - if (pkey->ameth) { - if (pkey->ameth->pub_encode) { - if (!pkey->ameth->pub_encode(pk, pkey)) { - OPENSSL_PUT_ERROR(X509, X509_R_PUBLIC_KEY_ENCODE_ERROR); - goto error; - } - } else { - OPENSSL_PUT_ERROR(X509, X509_R_METHOD_NOT_SUPPORTED); - goto error; - } - } else { - OPENSSL_PUT_ERROR(X509, X509_R_UNSUPPORTED_ALGORITHM); + const uint8_t *p = spki; + pk = d2i_X509_PUBKEY(NULL, &p, (long)spki_len); + if (pk == NULL || p != spki + spki_len) { + OPENSSL_PUT_ERROR(X509, X509_R_PUBLIC_KEY_DECODE_ERROR); goto error; } - if (*x != NULL) - X509_PUBKEY_free(*x); - + OPENSSL_free(spki); + X509_PUBKEY_free(*x); *x = pk; return 1; error: - if (pk != NULL) - X509_PUBKEY_free(pk); + X509_PUBKEY_free(pk); + OPENSSL_free(spki); return 0; } -/* - * g_pubkey_lock is used to protect the initialisation of the |pkey| member - * of |X509_PUBKEY| objects. Really |X509_PUBKEY| should have a - * |CRYPTO_once_t| inside it for this, but |CRYPTO_once_t| is private and - * |X509_PUBKEY| is not. - */ +/* g_pubkey_lock is used to protect the initialisation of the |pkey| member of + * |X509_PUBKEY| objects. Really |X509_PUBKEY| should have a |CRYPTO_once_t| + * inside it for this, but |CRYPTO_once_t| is private and |X509_PUBKEY| is + * not. */ static struct CRYPTO_STATIC_MUTEX g_pubkey_lock = CRYPTO_STATIC_MUTEX_INIT; EVP_PKEY *X509_PUBKEY_get(X509_PUBKEY *key) { EVP_PKEY *ret = NULL; + uint8_t *spki = NULL; if (key == NULL) goto error; @@ -143,26 +144,16 @@ EVP_PKEY *X509_PUBKEY_get(X509_PUBKEY *key) } CRYPTO_STATIC_MUTEX_unlock(&g_pubkey_lock); - if (key->public_key == NULL) - goto error; - - if ((ret = EVP_PKEY_new()) == NULL) { - OPENSSL_PUT_ERROR(X509, ERR_R_MALLOC_FAILURE); + /* Re-encode the |X509_PUBKEY| to DER and parse it. */ + int spki_len = i2d_X509_PUBKEY(key, &spki); + if (spki_len < 0) { goto error; } - - if (!EVP_PKEY_set_type(ret, OBJ_obj2nid(key->algor->algorithm))) { - OPENSSL_PUT_ERROR(X509, X509_R_UNSUPPORTED_ALGORITHM); - goto error; - } - - if (ret->ameth->pub_decode) { - if (!ret->ameth->pub_decode(ret, key)) { - OPENSSL_PUT_ERROR(X509, X509_R_PUBLIC_KEY_DECODE_ERROR); - goto error; - } - } else { - OPENSSL_PUT_ERROR(X509, X509_R_METHOD_NOT_SUPPORTED); + CBS cbs; + CBS_init(&cbs, spki, (size_t)spki_len); + ret = EVP_parse_public_key(&cbs); + if (ret == NULL || CBS_len(&cbs) != 0) { + OPENSSL_PUT_ERROR(X509, X509_R_PUBLIC_KEY_DECODE_ERROR); goto error; } @@ -177,12 +168,13 @@ EVP_PKEY *X509_PUBKEY_get(X509_PUBKEY *key) CRYPTO_STATIC_MUTEX_unlock(&g_pubkey_lock); } + OPENSSL_free(spki); return EVP_PKEY_up_ref(ret); error: - if (ret != NULL) - EVP_PKEY_free(ret); - return (NULL); + OPENSSL_free(spki); + EVP_PKEY_free(ret); + return NULL; } /* |