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>
diff --git a/library/pkcs7.c b/library/pkcs7.c
index 65dc83a..2299cfd 100644
--- a/library/pkcs7.c
+++ b/library/pkcs7.c
@@ -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 @@
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_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 @@
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_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 @@
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_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 @@
| 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 @@
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 @@
* 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_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 @@
*p = *p + len;
- return( 0 );
+out:
+ return( ret );
}
/**
@@ -267,60 +295,67 @@
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 @@
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 @@
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 @@
count++;
}
- return( count );
+ ret = count;
+ goto out;
cleanup:
signer = signers_set->next;
@@ -403,6 +445,8 @@
pkcs7_free_signer_info( prev );
mbedtls_free( prev );
}
+
+out:
return( ret );
}
@@ -425,39 +469,46 @@
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 @@
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 @@
/* 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 @@
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 @@
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 @@
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 @@
{
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 @@
signer = signer->next;
}
+out:
return( ret );
}
@@ -637,7 +701,7 @@
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 @@
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 @@
signer = signer->next;
}
+out:
return ( ret );
}