From 6ad3fd105cfe446b47dc8f7fc96c054c48eec5a6 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sat, 4 May 2019 07:37:58 +0100 Subject: [PATCH 1/5] Adapt x509write_crt.c to coding style Avoid lines longer than 80 characters and fix indentation. --- library/x509write_crt.c | 183 +++++++++++++++++++++++++--------------- 1 file changed, 115 insertions(+), 68 deletions(-) diff --git a/library/x509write_crt.c b/library/x509write_crt.c index b6cb745a3..6002a6605 100644 --- a/library/x509write_crt.c +++ b/library/x509write_crt.c @@ -64,39 +64,44 @@ void mbedtls_x509write_crt_free( mbedtls_x509write_cert *ctx ) mbedtls_platform_zeroize( ctx, sizeof( mbedtls_x509write_cert ) ); } -void mbedtls_x509write_crt_set_version( mbedtls_x509write_cert *ctx, int version ) +void mbedtls_x509write_crt_set_version( mbedtls_x509write_cert *ctx, + int version ) { ctx->version = version; } -void mbedtls_x509write_crt_set_md_alg( mbedtls_x509write_cert *ctx, mbedtls_md_type_t md_alg ) +void mbedtls_x509write_crt_set_md_alg( mbedtls_x509write_cert *ctx, + mbedtls_md_type_t md_alg ) { ctx->md_alg = md_alg; } -void mbedtls_x509write_crt_set_subject_key( mbedtls_x509write_cert *ctx, mbedtls_pk_context *key ) +void mbedtls_x509write_crt_set_subject_key( mbedtls_x509write_cert *ctx, + mbedtls_pk_context *key ) { ctx->subject_key = key; } -void mbedtls_x509write_crt_set_issuer_key( mbedtls_x509write_cert *ctx, mbedtls_pk_context *key ) +void mbedtls_x509write_crt_set_issuer_key( mbedtls_x509write_cert *ctx, + mbedtls_pk_context *key ) { ctx->issuer_key = key; } int mbedtls_x509write_crt_set_subject_name( mbedtls_x509write_cert *ctx, - const char *subject_name ) + const char *subject_name ) { return mbedtls_x509_string_to_names( &ctx->subject, subject_name ); } int mbedtls_x509write_crt_set_issuer_name( mbedtls_x509write_cert *ctx, - const char *issuer_name ) + const char *issuer_name ) { return mbedtls_x509_string_to_names( &ctx->issuer, issuer_name ); } -int mbedtls_x509write_crt_set_serial( mbedtls_x509write_cert *ctx, const mbedtls_mpi *serial ) +int mbedtls_x509write_crt_set_serial( mbedtls_x509write_cert *ctx, + const mbedtls_mpi *serial ) { int ret; @@ -106,8 +111,9 @@ int mbedtls_x509write_crt_set_serial( mbedtls_x509write_cert *ctx, const mbedtls return( 0 ); } -int mbedtls_x509write_crt_set_validity( mbedtls_x509write_cert *ctx, const char *not_before, - const char *not_after ) +int mbedtls_x509write_crt_set_validity( mbedtls_x509write_cert *ctx, + const char *not_before, + const char *not_after ) { if( strlen( not_before ) != MBEDTLS_X509_RFC5280_UTC_TIME_LEN - 1 || strlen( not_after ) != MBEDTLS_X509_RFC5280_UTC_TIME_LEN - 1 ) @@ -127,12 +133,12 @@ int mbedtls_x509write_crt_set_extension( mbedtls_x509write_cert *ctx, int critical, const unsigned char *val, size_t val_len ) { - return mbedtls_x509_set_extension( &ctx->extensions, oid, oid_len, - critical, val, val_len ); + return( mbedtls_x509_set_extension( &ctx->extensions, oid, oid_len, + critical, val, val_len ) ); } int mbedtls_x509write_crt_set_basic_constraints( mbedtls_x509write_cert *ctx, - int is_ca, int max_pathlen ) + int is_ca, int max_pathlen ) { int ret; unsigned char buf[9]; @@ -148,18 +154,21 @@ int mbedtls_x509write_crt_set_basic_constraints( mbedtls_x509write_cert *ctx, { if( max_pathlen >= 0 ) { - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_int( &c, buf, max_pathlen ) ); + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_int( &c, buf, + max_pathlen ) ); } MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_bool( &c, buf, 1 ) ); } MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, buf, len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &c, buf, MBEDTLS_ASN1_CONSTRUCTED | + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &c, buf, + MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ); - return mbedtls_x509write_crt_set_extension( ctx, MBEDTLS_OID_BASIC_CONSTRAINTS, - MBEDTLS_OID_SIZE( MBEDTLS_OID_BASIC_CONSTRAINTS ), - 0, buf + sizeof(buf) - len, len ); + return( + mbedtls_x509write_crt_set_extension( ctx, MBEDTLS_OID_BASIC_CONSTRAINTS, + MBEDTLS_OID_SIZE( MBEDTLS_OID_BASIC_CONSTRAINTS ), + 0, buf + sizeof(buf) - len, len ) ); } #if defined(MBEDTLS_SHA1_C) @@ -171,7 +180,8 @@ int mbedtls_x509write_crt_set_subject_key_identifier( mbedtls_x509write_cert *ct size_t len = 0; memset( buf, 0, sizeof(buf) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_pk_write_pubkey( &c, buf, ctx->subject_key ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_pk_write_pubkey( &c, buf, ctx->subject_key ) ); ret = mbedtls_sha1_ret( buf + sizeof( buf ) - len, len, buf + sizeof( buf ) - 20 ); @@ -181,11 +191,13 @@ int mbedtls_x509write_crt_set_subject_key_identifier( mbedtls_x509write_cert *ct len = 20; MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, buf, len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &c, buf, MBEDTLS_ASN1_OCTET_STRING ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_asn1_write_tag( &c, buf, MBEDTLS_ASN1_OCTET_STRING ) ); - return mbedtls_x509write_crt_set_extension( ctx, MBEDTLS_OID_SUBJECT_KEY_IDENTIFIER, - MBEDTLS_OID_SIZE( MBEDTLS_OID_SUBJECT_KEY_IDENTIFIER ), - 0, buf + sizeof(buf) - len, len ); + return mbedtls_x509write_crt_set_extension( ctx, + MBEDTLS_OID_SUBJECT_KEY_IDENTIFIER, + MBEDTLS_OID_SIZE( MBEDTLS_OID_SUBJECT_KEY_IDENTIFIER ), + 0, buf + sizeof(buf) - len, len ); } int mbedtls_x509write_crt_set_authority_key_identifier( mbedtls_x509write_cert *ctx ) @@ -196,7 +208,8 @@ int mbedtls_x509write_crt_set_authority_key_identifier( mbedtls_x509write_cert * size_t len = 0; memset( buf, 0, sizeof(buf) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_pk_write_pubkey( &c, buf, ctx->issuer_key ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_pk_write_pubkey( &c, buf, ctx->issuer_key ) ); ret = mbedtls_sha1_ret( buf + sizeof( buf ) - len, len, buf + sizeof( buf ) - 20 ); @@ -206,15 +219,19 @@ int mbedtls_x509write_crt_set_authority_key_identifier( mbedtls_x509write_cert * len = 20; MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, buf, len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &c, buf, MBEDTLS_ASN1_CONTEXT_SPECIFIC | 0 ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_asn1_write_tag( &c, buf, MBEDTLS_ASN1_CONTEXT_SPECIFIC | 0 ) ); MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, buf, len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &c, buf, MBEDTLS_ASN1_CONSTRUCTED | - MBEDTLS_ASN1_SEQUENCE ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_asn1_write_tag( &c, buf, + MBEDTLS_ASN1_CONSTRUCTED | + MBEDTLS_ASN1_SEQUENCE ) ); - return mbedtls_x509write_crt_set_extension( ctx, MBEDTLS_OID_AUTHORITY_KEY_IDENTIFIER, - MBEDTLS_OID_SIZE( MBEDTLS_OID_AUTHORITY_KEY_IDENTIFIER ), - 0, buf + sizeof( buf ) - len, len ); + return mbedtls_x509write_crt_set_extension( + ctx, MBEDTLS_OID_AUTHORITY_KEY_IDENTIFIER, + MBEDTLS_OID_SIZE( MBEDTLS_OID_AUTHORITY_KEY_IDENTIFIER ), + 0, buf + sizeof( buf ) - len, len ); } #endif /* MBEDTLS_SHA1_C */ @@ -249,8 +266,8 @@ int mbedtls_x509write_crt_set_key_usage( mbedtls_x509write_cert *ctx, return( MBEDTLS_ERR_X509_INVALID_FORMAT ); ret = mbedtls_x509write_crt_set_extension( ctx, MBEDTLS_OID_KEY_USAGE, - MBEDTLS_OID_SIZE( MBEDTLS_OID_KEY_USAGE ), - 1, c, (size_t)ret ); + MBEDTLS_OID_SIZE( MBEDTLS_OID_KEY_USAGE ), + 1, c, (size_t)ret ); if( ret != 0 ) return( ret ); @@ -271,8 +288,8 @@ int mbedtls_x509write_crt_set_ns_cert_type( mbedtls_x509write_cert *ctx, return( ret ); ret = mbedtls_x509write_crt_set_extension( ctx, MBEDTLS_OID_NS_CERT_TYPE, - MBEDTLS_OID_SIZE( MBEDTLS_OID_NS_CERT_TYPE ), - 0, c, (size_t)ret ); + MBEDTLS_OID_SIZE( MBEDTLS_OID_NS_CERT_TYPE ), + 0, c, (size_t)ret ); if( ret != 0 ) return( ret ); @@ -294,7 +311,8 @@ static int x509_write_time( unsigned char **p, unsigned char *start, (const unsigned char *) t + 2, size - 2 ) ); MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( p, start, len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( p, start, MBEDTLS_ASN1_UTC_TIME ) ); + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( p, start, + MBEDTLS_ASN1_UTC_TIME ) ); } else { @@ -302,15 +320,17 @@ static int x509_write_time( unsigned char **p, unsigned char *start, (const unsigned char *) t, size ) ); MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( p, start, len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( p, start, MBEDTLS_ASN1_GENERALIZED_TIME ) ); + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( p, start, + MBEDTLS_ASN1_GENERALIZED_TIME ) ); } return( (int) len ); } -int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, unsigned char *buf, size_t size, - int (*f_rng)(void *, unsigned char *, size_t), - void *p_rng ) +int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, + unsigned char *buf, size_t size, + int (*f_rng)(void *, unsigned char *, size_t), + void *p_rng ) { int ret; const char *sig_oid; @@ -352,27 +372,36 @@ int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, unsigned char *buf, /* Only for v3 */ if( ctx->version == MBEDTLS_X509_CRT_VERSION_3 ) { - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_x509_write_extensions( &c, tmp_buf, ctx->extensions ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_x509_write_extensions( &c, + tmp_buf, ctx->extensions ) ); MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, tmp_buf, len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &c, tmp_buf, MBEDTLS_ASN1_CONSTRUCTED | - MBEDTLS_ASN1_SEQUENCE ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_asn1_write_tag( &c, tmp_buf, + MBEDTLS_ASN1_CONSTRUCTED | + MBEDTLS_ASN1_SEQUENCE ) ); MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, tmp_buf, len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &c, tmp_buf, MBEDTLS_ASN1_CONTEXT_SPECIFIC | - MBEDTLS_ASN1_CONSTRUCTED | 3 ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_asn1_write_tag( &c, tmp_buf, + MBEDTLS_ASN1_CONTEXT_SPECIFIC | + MBEDTLS_ASN1_CONSTRUCTED | 3 ) ); } /* * SubjectPublicKeyInfo */ - MBEDTLS_ASN1_CHK_ADD( pub_len, mbedtls_pk_write_pubkey_der( ctx->subject_key, - tmp_buf, c - tmp_buf ) ); + MBEDTLS_ASN1_CHK_ADD( pub_len, + mbedtls_pk_write_pubkey_der( ctx->subject_key, + tmp_buf, c - tmp_buf ) ); c -= pub_len; len += pub_len; /* * Subject ::= Name */ - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_x509_write_names( &c, tmp_buf, ctx->subject ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_x509_write_names( &c, tmp_buf, + ctx->subject ) ); /* * Validity ::= SEQUENCE { @@ -381,32 +410,41 @@ int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, unsigned char *buf, */ sub_len = 0; - MBEDTLS_ASN1_CHK_ADD( sub_len, x509_write_time( &c, tmp_buf, ctx->not_after, - MBEDTLS_X509_RFC5280_UTC_TIME_LEN ) ); + MBEDTLS_ASN1_CHK_ADD( sub_len, + x509_write_time( &c, tmp_buf, ctx->not_after, + MBEDTLS_X509_RFC5280_UTC_TIME_LEN ) ); - MBEDTLS_ASN1_CHK_ADD( sub_len, x509_write_time( &c, tmp_buf, ctx->not_before, - MBEDTLS_X509_RFC5280_UTC_TIME_LEN ) ); + MBEDTLS_ASN1_CHK_ADD( sub_len, + x509_write_time( &c, tmp_buf, ctx->not_before, + MBEDTLS_X509_RFC5280_UTC_TIME_LEN ) ); len += sub_len; MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, tmp_buf, sub_len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &c, tmp_buf, MBEDTLS_ASN1_CONSTRUCTED | - MBEDTLS_ASN1_SEQUENCE ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_asn1_write_tag( &c, tmp_buf, + MBEDTLS_ASN1_CONSTRUCTED | + MBEDTLS_ASN1_SEQUENCE ) ); /* * Issuer ::= Name */ - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_x509_write_names( &c, tmp_buf, ctx->issuer ) ); + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_x509_write_names( &c, + tmp_buf, + ctx->issuer ) ); /* * Signature ::= AlgorithmIdentifier */ - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_algorithm_identifier( &c, tmp_buf, - sig_oid, strlen( sig_oid ), 0 ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_asn1_write_algorithm_identifier( &c, tmp_buf, + sig_oid, strlen( sig_oid ), 0 ) ); /* * Serial ::= INTEGER */ - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_mpi( &c, tmp_buf, &ctx->serial ) ); + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_mpi( &c, + tmp_buf, + &ctx->serial ) ); /* * Version ::= INTEGER { v1(0), v2(1), v3(2) } @@ -416,16 +454,22 @@ int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, unsigned char *buf, if( ctx->version != MBEDTLS_X509_CRT_VERSION_1 ) { sub_len = 0; - MBEDTLS_ASN1_CHK_ADD( sub_len, mbedtls_asn1_write_int( &c, tmp_buf, ctx->version ) ); + MBEDTLS_ASN1_CHK_ADD( sub_len, + mbedtls_asn1_write_int( &c, tmp_buf, + ctx->version ) ); len += sub_len; - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, tmp_buf, sub_len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &c, tmp_buf, MBEDTLS_ASN1_CONTEXT_SPECIFIC | - MBEDTLS_ASN1_CONSTRUCTED | 0 ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_asn1_write_len( &c, tmp_buf, sub_len ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_asn1_write_tag( &c, tmp_buf, + MBEDTLS_ASN1_CONTEXT_SPECIFIC | + MBEDTLS_ASN1_CONSTRUCTED | 0 ) ); } MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, tmp_buf, len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &c, tmp_buf, MBEDTLS_ASN1_CONSTRUCTED | - MBEDTLS_ASN1_SEQUENCE ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_asn1_write_tag( &c, tmp_buf, MBEDTLS_ASN1_CONSTRUCTED | + MBEDTLS_ASN1_SEQUENCE ) ); /* * Make signature @@ -436,8 +480,9 @@ int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, unsigned char *buf, return( ret ); } - if( ( ret = mbedtls_pk_sign( ctx->issuer_key, ctx->md_alg, hash, 0, sig, &sig_len, - f_rng, p_rng ) ) != 0 ) + if( ( ret = mbedtls_pk_sign( ctx->issuer_key, ctx->md_alg, + hash, 0, sig, &sig_len, + f_rng, p_rng ) ) != 0 ) { return( ret ); } @@ -457,7 +502,8 @@ int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, unsigned char *buf, len += sig_and_oid_len; MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c2, buf, len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &c2, buf, MBEDTLS_ASN1_CONSTRUCTED | + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &c2, buf, + MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ); return( (int) len ); @@ -467,9 +513,10 @@ int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, unsigned char *buf, #define PEM_END_CRT "-----END CERTIFICATE-----\n" #if defined(MBEDTLS_PEM_WRITE_C) -int mbedtls_x509write_crt_pem( mbedtls_x509write_cert *crt, unsigned char *buf, size_t size, - int (*f_rng)(void *, unsigned char *, size_t), - void *p_rng ) +int mbedtls_x509write_crt_pem( mbedtls_x509write_cert *crt, + unsigned char *buf, size_t size, + int (*f_rng)(void *, unsigned char *, size_t), + void *p_rng ) { int ret; unsigned char output_buf[4096]; From def43051688d9c08f689839233a829553477dbf1 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sat, 4 May 2019 07:54:36 +0100 Subject: [PATCH 2/5] Perform CRT writing in-place on the output buffer The CRT writing routine mbedtls_x509write_crt_der() prepares the TBS (to-be-signed) part of the CRT in a temporary stack-allocated buffer, copying it to the actual output buffer at the end of the routine. This comes at the cost of a very large stack buffer. Moreover, its size must be hardcoded to an upper bound for the lengths of all CRTs to be written through the routine. So far, this upper bound was set to 2Kb, which isn't sufficient some larger certificates, as was reported e.g. in #2631. This commit fixes this by changing mbedtls_x509write_crt_der() to write the certificate in-place in the output buffer, thereby avoiding the use of a statically sized stack buffer for the TBS. Fixes #2631. --- library/x509write_crt.c | 78 +++++++++++++++++++++++------------------ 1 file changed, 43 insertions(+), 35 deletions(-) diff --git a/library/x509write_crt.c b/library/x509write_crt.c index 6002a6605..fd6b52e2c 100644 --- a/library/x509write_crt.c +++ b/library/x509write_crt.c @@ -338,15 +338,14 @@ int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, unsigned char *c, *c2; unsigned char hash[64]; unsigned char sig[MBEDTLS_MPI_MAX_SIZE]; - unsigned char tmp_buf[2048]; size_t sub_len = 0, pub_len = 0, sig_and_oid_len = 0, sig_len; size_t len = 0; mbedtls_pk_type_t pk_alg; /* - * Prepare data to be signed in tmp_buf + * Prepare data to be signed at the end of the target buffer */ - c = tmp_buf + sizeof( tmp_buf ); + c = buf + size; /* Signature algorithm needed in TBS, and later for actual signature */ @@ -374,15 +373,15 @@ int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, { MBEDTLS_ASN1_CHK_ADD( len, mbedtls_x509_write_extensions( &c, - tmp_buf, ctx->extensions ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, tmp_buf, len ) ); + buf, ctx->extensions ) ); + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, buf, len ) ); MBEDTLS_ASN1_CHK_ADD( len, - mbedtls_asn1_write_tag( &c, tmp_buf, + mbedtls_asn1_write_tag( &c, buf, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, tmp_buf, len ) ); + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, buf, len ) ); MBEDTLS_ASN1_CHK_ADD( len, - mbedtls_asn1_write_tag( &c, tmp_buf, + mbedtls_asn1_write_tag( &c, buf, MBEDTLS_ASN1_CONTEXT_SPECIFIC | MBEDTLS_ASN1_CONSTRUCTED | 3 ) ); } @@ -392,7 +391,7 @@ int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, */ MBEDTLS_ASN1_CHK_ADD( pub_len, mbedtls_pk_write_pubkey_der( ctx->subject_key, - tmp_buf, c - tmp_buf ) ); + buf, c - buf ) ); c -= pub_len; len += pub_len; @@ -400,7 +399,7 @@ int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, * Subject ::= Name */ MBEDTLS_ASN1_CHK_ADD( len, - mbedtls_x509_write_names( &c, tmp_buf, + mbedtls_x509_write_names( &c, buf, ctx->subject ) ); /* @@ -411,39 +410,37 @@ int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, sub_len = 0; MBEDTLS_ASN1_CHK_ADD( sub_len, - x509_write_time( &c, tmp_buf, ctx->not_after, + x509_write_time( &c, buf, ctx->not_after, MBEDTLS_X509_RFC5280_UTC_TIME_LEN ) ); MBEDTLS_ASN1_CHK_ADD( sub_len, - x509_write_time( &c, tmp_buf, ctx->not_before, + x509_write_time( &c, buf, ctx->not_before, MBEDTLS_X509_RFC5280_UTC_TIME_LEN ) ); len += sub_len; - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, tmp_buf, sub_len ) ); + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, buf, sub_len ) ); MBEDTLS_ASN1_CHK_ADD( len, - mbedtls_asn1_write_tag( &c, tmp_buf, + mbedtls_asn1_write_tag( &c, buf, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ); /* * Issuer ::= Name */ - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_x509_write_names( &c, - tmp_buf, + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_x509_write_names( &c, buf, ctx->issuer ) ); /* * Signature ::= AlgorithmIdentifier */ MBEDTLS_ASN1_CHK_ADD( len, - mbedtls_asn1_write_algorithm_identifier( &c, tmp_buf, + mbedtls_asn1_write_algorithm_identifier( &c, buf, sig_oid, strlen( sig_oid ), 0 ) ); /* * Serial ::= INTEGER */ - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_mpi( &c, - tmp_buf, + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_mpi( &c, buf, &ctx->serial ) ); /* @@ -455,25 +452,26 @@ int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, { sub_len = 0; MBEDTLS_ASN1_CHK_ADD( sub_len, - mbedtls_asn1_write_int( &c, tmp_buf, - ctx->version ) ); + mbedtls_asn1_write_int( &c, buf, ctx->version ) ); len += sub_len; MBEDTLS_ASN1_CHK_ADD( len, - mbedtls_asn1_write_len( &c, tmp_buf, sub_len ) ); + mbedtls_asn1_write_len( &c, buf, sub_len ) ); MBEDTLS_ASN1_CHK_ADD( len, - mbedtls_asn1_write_tag( &c, tmp_buf, + mbedtls_asn1_write_tag( &c, buf, MBEDTLS_ASN1_CONTEXT_SPECIFIC | MBEDTLS_ASN1_CONSTRUCTED | 0 ) ); } - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, tmp_buf, len ) ); + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, buf, len ) ); MBEDTLS_ASN1_CHK_ADD( len, - mbedtls_asn1_write_tag( &c, tmp_buf, MBEDTLS_ASN1_CONSTRUCTED | + mbedtls_asn1_write_tag( &c, buf, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ); /* * Make signature */ + + /* Compute hash of CRT. */ if( ( ret = mbedtls_md( mbedtls_md_info_from_type( ctx->md_alg ), c, len, hash ) ) != 0 ) { @@ -487,22 +485,32 @@ int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, return( ret ); } - /* - * Write data to output buffer - */ + /* Move CRT to the front of the buffer to have space + * for the signature. */ + memmove( buf, c, len ); + c = buf + len; + + /* Add signature at the end of the buffer, + * making sure that it doesn't underflow + * into the CRT buffer. */ c2 = buf + size; - MBEDTLS_ASN1_CHK_ADD( sig_and_oid_len, mbedtls_x509_write_sig( &c2, buf, + MBEDTLS_ASN1_CHK_ADD( sig_and_oid_len, mbedtls_x509_write_sig( &c2, c, sig_oid, sig_oid_len, sig, sig_len ) ); - if( len > (size_t)( c2 - buf ) ) - return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); + /* + * Memory layout after this step: + * + * buf c=buf+len c2 buf+size + * [CRT0,...,CRTn, UNUSED, ..., UNUSED, SIG0, ..., SIGm] + */ - c2 -= len; - memcpy( c2, c, len ); + /* Move raw CRT to just before the signature. */ + c = c2 - len; + memmove( c, buf, len ); len += sig_and_oid_len; - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c2, buf, len ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &c2, buf, + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( &c, buf, len ) ); + MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( &c, buf, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ); From 4063ad22b39ca44e229f3499776a76cb3d6da2ae Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sat, 4 May 2019 08:12:47 +0100 Subject: [PATCH 3/5] Improve documentation of mbedtls_pem_write_buffer() In particular, mention that it supports overlapping input and output buffers. --- include/mbedtls/pem.h | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/pem.h b/include/mbedtls/pem.h index a29e9ce30..b37f9873a 100644 --- a/include/mbedtls/pem.h +++ b/include/mbedtls/pem.h @@ -112,17 +112,27 @@ void mbedtls_pem_free( mbedtls_pem_context *ctx ); * \brief Write a buffer of PEM information from a DER encoded * buffer. * - * \param header header string to write - * \param footer footer string to write - * \param der_data DER data to write - * \param der_len length of the DER data - * \param buf buffer to write to - * \param buf_len length of output buffer - * \param olen total length written / required (if buf_len is not enough) + * \param header The header string to write. + * \param footer The footer string to write. + * \param der_data The DER data to encode. + * \param der_len The length of the DER data \p der_data in Bytes. + * \param buf The buffer to write to. + * \param buf_len The length of the output buffer \p buf in Bytes. + * \param olen The address at which to store the total length written + * or required (if \p buf_len is not enough). * - * \return 0 on success, or a specific PEM or BASE64 error code. On - * MBEDTLS_ERR_BASE64_BUFFER_TOO_SMALL olen is the required - * size. + * \note You may pass \c NULL for \p buf and \c 0 for \p buf_len + * to request the length of the resulting PEM buffer in + * `*olen`. + * + * \note This function may be called with overlapping \p der_data + * and \p buf buffers. + * + * \return \c 0 on success. + * \return #MBEDTLS_ERR_BASE64_BUFFER_TOO_SMALL if \p buf isn't large + * enough to hold the PEM buffer. In this case, `*olen` holds + * the required minimum size of \p buf. + * \return Another PEM or BASE64 error code on other kinds of failure. */ int mbedtls_pem_write_buffer( const char *header, const char *footer, const unsigned char *der_data, size_t der_len, From 67d42597a96d3c5b537fe6eecd760045bce417f7 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sat, 4 May 2019 08:13:23 +0100 Subject: [PATCH 4/5] Avoid use of large stack buffers in mbedtls_x509_write_crt_pem() This commit rewrites mbedtls_x509write_crt_pem() to not use a statically size stack buffer to temporarily store the DER encoded form of the certificate to be written. This is not necessary because the DER-to-PEM conversion accepts overlapping input and output buffers. --- library/x509write_crt.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/library/x509write_crt.c b/library/x509write_crt.c index fd6b52e2c..3c2321403 100644 --- a/library/x509write_crt.c +++ b/library/x509write_crt.c @@ -527,18 +527,17 @@ int mbedtls_x509write_crt_pem( mbedtls_x509write_cert *crt, void *p_rng ) { int ret; - unsigned char output_buf[4096]; - size_t olen = 0; + size_t olen; - if( ( ret = mbedtls_x509write_crt_der( crt, output_buf, sizeof(output_buf), + if( ( ret = mbedtls_x509write_crt_der( crt, buf, size, f_rng, p_rng ) ) < 0 ) { return( ret ); } if( ( ret = mbedtls_pem_write_buffer( PEM_BEGIN_CRT, PEM_END_CRT, - output_buf + sizeof(output_buf) - ret, - ret, buf, size, &olen ) ) != 0 ) + buf + size - ret, ret, + buf, size, &olen ) ) != 0 ) { return( ret ); } From 73540c07771748fcf552755f55395d7cf7061695 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Sat, 4 May 2019 08:18:09 +0100 Subject: [PATCH 5/5] Adapt ChangeLog --- ChangeLog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ChangeLog b/ChangeLog index 58ff14734..c7154ba3a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -33,6 +33,9 @@ Bugfix for the parameter. * Add a check for MBEDTLS_X509_CRL_PARSE_C in ssl_server2, guarding the crl sni entry parameter. Reported by inestlerode in #560. + * Avoid use of statically sized stack buffers for certificate writing. + This previously limited the maximum size of DER encoded certificates + in mbedtls_x509write_crt_der() to 2Kb. Reported by soccerGB in #2631. Changes * Server's RSA certificate in certs.c was SHA-1 signed. In the default