bug#40397: 28.0.50; epg decrypt does not verify signed content in smime encrypted and signed message

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime encrypted and signed message

[Sebastian Fieber]

Hey there,

I'm currently running master on commit
1242ae904a9b7871658f11fb98da5730ea8838c9.

When I open an smime encrypted AND signed message in gnus with a content
type looking like this:

Content-Type: application/pkcs7-mime; smime-type=enveloped-data;
	name="smime.p7m"

I end up with a buffer looking like this:

Content-Type: application/x-pkcs7-mime; name=smime.p7m; smime-type=signed-data
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename=smime.p7m

[base64 encoded smime.p7m]

This is the signed content which would have to be verified again. I
tried to fix this myself but are really unfamiliar with the gnus
codebase. I tried to run mm-dissect-buffer on this content alone which
gives some results. I think a fix would look like this: there just needs
to be some checking whats inside the enveloped data that is being
correctly decrypted and if its another application/(x-)pkcs7-mime just
handle this one too.

Best regards
Sebastian


In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.14, cairo version 1.17.3)
 of 2020-03-21 built on comedian
Repository revision: 1242ae904a9b7871658f11fb98da5730ea8838c9
Repository branch: makepkg
Windowing system distributor 'The X.Org Foundation', version 11.0.12007000
System Description: Arch Linux

Recent messages:
nnimap web splitting mail...done
nnimap read 2k from disroot.org
Reading active file via nndraft...done
Checking new news...done
Auto-saving...
Outdated usage of ‘bbdb-search’
Parsing BBDB file ‘~/.emacs.d/bbdb’...done
Buffer *unsent mail* modified; kill anyway? (y or n) y
next-line: End of buffer
<s-backspace> is undefined

Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --mandir=/usr/share/man
 --pdfdir=/usr/share/doc/emacs/pdf --without-gconf --with-sound=alsa
 --with-x-toolkit=gtk3 --without-toolkit-scroll-bars --with-mailutils
 --with-gameuser=yes --with-xft 'CFLAGS=-march=x86-64 -mtune=generic -O2
 -pipe -fstack-protector-strong -fno-plt'
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now
 CPPFLAGS=-D_FORTIFY_SOURCE=2'

Configured features:
XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY
INOTIFY ACL GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF ZLIB GTK3
X11 XDBE XIM MODULES THREADS LIBSYSTEMD JSON PDUMPER LCMS2 GMP

Important settings:
  value of $LC_MONETARY: de_DE.utf8
  value of $LC_NUMERIC: de_DE.utf8
  value of $LC_TIME: de_DE.utf8
  value of $LANG: en_US.utf8
  locale-coding-system: utf-8-unix

Major mode: Group

Minor modes in effect:
  gnus-agent-group-mode: t
  shell-dirtrack-mode: t
  gnus-undo-mode: t
  auto-insert-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  global-company-mode: t
  company-mode: t
  global-morlock-mode: t
  eval-sexp-fu-flash-mode: t
  persistent-scratch-autosave-mode: t
  smartparens-global-mode: t
  guru-global-mode: t
  guru-mode: t
  show-paren-mode: t
  editorconfig-mode: t
  solaire-global-mode: t
  minibuffer-depth-indicate-mode: t
  save-place-mode: t
  guide-key-mode: t
  immortal-scratch-mode: t
  winner-mode: t
  diff-hl-flydiff-mode: t
  global-diff-hl-mode: t
  doom-modeline-mode: t
  projectile-mode: t
  savehist-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  temp-buffer-resize-mode: t
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t

Load-path shadows:
/home/judas/.emacs.d/elpa/cmake-mode-20190710.1319/cmake-mode hides /usr/share/emacs/site-lisp/cmake-mode
/home/judas/.emacs.d/elpa/less-css-mode-20161001.453/less-css-mode hides /usr/share/emacs/28.0.50/lisp/textmodes/less-css-mode

Features:
(shadow emacsbug bbdb-message sendmail nnir finder finder-inf lisp-mnt
skeleton gnus-html url-queue url-cache mm-url expand-region
subword-mode-expansions text-mode-expansions the-org-mode-expansions
er-basic-expansions expand-region-core expand-region-custom pulse sort
smiley gnus-cite pp cl-print debug magit-utils mule-util jka-compr
misearch multi-isearch info-colors eieio-opt speedbar ezimage dframe
help-fns radix-tree mm-archive mail-extr gnus-async gnus-bcklg qp
gnus-ml disp-table nndraft nnmh utf-7 nnfolder tabify editorconfig-core
editorconfig-core-handle editorconfig-fnmatch bbdb-gnus bbdb-mua
bbdb-com crm gnutls network-stream nsm gnus-agent gnus-srvr gnus-score
score-mode nnvirtual gnus-msg nntp gnus-cache vc-git edebug backtrace
lisp-extra-font-lock local-layer personal gnus-icalendar org-capture
ob-plantuml ob-ditaa ob-python ob-shell shell ob-json sound-wav deferred
notifications dbus ox-md ox-odt rng-loc rng-uri rng-parse rng-match
rng-dt rng-util rng-pttrn nxml-parse nxml-ns nxml-enc xmltok nxml-util
ox-latex ox-icalendar ox-html table ox-ascii ox-publish ox org-element
avl-tree org ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro
org-footnote org-src ob-comint org-pcomplete pcomplete org-list
org-faces org-entities noutline outline org-version ob-emacs-lisp
ob-core ob-eval org-table ol org-keys org-compat org-macs org-loaddefs
find-func gnus-art mm-uu mml2015 mm-view mml-smime smime dig gnus-sum
url url-proxy url-privacy url-expand url-methods url-history mailcap shr
url-cookie url-domsuf url-util svg xml dom gnus-group gnus-undo
gnus-start gnus-cloud nnimap nnmail mail-source utf7 netrc nnoo
parse-time iso8601 gnus-spec gnus-int gnus-range message rmc puny dired
dired-loaddefs format-spec rfc822 mml mml-sec mailabbrev mailheader
gnus-win mm-decode mm-bodies mm-encode mail-parse rfc2231 gmm-utils
icalendar diary-lib diary-loaddefs cal-menu calendar cal-loaddefs
epa-file epa derived epg epg-config bbdb bbdb-site timezone gnus
nnheader gnus-util rmail rmail-loaddefs rfc2047 rfc2045 ietf-drums
text-property-search time-date mail-utils mm-util mail-prsvr wid-edit
ansible-layer dotnet-layer mark-layer visible-mark sf-kbd sf-guix
haskell-layer cc-layer js-layer eglot-layer latex-layer org-layer
python-layer perl-layer php-layer web-layer gnus-layer convenience-layer
yatemplate autoinsert auto-complete-layer string-inflection
clojure-snippets cl-extra yasnippet company-oddmuse company-keywords
company-etags etags fileloop generator company-gtags
company-dabbrev-code company-dabbrev company-files company-capf
company-cmake company-xcode company-clang company-semantic company-eclim
company-template company-bbdb company pcase elisp-layer morlock paxedit
rainbow-delimiters paredit eval-sexp-fu std-layer server
display-line-numbers cap-words superword subword highlight-symbol
persistent-scratch smartparens help-mode xref project guru-mode edmacro
kmacro paren editorconfig face-remap solaire-mode mb-depth saveplace
guide-key advice popwin ace-window avy immortal-scratch cc-styles
cc-align cc-engine cc-vars cc-defs winner diff-hl-flydiff diff diff-hl
vc-dir ewoc vc vc-dispatcher diff-mode easy-mmode doom-modeline
doom-modeline-segments doom-modeline-env doom-modeline-core shrink-path
f s all-the-icons all-the-icons-faces data-material data-weathericons
data-octicons data-fileicons data-faicons data-alltheicons memoize dash
projectile grep ibuf-ext ibuffer ibuffer-loaddefs thingatpt savehist
diminish sf-autoloads loader cerbere-mode-autoloads
docblock-mode-autoloads warnings compile comint ansi-color ring
hyperlight-theme rx tex-site info package easymenu browse-url
url-handlers url-parse auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json subr-x map url-vars seq byte-opt gv
bytecomp byte-compile cconv cl-loaddefs cl-lib early-init tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win
x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind inotify lcms2 dynamic-setting
system-font-setting font-render-setting cairo move-toolbar gtk x-toolkit
x multi-tty make-network-process emacs)

Memory information:
((conses 16 550291 213990)
 (symbols 48 39611 1)
 (strings 32 198004 26591)
 (string-bytes 1 7496295)
 (vectors 16 68196)
 (vector-slots 8 1612421 168866)
 (floats 8 876 1697)
 (intervals 56 23869 2698)
 (buffers 1000 68))

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime

[Sebastian Fieber]

[-- Attachment #1: Type: text/plain, Size: 426 bytes --]

Hey there,

I just thought this may be hard to test as one has to have a smime
certificate to properly receive an encrypted mail.

If someone can point me to the right approach how to fix this I may be
able to dive a bit deeper into the gnus code and submit a bug report.

This is what I have tried now just to get something:

If I alter mm-view-pkcs7-decrypt after the insert epg-decrypt-string to
call something like this:


[-- Attachment #2: Type: application/emacs-lisp, Size: 63 bytes --]

[-- Attachment #3: Type: text/plain, Size: 59 bytes --]


and adjust mm-view-pkcs7-get-type to handle a third case


[-- Attachment #4: Type: application/emacs-lisp, Size: 90 bytes --]

[-- Attachment #5: Type: text/plain, Size: 70 bytes --]


and also mm-copy-to-buffer to check for carriage returns like this:


[-- Attachment #6: Type: application/emacs-lisp, Size: 43 bytes --]

[-- Attachment #7: Type: text/plain, Size: 627 bytes --]

(can't send the carriage return properly so \r it is here instead of ^M)

I am able to get an article buffer that still has the base64 encoded
signed blob in it but after it the verified content.

I have no idea where gnus normalizes the line endings to just newlines
and why the mm-view-pkcs7-get-type adjustment is needed. But calling
gnus-ime-display-part is of course not the right approach here. First
there should be some check if the decrypted content needs to be parsed
and handled again but I have no idea which function to write or feed the
decrypted content to.

I hope this may be helpful

Best regards
Sebastian

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime

[Sebastian Fieber]

[-- Attachment #1: Type: text/plain, Size: 1716 bytes --]

Hey,

Just forget my last mail. I just dug a bit deeper and found the culprit
I think.

With commit 84ef1ea8b524f8998fc8674b99cf8069e38dce4f these lines were
added:

--8<---------------cut here---------------start------------->8---
modified   lisp/gnus/mm-decode.el
@@ -1672,6 +1672,8 @@ If RECURSIVE, search recursively."
                    (t (y-or-n-p
                        (format "Decrypt (S/MIME) part? "))))
                   (mm-view-pkcs7 parts from))
+         (goto-char (point-min))
+         (insert "Content-type: text/plain\n\n")
          (setq parts (mm-dissect-buffer t)))))
      ((equal subtype "signed")
       (unless (and (setq protocol
@@ -1739,6 +1741,7 @@ If RECURSIVE, search recursively."
--8<---------------cut here---------------end--------------->8---

I don't quite know why the content-type is forced here to text/plain. So
if this line is removed the mm-dissect-buffer call does it's thing and
returns correctly whats inside the envelope (the real content-type
header in the decrypted envelope is parsed). Well almost...

I wrote in my last mail that I had to adjust mm-copy-to-buffer:

> and also mm-copy-to-buffer to check for carriage returns like this:
>
> (search-forward-regexp "^\r\n" nil 'move)
>
> (can't send the carriage return properly so \r it is here instead of ^M)

This is still needed as the decrypted content may still have carriage
returns in it. One could also remove the carriage returns in
mm-view-pkcs7-decrypt function of course. I'm not quite sure which is
the better approach.

In such a case the "Decrypt (S/MIME) part?" is asked too times. But hey
that isn't too bad I think.

I have attached a patch with the explained fix.

Best regards
Sebastian


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-fix-bug-40397.patch --]
[-- Type: text/x-patch, Size: 1447 bytes --]

From ee7ff9a8a083860d39d011c7e4df30cb63490fb9 Mon Sep 17 00:00:00 2001
From: fallchildren <sebastian.fieber@web.de>
Date: Sat, 4 Apr 2020 01:16:12 +0200
Subject: [PATCH] fix bug #40397

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

- don't insert Content-type header in front of decrypted content for
  smime decryption using mm-view-pkcs7
- also check for carriage return in mm-copy-to-buffer
---
 lisp/gnus/mm-decode.el | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lisp/gnus/mm-decode.el b/lisp/gnus/mm-decode.el
index 96695aabfd..d321fbeaaa 100644
--- a/lisp/gnus/mm-decode.el
+++ b/lisp/gnus/mm-decode.el
@@ -759,7 +759,7 @@ MIME-Version header before proceeding."
         (mb enable-multibyte-characters)
         beg)
     (goto-char (point-min))
-    (search-forward-regexp "^\n" nil 'move) ;; There might be no body.
+    (search-forward-regexp "^\r?\n" nil 'move) ;; There might be no body.
     (setq beg (point))
     (with-current-buffer
           (generate-new-buffer " *mm*")
@@ -1681,7 +1681,6 @@ If RECURSIVE, search recursively."
 			(format "Decrypt (S/MIME) part? "))))
 		   (mm-view-pkcs7 parts from))
 	  (goto-char (point-min))
-	  (insert "Content-type: text/plain\n\n")
 	  (setq parts (mm-dissect-buffer t)))))
      ((equal subtype "signed")
       (unless (and (setq protocol
--
2.25.2

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime

[Sebastian Fieber]

On Sa, Apr 04 2020, Sebastian Fieber <sebastian.fieber@web.de> wrote:

> In such a case the "Decrypt (S/MIME) part?" is asked too times. But hey
> that isn't too bad I think.

I just had some time to look into this even further and I noticed that
the mm-sec buttons for signatures/encryption are not displayed for the
whole application/pkcs7-mime stuff, too.  I am working on a patch to fix
this.

I think most of the code would look like the one in mml-smime.el (the
calls to mm-sec-* and getting error/success messages from epg).  The
hard part is to get the mm-security-handle or better the information
added about the pkcs7-mime signature by the mm-sec-* calls to some
function that will add these (which is gnus-mime-display-security ?).
The problem here is that the part is lost when the signature is verified
as the actual signed content parts will have replace it.

Best regards
Sebastian

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime

[Sebastian Fieber]

[-- Attachment #1: Type: text/plain, Size: 1220 bytes --]


On So, Apr 05 2020, Sebastian Fieber <sebastian.fieber@web.de> wrote:

> I just had some time to look into this even further and I noticed that
> the mm-sec buttons for signatures/encryption are not displayed for the
> whole application/pkcs7-mime stuff, too.  I am working on a patch to fix
> this.
>
> I think most of the code would look like the one in mml-smime.el (the
> calls to mm-sec-* and getting error/success messages from epg).  The
> hard part is to get the mm-security-handle or better the information
> added about the pkcs7-mime signature by the mm-sec-* calls to some
> function that will add these (which is gnus-mime-display-security ?).
> The problem here is that the part is lost when the signature is verified
> as the actual signed content parts will have replace it.
>
> Best regards
> Sebastian

Hey,

here is the resulting more thorough patch replacing the one before.

It's not finished completely as error handling and reporting via
mm-sec-error is still missing in mm-view-pkcs7-[decrypt/verify].

But displaying verification and encryption information via
gnus-mime-display-security does work (at least in the good case).

See the patch for more information.

I'd welcome any comments :)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-fix-bug-40397.patch --]
[-- Type: text/x-patch, Size: 11374 bytes --]

From 2d623b52c7810a293ad8309018ebc4973f1ff2e3 Mon Sep 17 00:00:00 2001
From: fallchildren <sebastian.fieber@web.de>
Date: Sat, 4 Apr 2020 01:16:12 +0200
Subject: [PATCH] fix bug #40397

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

- don't force Content-type header to text/plain in front of decrypted
  content for smime decryption using mm-view-pkcs7

- 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

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

- 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"

- adjust mm-possibly-verify-or-decrypt to check for smime-type to ask
  wether to verfiy or decrypt part and not to ask to decrypt either
  way

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

- in mm-view-pkcs7-decrypt also replace "^M\n" with newline and not only
  "\r\n" - I have no idea why this is needed

TODO: mm-view-pkcs7-decrypt and verify error handling and
reporting. ATM there is only the good case implemented - at least for
reporting with gnus-mime-display-security.
---
 lisp/gnus/gnus-art.el  | 28 ++++++++++++++++
 lisp/gnus/mm-decode.el | 74 +++++++++++++++++++++++++++++-------------
 lisp/gnus/mm-view.el   | 32 +++++++++++++++---
 3 files changed, 108 insertions(+), 26 deletions(-)

diff --git a/lisp/gnus/gnus-art.el b/lisp/gnus/gnus-art.el
index 6b9610d312..4ab629eda0 100644
--- a/lisp/gnus/gnus-art.el
+++ b/lisp/gnus/gnus-art.el
@@ -5986,6 +5986,34 @@ If nil, don't show those extra buttons."
    ((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)))))
diff --git a/lisp/gnus/mm-decode.el b/lisp/gnus/mm-decode.el
index 96695aabfd..5af2e50f66 100644
--- a/lisp/gnus/mm-decode.el
+++ b/lisp/gnus/mm-decode.el
@@ -473,6 +473,7 @@ The file will be saved in the directory `mm-tmp-directory'.")
 (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")

 (defvar mm-verify-function-alist
   '(("application/pgp-signature" mml2015-verify "PGP" mml2015-verify-test)
@@ -481,7 +482,15 @@ The file will be saved in the directory `mm-tmp-directory'.")
     ("application/pkcs7-signature" mml-smime-verify "S/MIME"
      mml-smime-verify-test)
     ("application/x-pkcs7-signature" mml-smime-verify "S/MIME"
-     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)))

 (defcustom mm-verify-option 'never
   "Option of verifying signed parts.
@@ -500,11 +509,16 @@ result of the verification."

 (autoload 'mml2015-decrypt "mml2015")
 (autoload 'mml2015-decrypt-test "mml2015")
+(autoload 'mm-view-pkcs7-decrypt "mm-view")

 (defvar mm-decrypt-function-alist
   '(("application/pgp-encrypted" mml2015-decrypt "PGP" mml2015-decrypt-test)
     ("application/x-gnus-pgp-encrypted" mm-uu-pgp-encrypted-extract-1 "PGP"
-     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/MIME" nil)))

 (defcustom mm-decrypt-option nil
   "Option of decrypting encrypted parts.
@@ -682,14 +696,23 @@ MIME-Version header before proceeding."
 				  (car ctl))
 	     (cons (car ctl) (mm-dissect-multipart ctl from))))
 	  (t
-	   (mm-possibly-verify-or-decrypt
-	    (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)
-	    ctl from))))
+	   (let* ((intermediate-result
+                   (mm-possibly-verify-or-decrypt
+                    (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)
+                    ctl from)))
+             (when (and (equal type "application")
+                        (or (equal subtype "pkcs7-mime")
+                            (equal subtype "x-pkcs7-mime")))
+               ;; 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 (list (car ctl) intermediate-result)))
+             intermediate-result))))
 	(when id
 	  (when (string-match " *<\\(.*\\)> *" id)
 	    (setq id (match-string 1 id)))
@@ -759,7 +782,7 @@ MIME-Version header before proceeding."
         (mb enable-multibyte-characters)
         beg)
     (goto-char (point-min))
-    (search-forward-regexp "^\n" nil 'move) ;; There might be no body.
+    (search-forward-regexp "^?\n" nil 'move) ;; There might be no body.
     (setq beg (point))
     (with-current-buffer
           (generate-new-buffer " *mm*")
@@ -1672,17 +1695,24 @@ If RECURSIVE, search recursively."
     (cond
      ((or (equal type "application/x-pkcs7-mime")
 	  (equal type "application/pkcs7-mime"))
-      (with-temp-buffer
-	(when (and (cond
-		    ((eq mm-decrypt-option 'never) nil)
-		    ((eq mm-decrypt-option 'always) t)
-		    ((eq mm-decrypt-option 'known) t)
-		    (t (y-or-n-p
-			(format "Decrypt (S/MIME) part? "))))
-		   (mm-view-pkcs7 parts from))
-	  (goto-char (point-min))
-	  (insert "Content-type: text/plain\n\n")
-	  (setq parts (mm-dissect-buffer t)))))
+      (let* ((smime-type (cdr (assoc 'smime-type ctl)))
+             (envelope-p (string= smime-type "enveloped-data"))
+             (decrypt-or-sign-option (if envelope-p
+                                         mm-decrypt-option
+                                       mm-sign-option))
+             (question (if envelope-p
+                           "Decrypt (S/MIME) part? "
+                         "Verify signed (S/MIME) part? ")))
+        (with-temp-buffer
+	  (when (and (cond
+		      ((eq decrypt-or-sign-option 'never) nil)
+		      ((eq decrypt-or-sign-option 'always) t)
+		      ((eq decrypt-or-sign-option 'known) t)
+		      (t (y-or-n-p
+			  (format question))))
+		     (mm-view-pkcs7 parts from))
+	    (goto-char (point-min))
+	    (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 828ac633dc..4c7350b55a 100644
--- a/lisp/gnus/mm-view.el
+++ b/lisp/gnus/mm-view.el
@@ -591,8 +591,20 @@ If MODE is not set, try to find mode automatically."
 	 (with-temp-buffer
 	   (insert-buffer-substring (mm-handle-buffer handle))
 	   (goto-char (point-min))
-	   (let ((part (base64-decode-string (buffer-string))))
-	     (epg-verify-string (epg-make-context 'CMS) part))))
+           (let* ((part (base64-decode-string (buffer-string)))
+                  (context (epg-make-context 'CMS))
+                  (plain (epg-verify-string context part)))
+             (mm-sec-status
+              'gnus-info
+              (epg-verify-result-to-string (epg-context-result-for context 'verify))
+              'gnus-details
+              nil
+              'protocol
+              ;; just mimik pkcs7-signature actually we are in pkcs7-mime
+              (concat (substring-no-properties (caadr handle))
+                      "_"
+                      (cdr (assoc 'smime-type (cadr handle)))))
+             plain)))
       (with-temp-buffer
 	(insert "MIME-Version: 1.0\n")
 	(mm-insert-headers "application/pkcs7-mime" "base64" "smime.p7m")
@@ -612,7 +624,19 @@ If MODE is not set, try to find mode automatically."
       ;; Use EPG/gpgsm
       (let ((part (base64-decode-string (buffer-string))))
 	(erase-buffer)
-	(insert (epg-decrypt-string (epg-make-context 'CMS) part)))
+	(insert
+         (let* ((context (epg-make-context 'CMS))
+                (plain (epg-decrypt-string context part)))
+           (mm-sec-status
+            'gnus-info
+            "OK"
+            'gnus-details
+            nil
+            'protocol
+            (concat (substring-no-properties (caadr handle))
+                    "_"
+                    (cdr (assoc 'smime-type (cadr handle)))))
+           plain)))
     ;; Use openssl
     (insert "MIME-Version: 1.0\n")
     (mm-insert-headers "application/pkcs7-mime" "base64" "smime.p7m")
@@ -626,7 +650,7 @@ If MODE is not set, try to find mode automatically."
 	 smime-keys nil nil nil (car-safe (car-safe smime-keys)))))
      from))
   (goto-char (point-min))
-  (while (search-forward "\r\n" nil t)
+  (while (search-forward-regexp "\r\n|\r\n" nil t)
     (replace-match "\n"))
   (goto-char (point-min)))

--
2.25.2

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime

[Noam Postavsky]

Sebastian Fieber <sebastian.fieber@web.de> writes:

> -  (while (search-forward "\r\n" nil t)
> +  (while (search-forward-regexp "\r\n|\^M\n" nil t)

This can't be right, it would search for a literal "|" on an otherwise
empty line.  And if you put "\\|" which is what I think you meant, then
both alternatives would be the same, so it still doesn't make sense.

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime

[Sebastian Fieber]

On So, Apr 05 2020, Noam Postavsky <npostavs@gmail.com> wrote:

> Sebastian Fieber <sebastian.fieber@web.de> writes:
>
>> -  (while (search-forward "\r\n" nil t)
>> +  (while (search-forward-regexp "\r\n|\^M\n" nil t)
>
> This can't be right, it would search for a literal "|" on an otherwise
> empty line.  And if you put "\\|" which is what I think you meant, then
> both alternatives would be the same, so it still doesn't make sense.

Yes, and there is another problem with this. Should have tested this mit
emacs -Q. Let me fix that and prepare a new patch.

Since you have looked over the patch: What do you think about the
approach to internally structure application/pkcs7-mime parts like
multipart parts containing the mime type with text properties until the
decrypted, maybe verified singlepart in the car of the handle?

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime

[Noam Postavsky]

Sebastian Fieber <sebastian.fieber@web.de> writes:

> On So, Apr 05 2020, Noam Postavsky <npostavs@gmail.com> wrote:
>
>> Sebastian Fieber <sebastian.fieber@web.de> writes:
>>
>>> -  (while (search-forward "\r\n" nil t)
>>> +  (while (search-forward-regexp "\r\n|\^M\n" nil t)
>>
>> This can't be right, it would search for a literal "|" on an otherwise
>> empty line.  And if you put "\\|" which is what I think you meant, then
>> both alternatives would be the same, so it still doesn't make sense.
>
> Yes, and there is another problem with this. Should have tested this mit
> emacs -Q. Let me fix that and prepare a new patch.

This hunk looks a bit suspicious to me as well, I don't think you can
apply operators like "?" to anchors.

@@ -759,7 +782,7 @@ MIME-Version header before proceeding."
         (mb enable-multibyte-characters)
         beg)
     (goto-char (point-min))
-    (search-forward-regexp "^\n" nil 'move) ;; There might be no body.
+    (search-forward-regexp "^?\n" nil 'move) ;; There might be no body.
     (setq beg (point))
     (with-current-buffer
           (generate-new-buffer " *mm*")

> Since you have looked over the patch: What do you think about the
> approach to internally structure application/pkcs7-mime parts like
> multipart parts containing the mime type with text properties until the
> decrypted, maybe verified singlepart in the car of the handle?

Sorry, I'm not familiar enough with how this code is currently
structured to say anything intelligent about that.

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime

[Sebastian Fieber]

[-- Attachment #1: Type: text/plain, Size: 3973 bytes --]

On Mo, Apr 06 2020, Noam Postavsky <npostavs@gmail.com> wrote:

> This hunk looks a bit suspicious to me as well, I don't think you can
> apply operators like "?" to anchors.
>
> @@ -759,7 +782,7 @@ MIME-Version header before proceeding."
>          (mb enable-multibyte-characters)
>          beg)
>      (goto-char (point-min))
> -    (search-forward-regexp "^\n" nil 'move) ;; There might be no body.
> +    (search-forward-regexp "^?\n" nil 'move) ;; There might be no body.
>      (setq beg (point))
>      (with-current-buffer
>            (generate-new-buffer " *mm*")
>

Yes, this sections is also wrong.

>> Since you have looked over the patch: What do you think about the
>> approach to internally structure application/pkcs7-mime parts like
>> multipart parts containing the mime type with text properties until the
>> decrypted, maybe verified singlepart in the car of the handle?
> Sorry, I'm not familiar enough with how this code is currently
> structured to say anything intelligent about that.

No problem :)

I have attached a new patch which fixes the problem and also does
implement support for the security buttons for application/pkcs7-mime
parts.  This is quite nice as application/pkcs7-mime parts are not
handled automatically by default in gnus.  ATM you have to set
mm-decrypt-option and mm-verify-option at least to 'ask.  So with this
supported it should now work out of the box even without setting
mm-decrypt-option and mm-verify-option because now gnus shows the
buttons properly and one can click on them and decrypt/verify the part
"manually".

This time the patch should be clean and was tested properly at least
with mml-smime-use 'epg.  I'm not quite sure if the patch breaks using
openssl as I didn't get this running.  Maybe someone can test this? If
this does break using openssl modifying mm-views decrypt and verify
function should suffice to fix any problems.

The gist of the patch is: treat application/pkcs7-mime like multipart
mails and especially multipart/encrypted with protocol
application/pgp-encrypted and change not more stuff than necessary.

Here is the commit message which is a bit more detailed (also found in
the patch):

"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:
- don't force Content-type header to text/plain in front of decrypted
  content for smime decryption using mm-view-pkcs7.  This fixes the
  initial bug where the signed part was not verified due to the wrong
  content type header.

- 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

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

- 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"

- 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

- 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

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

- in mm-view-pkcs7-verify also remove carriage returns like in
  mm-view-pkcs7-decrypt

- adjust gnus-mime-security-verify-or-decrypt to handle pkcs7-mime
  right with the done changes

TODO: mm-view-pkcs7-decrypt and verify error handling and
reporting. ATM there is only the good case implemented - at least for
reporting with gnus-mime-display-security."


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-fix-bug-40397.patch --]
[-- Type: text/x-patch, Size: 13674 bytes --]

From 3f85a1a72953f0877d2edcf56e872e7fe760b9f9 Mon Sep 17 00:00:00 2001
From: Sebastian Fieber <sebastian.fieber@web.de>
Date: Mon, 6 Apr 2020 20:45:05 +0200
Subject: [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:
- don't force Content-type header to text/plain in front of decrypted
  content for smime decryption using mm-view-pkcs7. This fixes the
  initial bug where the signed part was not verified due to the wrong
  content type header.

- 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

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

- 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"

- 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

- 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

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

- in mm-view-pkcs7-verify also remove carriage returns like in
  mm-view-pkcs7-decrypt

- adjust gnus-mime-security-verify-or-decrypt to handle pkcs7-mime
  right with the done changes

TODO: mm-view-pkcs7-decrypt and verify error handling and
reporting. ATM there is only the good case implemented - at least for
reporting with gnus-mime-display-security.
---
 lisp/gnus/gnus-art.el  | 60 ++++++++++++++++++++++++++++---
 lisp/gnus/mm-decode.el | 81 +++++++++++++++++++++++++++++++-----------
 lisp/gnus/mm-view.el   | 25 +++++++++++--
 3 files changed, 138 insertions(+), 28 deletions(-)

diff --git a/lisp/gnus/gnus-art.el b/lisp/gnus/gnus-art.el
index 6b9610d312..b130650df6 100644
--- a/lisp/gnus/gnus-art.el
+++ b/lisp/gnus/gnus-art.el
@@ -5986,6 +5986,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)))))
@@ -8733,9 +8761,16 @@ gnus-mime-security-verify-or-decrypt
     (with-current-buffer (mm-handle-multipart-original-buffer handle)
       (let* ((mm-verify-option 'known)
 	     (mm-decrypt-option 'known)
-	     (nparts (mm-possibly-verify-or-decrypt (cdr handle) handle)))
+             (pkcs7-mime-p (or (equal (car handle) "application/pkcs7-mime")
+                               (equal (car handle) "application/x-pkcs7-mime")))
+	     (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))
-	  (mm-destroy-parts (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
@@ -8793,8 +8828,25 @@ gnus-insert-mime-security-button
 	   (or (nth 2 (assoc protocol mm-verify-function-alist))
 	       (nth 2 (assoc protocol mm-decrypt-function-alist))
 	       "Unknown")
-	   (if (equal (car handle) "multipart/signed")
-	       " Signed" " Encrypted")
+	   (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 'protocol)
+                              "application/pkcs7-mime_signed-data"))
+                  " Signed")
+                 ((and (equal (car handle) "application/pkcs7-mime")
+                       (equal (mm-handle-multipart-ctl-parameter handle 'protocol)
+                              "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 'protocol)
+                              "application/x-pkcs7-mime_signed-data"))
+                  " Signed")
+                 ((and (equal (car handle) "application/x-pkcs7-mime")
+                       (equal (mm-handle-multipart-ctl-parameter handle 'protocol)
+                              "application/x-pkcs7-mime_enveloped-data"))
+                  " Encrypted"))
 	   " Part"))
 	 (gnus-tmp-info
 	  (or (mm-handle-multipart-ctl-parameter handle 'gnus-info)
diff --git a/lisp/gnus/mm-decode.el b/lisp/gnus/mm-decode.el
index 96695aabfd..da1a7c36a5 100644
--- a/lisp/gnus/mm-decode.el
+++ b/lisp/gnus/mm-decode.el
@@ -473,6 +473,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")

 (defvar mm-verify-function-alist
   '(("application/pgp-signature" mml2015-verify "PGP" mml2015-verify-test)
@@ -481,7 +482,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"
-     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)))

 (defcustom mm-verify-option 'never
   "Option of verifying signed parts.
@@ -500,11 +509,16 @@ mm-verify-option

 (autoload 'mml2015-decrypt "mml2015")
 (autoload 'mml2015-decrypt-test "mml2015")
+(autoload 'mm-view-pkcs7-decrypt "mm-view")

 (defvar mm-decrypt-function-alist
   '(("application/pgp-encrypted" mml2015-decrypt "PGP" mml2015-decrypt-test)
     ("application/x-gnus-pgp-encrypted" mm-uu-pgp-encrypted-extract-1 "PGP"
-     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/MIME" nil)))

 (defcustom mm-decrypt-option nil
   "Option of decrypting encrypted parts.
@@ -682,14 +696,29 @@ mm-dissect-buffer
 				  (car ctl))
 	     (cons (car ctl) (mm-dissect-multipart ctl from))))
 	  (t
-	   (mm-possibly-verify-or-decrypt
-	    (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)
-	    ctl from))))
+	   (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 handle 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 intermediate-result))))
+             intermediate-result))))
 	(when id
 	  (when (string-match " *<\\(.*\\)> *" id)
 	    (setq id (match-string 1 id)))
@@ -1672,17 +1701,27 @@ mm-possibly-verify-or-decrypt
     (cond
      ((or (equal type "application/x-pkcs7-mime")
 	  (equal type "application/pkcs7-mime"))
-      (with-temp-buffer
-	(when (and (cond
-		    ((eq mm-decrypt-option 'never) nil)
-		    ((eq mm-decrypt-option 'always) t)
-		    ((eq mm-decrypt-option 'known) t)
-		    (t (y-or-n-p
-			(format "Decrypt (S/MIME) part? "))))
-		   (mm-view-pkcs7 parts from))
-	  (goto-char (point-min))
-	  (insert "Content-type: text/plain\n\n")
-	  (setq parts (mm-dissect-buffer t)))))
+      (add-text-properties 0 (length (car ctl))
+			   (list 'buffer (car parts))
+			   (car ctl))
+      (let* ((smime-type (cdr (assoc 'smime-type ctl)))
+             (envelope-p (string= smime-type "enveloped-data"))
+             (decrypt-or-sign-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
+		      ((eq decrypt-or-sign-option 'never) nil)
+		      ((eq decrypt-or-sign-option 'always) t)
+		      ((eq decrypt-or-sign-option 'known) t)
+		      (t (y-or-n-p
+			  (format question)))))
+	    (mm-view-pkcs7 parts from)
+	    (goto-char (point-min))
+	    (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 828ac633dc..34da9464ce 100644
--- a/lisp/gnus/mm-view.el
+++ b/lisp/gnus/mm-view.el
@@ -591,8 +591,15 @@ mm-view-pkcs7-verify
 	 (with-temp-buffer
 	   (insert-buffer-substring (mm-handle-buffer handle))
 	   (goto-char (point-min))
-	   (let ((part (base64-decode-string (buffer-string))))
-	     (epg-verify-string (epg-make-context 'CMS) part))))
+           (let* ((part (base64-decode-string (buffer-string)))
+                  (context (epg-make-context 'CMS))
+                  (plain (epg-verify-string context part)))
+             (mm-sec-status
+              'gnus-info
+              (epg-verify-result-to-string (epg-context-result-for context 'verify))
+              'gnus-details
+              nil)
+             plain)))
       (with-temp-buffer
 	(insert "MIME-Version: 1.0\n")
 	(mm-insert-headers "application/pkcs7-mime" "base64" "smime.p7m")
@@ -601,6 +608,10 @@ mm-view-pkcs7-verify
       (if verified
 	  (insert verified)
 	(insert-buffer-substring smime-details-buffer)))
+    (goto-char (point-min))
+    (while (search-forward "\r\n" nil t)
+      (replace-match "\n"))
+    (goto-char (point-min))
     t))

 (autoload 'epg-decrypt-string "epg")
@@ -612,7 +623,15 @@ mm-view-pkcs7-decrypt
       ;; Use EPG/gpgsm
       (let ((part (base64-decode-string (buffer-string))))
 	(erase-buffer)
-	(insert (epg-decrypt-string (epg-make-context 'CMS) part)))
+	(insert
+         (let* ((context (epg-make-context 'CMS))
+                (plain (epg-decrypt-string context part)))
+           (mm-sec-status
+            'gnus-info
+            "OK"
+            'gnus-details
+            nil)
+           plain)))
     ;; Use openssl
     (insert "MIME-Version: 1.0\n")
     (mm-insert-headers "application/pkcs7-mime" "base64" "smime.p7m")
--
2.25.2

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime

[Noam Postavsky]

As I mentioned previously, I'm not really familiar enough with the code
to give a proper review, but I have a couple of minor comments.

Sebastian Fieber <sebastian.fieber@web.de> writes:

> +               (setq intermediate-result (cons (car ctl) (list intermediate-result))))

Or just

    (setq intermediate-result (list (car ctl) intermediate-result))

> @@ -1672,17 +1701,27 @@ mm-possibly-verify-or-decrypt
> -      (with-temp-buffer
> -	(when (and (cond
> -		    ((eq mm-decrypt-option 'never) nil)
> -		    ((eq mm-decrypt-option 'always) t)
> -		    ((eq mm-decrypt-option 'known) t)
> -		    (t (y-or-n-p
> -			(format "Decrypt (S/MIME) part? "))))
> -		   (mm-view-pkcs7 parts from))
> -	  (goto-char (point-min))
> -	  (insert "Content-type: text/plain\n\n")
> -	  (setq parts (mm-dissect-buffer t)))))
> +      (add-text-properties 0 (length (car ctl))
> +			   (list 'buffer (car parts))
> +			   (car ctl))
> +      (let* ((smime-type (cdr (assoc 'smime-type ctl)))
> +             (envelope-p (string= smime-type "enveloped-data"))
> +             (decrypt-or-sign-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
> +		      ((eq decrypt-or-sign-option 'never) nil)
> +		      ((eq decrypt-or-sign-option 'always) t)
> +		      ((eq decrypt-or-sign-option 'known) t)
> +		      (t (y-or-n-p
> +			  (format question)))))
> +	    (mm-view-pkcs7 parts from)
> +	    (goto-char (point-min))
> +	    (setq parts (mm-dissect-buffer t))))))

You moved the 'mm-view-pkcs7' call out of the condition.  If that was on
purpose, then you should remove the 'and', since it's now redundant.

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime

[Lars Ingebrigtsen]

Sebastian Fieber <sebastian.fieber@web.de> writes:

> I have attached a new patch which fixes the problem

Thanks; I didn't see this bug report before I fixed the text/plain thing
in a different way.  (So I think s/mime should basically work again now
in Emacs 27.)

> and also does
> implement support for the security buttons for application/pkcs7-mime
> parts.  This is quite nice as application/pkcs7-mime parts are not
> handled automatically by default in gnus.  ATM you have to set
> mm-decrypt-option and mm-verify-option at least to 'ask.  So with this
> supported it should now work out of the box even without setting
> mm-decrypt-option and mm-verify-option because now gnus shows the
> buttons properly and one can click on them and decrypt/verify the part
> "manually".

This sounds like a good addition to me, and would like to apply the
patch to Emacs 28.  It's a large patch, though, and you don't seem to
have copyright FSF assignment on file -- is that correct?  If it is,
would you be willing to sign such paperwork, and we can then apply the
patch?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime

[Sebastian Fieber]

On So, Aug 02 2020, Lars Ingebrigtsen <larsi@gnus.org> wrote:

>> and also does
>> implement support for the security buttons for application/pkcs7-mime
>> parts.  This is quite nice as application/pkcs7-mime parts are not
>> handled automatically by default in gnus.  ATM you have to set
>> mm-decrypt-option and mm-verify-option at least to 'ask.  So with this
>> supported it should now work out of the box even without setting
>> mm-decrypt-option and mm-verify-option because now gnus shows the
>> buttons properly and one can click on them and decrypt/verify the part
>> "manually".
>
> This sounds like a good addition to me, and would like to apply the
> patch to Emacs 28.  It's a large patch, though, and you don't seem to
> have copyright FSF assignment on file -- is that correct?  If it is,
> would you be willing to sign such paperwork, and we can then apply the
> patch?

Yes, I haven't done any copyright assignment yet but I'd be willing to
do so if someone can guide me a bit or point me to where I can find info
about what I have to do.

There are some untested and unimplemented stuff in my implementation.
If I remember correct there is no real handling of error cases which I
wanted to add so it is on par with the other security buttons
implementations.  So I'd like to work on this a bit more and provide a
more fully featured patch.  But I'm pretty busy right now with real
life, so this may take a few months as I'd need to find some time.

Nontheless I will check if I have done any changes to my provided patch
and resubmit it if I have any work pending - if you don't want to wait
for me and want to apply the patch anyway even without proper error
handling.

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime

[Eli Zaretskii]

> From: Sebastian Fieber <sebastian.fieber@web.de>
> Date: Sun, 02 Aug 2020 22:11:20 +0200
> Cc: 40397@debbugs.gnu.org
> 
> Yes, I haven't done any copyright assignment yet but I'd be willing to
> do so if someone can guide me a bit or point me to where I can find info
> about what I have to do.

Thanks, form sent off-list.

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime

[Lars Ingebrigtsen]

Sebastian Fieber <sebastian.fieber@web.de> writes:

> There are some untested and unimplemented stuff in my implementation.
> If I remember correct there is no real handling of error cases which I
> wanted to add so it is on par with the other security buttons
> implementations.

Sure, that sounds good.  Error handling is something that's lacking in
many parts of the Emacs handling of signing/encryption, unfortunately.

> So I'd like to work on this a bit more and provide a
> more fully featured patch.  But I'm pretty busy right now with real
> life, so this may take a few months as I'd need to find some time.

Sure, no hurry.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime encrypted and signed message

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> So I'd like to work on this a bit more and provide a
>> more fully featured patch.  But I'm pretty busy right now with real
>> life, so this may take a few months as I'd need to find some time.
>
> Sure, no hurry.  :-)

This was a year ago -- has there been any progress here?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime encrypted and signed message

[Sebastian Fieber]

[-- Attachment #1: Type: text/plain, Size: 800 bytes --]


On Mi, Jul 21 2021, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>>> So I'd like to work on this a bit more and provide a
>>> more fully featured patch.  But I'm pretty busy right now with real
>>> life, so this may take a few months as I'd need to find some time.
>>
>> Sure, no hurry.  :-)
>
> This was a year ago -- has there been any progress here?

Sorry, haven't got to it yet, but it's still on my TODO list.  Also the
paper work for the FSF from my employer is still not finished.

I will try my best to get some traction at my employer so the paperwork
gets done soon and the patch, as it is now, can at least be accepted.
I'd still like to work on it, but maybe that would be better as another
patch building upon this one?  What do you think?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 519 bytes --]

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime encrypted and signed message

[Lars Ingebrigtsen]

Sebastian Fieber <sebastian.fieber@web.de> writes:

> I will try my best to get some traction at my employer so the paperwork
> gets done soon and the patch, as it is now, can at least be accepted.
> I'd still like to work on it, but maybe that would be better as another
> patch building upon this one?  What do you think?

Sure; whatever is most convenient for you.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime encrypted and signed message

[Sebastian Fieber]

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


On Do, Jul 22 2021, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Sebastian Fieber <sebastian.fieber@web.de> writes:
>
>> I will try my best to get some traction at my employer so the paperwork
>> gets done soon and the patch, as it is now, can at least be accepted.
>> I'd still like to work on it, but maybe that would be better as another
>> patch building upon this one?  What do you think?
>
> Sure; whatever is most convenient for you.

And another half of a year later the paper work is done, yay!

*sigh*

You can now apply the patch, if it's still relevant.  Please let me know
if there are any problems or any help is needed.
-----BEGIN PGP SIGNATURE-----

iQFMBAEBCAA2FiEExRi5b+8xM5Vpvu7L3jJw+EOyhogFAmHCLXgYHHNlYmFzdGlh
bi5maWViZXJAd2ViLmRlAAoJEN4ycPhDsoaIguYH/2JRFOUJRl7aXfQLtKDOEl4T
O5eifGFE+95HcqUE9/zoL6vVZcSFAbCKW4sP/KmhLhtXgiiO5aTbwYmru9OcfjaC
mzCe0z5mvfUNsK/87jdKWv4StfeoiGyZwyaRSqB9u1A+nCyqc/fxKQObpbSAlOPv
Jh+5I6fhYRRe1GANaMpJkV/Zy2ijqwhDUFT+oGF79jtzj1HBw/0R2o7bhEx/a3f/
nueAMrkIDvVpsw2rlL0m6XqccVJH8rKlNh1gBnz3wWQMzUmi45hovtS3lWeiQLKl
DCWoJcyZi/CCbiWpR22KdBXFz6Uj9UXaZTD0cyUlsWy1WTJS8bTU6MmAwG5A0CA=
=MYNd
-----END PGP SIGNATURE-----

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime encrypted and signed message

[Lars Ingebrigtsen]

Sebastian Fieber <sebastian.fieber@web.de> writes:

> And another half of a year later the paper work is done, yay!

Yay.  😀

> *sigh*
>
> You can now apply the patch, if it's still relevant.  Please let me know
> if there are any problems or any help is needed.

Still seems relevant to me, but the patch no longer applies due to other
changes in Gnus.  Can you re-spin the patch for Emacs 29, and then I'll
get it committed.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime encrypted and signed message

[Sebastian Fieber]

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

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

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


On Mi, Dez 22 2021, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Still seems relevant to me, but the patch no longer applies due to other
> changes in Gnus.  Can you re-spin the patch for Emacs 29, and then I'll
> get it committed.

This one should apply :)


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

From=203f85a1a72953f0877d2edcf56e872e7fe760b9f9 Mon Sep 17 00:00:00 2001
From: Sebastian Fieber <sebastian.fieber@web.de>
Date: Mon, 6 Apr 2020 20:45:05 +0200
Subject: [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 don't force Content-type header to text/plain in front of decrypted
  content for smime decryption using mm-view-pkcs7. This fixes the
  initial bug where the signed part was not verified due to the wrong
  content type header.

=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 in mm-view-pkcs7-verify also remove carriage returns like in
  mm-view-pkcs7-decrypt

=2D adjust gnus-mime-security-verify-or-decrypt to handle pkcs7-mime
  right with the done changes

TODO: mm-view-pkcs7-decrypt and verify error handling and
reporting. ATM there is only the good case implemented - at least for
reporting with gnus-mime-display-security.
=2D--
 lisp/gnus/gnus-art.el  | 60 ++++++++++++++++++++++++++++---
 lisp/gnus/mm-decode.el | 81 +++++++++++++++++++++++++++++++-----------
 lisp/gnus/mm-view.el   | 25 +++++++++++--
 3 files changed, 138 insertions(+), 28 deletions(-)

diff --git a/lisp/gnus/gnus-art.el b/lisp/gnus/gnus-art.el
index 6b9610d312..b130650df6 100644
=2D-- a/lisp/gnus/gnus-art.el
+++ b/lisp/gnus/gnus-art.el
@@ -5986,6 +5986,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)))))
@@ -8733,9 +8761,16 @@ gnus-mime-security-verify-or-decrypt
     (with-current-buffer (mm-handle-multipart-original-buffer handle)
       (let* ((mm-verify-option 'known)
             (mm-decrypt-option 'known)
=2D            (nparts (mm-possibly-verify-or-decrypt (cdr handle) handle)))
+             (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))
=2D         (mm-destroy-parts (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
@@ -8793,8 +8828,25 @@ gnus-insert-mime-security-button
           (or (nth 2 (assoc protocol mm-verify-function-alist))
               (nth 2 (assoc protocol mm-decrypt-function-alist))
               "Unknown")
=2D          (if (equal (car handle) "multipart/signed")
=2D              " Signed" " Encrypted")
+          (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)
diff --git a/lisp/gnus/mm-decode.el b/lisp/gnus/mm-decode.el
index 96695aabfd..da1a7c36a5 100644
=2D-- a/lisp/gnus/mm-decode.el
+++ b/lisp/gnus/mm-decode.el
@@ -473,6 +473,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")

 (defvar mm-verify-function-alist
   '(("application/pgp-signature" mml2015-verify "PGP" mml2015-verify-test)
@@ -481,7 +482,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)))

 (defcustom mm-verify-option 'never
   "Option of verifying signed parts.
@@ -500,11 +509,16 @@ mm-verify-option

 (autoload 'mml2015-decrypt "mml2015")
 (autoload 'mml2015-decrypt-test "mml2015")
+(autoload 'mm-view-pkcs7-decrypt "mm-view")

 (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)))

 (defcustom mm-decrypt-option nil
   "Option of decrypting encrypted parts.
@@ -682,14 +696,29 @@ mm-dissect-buffer
                                  (car ctl))
             (cons (car ctl) (mm-dissect-multipart ctl from))))
          (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))))
+          (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)))
@@ -1672,17 +1701,27 @@ 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                   ((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
=2D                       (format "Decrypt (S/MIME) part? "))))
=2D                  (mm-view-pkcs7 parts from))
=2D         (goto-char (point-min))
=2D         (insert "Content-type: text/plain\n\n")
=2D         (setq parts (mm-dissect-buffer t)))))
+      (add-text-properties 0 (length (car ctl))
+                          (list 'buffer (car parts))
+                          (car ctl))
+      (let* ((smime-type (cdr (assoc 'smime-type ctl)))
+             (envelope-p (string=3D smime-type "enveloped-data"))
+             (decrypt-or-sign-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
+                     ((eq decrypt-or-sign-option 'never) nil)
+                     ((eq decrypt-or-sign-option 'always) t)
+                     ((eq decrypt-or-sign-option 'known) t)
+                     (t (y-or-n-p
+                         (format question)))))
+           (mm-view-pkcs7 parts from)
+           (goto-char (point-min))
+           (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 828ac633dc..34da9464ce 100644
=2D-- a/lisp/gnus/mm-view.el
+++ b/lisp/gnus/mm-view.el
@@ -591,8 +591,15 @@ mm-view-pkcs7-verify
         (with-temp-buffer
           (insert-buffer-substring (mm-handle-buffer handle))
           (goto-char (point-min))
=2D          (let ((part (base64-decode-string (buffer-string))))
=2D            (epg-verify-string (epg-make-context 'CMS) part))))
+           (let* ((part (base64-decode-string (buffer-string)))
+                  (context (epg-make-context 'CMS))
+                  (plain (epg-verify-string context part)))
+             (mm-sec-status
+              'gnus-info
+              (epg-verify-result-to-string (epg-context-result-for context=
 'verify))
+              'gnus-details
+              nil)
+             plain)))
       (with-temp-buffer
        (insert "MIME-Version: 1.0\n")
        (mm-insert-headers "application/pkcs7-mime" "base64" "smime.p7m")
@@ -601,6 +608,10 @@ mm-view-pkcs7-verify
       (if verified
          (insert verified)
        (insert-buffer-substring smime-details-buffer)))
+    (goto-char (point-min))
+    (while (search-forward "\r\n" nil t)
+      (replace-match "\n"))
+    (goto-char (point-min))
     t))

 (autoload 'epg-decrypt-string "epg")
@@ -612,7 +623,15 @@ 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))
+                (plain (epg-decrypt-string context part)))
+           (mm-sec-status
+            'gnus-info
+            "OK"
+            'gnus-details
+            nil)
+           plain)))
     ;; Use openssl
     (insert "MIME-Version: 1.0\n")
     (mm-insert-headers "application/pkcs7-mime" "base64" "smime.p7m")
=2D-
2.25.2

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

iQFMBAEBCAA2FiEExRi5b+8xM5Vpvu7L3jJw+EOyhogFAmHEvIoYHHNlYmFzdGlh
bi5maWViZXJAd2ViLmRlAAoJEN4ycPhDsoaIkKkIAJ+tu42DlPifuSut5AlCdslt
ecuVg2xg5xG5P64kxmLr7OcW0SnS+CaUEOj4tynxuVuMayUIKXNqmWJFHEpaQYUc
rWbJ666P2IhyUIq1cb/hu7iJ3i7u2wHUns35WoqFC+bTI+O/zfmnpfVfBVvK6Xg0
W4TPePd6JiXyIrFGArusukQP6TZ6mn7vYNcETYWZ+UlBV/UgzQAc5LSx8eoFPXoi
qkA6QLYI+TE2uGKbtzEBC7eqDF2HBnNbTOasrlJTVvEfIEd9IYBCXv6nEqvrITHg
reYp75OTHSNop2ZI/fIQJLHWFb+KzOZBsrJ0rpnAOp8LYp7/p+iejigCNOmvEgA=
=JY09
-----END PGP SIGNATURE-----

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime encrypted and signed message

[Sebastian Fieber]

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


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!
-----BEGIN PGP SIGNATURE-----

iQFMBAEBCAA2FiEExRi5b+8xM5Vpvu7L3jJw+EOyhogFAmHEvTgYHHNlYmFzdGlh
bi5maWViZXJAd2ViLmRlAAoJEN4ycPhDsoaI9YAIAKjmNi3mDtze0QwDU5vIA8AC
vvtofx7euJu1cxzm8H8bRg4XbtXWUYWXJF9LSDApwQZHqDohDGCLL2AlplOvy3Y4
tdZ7kjGxd7u20CwPPUC9/ILK9h3u+S03svUZrHyQnVseaL83r/z1NjjdLvzD18jz
MCa7j/knnsUnXmt/uWaXbudgh11L4FhdoJuttZmIdQ4UksnIKj0dJ5MflIDoIWEU
7dL3DOV6RYl+ntl8cdPyEdeY8r97uEW22NZawp5rVgXxrIz+kNuwgGAMcWNnytzw
se7h2O/Q8jtura+tPz4A7T19dZOcOtKFnoQWBnQUpJtkevSCqPgmjvTc+B1H+mw=
=43kN
-----END PGP SIGNATURE-----

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime encrypted and signed message

[Sebastian Fieber]

[-- 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-----

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime encrypted and signed message

[Sebastian Fieber]

[-- Attachment #1: Type: text/plain, Size: 315 bytes --]


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!

Last mail seems to be broken.  Here is the correct patch again.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-PATCH-fix-bug-40397.patch --]
[-- Type: text/x-patch, Size: 15592 bytes --]

From 84ebb0331a0e16b1b767483c9d0bd1c140d73f09 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:
- 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

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

- 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"

- 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

- 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

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

- adjust gnus-mime-security-verify-or-decrypt to handle pkcs7-mime
  right with the done changes
---
 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
--- 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)
-	     (mm-decrypt-option 'known)
-	     (nparts (mm-possibly-verify-or-decrypt (cdr handle) handle)))
-	(unless (eq nparts (cdr handle))
-	  (mm-destroy-parts (cdr handle))
-	  (setcdr handle nparts))))
+            (mm-decrypt-option 'known)
+             (pkcs7-mime-p (or (equal (car handle) "application/pkcs7-mime")
+                               (equal (car handle) "application/x-pkcs7-mime")))
+            (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
-	   (or (nth 2 (assoc protocol mm-verify-function-alist))
-	       (nth 2 (assoc protocol mm-decrypt-function-alist))
-	       "Unknown")
-	   (if (equal (car handle) "multipart/signed")
-	       " Signed" " Encrypted")
-	   " Part"))
-	 (gnus-tmp-info
-	  (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 'protocol)
+                              "application/pkcs7-mime_signed-data"))
+                  " Signed")
+                 ((and (equal (car handle) "application/pkcs7-mime")
+                       (equal (mm-handle-multipart-ctl-parameter handle 'protocol)
+                              "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 'protocol)
+                              "application/x-pkcs7-mime_signed-data"))
+                  " Signed")
+                 ((and (equal (car handle) "application/x-pkcs7-mime")
+                       (equal (mm-handle-multipart-ctl-parameter handle 'protocol)
+                              "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
--- 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")

 (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"
-     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)))

 (defcustom mm-verify-option 'never
   "Option of verifying signed parts.
@@ -501,11 +510,16 @@ mm-verify-option

 (autoload 'mml2015-decrypt "mml2015")
 (autoload 'mml2015-decrypt-test "mml2015")
+(autoload 'mm-view-pkcs7-decrypt "mm-view")

 (defvar mm-decrypt-function-alist
   '(("application/pgp-encrypted" mml2015-decrypt "PGP" mml2015-decrypt-test)
     ("application/x-gnus-pgp-encrypted" mm-uu-pgp-encrypted-extract-1 "PGP"
-     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/MIME" nil)))

 (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))))
-	  (t
-	   (mm-possibly-verify-or-decrypt
-	    (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)
-	    ctl from))))
-	(when id
-	  (when (string-match " *<\\(.*\\)> *" id)
-	    (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 handle 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 intermediate-result))))
+             intermediate-result))))
+        (when id
+          (when (string-match " *<\\(.*\\)> *" id)
+            (setq id (match-string 1 id)))
 	  (push (cons id result) mm-content-id-alist))
 	result))))

@@ -1677,43 +1706,40 @@ mm-possibly-verify-or-decrypt
     (cond
      ((or (equal type "application/x-pkcs7-mime")
 	  (equal type "application/pkcs7-mime"))
-      (with-temp-buffer
-	(when (and (cond
-		    ((equal smime-type "signed-data") t)
-		    ((eq mm-decrypt-option 'never) nil)
-		    ((eq mm-decrypt-option 'always) t)
-		    ((eq mm-decrypt-option 'known) t)
-		    (t (y-or-n-p "Decrypt (S/MIME) part? ")))
-		   (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
-		(if (equal smime-type "signed-data")
-		    (list (propertize
-			   "multipart/signed"
-			   'protocol "application/pkcs7-signature"
-			   'gnus-info
-			   (format
-			    "%s:%s"
-			    (get-text-property 0 'gnus-info
-					       (car mm-security-handle))
-			    (get-text-property 0 'gnus-details
-					       (car mm-security-handle))))
-			  (mm-dissect-buffer t)
-			  parts)
-		  (mm-dissect-buffer t))))))
+      (add-text-properties 0 (length (car ctl))
+                           (list 'buffer (car parts))
+                           (car ctl))
+      (let* ((envelope-p (string= 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
--- 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)
-	       (let ((result (car (epg-context-result-for context 'verify))))
+	       (let ((result (epg-context-result-for context 'verify)))
 		 (mm-sec-status
-		  'gnus-info (epg-signature-status result)
-		  'gnus-details
-		  (format "%s:%s" (epg-signature-validity result)
-			  (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)
-	(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")
--
2.20.1

bug#40397: 28.0.50; epg decrypt does not verify signed content in smime encrypted and signed message

[Lars Ingebrigtsen]

Sebastian Fieber <sebastian.fieber@web.de> writes:

> Last mail seems to be broken.  Here is the correct patch again.

Thanks; applied to Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no