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

more accurate error message #23

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

philippecery
Copy link

Hi @youmark,

In issue #17, @Fire1nRain seemed confused by the error message "only PKCS#5 v2.0 supported". Indeed, the error occured, not because PKCS#5 v2.0 was not supported - based on the meta data, that private key seemed encrypted using PKCS#5 v2.0 - but because he passed an invalid ASN1 block - most likely, as @conradoplg mentioned, the raw PEM he read from the file - which is why asn1.Unmarshal failed.
So, I suggest this change to the error message (1) to be consistent with the error message returned by x509.ParsePKCS8PrivateKey when asn1.Unmarshal fails and (2) to help developers who inadvertently passed an invalid ASN1 and think it is a bug.

Hope that helps clarify things. Thanks for this library, very helpful.

Regards,
Philippe.

@UlrichEckhardt
Copy link
Contributor

I've run the tests on this and it doesn't cause regressions. I like the fact that it also adds a bunch of test cases. I'd vote for merging this PR.

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