From beaf396619429db06e593427be829635f4dc4542 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 0fc94fed2..68a9523e0 100644 --- a/library/x509write_crt.c +++ b/library/x509write_crt.c @@ -101,39 +101,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; @@ -143,8 +148,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 ) @@ -164,12 +170,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]; @@ -185,18 +191,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) @@ -208,7 +217,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 ); @@ -218,11 +228,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 ) @@ -233,7 +245,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 ); @@ -243,15 +256,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 */ @@ -298,8 +315,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 ); @@ -325,8 +342,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 ); @@ -348,7 +365,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 { @@ -356,15 +374,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; @@ -406,27 +426,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 { @@ -435,32 +464,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) } @@ -470,16 +508,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 @@ -490,8 +534,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 ); } @@ -511,7 +556,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 ); @@ -521,9 +567,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 eeea9ead3c673065f65eb3c1ae25f68951e0a2c5 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 68a9523e0..feb8c5fcd 100644 --- a/library/x509write_crt.c +++ b/library/x509write_crt.c @@ -392,15 +392,14 @@ int mbedtls_x509write_crt_der( mbedtls_x509write_cert *ctx, unsigned char *c, *c2; unsigned char hash[64]; unsigned char sig[SIGNATURE_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 */ @@ -428,15 +427,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 ) ); } @@ -446,7 +445,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; @@ -454,7 +453,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 ) ); /* @@ -465,39 +464,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 ) ); /* @@ -509,25 +506,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 ) { @@ -541,22 +539,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 cfc77d49bd7d6f5d3d11c8f6c10255e394af7f7a 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 16b610141..1d4474110 100644 --- a/include/mbedtls/pem.h +++ b/include/mbedtls/pem.h @@ -139,17 +139,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 c33e92189a7a0d5a9a73b83895127f6465825391 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 feb8c5fcd..c3899807f 100644 --- a/library/x509write_crt.c +++ b/library/x509write_crt.c @@ -581,18 +581,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 f90597f21e442ffc730f66cf5cdee3a435d4e1ec 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.d/bugfix_PR_2632.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ChangeLog.d/bugfix_PR_2632.txt diff --git a/ChangeLog.d/bugfix_PR_2632.txt b/ChangeLog.d/bugfix_PR_2632.txt new file mode 100644 index 000000000..1d54b4dd2 --- /dev/null +++ b/ChangeLog.d/bugfix_PR_2632.txt @@ -0,0 +1,4 @@ +Bugfix + * 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.