From f9ccee5c4c6cb0d4197b08ebeb36c1dceffe82e8 Mon Sep 17 00:00:00 2001
From: Thorsten Kukuk <kukuk@suse.com>
Date: Thu, 14 Nov 2024 10:27:28 +0100
Subject: [PATCH] pam_access: rework resolving of tokens as hostname

* modules/pam_access/pam_access.c: separate resolving of IP addresses
  from hostnames. Don't resolve TTYs or display variables as hostname
  (#834).
  Add "nodns" option to disallow resolving of tokens as hostname.
* modules/pam_access/pam_access.8.xml: document nodns option
* modules/pam_access/access.conf.5.xml: document that hostnames should
  be written as FQHN.

Upstream-Status: Backport [https://github.com/linux-pam/linux-pam/commit/940747f88c16e029b69a74e80a2e94f65cb3e628]
CVE: CVE-2024-10963
Signed-off-by: Stanislav Vovk <stanislav.vovk@est.tech>
---
 modules/pam_access/access.conf.5.xml |  4 ++
 modules/pam_access/pam_access.8.xml  | 46 ++++++++++++------
 modules/pam_access/pam_access.c      | 72 +++++++++++++++++++++++++++-
 3 files changed, 105 insertions(+), 17 deletions(-)

diff --git a/modules/pam_access/access.conf.5.xml b/modules/pam_access/access.conf.5.xml
index ff1cb223..158fc7df 100644
--- a/modules/pam_access/access.conf.5.xml
+++ b/modules/pam_access/access.conf.5.xml
@@ -220,10 +220,14 @@
       the fields they are ignored. However if the list separator is changed with the
       <emphasis>listsep</emphasis> option, the spaces will become part of the actual
       item and the line will be most probably ignored. For this reason, it is not
       recommended to put spaces around the ':' characters.
     </para>
+    <para>
+      Hostnames should be written as Fully-Qualified Host Name (FQHN) to avoid
+      confusion with device names or PAM service names.
+    </para>
   </refsect1>
 
   <refsect1 xml:id="access.conf-see_also">
     <title>SEE ALSO</title>
     <para>
diff --git a/modules/pam_access/pam_access.8.xml b/modules/pam_access/pam_access.8.xml
index 010e749e..1182c907 100644
--- a/modules/pam_access/pam_access.8.xml
+++ b/modules/pam_access/pam_access.8.xml
@@ -20,15 +20,18 @@
     <cmdsynopsis xml:id="pam_access-cmdsynopsis" sepchar=" ">
       <command>pam_access.so</command>
       <arg choice="opt" rep="norepeat">
         debug
       </arg>
+      <arg choice="opt" rep="norepeat">
+        noaudit
+      </arg>
       <arg choice="opt" rep="norepeat">
         nodefgroup
       </arg>
       <arg choice="opt" rep="norepeat">
-        noaudit
+        nodns
       </arg>
       <arg choice="opt" rep="norepeat">
         accessfile=<replaceable>file</replaceable>
       </arg>
       <arg choice="opt" rep="norepeat">
@@ -127,10 +130,37 @@
             Do not report logins from disallowed hosts and ttys to the audit subsystem.
           </para>
         </listitem>
       </varlistentry>
 
+      <varlistentry>
+        <term>
+          nodefgroup
+        </term>
+        <listitem>
+          <para>
+            User tokens which are not enclosed in parentheses will not be
+	    matched against the group database. The backwards compatible default is
+            to try the group database match even for tokens not enclosed
+            in parentheses.
+          </para>
+        </listitem>
+      </varlistentry>
+
+      <varlistentry>
+        <term>
+          nodns
+        </term>
+        <listitem>
+          <para>
+	    Do not try to resolve tokens as hostnames, only IPv4 and IPv6
+	    addresses will be resolved. Which means to allow login from a
+	    remote host, the IP addresses need to be specified in <filename>access.conf</filename>.
+          </para>
+        </listitem>
+      </varlistentry>
+
       <varlistentry>
         <term>
           fieldsep=separators
         </term>
         <listitem>
@@ -168,24 +198,10 @@
             "Domain Admins" contain a space.
           </para>
         </listitem>
       </varlistentry>
 
-      <varlistentry>
-        <term>
-          nodefgroup
-        </term>
-        <listitem>
-          <para>
-            User tokens which are not enclosed in parentheses will not be
-	    matched against the group database. The backwards compatible default is
-            to try the group database match even for tokens not enclosed
-            in parentheses.
-          </para>
-        </listitem>
-      </varlistentry>
-
     </variablelist>
   </refsect1>
 
   <refsect1 xml:id="pam_access-types">
     <title>MODULE TYPES PROVIDED</title>
diff --git a/modules/pam_access/pam_access.c b/modules/pam_access/pam_access.c
index f70b7e49..d06496c3 100644
--- a/modules/pam_access/pam_access.c
+++ b/modules/pam_access/pam_access.c
@@ -97,10 +97,11 @@ struct login_info {
     const char *config_file;
     const char *hostname;
     int debug;				/* Print debugging messages. */
     int only_new_group_syntax;		/* Only allow group entries of the form "(xyz)" */
     int noaudit;			/* Do not audit denials */
+    int nodns;                          /* Do not try to resolve tokens as hostnames */
     const char *fs;			/* field separator */
     const char *sep;			/* list-element separator */
     int from_remote_host;               /* If PAM_RHOST was used for from */
     struct addrinfo *res;		/* Cached DNS resolution of from */
     int gai_rv;				/* Cached retval of getaddrinfo */
@@ -148,10 +149,12 @@ parse_args(pam_handle_t *pamh, struct login_info *loginfo,
 	    loginfo->debug = YES;
 	} else if (strcmp (argv[i], "nodefgroup") == 0) {
 	    loginfo->only_new_group_syntax = YES;
 	} else if (strcmp (argv[i], "noaudit") == 0) {
 	    loginfo->noaudit = YES;
+	} else if (strcmp (argv[i], "nodns") == 0) {
+	    loginfo->nodns = YES;
 	} else {
 	    pam_syslog(pamh, LOG_ERR, "unrecognized option [%s]", argv[i]);
 	}
     }
 
@@ -730,11 +733,11 @@ remote_match (pam_handle_t *pamh, char *tok, struct login_info *item)
 
     if (tok[0] == '.') {			/* domain: match last fields */
       if ((str_len = strlen(string)) > tok_len
 	  && strcasecmp(tok, string + str_len - tok_len) == 0)
 	return YES;
-    } else if (tok[tok_len - 1] == '.') {       /* internet network numbers (end with ".") */
+    } else if (tok[tok_len - 1] == '.') {       /* internet network numbers/subnet (end with ".") */
       struct addrinfo hint;
 
       memset (&hint, '\0', sizeof (hint));
       hint.ai_flags = AI_CANONNAME;
       hint.ai_family = AF_INET;
@@ -805,10 +808,43 @@ string_match (pam_handle_t *pamh, const char *tok, const char *string,
     }
     return (NO);
 }
 
 
+static int
+is_device (pam_handle_t *pamh, const char *tok)
+{
+  struct stat st;
+  const char *dev = "/dev/";
+  char *devname;
+
+  devname = malloc (strlen(dev) + strlen (tok) + 1);
+  if (devname == NULL) {
+      pam_syslog(pamh, LOG_ERR, "Cannot allocate memory for device name: %m");
+      /*
+       * We should return an error and abort, but pam_access has no good
+       * error handling.
+       */
+      return NO;
+  }
+
+  char *cp = stpcpy (devname, dev);
+  strcpy (cp, tok);
+
+  if (lstat(devname, &st) != 0)
+    {
+      free (devname);
+      return NO;
+    }
+  free (devname);
+
+  if (S_ISCHR(st.st_mode))
+    return YES;
+
+  return NO;
+}
+
 /* network_netmask_match - match a string against one token
  * where string is a hostname or ip (v4,v6) address and tok
  * represents either a hostname, a single ip (v4,v6) address
  * or a network/netmask
  */
@@ -866,14 +902,46 @@ network_netmask_match (pam_handle_t *pamh,
 	if (getaddrinfo (tok, NULL, NULL, &ai) != 0)
 	  {
 	    return NO;
 	  }
       }
+    else if (isipaddr(tok, NULL, NULL) == YES)
+      {
+	if (getaddrinfo (tok, NULL, NULL, &ai) != 0)
+	  {
+	    if (item->debug)
+	      pam_syslog(pamh, LOG_DEBUG, "cannot resolve IP address \"%s\"", tok);
+
+	    return NO;
+	  }
+	netmask_ptr = NULL;
+      }
+    else if (item->nodns)
+      {
+	/* Only hostnames are left, which we would need to resolve via DNS */
+	return NO;
+      }
     else
       {
+	/* Bail out on X11 Display entries and ttys. */
+	if (tok[0] == ':')
+	  {
+	    if (item->debug)
+	      pam_syslog (pamh, LOG_DEBUG,
+			  "network_netmask_match: tok=%s is X11 display", tok);
+	    return NO;
+	  }
+	if (is_device (pamh, tok))
+	  {
+	    if (item->debug)
+	      pam_syslog (pamh, LOG_DEBUG,
+			  "network_netmask_match: tok=%s is a TTY", tok);
+	    return NO;
+	  }
+
         /*
-	 * It is either an IP address or a hostname.
+	 * It is most likely a hostname.
 	 * Let getaddrinfo sort everything out
 	 */
 	if (getaddrinfo (tok, NULL, NULL, &ai) != 0)
 	  {
 	    pam_syslog(pamh, LOG_ERR, "cannot resolve hostname \"%s\"", tok);
-- 
2.43.5

