PKCS7 parsing fails with empty digestAlgorithm

[JWoyack]

Summary

PKCS7 parsing fails with -0x5560 when signedData contains an empty digestAlgorithms field.

System information

Mbed TLS version (number or commit id): mbedtls fork of ncs-v3.1.1 (https://github.com/nrfconnect/sdk-mbedtls/tree/main)
Operating system and version: Ubuntu 22.04.5 LTS
Configuration (if not default, please attach mbedtls_config.h): MBEDTLS_PKCS7_C enabled
Compiler and options (if you used a pre-built binary, please indicate how you obtained it):
Additional environment information:

Expected behavior

mbedtls_pkcs7_parse_der() successfully parses certificates with empty digestAlgorithm within their signedData

Actual behavior

mbedtls_pkcs7_parse_der() fails with error code -0x5560, which is an overlay of -0x5500 MBEDTLS_ERR_PKCS7_INVALID_ALG and -0x0060 MBEDTLS_ERR_ASN1_OUT_OF_DATA.

Steps to reproduce

Try parsing a PKCS7 container where the digestAlgorithm is empty:

E.g:

SEQUENCE (2 elem)
  OBJECT IDENTIFIER 1.2.840.113549.1.7.2 signedData (PKCS7)
  [0] (1 elem)
    SEQUENCE (5 elem)
      INTEGER 1
      SET (0 elem) /* <-- This is the element that gets the function to fail */
      SEQUENCE (1 elem)
        OBJECT IDENTIFIER 1.2.840.113549.1.7.1 data (PKCS7)

respective binary blob (the 0x31 0x00 represents the SET (0 elem) member):

30 82 03 C3 06 09 2A 86 48 86 F7 0D 01 07 02 A0
82 03 B4 30 82 03 B0 02 01 01 31 00 30 0B 06 09
2A 86 48 86 F7 0D 01 07 01 A0 82 03 98 30 82 03

Additional information

As far as I've understood RFC 2315 Section 9.1 SignedData, the field after the version shall contain the digestAlgorithms.

This is even more explicitly specified in the notes section:

According to readthedocs pkcs7.h, The signedData syntax follows version 1.

Therefore I would assume that the SET (0 elem) (which should equal an empty digestAlgorithm in my opinion) should be able to be parsed without any errors.

Now here whats happening within mbedtls:

Parsing works fine until the function pkcs7_get_digest_algorithm_set(). Here the function mbedtls_asn1_get_tag() is called, which returns a len parameter of 0.
In the upcoming calculation

end = *p + len;     /* Since len is 0, end is equal to *p */

Now the following function mbedtls_asn1_get_alg_null()

and inside of it mbedtls_asn1_get_alg() is called.
Here's a check for:

    if ((end - *p) < 1) {    /* This fails since end is equal to *p */
        return MBEDTLS_ERR_ASN1_OUT_OF_DATA;
    }

Therefore MBEDTLS_ERR_ASN1_OUT_OF_DATA is returned, its being merged with MBEDTLS_ERR_PKCS7_INVALID_ALG and therefore returns with error code -0x5560 without continuing the parsing process.

Now the question:
Is this a valid bug and empty digestAlgorithms should be parsable, or am I interpreting the standard wrong?

Additional info:
openssl and online tools are parsing such PKCS7 containers without any issue.

Could you help me out with this issue?

[gilles-peskine-arm]

I agree with your analysis that an empty DigestAlgorithmIdentifiers set seems valid. It looks like the code of pkcs7_get_digest_algorithm_set() assumes that there is exactly one algorithm in the set, and the limitation to at most one algorithm is deliberate but the limitation to at least one algorithm is unintended.

(There's a note that in version 0 of the format, the set could not be empty. But our implementation only supports version 1, where the set can be empty.)

The code that uses the result of pkcs7_get_digest_algorithm_set() also checks that the given digest algorithm is supported and would error out otherwise. So it's not as simple as patching pkcs7_get_digest_algorithm_set().

I'm not familiar with PKCS7. What can you do with a PKCS7 SignedData that has an empty DigestAlgorithmIdentifiers? Wouldn't there be no way to verify its signature?