From 3d15a94add73c195fa0e68250afae8c030818437 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sun, 14 Mar 2021 14:46:21 -0400 Subject: [PATCH] Document ASN1_TYPE and related functions. The representation here is a bit more messy than necessary. In doing so, clean up the variable names and smooth away two rough edges: - X509_ALGOR_get0 would leave *out_param_value uninitialized if *out_param_type is V_ASN1_UNDEF. Instead, set it to NULL, so callers do not accidentally use an uninitialized pointer. - X509_PUBKEY_set0_param, if key is NULL, would leave the key alone. No one calls this function externally and none of the (since removed) callers in OpenSSL rely on this behavior. A NULL check here adds a discontinuity at the empty string that seems unnecessary here: changing the algorithm without changing the key isn't useful. (Note the API doesn't support changing the key without the algorithm.) Note for reviewing: the representation of ASN1_TYPE is specified somewhat indirectly. ASN1_TYPE uses the ASN1_ANY ASN1_ITEM, which has utype V_ASN1_ANY. Then you look at asn1_d2i_ex_primitive and asn1_ex_c2i which peel off the ASN1_TYPE layer and parse directly into the value field, with a fixup for NULL. Hopefully we can rework this someday... Change-Id: I628c4e20f8ea2fd036132242337f4dcac5ba5015 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46165 Commit-Queue: David Benjamin Reviewed-by: Adam Langley --- crypto/asn1/asn1_lib.c | 3 +- crypto/x509/x_algor.c | 28 ++++---- crypto/x509/x_pubkey.c | 41 ++++++------ include/openssl/asn1.h | 149 ++++++++++++++++++++++++++++++----------- include/openssl/x509.h | 79 +++++++++++++++++++--- 5 files changed, 215 insertions(+), 85 deletions(-) diff --git a/crypto/asn1/asn1_lib.c b/crypto/asn1/asn1_lib.c index db8afac09..b13a82a9e 100644 --- a/crypto/asn1/asn1_lib.c +++ b/crypto/asn1/asn1_lib.c @@ -370,8 +370,7 @@ int ASN1_STRING_set(ASN1_STRING *str, const void *_data, int len) void ASN1_STRING_set0(ASN1_STRING *str, void *data, int len) { - if (str->data) - OPENSSL_free(str->data); + OPENSSL_free(str->data); str->data = data; str->length = len; } diff --git a/crypto/x509/x_algor.c b/crypto/x509/x_algor.c index 7b3d7903d..a7d5dd6e5 100644 --- a/crypto/x509/x_algor.c +++ b/crypto/x509/x_algor.c @@ -103,19 +103,23 @@ int X509_ALGOR_set0(X509_ALGOR *alg, ASN1_OBJECT *aobj, int ptype, void *pval) return 1; } -void X509_ALGOR_get0(const ASN1_OBJECT **paobj, int *pptype, const void **ppval, - const X509_ALGOR *algor) +void X509_ALGOR_get0(const ASN1_OBJECT **out_obj, int *out_param_type, + const void **out_param_value, const X509_ALGOR *alg) { - if (paobj) - *paobj = algor->algorithm; - if (pptype) { - if (algor->parameter == NULL) { - *pptype = V_ASN1_UNDEF; - return; - } else - *pptype = algor->parameter->type; - if (ppval) - *ppval = algor->parameter->value.ptr; + if (out_obj != NULL) { + *out_obj = alg->algorithm; + } + if (out_param_type != NULL) { + int type = V_ASN1_UNDEF; + void *value = NULL; + if (alg->parameter != NULL) { + type = alg->parameter->type; + value = alg->parameter->value.ptr; + } + *out_param_type = type; + if (out_param_value != NULL) { + *out_param_value = value; + } } } diff --git a/crypto/x509/x_pubkey.c b/crypto/x509/x_pubkey.c index c3fb1e390..9e2f7048f 100644 --- a/crypto/x509/x_pubkey.c +++ b/crypto/x509/x_pubkey.c @@ -180,34 +180,33 @@ EVP_PKEY *X509_PUBKEY_get(X509_PUBKEY *key) return NULL; } -int X509_PUBKEY_set0_param(X509_PUBKEY *pub, ASN1_OBJECT *aobj, int ptype, - void *pval, unsigned char *penc, int penclen) +int X509_PUBKEY_set0_param(X509_PUBKEY *pub, ASN1_OBJECT *obj, int param_type, + void *param_value, uint8_t *key, int key_len) { - if (!X509_ALGOR_set0(pub->algor, aobj, ptype, pval)) + if (!X509_ALGOR_set0(pub->algor, obj, param_type, param_value)) { return 0; - if (penc) { - if (pub->public_key->data) - OPENSSL_free(pub->public_key->data); - pub->public_key->data = penc; - pub->public_key->length = penclen; - /* Set number of unused bits to zero */ - pub->public_key->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); - pub->public_key->flags |= ASN1_STRING_FLAG_BITS_LEFT; } + + ASN1_STRING_set0(pub->public_key, key, key_len); + /* Set the number of unused bits to zero. */ + pub->public_key->flags &= ~(ASN1_STRING_FLAG_BITS_LEFT | 0x07); + pub->public_key->flags |= ASN1_STRING_FLAG_BITS_LEFT; return 1; } -int X509_PUBKEY_get0_param(ASN1_OBJECT **ppkalg, - const unsigned char **pk, int *ppklen, - X509_ALGOR **pa, X509_PUBKEY *pub) +int X509_PUBKEY_get0_param(ASN1_OBJECT **out_obj, const uint8_t **out_key, + int *out_key_len, X509_ALGOR **out_alg, + X509_PUBKEY *pub) { - if (ppkalg) - *ppkalg = pub->algor->algorithm; - if (pk) { - *pk = pub->public_key->data; - *ppklen = pub->public_key->length; + if (out_obj != NULL) { + *out_obj = pub->algor->algorithm; + } + if (out_key != NULL) { + *out_key = pub->public_key->data; + *out_key_len = pub->public_key->length; + } + if (out_alg != NULL) { + *out_alg = pub->algor; } - if (pa) - *pa = pub->algor; return 1; } diff --git a/include/openssl/asn1.h b/include/openssl/asn1.h index 3c75ee884..4857e3e21 100644 --- a/include/openssl/asn1.h +++ b/include/openssl/asn1.h @@ -197,9 +197,8 @@ extern "C" { // the DER encoding of the value. For example, the UNIX epoch would be // "19700101000000Z" for a GeneralizedTime and "700101000000Z" for a UTCTime. // -// TODO(davidben): |ASN1_TYPE| additionally uses |ASN1_STRING| to represent -// various other odd cases. It also likes to assume unknown universal tags are -// string types. Make a note here when documenting |ASN1_TYPE|. +// |ASN1_STRING|, when stored in an |ASN1_TYPE|, may also represent an element +// with tag not directly supported by this library. See |ASN1_TYPE| for details. // // |ASN1_STRING| additionally has the following typedefs: |ASN1_BIT_STRING|, // |ASN1_BMPSTRING|, |ASN1_ENUMERATED|, |ASN1_GENERALIZEDTIME|, @@ -313,6 +312,114 @@ OPENSSL_EXPORT void ASN1_STRING_set0(ASN1_STRING *str, void *data, int len); // types. +// Arbitrary elements. + +// ASN1_VALUE_st (aka |ASN1_VALUE|) is an opaque type used internally in the +// library. +typedef struct ASN1_VALUE_st ASN1_VALUE; + +// An asn1_type_st (aka |ASN1_TYPE|) represents an arbitrary ASN.1 element, +// typically used used for ANY types. It contains a |type| field and a |value| +// union dependent on |type|. +// +// WARNING: This struct has a complex representation. Callers performing +// non-trivial operations on this type are encouraged to use |CBS| and |CBB| +// from , and convert to or from |ASN1_TYPE| with +// |d2i_ASN1_TYPE| or |i2d_ASN1_TYPE|. +// +// The |type| field corresponds to the tag of the ASN.1 element being +// represented: +// +// If |type| is a |V_ASN1_*| constant for an ASN.1 string-like type, as defined +// by |ASN1_STRING|, the tag matches the constant. |value| contains an +// |ASN1_STRING| pointer (equivalently, one of the more specific typedefs). See +// |ASN1_STRING| for details on the representation. Unlike |ASN1_STRING|, +// |ASN1_TYPE| does not use the |V_ASN1_NEG| flag for negative INTEGER and +// ENUMERATE values. For a negative value, the |ASN1_TYPE|'s |type| will be +// |V_ASN1_INTEGER| or |V_ASN1_ENUMERATED|, but |value| will an |ASN1_STRING| +// whose |type| is |V_ASN1_NEG_INTEGER| or |V_ASN1_NEG_ENUMERATED|. +// +// If |type| is |V_ASN1_OBJECT|, the tag is OBJECT IDENTIFIER and |value| +// contains an |ASN1_OBJECT| pointer. +// +// If |type| is |V_ASN1_NULL|, the tag is NULL. |value| contains a NULL pointer. +// +// If |type| is |V_ASN1_BOOLEAN|, the tag is BOOLEAN. |value| contains an +// |ASN1_BOOLEAN|. While |ASN1_BOOLEAN| is an |int|, any unused bits of |value| +// must also be zero, so callers must not construct these values manually. +// +// TODO(davidben): Fix code which relies on this. It's anything which looks at +// |value.ptr| or |value.asn1_value| unconditionally. +// +// If |type| is |V_ASN1_SEQUENCE|, |V_ASN1_SET|, or |V_ASN1_OTHER|, the tag is +// SEQUENCE, SET, or some non-universal tag, respectively. |value| is an +// |ASN1_STRING| containing the entire element, including the tag and length. +// The |ASN1_STRING|'s |type| field matches the containing |ASN1_TYPE|'s |type|. +// +// Other positive values of |type|, up to |V_ASN1_MAX_UNIVERSAL|, correspond to +// universal primitive tags not directly supported by this library. |value| is +// an |ASN1_STRING| containing the body of the element, excluding the tag +// and length. The |ASN1_STRING|'s |type| field matches the containing +// |ASN1_TYPE|'s |type|. +struct asn1_type_st { + int type; + union { + char *ptr; + ASN1_BOOLEAN boolean; + ASN1_STRING *asn1_string; + ASN1_OBJECT *object; + ASN1_INTEGER *integer; + ASN1_ENUMERATED *enumerated; + ASN1_BIT_STRING *bit_string; + ASN1_OCTET_STRING *octet_string; + ASN1_PRINTABLESTRING *printablestring; + ASN1_T61STRING *t61string; + ASN1_IA5STRING *ia5string; + ASN1_GENERALSTRING *generalstring; + ASN1_BMPSTRING *bmpstring; + ASN1_UNIVERSALSTRING *universalstring; + ASN1_UTCTIME *utctime; + ASN1_GENERALIZEDTIME *generalizedtime; + ASN1_VISIBLESTRING *visiblestring; + ASN1_UTF8STRING *utf8string; + // set and sequence are left complete and still contain the entire element. + ASN1_STRING *set; + ASN1_STRING *sequence; + ASN1_VALUE *asn1_value; + } value; +}; + +// ASN1_TYPE_get returns the type of |a|, which will be one of the |V_ASN1_*| +// constants, or zero if |a| is not fully initialized. +OPENSSL_EXPORT int ASN1_TYPE_get(const ASN1_TYPE *a); + +// ASN1_TYPE_set sets |a| to an |ASN1_TYPE| of type |type| and value |value|, +// releasing the previous contents of |a|. +// +// If |type| is |V_ASN1_BOOLEAN|, |a| is set to FALSE if |value| is NULL and +// TRUE otherwise. If setting |a| to TRUE, |value| may be an invalid pointer, +// such as (void*)1. +// +// If |type| is |V_ASN1_NULL|, |value| must be NULL. +// +// For other values of |type|, this function takes ownership of |value|, which +// must point to an object of the corresponding type. See |ASN1_TYPE| for +// details. +OPENSSL_EXPORT void ASN1_TYPE_set(ASN1_TYPE *a, int type, void *value); + +// ASN1_TYPE_set1 behaves like |ASN1_TYPE_set| except it does not take ownership +// of |value|. It returns one on success and zero on error. +OPENSSL_EXPORT int ASN1_TYPE_set1(ASN1_TYPE *a, int type, const void *value); + +// ASN1_TYPE_cmp returns zero if |a| and |b| are equal and some non-zero value +// otherwise. Note this function can only be used for equality checks, not an +// ordering. +OPENSSL_EXPORT int ASN1_TYPE_cmp(const ASN1_TYPE *a, const ASN1_TYPE *b); + +// TODO(davidben): Most of |ASN1_TYPE|'s APIs are hidden behind macros. Expand +// the macros, document them, and move them to this section. + + // Underdocumented functions. // // The following functions are not yet documented and organized. @@ -424,8 +531,6 @@ typedef struct asn1_string_table_st { // see asn1t.h typedef struct ASN1_TEMPLATE_st ASN1_TEMPLATE; typedef struct ASN1_TLC_st ASN1_TLC; -// This is just an opaque pointer -typedef struct ASN1_VALUE_st ASN1_VALUE; // Declare ASN1 functions: the implement macro in in asn1t.h @@ -591,35 +696,6 @@ typedef const ASN1_ITEM ASN1_ITEM_EXP; DEFINE_STACK_OF(ASN1_INTEGER) DECLARE_ASN1_SET_OF(ASN1_INTEGER) -struct asn1_type_st { - int type; - union { - char *ptr; - ASN1_BOOLEAN boolean; - ASN1_STRING *asn1_string; - ASN1_OBJECT *object; - ASN1_INTEGER *integer; - ASN1_ENUMERATED *enumerated; - ASN1_BIT_STRING *bit_string; - ASN1_OCTET_STRING *octet_string; - ASN1_PRINTABLESTRING *printablestring; - ASN1_T61STRING *t61string; - ASN1_IA5STRING *ia5string; - ASN1_GENERALSTRING *generalstring; - ASN1_BMPSTRING *bmpstring; - ASN1_UNIVERSALSTRING *universalstring; - ASN1_UTCTIME *utctime; - ASN1_GENERALIZEDTIME *generalizedtime; - ASN1_VISIBLESTRING *visiblestring; - ASN1_UTF8STRING *utf8string; - // set and sequence are left complete and still - // contain the set or sequence bytes - ASN1_STRING *set; - ASN1_STRING *sequence; - ASN1_VALUE *asn1_value; - } value; -}; - DEFINE_STACK_OF(ASN1_TYPE) DECLARE_ASN1_SET_OF(ASN1_TYPE) @@ -706,11 +782,6 @@ typedef struct BIT_STRING_BITNAME_st { DECLARE_ASN1_FUNCTIONS_fname(ASN1_TYPE, ASN1_ANY, ASN1_TYPE) -OPENSSL_EXPORT int ASN1_TYPE_get(const ASN1_TYPE *a); -OPENSSL_EXPORT void ASN1_TYPE_set(ASN1_TYPE *a, int type, void *value); -OPENSSL_EXPORT int ASN1_TYPE_set1(ASN1_TYPE *a, int type, const void *value); -OPENSSL_EXPORT int ASN1_TYPE_cmp(const ASN1_TYPE *a, const ASN1_TYPE *b); - OPENSSL_EXPORT ASN1_OBJECT *ASN1_OBJECT_new(void); OPENSSL_EXPORT void ASN1_OBJECT_free(ASN1_OBJECT *a); OPENSSL_EXPORT int i2d_ASN1_OBJECT(const ASN1_OBJECT *a, unsigned char **pp); diff --git a/include/openssl/x509.h b/include/openssl/x509.h index 5263d3110..24c589ba4 100644 --- a/include/openssl/x509.h +++ b/include/openssl/x509.h @@ -905,12 +905,54 @@ OPENSSL_EXPORT X509_CRL *X509_CRL_dup(X509_CRL *crl); OPENSSL_EXPORT X509_REVOKED *X509_REVOKED_dup(X509_REVOKED *rev); OPENSSL_EXPORT X509_REQ *X509_REQ_dup(X509_REQ *req); OPENSSL_EXPORT X509_ALGOR *X509_ALGOR_dup(X509_ALGOR *xn); -OPENSSL_EXPORT int X509_ALGOR_set0(X509_ALGOR *alg, ASN1_OBJECT *aobj, - int ptype, void *pval); -OPENSSL_EXPORT void X509_ALGOR_get0(const ASN1_OBJECT **paobj, int *pptype, - const void **ppval, - const X509_ALGOR *algor); + +// X509_ALGOR_set0 sets |alg| to an AlgorithmIdentifier with algorithm |obj| and +// parameter determined by |param_type| and |param_value|. It returns one on +// success and zero on error. This function takes ownership of |obj| and +// |param_value| on success. +// +// If |param_type| is |V_ASN1_UNDEF|, the parameter is omitted. If |param_type| +// is zero, the parameter is left unchanged. Otherwise, |param_type| and +// |param_value| are interpreted as in |ASN1_TYPE_set|. +// +// Note omitting the parameter (|V_ASN1_UNDEF|) and encoding an explicit NULL +// value (|V_ASN1_NULL|) are different. Some algorithms require one and some the +// other. Consult the relevant specification before calling this function. The +// correct parameter for an RSASSA-PKCS1-v1_5 signature is |V_ASN1_NULL|. The +// correct one for an ECDSA or Ed25519 signature is |V_ASN1_UNDEF|. +OPENSSL_EXPORT int X509_ALGOR_set0(X509_ALGOR *alg, ASN1_OBJECT *obj, + int param_type, void *param_value); + +// X509_ALGOR_get0 sets |*out_obj| to the |alg|'s algorithm. If |alg|'s +// parameter is omitted, it sets |*out_param_type| and |*out_param_value| to +// |V_ASN1_UNDEF| and NULL. Otherwise, it sets |*out_param_type| and +// |*out_param_value| to the parameter, using the same representation as +// |ASN1_TYPE_set0|. See |ASN1_TYPE_set0| and |ASN1_TYPE| for details. +// +// Callers that require the parameter in serialized form should, after checking +// for |V_ASN1_UNDEF|, use |ASN1_TYPE_set1| and |d2i_ASN1_TYPE|, rather than +// inspecting |*out_param_value|. +// +// Each of |out_obj|, |out_param_type|, and |out_param_value| may be NULL to +// ignore the output. If |out_param_type| is NULL, |out_param_value| is ignored. +// +// WARNING: If |*out_param_type| is set to |V_ASN1_UNDEF|, OpenSSL and older +// revisions of BoringSSL leave |*out_param_value| unset rather than setting it +// to NULL. Callers that support both OpenSSL and BoringSSL should not assume +// |*out_param_value| is uniformly initialized. +OPENSSL_EXPORT void X509_ALGOR_get0(const ASN1_OBJECT **out_obj, + int *out_param_type, + const void **out_param_value, + const X509_ALGOR *alg); + +// X509_ALGOR_set_md sets |alg| to the hash function |md|. Note this +// AlgorithmIdentifier represents the hash function itself, not a signature +// algorithm that uses |md|. OPENSSL_EXPORT void X509_ALGOR_set_md(X509_ALGOR *alg, const EVP_MD *md); + +// X509_ALGOR_cmp returns zero if |a| and |b| are equal, and some non-zero value +// otherwise. Note this function can only be used for equality checks, not an +// ordering. OPENSSL_EXPORT int X509_ALGOR_cmp(const X509_ALGOR *a, const X509_ALGOR *b); OPENSSL_EXPORT X509_NAME *X509_NAME_dup(X509_NAME *xn); @@ -1538,12 +1580,27 @@ OPENSSL_EXPORT int PKCS8_pkey_get0(ASN1_OBJECT **ppkalg, const unsigned char **pk, int *ppklen, X509_ALGOR **pa, PKCS8_PRIV_KEY_INFO *p8); -OPENSSL_EXPORT int X509_PUBKEY_set0_param(X509_PUBKEY *pub, ASN1_OBJECT *aobj, - int ptype, void *pval, - unsigned char *penc, int penclen); -OPENSSL_EXPORT int X509_PUBKEY_get0_param(ASN1_OBJECT **ppkalg, - const unsigned char **pk, int *ppklen, - X509_ALGOR **pa, X509_PUBKEY *pub); +// X509_PUBKEY_set0_param sets |pub| to a key with AlgorithmIdentifier +// determined by |obj|, |param_type|, and |param_value|, and an encoded +// public key of |key|. On success, it takes ownership of all its parameters and +// returns one. Otherwise, it returns zero. |key| must have been allocated by +// |OPENSSL_malloc|. +// +// |obj|, |param_type|, and |param_value| are interpreted as in +// |X509_ALGOR_set0|. See |X509_ALGOR_set0| for details. +OPENSSL_EXPORT int X509_PUBKEY_set0_param(X509_PUBKEY *pub, ASN1_OBJECT *obj, + int param_type, void *param_value, + uint8_t *key, int key_len); + +// X509_PUBKEY_get0_param outputs fields of |pub| and returns one. If |out_obj| +// is not NULL, it sets |*out_obj| to AlgorithmIdentifier's OID. If |out_key| +// is not NULL, it sets |*out_key| and |*out_key_len| to the encoded public key. +// If |out_alg| is not NULL, it sets |*out_alg| to the AlgorithmIdentifier. +OPENSSL_EXPORT int X509_PUBKEY_get0_param(ASN1_OBJECT **out_obj, + const uint8_t **out_key, + int *out_key_len, + X509_ALGOR **out_alg, + X509_PUBKEY *pub); OPENSSL_EXPORT int X509_check_trust(X509 *x, int id, int flags); OPENSSL_EXPORT int X509_TRUST_get_count(void);