diff --git a/ChangeLog.d/x509write_csr_heap_alloc.txt b/ChangeLog.d/x509write_csr_heap_alloc.txt new file mode 100644 index 000000000..abce20c4d --- /dev/null +++ b/ChangeLog.d/x509write_csr_heap_alloc.txt @@ -0,0 +1,4 @@ +Changes + * Reduce the stack consumption of mbedtls_x509write_csr_der() which + previously could lead to stack overflow on constrained devices. + Contributed by Doru Gucea and Simon Leet in #3464. diff --git a/library/x509write_csr.c b/library/x509write_csr.c index df2ebb26a..4d51bbd03 100644 --- a/library/x509write_csr.c +++ b/library/x509write_csr.c @@ -140,7 +140,8 @@ int mbedtls_x509write_csr_set_ns_cert_type( mbedtls_x509write_csr *ctx, static int x509write_csr_der_internal( mbedtls_x509write_csr *ctx, unsigned char *buf, - size_t size, unsigned char *sig, + size_t size, + unsigned char *sig, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { @@ -157,12 +158,8 @@ static int x509write_csr_der_internal( mbedtls_x509write_csr *ctx, size_t hash_len; psa_algorithm_t hash_alg = mbedtls_psa_translate_md( ctx->md_alg ); #endif /* MBEDTLS_USE_PSA_CRYPTO */ - /* - * Writing strategy: - * 1. start writing from the back of buf - * 2. sign the written data and place the signature at the start of buf - * 3. compact memory locations by moving the signature towards right - */ + + /* Write the CSR backwards starting from the end of buf */ c = buf + size; MBEDTLS_ASN1_CHK_ADD( len, mbedtls_x509_write_extensions( &c, buf, @@ -171,25 +168,34 @@ static int x509write_csr_der_internal( mbedtls_x509write_csr *ctx, if( len ) { 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 ) ); 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_SET ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_asn1_write_tag( + &c, buf, + MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SET ) ); - MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_oid( &c, buf, - MBEDTLS_OID_PKCS9_CSR_EXT_REQ, - MBEDTLS_OID_SIZE( MBEDTLS_OID_PKCS9_CSR_EXT_REQ ) ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_asn1_write_oid( + &c, buf, MBEDTLS_OID_PKCS9_CSR_EXT_REQ, + MBEDTLS_OID_SIZE( MBEDTLS_OID_PKCS9_CSR_EXT_REQ ) ) ); 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 ) ); } 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_CONTEXT_SPECIFIC ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_asn1_write_tag( + &c, buf, + MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_CONTEXT_SPECIFIC ) ); MBEDTLS_ASN1_CHK_ADD( pub_len, mbedtls_pk_write_pubkey_der( ctx->key, buf, c - buf ) ); @@ -208,11 +214,13 @@ static int x509write_csr_der_internal( mbedtls_x509write_csr *ctx, MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_int( &c, buf, 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 ) ); /* - * Prepare signature + * Sign the written CSR data into the sig buffer * Note: hash errors can happen only after an internal error */ #if defined(MBEDTLS_USE_PSA_CRYPTO) @@ -251,22 +259,38 @@ static int x509write_csr_der_internal( mbedtls_x509write_csr *ctx, return( ret ); } - /* reserve space for the signature at the end of buf */ + /* + * Move the written CSR data to the start of buf to create space for + * writing the signature into buf. + */ memmove( buf, c, len ); - /* copy the signature */ + /* + * Write sig and its OID into buf backwards from the end of buf. + * Note: mbedtls_x509_write_sig will check for c2 - ( buf + len ) < sig_len + * and return MBEDTLS_ERR_ASN1_BUF_TOO_SMALL if needed. + */ c2 = buf + size; - MBEDTLS_ASN1_CHK_ADD( sig_and_oid_len, mbedtls_x509_write_sig( &c2, - buf + len, sig_oid, sig_oid_len, sig, sig_len ) ); + MBEDTLS_ASN1_CHK_ADD( sig_and_oid_len, + mbedtls_x509_write_sig( &c2, buf + len, sig_oid, sig_oid_len, + sig, sig_len ) ); - /* compact oids and signature memory locations */ + /* + * Compact the space between the CSR data and signature by moving the + * CSR data to the start of the signature. + */ c2 -= len; memmove( c2, buf, len ); + /* ASN encode the total size and tag the CSR data with it. */ 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_SEQUENCE ) ); + MBEDTLS_ASN1_CHK_ADD( len, + mbedtls_asn1_write_tag( + &c2, buf, + MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) ); + + /* Zero the unused bytes at the start of buf */ memset( buf, 0, c2 - buf); return( (int) len ); @@ -280,9 +304,9 @@ int mbedtls_x509write_csr_der( mbedtls_x509write_csr *ctx, unsigned char *buf, int ret; unsigned char *sig; - if( ( sig = mbedtls_calloc( 1, MBEDTLS_MPI_MAX_SIZE ) ) == NULL ) + if( ( sig = mbedtls_calloc( 1, MBEDTLS_PK_SIGNATURE_MAX_SIZE ) ) == NULL ) { - return( MBEDTLS_ERR_ASN1_ALLOC_FAILED ); + return( MBEDTLS_ERR_X509_ALLOC_FAILED ); } ret = x509write_csr_der_internal( ctx, buf, size, sig, f_rng, p_rng );