Merge pull request #2256 from zdc/T5577-circinus

T5577: Optimized PAM configs for RADIUS/TACACS+

11 files changed, 141 insertions, 62 deletions

diff --git a/debian/vyos-1x.postinst b/debian/vyos-1x.postinst
index b43416152..860319edf 100644
--- a/debian/vyos-1x.postinst
+++ b/debian/vyos-1x.postinst
@@ -29,14 +29,9 @@ do
     sed -i "/^# Standard Un\*x authentication\./i${PAM_CONFIG}" $file
 done
 
-# We do not make use of a TACACS UNIX group - drop it
-if grep -q '^tacacs' /etc/group; then
-    delgroup tacacs
-fi
-
-# Both RADIUS and TACACS users belong to aaa group - this must be added first
-if ! grep -q '^aaa' /etc/group; then
-    addgroup --firstgid 1000 --quiet aaa
+# We need to have a group for RADIUS service users to use it inside PAM rules
+if ! grep -q '^radius' /etc/group; then
+    addgroup --firstgid 1000 --quiet radius
 fi
 
 # Remove TACACS user added by base package - we use our own UID range and group
@@ -53,6 +48,11 @@ if grep -q '^tacacs' /etc/passwd; then
     fi
 fi
 
+# Remove TACACS+ PAM default profile
+if [[ -e /usr/share/pam-configs/tacplus ]]; then
+    rm /usr/share/pam-configs/tacplus
+fi
+
 # Add TACACS system users required for TACACS based system authentication
 if ! grep -q '^tacacs' /etc/passwd; then
     # Add the tacacs group and all 16 possible tacacs privilege-level users to
@@ -64,14 +64,13 @@ if ! grep -q '^tacacs' /etc/passwd; then
     level=0
     vyos_group=vyattaop
     while [ $level -lt 16 ]; do
-        adduser --quiet --system --firstuid 900 --disabled-login --ingroup users \
+        adduser --quiet --system --firstuid 900 --disabled-login --ingroup tacacs \
             --no-create-home --gecos "TACACS+ mapped user at privilege level ${level}" \
             --shell /bin/vbash tacacs${level}
         adduser --quiet tacacs${level} frrvty
         adduser --quiet tacacs${level} adm
         adduser --quiet tacacs${level} dip
         adduser --quiet tacacs${level} users
-        adduser --quiet tacacs${level} aaa
         if [ $level -lt 15 ]; then
             adduser --quiet tacacs${level} vyattaop
             adduser --quiet tacacs${level} operator
@@ -87,7 +86,7 @@ fi
 
 # Add RADIUS operator user for RADIUS authenticated users to map to
 if ! grep -q '^radius_user' /etc/passwd; then
-    adduser --quiet --firstuid 1000 --disabled-login --ingroup users \
+    adduser --quiet --firstuid 1000 --disabled-login --ingroup radius \
         --no-create-home --gecos "RADIUS mapped user at privilege level operator" \
         --shell /sbin/radius_shell radius_user
     adduser --quiet radius_user frrvty
@@ -96,12 +95,11 @@ if ! grep -q '^radius_user' /etc/passwd; then
     adduser --quiet radius_user adm
     adduser --quiet radius_user dip
     adduser --quiet radius_user users
-    adduser --quiet radius_user aaa
 fi
 
 # Add RADIUS admin user for RADIUS authenticated users to map to
 if ! grep -q '^radius_priv_user' /etc/passwd; then
-    adduser --quiet --firstuid 1000 --disabled-login --ingroup users \
+    adduser --quiet --firstuid 1000 --disabled-login --ingroup radius \
         --no-create-home --gecos "RADIUS mapped user at privilege level admin" \
         --shell /sbin/radius_shell radius_priv_user
     adduser --quiet radius_priv_user frrvty
@@ -112,7 +110,6 @@ if ! grep -q '^radius_priv_user' /etc/passwd; then
     adduser --quiet radius_priv_user disk
     adduser --quiet radius_priv_user users
     adduser --quiet radius_priv_user frr
-    adduser --quiet radius_priv_user aaa
 fi
 
 # add hostsd group for vyos-hostsd
diff --git a/debian/vyos-1x.preinst b/debian/vyos-1x.preinst
index 16c118cb7..9bd6331a8 100644
--- a/debian/vyos-1x.preinst
+++ b/debian/vyos-1x.preinst
@@ -2,8 +2,6 @@ dpkg-divert --package vyos-1x --add --no-rename /etc/securetty
 dpkg-divert --package vyos-1x --add --no-rename /etc/security/capability.conf
 dpkg-divert --package vyos-1x --add --no-rename /lib/systemd/system/lcdproc.service
 dpkg-divert --package vyos-1x --add --no-rename /etc/logrotate.d/conntrackd
-dpkg-divert --package vyos-1x --add --no-rename /usr/share/pam-configs/radius
-dpkg-divert --package vyos-1x --add --no-rename /usr/share/pam-configs/tacplus
 dpkg-divert --package vyos-1x --add --no-rename /etc/rsyslog.conf
 dpkg-divert --package vyos-1x --add --no-rename /etc/skel/.bashrc
 dpkg-divert --package vyos-1x --add --no-rename /etc/skel/.profile
diff --git a/interface-definitions/include/radius-server-ipv4-ipv6.xml.i b/interface-definitions/include/radius-server-ipv4-ipv6.xml.i
index efd418bb2..a0cdcd7c3 100644
--- a/interface-definitions/include/radius-server-ipv4-ipv6.xml.i
+++ b/interface-definitions/include/radius-server-ipv4-ipv6.xml.i
@@ -46,6 +46,26 @@
         <multi/>
       </properties>
     </leafNode>
+    <leafNode name="security-mode">
+      <properties>
+        <help>Security mode for RADIUS authentication</help>
+        <completionHelp>
+          <list>mandatory optional</list>
+        </completionHelp>
+        <valueHelp>
+          <format>mandatory</format>
+          <description>Deny access immediately if RADIUS answers with Access-Reject</description>
+        </valueHelp>
+        <valueHelp>
+          <format>optional</format>
+          <description>Pass to the next authentication method if RADIUS answers with Access-Reject</description>
+        </valueHelp>
+        <constraint>
+          <regex>(mandatory|optional)</regex>
+        </constraint>
+      </properties>
+      <defaultValue>optional</defaultValue>
+    </leafNode>
   </children>
 </node>
 <!-- include end -->
diff --git a/interface-definitions/system-login.xml.in b/interface-definitions/system-login.xml.in
index 71db8b1d6..30fea91b0 100644
--- a/interface-definitions/system-login.xml.in
+++ b/interface-definitions/system-login.xml.in
@@ -259,6 +259,26 @@
                   </constraint>
                 </properties>
               </leafNode>
+              <leafNode name="security-mode">
+                <properties>
+                  <help>Security mode for TACACS+ authentication</help>
+                  <completionHelp>
+                    <list>mandatory optional</list>
+                  </completionHelp>
+                  <valueHelp>
+                    <format>mandatory</format>
+                    <description>Deny access immediately if TACACS+ answers with REJECT</description>
+                  </valueHelp>
+                  <valueHelp>
+                    <format>optional</format>
+                    <description>Pass to the next authentication method if TACACS+ answers with REJECT</description>
+                  </valueHelp>
+                  <constraint>
+                    <regex>(mandatory|optional)</regex>
+                  </constraint>
+                </properties>
+                <defaultValue>optional</defaultValue>
+              </leafNode>
               #include <include/radius-timeout.xml.i>
               #include <include/interface/vrf.xml.i>
             </children>
diff --git a/src/conf_mode/system-login.py b/src/conf_mode/system-login.py
index 02c97afaa..87a269499 100755
--- a/src/conf_mode/system-login.py
+++ b/src/conf_mode/system-login.py
@@ -104,6 +104,9 @@ def get_config(config=None):
     # prune TACACS global defaults if not set by user
     if login.from_defaults(['tacacs']):
         del login['tacacs']
+    # same for RADIUS
+    if login.from_defaults(['radius']):
+        del login['radius']
 
     # create a list of all users, cli and users
     all_users = list(set(local_users + cli_users))
@@ -377,17 +380,23 @@ def apply(login):
             except Exception as e:
                 raise ConfigError(f'Deleting user "{user}" raised exception: {e}')
 
-    # Enable RADIUS in PAM configuration
-    pam_cmd = '--remove'
+    # Enable/disable RADIUS in PAM configuration
+    cmd('pam-auth-update --disable radius-mandatory radius-optional')
     if 'radius' in login:
-        pam_cmd = '--enable'
-    cmd(f'pam-auth-update --package {pam_cmd} radius')
-
-    # Enable/Disable TACACS in PAM configuration
-    pam_cmd = '--remove'
+        if login['radius'].get('security_mode', '') == 'mandatory':
+            pam_profile = 'radius-mandatory'
+        else:
+            pam_profile = 'radius-optional'
+        cmd(f'pam-auth-update --enable {pam_profile}')
+
+    # Enable/disable TACACS+ in PAM configuration
+    cmd('pam-auth-update --disable tacplus-mandatory tacplus-optional')
     if 'tacacs' in login:
-        pam_cmd = '--enable'
-    cmd(f'pam-auth-update --package {pam_cmd} tacplus')
+        if login['tacacs'].get('security_mode', '') == 'mandatory':
+            pam_profile = 'tacplus-mandatory'
+        else:
+            pam_profile = 'tacplus-optional'
+        cmd(f'pam-auth-update --enable {pam_profile}')
 
     return None
 
diff --git a/src/pam-configs/radius b/src/pam-configs/radius
deleted file mode 100644
index eee9cb93e..000000000
--- a/src/pam-configs/radius
+++ /dev/null
@@ -1,20 +0,0 @@
-Name: RADIUS authentication
-Default: no
-Priority: 257
-Auth-Type: Primary
-Auth:
-    [default=ignore success=2] pam_succeed_if.so service = sudo
-    [default=ignore success=ignore] pam_succeed_if.so user ingroup aaa quiet
-    [authinfo_unavail=ignore success=end default=ignore] pam_radius_auth.so
-
-Account-Type: Primary
-Account:
-    [default=ignore success=2] pam_succeed_if.so service = sudo
-    [default=ignore success=ignore] pam_succeed_if.so user ingroup aaa quiet
-    [authinfo_unavail=ignore success=end perm_denied=bad default=ignore] pam_radius_auth.so
-
-Session-Type: Additional
-Session:
-    [default=ignore success=2] pam_succeed_if.so service = sudo
-    [default=ignore success=ignore] pam_succeed_if.so user ingroup aaa quiet
-    [authinfo_unavail=ignore success=ok default=ignore] pam_radius_auth.so
diff --git a/src/pam-configs/radius-mandatory b/src/pam-configs/radius-mandatory
new file mode 100644
index 000000000..3368fe7ff
--- /dev/null
+++ b/src/pam-configs/radius-mandatory
@@ -0,0 +1,19 @@
+Name: RADIUS authentication (mandatory mode)
+Default: no
+Priority: 576
+
+Auth-Type: Primary
+Auth-Initial:
+    [default=ignore success=end auth_err=die perm_denied=die user_unknown=die] pam_radius_auth.so
+Auth:
+    [default=ignore success=end auth_err=die perm_denied=die user_unknown=die] pam_radius_auth.so use_first_pass
+
+Account-Type: Primary
+Account:
+    [default=ignore success=1] pam_succeed_if.so user notingroup radius quiet
+    [default=ignore success=end] pam_radius_auth.so
+
+Session-Type: Additional
+Session:
+    [default=ignore success=1] pam_succeed_if.so user notingroup radius quiet
+    [default=bad success=ok] pam_radius_auth.so
diff --git a/src/pam-configs/radius-optional b/src/pam-configs/radius-optional
new file mode 100644
index 000000000..73085061d
--- /dev/null
+++ b/src/pam-configs/radius-optional
@@ -0,0 +1,19 @@
+Name: RADIUS authentication (optional mode)
+Default: no
+Priority: 576
+
+Auth-Type: Primary
+Auth-Initial:
+    [default=ignore success=end] pam_radius_auth.so
+Auth:
+    [default=ignore success=end] pam_radius_auth.so use_first_pass
+
+Account-Type: Primary
+Account:
+    [default=ignore success=1] pam_succeed_if.so user notingroup radius quiet
+    [default=ignore success=end] pam_radius_auth.so
+
+Session-Type: Additional
+Session:
+    [default=ignore success=1] pam_succeed_if.so user notingroup radius quiet
+    [default=ignore success=ok perm_denied=bad user_unknown=bad] pam_radius_auth.so
diff --git a/src/pam-configs/tacplus b/src/pam-configs/tacplus
deleted file mode 100644
index 66a1eaa4c..000000000
--- a/src/pam-configs/tacplus
+++ /dev/null
@@ -1,17 +0,0 @@
-Name: TACACS+ authentication
-Default: no
-Priority: 257
-Auth-Type: Primary
-Auth:
-    [default=ignore success=ignore] pam_succeed_if.so user ingroup aaa quiet
-    [authinfo_unavail=ignore success=end auth_err=bad default=ignore] pam_tacplus.so include=/etc/tacplus_servers login=login
-
-Account-Type: Primary
-Account:
-    [default=ignore success=ignore] pam_succeed_if.so user ingroup aaa quiet
-    [authinfo_unavail=ignore success=end perm_denied=bad default=ignore] pam_tacplus.so include=/etc/tacplus_servers login=login
-
-Session-Type: Additional
-Session:
-    [default=ignore success=ignore] pam_succeed_if.so user ingroup aaa quiet
-    [authinfo_unavail=ignore success=ok default=ignore] pam_tacplus.so include=/etc/tacplus_servers login=login
diff --git a/src/pam-configs/tacplus-mandatory b/src/pam-configs/tacplus-mandatory
new file mode 100644
index 000000000..ffccece19
--- /dev/null
+++ b/src/pam-configs/tacplus-mandatory
@@ -0,0 +1,17 @@
+Name: TACACS+ authentication (mandatory mode)
+Default: no
+Priority: 576
+
+Auth-Type: Primary
+Auth:
+    [default=ignore success=end auth_err=die perm_denied=die user_unknown=die] pam_tacplus.so include=/etc/tacplus_servers login=login
+
+Account-Type: Primary
+Account:
+    [default=ignore success=1] pam_succeed_if.so user notingroup tacacs quiet
+    [default=bad success=end] pam_tacplus.so include=/etc/tacplus_servers login=login
+
+Session-Type: Additional
+Session:
+    [default=ignore success=1] pam_succeed_if.so user notingroup tacacs quiet
+    [default=bad success=ok] pam_tacplus.so include=/etc/tacplus_servers login=login
diff --git a/src/pam-configs/tacplus-optional b/src/pam-configs/tacplus-optional
new file mode 100644
index 000000000..095c3a164
--- /dev/null
+++ b/src/pam-configs/tacplus-optional
@@ -0,0 +1,17 @@
+Name: TACACS+ authentication (optional mode)
+Default: no
+Priority: 576
+
+Auth-Type: Primary
+Auth:
+    [default=ignore success=end] pam_tacplus.so include=/etc/tacplus_servers login=login
+
+Account-Type: Primary
+Account:
+    [default=ignore success=1] pam_succeed_if.so user notingroup tacacs quiet
+    [default=ignore success=end auth_err=bad perm_denied=bad user_unknown=bad] pam_tacplus.so include=/etc/tacplus_servers login=login
+
+Session-Type: Additional
+Session:
+    [default=ignore success=1] pam_succeed_if.so user notingroup tacacs quiet
+    [default=ignore success=ok session_err=bad user_unknown=bad] pam_tacplus.so include=/etc/tacplus_servers login=login