From adc282a5e881293540d02db94c02f6bfb002101a Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 16 Aug 2019 17:14:25 +0100 Subject: [PATCH] Add zero-cost abstraction layer for CRT verification chain When verifying an X.509 certificate, the current verification logic maintains an instance of the internal mbedtls_x509_crt_verify_chain structure representing the state of the verification process. This instance references the list of certificates that comprise the chain built so far together with their verification flags. This information must be stored during verification because it's being passed to the verification callback at the end of verification - if the user has specified those. If the user hasn't specified a verification callback, it is not necessary to maintain the list of CRTs, and it is also not necessary to maintain verification flags for each CRT individually, as they're merged at the end of the verification process. To allow a readable simplification of the code in case no verification callbacks are used, this commit introduces a zero-cost abstraction layer for the functionality that's required from the verification chain structure: - init/reset - add a new CRT to the chain - get pointer to current CRT flags - add flags to EE certificate - get current chain length - trigger callbacks and get final (merged) flags This gives flexibility for re-implementing the verification chain structure, e.g. in the case where no verification callbacks are provided, and there's hence no need to store CRTs and flags individually. This will be done in a later commit. --- include/mbedtls/x509_crt.h | 3 + library/x509_crt.c | 152 ++++++++++++++++++++++--------------- 2 files changed, 92 insertions(+), 63 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index f0801df79..e90f6a09a 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -249,6 +249,9 @@ typedef struct /* for find_parent_in() */ mbedtls_x509_crt *parent; /* non-null iff parent_in in progress */ + /* current child CRT */ + mbedtls_x509_crt *cur_crt; + #if defined(MBEDTLS_HAVE_TIME_DATE) mbedtls_x509_crt *fallback_parent; int fallback_signature_is_good; diff --git a/library/x509_crt.c b/library/x509_crt.c index 730126be8..a04e33ccb 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -669,23 +669,6 @@ static int x509_check_wildcard( char const *cn, } #endif /* !MBEDTLS_X509_REMOVE_HOSTNAME_VERIFICATION */ -/* - * Reset (init or clear) a verify_chain - */ -static void x509_crt_verify_chain_reset( - mbedtls_x509_crt_verify_chain *ver_chain ) -{ - size_t i; - - for( i = 0; i < MBEDTLS_X509_MAX_VERIFY_CHAIN_SIZE; i++ ) - { - ver_chain->items[i].crt = NULL; - ver_chain->items[i].flags = (uint32_t) -1; - } - - ver_chain->len = 0; -} - /* * Version ::= INTEGER { v1(0), v2(1), v3(2) } */ @@ -3202,6 +3185,82 @@ static int x509_crt_check_ee_locally_trusted( return( -1 ); } +/* + * Reset (init or clear) a verify_chain + */ +static void x509_crt_verify_chain_reset( + mbedtls_x509_crt_verify_chain *ver_chain ) +{ + size_t i; + + for( i = 0; i < MBEDTLS_X509_MAX_VERIFY_CHAIN_SIZE; i++ ) + { + ver_chain->items[i].crt = NULL; + ver_chain->items[i].flags = (uint32_t) -1; + } + + ver_chain->len = 0; +} + +/* + * Merge the flags for all certs in the chain, after calling callback + */ +static int x509_crt_verify_chain_get_flags( + const mbedtls_x509_crt_verify_chain *ver_chain, + uint32_t *flags, + int (*f_vrfy)(void *, mbedtls_x509_crt *, int, uint32_t *), + void *p_vrfy ) +{ + int ret; + unsigned i; + uint32_t cur_flags; + const mbedtls_x509_crt_verify_chain_item *cur; + + for( i = ver_chain->len; i != 0; --i ) + { + cur = &ver_chain->items[i-1]; + cur_flags = cur->flags; + + if( NULL != f_vrfy ) + if( ( ret = f_vrfy( p_vrfy, cur->crt, (int) i-1, &cur_flags ) ) != 0 ) + return( ret ); + + *flags |= cur_flags; + } + + return( 0 ); +} + +static void x509_crt_verify_chain_add_ee_flags( + mbedtls_x509_crt_verify_chain *chain, + uint32_t ee_flags ) +{ + chain->items[0].flags |= ee_flags; +} + +static void x509_crt_verify_chain_add_crt( + mbedtls_x509_crt_verify_chain *chain, + mbedtls_x509_crt *crt ) +{ + mbedtls_x509_crt_verify_chain_item *cur; + cur = &chain->items[chain->len]; + cur->crt = crt; + cur->flags = 0; + chain->len++; +} + +static uint32_t* x509_crt_verify_chain_get_cur_flags( + mbedtls_x509_crt_verify_chain *chain ) +{ + return( &chain->items[chain->len - 1].flags ); +} + +static unsigned x509_crt_verify_chain_len( + mbedtls_x509_crt_verify_chain const *chain ) +{ + return( chain->len ); +} + /* * Build and verify a certificate chain * @@ -3254,7 +3313,6 @@ static int x509_crt_verify_chain( * catch potential issues with jumping ahead when restarting */ int ret; uint32_t *flags; - mbedtls_x509_crt_verify_chain_item *cur; mbedtls_x509_crt *child_crt; mbedtls_x509_crt *parent_crt; int parent_is_trusted; @@ -3269,10 +3327,7 @@ static int x509_crt_verify_chain( /* restore saved state */ *ver_chain = rs_ctx->ver_chain; /* struct copy */ self_cnt = rs_ctx->self_cnt; - - /* restore derived state */ - cur = &ver_chain->items[ver_chain->len - 1]; - child_crt = cur->crt; + child_crt = rs_ctx->cur_crt; child_is_trusted = 0; goto find_parent; @@ -3291,16 +3346,13 @@ static int x509_crt_verify_chain( int self_issued; /* Add certificate to the verification chain */ - cur = &ver_chain->items[ver_chain->len]; - cur->crt = child_crt; - cur->flags = 0; - ver_chain->len++; + x509_crt_verify_chain_add_crt( ver_chain, child_crt ); #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE) find_parent: #endif - flags = &cur->flags; + flags = x509_crt_verify_chain_get_cur_flags( ver_chain ); { mbedtls_x509_crt_sig_info child_sig; @@ -3342,7 +3394,7 @@ find_parent: *flags |= MBEDTLS_X509_BADCERT_BAD_PK; /* Special case: EE certs that are locally trusted */ - if( ver_chain->len == 1 && self_issued && + if( x509_crt_verify_chain_len( ver_chain ) == 1 && self_issued && x509_crt_check_ee_locally_trusted( child, trust_ca ) == 0 ) { mbedtls_x509_crt_frame_release( child_crt ); @@ -3364,7 +3416,8 @@ find_parent: ret = x509_crt_find_parent( &child_sig, child_crt->next, trust_ca, &parent_crt, &parent_is_trusted, &signature_is_good, - ver_chain->len - 1, self_cnt, rs_ctx ); + x509_crt_verify_chain_len( ver_chain ) - 1, + self_cnt, rs_ctx ); x509_crt_free_sig_info( &child_sig ); } @@ -3376,6 +3429,7 @@ find_parent: rs_ctx->in_progress = x509_crt_rs_find_parent; rs_ctx->self_cnt = self_cnt; rs_ctx->ver_chain = *ver_chain; /* struct copy */ + rs_ctx->cur_crt = child_crt; return( ret ); } #else @@ -3392,13 +3446,14 @@ find_parent: /* 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 && self_issued ) + if( x509_crt_verify_chain_len( ver_chain ) != 1 && self_issued ) self_cnt++; /* path_cnt is 0 for the first intermediate CA, * and if parent is trusted it's not an intermediate CA */ if( ! parent_is_trusted && - ver_chain->len > MBEDTLS_X509_MAX_INTERMEDIATE_CA ) + x509_crt_verify_chain_len( ver_chain ) > + MBEDTLS_X509_MAX_INTERMEDIATE_CA ) { /* return immediately to avoid overflow the chain array */ return( MBEDTLS_ERR_X509_FATAL_ERROR ); @@ -3552,35 +3607,6 @@ static int x509_crt_verify_name( const mbedtls_x509_crt *crt, } #endif /* !MBEDTLS_X509_REMOVE_HOSTNAME_VERIFICATION */ -/* - * Merge the flags for all certs in the chain, after calling callback - */ -static int x509_crt_merge_flags_with_cb( - uint32_t *flags, - const mbedtls_x509_crt_verify_chain *ver_chain, - int (*f_vrfy)(void *, mbedtls_x509_crt *, int, uint32_t *), - void *p_vrfy ) -{ - int ret; - unsigned i; - uint32_t cur_flags; - const mbedtls_x509_crt_verify_chain_item *cur; - - for( i = ver_chain->len; i != 0; --i ) - { - cur = &ver_chain->items[i-1]; - cur_flags = cur->flags; - - if( NULL != f_vrfy ) - if( ( ret = f_vrfy( p_vrfy, cur->crt, (int) i-1, &cur_flags ) ) != 0 ) - return( ret ); - - *flags |= cur_flags; - } - - return( 0 ); -} - /* * Verify the certificate validity (default profile, not restartable) */ @@ -3714,13 +3740,13 @@ int mbedtls_x509_crt_verify_restartable( mbedtls_x509_crt *crt, goto exit; /* Merge end-entity flags */ - ver_chain.items[0].flags |= ee_flags; + x509_crt_verify_chain_add_ee_flags( &ver_chain, ee_flags ); /* Build final flags, calling callback on the way if any */ #if !defined(MBEDTLS_X509_REMOVE_VERIFY_CALLBACK) - ret = x509_crt_merge_flags_with_cb( flags, &ver_chain, f_vrfy, p_vrfy ); + ret = x509_crt_verify_chain_get_flags( &ver_chain, flags, f_vrfy, p_vrfy ); #else - ret = x509_crt_merge_flags_with_cb( flags, &ver_chain, NULL, NULL ); + ret = x509_crt_verify_chain_get_flags( &ver_chain, flags, NULL, NULL ); #endif /* MBEDTLS_X509_REMOVE_VERIFY_CALLBACK */ exit: