unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Sebastian Fieber <sebastian.fieber@web.de>
To: Lars Ingebrigtsen <larsi@gnus.org>
Cc: 40397@debbugs.gnu.org
Subject: bug#40397: 28.0.50; epg decrypt does not verify signed content in smime encrypted and signed message
Date: Thu, 23 Dec 2021 19:25:17 +0100	[thread overview]
Message-ID: <87czln6uwi.fsf@web.de> (raw)
In-Reply-To: <87h7az6v9j.fsf@web.de> (Sebastian Fieber's message of "Thu, 23 Dec 2021 19:17:28 +0100")

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: multipart/mixed; boundary="=-=-=", Size: 16519 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

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


On Do, Dez 23 2021, Sebastian Fieber <sebastian.fieber@web.de> wrote:

> On Do, Dez 23 2021, Sebastian Fieber <sebastian.fieber@web.de> wrote:
>
>> This one should apply :)
>
> Wait, this was the wrong one.  I'll send the right one during the day!

And here is the right one.


- --=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment; filename=0001-PATCH-fix-bug-40397.patch
Content-Transfer-Encoding: quoted-printable

From=2084ebb0331a0e16b1b767483c9d0bd1c140d73f09 Mon Sep 17 00:00:00 2001
From: Sebastian Fieber <sebastian.fieber@web.de>
Date: Thu, 23 Dec 2021 15:38:09 +0100
Subject: [PATCH] [PATCH] fix bug #40397

This fixes S/MIME encrypted AND signed mails where in the encrypted
pkcs7 envelope is a signed pkcs7 structure.

Also this patch enables proper security-buttons for pkcs7-mime
encrypted and/or signed mails.

Changes:
=2D structure the result of mm-dissect-buffer of application/pkcs7-mime
  like a multipart mail so there is no loosing of information of
  verification and decryption results which can now be displayed by
  gnus-mime-display-security

=2D adjust gnus-mime-display-part to handle application/pkcs7-mime like
  multipart/encrypted or multipart/signed

=2D add dummy entries to mm-verify-function-alist and
  mm-decrypt-function-alist so gnus-mime-display-security correctly
  displays "S/MIME" and not "unknown protocol"

=2D don't just check for multipart/signed in
  gnus-insert-mime-security-button but also for the pkcs7-mime mimetypes
  to print "Encrypted" or "Signed" accordingly in the security button

=2D adjust mm-possibly-verify-or-decrypt to check for smime-type to ask
  wether to verify or decrypt the part and not to always ask to decrypt

=2D adjust mm-view-pkcs7-decrypt and verify to call mm-sec-status so
  success information can be displayed by gnus-mime-display-security

=2D adjust gnus-mime-security-verify-or-decrypt to handle pkcs7-mime
  right with the done changes
=2D--
 lisp/gnus/gnus-art.el  |  78 ++++++++++++++++++++-----
 lisp/gnus/mm-decode.el | 128 +++++++++++++++++++++++++----------------
 lisp/gnus/mm-view.el   |  13 +++--
 3 files changed, 149 insertions(+), 70 deletions(-)

diff --git a/lisp/gnus/gnus-art.el b/lisp/gnus/gnus-art.el
index b7701f10a5..a83f4b7d59 100644
=2D-- a/lisp/gnus/gnus-art.el
+++ b/lisp/gnus/gnus-art.el
@@ -6084,6 +6084,34 @@ gnus-mime-display-part
    ((equal (car handle) "multipart/encrypted")
     (gnus-add-wash-type 'encrypted)
     (gnus-mime-display-security handle))
+   ;; pkcs7-mime handling:
+   ;;
+   ;; although not really multipart these are structured internally by
+   ;; mm-dissect-buffer like multipart to not discard the decryption
+   ;; and verification results
+   ;;
+   ;; application/pkcs7-mime
+   ((and (equal (car handle) "application/pkcs7-mime")
+         (equal (mm-handle-multipart-ctl-parameter handle 'protocol)
+                "application/pkcs7-mime_signed-data"))
+    (gnus-add-wash-type 'signed)
+    (gnus-mime-display-security handle))
+   ((and (equal (car handle) "application/pkcs7-mime")
+         (equal (mm-handle-multipart-ctl-parameter handle 'protocol)
+                "application/pkcs7-mime_enveloped-data"))
+    (gnus-add-wash-type 'encrypted)
+    (gnus-mime-display-security handle))
+   ;; application/x-pkcs7-mime
+   ((and (equal (car handle) "application/x-pkcs7-mime")
+         (equal (mm-handle-multipart-ctl-parameter handle 'protocol)
+                "application/x-pkcs7-mime_signed-data"))
+    (gnus-add-wash-type 'signed)
+    (gnus-mime-display-security handle))
+   ((and (equal (car handle) "application/x-pkcs7-mime")
+         (equal (mm-handle-multipart-ctl-parameter handle 'protocol)
+                "application/x-pkcs7-mime_enveloped-data"))
+    (gnus-add-wash-type 'encrypted)
+    (gnus-mime-display-security handle))
    ;; Other multiparts are handled like multipart/mixed.
    (t
     (gnus-mime-display-mixed (cdr handle)))))
@@ -8833,11 +8861,18 @@ gnus-mime-security-verify-or-decrypt
     (setq point (point))
     (with-current-buffer (mm-handle-multipart-original-buffer handle)
       (let* ((mm-verify-option 'known)
=2D	     (mm-decrypt-option 'known)
=2D	     (nparts (mm-possibly-verify-or-decrypt (cdr handle) handle)))
=2D	(unless (eq nparts (cdr handle))
=2D	  (mm-destroy-parts (cdr handle))
=2D	  (setcdr handle nparts))))
+            (mm-decrypt-option 'known)
+             (pkcs7-mime-p (or (equal (car handle) "application/pkcs7-mime=
")
+                               (equal (car handle) "application/x-pkcs7-mi=
me")))
+            (nparts (if pkcs7-mime-p
+                         (list (mm-possibly-verify-or-decrypt (cadr handle=
) (cadadr handle)))
+                       (mm-possibly-verify-or-decrypt (cdr handle) handle)=
)))
+       (unless (eq nparts (cdr handle))
+          ;; if pkcs7-mime don't destroy the parts as the buffer in
+          ;; the cdr still needs to be accessible
+          (when (not pkcs7-mime-p)
+           (mm-destroy-parts (cdr handle)))
+         (setcdr handle nparts))))
     (gnus-mime-display-security handle)
     (when region
       (delete-region (point) (cdr region))
@@ -8891,14 +8926,31 @@ gnus-insert-mime-security-button
   (let* ((protocol (mm-handle-multipart-ctl-parameter handle 'protocol))
 	 (gnus-tmp-type
 	  (concat
=2D	   (or (nth 2 (assoc protocol mm-verify-function-alist))
=2D	       (nth 2 (assoc protocol mm-decrypt-function-alist))
=2D	       "Unknown")
=2D	   (if (equal (car handle) "multipart/signed")
=2D	       " Signed" " Encrypted")
=2D	   " Part"))
=2D	 (gnus-tmp-info
=2D	  (or (mm-handle-multipart-ctl-parameter handle 'gnus-info)
+          (or (nth 2 (assoc protocol mm-verify-function-alist))
+              (nth 2 (assoc protocol mm-decrypt-function-alist))
+              "Unknown")
+          (cond ((equal (car handle) "multipart/signed") " Signed")
+                ((equal (car handle) "multipart/encrypted") " Encrypted")
+                 ((and (equal (car handle) "application/pkcs7-mime")
+                       (equal (mm-handle-multipart-ctl-parameter handle 'p=
rotocol)
+                              "application/pkcs7-mime_signed-data"))
+                  " Signed")
+                 ((and (equal (car handle) "application/pkcs7-mime")
+                       (equal (mm-handle-multipart-ctl-parameter handle 'p=
rotocol)
+                              "application/pkcs7-mime_enveloped-data"))
+                  " Encrypted")
+                 ;; application/x-pkcs7-mime
+                 ((and (equal (car handle) "application/x-pkcs7-mime")
+                       (equal (mm-handle-multipart-ctl-parameter handle 'p=
rotocol)
+                              "application/x-pkcs7-mime_signed-data"))
+                  " Signed")
+                 ((and (equal (car handle) "application/x-pkcs7-mime")
+                       (equal (mm-handle-multipart-ctl-parameter handle 'p=
rotocol)
+                              "application/x-pkcs7-mime_enveloped-data"))
+                  " Encrypted"))
+          " Part"))
+        (gnus-tmp-info
+         (or (mm-handle-multipart-ctl-parameter handle 'gnus-info)
 	      "Undecided"))
 	 (gnus-tmp-details
 	  (mm-handle-multipart-ctl-parameter handle 'gnus-details))
diff --git a/lisp/gnus/mm-decode.el b/lisp/gnus/mm-decode.el
index d781407cdc..8d63c8552f 100644
=2D-- a/lisp/gnus/mm-decode.el
+++ b/lisp/gnus/mm-decode.el
@@ -474,6 +474,7 @@ mm-dissect-default-type
 (autoload 'mml2015-verify-test "mml2015")
 (autoload 'mml-smime-verify "mml-smime")
 (autoload 'mml-smime-verify-test "mml-smime")
+(autoload 'mm-view-pkcs7-verify "mm-view")
=20
 (defvar mm-verify-function-alist
   '(("application/pgp-signature" mml2015-verify "PGP" mml2015-verify-test)
@@ -482,7 +483,15 @@ mm-verify-function-alist
     ("application/pkcs7-signature" mml-smime-verify "S/MIME"
      mml-smime-verify-test)
     ("application/x-pkcs7-signature" mml-smime-verify "S/MIME"
=2D     mml-smime-verify-test)))
+     mml-smime-verify-test)
+    ("application/x-pkcs7-signature" mml-smime-verify "S/MIME"
+     mml-smime-verify-test)
+    ;; these are only used for security-buttons and contain the
+    ;; smime-type after the underscore
+    ("application/pkcs7-mime_signed-data" mm-view-pkcs7-verify "S/MIME"
+     nil)
+    ("application/x-pkcs7-mime_signed-data" mml-view-pkcs7-verify "S/MIME"
+     nil)))
=20
 (defcustom mm-verify-option 'never
   "Option of verifying signed parts.
@@ -501,11 +510,16 @@ mm-verify-option
=20
 (autoload 'mml2015-decrypt "mml2015")
 (autoload 'mml2015-decrypt-test "mml2015")
+(autoload 'mm-view-pkcs7-decrypt "mm-view")
=20
 (defvar mm-decrypt-function-alist
   '(("application/pgp-encrypted" mml2015-decrypt "PGP" mml2015-decrypt-tes=
t)
     ("application/x-gnus-pgp-encrypted" mm-uu-pgp-encrypted-extract-1 "PGP"
=2D     mm-uu-pgp-encrypted-test)))
+     mm-uu-pgp-encrypted-test)
+    ;; these are only used for security-buttons and contain the
+    ;; smime-type after the underscore
+    ("application/pkcs7-mime_enveloped-data" mm-view-pkcs7-decrypt "S/MIME=
" nil)
+    ("application/x-pkcs7-mime_enveloped-data" mm-view-pkcs7-decrypt "S/MI=
ME" nil)))
=20
 (defcustom mm-decrypt-option nil
   "Option of decrypting encrypted parts.
@@ -682,18 +696,33 @@ mm-dissect-buffer
 					'start start)
 				  (car ctl))
 	     (cons (car ctl) (mm-dissect-multipart ctl from))))
=2D	  (t
=2D	   (mm-possibly-verify-or-decrypt
=2D	    (mm-dissect-singlepart
=2D	     ctl
=2D	     (and cte (intern (downcase (mail-header-strip-cte cte))))
=2D	     no-strict-mime
=2D	     (and cd (mail-header-parse-content-disposition cd))
=2D	     description id)
=2D	    ctl from))))
=2D	(when id
=2D	  (when (string-match " *<\\(.*\\)> *" id)
=2D	    (setq id (match-string 1 id)))
+          (t
+           (let* ((handle
+                   (mm-dissect-singlepart
+                    ctl
+                    (and cte (intern (downcase (mail-header-strip-cte cte)=
)))
+                    no-strict-mime
+                    (and cd (mail-header-parse-content-disposition cd))
+                    description id))
+                  (intermediate-result (mm-possibly-verify-or-decrypt hand=
le ctl from)))
+             (when (and (equal type "application")
+                        (or (equal subtype "pkcs7-mime")
+                            (equal subtype "x-pkcs7-mime")))
+               (add-text-properties 0
+                                    (length (car ctl))
+                                    (list 'protocol
+                                          (concat (substring-no-properties=
 (car ctl))
+                                                  "_"
+                                                  (cdr (assoc 'smime-type =
ctl))))
+                                    (car ctl))
+               ;; if this is a pkcs7-mime lets treat this special and
+               ;; more like multipart so the pkcs7-mime part does not
+               ;; get ignored
+               (setq intermediate-result (cons (car ctl) (list intermediat=
e-result))))
+             intermediate-result))))
+        (when id
+          (when (string-match " *<\\(.*\\)> *" id)
+            (setq id (match-string 1 id)))
 	  (push (cons id result) mm-content-id-alist))
 	result))))
=20
@@ -1677,43 +1706,40 @@ mm-possibly-verify-or-decrypt
     (cond
      ((or (equal type "application/x-pkcs7-mime")
 	  (equal type "application/pkcs7-mime"))
=2D      (with-temp-buffer
=2D	(when (and (cond
=2D		    ((equal smime-type "signed-data") t)
=2D		    ((eq mm-decrypt-option 'never) nil)
=2D		    ((eq mm-decrypt-option 'always) t)
=2D		    ((eq mm-decrypt-option 'known) t)
=2D		    (t (y-or-n-p "Decrypt (S/MIME) part? ")))
=2D		   (mm-view-pkcs7 parts from))
=2D	  (goto-char (point-min))
=2D	  ;; The encrypted document is a MIME part, and may use either
=2D	  ;; CRLF (Outlook and the like) or newlines for end-of-line
=2D	  ;; markers.  Translate from CRLF.
=2D	  (while (search-forward "\r\n" nil t)
=2D	    (replace-match "\n"))
=2D	  ;; Normally there will be a Content-type header here, but
=2D	  ;; some mailers don't add that to the encrypted part, which
=2D	  ;; makes the subsequent re-dissection fail here.
=2D	  (save-restriction
=2D	    (mail-narrow-to-head)
=2D	    (unless (mail-fetch-field "content-type")
=2D	      (goto-char (point-max))
=2D	      (insert "Content-type: text/plain\n\n")))
=2D	  (setq parts
=2D		(if (equal smime-type "signed-data")
=2D		    (list (propertize
=2D			   "multipart/signed"
=2D			   'protocol "application/pkcs7-signature"
=2D			   'gnus-info
=2D			   (format
=2D			    "%s:%s"
=2D			    (get-text-property 0 'gnus-info
=2D					       (car mm-security-handle))
=2D			    (get-text-property 0 'gnus-details
=2D					       (car mm-security-handle))))
=2D			  (mm-dissect-buffer t)
=2D			  parts)
=2D		  (mm-dissect-buffer t))))))
+      (add-text-properties 0 (length (car ctl))
+                           (list 'buffer (car parts))
+                           (car ctl))
+      (let* ((envelope-p (string=3D smime-type "enveloped-data"))
+             (decrypt-or-verify-option (if envelope-p
+                                           mm-decrypt-option
+                                         mm-verify-option))
+             (question (if envelope-p
+                           "Decrypt (S/MIME) part? "
+                         "Verify signed (S/MIME) part? ")))
+        (with-temp-buffer
+	  (when (and (cond
+		      ((equal smime-type "signed-data") t)
+		      ((eq decrypt-or-verify-option 'never) nil)
+		      ((eq decrypt-or-verify-option 'always) t)
+		      ((eq decrypt-or-verify-option 'known) t)
+		      (t (y-or-n-p (format question))))
+                     (mm-view-pkcs7 parts from))
+
+	    (goto-char (point-min))
+	    ;; The encrypted document is a MIME part, and may use either
+	    ;; CRLF (Outlook and the like) or newlines for end-of-line
+	    ;; markers.  Translate from CRLF.
+	    (while (search-forward "\r\n" nil t)
+	      (replace-match "\n"))
+	    ;; Normally there will be a Content-type header here, but
+	    ;; some mailers don't add that to the encrypted part, which
+	    ;; makes the subsequent re-dissection fail here.
+	    (save-restriction
+	      (mail-narrow-to-head)
+	      (unless (mail-fetch-field "content-type")
+	        (goto-char (point-max))
+	        (insert "Content-type: text/plain\n\n")))
+	    (setq parts (mm-dissect-buffer t))))))
      ((equal subtype "signed")
       (unless (and (setq protocol
 			 (mm-handle-multipart-ctl-parameter ctl 'protocol))
diff --git a/lisp/gnus/mm-view.el b/lisp/gnus/mm-view.el
index d2a6d2cf5d..319bc745ff 100644
=2D-- a/lisp/gnus/mm-view.el
+++ b/lisp/gnus/mm-view.el
@@ -634,12 +634,9 @@ mm-view-pkcs7-verify
 		 (context (epg-make-context 'CMS)))
 	     (prog1
 		 (epg-verify-string context part)
=2D	       (let ((result (car (epg-context-result-for context 'verify))))
+	       (let ((result (epg-context-result-for context 'verify)))
 		 (mm-sec-status
=2D		  'gnus-info (epg-signature-status result)
=2D		  'gnus-details
=2D		  (format "%s:%s" (epg-signature-validity result)
=2D			  (epg-signature-key-id result))))))))
+		  'gnus-info (epg-verify-result-to-string result)))))))
       (with-temp-buffer
 	(insert "MIME-Version: 1.0\n")
 	(mm-insert-headers "application/pkcs7-mime" "base64" "smime.p7m")
@@ -659,7 +656,11 @@ mm-view-pkcs7-decrypt
       ;; Use EPG/gpgsm
       (let ((part (base64-decode-string (buffer-string))))
 	(erase-buffer)
=2D	(insert (epg-decrypt-string (epg-make-context 'CMS) part)))
+	(insert
+         (let ((context (epg-make-context 'CMS)))
+           (prog1
+               (epg-decrypt-string context part)
+             (mm-sec-status 'gnus-info "OK")))))
     ;; Use openssl
     (insert "MIME-Version: 1.0\n")
     (mm-insert-headers "application/pkcs7-mime" "base64" "smime.p7m")
=2D-=20
2.20.1


- --=-=-=--
-----BEGIN PGP SIGNATURE-----

iQFMBAEBCAA2FiEExRi5b+8xM5Vpvu7L3jJw+EOyhogFAmHEvw0YHHNlYmFzdGlh
bi5maWViZXJAd2ViLmRlAAoJEN4ycPhDsoaI06wIAK8rjUKQBCWdwEdAlFzrIOym
mwjjFmSlrKefWJskVdcAO/Ve5EL905kR58LrlIUnZL0jzdqmN6NbLuDWJysDKRua
OX+oMIPEWzfH0NKiiefMHBPSnEJb75xhICZQcye4F7YsSN9gp0SzZqolCkG6RG2g
y8N7AALsconk17JH+FpJyZ+J5lg3CQbz6kSAcnW1gKM79OkGkDXi5K1IusZ7b7MR
fQfOD1EKGNiFo4mQsix6NLrdpvRM2MyO0J2YRaemyiEJOmaViP2JAIYOwdd6P9kA
HdQ41YmGXqWTvvDv6l7AYIjIZftlXKOg1xoeJzb3ARRFChbok72SgEDu4ffD0C8=
=q7aE
-----END PGP SIGNATURE-----






  reply	other threads:[~2021-12-23 18:25 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-02 23:37 bug#40397: 28.0.50; epg decrypt does not verify signed content in smime encrypted and signed message Sebastian Fieber
2020-04-03  6:47 ` bug#40397: 28.0.50; epg decrypt does not verify signed content in smime Sebastian Fieber
2020-04-03 23:22   ` Sebastian Fieber
2020-04-05  0:37     ` Sebastian Fieber
2020-04-06  0:04       ` Sebastian Fieber
2020-04-06  1:17         ` Noam Postavsky
2020-04-06  7:01           ` Sebastian Fieber
2020-04-06 16:32             ` Noam Postavsky
2020-04-07 19:22 ` Sebastian Fieber
2020-04-19 12:16   ` Noam Postavsky
2020-08-02  6:02   ` Lars Ingebrigtsen
2020-08-02 20:11     ` Sebastian Fieber
2020-08-03  2:26       ` Eli Zaretskii
2020-08-03  6:06       ` Lars Ingebrigtsen
2021-07-21 15:41         ` bug#40397: 28.0.50; epg decrypt does not verify signed content in smime encrypted and signed message Lars Ingebrigtsen
2021-07-21 18:07           ` Sebastian Fieber
2021-07-21 22:02             ` Lars Ingebrigtsen
2021-12-21 19:39               ` Sebastian Fieber
2021-12-22 12:44                 ` Lars Ingebrigtsen
2021-12-23 18:14                   ` Sebastian Fieber
2021-12-23 18:17                     ` Sebastian Fieber
2021-12-23 18:25                       ` Sebastian Fieber [this message]
2021-12-23 21:06                       ` Sebastian Fieber
2021-12-24  9:44                         ` Lars Ingebrigtsen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87czln6uwi.fsf@web.de \
    --to=sebastian.fieber@web.de \
    --cc=40397@debbugs.gnu.org \
    --cc=larsi@gnus.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).