[Emacs-diffs] netsec bc1cf28 2/2: Change nsm-should-check to look at local subnets

[Robert Pluim]

branch: netsec
commit bc1cf28da5532c6052eade7b5d19bb59e7e1f7bf
Author: Robert Pluim <address@hidden>
Commit: Robert Pluim <address@hidden>

    Change nsm-should-check to look at local subnets
    
    * lisp/net/nsm.el (nsm-network-same-subnet): New function.  Checks
    if an ip address is in the same subnet as another one.
    (nsm-should-check): Use nsm-network-same-subnet to see if we're
    connecting to a local subnet machine.  Remove checks for RFC1918 addresses.
    
    * test/lisp/net/nsm-tests.el: New file.  Test nsm-should-check 
functionality.
---
 lisp/net/nsm.el            | 85 ++++++++++++++++++++++------------------------
 test/lisp/net/nsm-tests.el | 69 +++++++++++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+), 44 deletions(-)

diff --git a/lisp/net/nsm.el b/lisp/net/nsm.el
index b59ea07..b0eff81 100644
--- a/lisp/net/nsm.el
+++ b/lisp/net/nsm.el
@@ -204,54 +204,51 @@ SETTINGS are the same as those supplied to each check 
function.
 RESULTS is an alist where the keys are the checks run and the
 values the results of the checks.")
 
+(defun nsm-network-same-subnet (local-ip mask ip)
+  "Returns t if IP is in the same subnet as LOCAL-IP/MASK.
+LOCAL-IP, MASK, and IP are specified as vectors of integers, and
+are expected to have the same length.  Works for both IPv4 and
+IPv6 addresses."
+  (let ((matches t)
+        (length (length local-ip)))
+    (unless (memq length '(4 5 8 9))
+      (error "Unexpected length of IP address %S" local-ip))
+    (dotimes (i length)
+      (setq matches (and matches
+                         (=
+                          (logand (aref local-ip i)
+                                  (aref mask i))
+                          (logand (aref ip i)
+                                  (aref mask i))))))
+    matches))
+
 (defun nsm-should-check (host)
   "Determines whether NSM should check for TLS problems for HOST.
 
 If `nsm-trust-local-network' is or returns non-nil, and if the
-host address is a localhost address, a machine address, a direct
-link or a private network address, this function returns
-nil.  Non-nil otherwise."
-  (let* ((address (or (nslookup-host-ipv4 host nil 'vector)
-                      (nslookup-host-ipv6 host nil 'vector)))
-         (ipv4? (eq (length address) 4)))
-    (not
-     (or (if ipv4?
-             (or
-              ;; (0.x.x.x) this machine
-              (eq (aref address 0) 0)
-              ;; (127.x.x.x) localhost
-              (eq (aref address 0) 0))
-           (or
-            ;; (::) IPv6 this machine
-            (not (cl-mismatch address [0 0 0 0 0 0 0 0]))
-            ;; (::1) IPv6 localhost
-            (not (cl-mismatch address [0 0 0 0 0 0 0 1]))))
-         (and (or (and (functionp nsm-trust-local-network)
-                       (funcall nsm-trust-local-network))
-                  nsm-trust-local-network)
-              (if ipv4?
-                  (or
-                   ;; (10.x.x.x) private
-                   (eq (aref address 0) 10)
-                   ;; (172.16.x.x) private
-                   (and (eq (aref address 0) 172)
-                        (eq (aref address 0) 16))
-                   ;; (192.168.x.x) private
-                   (and (eq (aref address 0) 192)
-                        (eq (aref address 0) 168))
-                   ;; (198.18.x.x) private
-                   (and (eq (aref address 0) 198)
-                        (eq (aref address 0) 18))
-                   ;; (169.254.x.x) link-local
-                   (and (eq (aref address 0) 169)
-                        (eq (aref address 0) 254)))
-                (memq (aref address 0)
-                      '(
-                        64512  ;; (fc00::) IPv6 unique local address
-                        64768  ;; (fd00::) IPv6 unique local address
-                        65152  ;; (fe80::) IPv6 link-local
-                        )
-                      )))))))
+host address is a localhost address, or in the same subnet as one
+of the local interfaces, this function returns nil.  Non-nil
+otherwise."
+  (let ((addresses (network-lookup-address-info host))
+        (network-interface-list (network-interface-list))
+        (off-net t))
+    (when
+     (or (and (functionp nsm-trust-local-network)
+              (funcall nsm-trust-local-network))
+         nsm-trust-local-network)
+     (mapc
+      (lambda (address)
+        (mapc
+         (lambda (iface)
+           (let ((info (network-interface-info (car iface))))
+             (when
+                 (nsm-network-same-subnet (substring (car info) 0 -1)
+                                          (substring (car (cddr info)) 0 -1)
+                                          address)
+               (setq off-net nil))))
+         network-interface-list))
+      addresses))
+     off-net))
 
 (defun nsm-check-tls-connection (process host port status settings)
   "Check TLS connection against potential security problems.
diff --git a/test/lisp/net/nsm-tests.el b/test/lisp/net/nsm-tests.el
new file mode 100644
index 0000000..bf6ac04
--- /dev/null
+++ b/test/lisp/net/nsm-tests.el
@@ -0,0 +1,69 @@
+;;; network-stream-tests.el --- tests for network security manager -*- 
lexical-binding: t; -*-
+
+;; Copyright (C) 2019 Free Software Foundation, Inc.
+
+;; Author: Robert Pluim <address@hidden>
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+
+;;; Code:
+
+(require 'nsm)
+(eval-when-compile (require 'cl-lib))
+
+(ert-deftest nsm-check-local-subnet-ipv4 ()
+  "Check that nsm can be avoided for local subnets."
+  (let ((local-ip '[172 26 128 160 0])
+        (mask '[255 255 255 0 0])
+
+        (wrong-length-mask '[255 255 255])
+        (wrong-mask '[255 255 255 255 0])
+        (remote-ip-yes '[172 26 128 161 0])
+        (remote-ip-no '[172 26 129 161 0]))
+
+    (should (eq t (nsm-network-same-subnet local-ip mask remote-ip-yes)))
+    (should (eq nil (nsm-network-same-subnet local-ip mask remote-ip-no)))
+    (should-error (nsm-network-same-subnet local-ip wrong-length-mask 
remote-ip-yes))
+    (should (eq nil (nsm-network-same-subnet local-ip wrong-mask 
remote-ip-yes)))
+    (should (eq t (nsm-should-check "google.com")))
+    (should (eq t (nsm-should-check "127.1")))
+    (should (eq t (nsm-should-check "localhost")))
+    (let ((nsm-trust-local-network t))
+      (should (eq t (nsm-should-check "google.com")))
+      (should (eq nil (nsm-should-check "127.1")))
+      (should (eq nil (nsm-should-check "localhost"))))))
+
+;; FIXME This will never return true, since
+;; network-interface-list only gives the primary address of each
+;; interface, which will be the IPv4 one
+(defun nsm-ipv6-is-available ()
+  (and (featurep 'make-network-process '(:family ipv6))
+       (cl-rassoc-if
+        (lambda (elt)
+          (eq 9 (length elt)))
+        (network-interface-list))))
+
+(ert-deftest nsm-check-local-subnet-ipv6 ()
+  (skip-unless (nsm-ipv6-is-available))
+  (should (eq t (nsm-should-check "::1")))
+  (let ((nsm-trust-local-network t))
+    (should (eq nil (nsm-should-check "::1")))))
+
+
+;;; nsm-tests.el ends here