Commit fde2257f authored by Viktor Dukhovni's avatar Viktor Dukhovni
Browse files

Fix i2d_X509_AUX, update docs and add tests 



When *pp is NULL, don't write garbage, return an unexpected pointer
or leak memory on error.

Reviewed-by: default avatarDr. Stephen Henson <steve@openssl.org>
parent 9b5164ce

crypto/x509/x_x509.c

+52 −2
Original line number Diff line number Diff line
@@ -181,12 +181,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);
@@ -200,6 +214,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;

doc/crypto/d2i_X509.pod

+13 −1
Original line number Diff line number Diff line
@@ -9,8 +9,10 @@ i2d_X509_fp - X509 encode and decode functions 


 #include <openssl/x509.h>



 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<x> into DER format.

If B<out> is not B<NULL> 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<bp>.

test/build.info

+5 −1
Original line number Diff line number Diff line
@@ -16,7 +16,7 @@ IF[{- !$disabled{tests} -}] 
          constant_time_test verify_extra_test clienthellotest \

          packettest asynctest secmemtest srptest memleaktest \

          dtlsv1listentest ct_test threadstest afalgtest d2i_test \

          ssl_test_ctx_test ssl_test

          ssl_test_ctx_test ssl_test x509aux



  SOURCE[aborttest]=aborttest.c

  INCLUDE[aborttest]={- rel2abs(catdir($builddir,"../include")) -} ../include
@@ -237,4 +237,8 @@ IF[{- !$disabled{tests} -}] 
  INCLUDE[testutil.o]=..

  INCLUDE[ssl_test_ctx.o]={- rel2abs(catdir($builddir,"../include")) -} ../include

  INCLUDE[handshake_helper.o]={- rel2abs(catdir($builddir,"../include")) -} ../include



  SOURCE[x509aux]=x509aux.c

  INCLUDE[x509aux]={- rel2abs(catdir($builddir,"../include")) -} ../include

  DEPEND[x509aux]=../libcrypto

ENDIF

test/danetest.c

+2 −3
Original line number Diff line number Diff line
@@ -475,7 +475,7 @@ int main(int argc, char *argv[]) 
    progname = argv[0];

    if (argc != 4) {

        test_usage();

        EXIT(1);

        EXIT(ret);

    }

    basedomain = argv[1];

    CAfile = argv[2];
@@ -492,10 +492,9 @@ int main(int argc, char *argv[]) 
    if (f == NULL) {

        fprintf(stderr, "%s: Error opening tlsa record file: '%s': %s\n",

                progname, tlsafile, strerror(errno));

        return 0;

        EXIT(ret);

    }





    ctx = SSL_CTX_new(TLS_client_method());

    if (SSL_CTX_dane_enable(ctx) <= 0) {

        print_errors();

test/recipes/80-test_x509aux.t

0 → 100644
+27 −0
Original line number Diff line number Diff line 
#! /usr/bin/env perl

# Copyright 2015-2016 The OpenSSL Project Authors. All Rights Reserved.

#

# Licensed under the OpenSSL license (the "License").  You may not use

# this file except in compliance with the License.  You can obtain a copy

# in the file LICENSE in the source distribution or at

# https://www.openssl.org/source/license.html





use strict;

use warnings;

use OpenSSL::Test qw/:DEFAULT srctop_file/;

use OpenSSL::Test::Utils;



setup("test_x509aux");



plan skip_all => "test_dane uses ec which is not supported by this OpenSSL build"

    if disabled("ec");



plan tests => 1;                # The number of tests being performed



ok(run(test(["x509aux", 

                srctop_file("test", "certs", "roots.pem"),

                srctop_file("test", "certs", "root+anyEKU.pem"),

                srctop_file("test", "certs", "root-anyEKU.pem"),

                srctop_file("test", "certs", "root-cert.pem")]

        )), "x509aux tests");