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

PR ready: fix signature version vs. key version mix-up during signature generation #74

Closed
falko-strenzke opened this issue Jul 23, 2024 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@falko-strenzke
Copy link
Contributor

falko-strenzke commented Jul 23, 2024

Justus Winter reported on the mailinglist:

RFC4880, Section 5.2.4. Computing Signatures says:

When a signature is made over a key, the hash data starts with the
octet 0x99, followed by a two-octet length of the key, and then body
of the key packet. (Note that this is an old-style packet header for
a key packet with two-octet length.)

Almost-RFC9580, Section 5.2.4. Computing Signatures says:

When a version 4 signature is made over a key, the hash data starts
with the octet 0x99, followed by a 2-octet length of the key,
followed by the body of the key packet. When a version 6 signature is
made over a key, the hash data starts with the salt and then octet
0x9B, followed by a 4-octet length of the key, followed by the body
of the key packet.

Note how the key packet's hashed data changes based not on the version
of the key, but on the version of the signature being generated. I find
that very surprising, and in my v6 implementation I promptly did it
"wrong". Today, I surveyed all other WIP v6 implementations, and found
it is split 50/50:

"Wrong":

https://gitlab.com/sequoia-pgp/sequoia/-/blob/crypto-refresh/openpgp/src/crypto/hash.rs?ref_type=heads#L439
https://github.com/rnpgp/rnp/blob/main/src/librepgp/stream-sig.cpp#L59
https://github.com/ProtonMail/go-crypto/blob/main/openpgp/packet/public_key.go#L585

"Right":

https://github.com/openpgpjs/openpgpjs/blob/v6/src/packet/signature.js#L691
https://github.com/bcgit/bc-java/pull/1725/files#diff-9fd07bf1faf3282efab567e3fea5e5828b88069c05578f622fffeca13a69d2cc
https://github.com/dkg/PGPy/blob/dkg/crypto-refresh/pgpy/pgp.py#L479

Justus ask if the C-R should be treated as buggy in this point, so we should hold this until that is clarified among the implementers.

@TJ-91
Copy link
Collaborator

TJ-91 commented Aug 15, 2024

PR to address this in RNP: rnpgp/rnp#2261

@TJ-91 TJ-91 added the bug Something isn't working label Aug 20, 2024
@falko-strenzke falko-strenzke changed the title fix signature version vs. key version mix-up during signature generation PR ready: fix signature version vs. key version mix-up during signature generation Aug 21, 2024
@TJ-91
Copy link
Collaborator

TJ-91 commented Nov 4, 2024

merged

@TJ-91 TJ-91 closed this as completed Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants