rnpgp / rnp

RNP: high performance C++ OpenPGP library used by Mozilla Thunderbird
https://www.rnpgp.org
Other
201 stars 55 forks source link

RNP reports wrong lbits, invalid key, bad signature for key #1226

Closed kaie closed 4 years ago

kaie commented 4 years ago

Originally reported here: https://thunderbird.topicbox.com/groups/e2ee/T0eb771b4b83e55f4/possible-bug

I'll provide the reduced input below:

Fetch this key: https://keys.openpgp.org/search?q=8FA94E79AD6AB56EE38CE5CBAC46EFE6DE500B3E

I'll attach a msg.eml file and a msg.eml.asc file.

"gpg --verify msg.eml.asc" reports a good signature

"rnp --verify" reports:

[signature_validate() /home/user/github/rnp/src/lib/crypto/signatures.cpp:214] wrong lbits [signature_check() /home/user/github/rnp/src/librepgp/stream-sig.cpp:1051] invalid or untrusted key BAD signature made Sun Jul 26 11:49:59 2020 using RSA (Encrypt or Sign) key 969e018fde6cdca1 sub 2048/RSA (Encrypt or Sign) 969e018fde6cdca1 2009-11-12 [ESCA] 65008dc220aae2a2574d6cd5969e018fde6cdca1 Signature verification failure: 1 invalid signature(s), 0 unknown signature(s)

kaie commented 4 years ago

msg.eml.zip

nwalfield commented 4 years ago

I checked this against Sequoia and we also can't verify this message:

$ sq verify --sender-cert-file 8FA94E79AD6AB56EE38CE5CBAC46EFE6DE500B3E.asc msg.eml --detached msg.eml.asc 
Error verifying checksum from 6500 8DC2 20AA E2A2 574D  6CD5 969E 018F DE6C DCA1: Message has been manipulated
1 bad checksum.
Error: Verification failed

Looking at the signature:

$ sq packet dump msg.eml.asc 
Signature Packet, old CTB, 307 bytes
    Version: 4
    Type: Text
    Pk algo: RSA (Encrypt or Sign)
    Hash algo: SHA512
    Hashed area:
      Issuer Fingerprint: 6500 8DC2 20AA E2A2 574D  6CD5 969E 018F DE6C DCA1
      Signature creation time: 2020-07-26 09:49:59 UTC
    Unhashed area:
      Issuer: 969E 018F DE6C DCA1
    Digest prefix: 3717
    Level: 0 (signature over data)

It appears to be an ASCII signature, not a binary signature. According to the RFC, the line endings need to be normalized to crlf. msg.eml, however, just uses cr (0xa) as line endings. Using unix2dos to convert the line endings, Sequoia is then able to verify the signature:

$ unix2dos msg.eml
unix2dos: converting file msg.eml to DOS format...
$ sq verify --sender-cert-file 8FA94E79AD6AB56EE38CE5CBAC46EFE6DE500B3E.asc --detached msg.eml.asc  msg.eml 
Good signature from 969E 018F DE6C DCA1
1 good signature.

Perhaps rnp has the same problem.

ni4 commented 4 years ago

@kaie Thanks for reporting, and thanks for investigations, @nwalfield

Within rnp case goes little more forward:

I will provide a PR with fixes shortly.

nwalfield commented 4 years ago

Here's the output of sq inspect on the key:

$ sq inspect 8FA94E79AD6AB56EE38CE5CBAC46EFE6DE500B3E.asc 
8FA94E79AD6AB56EE38CE5CBAC46EFE6DE500B3E.asc: OpenPGP Certificate.

    Fingerprint: 8FA9 4E79 AD6A B56E E38C  E5CB AC46 EFE6 DE50 0B3E
Public-key algo: RSA (Encrypt or Sign)
Public-key size: 2048 bits
  Creation time: 2009-11-12 12:33:04 UTC
Expiration time: 2021-10-13 09:11:06 UTC (creation time + P4352DT74282S)
      Key flags: certification

         Subkey: 6500 8DC2 20AA E2A2 574D  6CD5 969E 018F DE6C DCA1
Public-key algo: RSA (Encrypt or Sign)
Public-key size: 2048 bits
  Creation time: 2009-11-12 13:15:07 UTC
Expiration time: 2021-10-13 09:24:17 UTC (creation time + P4352DT72550S)
      Key flags: signing

         Subkey: 2E0F 8C51 BC77 58A3 3795  79D9 26F7 563E 73A3 3BEE
Public-key algo: RSA (Encrypt or Sign)
Public-key size: 2048 bits
  Creation time: 2009-11-12 13:15:36 UTC
Expiration time: 2021-10-13 09:24:17 UTC (creation time + P4352DT72521S)
      Key flags: transport encryption, data-at-rest encryption

         UserID: Peter Lebbing <peter@digitalbrains.com>
teythoon commented 4 years ago

Test for linebreak normalization: https://tests.sequoia-pgp.org/#Detached_signatures__Linebreak_normalization

kaie commented 4 years ago

cross link: https://bugzilla.mozilla.org/show_bug.cgi?id=1657094

rrrooommmaaa commented 4 years ago

@ni4 Have you started working on this? I have "wrong lbits" error in some test on MSVC, e.g. test_stream_signatures. Do you want me to debug or will you?

ni4 commented 4 years ago

@rrrooommmaaa Wrong lbits error was fixed in #1227, this issues is not closed since I'm still working on the second part - #1228 There is a number of tests where wrong lbits messages should be displayed, since they validate malformed/modified signatures. Or do you have test_stream_signatures() test failing?

rrrooommmaaa commented 4 years ago

@ni4 yes, assert_rnp_success(signature_validate(&sig, pgp_key_get_material(key), &hash)); line 371 failing

ni4 commented 4 years ago

@rrrooommmaaa Then it should be definitely Windows-related issue, like something is not zeroed/incorrectly copied/whatever else. I don't have Windows environment set up right now, however if you'll have major problems debugging it - feel free to ask for help, I'll setup it.

rrrooommmaaa commented 4 years ago

@ni4 Exactly my problem. src/tests/data/test_stream_signatures/source.txt received extra CR when cloning from github because I omitted to issue git config --global core.autocrlf false

ni4 commented 4 years ago

@rrrooommmaaa Ah, I forgot about this case, got hit by the same while implementing support for MinGW. Actually it should be a bit different - some of signatures in test_stream_signatures are done in binary mode, so changing eol sequence in source.txt makes signature invalid.

rrrooommmaaa commented 4 years ago

@ni4 I can add a fix to replace \r\n with \n in failing tests (test_stream_signtaures etc.)

ni4 commented 4 years ago

@rrrooommmaaa The better solution would be to add .gitattributes with corresponding flags for the affected files.