1953402 - testGitCommitSpoof verifies valid cleartext signed message, expects failure

Status: UNCONFIRMED

(MailNews Core :: Security: OpenPGP, defect)

Description

[Justus Winter]

Attachment: rnp_op_verify_input.as-is

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:128.0) Gecko/20100101 Firefox/128.0

Steps to reproduce:

testGitCommitSpoof verifies valid cleartext signed message, and expects failure from the OpenPGP implementation. The comment in the test says:

// Mistmatch (at least) due to the fact that content had to be modified
// for it to mimic a message.

However, the message has not been modified as can easily be checked by verifying the signature. I have attached both the message as given to rnp_op_verify_create, and the certificate to verify the message. Consider:

% sq verify --signer-file data/keys/benny-hashwell-0xc55ccd9c5ab482b2-pub.asc --message </tmp/rnp_op_verify_input.as-is
Authenticated signature made by 123B9416B238CA89A23A554DC55CCD9C5AB482B2 (Benny Hashwell
<[e-mail]> (UNAUTHENTICATED))

tree 870fc0aaa5383d845fa9431c9d08cfd486e11a65
parent 6f85fa977ab95931222920a2cc48c81200e9ad48
author Benny Hashwell <[e-mail]> 1697890465 +0000
committer Benny Hashwell <[e-mail]> 1697890465 +0000

Hi,

sad to say that we had to cut our expenses and dispense of your services.
Hope you find something good elsewhere!

Yours,
Benny
1 authenticated signature.

The message is using the cleartext signature framework. It is ever so slightly out of spec:

• It doesn't have a 'Hash' header. But, an attacker can add that easily.
• It ends in two newlines, which may derail implementations that reject any leading or trailing texts. But, an attacker can easily strip that extra newline, I think.
• It is a binary signature, which is a bit odd for a CSF message, but RFC4880 doesn't mandate that text signatures are used with CSF with a single word, and RFC9580 seems to merely suggest it.

So I don't understand the tests expectation. Clearly, contrary what the comment in the test claims, the content has not been modified in a meaningful way.

Actual results:

Running the test with the Sequoia Octopus for Thunderbird fails, because we report a valid signature.

Expected results:

The test should succeed. Alternatively, the test should be changed to actually modify the content in a meaningful way.

Comment 1

[Justus Winter]

Attachment: benny-hashwell-0xc55ccd9c5ab482b2-pub.asc

Comment 2

[Magnus Melin [:mkmelin]]

Hmm, that comment may be off. For reference, this was bug 1862625 (I'll cc you on that.)

Comment 3

[Justus Winter]

As Marcus correctly observes, there is no context separation for signatures in OpenPGP (well, you can do it using separate signing subkeys which are marked for just one use case). Therefore, for better or worse, OpenPGP signatures can be re-interpreted in different context.

I don't understand how the bug was fixed. All the referenced patches seem to tweak the MIME handling, I haven't seen anything related to the OpenPGP implementation or glue at all. Can you please point me to the OpenPGP part of the fix for the referenced bug?

I can imagine several reasons to reject the message:

• It looks like a OpenPGP/MIME encrypted message, but doesn't use an encryption container.
• It embeds a message using the cleartext signature framework in a MIME message.

Neither of these seems to be checked, or else I wouldn't see the issue with our implementation.

I don't know on what terms RNP rejects the signature on the CSF message embedded in the message, but the tests seems to overfit on RNP, possibly merely masking the problem without having it addressed properly.

I don't understand why the test vector uses CSF in the first place. And, in fact, Marcus' original test vector did not! Instead, Marcus used an old-style inline-signed message:

teythoon@europ ~ % cat downloads/sigspoof-git-commit.eml
Date: Sat, 21 Oct 2023 23:55:39 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
From: Benny Hashwell <[e-mail]>
To: Alice Scrambler <[e-mail]>
Subject: You are fired!
Content-Type: multipart/encrypted;
protocol="application/pgp-encrypted";
boundary="------------lMWTRZPG9u87fW0tsiWBI13c"

This is an OpenPGP/MIME encrypted message (RFC 4880 and 3156)
--------------lMWTRZPG9u87fW0tsiWBI13c
Content-Type: application/pgp-encrypted
Content-Description: PGP/MIME version identification

Version: 1

--------------lMWTRZPG9u87fW0tsiWBI13c
Content-Type: application/octet-stream; name="encrypted.asc"
Content-Description: OpenPGP encrypted message
Content-Disposition: inline; filename="encrypted.asc"

-----BEGIN PGP MESSAGE-----

iQHPBAABCgA5FiEEEjuUFrI4yomiOlVNxVzNnFq0grIFAmUzwLAbHGJlbm55Lmhh
c2h3ZWxsQGV4YW1wbGUuY29tAAoJEMVczZxatIKynEcL/1ao7RzOEow9VR+KaADw
AQ7e2a1RWxoraR27aUAqhEfpmfV4RHgTWJAz5qJtBSGkxkXKqKvArTv5qjgTl3Kq
iMd6szwfUHbyeX4qN3xs6luNzzx/dZtW9ls1ZWh4zgBrnQd7XFi+ypWvMsTHnS/c
VINejs54OW7CChOclZifkBNnV0Px+MNdcJqgf/FmOqc0Wt3IL/5wDkKh6xNyeXtw
lE4tcDErhZBx4Gfaurj+1/j4DKMj4VoVAj4ab7UQlIQWTswiJfCR6CML2yAz+Bi5
iU6uF4N6IfkxTKTsIgy3YuADKeE7lF+so/ofB+vtRJCq53i5HvDoyXMwm7tobdYz
/MFg8ILp2dH0XBJZ5rQHX+IElRvTLys3xCqQrQrpO1Aa2vO3kWi4x9a3ocGbqsy7
HVjaAQ+3A688YrTwjTtB9JaUM2QZYEnig5C3sN8L4ISRviI37y7XSMyblu5FCjzo
f2k3o03R9sD/WouRqu572SFaItAOWExLy+fmK+yFZ5QkWq0BgmINcGxhaW50ZXh0
LnJhd2U0RBN0cmVlIDg3MGZjMGFhYTUzODNkODQ1ZmE5NDMxYzlkMDhjZmQ0ODZl
MTFhNjUKcGFyZW50IDZmODVmYTk3N2FiOTU5MzEyMjI5MjBhMmNjNDhjODEyMDBl
OWFkNDgKYXV0aG9yIEJlbm55IEhhc2h3ZWxsIDxiZW5ueS5oYXNod2VsbEBleGFt
cGxlLmNvbT4gMTY5Nzg5MDQ2NSArMDAwMApjb21taXR0ZXIgQmVubnkgSGFzaHdl
bGwgPGJlbm55Lmhhc2h3ZWxsQGV4YW1wbGUuY29tPiAxNjk3ODkwNDY1ICswMDAw
CgpIaSwKCnNhZCB0byBzYXkgdGhhdCB3ZSBoYWQgdG8gY3V0IG91ciBleHBlbnNl
cyBhbmQgZGlzcGVuc2Ugb2YgeW91ciBzZXJ2aWNlcy4KSG9wZSB5b3UgZmluZCBz
b21ldGhpbmcgZ29vZCBlbHNld2hlcmUhCgpZb3VycywKQmVubnkK
=OOlJ
-----END PGP MESSAGE-----

--------------lMWTRZPG9u87fW0tsiWBI13c--
teythoon@europ ~ % sq packet dump downloads/sigspoof-git-commit.eml
Signature Packet, old CTB, 463 bytes
Version: 4
Type: Binary
Pk algo: RSA
Hash algo: SHA512
Hashed area:
Issuer Fingerprint: 123B9416B238CA89A23A554DC55CCD9C5AB482B2
Signature creation time: 2023-10-21 12:14:40 UTC
Signer's User ID: [e-mail]
Unhashed area:
Issuer: C55CCD9C5AB482B2
Digest prefix: 9C47
Level: 0 (signature over data)

Literal Data Packet, old CTB, 386 bytes
Format: Binary data
Filename: plaintext.raw
Timestamp: 2023-10-21 21:35:15 UTC
Content: tree 870fc0aaa5383d845fa9431c9d08cfd486e...

Further, I don't exactly understand what problem you are trying to address, and how. Barring any mechanism of providing a signing context, it will always be possible to create a (relatively) well-formed, modern OpenPGP MIME signed message, and it will be always possible to create an CSF message.

Comment 4

[Magnus Melin [:mkmelin]]

If I understand/remember correctly the original bug was fixed by making the initial tree/parent and following lines visible but keeping the signature check as it was. I had assumed the check was failing since we have to modify the mime slightly to do so.

Comment 5

[Justus Winter]

I did some tracing through RNP trying to verify the artifact. RNP rejects it for two reasons:

• The "Hash: SHA512" header is missing. AFAICS an attacker could add that easily.
• When RNP verifies a message using the cleartext signature framework (those with "-----BEGIN PGP SIGNED MESSAGE-----" and human-readable text), it unconditionally canonicalizes line endings no \r\n when signing, regardless of whether the signature is marked as binary (it is in the test vector), or text.
  • There are some ways "around" this. For example, the message could simply not have used the CSF framework, but instead used an inline-signed message (incidentally, that is what Marcus' original test vector seems to do). Or, the source signature could have been a text signature in the first place.

So your test seems to boil down to "Does my PGP implementation reject a message using the cleartext signature framework that uses a binary signature?". Does that match your expectations, and does that provide the protection that you had hoped to implement?