Skip to content

Commit

Permalink
pkcs7: Check that hash algs are in digestAlgorithms
Browse files Browse the repository at this point in the history 
Since only a single hash algorithm is currenlty supported, this avoids
having to perform hashing more than once.

Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
  • Loading branch information
@DemiMarie
DemiMarie committed Dec 14, 2022
1 parent 3df86d3 commit 91d53d0
Showing 1 changed file with 51 additions and 48 deletions.
99 changes: 51 additions & 48 deletions library/pkcs7.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,8 @@ static int pkcs7_get_signature( unsigned char **p, unsigned char *end,
* and unauthenticatedAttributes.
**/
static int pkcs7_get_signer_info( unsigned char **p, unsigned char *end,
mbedtls_pkcs7_signer_info *signer )
mbedtls_pkcs7_signer_info *signer,
mbedtls_x509_buf *alg )
{
unsigned char *end_signer, *end_signer_identifier;
int asn1_ret = 0, ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
Expand Down Expand Up @@ -279,6 +280,15 @@ static int pkcs7_get_signer_info( unsigned char **p, unsigned char *end,
if( ret != 0 )
goto out;

/* Check that the digest algorithm used matches the one provided earlier */
if( signer->alg_identifier.tag != alg->tag ||
signer->alg_identifier.len != alg->len ||
memcmp( signer->alg_identifier.p, alg->p, alg->len ) != 0 )
{
ret = MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO;
goto out;
}

/* Asssume signedAttrs is nonexistent */
ret = pkcs7_get_digest_algorithm( p, end_signer, &signer->sig_alg_identifier );
if( ret != 0 )
Expand Down Expand Up @@ -327,7 +337,8 @@ static void pkcs7_free_signer_info( mbedtls_pkcs7_signer_info *signer )
* Return negative error code for failure.
**/
static int pkcs7_get_signers_info_set( unsigned char **p, unsigned char *end,
mbedtls_pkcs7_signer_info *signers_set )
mbedtls_pkcs7_signer_info *signers_set,
mbedtls_x509_buf *digest_alg )
{
unsigned char *end_set;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
Expand All @@ -345,7 +356,7 @@ static int pkcs7_get_signers_info_set( unsigned char **p, unsigned char *end,

end_set = *p + len;

ret = pkcs7_get_signer_info( p, end_set, signers_set );
ret = pkcs7_get_signer_info( p, end_set, signers_set, digest_alg );
if( ret != 0 )
goto cleanup;
count++;
Expand All @@ -361,7 +372,7 @@ static int pkcs7_get_signers_info_set( unsigned char **p, unsigned char *end,
goto cleanup;
}

ret = pkcs7_get_signer_info( p, end_set, signer );
ret = pkcs7_get_signer_info( p, end_set, signer, digest_alg );
if( ret != 0 ) {
mbedtls_free( signer );
goto cleanup;
Expand Down Expand Up @@ -471,7 +482,7 @@ static int pkcs7_get_signed_data( unsigned char *buf, size_t buflen,
signed_data->no_of_crls = 0;

/* Get signers info */
ret = pkcs7_get_signers_info_set( &p, end, &signed_data->signers );
ret = pkcs7_get_signers_info_set( &p, end, &signed_data->signers, &signed_data->digest_alg_identifiers );
if( ret < 0 )
return( ret );

Expand Down Expand Up @@ -603,6 +614,39 @@ static int mbedtls_pkcs7_data_or_hash_verify( mbedtls_pkcs7 *pkcs7,
return( MBEDTLS_ERR_PKCS7_CERT_DATE_INVALID );
}

ret = mbedtls_oid_get_md_alg( &pkcs7->signed_data.digest_alg_identifiers, &md_alg );
if( ret != 0 )
return( ret );

md_info = mbedtls_md_info_from_type( md_alg );
if( md_info == NULL )
return( MBEDTLS_ERR_PKCS7_VERIFY_FAIL );

hash = mbedtls_calloc( mbedtls_md_get_size( md_info ), 1 );
if( hash == NULL )
return( MBEDTLS_ERR_PKCS7_ALLOC_FAILED );
/* BEGIN must free hash before jumping out */

if( is_data_hash )
{
if( datalen != mbedtls_md_get_size( md_info ))
ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
else
memcpy(hash, data, datalen);
}
else
{
ret = mbedtls_md( md_info, data, datalen, hash );
}
if( ret != 0 )
{
mbedtls_free( hash );
return( MBEDTLS_ERR_PKCS7_VERIFY_FAIL );
}

/* assume failure */
ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;

/*
* Potential TODOs
* Currently we iterate over all signers and return success if any of them
Expand All @@ -612,60 +656,19 @@ static int mbedtls_pkcs7_data_or_hash_verify( mbedtls_pkcs7 *pkcs7,
* identification and SignerIdentifier fields first. That would also allow
* us to distinguish between 'no signature for key' and 'signature for key
* failed to validate'.
*
* We could also cache hashes by md, so if there are several sigs all using
* the same algo we don't recalculate the hash each time.
*/
for( signer = &pkcs7->signed_data.signers; signer; signer = signer->next )
{
ret = mbedtls_oid_get_md_alg( &signer->alg_identifier, &md_alg );
if( ret != 0 )
{
ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
continue;
}

md_info = mbedtls_md_info_from_type( md_alg );
if( md_info == NULL )
{
ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
continue;
}

hash = mbedtls_calloc( mbedtls_md_get_size( md_info ), 1 );
if( hash == NULL )
{
return( MBEDTLS_ERR_PKCS7_ALLOC_FAILED );
}
/* BEGIN must free hash before jumping out */
if( is_data_hash )
{
if( datalen != mbedtls_md_get_size( md_info ))
ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
else
memcpy(hash, data, datalen);
}
else
{
ret = mbedtls_md( md_info, data, datalen, hash );
}
if( ret != 0 )
{
ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
mbedtls_free( hash );
continue;
}

ret = mbedtls_pk_verify( &pk_cxt, md_alg, hash,
mbedtls_md_get_size( md_info ),
signer->sig.p, signer->sig.len );
mbedtls_free( hash );
/* END must free hash before jumping out */

if( ret == 0 )
break;
}

mbedtls_free( hash );
/* END must free hash before jumping out */
return( ret );
}

Expand Down

0 comments on commit 91d53d0

Please sign in to comment.