Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"parameters" missing when using rsa / rsassa_pkcs1v15 #23

Closed
joernheissler opened this issue Aug 29, 2016 · 2 comments
Closed

"parameters" missing when using rsa / rsassa_pkcs1v15 #23

joernheissler opened this issue Aug 29, 2016 · 2 comments

Comments

@joernheissler
Copy link
Collaborator

There's a feature in algos.SignedDigestAlgorithm to set parameters to Null when using certain algorithms. "rsassa_pkcs1v15" is missing.
While this is easily fixed, same problem occurs for KeyEncryptionAlgorithm.

I'm wondering why you're using different classes, according to rfc5652 both SignatureAlgorithmIdentifier and KeyEncryptionAlgorithmIdentifier are the same as AlgorithmIdentifier. And rfc4055 defines this "parameters MUST be present and MUST be NULL" constraint for AlgorithmIdentifier.
So shouldn't the logic for Null be inside AlgorithmIdentifier and the other two classes inherit from here?

@wbond
Copy link
Owner

wbond commented Aug 30, 2016

The reason algos.SignedDigestAlgorithmId and cms.KeyEncryptionAlgorithmId are different is because they are for different purposes - in other words, primarily for semantics.

I'm not keen in using inheritance with a class that has _fields, especially when one of the fields would need to be redefined for the child class.

I created a mixin in 381a4da that should ensure we have core.Null() for parameters of the various structures that use the OIDs from RFC 4055. I'm not 100% happy with the solution, if only because if someone created their own AlgorithmIdentifier structure that uses the OIDs, they would have to include algos._ForceNullParameters, which intended to be "private".

@joernheissler
Copy link
Collaborator Author

Thankyou, fix confirmed working.

@wbond wbond closed this as completed Aug 30, 2016
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

No branches or pull requests

2 participants