From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Jimmy Yuen Ho Wong Newsgroups: gmane.emacs.bugs Subject: bug#31946: 27.0.50; The NSM should warn about more TLS problems Date: Sun, 1 Jul 2018 00:15:49 +0100 Message-ID: References: <87fu1apchn.fsf@gmail.com> <83in65r4n9.fsf@gnu.org> <87y3f1njku.fsf@gmail.com> <87tvpnojgt.fsf@gmail.com> <87lgawm2z3.fsf@gmail.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" X-Trace: blaine.gmane.org 1530400510 9245 195.159.176.226 (30 Jun 2018 23:15:10 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sat, 30 Jun 2018 23:15:10 +0000 (UTC) Cc: Lars Ingebrigtsen , 31946@debbugs.gnu.org To: Noam Postavsky Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sun Jul 01 01:15:05 2018 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fZP56-0002Ia-Ss for geb-bug-gnu-emacs@m.gmane.org; Sun, 01 Jul 2018 01:15:05 +0200 Original-Received: from localhost ([::1]:48261 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fZP7D-0006tw-PP for geb-bug-gnu-emacs@m.gmane.org; Sat, 30 Jun 2018 19:17:15 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:50849) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fZP75-0006te-8S for bug-gnu-emacs@gnu.org; Sat, 30 Jun 2018 19:17:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fZP70-0001q9-GE for bug-gnu-emacs@gnu.org; Sat, 30 Jun 2018 19:17:05 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:33625) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fZP70-0001pc-Ba for bug-gnu-emacs@gnu.org; Sat, 30 Jun 2018 19:17:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1fZP70-0007cw-0S for bug-gnu-emacs@gnu.org; Sat, 30 Jun 2018 19:17:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Jimmy Yuen Ho Wong Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 30 Jun 2018 23:17:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 31946 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: security Original-Received: via spool by 31946-submit@debbugs.gnu.org id=B31946.153040057829264 (code B ref 31946); Sat, 30 Jun 2018 23:17:01 +0000 Original-Received: (at 31946) by debbugs.gnu.org; 30 Jun 2018 23:16:18 +0000 Original-Received: from localhost ([127.0.0.1]:41522 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fZP6H-0007bw-M3 for submit@debbugs.gnu.org; Sat, 30 Jun 2018 19:16:18 -0400 Original-Received: from mail-io0-f181.google.com ([209.85.223.181]:42835) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1fZP6F-0007bj-Co for 31946@debbugs.gnu.org; Sat, 30 Jun 2018 19:16:15 -0400 Original-Received: by mail-io0-f181.google.com with SMTP id r24-v6so11620101ioh.9 for <31946@debbugs.gnu.org>; Sat, 30 Jun 2018 16:16:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=EygzMz1dz5XAbew+qaF8nfup1wAFWQDmG/iHmHBWiL0=; b=IP0enpcFxNLeCuiFOvpwZN8FWDHFvTXGQwhVHhJKrODqlWJvXVCQdV7h/TKGccznJh VbGl4PGtTcGW+j44B7UUx27HE2sK9hEC8nfupG4MczJfkzw2LJzM2066XrBTyJ4NVtdf ErAyWULQurXAwu0XykWcQ0/Daq0o1spCGeYkx3divYesQtIA8pNWD42zoe5mtaAKDRh4 r/doJvgFlXEs0G0JATDqkIBPHSXKo8rCBkZEjcWDWGZTvo8sZ4SEh7y/nOvTYda1J7lL v91qwiaMAXWmUhidu7JMjQviI+VzSzIAJ43fe9Dxvx0T1vNfhlDkDLvRDdAKUH8yB4MU GrGQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=EygzMz1dz5XAbew+qaF8nfup1wAFWQDmG/iHmHBWiL0=; b=rbOc/f9EEb/OKxCSoKNR/72yVVqwaJQw47lwIU7Cn/sj6+HrSjbTadi2rSZxpYUUjl LncQrmmmg77WDCkwGAP2s5hg+8J9Esaoi7feLG2rTV7Zi3Kzerc2vZpoSq8doKDEBwy1 QKBNVwedFKnj1O5l7YnsvG5yg9MD+YHGYtQ6+JYO4/pdSqkhXjLJeYM2e9I0gQYEGbz0 99SxV/N7Pb1oOyOMP1gQcaMzwF3/1G8iQh0tReyE17K9MHuCow5AWZRCNgYYXpkp4tEo oJkjUaPFeuyETFctinDx47HY3eooJXTFsyygJJVMr7u2DabjPv3owxViosqM/gEhHSUa rCng== X-Gm-Message-State: APt69E0S96TITwqcQmtLoMt/VVxfCGitdXSm2baIw1JCq3AbrWUAoQ5S uxoh/tvbLZWzq9Ee6K/Fkuy3q1zhvjsn0IiwO/c= X-Google-Smtp-Source: AAOMgpeM0WVnUSTcG4r1aQlbKThfvNHGL/cDtN+lw+pozL17Lq+4q3iJaJx7ktBwjalz7qwCezKSciJLZUvjxy99qVQ= X-Received: by 2002:a6b:e008:: with SMTP id z8-v6mr4522109iog.296.1530400569747; Sat, 30 Jun 2018 16:16:09 -0700 (PDT) Original-Received: by 2002:a02:985d:0:0:0:0:0 with HTTP; Sat, 30 Jun 2018 16:15:49 -0700 (PDT) In-Reply-To: <87lgawm2z3.fsf@gmail.com> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:148033 Archived-At: Thanks. I'll update the patch later. I'm having a headache about what to do with the prompts now. Currently, if a cert didn't verify, there's a prompt. If a cert did verify but was "changed", I get another prompt. But "change" is determined by a public key ID hash instead of the actual fingerprint (but `nsm-fingerprint` treat the public key ID as the fingerprint), and the prompt happens *before* cipher suite checks, in which you may get another prompt. I think I need to do another pass at this to get the check ordering right so I can get just one prompt for all problems found. Basically, I'm trying to break the certificate check into multiple checks (I'll do that for DHE too) and figure out what to do with the fingerprint. Let me know if I'm going way off track here... On Sat, Jun 30, 2018 at 9:30 PM, Noam Postavsky wrote: > Jimmy Yuen Ho Wong writes: > >> I've manually tested this patch a bit, but please give this patch a >> look and see if I've missed anything. I need all the feedbacks I can >> get for this. > > Overall, I'd say this looks pretty good. Some (mostly minor) comments > on the details below. > >> * lisp/net/nsm.el >> (nsm-check-certificate, nsm-fingerprint-ok-p, >> nsm-check-plain-connection): Pre-format query messages before passing > > It should be formatted as > > (nsm-check-certificate, nsm-fingerprint-ok-p) > (nsm-check-plain-connection): Pre-format query messages before passing > >> (nsm-protocol-check--diffie-hellman-prime-bits): Rename to >> nsm-protocol-check--dhe-kx. Checks for prime bits < 1024 for 'medium > ^ > > Periods should be double spaced, this applies in docstrings as well. > >> nsm-protocol-check--rc4-cipher. Fix bug where it was previously >> checking for non-existent cipher name RC4 in GnuTLS instead of >> ARCFOUR. > > Yikes, that's a good catch. > >> (defvar network-security-protocol-checks >> + '((rsa-kx high) >> + (dhe-kx medium) >> + (anon-kx medium) >> + (export-kx medium) >> + (cbc-cipher high) >> + (ecdsa-cbc-cipher medium) >> + (3des-cipher medium) >> + (des-cipher medium) >> + (rc4-cipher medium) >> + (rc2-cipher medium) >> + (null-cipher medium) >> + (sha1-sig medium) >> + (md5-sig medium) >> (ssl medium)) > >> @@ -198,87 +207,370 @@ network-security-protocol-checks >> HOST PORT STATUS OPTIONAL-PARAMETER.") >> >> (defun nsm-check-protocol (process host port status settings) >> + (let ((results >> + (cl-remove-if-not >> + #'cdr >> + (cl-loop for check in network-security-protocol-checks > > This cl-remove-if-not over a cl-loop collect seems a bit awkward. How > about > > (cl-loop for (name level . _) in network-security-protocol-checks > for type = (intern (format ":%s" name)) > ;; Skip the check if the user has already said that this > ;; host is OK for this type of "error". > for result = > (and (not (memq type (plist-get settings :conditions))) > (>= (nsm-level network-security-level) > (nsm-level level)) > (funcall (intern (format "nsm-protocol-check--%s" name)) > host port status)) > when result > collect (cons type result)) > >> +(defun nsm-protocol-check--dhe-kx (host port status) >> + "Check for finite field ephemeral Diffie-Hellman key exchange. >> + >> +If `network-security-level' is 'medium, and a DHE key exchange >> +method was used, this function queries the user if the prime bit >> +length is < 1024. >> + >> +If `network-security-level' is 'high or above, and a DHE key >> +exchange method was used, this function queries the user even if >> +the prime bit length is >= 1024. > > It's kind of inconvenient that this function hardcodes the security > levels; it also makes reading the current settings more difficult (e.g., > when I saw (dhe-kx medium) at first, I thought you were going to warn > about DHE on level medium). Can we do better here? Maybe split in two? > (By the way, the network-security-level values in docstrings should be > formatted as `medium' and `high', not single quoted.) > >> +In 2014, the discovery of Logjam[1] had proven non-elliptic-curve >> +Diffie-Hellman key exchange with < 1024 prime bit length to be >> +unsafe. > > I'd actually say, DH smaller than 1024 bits was known to be unsafe > before that, the logjam attack allows a man-in-the-middle to downgrade > what would have been a >= 1024 bit connection to "export" grade (e.g., > 512 bits). > >> + (if (and (>= (nsm-level network-security-level) (nsm-level 'medium)) >> + (< prime-bits 1024)) >> + (setq msg (format-message >> + "Diffie-Hellman prime bits (%s) too low (%s)" > > I would phrase this as > > "Diffie-Hellman prime bits (%d) lower than `gnutls-min-prime-bits' (%d)" > >> + prime-bits gnutls-min-prime-bits))) >> + (if (>= (nsm-level network-security-level) (nsm-level 'high)) >> + (setq msg (concat >> + msg >> + (format-message >> + "non-elliptic-curve ephemeral Diffie-Hellman key exchange method (%s) maybe using an unsafe prime" > > I would phrase this as > > "non-standardized Diffie-Hellman parameters cannot be validated" > > (this covers the non-elliptic-curveness as well; the reason elliptic > curves are safe is that they're standardized and pre-validated.) > > And you're missing a space between the messages, in the case where you > hit both of them. > >> +(defun nsm-protocol-check--anon-kx (host port status) >> + "Check for anonymous key exchange. >> + >> +Anonymouse key exchange exposes the connection to MITM attacks. >> + >> +Reference: >> + >> +GnuTLS authors (2018). \"GnuTLS Manual 4.3.3 Anonymous >> +authentication\", >> +`https://www.gnutls.org/manual/gnutls.html\#Anonymous-authentication'" > ^ > typo? > >> +(defun nsm-protocol-check--export-kx (host port status) >> + "Check for EXPORT key exchange. >> + >> +EXPORT cipher suites are a family of 40-bit effective security >> +algorithms legally exportable by the United States in the early 90s. >> +They can be broken in seconds on 2018 hardware. >> + >> +Recent version of GnuTLS does not enable this key exchange by default, > > This should be "Recent versions of GnuTLS do not..." > >> +but can be enabled if requested. This check is mainly provided to > ^ > it > >> +;; Cipher checks >> + >> +(defun nsm-protocol-check--cbc-cipher (host port status) >> + "Check for CBC mode ciphers. >> + >> +CBC mode cipher in TLS versions earlier than 1.3 are problematic >> +because of MAC-then-encrypt. This construction is vulnerable to >> +padding oracle attacks[1]. > > I think the TLS version reference should be dropped, unless TLS 1.3 uses > CBC with encrypt-then-MAC? I understood it just deprecates CBC > altogether. > >> +(defun nsm-protocol-check--3des-cipher (host port status) >> + "Check for 3DES ciphers. >> + >> +3DES is considered a weak cipher by NIST as it only has 80 bits > > Is it possible to distinguish between 3DES 2-key and 3DES 3-key? (the > latter giving 112 bit security, which is still a bit low, but probably > acceptable for medium level) >