bug#67931: [PATCH] Use S/MIME key from content for mail signing via OpenSSL

[illia]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> I'm mostly trying to understand how broken this was, prior to this
> patch. Obviously there was the hard-coding of the key, the original
> issue. Has encryption been broken this whole time, too?

Encryption is working as intended, I haven't encountered any problems
with it yet.

> Encryption is a separate MML tag, right? And also a separate cert (the
> recipient's, not the user's). Why would additional certificates on your
> own certfile interfere with the process of encrypting to the user?

Actually, when signing and encrypting at the same time, both use a
single "signencrypt" tag. This is what mml-secure-message-encrypt-smime
outputs currently:

<#secure method=smime mode=signencrypt keyfile=keyfile.pem certfile=recip.gpg>

mml-parse-1 converts this into an alist, spliting "signencrypt" into two
separate "sign" and "encrypt" parameters.  These are then processed in
mml-generate-mime-1, which consults mml-signencrypt-style-alist if it
encounters both sign and encrypt in the same tag.

With my previous patch (6 May) reusing the certfile parameter, the tag
would include chain certificates as certfiles:

<#secure method=smime mode=signencrypt keyfile=keyfile.pem certfile=chain.pem 
certfile=recip.pem>

With the same alist is passed to both mml-smime-openssl-sign and
mml-smime-openssl-encrypt, this had the unintended effect of (1)
encrypting for chain.pem and (2) including recip{1,2}.pem in the message
when signing.

With the latest patch, the tag looks like this:

<#secure method=smime mode=signencrypt keyfile=keyfile.pem chainfile=chain.pem 
certfile=recip.pem>

As mml-smime-openssl-sign expects chainfiles, mml-smime-openssl-encrypt
expects certfiles, and they don't interfere with each other anymore.

> I'm not trying to be difficult, I'd just like to have a better grasp of
> what's going on here!

No worries, I appreciate the additional caution with security-sensitive
code.  Also that part of the code seems to have been a bit neglected.

Illia