[Skiboot] [PATCH v2 02/12] crypto: add out-of-tree mbedtls pkcs7 parser

[Stefan Berger]

On 1/19/20 9:36 PM, Eric Richter wrote:
> +static int pkcs7_get_signature( unsigned char **p, unsigned char *end,
> +              mbedtls_pkcs7_buf *signature )
> +{
> +      int ret;
> +      size_t len = 0;
> +
> +      ret = mbedtls_asn1_get_tag(p, end, &len, MBEDTLS_ASN1_OCTET_STRING);
> +      if ( ret != 0 )
> +              return ( MBEDTLS_ERR_PKCS7_INVALID_FORMAT + ret );

My review of this is here at the bottom of this page: 
https://github.com/naynajain/mbedtls/compare/mbedtls-a2.16-pkcs7v1.0

My concern about these 'error sums' is like this here:

#define MBEDTLS_ERR_PKCS7_INVALID_FORMAT                   -0x7180



+      ret = pkcs7_get_signature( p, end, &signers_set->sig );
+      if ( ret != 0 )
+              return ( MBEDTLS_ERR_PKCS7_INVALID_FORMAT + ret );

I think you should just return 'ret' here since otherwise you return 2 * MBEDTLS_ERR_PKCS7_INVALID_FORMAT + ret, which doesn't seem right. May want to check throughout the code.