Simplify control flow in PKCS7 functions

Remove useless goto in several functions.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This commit is contained in:
Gilles Peskine 2022-11-27 21:46:56 +01:00
parent 391005cb3b
commit 47a732635b

View File

@ -103,15 +103,13 @@ static int pkcs7_get_content_info_type( unsigned char **p, unsigned char *end,
| MBEDTLS_ASN1_SEQUENCE ); | MBEDTLS_ASN1_SEQUENCE );
if( ret != 0 ) { if( ret != 0 ) {
*p = start; *p = start;
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO, ret ); return( MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO, ret ) );
goto out;
} }
ret = mbedtls_asn1_get_tag( p, end, &len, MBEDTLS_ASN1_OID ); ret = mbedtls_asn1_get_tag( p, end, &len, MBEDTLS_ASN1_OID );
if( ret != 0 ) { if( ret != 0 ) {
*p = start; *p = start;
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO, ret ); return( MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO, ret ) );
goto out;
} }
pkcs7->tag = MBEDTLS_ASN1_OID; pkcs7->tag = MBEDTLS_ASN1_OID;
@ -119,7 +117,6 @@ static int pkcs7_get_content_info_type( unsigned char **p, unsigned char *end,
pkcs7->p = *p; pkcs7->p = *p;
*p += len; *p += len;
out:
return( ret ); return( ret );
} }
@ -153,8 +150,7 @@ static int pkcs7_get_digest_algorithm_set( unsigned char **p,
| MBEDTLS_ASN1_SET ); | MBEDTLS_ASN1_SET );
if( ret != 0 ) if( ret != 0 )
{ {
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_ALG, ret ); return( MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_ALG, ret ) );
goto out;
} }
end = *p + len; end = *p + len;
@ -162,16 +158,14 @@ static int pkcs7_get_digest_algorithm_set( unsigned char **p,
ret = mbedtls_asn1_get_alg_null( p, end, alg ); ret = mbedtls_asn1_get_alg_null( p, end, alg );
if( ret != 0 ) if( ret != 0 )
{ {
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_ALG, ret ); return( MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_ALG, ret ) );
goto out;
} }
/** For now, it assumes there is only one digest algorithm specified **/ /** For now, it assumes there is only one digest algorithm specified **/
if ( *p != end ) if ( *p != end )
ret = MBEDTLS_ERR_PKCS7_FEATURE_UNAVAILABLE; return( MBEDTLS_ERR_PKCS7_FEATURE_UNAVAILABLE );
out: return( 0 );
return( ret );
} }
/** /**
@ -195,10 +189,9 @@ static int pkcs7_get_certificates( unsigned char **p, unsigned char *end,
| MBEDTLS_ASN1_CONTEXT_SPECIFIC ) ) != 0 ) | MBEDTLS_ASN1_CONTEXT_SPECIFIC ) ) != 0 )
{ {
if( ret == MBEDTLS_ERR_ASN1_UNEXPECTED_TAG ) if( ret == MBEDTLS_ERR_ASN1_UNEXPECTED_TAG )
ret = 0; return( 0 );
else else
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_FORMAT, ret ); return( MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_FORMAT, ret ) );
goto out;
} }
start = *p; start = *p;
end_set = *p + len1; end_set = *p + len1;
@ -207,8 +200,7 @@ static int pkcs7_get_certificates( unsigned char **p, unsigned char *end,
| MBEDTLS_ASN1_SEQUENCE ); | MBEDTLS_ASN1_SEQUENCE );
if( ret != 0 ) if( ret != 0 )
{ {
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_CERT, ret ); return( MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_CERT, ret ) );
goto out;
} }
end_cert = *p + len2; end_cert = *p + len2;
@ -221,15 +213,13 @@ static int pkcs7_get_certificates( unsigned char **p, unsigned char *end,
*/ */
if ( end_cert != end_set ) if ( end_cert != end_set )
{ {
ret = MBEDTLS_ERR_PKCS7_FEATURE_UNAVAILABLE; return( MBEDTLS_ERR_PKCS7_FEATURE_UNAVAILABLE );
goto out;
} }
*p = start; *p = start;
if( ( ret = mbedtls_x509_crt_parse_der( certs, *p, len1 ) ) < 0 ) if( ( ret = mbedtls_x509_crt_parse_der( certs, *p, len1 ) ) < 0 )
{ {
ret = MBEDTLS_ERR_PKCS7_INVALID_CERT; return( MBEDTLS_ERR_PKCS7_INVALID_CERT );
goto out;
} }
*p = *p + len1; *p = *p + len1;
@ -238,10 +228,7 @@ static int pkcs7_get_certificates( unsigned char **p, unsigned char *end,
* Since in this version we strictly support single certificate, and reaching * Since in this version we strictly support single certificate, and reaching
* here implies we have parsed successfully, we return 1. * here implies we have parsed successfully, we return 1.
*/ */
ret = 1; return( 1 );
out:
return( ret );
} }
/** /**
@ -255,7 +242,7 @@ static int pkcs7_get_signature( unsigned char **p, unsigned char *end,
ret = mbedtls_asn1_get_tag( p, end, &len, MBEDTLS_ASN1_OCTET_STRING ); ret = mbedtls_asn1_get_tag( p, end, &len, MBEDTLS_ASN1_OCTET_STRING );
if( ret != 0 ) if( ret != 0 )
goto out; return( ret );
signature->tag = MBEDTLS_ASN1_OCTET_STRING; signature->tag = MBEDTLS_ASN1_OCTET_STRING;
signature->len = len; signature->len = len;
@ -263,8 +250,7 @@ static int pkcs7_get_signature( unsigned char **p, unsigned char *end,
*p = *p + len; *p = *p + len;
out: return( 0 );
return( ret );
} }
/** /**
@ -382,34 +368,32 @@ static int pkcs7_get_signers_info_set( unsigned char **p, unsigned char *end,
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
int count = 0; int count = 0;
size_t len = 0; size_t len = 0;
mbedtls_pkcs7_signer_info *signer, *prev;
ret = mbedtls_asn1_get_tag( p, end, &len, MBEDTLS_ASN1_CONSTRUCTED ret = mbedtls_asn1_get_tag( p, end, &len, MBEDTLS_ASN1_CONSTRUCTED
| MBEDTLS_ASN1_SET ); | MBEDTLS_ASN1_SET );
if( ret != 0 ) if( ret != 0 )
{ {
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO, ret ); return( MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO, ret ) );
goto out;
} }
/* Detect zero signers */ /* Detect zero signers */
if( len == 0 ) if( len == 0 )
{ {
ret = 0; return( 0 );
goto out;
} }
end_set = *p + len; end_set = *p + len;
ret = pkcs7_get_signer_info( p, end_set, signers_set ); ret = pkcs7_get_signer_info( p, end_set, signers_set );
if( ret != 0 ) if( ret != 0 )
goto out; return( ret );
count++; count++;
prev = signers_set; mbedtls_pkcs7_signer_info *prev = signers_set;
while( *p != end_set ) while( *p != end_set )
{ {
signer = mbedtls_calloc( 1, sizeof( mbedtls_pkcs7_signer_info ) ); mbedtls_pkcs7_signer_info *signer =
mbedtls_calloc( 1, sizeof( mbedtls_pkcs7_signer_info ) );
if( !signer ) if( !signer )
{ {
ret = MBEDTLS_ERR_PKCS7_ALLOC_FAILED; ret = MBEDTLS_ERR_PKCS7_ALLOC_FAILED;
@ -426,12 +410,11 @@ static int pkcs7_get_signers_info_set( unsigned char **p, unsigned char *end,
count++; count++;
} }
ret = count; return( count );
goto out;
cleanup: cleanup:
pkcs7_free_signer_info( signers_set ); pkcs7_free_signer_info( signers_set );
signer = signers_set->next; mbedtls_pkcs7_signer_info *signer = signers_set->next;
while( signer != NULL ) while( signer != NULL )
{ {
prev = signer; prev = signer;
@ -440,8 +423,6 @@ cleanup:
mbedtls_free( prev ); mbedtls_free( prev );
} }
signers_set->next = NULL; signers_set->next = NULL;
out:
return( ret ); return( ret );
} }
@ -471,8 +452,7 @@ static int pkcs7_get_signed_data( unsigned char *buf, size_t buflen,
| MBEDTLS_ASN1_SEQUENCE ); | MBEDTLS_ASN1_SEQUENCE );
if( ret != 0 ) if( ret != 0 )
{ {
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_FORMAT, ret ); return( MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_FORMAT, ret ) );
goto out;
} }
end_set = p + len; end_set = p + len;
@ -480,37 +460,35 @@ static int pkcs7_get_signed_data( unsigned char *buf, size_t buflen,
/* Get version of signed data */ /* Get version of signed data */
ret = pkcs7_get_version( &p, end_set, &signed_data->version ); ret = pkcs7_get_version( &p, end_set, &signed_data->version );
if( ret != 0 ) if( ret != 0 )
goto out; return( ret );
/* Get digest algorithm */ /* Get digest algorithm */
ret = pkcs7_get_digest_algorithm_set( &p, end_set, ret = pkcs7_get_digest_algorithm_set( &p, end_set,
&signed_data->digest_alg_identifiers ); &signed_data->digest_alg_identifiers );
if( ret != 0 ) if( ret != 0 )
goto out; return( ret );
ret = mbedtls_oid_get_md_alg( &signed_data->digest_alg_identifiers, &md_alg ); ret = mbedtls_oid_get_md_alg( &signed_data->digest_alg_identifiers, &md_alg );
if( ret != 0 ) if( ret != 0 )
{ {
ret = MBEDTLS_ERR_PKCS7_INVALID_ALG; return( MBEDTLS_ERR_PKCS7_INVALID_ALG );
goto out;
} }
/* Do not expect any content */ /* Do not expect any content */
ret = pkcs7_get_content_info_type( &p, end_set, &signed_data->content.oid ); ret = pkcs7_get_content_info_type( &p, end_set, &signed_data->content.oid );
if( ret != 0 ) if( ret != 0 )
goto out; return( ret );
if( MBEDTLS_OID_CMP( MBEDTLS_OID_PKCS7_DATA, &signed_data->content.oid ) ) if( MBEDTLS_OID_CMP( MBEDTLS_OID_PKCS7_DATA, &signed_data->content.oid ) )
{ {
ret = MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO; return( MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO );
goto out;
} }
/* Look for certificates, there may or may not be any */ /* Look for certificates, there may or may not be any */
mbedtls_x509_crt_init( &signed_data->certs ); mbedtls_x509_crt_init( &signed_data->certs );
ret = pkcs7_get_certificates( &p, end_set, &signed_data->certs ); ret = pkcs7_get_certificates( &p, end_set, &signed_data->certs );
if( ret < 0 ) if( ret < 0 )
goto out; return( ret );
signed_data->no_of_certs = ret; signed_data->no_of_certs = ret;
@ -525,18 +503,15 @@ static int pkcs7_get_signed_data( unsigned char *buf, size_t buflen,
/* Get signers info */ /* Get signers info */
ret = pkcs7_get_signers_info_set( &p, end_set, &signed_data->signers ); ret = pkcs7_get_signers_info_set( &p, end_set, &signed_data->signers );
if( ret < 0 ) if( ret < 0 )
goto out; return( ret );
signed_data->no_of_signers = ret; signed_data->no_of_signers = ret;
/* Don't permit trailing data */ /* Don't permit trailing data */
if ( p != end ) if ( p != end )
ret = MBEDTLS_ERR_PKCS7_INVALID_FORMAT; return( MBEDTLS_ERR_PKCS7_INVALID_FORMAT );
else
ret = 0;
out: return( 0 );
return( ret );
} }
int mbedtls_pkcs7_parse_der( mbedtls_pkcs7 *pkcs7, const unsigned char *buf, int mbedtls_pkcs7_parse_der( mbedtls_pkcs7 *pkcs7, const unsigned char *buf,
@ -548,10 +523,9 @@ int mbedtls_pkcs7_parse_der( mbedtls_pkcs7 *pkcs7, const unsigned char *buf,
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
int isoidset = 0; int isoidset = 0;
if( !pkcs7 ) if( pkcs7 == NULL )
{ {
ret = MBEDTLS_ERR_PKCS7_BAD_INPUT_DATA; return( MBEDTLS_ERR_PKCS7_BAD_INPUT_DATA );
goto out;
} }
/* make an internal copy of the buffer for parsing */ /* make an internal copy of the buffer for parsing */
@ -631,15 +605,13 @@ static int mbedtls_pkcs7_data_or_hash_verify( mbedtls_pkcs7 *pkcs7,
if( pkcs7->signed_data.no_of_signers == 0 ) if( pkcs7->signed_data.no_of_signers == 0 )
{ {
ret = MBEDTLS_ERR_PKCS7_INVALID_CERT; return( MBEDTLS_ERR_PKCS7_INVALID_CERT );
goto out;
} }
if( mbedtls_x509_time_is_past( &cert->valid_to ) || if( mbedtls_x509_time_is_past( &cert->valid_to ) ||
mbedtls_x509_time_is_future( &cert->valid_from )) mbedtls_x509_time_is_future( &cert->valid_from ))
{ {
ret = MBEDTLS_ERR_PKCS7_CERT_DATE_INVALID; return( MBEDTLS_ERR_PKCS7_CERT_DATE_INVALID );
goto out;
} }
/* /*
@ -673,9 +645,9 @@ static int mbedtls_pkcs7_data_or_hash_verify( mbedtls_pkcs7 *pkcs7,
hash = mbedtls_calloc( mbedtls_md_get_size( md_info ), 1 ); hash = mbedtls_calloc( mbedtls_md_get_size( md_info ), 1 );
if( hash == NULL ) { if( hash == NULL ) {
ret = MBEDTLS_ERR_PKCS7_ALLOC_FAILED; return( MBEDTLS_ERR_PKCS7_ALLOC_FAILED );
goto out;
} }
/* BEGIN must free hash before jumping out */
if( is_data_hash ) if( is_data_hash )
{ {
if( datalen != mbedtls_md_get_size( md_info )) if( datalen != mbedtls_md_get_size( md_info ))
@ -698,12 +670,12 @@ static int mbedtls_pkcs7_data_or_hash_verify( mbedtls_pkcs7 *pkcs7,
mbedtls_md_get_size( md_info ), mbedtls_md_get_size( md_info ),
signer->sig.p, signer->sig.len ); signer->sig.p, signer->sig.len );
mbedtls_free( hash ); mbedtls_free( hash );
/* END must free hash before jumping out */
if( ret == 0 ) if( ret == 0 )
break; break;
} }
out:
return( ret ); return( ret );
} }
int mbedtls_pkcs7_signed_data_verify( mbedtls_pkcs7 *pkcs7, int mbedtls_pkcs7_signed_data_verify( mbedtls_pkcs7 *pkcs7,