From 1e0677acc10f30a1c96b918082139b019b49f647 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 25 Feb 2019 14:58:22 +0000 Subject: [PATCH] Make use of CRT acquire/release for child in CRT chain verification During CRT verification, `x509_crt_check_signature()` checks whether a candidate parent CRT correctly signs the current child CRT. This commit rewrites this function to use the new acquire/release framework for using CRTs. --- library/x509_crt.c | 113 +++++++++++++++++++++++++++++++-------------- 1 file changed, 78 insertions(+), 35 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 218461d20..29938c232 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -2162,38 +2162,66 @@ static int x509_crt_verifycrl( unsigned char *crt_serial, /* * Check the signature of a certificate by its parent */ -static int x509_crt_check_signature( const mbedtls_x509_crt *child, +static int x509_crt_check_signature( const mbedtls_x509_crt_frame *child, mbedtls_x509_crt *parent, mbedtls_x509_crt_restart_ctx *rs_ctx ) { - const mbedtls_md_info_t *md_info; + int ret; + size_t hash_len; unsigned char hash[MBEDTLS_MD_MAX_SIZE]; - md_info = mbedtls_md_info_from_type( child->sig_md ); - if( mbedtls_md( md_info, child->tbs.p, child->tbs.len, hash ) != 0 ) + mbedtls_md_type_t sig_md; + mbedtls_pk_type_t sig_pk; + void *sig_opts; + + const mbedtls_md_info_t *md_info; + +#if defined(MBEDTLS_X509_RSASSA_PSS_SUPPORT) { - /* Note: this can't happen except after an internal error */ - return( -1 ); + /* Get signature options -- currently only + * necessary for RSASSA-PSS. */ + unsigned char *p = child->sig_alg.p; + unsigned char *end = p + child->sig_alg.len; + ret = mbedtls_x509_get_sig_alg_raw( &p, end, &sig_md, + &sig_pk, &sig_opts ); + if( ret != 0 ) + { + /* Note: this can't happen except after an internal error */ + return( -1 ); + } } +#else /* MBEDTLS_X509_RSASSA_PSS_SUPPORT */ + sig_md = child->sig_md; + sig_pk = child->sig_pk; + sig_opts = NULL; +#endif /* !MBEDTLS_X509_RSASSA_PSS_SUPPORT */ + + md_info = mbedtls_md_info_from_type( sig_md ); + if( mbedtls_md( md_info, child->tbs.p, child->tbs.len, hash ) != 0 ) + return( -1 ); + + hash_len = mbedtls_md_get_size( md_info ); /* Skip expensive computation on obvious mismatch */ - if( ! mbedtls_pk_can_do( &parent->pk, child->sig_pk ) ) + if( ! mbedtls_pk_can_do( &parent->pk, sig_pk ) ) return( -1 ); #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) if( rs_ctx != NULL && child->sig_pk == MBEDTLS_PK_ECDSA ) { return( mbedtls_pk_verify_restartable( &parent->pk, - child->sig_md, hash, mbedtls_md_get_size( md_info ), + sig_md, hash, mbedtls_md_get_size( md_info ), child->sig.p, child->sig.len, &rs_ctx->pk ) ); } #else (void) rs_ctx; #endif - return( mbedtls_pk_verify_ext( child->sig_pk, child->sig_opts, &parent->pk, - child->sig_md, hash, mbedtls_md_get_size( md_info ), - child->sig.p, child->sig.len ) ); + ret = mbedtls_pk_verify_ext( sig_pk, sig_opts, &parent->pk, + sig_md, hash, hash_len, + child->sig.p, child->sig.len ); + mbedtls_free( sig_opts ); + return( ret ); } /* @@ -2202,14 +2230,14 @@ static int x509_crt_check_signature( const mbedtls_x509_crt *child, * * top means parent is a locally-trusted certificate */ -static int x509_crt_check_parent( const mbedtls_x509_crt *child, +static int x509_crt_check_parent( const mbedtls_x509_crt_frame *child, const mbedtls_x509_crt *parent, int top ) { int need_ca_bit; /* Parent must be the issuer */ - if( mbedtls_x509_name_cmp_raw( &child->issuer_raw_no_hdr, + if( mbedtls_x509_name_cmp_raw( &child->issuer_raw, &parent->subject_raw_no_hdr, NULL, NULL ) != 0 ) { @@ -2281,7 +2309,7 @@ static int x509_crt_check_parent( const mbedtls_x509_crt *child, * - MBEDTLS_ERR_ECP_IN_PROGRESS otherwise */ static int x509_crt_find_parent_in( - mbedtls_x509_crt *child, + mbedtls_x509_crt_frame const *child, mbedtls_x509_crt *candidates, mbedtls_x509_crt **r_parent, int *r_signature_is_good, @@ -2406,7 +2434,8 @@ check_signature: * - MBEDTLS_ERR_ECP_IN_PROGRESS otherwise */ static int x509_crt_find_parent( - mbedtls_x509_crt *child, + mbedtls_x509_crt_frame const *child, + mbedtls_x509_crt *rest, mbedtls_x509_crt *trust_ca, mbedtls_x509_crt **parent, int *parent_is_trusted, @@ -2430,7 +2459,7 @@ static int x509_crt_find_parent( #endif while( 1 ) { - search_list = *parent_is_trusted ? trust_ca : child->next; + search_list = *parent_is_trusted ? trust_ca : rest; ret = x509_crt_find_parent_in( child, search_list, parent, signature_is_good, @@ -2473,14 +2502,14 @@ static int x509_crt_find_parent( * check for self-issued as self-signatures are not checked) */ static int x509_crt_check_ee_locally_trusted( - mbedtls_x509_crt *crt, - mbedtls_x509_crt *trust_ca ) + mbedtls_x509_crt_frame const *crt, + mbedtls_x509_crt const *trust_ca ) { - mbedtls_x509_crt *cur; + mbedtls_x509_crt const *cur; /* must be self-issued */ - if( mbedtls_x509_name_cmp_raw( &crt->issuer_raw_no_hdr, - &crt->subject_raw_no_hdr, + if( mbedtls_x509_name_cmp_raw( &crt->issuer_raw, + &crt->subject_raw, NULL, NULL ) != 0 ) { return( -1 ); @@ -2553,7 +2582,7 @@ static int x509_crt_verify_chain( int ret; uint32_t *flags; mbedtls_x509_crt_verify_chain_item *cur; - mbedtls_x509_crt *child; + mbedtls_x509_crt *child_crt; mbedtls_x509_crt *parent; int parent_is_trusted; int child_is_trusted; @@ -2570,20 +2599,24 @@ static int x509_crt_verify_chain( /* restore derived state */ cur = &ver_chain->items[ver_chain->len - 1]; - child = cur->crt; + child_crt = cur->crt; + + child_is_trusted = 0; goto find_parent; } #endif /* MBEDTLS_ECDSA_C && MBEDTLS_ECP_RESTARTABLE */ - child = crt; + child_crt = crt; self_cnt = 0; parent_is_trusted = 0; child_is_trusted = 0; while( 1 ) { + mbedtls_x509_crt_frame *child; + /* Add certificate to the verification chain */ cur = &ver_chain->items[ver_chain->len]; - cur->crt = child; + cur->crt = child_crt; cur->flags = 0; ver_chain->len++; @@ -2591,6 +2624,10 @@ static int x509_crt_verify_chain( find_parent: #endif + ret = x509_crt_frame_acquire( child_crt, &child ); + if( ret != 0 ) + return( MBEDTLS_ERR_X509_FATAL_ERROR ); + flags = &cur->flags; /* Check time-validity (all certificates) */ @@ -2602,7 +2639,7 @@ find_parent: /* Stop here for trusted roots (but not for trusted EE certs) */ if( child_is_trusted ) - return( 0 ); + goto release; /* Check signature algorithm: MD & PK algs */ if( x509_profile_check_md_alg( profile, child->sig_md ) != 0 ) @@ -2615,13 +2652,13 @@ find_parent: if( ver_chain->len == 1 && x509_crt_check_ee_locally_trusted( child, trust_ca ) == 0 ) { - return( 0 ); + goto release; } /* Look for a parent in trusted CAs or up the chain */ ret = x509_crt_find_parent( child, trust_ca, &parent, - &parent_is_trusted, &signature_is_good, - ver_chain->len - 1, self_cnt, rs_ctx ); + &parent_is_trusted, &signature_is_good, + ver_chain->len - 1, self_cnt, rs_ctx ); #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) if( rs_ctx != NULL && ret == MBEDTLS_ERR_ECP_IN_PROGRESS ) @@ -2631,7 +2668,7 @@ find_parent: rs_ctx->self_cnt = self_cnt; rs_ctx->ver_chain = *ver_chain; /* struct copy */ - return( ret ); + goto release; } #else (void) ret; @@ -2641,15 +2678,15 @@ find_parent: if( parent == NULL ) { *flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED; - return( 0 ); + goto release; } /* Count intermediate self-issued (not necessarily self-signed) certs. * These can occur with some strategies for key rollover, see [SIRO], * and should be excluded from max_pathlen checks. */ if( ver_chain->len != 1 && - mbedtls_x509_name_cmp_raw( &child->issuer_raw_no_hdr, - &child->subject_raw_no_hdr, + mbedtls_x509_name_cmp_raw( &child->issuer_raw, + &child->subject_raw, NULL, NULL ) == 0 ) { self_cnt++; @@ -2661,7 +2698,8 @@ find_parent: ver_chain->len > MBEDTLS_X509_MAX_INTERMEDIATE_CA ) { /* return immediately to avoid overflow the chain array */ - return( MBEDTLS_ERR_X509_FATAL_ERROR ); + ret = MBEDTLS_ERR_X509_FATAL_ERROR; + goto release; } /* signature was checked while searching parent */ @@ -2682,11 +2720,16 @@ find_parent: #endif /* prepare for next iteration */ - child = parent; + x509_crt_frame_release( child_crt, child ); + child_crt = parent; parent = NULL; child_is_trusted = parent_is_trusted; signature_is_good = 0; } + +release: + x509_crt_frame_release( child_crt, child ); + return( ret ); } /*