Reimplement d2i_PrivateKey.
Functions which lose object reuse and need auditing:
- d2i_PrivateKey
This removes evp_asn1.c's dependency on the old stack. (Aside from
obj/.) It also takes old_priv_decode out of EVP_ASN1_METHOD in favor of
calling out to the new-style function. EVP_ASN1_METHOD no longer has any
old-style type-specific serialization hooks, only the PKCS#8 and SPKI
ones.
BUG=499653
Change-Id: Ic142dc05a5505b50e4717c260d3893b20e680194
Reviewed-on: https://boringssl-review.googlesource.com/7027
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/evp/evp_asn1.c b/crypto/evp/evp_asn1.c
index 20fa2c1..f9bd51c 100644
--- a/crypto/evp/evp_asn1.c
+++ b/crypto/evp/evp_asn1.c
@@ -62,7 +62,6 @@
#include <openssl/err.h>
#include <openssl/obj.h>
#include <openssl/rsa.h>
-#include <openssl/x509.h>
#include "internal.h"
@@ -164,61 +163,74 @@
return key->ameth->priv_encode(cbb, key);
}
+static EVP_PKEY *old_priv_decode(CBS *cbs, int type) {
+ EVP_PKEY *ret = EVP_PKEY_new();
+ if (ret == NULL) {
+ return NULL;
+ }
+
+ switch (type) {
+ case EVP_PKEY_EC: {
+ EC_KEY *ec_key = EC_KEY_parse_private_key(cbs, NULL);
+ if (ec_key == NULL || !EVP_PKEY_assign_EC_KEY(ret, ec_key)) {
+ EC_KEY_free(ec_key);
+ goto err;
+ }
+ return ret;
+ }
+ case EVP_PKEY_DSA: {
+ DSA *dsa = DSA_parse_private_key(cbs);
+ if (dsa == NULL || !EVP_PKEY_assign_DSA(ret, dsa)) {
+ DSA_free(dsa);
+ goto err;
+ }
+ return ret;
+ }
+ case EVP_PKEY_RSA: {
+ RSA *rsa = RSA_parse_private_key(cbs);
+ if (rsa == NULL || !EVP_PKEY_assign_RSA(ret, rsa)) {
+ RSA_free(rsa);
+ goto err;
+ }
+ return ret;
+ }
+ default:
+ OPENSSL_PUT_ERROR(EVP, EVP_R_UNKNOWN_PUBLIC_KEY_TYPE);
+ goto err;
+ }
+
+err:
+ EVP_PKEY_free(ret);
+ return NULL;
+}
+
EVP_PKEY *d2i_PrivateKey(int type, EVP_PKEY **out, const uint8_t **inp,
long len) {
- EVP_PKEY *ret;
+ if (len < 0) {
+ OPENSSL_PUT_ERROR(EVP, EVP_R_DECODE_ERROR);
+ return NULL;
+ }
- if (out == NULL || *out == NULL) {
- ret = EVP_PKEY_new();
+ /* Parse with the legacy format. */
+ CBS cbs;
+ CBS_init(&cbs, *inp, (size_t)len);
+ EVP_PKEY *ret = old_priv_decode(&cbs, type);
+ if (ret == NULL) {
+ /* Try again with PKCS#8. */
+ ERR_clear_error();
+ CBS_init(&cbs, *inp, (size_t)len);
+ ret = EVP_parse_private_key(&cbs);
if (ret == NULL) {
- OPENSSL_PUT_ERROR(EVP, ERR_R_EVP_LIB);
return NULL;
}
- } else {
- ret = *out;
- }
-
- if (!EVP_PKEY_set_type(ret, type)) {
- OPENSSL_PUT_ERROR(EVP, EVP_R_UNKNOWN_PUBLIC_KEY_TYPE);
- goto err;
- }
-
- const uint8_t *in = *inp;
- /* If trying to remove |old_priv_decode|, note that some code depends on this
- * function writing into |*out| and the |priv_decode| path doesn't support
- * that. */
- if (!ret->ameth->old_priv_decode ||
- !ret->ameth->old_priv_decode(ret, &in, len)) {
- if (ret->ameth->priv_decode) {
- /* Reset |in| in case |old_priv_decode| advanced it on error. */
- in = *inp;
- PKCS8_PRIV_KEY_INFO *p8 = d2i_PKCS8_PRIV_KEY_INFO(NULL, &in, len);
- if (!p8) {
- goto err;
- }
- EVP_PKEY_free(ret);
- ret = EVP_PKCS82PKEY(p8);
- PKCS8_PRIV_KEY_INFO_free(p8);
- if (ret == NULL) {
- goto err;
- }
- } else {
- OPENSSL_PUT_ERROR(EVP, ERR_R_ASN1_LIB);
- goto err;
- }
}
if (out != NULL) {
+ EVP_PKEY_free(*out);
*out = ret;
}
- *inp = in;
+ *inp = CBS_data(&cbs);
return ret;
-
-err:
- if (out == NULL || *out != ret) {
- EVP_PKEY_free(ret);
- }
- return NULL;
}
/* num_elements parses one SEQUENCE from |in| and returns the number of elements
diff --git a/crypto/evp/internal.h b/crypto/evp/internal.h
index 3347cd4..d509539 100644
--- a/crypto/evp/internal.h
+++ b/crypto/evp/internal.h
@@ -112,12 +112,6 @@
int (*param_cmp)(const EVP_PKEY *a, const EVP_PKEY *b);
void (*pkey_free)(EVP_PKEY *pkey);
-
- /* Legacy functions for old PEM */
-
- int (*old_priv_decode)(EVP_PKEY *pkey, const uint8_t **pder,
- int derlen);
-
} /* EVP_PKEY_ASN1_METHOD */;
diff --git a/crypto/evp/p_dsa_asn1.c b/crypto/evp/p_dsa_asn1.c
index c3dbe50..f6d625e 100644
--- a/crypto/evp/p_dsa_asn1.c
+++ b/crypto/evp/p_dsa_asn1.c
@@ -241,18 +241,6 @@
static void int_dsa_free(EVP_PKEY *pkey) { DSA_free(pkey->pkey.dsa); }
-static int old_dsa_priv_decode(EVP_PKEY *pkey, const uint8_t **pder,
- int derlen) {
- DSA *dsa;
- dsa = d2i_DSAPrivateKey(NULL, pder, derlen);
- if (dsa == NULL) {
- OPENSSL_PUT_ERROR(EVP, ERR_R_DSA_LIB);
- return 0;
- }
- EVP_PKEY_assign_DSA(pkey, dsa);
- return 1;
-}
-
const EVP_PKEY_ASN1_METHOD dsa_asn1_meth = {
EVP_PKEY_DSA,
@@ -274,5 +262,4 @@
dsa_cmp_parameters,
int_dsa_free,
- old_dsa_priv_decode,
};
diff --git a/crypto/evp/p_ec_asn1.c b/crypto/evp/p_ec_asn1.c
index 738b7fb..4e51440 100644
--- a/crypto/evp/p_ec_asn1.c
+++ b/crypto/evp/p_ec_asn1.c
@@ -238,17 +238,6 @@
return EC_KEY_is_opaque(pkey->pkey.ec);
}
-static int old_ec_priv_decode(EVP_PKEY *pkey, const uint8_t **pder,
- int derlen) {
- EC_KEY *ec;
- if (!(ec = d2i_ECPrivateKey(NULL, pder, derlen))) {
- OPENSSL_PUT_ERROR(EVP, EVP_R_DECODE_ERROR);
- return 0;
- }
- EVP_PKEY_assign_EC_KEY(pkey, ec);
- return 1;
-}
-
const EVP_PKEY_ASN1_METHOD ec_asn1_meth = {
EVP_PKEY_EC,
@@ -270,5 +259,4 @@
ec_cmp_parameters,
int_ec_free,
- old_ec_priv_decode,
};
diff --git a/crypto/evp/p_rsa_asn1.c b/crypto/evp/p_rsa_asn1.c
index 7138ad7..3a00241 100644
--- a/crypto/evp/p_rsa_asn1.c
+++ b/crypto/evp/p_rsa_asn1.c
@@ -175,17 +175,6 @@
static void int_rsa_free(EVP_PKEY *pkey) { RSA_free(pkey->pkey.rsa); }
-static int old_rsa_priv_decode(EVP_PKEY *pkey, const uint8_t **pder,
- int derlen) {
- RSA *rsa = d2i_RSAPrivateKey(NULL, pder, derlen);
- if (rsa == NULL) {
- OPENSSL_PUT_ERROR(EVP, ERR_R_RSA_LIB);
- return 0;
- }
- EVP_PKEY_assign_RSA(pkey, rsa);
- return 1;
-}
-
const EVP_PKEY_ASN1_METHOD rsa_asn1_meth = {
EVP_PKEY_RSA,
@@ -205,6 +194,4 @@
0,0,0,
int_rsa_free,
-
- old_rsa_priv_decode,
};
diff --git a/include/openssl/evp.h b/include/openssl/evp.h
index f9b9496..595672c 100644
--- a/include/openssl/evp.h
+++ b/include/openssl/evp.h
@@ -698,11 +698,10 @@
/* d2i_PrivateKey parses an ASN.1, DER-encoded, private key from |len| bytes at
* |*inp|. If |out| is not NULL then, on exit, a pointer to the result is in
- * |*out|. If |*out| is already non-NULL on entry then the result is written
- * directly into |*out|, otherwise a fresh |EVP_PKEY| is allocated. However,
- * one should not depend on writing into |*out| because this behaviour is
- * likely to change in the future. On successful exit, |*inp| is advanced past
- * the DER structure. It returns the result or NULL on error.
+ * |*out|. Note that, even if |*out| is already non-NULL on entry, it will not
+ * be written to. Rather, a fresh |EVP_PKEY| is allocated and the previous one
+ * is freed. On successful exit, |*inp| is advanced past the DER structure. It
+ * returns the result or NULL on error.
*
* This function tries to detect one of several formats. Instead, use
* |EVP_parse_private_key| for a PrivateKeyInfo, |RSA_parse_private_key| for an
