ring icon indicating copy to clipboard operation
ring copied to clipboard

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

Open ShadowJonathan opened this issue 2 years ago • 3 comments

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 https://github.com/briansmith/ring/issues/1299 and https://github.com/briansmith/ring/issues/834, i strongly recommend switching to a pkcs8-parser library, instead of manually wrapping and generating keys like this.

ShadowJonathan avatar Feb 19 '22 12:02 ShadowJonathan

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.

briansmith avatar Feb 20 '22 02:02 briansmith

https://github.com/briansmith/ring/issues/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.

ShadowJonathan avatar Feb 20 '22 07:02 ShadowJonathan

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 https://github.com/briansmith/ring/pull/1466.

briansmith avatar Feb 21 '22 00:02 briansmith

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.

NoelTautges avatar Sep 22 '22 18:09 NoelTautges