Version 2.1.96 broke SignerInfo parsing

[sergeypayu]

Unfortunately, the fix you made in 2.1.96 broke parsing completely legit SignerInfo data. Here is an example of the data that now gives "Wrong values for Choice type" error:

https://lapo.it/asn1js/#MIICoQIBAaAiBCCt0OZVQWI9B_pECwXJKCZvqKAueUsQxvAjliiduzY-nTAMBgoqhiQCAQEBAQIBoIICFzAvBgkqhkiG9w0BCQQxIgQggb05gxSaJQdBS9Dx9eXgDySHySlxH5lNcqSVPZssnGgwGAYJKoZIhvcNAQkDMQsGCSqGSIb3DQEHATCCAcgGCyqGSIb3DQEJEAIvMYIBtzCCAbMwggGvMIIBqzAMBgoqhiQCAQEBAQIBBCCmhZDxElXQCmldl2CEWSac3APNa1MslhGIJ36PfaixGjCCAXcwggFdpIIBWTCCAVUxVDBSBgNVBAoMS9CG0L3RhNC-0YDQvNCw0YbRltC50L3Qvi3QtNC-0LLRltC00LrQvtCy0LjQuSDQtNC10L_QsNGA0YLQsNC80LXQvdGCINCU0KTQoTFeMFwGA1UECwxV0KPQv9GA0LDQstC70ZbQvdC90Y8gKNGG0LXQvdGC0YApINGB0LXRgNGC0LjRhNGW0LrQsNGG0ZbRlyDQutC70Y7Rh9GW0LIg0IbQlNCUINCU0KTQoTFiMGAGA1UEAwxZ0JDQutGA0LXQtNC40YLQvtCy0LDQvdC40Lkg0YbQtdC90YLRgCDRgdC10YDRgtC40YTRltC60LDRhtGW0Zcg0LrQu9GO0YfRltCyINCG0JTQlCDQlNCk0KExGTAXBgNVBAUMEFVBLTM5Mzg0NDc2LTIwMTgxCzAJBgNVBAYTAlVBMREwDwYDVQQHDAjQmtC40ZfQsgIUILTk7Q0wmYwEAAAAmVosAK7ReAAwDQYLKoYkAgEBAQEDAQEEQKxdgj8Y5sTGOh_zp33XxAF4UTSoa5FJ4BHiRdcWQjcMVvSlv8vZU7kK1DW9y0G9g0tOLkGltbt_BLUJdyd8i0M

SignerIdentifier is a tagged object which is parsed as Constructed by asn1js. So changing Constructed to Primitive in the schema is not working.

[sergeypayu]

325 #324

[microshine]

RFC 5652 says that subjectKeyIdentifier is implicit.

ASN.1 Module

DEFINITIONS IMPLICIT TAGS ::=
BEGIN

SignerIdentifier ::= CHOICE {
  issuerAndSerialNumber IssuerAndSerialNumber,
  subjectKeyIdentifier [0] SubjectKeyIdentifier }

As I can see in your example your sid is explicit

[0] (1 elem)
  OCTET STRING (32 byte) ADD0E65541623D07FA440B05C928266FA8A02E794B10C6F02396289DBB363E9D

but must be implicit

  [0] (32 byte) ADD0E65541623D07FA440B05C928266FA8A02E794B10C6F02396289DBB363E9D

OpenSSL declares Context-specific [0] as implicit field. See source code

Please check out how your signed data was created

[sergeypayu]

Openssl can parse this signedData just fine:

openssl cms -in signedData.dat -inform DER -noout -cmsout -print

[microshine]

Font one more interesting thing in your example.

version is the syntax version number. If the SignerIdentifier is the CHOICE issuerAndSerialNumber, then the version MUST be 1. If the SignerIdentifier is subjectKeyIdentifier, then the version MUST be 3.

Your SignerVersion is version 1

Could you send me the signed file to microshine@mail.ru for testing?

[sergeypayu]

Yes, there is a confusion with the version number for some reason. But SignerIdentifier should be accepted as both Primitive and OctetString.

Here is the link to bouncy castle source: https://github.com/bcgit/bc-java/blob/0d1e3a010edfbf64be9da33120086b5d8d91fdc1/util/src/main/java/org/bouncycastle/asn1/cms/SignerIdentifier.java#L51

Accepted inputs:
     * <ul>
     * <li> null &rarr; null
     * <li> {@link SignerIdentifier} object
     * <li> {@link IssuerAndSerialNumber} object
     * <li> {@link org.bouncycastle.asn1.ASN1OctetString#getInstance(java.lang.Object) ASN1OctetString} input formats with SignerIdentifier structure inside
     * <li> {@link org.bouncycastle.asn1.ASN1Primitive ASN1Primitive} for SignerIdentifier constructor.
     * </ul>

[microshine]

I confirm that BouncyCastle supports both times Primitive and OctetString

Fixing PKIjs

[microshine]

@sergeypayu Please try the latest version pkijs@2.1.97

[sergeypayu]

Yes, I confirm, 2.1.97 works as expected. Thanks for the quick fix!