Opaquify PKCS8_PRIV_KEY_INFO. This is partially imported from upstream's 54dbf42398e23349b59f258a3dd60387bbc5ba13 which does something similar. In doing so, remove the pkcs8->broken field, which is a remnant of some parsing hacks we long since removed (PKCS8_set_broken). The immediate motivation is, if this sticks, this would make it easier to detach i2d_PKCS8_PRIV_KEY_INFO and d2i_PKCS8_PRIV_KEY_INFO from the old ASN.1 code. Update-Note: Direct accesses of PKCS8_PRIV_KEY_INFO now need to use the accessors. Code search suggests no one uses the fields. Even the accessors are virtually unused (the one thing which uses it doesn't need it). Bug: chromium:1102458 Change-Id: I57054de3fe412079f7387dc99291250e873b1471 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42006 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/pkcs8/internal.h b/crypto/pkcs8/internal.h index c3302f7..b5d42ba 100644 --- a/crypto/pkcs8/internal.h +++ b/crypto/pkcs8/internal.h
@@ -63,6 +63,13 @@ #endif +struct pkcs8_priv_key_info_st { + ASN1_INTEGER *version; + X509_ALGOR *pkeyalg; + ASN1_OCTET_STRING *pkey; + STACK_OF(X509_ATTRIBUTE) *attributes; +}; + // pkcs8_pbe_decrypt decrypts |in| using the PBE scheme described by // |algorithm|, which should be a serialized AlgorithmIdentifier structure. On // success, it sets |*out| to a newly-allocated buffer containing the decrypted
diff --git a/crypto/pkcs8/pkcs8_x509.c b/crypto/pkcs8/pkcs8_x509.c index 4458b56..a2f9075 100644 --- a/crypto/pkcs8/pkcs8_x509.c +++ b/crypto/pkcs8/pkcs8_x509.c
@@ -96,10 +96,8 @@ // Since the structure must still be valid use ASN1_OP_FREE_PRE if (operation == ASN1_OP_FREE_PRE) { PKCS8_PRIV_KEY_INFO *key = (PKCS8_PRIV_KEY_INFO *)*pval; - if (key->pkey && key->pkey->type == V_ASN1_OCTET_STRING && - key->pkey->value.octet_string) { - OPENSSL_cleanse(key->pkey->value.octet_string->data, - key->pkey->value.octet_string->length); + if (key->pkey) { + OPENSSL_cleanse(key->pkey->data, key->pkey->length); } } return 1; @@ -108,12 +106,45 @@ ASN1_SEQUENCE_cb(PKCS8_PRIV_KEY_INFO, pkey_cb) = { ASN1_SIMPLE(PKCS8_PRIV_KEY_INFO, version, ASN1_INTEGER), ASN1_SIMPLE(PKCS8_PRIV_KEY_INFO, pkeyalg, X509_ALGOR), - ASN1_SIMPLE(PKCS8_PRIV_KEY_INFO, pkey, ASN1_ANY), + ASN1_SIMPLE(PKCS8_PRIV_KEY_INFO, pkey, ASN1_OCTET_STRING), ASN1_IMP_SET_OF_OPT(PKCS8_PRIV_KEY_INFO, attributes, X509_ATTRIBUTE, 0) } ASN1_SEQUENCE_END_cb(PKCS8_PRIV_KEY_INFO, PKCS8_PRIV_KEY_INFO) IMPLEMENT_ASN1_FUNCTIONS(PKCS8_PRIV_KEY_INFO) +int PKCS8_pkey_set0(PKCS8_PRIV_KEY_INFO *priv, ASN1_OBJECT *aobj, int version, + int ptype, void *pval, uint8_t *penc, int penclen) { + if (version >= 0 && + !ASN1_INTEGER_set(priv->version, version)) { + return 0; + } + + if (!X509_ALGOR_set0(priv->pkeyalg, aobj, ptype, pval)) { + return 0; + } + + if (penc != NULL) { + ASN1_STRING_set0(priv->pkey, penc, penclen); + } + + return 1; +} + +int PKCS8_pkey_get0(ASN1_OBJECT **ppkalg, const uint8_t **pk, int *ppklen, + X509_ALGOR **pa, PKCS8_PRIV_KEY_INFO *p8) { + if (ppkalg) { + *ppkalg = p8->pkeyalg->algorithm; + } + if (pk) { + *pk = ASN1_STRING_data(p8->pkey); + *ppklen = ASN1_STRING_length(p8->pkey); + } + if (pa) { + *pa = p8->pkeyalg; + } + return 1; +} + EVP_PKEY *EVP_PKCS82PKEY(PKCS8_PRIV_KEY_INFO *p8) { uint8_t *der = NULL; int der_len = i2d_PKCS8_PRIV_KEY_INFO(p8, &der);
diff --git a/crypto/x509/x509.c b/crypto/x509/x509.c index 188fd49..9049a35 100644 --- a/crypto/x509/x509.c +++ b/crypto/x509/x509.c
@@ -65,73 +65,6 @@ * it to avoid downstream churn. */ OPENSSL_DECLARE_ERROR_REASON(X509, UNSUPPORTED_ALGORITHM) -int PKCS8_pkey_set0(PKCS8_PRIV_KEY_INFO *priv, ASN1_OBJECT *aobj, int version, - int ptype, void *pval, uint8_t *penc, int penclen) { - uint8_t **ppenc = NULL; - if (version >= 0) { - if (!ASN1_INTEGER_set(priv->version, version)) { - return 0; - } - } - - if (penc) { - int pmtype; - ASN1_OCTET_STRING *oct; - - oct = ASN1_OCTET_STRING_new(); - if (!oct) { - return 0; - } - oct->data = penc; - ppenc = &oct->data; - oct->length = penclen; - if (priv->broken == PKCS8_NO_OCTET) { - pmtype = V_ASN1_SEQUENCE; - } else { - pmtype = V_ASN1_OCTET_STRING; - } - ASN1_TYPE_set(priv->pkey, pmtype, oct); - } - - if (!X509_ALGOR_set0(priv->pkeyalg, aobj, ptype, pval)) { - /* If call fails do not swallow 'enc' */ - if (ppenc) { - *ppenc = NULL; - } - return 0; - } - - return 1; -} - -int PKCS8_pkey_get0(ASN1_OBJECT **ppkalg, const uint8_t **pk, int *ppklen, - X509_ALGOR **pa, PKCS8_PRIV_KEY_INFO *p8) { - if (ppkalg) { - *ppkalg = p8->pkeyalg->algorithm; - } - - if (p8->pkey->type == V_ASN1_OCTET_STRING) { - p8->broken = PKCS8_OK; - if (pk) { - *pk = p8->pkey->value.octet_string->data; - *ppklen = p8->pkey->value.octet_string->length; - } - } else if (p8->pkey->type == V_ASN1_SEQUENCE) { - p8->broken = PKCS8_NO_OCTET; - if (pk) { - *pk = p8->pkey->value.sequence->data; - *ppklen = p8->pkey->value.sequence->length; - } - } else { - return 0; - } - - if (pa) { - *pa = p8->pkeyalg; - } - return 1; -} - int X509_signature_dump(BIO *bp, const ASN1_STRING *sig, int indent) { const uint8_t *s; int i, n;
diff --git a/include/openssl/x509.h b/include/openssl/x509.h index 58dfad3..94dcc79 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h
@@ -460,21 +460,6 @@ ASN1_BIT_STRING *signature; } /* NETSCAPE_SPKI */; -// PKCS#8 private key info structure - -struct pkcs8_priv_key_info_st { - int broken; // Flag for various broken formats -#define PKCS8_OK 0 -#define PKCS8_NO_OCTET 1 -#define PKCS8_EMBEDDED_PARAM 2 -#define PKCS8_NS_DB 3 -#define PKCS8_NEG_PRIVKEY 4 - ASN1_INTEGER *version; - X509_ALGOR *pkeyalg; - ASN1_TYPE *pkey; // Should be OCTET STRING but some are broken - STACK_OF(X509_ATTRIBUTE) * attributes; -}; - #ifdef __cplusplus } #endif