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

[Illia Ostapyshyn]

Hi Stefan,

I've been investigating this issue a bit more and discovered bug#17780.  My
original patch basically reverts its "fix" ac1507a8b6 (which wasn't a proper
fix), and there is another issue present.  I'm sending a new patch that fixes
both issues for good.  To recap:

- When composing a message signed with S/MIME, the workflow is to insert a
  "sign tag" using `mml-secure-sign-smime'.  When using openssl (as per
  mml-smime-use), this will search `smime-keys' for the keyfile and certs
  corresponding to the message sender (From header) and generate a sign MML
  tag [1].  Then, just before the message is sent, `mml-generate-mime' parses
  the tag and converts it into an alist passed to `mml-smime-openssl-sign',
  which executes openssl with the respective arguments from the alist/mml tag.

- Prior to bug#17780 patch this process would use the right keyfile from
  smime-keys, but would ignore additional certificates to be included in the
  message (third member of `smime-keys' entry).  The generated MML tag did not
  include certfiles and `mml-smime-openssl-sign' did not have the logic to
  process these, even if they were included in the tag/received alist.

- The applied patch ac1507a8b6 just uses (cdar smime-keys), which now includes
  the certfiles, but always takes the first entry of `smime-keys'.  If the
  user has setup several entries, i.e., different keys for subsequent mail
  addresses, this results in wrong keyfile/certs being used.  This is
  bug#67931.

The new patch complements `mml-secure-sign-smime' to include certfiles in the
generated tag.  With this, certfiles appear in the alist for
`mml-smime-openssl-sign', which is modified to process these entries and
forward them to `smime-sign-buffer'.

It also fixes a typo in documentation of `smime-sign-region': caar is meant to
be cadr.

> Could you please provide a way to reproduce the issue that you're
> seeing?

Here's a way to reproduce this in emacs -Q:

1. Start composing a message from bar@localhost with

(progn
  (setq mml-smime-use 'openssl
        smime-keys '(("foo@localhost" "foo.pem" ("chain1foo.pem" 
"chain2foo.pem"))
                     ("bar@localhost" "bar.pem" ("chain1bar.pem" 
"chain2bar.pem"))
                     ("baz@localhost" "baz.pem" ("chain1baz.pem" 
"chain2baz.pem"))))
  (debug-on-entry #'smime-sign-buffer)
  (compose-mail "test@example.org" "#67931 reproducer" '((from . 
"bar@localhost"))))

2. Use `mml-secure-sign-smime' (C-c RET S s) to insert a tag on top of the
   message with the proper path for message sender bar@localhost:
   <#part sign=smime keyfile=bar.pem>

3. Use `message-send-and-exit` (C-c C-c) to trigger the breakpoint. This
   yields the following backtrace:

Debugger entered--entering a function:
* smime-sign-buffer(("foo.pem" ("chain1foo.pem" "chain2foo.pem")))
  mml-smime-openssl-sign((part (sign . "smime") (keyfile . "bar.pem") 
(tag-location . 202) (contents . "")))
  mml-smime-sign((part (sign . "smime") (keyfile . "bar.pem") (tag-location . 
202) (contents . "")))
  mml-smime-sign-buffer((part (sign . "smime") (keyfile . "bar.pem") 
(tag-location . 202) (contents . "")))
  mml-generate-mime-1((part (sign . "smime") (keyfile . "bar.pem") 
(tag-location . 202) (contents . "")))
  mml-generate-mime(nil nil)
  message-encode-message-body()
  message-send-mail(nil)
  message-send-via-mail(nil)
  message-send(nil)
  message-send-and-exit(nil)
  funcall-interactively(message-send-and-exit nil)
  command-execute(message-send-and-exit)

Here, `smime-sign-buffer' signs the buffer with foo.pem, which corresponds to
smime-keys entry for foo@localhost, not bar@localhost.  As I described, (cdar
smime-keys) on line 136 in mml-smime.el always uses the first entry of
`smime-keys' regardless of the tag parameters.

In theory, `mml-smime-openssl-sign' should not access `smime-keys' at all, as
the keyfile/certfiles selection is handled (including the removed error
message and customize call) during sign tag generation in
`mml-secure-sign-smime'.  Instead, `mml-smime-openssl-sign' should use the
information from the tag passed in the cont argument (seen in the backtrace).

This is the case with this patch.  With it applied, the behavior changes:

- In step 2, the inserted tag now includes all the certfiles:
  <#part sign=smime keyfile=bar.pem certfile=chain1bar.pem 
certfile=chain2bar.pem>

- In step 3, `smime-sign-buffer' receives proper keyfile and all certfiles.

* smime-sign-buffer(("bar.pem" ("chain1bar.pem" "chain2bar.pem")))
  mml-smime-openssl-sign((part (sign . "smime") (keyfile . "bar.pem") (certfile 
. "chain1bar.pem") (certfile . "chain2bar.pem") (tag-location . 202) (contents 
. "")))

I've also updated the MML definition in documentation, since certfile
parameter is now common to both sign and encrypt tags.  Regarding the remark
about multiple entries: this is not new and already the case when encrypting
for multiple recipients (try `mml-secure-encrypt-smime'), but IMHO worth
clarifying, in case users desire write MML tags manually.

[1] 
https://www.gnu.org/software/emacs/manual/html_node/emacs-mime/MML-Definition.html