Skip to content

Conversation

eulercb
Copy link

@eulercb eulercb commented Dec 2, 2018

Fix issue #162.

Below is a sample of a signed CMS before the patch:
MIIEagYJKoZIhvcNAQcCoIIEWzCCBFcCAQExDzANBglghkgBZQMEAgEFADATBgkqhkiG9w0BBwGgBgQEVEVTVKCCAoQwggKAMIIBaKADAgECAggX/rexsv6GJjANBgkqhkiG9w0BAQsFADAAMB4XDTE3MTIwMjE2MjM1NFoXDTE5MTIwMjE2MjM1NFowADCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALf3OMZ75RFL5zTZNQViSPF+2AuNlW6haqaZFjfIRdlZQ97MoqFTxzPgPBr3sovaraO5/3kolxm03bj2BGNcShp8lDKQKk3An1qKN8no7XLDXV216PukH/lbzLX1E78gKNeEovX/TmtiDTCiJValzmflXZ58HAhaffOaKbC3qdU1BGsuNx+KTIogjjR0ScPqX5WeaGeOc3WHXFxmkoc/CNx0KREMGwAYiaZhjNsEfFcVG9inqS/loVz4+qviXxOwUXFrYiQhuDLQFn9PVAtTiXZ/SFXk+/HDmUXisRdXDZfdKcE4z9X1IrdEl9YZrXgYIlVswmF73EdcC/FHThosnI0CAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAqROROypDe2KKo5QQlHcn1KQ8nkuNZ6w3y8kldf5RXkf6tKjvXlMDoXxFfXL0JEAPrkmPEjC81A6hRpaS3PTEwKqOzBTGE9fB2N2CzX3l5utgvZQ1j+bBlCiyKzYwaR237hA/uprSycKYP5vRwATn6fjBGL4UDvNTauApHt01A3TKk4LdswPgBf7fT1bChOd6Y3RVHJecGDJueFhJAZplUPXH4rr6wv4QayvSztt8H3aY56gbtQ13eqJlZ7jZrrcrUEdghOscF+YwPkboQPKPincygzOukdLjJadbGeWUF8jvHtQwCLTpYx1wRsvBHBxQS6aaxf0q5+C4cSCxCFcO/TGCAaIwggGeAgEBMAwwAAIIF/63sbL+hiYwDQYJYIZIAWUDBAIBBQCgaTAYBgkqhkiG9w0BCQMxCwYJKoZIhvcNAQcBMBwGCSqGSIb3DQEJBTEPFw0xODEyMDIxNjIzNTRaMC8GCSqGSIb3DQEJBDEiBCCU7gWTNeWH5QHMS/kGE+CBTwCnsIvHxkj9hloq9qIswjANBgkqhkiG9w0BAQEFAASCAQCEBggHRHDqOJixEO2PWyml7YlcVRuit6BkAZR0TeTnN8u+kP2Q2as2Gyf5tIqSwgyQuVQznE+9+3SeC2hQUPu9VbIBn0rX9siXbPPqfMt5u8VfNyVennbuqtdW0gBWo8DGVMTCNeZKXmW7fnsYSh2AuMSdKFXmRaj5BJlEFjY69F2Gmc/PxoqH5kf1yuznZ+gC7XrTHuHt3Hqmk+0fvG4DWMqKDqvAyhc+fVb//HOsjAtc6gP8nfaw5/jEeCAc633YOVkv6BTM29D1DSr7+RWHKa3FNFgnKzaMQH873xh+EN8CHe7g4MeQmgWg0+t1g3UtRBCzDOwcPuUZwQc5MWMM

And one after the patch:
MIIEaAYJKoZIhvcNAQcCoIIEWTCCBFUCAQExDTALBglghkgBZQMEAgEwEwYJKoZIhvcNAQcBoAYEBFRFU1SgggKFMIICgTCCAWmgAwIBAgIJALN9yL4UVw5iMA0GCSqGSIb3DQEBCwUAMAAwHhcNMTcxMjAyMTYyNjE4WhcNMTkxMjAyMTYyNjE4WjAAMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA2qk1JDtshgz7r81yHSyvr5dc8oay3FV7WqMzcYlNPsdbgd2hbSlkWrcNnKzyokmqEId0wV2wRQ6Zvaw0v9yFwGSwsOv+mQofc6iOi6vc/VRFib6o06dtnuBZrrpwl6P2dt8uUb3BtwwD+2MD+q3X/LG/gK9RXLZh9kWmGbTXdU8GHSK/iO41pU88ZQ/iuzJrEYF89W/XCNYdKDf20jv8N78AL9BUawAJrqGRgtS+HT9oodLzYGoEIR9UeIao/WxmGncUJuVqZSWhOz9svGyPwHFiKbC2GkUQq2/gnzHN5HTlVHmk8u4RtEnh5USZJ5NRuS5pHk2gD1Mc/GCYjC9qsQIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQA4rsYYH8JWlHElOK34arfccQXnYRW2dyLBtKMB5rLawV3rJ6v8YwtXPVInKJANCcPMpmzsvNqH7EC2wxePBU/L01O+gxAZbhKDFxBmRkIsa5THf19B8pdUM3NjhapNtLJ5zDMzF9LoRNxWix1CLTGdOzCasGhHuHqw6QlieXf5xUp5Mqxo7QyJAP30O0OdN6+mK8obR8fk4AnzIacx2yyZTox6z9GvhQN1TOz5Q+P45YdEM2Y5jclKhktNvcMxf1ENPjcgmKiv3BrYR7hVk31vzwIL5rf9LTZJSrbyk5in8gQBjmTE0f6rOcHnBpUgRDqpYCmGY4K8WeNZVLnZGoHzMYIBoTCCAZ0CAQEwDTAAAgkAs33IvhRXDmIwCwYJYIZIAWUDBAIBoGkwGAYJKoZIhvcNAQkDMQsGCSqGSIb3DQEHATAcBgkqhkiG9w0BCQUxDxcNMTgxMjAyMTYyNjE5WjAvBgkqhkiG9w0BCQQxIgQglO4FkzXlh+UBzEv5BhPggU8Ap7CLx8ZI/YZaKvaiLMIwDQYJKoZIhvcNAQEBBQAEggEANQ8fYzEvkE7U81swXfZmo+HX9myGjqQlA7bLqTbK4oX+VOAvY7tNgf5MlYdIrRZsPO3i0GZq3x4DVzWzRXM3dt5sj02GbYeJzJcusxX6p22NmA6RfsN7celynhdtnj+CjAec+pYmyZaQokSCtGMSybVg2SnK50XtUJJRmL/6iCQLX8fuyIJpULQovbE9aQ6e+3US9dHqCGitE74VAmLb7R9XAYJoVDvLlasQbZFNZ9h80tMg0YQanq6WqBzheRYUuygCt1RmzXhAFgVFAJwF+cIjsq0fGP1s2vQSDN6i/M2euUiCazicSKRZqnXFC45s09us61IHgHborQ67DLONhQ==

I hope there are no side effects by this change.

@bcgit
Copy link
Collaborator

bcgit commented Dec 2, 2018

https://tools.ietf.org/html/rfc3370 section 2 specifies that the NULL parameters field is mandatory for some algorithms, optional for some others. For compatibility reasons we have always added it.

I do note that RFC 5754 section says that implementations MUST leave out the NULL for SHA-2 except where it is used in PKCS#1... it does say both should be accepted, but we need to look into this further. The patch proposed is not enough but it does appear something should be changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants