From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.io/gmane.emacs.gnus.general/83954 Path: news.gmane.org!not-for-mail From: Ted Zlatanov Newsgroups: gmane.emacs.gnus.general,gmane.emacs.devel Subject: Re: Builtin GnuTLS support and certificate verification Date: Sun, 08 Dec 2013 11:08:18 -0500 Organization: =?utf-8?B?0KLQtdC+0LTQvtGAINCX0LvQsNGC0LDQvdC+0LI=?= @ Cienfuegos Message-ID: <87ob4rwcjx.fsf@flea.lifelogs.com> References: <87iowbt5dq.fsf@guybrush.luffy.cx> <878ux782na.fsf@dex.adm.naquadah.org> <874n7uu2gg.fsf@guybrush.luffy.cx> <87txftsnub.fsf@flea.lifelogs.com> <87li13q3dy.fsf@flea.lifelogs.com> <87a9hjaj2d.fsf@guybrush.luffy.cx> <87r4anhrh3.fsf@flea.lifelogs.com> <871u2g1ofu.fsf@dex.adm.naquadah.org> <87vbz0vun4.fsf@flea.lifelogs.com> <87y53v7n44.fsf@guybrush.luffy.cx> Reply-To: ding@gnus.org NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: ger.gmane.org 1386518899 19546 80.91.229.3 (8 Dec 2013 16:08:19 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 8 Dec 2013 16:08:19 +0000 (UTC) Cc: emacs-devel@gnu.org To: ding@gnus.org Original-X-From: ding-owner+M32209@lists.math.uh.edu Sun Dec 08 17:08:24 2013 Return-path: Envelope-to: ding-account@gmane.org Original-Received: from util0.math.uh.edu ([129.7.128.18]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1Vpgu6-0003uM-Lw for ding-account@gmane.org; Sun, 08 Dec 2013 17:08:23 +0100 Original-Received: from localhost ([127.0.0.1] helo=lists.math.uh.edu) by util0.math.uh.edu with smtp (Exim 4.63) (envelope-from ) id 1VpgtK-0001zE-KQ; Sun, 08 Dec 2013 10:07:34 -0600 Original-Received: from mx1.math.uh.edu ([129.7.128.32]) by util0.math.uh.edu with esmtps (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1VpgtH-0001yz-Bk for ding@lists.math.uh.edu; Sun, 08 Dec 2013 10:07:31 -0600 Original-Received: from quimby.gnus.org ([80.91.231.51]) by mx1.math.uh.edu with esmtps (TLSv1:AES128-SHA:128) (Exim 4.76) (envelope-from ) id 1VpgtA-0006zi-SV for ding@lists.math.uh.edu; Sun, 08 Dec 2013 10:07:30 -0600 Original-Received: from plane.gmane.org ([80.91.229.3]) by quimby.gnus.org with esmtp (Exim 4.80) (envelope-from ) id 1Vpgt8-0003DI-Ur for ding@gnus.org; Sun, 08 Dec 2013 17:07:22 +0100 Original-Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1Vpgt7-0003Ru-NL for ding@gnus.org; Sun, 08 Dec 2013 17:07:21 +0100 Original-Received: from c-98-229-61-72.hsd1.ma.comcast.net ([98.229.61.72]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Sun, 08 Dec 2013 17:07:21 +0100 Original-Received: from tzz by c-98-229-61-72.hsd1.ma.comcast.net with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Sun, 08 Dec 2013 17:07:21 +0100 X-Injected-Via-Gmane: http://gmane.org/ Mail-Followup-To: ding@gnus.org Original-Lines: 289 Original-X-Complaints-To: usenet@ger.gmane.org X-Gmane-NNTP-Posting-Host: c-98-229-61-72.hsd1.ma.comcast.net X-Face: bd.DQ~'29fIs`T_%O%C\g%6jW)yi[zuz6;d4V0`@y-~$#3P_Ng{@m+e4o<4P'#(_GJQ%TT= D}[Ep*b!\e,fBZ'j_+#"Ps?s2!4H2-Y"sx" Mail-Copies-To: never User-Agent: Gnus/5.130008 (Ma Gnus v0.8) Emacs/24.3.50 (gnu/linux) Cancel-Lock: sha1:+jmGaevbISUGni7/HSBcGKdoGTA= X-Spam-Score: -2.0 (--) List-ID: Precedence: bulk Xref: news.gmane.org gmane.emacs.gnus.general:83954 gmane.emacs.devel:166204 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On Sun, 08 Dec 2013 09:39:23 +0100 Vincent Bernat wrote: VB> ❦ 8 décembre 2013 05:22 CET, Ted Zlatanov  : >>>> Verification options could be: >>>> >>>> - `expired-certificate` >>>> - `revoked-certificate` >>>> - `untrusted-certificate` >>>> - `hostname-mismatch` >> >> I'm not sure this granularity is necessary. I just have :trustfiles and >> :hostname as options right now. Anyone else with an opinion? VB> I thought you wanted something granular, hence the proposition. I don't VB> know of any other software proposing granular verification. So, no VB> problem with dropping this. Yeah, I went back and forth. I think two (:trustfiles and :hostname) verification options is the right balance for now. I couldn't think of any cases where I'd want more granularity, but that's why I'm asking :) >> My concern is that suddenly connections will start failing for our users >> and bug reports will flow, and I don't have time to explain to everyone >> why their self-signed certificates need exceptions. This can be really, >> really annoying. But logging in *Messages* is not very useful either, >> users don't read it. So what's the right thing? How about a default >> behavior of flashing a warning, then sit-for 3 seconds? A hard error >> can be optional but not the default. VB> For a first release, just default to the previous behaviour. However, VB> some day, it would be nice to be "secure by default". http://debbugs.gnu.org/13374 and other related bugs are about enabling "secure by default" and I would like to make that change now, before the Emacs code freeze. It feels like the sit-for option is safest. It preserves the current behavior while forcing the user to notice something's wrong. In the next release, based on user feedback, we can turn verify-error on. If no one disagrees, I will implement it on top of the patch attached here. >> I'm also not sure I like the look and feel of the `gnutls-verify-error' >> defcustom. It's kind of awkward. VB> I am don't know defcustom enough to know if this would give something VB> like this: VB> '((".*\\.internal" :hostname) VB> (".*" :trustfiles :hostname)) VB> If yes, this seems fine for me. Just add the possible values for tags in VB> the document string. Yes, except the cdr is a nested list because I use the "set" customization type. So it's '((".*" (:hostname :trustfiles))) >> I'd like to get this done before the Emacs code freeze next week or so. >> Please give me your opinions and test the code. VB> I'll try to test later this day. Thanks. Posting patch to emacs-devel as well for review and feedback. Ted --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=gnutls-verify-error.patch === modified file 'lisp/net/gnutls.el' --- lisp/net/gnutls.el 2013-09-18 04:50:54 +0000 +++ lisp/net/gnutls.el 2013-12-08 04:00:04 +0000 @@ -51,6 +51,19 @@ :type '(choice (const nil) string)) +(defcustom gnutls-verify-error nil + "If non-nil, this should be a list of checks per hostname regex or t." + :group 'gnutls + :type '(choice + (const t) + (repeat :tag "List of hostname regexps with flags for each" + (list + (choice :tag "Hostname" + (const ".*" :tag "Any hostname") + regexp) + (set (const :trustfiles) + (const :hostname)))))) + (defcustom gnutls-trustfiles '( "/etc/ssl/certs/ca-certificates.crt" ; Debian, Ubuntu, Gentoo and Arch Linux @@ -138,19 +151,25 @@ \(see `gnutls-min-prime-bits' for more information). Use nil for the default. -When VERIFY-HOSTNAME-ERROR is not nil, an error will be raised -when the hostname does not match the presented certificate's host -name. The exact verification algorithm is a basic implementation -of the matching described in RFC2818 (HTTPS), which takes into -account wildcards, and the DNSName/IPAddress subject alternative -name PKIX extension. See GnuTLS' gnutls_x509_crt_check_hostname -for details. When VERIFY-HOSTNAME-ERROR is nil, only a warning -will be issued. - -When VERIFY-ERROR is not nil, an error will be raised when the -peer certificate verification fails as per GnuTLS' -gnutls_certificate_verify_peers2. Otherwise, only warnings will -be shown about the verification failure. +VERIFY-HOSTNAME-ERROR is a backwards compatibility option for +putting `:hostname' in VERIFY-ERROR. + +When VERIFY-ERROR is t or a list containing `:trustfiles', an +error will be raised when the peer certificate verification fails +as per GnuTLS' gnutls_certificate_verify_peers2. Otherwise, only +warnings will be shown about the verification failure. + +When VERIFY-ERROR is t or a list containing `:hostname', an error +will be raised when the hostname does not match the presented +certificate's host name. The exact verification algorithm is a +basic implementation of the matching described in +RFC2818 (HTTPS), which takes into account wildcards, and the +DNSName/IPAddress subject alternative name PKIX extension. See +GnuTLS' gnutls_x509_crt_check_hostname for details. Otherwise, +only a warning will be issued. + +Note that the list in `gnutls-verify-error', matched against the +HOSTNAME, is the default VERIFY-ERROR. VERIFY-FLAGS is a numeric OR of verification flags only for `gnutls-x509pki' connections. See GnuTLS' x509.h for details; @@ -183,8 +202,28 @@ (if gnutls-algorithm-priority (upcase gnutls-algorithm-priority) "NORMAL"))))) + (verify-error (or verify-error + ;; this uses the value of `gnutls-verify-error' + (cond + ;; if t, pass it on + ((eq gnutls-verify-error t) + t) + ;; if a list, look for hostname matches + ((listp gnutls-verify-error) + (mapcan + (lambda (check) + (when (string-match (car check) hostname) + (cdr check))) + gnutls-verify-error)) + ;; else it's nil + (t nil)))) (min-prime-bits (or min-prime-bits gnutls-min-prime-bits)) - (params `(:priority ,priority-string + params ret) + + (when verify-hostname-error + (push :hostname verify-error)) + + (setq params `(:priority ,priority-string :hostname ,hostname :loglevel ,gnutls-log-level :min-prime-bits ,min-prime-bits @@ -193,9 +232,7 @@ :keylist ,keylist :verify-flags ,verify-flags :verify-error ,verify-error - :verify-hostname-error ,verify-hostname-error :callbacks nil)) - ret) (gnutls-message-maybe (setq ret (gnutls-boot process type params)) === modified file 'src/gnutls.c' --- src/gnutls.c 2013-11-30 13:31:39 +0000 +++ src/gnutls.c 2013-12-08 04:10:44 +0000 @@ -49,7 +49,7 @@ static Lisp_Object QCgnutls_bootprop_hostname; static Lisp_Object QCgnutls_bootprop_min_prime_bits; static Lisp_Object QCgnutls_bootprop_verify_flags; -static Lisp_Object QCgnutls_bootprop_verify_hostname_error; +static Lisp_Object QCgnutls_bootprop_verify_error; /* Callback keys for `gnutls-boot'. Unused currently. */ static Lisp_Object QCgnutls_bootprop_callbacks_verify; @@ -753,8 +753,12 @@ :verify-flags is a bitset as per GnuTLS' gnutls_certificate_set_verify_flags. -:verify-hostname-error, if non-nil, makes a hostname mismatch an -error. Otherwise it will be just a warning. +:verify-hostname-error is ignored. Pass :hostname in :verify-error +instead. + +:verify-error is a list of symbols to express verification checks or +`t' to do all checks. Currently it can contain `:trustfiles' and +`:hostname' to verify the certificate or the hostname respectively. :min-prime-bits is the minimum accepted number of bits the client will accept in Diffie-Hellman key exchange. @@ -798,8 +802,7 @@ /* Lisp_Object callbacks; */ Lisp_Object loglevel; Lisp_Object hostname; - /* Lisp_Object verify_error; */ - Lisp_Object verify_hostname_error; + Lisp_Object verify_error; Lisp_Object prime_bits; CHECK_PROCESS (proc); @@ -818,11 +821,14 @@ keylist = Fplist_get (proplist, QCgnutls_bootprop_keylist); crlfiles = Fplist_get (proplist, QCgnutls_bootprop_crlfiles); loglevel = Fplist_get (proplist, QCgnutls_bootprop_loglevel); - verify_hostname_error = Fplist_get (proplist, QCgnutls_bootprop_verify_hostname_error); + verify_error = Fplist_get (proplist, QCgnutls_bootprop_verify_error); prime_bits = Fplist_get (proplist, QCgnutls_bootprop_min_prime_bits); + if (!Flistp (verify_error)) + error ("gnutls-boot: invalid :verify_error parameter (not a list)"); + if (!STRINGP (hostname)) - error ("gnutls-boot: invalid :hostname parameter"); + error ("gnutls-boot: invalid :hostname parameter (not a string)"); c_hostname = SSDATA (hostname); state = XPROCESS (proc)->gnutls_state; @@ -1047,14 +1053,17 @@ if (peer_verification != 0) { - if (NILP (verify_hostname_error)) - GNUTLS_LOG2 (1, max_log_level, "certificate validation failed:", - c_hostname); - else - { + if (EQ (verify_error, Qt) + || !NILP (Fmember (QCgnutls_bootprop_trustfiles, verify_error))) + { emacs_gnutls_deinit (proc); error ("Certificate validation failed %s, verification code %d", c_hostname, peer_verification); + } + else + { + GNUTLS_LOG2 (1, max_log_level, "certificate validation failed:", + c_hostname); } } @@ -1094,14 +1103,17 @@ if (!fn_gnutls_x509_crt_check_hostname (gnutls_verify_cert, c_hostname)) { - if (NILP (verify_hostname_error)) - GNUTLS_LOG2 (1, max_log_level, "x509 certificate does not match:", - c_hostname); - else - { + if (EQ (verify_error, Qt) + || !NILP (Fmember (QCgnutls_bootprop_hostname, verify_error))) + { fn_gnutls_x509_crt_deinit (gnutls_verify_cert); emacs_gnutls_deinit (proc); error ("The x509 certificate does not match \"%s\"", c_hostname); + } + else + { + GNUTLS_LOG2 (1, max_log_level, "x509 certificate does not match:", + c_hostname); } } fn_gnutls_x509_crt_deinit (gnutls_verify_cert); @@ -1161,7 +1173,7 @@ DEFSYM (QCgnutls_bootprop_min_prime_bits, ":min-prime-bits"); DEFSYM (QCgnutls_bootprop_loglevel, ":loglevel"); DEFSYM (QCgnutls_bootprop_verify_flags, ":verify-flags"); - DEFSYM (QCgnutls_bootprop_verify_hostname_error, ":verify-hostname-error"); + DEFSYM (QCgnutls_bootprop_verify_error, ":verify-error"); DEFSYM (Qgnutls_e_interrupted, "gnutls-e-interrupted"); Fput (Qgnutls_e_interrupted, Qgnutls_code, --=-=-=--