pkcs7: Unite function return style

In response to feedback[1], standardize return variable
management across all pkcs7 functions.

Additionally, when adding return codes from two error values,
use `MBEDTLS_ERROR_ADD` as recommended [2].

[1] https://github.com/Mbed-TLS/mbedtls/pull/3431#discussion_r953634781
[2] https://github.com/Mbed-TLS/mbedtls/pull/3431#discussion_r953635128

Signed-off-by: Nick Child <nick.child@ibm.com>
This commit is contained in:
Nick Child 2022-09-12 16:21:02 -05:00
parent 62b2d7e7d4
commit 9f4fb3e63f

View File

@ -24,6 +24,7 @@
#include "mbedtls/x509_crt.h"
#include "mbedtls/x509_crl.h"
#include "mbedtls/oid.h"
#include "mbedtls/error.h"
#include <stdlib.h>
#include <stdio.h>
@ -64,15 +65,16 @@ void mbedtls_pkcs7_init( mbedtls_pkcs7 *pkcs7 )
static int pkcs7_get_next_content_len( unsigned char **p, unsigned char *end,
size_t *len )
{
int ret;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
if( ( ret = mbedtls_asn1_get_tag( p, end, len, MBEDTLS_ASN1_CONSTRUCTED
| MBEDTLS_ASN1_CONTEXT_SPECIFIC ) ) != 0 )
ret = mbedtls_asn1_get_tag( p, end, len, MBEDTLS_ASN1_CONSTRUCTED
| MBEDTLS_ASN1_CONTEXT_SPECIFIC );
if( ret != 0 )
{
return( MBEDTLS_ERR_PKCS7_INVALID_FORMAT + ret );
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_FORMAT, ret );
}
return( 0 );
return( ret );
}
/**
@ -81,16 +83,17 @@ static int pkcs7_get_next_content_len( unsigned char **p, unsigned char *end,
**/
static int pkcs7_get_version( unsigned char **p, unsigned char *end, int *ver )
{
int ret;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
if( ( ret = mbedtls_asn1_get_int( p, end, ver ) ) != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_VERSION + ret );
ret = mbedtls_asn1_get_int( p, end, ver );
if( ret != 0 )
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_VERSION, ret );
/* If version != 1, return invalid version */
if( *ver != MBEDTLS_PKCS7_SUPPORTED_VERSION )
return( MBEDTLS_ERR_PKCS7_INVALID_VERSION );
ret = MBEDTLS_ERR_PKCS7_INVALID_VERSION;
return( 0 );
return( ret );
}
/**
@ -103,26 +106,29 @@ static int pkcs7_get_content_info_type( unsigned char **p, unsigned char *end,
mbedtls_pkcs7_buf *pkcs7 )
{
size_t len = 0;
int ret;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
unsigned char *start = *p;
ret = mbedtls_asn1_get_tag( p, end, &len, MBEDTLS_ASN1_CONSTRUCTED
| MBEDTLS_ASN1_SEQUENCE );
if( ret != 0 ) {
*p = start;
return( MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO + ret );
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO, ret );
goto out;
}
ret = mbedtls_asn1_get_tag( p, end, &len, MBEDTLS_ASN1_OID );
if( ret != 0 ) {
*p = start;
return( MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO + ret );
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO, ret );
goto out;
}
pkcs7->tag = MBEDTLS_ASN1_OID;
pkcs7->len = len;
pkcs7->p = *p;
out:
return( ret );
}
@ -134,12 +140,12 @@ static int pkcs7_get_content_info_type( unsigned char **p, unsigned char *end,
static int pkcs7_get_digest_algorithm( unsigned char **p, unsigned char *end,
mbedtls_x509_buf *alg )
{
int ret;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
if( ( ret = mbedtls_asn1_get_alg_null( p, end, alg ) ) != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_ALG );
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_ALG, ret );
return( 0 );
return( ret );
}
/**
@ -150,24 +156,31 @@ static int pkcs7_get_digest_algorithm_set( unsigned char **p,
mbedtls_x509_buf *alg )
{
size_t len = 0;
int ret;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
ret = mbedtls_asn1_get_tag( p, end, &len, MBEDTLS_ASN1_CONSTRUCTED
| MBEDTLS_ASN1_SET );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_ALG + ret );
{
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_ALG, ret );
goto out;
}
end = *p + len;
/** For now, it assumes there is only one digest algorithm specified **/
ret = mbedtls_asn1_get_alg_null( p, end, alg );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_ALG + ret );
{
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_ALG, ret );
goto out;
}
if ( *p != end )
return ( MBEDTLS_ERR_PKCS7_INVALID_FORMAT );
ret = MBEDTLS_ERR_PKCS7_INVALID_FORMAT;
return( 0 );
out:
return( ret );
}
/**
@ -182,7 +195,7 @@ static int pkcs7_get_digest_algorithm_set( unsigned char **p,
static int pkcs7_get_certificates( unsigned char **p, unsigned char *end,
mbedtls_x509_crt *certs )
{
int ret;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
size_t len1 = 0;
size_t len2 = 0;
unsigned char *end_set, *end_cert;
@ -192,9 +205,10 @@ static int pkcs7_get_certificates( unsigned char **p, unsigned char *end,
| MBEDTLS_ASN1_CONTEXT_SPECIFIC ) ) != 0 )
{
if( ret == MBEDTLS_ERR_ASN1_UNEXPECTED_TAG )
return( 0 );
return( MBEDTLS_ERR_PKCS7_INVALID_FORMAT + ret );
ret = 0;
else
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_FORMAT, ret );
goto out;
}
start = *p;
end_set = *p + len1;
@ -202,7 +216,10 @@ static int pkcs7_get_certificates( unsigned char **p, unsigned char *end,
ret = mbedtls_asn1_get_tag( p, end_set, &len2, MBEDTLS_ASN1_CONSTRUCTED
| MBEDTLS_ASN1_SEQUENCE );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_CERT + ret );
{
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_CERT, ret );
goto out;
}
end_cert = *p + len2;
@ -213,18 +230,28 @@ static int pkcs7_get_certificates( unsigned char **p, unsigned char *end,
* The behaviour would be improved with addition of multiple signer support.
*/
if (end_cert != end_set)
return ( MBEDTLS_ERR_PKCS7_INVALID_CERT );
{
ret = MBEDTLS_ERR_PKCS7_INVALID_CERT;
goto out;
}
*p = start;
if( ( ret = mbedtls_x509_crt_parse( certs, *p, len1 ) ) < 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_CERT );
{
ret = MBEDTLS_ERR_PKCS7_INVALID_CERT;
goto out;
}
*p = *p + len1;
/* Since in this version we strictly support single certificate, and reaching
* here implies we have parsed successfully, we return 1. */
/*
* Since in this version we strictly support single certificate, and reaching
* here implies we have parsed successfully, we return 1.
*/
ret = 1;
return( 1 );
out:
return( ret );
}
/**
@ -233,12 +260,12 @@ static int pkcs7_get_certificates( unsigned char **p, unsigned char *end,
static int pkcs7_get_signature( unsigned char **p, unsigned char *end,
mbedtls_pkcs7_buf *signature )
{
int ret;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
size_t len = 0;
ret = mbedtls_asn1_get_tag( p, end, &len, MBEDTLS_ASN1_OCTET_STRING );
if( ret != 0 )
return( ret );
goto out;
signature->tag = MBEDTLS_ASN1_OCTET_STRING;
signature->len = len;
@ -246,7 +273,8 @@ static int pkcs7_get_signature( unsigned char **p, unsigned char *end,
*p = *p + len;
return( 0 );
out:
return( ret );
}
/**
@ -267,60 +295,67 @@ static int pkcs7_get_signer_info( unsigned char **p, unsigned char *end,
mbedtls_pkcs7_signer_info *signer )
{
unsigned char *end_signer;
int ret;
int asn1_ret = 0, ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
size_t len = 0;
ret = mbedtls_asn1_get_tag( p, end, &len, MBEDTLS_ASN1_CONSTRUCTED
asn1_ret = mbedtls_asn1_get_tag( p, end, &len, MBEDTLS_ASN1_CONSTRUCTED
| MBEDTLS_ASN1_SEQUENCE );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO + ret );
if( asn1_ret != 0 )
goto out;
end_signer = *p + len;
ret = pkcs7_get_version( p, end_signer, &signer->version );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO );
goto out;
ret = mbedtls_asn1_get_tag( p, end_signer, &len, MBEDTLS_ASN1_CONSTRUCTED
| MBEDTLS_ASN1_SEQUENCE );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO + ret );
asn1_ret = mbedtls_asn1_get_tag( p, end_signer, &len,
MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE );
if( asn1_ret != 0 )
goto out;
/* Parsing IssuerAndSerialNumber */
signer->issuer_raw.p = *p;
ret = mbedtls_asn1_get_tag( p, end_signer, &len, MBEDTLS_ASN1_CONSTRUCTED
| MBEDTLS_ASN1_SEQUENCE );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO + ret );
asn1_ret = mbedtls_asn1_get_tag( p, end_signer, &len,
MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE );
if( asn1_ret != 0 )
goto out;
ret = mbedtls_x509_get_name( p, *p + len, &signer->issuer );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO );
goto out;
signer->issuer_raw.len = *p - signer->issuer_raw.p;
ret = mbedtls_x509_get_serial( p, end_signer, &signer->serial );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO );
goto out;
ret = pkcs7_get_digest_algorithm( p, end_signer, &signer->alg_identifier );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO );
goto out;
ret = pkcs7_get_digest_algorithm( p, end_signer, &signer->sig_alg_identifier );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO );
goto out;
ret = pkcs7_get_signature( p, end_signer, &signer->sig );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO );
goto out;
/* Do not permit any unauthenticated attributes */
if( *p != end_signer )
return ( MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO );
ret = MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO;
return( 0 );
out:
if( asn1_ret != 0 )
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO,
asn1_ret );
else if( ret != 0 )
ret = MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO;
return( ret );
}
static void pkcs7_free_signer_info( mbedtls_pkcs7_signer_info *signer )
@ -350,7 +385,7 @@ static int pkcs7_get_signers_info_set( unsigned char **p, unsigned char *end,
mbedtls_pkcs7_signer_info *signers_set )
{
unsigned char *end_set;
int ret;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
int count = 0;
size_t len = 0;
mbedtls_pkcs7_signer_info *signer, *prev;
@ -358,17 +393,23 @@ static int pkcs7_get_signers_info_set( unsigned char **p, unsigned char *end,
ret = mbedtls_asn1_get_tag( p, end, &len, MBEDTLS_ASN1_CONSTRUCTED
| MBEDTLS_ASN1_SET );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO + ret );
{
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_SIGNER_INFO, ret );
goto out;
}
/* Detect zero signers */
if( len == 0 )
return( 0 );
{
ret = 0;
goto out;
}
end_set = *p + len;
ret = pkcs7_get_signer_info( p, end_set, signers_set );
if( ret != 0 )
return( ret );
goto out;
count++;
prev = signers_set;
@ -391,7 +432,8 @@ static int pkcs7_get_signers_info_set( unsigned char **p, unsigned char *end,
count++;
}
return( count );
ret = count;
goto out;
cleanup:
signer = signers_set->next;
@ -403,6 +445,8 @@ cleanup:
pkcs7_free_signer_info( prev );
mbedtls_free( prev );
}
out:
return( ret );
}
@ -425,39 +469,46 @@ static int pkcs7_get_signed_data( unsigned char *buf, size_t buflen,
unsigned char *end = buf + buflen;
unsigned char *end_set;
size_t len = 0;
int ret;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
mbedtls_md_type_t md_alg;
ret = mbedtls_asn1_get_tag( &p, end, &len, MBEDTLS_ASN1_CONSTRUCTED
| MBEDTLS_ASN1_SEQUENCE );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_FORMAT + ret );
{
ret = MBEDTLS_ERROR_ADD( MBEDTLS_ERR_PKCS7_INVALID_FORMAT, ret );
goto out;
}
end_set = p + len;
/* Get version of signed data */
ret = pkcs7_get_version( &p, end_set, &signed_data->version );
if( ret != 0 )
return( ret );
goto out;
/* Get digest algorithm */
ret = pkcs7_get_digest_algorithm_set( &p, end_set,
&signed_data->digest_alg_identifiers );
if( ret != 0 )
return( ret );
goto out;
ret = mbedtls_oid_get_md_alg( &signed_data->digest_alg_identifiers, &md_alg );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_INVALID_ALG );
{
ret = MBEDTLS_ERR_PKCS7_INVALID_ALG;
goto out;
}
/* Do not expect any content */
ret = pkcs7_get_content_info_type( &p, end_set, &signed_data->content.oid );
if( ret != 0 )
return( ret );
goto out;
if( MBEDTLS_OID_CMP( MBEDTLS_OID_PKCS7_DATA, &signed_data->content.oid ) )
{
return( MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO ) ;
ret = MBEDTLS_ERR_PKCS7_INVALID_CONTENT_INFO;
goto out;
}
p = p + signed_data->content.oid.len;
@ -466,7 +517,7 @@ static int pkcs7_get_signed_data( unsigned char *buf, size_t buflen,
mbedtls_x509_crt_init( &signed_data->certs );
ret = pkcs7_get_certificates( &p, end_set, &signed_data->certs );
if( ret < 0 )
return( ret ) ;
goto out;
signed_data->no_of_certs = ret;
@ -481,15 +532,17 @@ static int pkcs7_get_signed_data( unsigned char *buf, size_t buflen,
/* Get signers info */
ret = pkcs7_get_signers_info_set( &p, end_set, &signed_data->signers );
if( ret < 0 )
return( ret );
goto out;
signed_data->no_of_signers = ret;
/* Don't permit trailing data */
if ( p != end )
ret = MBEDTLS_ERR_PKCS7_INVALID_FORMAT;
else
ret = 0;
ret = 0;
out:
return( ret );
}
@ -499,17 +552,21 @@ int mbedtls_pkcs7_parse_der( mbedtls_pkcs7 *pkcs7, const unsigned char *buf,
unsigned char *start;
unsigned char *end;
size_t len = 0;
int ret;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
int isoidset = 0;
if( !pkcs7 )
return( MBEDTLS_ERR_PKCS7_BAD_INPUT_DATA );
{
ret = MBEDTLS_ERR_PKCS7_BAD_INPUT_DATA;
goto out;
}
/* make an internal copy of the buffer for parsing */
pkcs7->raw.p = start = mbedtls_calloc( 1, buflen );
if( pkcs7->raw.p == NULL )
{
return( MBEDTLS_ERR_PKCS7_ALLOC_FAILED );
ret = MBEDTLS_ERR_PKCS7_ALLOC_FAILED;
goto out;
}
memcpy( start, buf, buflen );
pkcs7->raw.len = buflen;
@ -573,7 +630,7 @@ int mbedtls_pkcs7_signed_data_verify( mbedtls_pkcs7 *pkcs7,
size_t datalen )
{
int ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
unsigned char *hash;
mbedtls_pk_context pk_cxt = cert->pk;
const mbedtls_md_info_t *md_info;
@ -581,8 +638,10 @@ int mbedtls_pkcs7_signed_data_verify( mbedtls_pkcs7 *pkcs7,
mbedtls_pkcs7_signer_info *signer;
if( pkcs7->signed_data.no_of_signers == 0 )
return( MBEDTLS_ERR_PKCS7_VERIFY_FAIL );
{
ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
goto out;
}
/*
* Potential TODOs
@ -602,20 +661,24 @@ int mbedtls_pkcs7_signed_data_verify( mbedtls_pkcs7 *pkcs7,
{
ret = mbedtls_oid_get_md_alg( &signer->alg_identifier, &md_alg );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_VERIFY_FAIL );
{
ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
goto out;
}
md_info = mbedtls_md_info_from_type( md_alg );
hash = mbedtls_calloc( mbedtls_md_get_size( md_info ), 1 );
if( hash == NULL ) {
return( MBEDTLS_ERR_PKCS7_ALLOC_FAILED );
ret = MBEDTLS_ERR_PKCS7_ALLOC_FAILED;
goto out;
}
ret = mbedtls_md( md_info, data, datalen, hash );
if( ret != 0 )
{
mbedtls_free( hash );
return( ret );
goto out;
}
ret = mbedtls_pk_verify( &pk_cxt, md_alg, hash,
@ -630,6 +693,7 @@ int mbedtls_pkcs7_signed_data_verify( mbedtls_pkcs7 *pkcs7,
signer = signer->next;
}
out:
return( ret );
}
@ -637,7 +701,7 @@ int mbedtls_pkcs7_signed_hash_verify( mbedtls_pkcs7 *pkcs7,
const mbedtls_x509_crt *cert,
const unsigned char *hash, size_t hashlen)
{
int ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
const mbedtls_md_info_t *md_info;
mbedtls_md_type_t md_alg;
mbedtls_pk_context pk_cxt;
@ -646,14 +710,20 @@ int mbedtls_pkcs7_signed_hash_verify( mbedtls_pkcs7 *pkcs7,
pk_cxt = cert->pk;
if( pkcs7->signed_data.no_of_signers == 0 )
return( MBEDTLS_ERR_PKCS7_VERIFY_FAIL );
{
ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
goto out;
}
signer = &pkcs7->signed_data.signers;
while( signer )
{
ret = mbedtls_oid_get_md_alg( &signer->alg_identifier, &md_alg );
if( ret != 0 )
return( MBEDTLS_ERR_PKCS7_VERIFY_FAIL );
{
ret = MBEDTLS_ERR_PKCS7_VERIFY_FAIL;
goto out;
}
md_info = mbedtls_md_info_from_type( md_alg );
@ -673,6 +743,7 @@ int mbedtls_pkcs7_signed_hash_verify( mbedtls_pkcs7 *pkcs7,
signer = signer->next;
}
out:
return ( ret );
}