search

openpgp-pqc / draft-openpgp-pqc

Repository of the WIP draft-ietf-openpgp-pqc
Other
8 stars 2 forks source link

binding signature hash for PQC encryption v4 keys #91

Closed falko-strenzke closed 4 months ago

falko-strenzke commented 7 months ago

Kai Engert kindly pointed out that Werner Koch had mentioned (on the LibrePGP list?) that with v4 PQC keys SHA-1 binding signatures would still be allowed. I currently don't see a problem with this, as I don't see how collission attacks against key binding signatures could be possible. The attacker would have to be able to supply one public key for which he has a collision and have that be signed by the victim.

Still, we might want to mandate that at least SHA-256 must be used in the binding signatures. Any opinions @wussler @fluppe2 @TJ-91 ?

kaie commented 7 months ago

Werner had mentioned it in this message: https://lists.gnupg.org/pipermail/librepgp-discuss/2024/000045.html

TJ-91 commented 7 months ago

For v4 fingerprints (that seems to be what Werner is talking about?) we will not be able to do anything. For other purposes, I think the Crypto Refresh already forbids SHA-1 for most things, see https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-13#section-9.5

kaie commented 7 months ago

Couldn't your draft include a statement like this? "Applications that support PQC encryption keys defined in this document MUST ensure that the binding signature for the PQC subkey uses a SHA-256 signature or a stronger has. PQC encryption subkeys that use SHA-1 for the binding signature MUST be ignored."

dkg commented 7 months ago

On Tue 2024-02-13 05:22:20 -0800, Falko Strenzke wrote:

I don't see how collission attacks against key binding signatures could be possible. The attacker would have to be able to supply one public key for which he has a collision and have that be signed by the victim.

What about a scenario where an attacker offers the user a decryption-capable subkey (e.g. by providing them with a smartcard)?

Even if we don't think this could ever be a plausible scenario, there is a higher-level, simpler story that is useful to be able to tell, and it goes like this:

Carving out random corner exceptions for one nuanced case or another just weakens that story and makes it more challenging to offer clarity for implementers and analysts.

TJ-91 commented 7 months ago

SHA-1 is never used in modern OpenPGP

I think that is too ambiguous. Is v4 + PQC "modern"? For v6 it's clear and a trivial statement. For v4 we have the ambiguity: You can implement the old standard or the crypto refresh. The Crypto Refresh forbids SHA-1 with the exception where it's really needed (fingerprint, v1 SEIPD). The Crypto Refresh explicitly mentions signatures:

Implementations MUST NOT generate signatures with MD5, SHA-1, or RIPEMD-160.

So, we can add all sorts of statements that the Crypto Refresh already makes but I think this will bloat the PQC spec. We can either clarify that implementations should/must follow the Crypto Refresh's v4 specification, or we can add some broad statements about deprecated hash and symmetric encryption algorithms. Personally, I would like to go with the first option. Some further clarification can be provided in the security considerations, if needed.

Would that be a satisfactory solution to this issue?

dkg commented 7 months ago

On Thu 2024-02-15 02:32:16 -0800, Johannes Roth wrote:

SHA-1 is never used in modern OpenPGP

I think that is too ambiguous.

That's exactly my point. It's already too ambiguous. please don't make it more ambiguous.

Is v4 + PQC "modern"?

If by "v4 + PQC" you mean "following only the guidance in RFC 4880, but with a post-quantum algorithm", this combination on its face is not modern. v4 was defined in RFC 4880 in 2007, over 16 years ago. Even at that time, it was known (via Wang et al.) that SHA-1 was not everything it was hoped it would be.

For v4 we have the ambiguity: You can implement the old standard or the crypto refresh.

Where the crypto-refresh offers clarification for packets initially defined in RFC 4880, the PQC draft should absolutely follow it. It's 2024, and there is absolutely no reason to accommodate an implementation that is capable of understanding ML-KEM, ML-DSA, or SLH-DSA, but still decides to publish and rely on SHA-1 for its signatures.

So, we can add all sorts of statements that the Crypto Refresh already makes but I think this will bloat the PQC spec.

I don't think it's much of a bloat to say something simple like:

Subkey binding signatures over algorithms described in this document
and primary key binding signatures made by algorithms described in
this document MUST NOT be made with `MD5`, `SHA-1`, or `RIPEMD-160`.
A receiving implementation MUST be treated such a signature as
invalid.
falko-strenzke commented 7 months ago

I don't think it's much of a bloat to say something simple like: Subkey binding signatures over algorithms described in this document and primary key binding signatures made by algorithms described in this document MUST NOT be made with MD5, SHA-1, or RIPEMD-160. A receiving implementation MUST be treated such a signature as invalid.

In principle I agree, that this text makes sense. But if we should find more and more requirements from the crypto-refresh that we need to duplicate, I would prefer to find another way of expressing the same thing. We could then for instance say that for v4/PQC, the v4 imlementation MUST be subject to the specifications made in the crypto-refresh vor v4.

TJ-91 commented 7 months ago

Another requirement would be to forbid encryption via TripleDES, CAST5, IDEA.

If it's only those two cases (hashes and symmetric encryption), I'm fine with it, otherwise I would also prefer to simply mandate the Crypto Refresh's v4 spec. Or is there a reason to adopt only a few selected requirements from the crypto refresh?

kaie commented 7 months ago

Just thinking out loud, if the RFC for PQC was self-contained, and didn't depend on the crypto-refresh document, then maybe some implementations would be willing to adopt and implement the PQC document, even if they aren't ready for the crypto-refresh specification. By adding a reference to the crypto-refresh document, it could be considered a hint that the RFC for PQC cannot be implemented independently.

wussler commented 7 months ago

We do have to reference somehow the OpenPGP spec, and I don't think it's a good idea to reference RFC4880, since the upcoming RFC obsoletes it

TJ-91 commented 7 months ago

@kaie That sounds reasonable. I think the most constructive way forward would be to look at the differences between the Crypto Refresh and RFC 4880 (+Camellia/ECC) and solely concentrate on v4. Then we can see if we need all of it, or only some of it.

Just a note: this all only affects v4 certificates with a PQ/T encryption subkey. PQ(/T) signatures are only allowed for v6 currently.

When skimming through https://datatracker.ietf.org/doc/html/draft-ietf-openpgp-crypto-refresh-13#name-upgrade-guidance-adapting-i I think nothing really hurts too much for an implementation willing to implement PQC.

Please tell me if I've missed something big. I see three options:

  1. Mandating to follow the Crypto Refresh's v4 spec.
  2. Decide on a "minimal set of additions to RFC 4880(+Camellia/ECC)" that we deem necessary for PQC.
  3. Make no restrictions at all and just let implementations add a new public-key algorithm.

I think 1) is the easiest solution but would be willing to go with 2) if we see a clear reason that this could speed up the adoption of PQC encryption. I don't really see it, though. As for option 3) I guess we have ruled that out already.

dkg commented 7 months ago

On Thu 2024-02-15 06:13:32 -0800, Falko Strenzke wrote:

for v4/PQC, the v4 imlementation MUST be subject to the specifications made in the crypto-refresh vor v4.

Remember that "v4" only refers to keys and signatures, and not to encryption. But i think we need to talk about encryption as well.

The draft will likely avoid referring to RFC 4880 entirely and only refer to the RFC that the crypto-refresh becomes.

We have then, i think, two options:

I lean toward the latter, because we are already coupling classical symmetric algorithms with the PQC algorithms in this draft (e.g. SLH-DSA), and i don't think it's "bloat" to add a sentence or two as explicit clarification.

TJ-91 commented 7 months ago

explicitly call out the same constraints, while referring to the appropriate sections in the crypto-refresh.

I think nobody is against adding a sentence or two for clarification, even if it's duplicated from the Crypto Refresh. What do we do with "avoiding SED" or "avoiding v3" or changes to "S2K"? Do we add that to the PQC document? Why or why not? Can we agree on a set of statements that we need to duplicate while not introducing ambiguitiy by leaving out only some?

wussler commented 4 months ago

I would add something as easy as

Subkey binding signatures over algorithms described in this document
and primary key binding signatures made by algorithms described in
this document MUST NOT be made with `MD5`, `SHA-1`, or `RIPEMD-160`.
A receiving implementation MUST be treated such a signature as
invalid.

We already invalidate IDEA/3DES/... because we mandate the use of AES in the v1 SEIPD and v2 SEIPD requires a cipher that can be used in AEAD mode. We could add a paragraph for SED, that's already fulfilled from all implementations (no implementation burden)

Implementations MUST NOT use Symmetrically Encrypted Data packets (tag 9)
to encrypt data protected with the algorithms described in this document.

And that's it for me, no other specification.

TJ-91 commented 4 months ago

I created a PR in #113 that includes those two statements at hopefully the right places

kaie commented 4 months ago

Thank you for updating the spec with this recommendation.

I'd like to give feedback that the following sentence wasn't immediately clear to me.

Implementations MUST NOT use Symmetrically Encrypted Data packets (tag 9)
to encrypt data protected with the algorithms described in this document.

It confused me to read that algorithms from this document shouldn't be used for encryption.

Thanks to @vanitasvitae for explaining it to me: Your sentence forbids the use with the old non-integrity protected package, thereby requiring something better (such as the packet with tag 18).

You could consider to slightly tweak the sentence to make it clearer that you want to require the use of newer, encryption packet formats. Sorry for not pointing this out earlier.

wussler commented 4 months ago

I could propose

Implementations MUST NOT use the obsolete Symmetrically Encrypted Data packets (tag 9) to encrypt data protected with the algorithms described in this document.

kaie commented 4 months ago

I think adding the word "obsolete" as you propose would be very helpful!