From 36c37944909496a123e2656ad1f651769a7cc72f Mon Sep 17 00:00:00 2001 From: Viktor Dukhovni Date: Mon, 2 May 2016 15:00:21 -0400 Subject: [PATCH] Fix i2d_X509_AUX and update docs When *pp is NULL, don't write garbage, return an unexpected pointer or leak memory on error. Reviewed-by: Dr. Stephen Henson --- crypto/asn1/x_x509.c | 54 +++++++++++++++++++++++++++++++++++++++-- doc/crypto/d2i_X509.pod | 14 ++++++++++- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/crypto/asn1/x_x509.c b/crypto/asn1/x_x509.c index e31e1e750d..aada4a8413 100644 --- a/crypto/asn1/x_x509.c +++ b/crypto/asn1/x_x509.c @@ -199,12 +199,26 @@ X509 *d2i_X509_AUX(X509 **a, const unsigned char **pp, long length) return NULL; } -int i2d_X509_AUX(X509 *a, unsigned char **pp) +/* + * Serialize trusted certificate to *pp or just return the required buffer + * length if pp == NULL. We ultimately want to avoid modifying *pp in the + * error path, but that depends on similar hygiene in lower-level functions. + * Here we avoid compounding the problem. + */ +static int i2d_x509_aux_internal(X509 *a, unsigned char **pp) { int length, tmplen; unsigned char *start = pp != NULL ? *pp : NULL; + + OPENSSL_assert(pp == NULL || *pp != NULL); + + /* + * This might perturb *pp on error, but fixing that belongs in i2d_X509() + * not here. It should be that if a == NULL length is zero, but we check + * both just in case. + */ length = i2d_X509(a, pp); - if (length < 0 || a == NULL) + if (length <= 0 || a == NULL) return length; tmplen = i2d_X509_CERT_AUX(a->aux, pp); @@ -218,6 +232,42 @@ int i2d_X509_AUX(X509 *a, unsigned char **pp) return length; } +/* + * Serialize trusted certificate to *pp, or just return the required buffer + * length if pp == NULL. + * + * When pp is not NULL, but *pp == NULL, we allocate the buffer, but since + * we're writing two ASN.1 objects back to back, we can't have i2d_X509() do + * the allocation, nor can we allow i2d_X509_CERT_AUX() to increment the + * allocated buffer. + */ +int i2d_X509_AUX(X509 *a, unsigned char **pp) +{ + int length; + unsigned char *tmp; + + /* Buffer provided by caller */ + if (pp == NULL || *pp != NULL) + return i2d_x509_aux_internal(a, pp); + + /* Obtain the combined length */ + if ((length = i2d_x509_aux_internal(a, NULL)) <= 0) + return length; + + /* Allocate requisite combined storage */ + *pp = tmp = OPENSSL_malloc(length); + if (tmp == NULL) + return -1; /* Push error onto error stack? */ + + /* Encode, but keep *pp at the originally malloced pointer */ + length = i2d_x509_aux_internal(a, &tmp); + if (length <= 0) { + OPENSSL_free(*pp); + *pp = NULL; + } + return length; +} + int i2d_re_X509_tbs(X509 *x, unsigned char **pp) { x->cert_info->enc.modified = 1; diff --git a/doc/crypto/d2i_X509.pod b/doc/crypto/d2i_X509.pod index 5b7c16fd03..2743bc73e7 100644 --- a/doc/crypto/d2i_X509.pod +++ b/doc/crypto/d2i_X509.pod @@ -9,8 +9,10 @@ i2d_X509_fp - X509 encode and decode functions #include - X509 *d2i_X509(X509 **px, const unsigned char **in, int len); + X509 *d2i_X509(X509 **px, const unsigned char **in, long len); + X509 *d2i_X509_AUX(X509 **px, const unsigned char **in, long len); int i2d_X509(X509 *x, unsigned char **out); + int i2d_X509_AUX(X509 *x, unsigned char **out); X509 *d2i_X509_bio(BIO *bp, X509 **x); X509 *d2i_X509_fp(FILE *fp, X509 **x); @@ -37,6 +39,11 @@ below, and the discussion in the RETURN VALUES section). If the call is successful B<*in> is incremented to the byte following the parsed data. +d2i_X509_AUX() is similar to d2i_X509() but the input is expected to consist of +an X509 certificate followed by auxiliary trust information. +This is used by the PEM routines to read "TRUSTED CERTIFICATE" objects. +This function should not be called on untrusted input. + i2d_X509() encodes the structure pointed to by B into DER format. If B is not B is writes the DER encoded data to the buffer at B<*out>, and increments it to point after the data just written. @@ -48,6 +55,11 @@ allocated for a buffer and the encoded data written to it. In this case B<*out> is not incremented and it points to the start of the data just written. +i2d_X509_AUX() is similar to i2d_X509(), but the encoded output contains both +the certificate and any auxiliary trust information. +This is used by the PEM routines to write "TRUSTED CERTIFICATE" objects. +Note, this is a non-standard OpenSSL-specific data format. + d2i_X509_bio() is similar to d2i_X509() except it attempts to parse data from BIO B. -- 2.25.1