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

Ed25519 pkcs8 documents do not have RFC-valid public key part #1464

Closed
ShadowJonathan opened this issue Feb 19, 2022 · 4 comments · Fixed by #1680
Closed

Ed25519 pkcs8 documents do not have RFC-valid public key part #1464

ShadowJonathan opened this issue Feb 19, 2022 · 4 comments · Fixed by #1680
Milestone

Comments

@ShadowJonathan
Copy link

ShadowJonathan commented Feb 19, 2022

I discovered this while working with pkcs8 0.8;

In the key template, ring prefixes the public key bit with A1 23 03 21 (Context Specific [1], then BITSTRING), while the RFC works with "Context Specific [1]" only, the parser is then supposed to infer the following data is a BITSTRING, without prefix.


Furthermore, together with #1299 and #834, i strongly recommend switching to a pkcs8-parser library, instead of manually wrapping and generating keys like this.

@briansmith briansmith changed the title pkcs8 documents do not have RFC-valid public key part Ed25519 pkcs8 documents do not have RFC-valid public key part Feb 20, 2022
@briansmith
Copy link
Owner

First, thanks for reporting this. I think I have a fix already in my local tree. I have some questions though.

Furthermore, together with #1299

You filed #1299, and then very soon after closed it, saying "this behaviour is - albeit confusing - intended." I thought when you wrote "this behaviour is - albeit confusing - intended" you were saying that the bug report was invalid. Should we reopen #1299?

and #834

It turns out that I was mistaken when I filed #834. The issue is actually exactly the same as the one you're reporting in this issue (#1464): We're encoding/decoding the public key as though it is EXPLICITLY tagged, but it is actually IMPLICITLY tagged.

i strongly recommend switching to a pkcs8-parser library, instead of manually wrapping and generating keys like this.

As you noted elsewhere, other PKCS#8 libraries have had the same or similar issues.

@ShadowJonathan
Copy link
Author

#1299 was about a bug that OpenSSL had wrt this, but that they refuse to revert, and effectively has become standard.

That bug is fine, this is not, I'm recommending to move to those libraries as other libraries have undoubtedly encountered these problems before, and applied fixes themselves.

@briansmith
Copy link
Owner

As a first step, I've refactored the Ed25519 PKCS#8 parser tests to get better coverage, which will enable the fixing of this. Those changes are in #1466.

@NoelTautges
Copy link
Contributor

I came across this bug while using pkcs8 0.9.

Ed25519 PKCS#8 v2 key documents fail to parse under the stricter parsing rules in der >=0.5.1, which checks if implicit context-specific tags' inner tags are allowed to be constructed (1, 2 - tags can only be constructed if they're ORed with CONSTRUCTED_FLAG, for reference). This parsing behavior concurs with the X.690 spec (section 10.2 on page 20) which states that "for bitstring, octetstring and restricted character string types, the constructed form of encoding shall not be used."

ed25519_pkcs8_v2_template.der includes the public key bitstring in its constructed form, which fails the check and causes an error.

It seems that this issue was recognized and fixed on the parsing side in #1480, but ed25519_pkcs8_v2_template.der was never fixed to include the correct primitive encoding, something that was planned but didn't end up happening. Life gets in the way of these things.

I'll try to make a PR for this. Wish me luck.

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