summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorChristian Breunig <christian@breunig.cc>2025-12-09 23:14:50 +0100
committerChristian Breunig <christian@breunig.cc>2025-12-17 21:26:58 +0100
commitcd11ba957a33933742304b4efbba77461e803cb3 (patch)
treec2ecb82a831f3e7c763579bf451b7c456b41502f
parent3d5ebd5956c4e4333e66097d6d339c2329c21295 (diff)
downloadvyos-1x-cd11ba957a33933742304b4efbba77461e803cb3.tar.gz
vyos-1x-cd11ba957a33933742304b4efbba77461e803cb3.zip
login: T8086: replace getpwall() user enumeration to avoid NSS/TACACS timeouts
The previous implementation of "system login" relied on Python's pwd.getpwall() to enumerate user accounts. This forces a full walk through the NSS stack, which is acceptable in general but problematic for our use-case. VyOS only needs information about locally created accounts and not remote accounts provided via AAA backends such as TACACS or RADIUS. When TACACS servers are unreachable, NSS lookups become extremely slow due to repeated timeouts. As a result, any operation triggering pwd.getpwall() (including configuration commits) can stall for several minutes. This change introduces a dedicated helper, get_local_passwd_entries(), which reads /etc/passwd directly and avoids NSS entirely. Since only local UIDs are relevant, this provides all required data with no external dependencies. Performance improvement on VyOS 1.4.3 with two unreachable TACACS servers: # set system login tacacs server 192.168.1.50 key test123 # set system login tacacs server 192.168.1.51 key test123 # time commit Before: real 3m29.825s user 0m0.329s sys 0m0.246s After: real 0m1.464s user 0m0.337s sys 0m0.195s This significantly improves commit performance and removes sensitivity to AAA server outages.
-rw-r--r--python/vyos/utils/auth.py59
-rwxr-xr-xsrc/op_mode/show_users.py4
-rw-r--r--src/tests/test_utils.py45
-rw-r--r--src/tests/test_utils_auth.py75
4 files changed, 134 insertions, 49 deletions
diff --git a/python/vyos/utils/auth.py b/python/vyos/utils/auth.py
index 462332332..4aa446455 100644
--- a/python/vyos/utils/auth.py
+++ b/python/vyos/utils/auth.py
@@ -18,10 +18,13 @@ import math
import re
import string
-from enum import StrEnum
+from dataclasses import dataclass
from decimal import Decimal
-from pwd import getpwall
from pwd import getpwnam
+from enum import StrEnum
+from typing import List
+from typing import Optional
+
from vyos.utils.process import cmd
# Minimum UID used when adding system users
@@ -146,12 +149,62 @@ def get_current_user() -> str:
current_user = os.environ['USER']
return current_user
+@dataclass
+class PasswdEntry:
+ pw_name: str
+ pw_passwd: str
+ pw_uid: int
+ pw_gid: int
+ pw_gecos: str
+ pw_dir: str
+ pw_shell: str
+
+def get_local_passwd_entries(uid: Optional[int] = None) -> PasswdEntry | List[PasswdEntry] | None:
+ """
+ If uid is None: return a list of all passwd entries.
+ If uid is given: return the matching entry or None.
+ """
+ entries = []
+ with open('/etc/passwd', 'r') as f:
+ for line in f:
+ line = line.strip()
+ if not line or line.startswith("#"):
+ continue
+ parts = line.split(":")
+ if len(parts) != 7:
+ continue
+
+ try:
+ entry = PasswdEntry(
+ pw_name=parts[0],
+ pw_passwd=parts[1],
+ pw_uid=int(parts[2]),
+ pw_gid=int(parts[3]),
+ pw_gecos=parts[4],
+ pw_dir=parts[5],
+ pw_shell=parts[6],
+ )
+ except ValueError:
+ # Skip entries with non-numeric UID or GID
+ continue
+
+ # If searching for a specific UID, return immediately if found
+ if uid is not None and entry.pw_uid == uid:
+ return entry
+
+ entries.append(entry)
+
+ # uid given but not found
+ if uid is not None:
+ return None
+
+ return entries
def get_local_users(min_uid=MIN_USER_UID, max_uid=MAX_USER_UID) -> list:
"""Return list of dynamically allocated users (see Debian Policy Manual)"""
local_users = []
- for s_user in getpwall():
+ for s_user in get_local_passwd_entries():
if s_user.pw_uid < min_uid:
continue
if s_user.pw_uid > max_uid:
diff --git a/src/op_mode/show_users.py b/src/op_mode/show_users.py
index bccfaf991..c60b9932b 100755
--- a/src/op_mode/show_users.py
+++ b/src/op_mode/show_users.py
@@ -79,7 +79,9 @@ def list_users():
vyos_users = cfg.list_effective_nodes('system login user')
users = []
with open('/var/log/lastlog', 'rb') as lastlog_file:
- for (name, _, uid, _, _, _, _) in pwd.getpwall():
+ for entry in get_local_passwd_entries():
+ name = entry.pw_name
+ uid = entry.pw_uid
lastlog_info = decode_lastlog(lastlog_file, uid)
if lastlog_info is None:
continue
diff --git a/src/tests/test_utils.py b/src/tests/test_utils.py
index 12fda84d0..9cf6f7a1e 100644
--- a/src/tests/test_utils.py
+++ b/src/tests/test_utils.py
@@ -12,12 +12,8 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-import pwd
from unittest import TestCase
-from vyos.utils import auth
-
-
class TestVyOSUtils(TestCase):
def test_key_mangling(self):
from vyos.utils.dict import mangle_dict_keys
@@ -41,44 +37,3 @@ class TestVyOSUtils(TestCase):
self.assertEqual(list_strip(lst, rsb, right=True), ['a', 'b', 'c'])
self.assertEqual(list_strip(lst, non), [])
self.assertEqual(list_strip(sub, lst), [])
-
-class TestVyOSUtilsAuth(TestCase):
-
- def test_get_local_users_returns_existing_usernames(self):
- # Returned users exist, skip list is excluded, and UIDs are in range
-
- all_users = set(s_user.pw_name for s_user in pwd.getpwall())
- local_users = auth.get_local_users()
-
- # All returned users must really exist
- for user in local_users:
- self.assertIn(user, all_users)
-
- # Nobody in the skip list
- for skipped in auth.SYSTEM_USER_SKIP_LIST:
- self.assertNotIn(skipped, local_users)
-
- # All are within UID range
- for s_user in pwd.getpwall():
- if s_user.pw_name in local_users:
- self.assertGreaterEqual(s_user.pw_uid, auth.MIN_USER_UID)
- self.assertLessEqual(s_user.pw_uid, auth.MAX_USER_UID)
-
- def test_get_user_home_dir_for_real_user(self):
- # User's homedir is a non-empty string for a valid user
-
- local_users = auth.get_local_users()
- if local_users:
- for user in local_users:
- home_dir = auth.get_user_home_dir(user)
- self.assertIsInstance(home_dir, str)
- self.assertTrue(bool(home_dir)) # Should not be empty
- else:
- self.skipTest("No suitable non-system users found on this system")
-
- def test_get_user_home_dir_invalid_user(self):
- # Raises KeyError for nonexistent username
-
- user = "__this_user_does_not_exist__" # Test using unlikely username
- with self.assertRaises(KeyError):
- auth.get_user_home_dir(user)
diff --git a/src/tests/test_utils_auth.py b/src/tests/test_utils_auth.py
new file mode 100644
index 000000000..f3b52ab29
--- /dev/null
+++ b/src/tests/test_utils_auth.py
@@ -0,0 +1,75 @@
+# Copyright VyOS maintainers and contributors <maintainers@vyos.io>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 or later as
+# published by the Free Software Foundation.
+#
+# This program 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 this program. If not, see <http://www.gnu.org/licenses/>.
+
+import pwd
+import unittest
+
+from vyos.utils import auth
+
+class TestVyOSUtilsAuth(unittest.TestCase):
+ def test_uid_root(self):
+ self.assertEqual(auth.get_local_passwd_entries(0).pw_name, 'root')
+ self.assertEqual(auth.get_local_passwd_entries(0).pw_uid, 0)
+
+ def test_uid_daemon(self):
+ uid = None
+ for user in auth.get_local_passwd_entries():
+ if user.pw_name == 'daemon':
+ uid = user.pw_uid
+ break
+
+ self.assertEqual(auth.get_local_passwd_entries(uid).pw_name, 'daemon')
+ self.assertEqual(auth.get_local_passwd_entries(uid).pw_uid, uid)
+
+ def test_uid_not_found(self):
+ self.assertEqual(auth.get_local_passwd_entries(5465487635), None)
+
+ def test_get_local_users_returns_existing_usernames(self):
+ # Returned users exist, skip list is excluded, and UIDs are in range
+
+ all_users = set(s_user.pw_name for s_user in pwd.getpwall())
+ local_users = auth.get_local_users()
+
+ # All returned users must really exist
+ for user in local_users:
+ self.assertIn(user, all_users)
+
+ # Nobody in the skip list
+ for skipped in auth.SYSTEM_USER_SKIP_LIST:
+ self.assertNotIn(skipped, local_users)
+
+ # All are within UID range
+ for s_user in pwd.getpwall():
+ if s_user.pw_name in local_users:
+ self.assertGreaterEqual(s_user.pw_uid, auth.MIN_USER_UID)
+ self.assertLessEqual(s_user.pw_uid, auth.MAX_USER_UID)
+
+ def test_get_user_home_dir_for_real_user(self):
+ # User's homedir is a non-empty string for a valid user
+
+ local_users = auth.get_local_users()
+ if local_users:
+ for user in local_users:
+ home_dir = auth.get_user_home_dir(user)
+ self.assertIsInstance(home_dir, str)
+ self.assertTrue(bool(home_dir)) # Should not be empty
+ else:
+ self.skipTest("No suitable non-system users found on this system")
+
+ def test_get_user_home_dir_invalid_user(self):
+ # Raises KeyError for nonexistent username
+
+ user = "__this_user_does_not_exist__" # Test using unlikely username
+ with self.assertRaises(KeyError):
+ auth.get_user_home_dir(user)