S/MIME signature: "Content-Type: text/plain" added to message (39.0.0)

[dirksammel]

Hey,

I'm observing a weird behavior when signing with S/MIME since version 39.0.0: it seems like an additional "Content-Type: text/plain" is added to the message after signing it.

My code looks like this:

from cryptography import x509
from cryptography.hazmat.primitives import hashes, serialization
from cryptography.hazmat.primitives.serialization import pkcs7

with open("test.cert", "rb") as cc:
    cert = x509.load_pem_x509_certificate(cc.read())

with open("test.key", "rb") as ck:
    key = serialization.load_pem_private_key(ck.read(), None)

msg = "test"

options = [pkcs7.PKCS7Options.DetachedSignature, pkcs7.PKCS7Options.Text]

signed_msg = (
    pkcs7.PKCS7SignatureBuilder()
    .set_data(bytes(msg, "utf-8"))
    .add_signer(cert, key, hashes.SHA256())
    .sign(serialization.Encoding.SMIME, options)
)

with open("/tmp/msg.txt", "wb") as msg_file:
    msg_file.write(signed_msg)

With cryptography 38.0.4, the relevant part is

This is an S/MIME signed message

------A78EE028A05FC73322259A20ED9EAAAE
Content-Type: text/plain

test
------A78EE028A05FC73322259A20ED9EAAAE

and openssl smime -verify -in /tmp/msg.txt -noverify is successful.

With cryptography 39.0.0, I get

This is an S/MIME signed message

--===============0873037622973214044==
Content-Type: text/plain

Content-Type: text/plain

test
--===============0873037622973214044==

and the verification with openssl fails.
After manually removing the additional "Content-Type: text/plain" from the file, it succeeds again.

[reaperhulk]

Thanks for the report. This is our bug. In 39 we rewrote part of the S/MIME code and introduced this issue. We canonicalize the input in text mode (which consists of converting line endings to \r\n) and we add the text/plain value in the same function. This is useful since we need to sign over the content-type as well as the payload, but then we call another smime_encode function if you pass serialization.Encoding.SMIME, which duplicates it. The tests failed to catch it because they looked for the presence of text/plain and did not assert the number of them and our verification test passed too because we don't extract the signature test value from the output.

We'll get a fix up soon.

[alex]

I'm not sure I understand why our verification tests still pass here.

[reaperhulk]

They pass because we sign the right thing but embed the wrong thing into the larger smime payload and then the verify call doesn't use what's in the smime payload. The test is

cryptography/tests/hazmat/primitives/test_pkcs7.py

Lines 495 to 506 in d90ed2b

	assert b"text/plain" in sig_pem
	# When passing the Text option the header is prepended so the actual
	# signed data is this.
	signed_data = b"Content-Type: text/plain\r\n\r\nhello world"
	_pkcs7_verify(
	serialization.Encoding.SMIME,
	sig_pem,
	signed_data,
	[cert],
	options,
	backend,
	)

but it should be:

        assert sig_pem.count(b"text/plain") == 1
        # Parse the message to get the signed data, which is the
        # first payload in the message
        message = email.parser.BytesParser().parsebytes(sig_pem)
        signed_data = message.get_payload()[0].as_bytes()
        _pkcs7_verify(...)

Either of the changes to the test above would have caught this bug.

[alex]

Gotcha,thanks.

…

On Wed, Feb 15, 2023 at 7:15 PM Paul Kehrer ***@***.***> wrote: They pass because we sign the right thing but embed the wrong thing into the larger smime payload and then the verify call doesn't use what's in the smime payload. The test is https://github.com/pyca/cryptography/blob/d90ed2b2bc6d02720191d6449be098451677b3fd/tests/hazmat/primitives/test_pkcs7.py#L495-L506 but it should be: assert sig_pem.count(b"text/plain") == 1 # Parse the message to get the signed data, which is the # first payload in the message message = email.parser.BytesParser().parsebytes(sig_pem) signed_data = message.get_payload()[0].as_bytes() _pkcs7_verify(...) Either of the changes to the test above would have caught this bug. — Reply to this email directly, view it on GitHub <#8298 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAGBCYZE3NAQRXF4AFSVDWXVWT3ANCNFSM6AAAAAAU47XZFU> . You are receiving this because you commented.Message ID: ***@***.***>

-- All that is necessary for evil to succeed is for good people to do nothing.

[dirksammel]

@reaperhulk Thanks for the answer and explanation!