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

error validating signature #939

Closed
Valodim opened this issue Aug 2, 2019 · 9 comments
Closed

error validating signature #939

Valodim opened this issue Aug 2, 2019 · 9 comments

Comments

@Valodim
Copy link

Valodim commented Aug 2, 2019

Hey there,

I ran into a case where a signature won't verify, after decryption. I tried to minimize it:

https://gist.github.com/Valodim/59ae7c8a54abd7746412ae114c333721

This was run with npm install openpgp, node testcase.js, and outputs as annotated. The same test case works with GnuPG. Since this message is encrypted and decrypts correctly, but the signature neither verifies directly, nor when calling verify() on signature + message data afterwards.

I tried to make the test case self-contained. The encrypted message was generated with OpenKeychain and K-9. Let me know if you need anything else.

@chesnokovilya
Copy link
Contributor

I could confirm that test case is correct. It seems that this is related to #854

@chesnokovilya
Copy link
Contributor

chesnokovilya commented Aug 5, 2019

Which is related to the bug in https://github.com/indutny/elliptic library.

@chesnokovilya
Copy link
Contributor

chesnokovilya commented Aug 6, 2019

Upon further investigation: signature packet contains according to rfc 4880:
Two-octet field holding the left 16 bits of the signed hash value.

In the message those two bytes are:
cf 83
which are incorrect values. It is hash of empty message.
openssl dgst -sha512 empty.txt

So the problem here is incorrect encoding of the signature in the message.

@twiss
Copy link
Member

twiss commented Aug 6, 2019

@chesnokovilya Thanks for investigating!

@Valodim Just to be explicit, this means there's probably a bug in OpenKeychain, perhaps you could open an issue with them?

(I understand that GPG does verify the signature; OpenPGP.js is apparently more strict in this regard. Nevertheless, unless this issue is widespread, I'd prefer to fix this issue in the software that generated the signature, if at all possible.)

@twiss twiss closed this as completed Aug 6, 2019
@dkg
Copy link
Contributor

dkg commented Aug 27, 2019

Over on https://gist.github.com/59ae7c8a54abd7746412ae114c333721#gistcomment-3005648 , @Valodim says:

This turned out to be an issue in OpenKeychain, which for P-256 signatures didn't include the correct checksum in the signature packet. Those are not cryptograhpically relevant and GnuPG doesn't mind, but openpgp.js rejects them.

I don't know whether openpgp.js wants to reconsider its rejection of a signature that it would otherwise be able to validate.

@Valodim
Copy link
Author

Valodim commented Aug 27, 2019

I agree with @dkg here. The checksum has no bearing cryptographically, so I don't see any argument for checking it or even processing it at all. This is similar to the useless checksum in ascii armor, which is also ignored by most implementations at this point from what I can tell.

@tomholub
Copy link
Contributor

Are OpenKeychain folks not fixing this? There's no downside to following the spec on their end.

@Valodim
Copy link
Author

Valodim commented Aug 28, 2019

I'm a bit short on time and OpenKeychain is kind of swapped out at the moment, but sure, we plan to fix it. But that won't fix the signatures with this problem which are out there already.

@twiss
Copy link
Member

twiss commented Aug 28, 2019

I don't know whether openpgp.js wants to reconsider its rejection of a signature that it would otherwise be able to validate.

Yeah, we might reconsider at some point. That said,

I don't see any argument for checking it or even processing it at all.

Comparing two bytes is faster than verifying a signature. In a spec-conformant signature packet, if the bytes don't match, the signature won't verify either, hence why we don't try to, currently.

Another argument is that having at least one implementation that checks this, even if most don't, means that bugs in the generation in other implementations get caught, as happened here. Then, if one day someone has a usecase where quickly rejecting invalid signatures is important, there's a higher chance that these checksums work.

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

No branches or pull requests

5 participants