search

AVSystem / avs_commons

A set of common code used in AVSystem for projects written in C.
Apache License 2.0
7 stars 12 forks source link

PKCS7 parsing error using pkcs7_signed_data_parse #309

Closed tfloch closed 1 year ago

tfloch commented 1 year ago

Hello,

Using mbedtls, avs_crypto_parse_pkcs7_certs_only fails to parse my PKCS7 file.

Code example here: https://github.com/tfloch/avs_pkcs7_parse_sample

The problem seems to be that it contains multiples "Undefined length" ASN1 tags.

I managed to get the parsing working, using this dirty patch: https://github.com/AVSystem/avs_commons/compare/master...tfloch:avs_commons:parse_pkcs7_with_multiples_indef_length_asn1_tags_workarround

Thank you for your support :)

kFYatek commented 1 year ago

Hi @tfloch,

avs_crypto_parse_pkcs7_certs_only() is not intended to be able to parse every possible generic PKCS#7 file. It is only intended for files with MIME type of application/pkcs7-mime; smime-type=certs-only, which is also defined in RFC 8551 as a Certificate Management Message.

To quote from that RFC, in such a message:

The SignedData encapContentInfo eContent field MUST be absent, and the signerInfos field MUST be empty.

Your "bad" file not only contains "indefinite length" elements, but it also has that eContent field present. The modification to the code you posted attempts to skip exactly over that specific element of the ASN.1 structure.

Applying that patch, or anything similar, would allow accepting files that do not fit the definition of a "certs-only" PKCS#7 file, which is against the requirements of the RFC.

Also please note that a file that contains "indefinite length" elements is, by definition, not a DER file, but rather BER (DER being a feature-limited subset of BER). This is fine in this case, as avs_crypto_parse_pkcs7_certs_only() is intended to support BER data, but since the file extension suggests DER, this may indicate some other problem in your data flow that you may want to examine in detail.

May I ask what is your use case? How were your files generated and what are you trying to accomplish by parsing them?

tfloch commented 1 year ago

Hi @kFYatek,

Thank you for you analysis :)

Your "bad" file not only contains "indefinite length" elements, but it also has that eContent field present.

Ok!

Also please note that a file that contains "indefinite length" elements is, by definition, not a DER file, but rather BER.

Ok!

May I ask what is your use case? How were your files generated and what are you trying to accomplish by parsing them?

We have developed an EST-coaps (=RFC 9148) client using avs_commons. This bad.pkcs7.der content is produced by the EST server we are talking to, developed by our colleagues.

If I understand correctly RFC 9148, PKCS7 content sent by the EST Server is supposed to be in DER, nor BER.

In conclusion, the EST server does not seem to complies with standard:

kFYatek commented 1 year ago

Hi @tfloch,

eContent field in PKCS7 should be absent

That depends on the context, actually. In the context of EST-coaps, the /skc request calls for full PKCS#7, in which case your file would be OK, I believe. However, avs_commons does not support the entire PKCS#7 format, only the "certs-only" subset.

However, the /skg request only expects a "certs-only" file, which cannot contain eContent field.

PKCS7 should be DER encoded

To be honest, I'm not sure if that is specified explicitly anywhere. However, DER is generally most effective in terms of data usage and as such, it's preferred.

tfloch commented 1 year ago

That depends on the context, actually. In the context of EST-coaps, the /skc request calls for full PKCS#7, in which case your file would be OK, I believe. However, avs_commons does not support the entire PKCS#7 format, only the "certs-only" subset.

We only uses /crts and /sen.

In https://datatracker.ietf.org/doc/html/rfc9148, I read:

Content-Format 287 can be used in place of 281 to carry a single certificate instead of a PKCS #7 container in a /crts, /sen, /sren, or /skg response. Content-Format 281 MUST be supported by EST-coaps servers. Servers MAY also support Content-Format 287.

So if I am right:

PKCS7 should be DER encoded

To be honest, I'm not sure if that is specified explicitly anywhere.

You are right, that's not perfectly clear.

I only found https://datatracker.ietf.org/doc/rfc8951/, which says:

This document updates [RFC7030] to require the POST request and payload response of all endpoints using base64 encoding, as specified in Section 4 of [RFC4648]. In both cases, the Distinguished Encoding Rules (DER) [X.690] are used to produce the input or the base64 encoding routine.

Thank you again for you help!

tfloch commented 1 year ago

Based on our discussion, the issue should be closed :)