Re: A couple of questions and concerns about Emacs network security

[Jimmy Yuen Ho Wong <wyuenho@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1715 bytes --]

Here's the updated patch.

Suggested material to include in the manual or NEWS:

Emacs now has preliminary checks for revoked TLS certificates. To utililize
this feature, users are advised to install the IGTF trust anchor
distribution and FetchCRL packages for their system, and update their
.crl.pem files periodically in order to get the most updated Certificate
Revocation Lists. Refer to the docstring of `gnutls-crlfiles' for details
on how to supply their file locations to Emacs.



On Mon, Jun 25, 2018 at 6:16 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Jimmy Yuen Ho Wong <wyuenho@gmail.com>
> > Date: Sun, 24 Jun 2018 22:30:50 +0100
> > Cc: Eli Zaretskii <eliz@gnu.org>, Paul Eggert <eggert@cs.ucla.edu>,
> >       Noam Postavsky <npostavs@gmail.com>, emacs-devel@gnu.org
> >
> > Here's the patch to get GnuTLS to do CRL checking.
>
> Thanks, a few minor comments:
>
> > +(defcustom gnutls-crlfiles
> > +  '(
> > +    "/etc/grid-security/certificates/*.crl.pem"
> > +    )
> > +  "List of CRL file paths or a function returning said list.
> > +If a file path contains a glob pattern, it will be expanded.
> > +The files may be in PEM or DER format, as per the GnuTLS documentation.
> > +The files may not exist, in which case they will be ignored."
> > +  :group 'gnutls
> > +  :type '(choice (function :tag "Function to produce list of CRL
> filenames")
> > +                 (repeat (file :tag "CRL filename"))))
>
> This should have a :version tag.
>
> Do we really want/need to allow a function here?  Isn't a list of
> files enough?  A function is more dangerous than a string, especially
> in a security-sensitive place.
>
> Finally, I think this defcustom needs to be called out in NEWS.
>

[-- Attachment #1.2: Type: text/html, Size: 2577 bytes --]

[-- Attachment #2: 0001-Check-TLS-certs-against-CRL.patch --]
[-- Type: application/octet-stream, Size: 3164 bytes --]

From 9a9b4232a28fc8a20d0d217e90541f5c7e27c7dd Mon Sep 17 00:00:00 2001
From: Jimmy Yuen Ho Wong <wyuenho@gmail.com>
Date: Mon, 25 Jun 2018 18:46:57 +0100
Subject: [PATCH] Check TLS certs against CRL

* lisp/net/gnutls.el (gnutls-boot-parameters): Return
  `gnutls-crlfiles' in `:crlfiles'.
  (gnutls-crlfiles): New defcustom.
  (gnutls--get-files): New defun.
  (gnutls-trustfiles, gnutls-crlfiles): Delegate to
  `gnutls--get-files' to return a list of filenames, accepts glob pattern.
---
 lisp/net/gnutls.el | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/lisp/net/gnutls.el b/lisp/net/gnutls.el
index 09df019e2e..650dbf8943 100644
--- a/lisp/net/gnutls.el
+++ b/lisp/net/gnutls.el
@@ -96,12 +96,26 @@ gnutls-trustfiles
     "/etc/ssl/cert.pem"                      ; macOS
     )
   "List of CA bundle location filenames or a function returning said list.
+If a file path contains glob wildcards, they will be expanded.
 The files may be in PEM or DER format, as per the GnuTLS documentation.
 The files may not exist, in which case they will be ignored."
   :group 'gnutls
   :type '(choice (function :tag "Function to produce list of bundle filenames")
                  (repeat (file :tag "Bundle filename"))))
 
+(defcustom gnutls-crlfiles
+  '(
+    "/etc/grid-security/certificates/*.crl.pem"
+    )
+  "List of CRL file paths or a function returning said list.
+If a file path contains glob wildcards, they will be expanded.
+The files may be in PEM or DER format, as per the GnuTLS documentation.
+The files may not exist, in which case they will be ignored."
+  :group 'gnutls
+  :type '(choice (function :tag "Function to produce list of CRL filenames")
+                 (repeat (file :tag "CRL filename")))
+  :version "27.1")
+
 ;;;###autoload
 (defcustom gnutls-min-prime-bits 256
   ;; Several mail servers send fewer bits than the GnuTLS default.
@@ -263,6 +277,7 @@ gnutls-log-level
 It must be omitted, a number, or nil; if omitted or nil it
 defaults to GNUTLS_VERIFY_ALLOW_X509_V1_CA_CRT."
   (let* ((trustfiles (or trustfiles (gnutls-trustfiles)))
+         (crlfiles (or crlfiles (gnutls-crlfiles)))
          (maybe-dumbfw (if (memq 'ClientHello\ Padding (gnutls-available-p))
                            ":%DUMBFW"
                          ""))
@@ -304,13 +319,18 @@ gnutls-log-level
                 :verify-error ,verify-error
                 :callbacks nil)))
 
+(defun gnutls--get-files (files)
+  (cl-loop for f in files
+           if f do (setq f (if (functionp f) (funcall f) f))
+           append (cl-delete-if-not #'file-exists-p (file-expand-wildcards f t))))
+
 (defun gnutls-trustfiles ()
   "Return a list of usable trustfiles."
-  (delq nil
-        (mapcar (lambda (f) (and f (file-exists-p f) f))
-                (if (functionp gnutls-trustfiles)
-                    (funcall gnutls-trustfiles)
-                  gnutls-trustfiles))))
+  (gnutls--get-files gnutls-trustfiles))
+
+(defun gnutls-crlfiles ()
+  "Return a list of usable CRL files."
+  (gnutls--get-files gnutls-crlfiles))
 
 (declare-function gnutls-error-string "gnutls.c" (error))
 
-- 
2.18.0