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

[Nayna]

On 1/23/20 8:45 AM, Stefan Berger wrote:
> 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.

Thanks Stefan !! I fixed the error return codes to add only low-level 
ASN.1 parsing related error, otherwise return the error as is. I also 
updated some of the other error codes for more clarity. The fixes also 
include your other feedback. The updated version is pushed as - 
https://github.com/naynajain/mbedtls/commits/mbedtls-2.16-pkcs7v1.1.

Thanks & Regards,

       - Nayna