yuzu-emu/mbedtls
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

pkcs7: Respond to feeback on parsing logic

Browse source 
After recieving review on the pkcs7 parsing functions, attempt
to use better API's, increase consisitency and use better
documentation. The changes are in response to the following
comments:
 - use mbedtls_x509_crt_parse_der instead of mbedtls_x509_crt_parse [1]
 - make lack of support for authenticatedAttributes more clear [2]
 - increment pointer in pkcs7_get_content_info_type rather than after [3]
 - rename `start` to `p` for consistency in mbedtls_pkcs7_parse_der [4]

[1] https://github.com/Mbed-TLS/mbedtls/pull/3431#discussion_r992509630
[2] https://github.com/Mbed-TLS/mbedtls/pull/3431#discussion_r992562450
[3] https://github.com/Mbed-TLS/mbedtls/pull/3431#discussion_r992741877
[4] https://github.com/Mbed-TLS/mbedtls/pull/3431#discussion_r992754103

Signed-off-by: Nick Child <nick.child@ibm.com>
This commit is contained in:
Nick Child 2022-10-28 12:28:54 -05:00
parent 73621ef0f0
commit bb82ab764f
2 changed files with 17 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
include/mbedtls/pkcs7.h
View file

@ -38,6 +38,9 @@
* - The RFC specifies the Signed Data type can contain
* certificate-revocation lists (crls). This implementation has no support
* for crls so it is assumed to be an empty list.
* - The RFC allows for SignerInfo structure to optionally contain
* unauthenticatedAttributes and authenticatedAttributes. In Mbed TLS it is
* assumed these fields are empty.
*/
#ifndef MBEDTLS_PKCS7_H

27
library/pkcs7.c
View file

@ -126,6 +126,7 @@ static int pkcs7_get_content_info_type( unsigned char **p, unsigned char *end,
pkcs7->tag = MBEDTLS_ASN1_OID;
pkcs7->len = len;
pkcs7->p = *p;
*p += len;
out:
return( ret );
@ -197,8 +198,7 @@ static int pkcs7_get_certificates( unsigned char **p, unsigned char *end,
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
size_t len1 = 0;
size_t len2 = 0;
unsigned char *end_set, *end_cert;
unsigned char *start = *p;
unsigned char *end_set, *end_cert, *start;
if( ( ret = mbedtls_asn1_get_tag( p, end, &len1, MBEDTLS_ASN1_CONSTRUCTED
| MBEDTLS_ASN1_CONTEXT_SPECIFIC ) ) != 0 )
@ -235,7 +235,7 @@ static int pkcs7_get_certificates( unsigned char **p, unsigned char *end,
}
*p = start;
if( ( ret = mbedtls_x509_crt_parse( certs, *p, len1 ) ) < 0 )
if( ( ret = mbedtls_x509_crt_parse_der( certs, *p, len1 ) ) < 0 )
{
ret = MBEDTLS_ERR_PKCS7_INVALID_CERT;
goto out;
@ -289,6 +289,8 @@ out:
* [1] IMPLICIT Attributes OPTIONAL,
* Returns 0 if the signerInfo is valid.
* Return negative error code for failure.
* Structure must not contain vales for authenticatedAttributes
* and unauthenticatedAttributes.
**/
static int pkcs7_get_signer_info( unsigned char **p, unsigned char *end,
mbedtls_pkcs7_signer_info *signer )
@ -335,6 +337,8 @@ static int pkcs7_get_signer_info( unsigned char **p, unsigned char *end,
if( ret != 0 )
goto out;
/* Asssume authenticatedAttributes is nonexistent */
ret = pkcs7_get_digest_algorithm( p, end_signer, &signer->sig_alg_identifier );
if( ret != 0 )
goto out;
@ -510,8 +514,6 @@ static int pkcs7_get_signed_data( unsigned char *buf, size_t buflen,
goto out;
}
p = p + signed_data->content.oid.len;
/* Look for certificates, there may or may not be any */
mbedtls_x509_crt_init( &signed_data->certs );
ret = pkcs7_get_certificates( &p, end_set, &signed_data->certs );
@ -548,7 +550,7 @@ out:
int mbedtls_pkcs7_parse_der( mbedtls_pkcs7 *pkcs7, const unsigned char *buf,
const size_t buflen )
{
unsigned char *start;
unsigned char *p;
unsigned char *end;
size_t len = 0;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
@ -561,17 +563,17 @@ int mbedtls_pkcs7_parse_der( mbedtls_pkcs7 *pkcs7, const unsigned char *buf,
}
/* make an internal copy of the buffer for parsing */
pkcs7->raw.p = start = mbedtls_calloc( 1, buflen );
pkcs7->raw.p = p = mbedtls_calloc( 1, buflen );
if( pkcs7->raw.p == NULL )
{
ret = MBEDTLS_ERR_PKCS7_ALLOC_FAILED;
goto out;
}
memcpy( start, buf, buflen );
memcpy( p, buf, buflen );
pkcs7->raw.len = buflen;
end = start + buflen;
end = p + buflen;
ret = pkcs7_get_content_info_type( &start, end, &pkcs7->content_type_oid );
ret = pkcs7_get_content_info_type( &p, end, &pkcs7->content_type_oid );
if( ret != 0 )
{
len = buflen;
@ -596,14 +598,13 @@ int mbedtls_pkcs7_parse_der( mbedtls_pkcs7 *pkcs7, const unsigned char *buf,
}
isoidset = 1;
start = start + pkcs7->content_type_oid.len;
ret = pkcs7_get_next_content_len( &start, end, &len );
ret = pkcs7_get_next_content_len( &p, end, &len );
if( ret != 0 )
goto out;
try_data:
ret = pkcs7_get_signed_data( start, len, &pkcs7->signed_data );
ret = pkcs7_get_signed_data( p, len, &pkcs7->signed_data );
if ( ret != 0 )
goto out;