From da535ef5697f6ce87a7f34ff185e4df239e6af63 Mon Sep 17 00:00:00 2001 From: Christian Poessinger Date: Fri, 14 Oct 2022 20:00:25 +0200 Subject: login: 2fa: T874: fix Google authenticator issues Move default values of TOTP configuration from a global to a per user setting. This makes the entire code easier as no global configuration must be blended into the per user config dict. Also it should be possible to set the authentication window "multiple concurrent keys" individual per user. set system login user vyos authentication otp key 'gzkmajid7na2oltajs4kbuq7lq' set system login user vyos authentication plaintext-password 'vyos' --- data/templates/login/pam_otp_ga.conf.j2 | 2 +- debian/vyos-1x.postinst | 4 +- interface-definitions/system-login.xml.in | 108 +++++++++++++---------------- smoketest/scripts/cli/test_system_login.py | 8 +++ src/conf_mode/system-login.py | 36 ++++++---- 5 files changed, 80 insertions(+), 78 deletions(-) diff --git a/data/templates/login/pam_otp_ga.conf.j2 b/data/templates/login/pam_otp_ga.conf.j2 index 4c1f411d1..cf51ce089 100644 --- a/data/templates/login/pam_otp_ga.conf.j2 +++ b/data/templates/login/pam_otp_ga.conf.j2 @@ -1,5 +1,5 @@ {% if authentication.otp.key is vyos_defined %} -{{ authentication.otp.key }} +{{ authentication.otp.key | upper }} " RATE_LIMIT {{ authentication.otp.rate_limit }} {{ authentication.otp.rate_time }} " WINDOW_SIZE {{ authentication.otp.window_size }} " DISALLOW_REUSE diff --git a/debian/vyos-1x.postinst b/debian/vyos-1x.postinst index 9766c91b1..031e91595 100644 --- a/debian/vyos-1x.postinst +++ b/debian/vyos-1x.postinst @@ -23,11 +23,11 @@ fi # Add 2FA support for SSH sudo grep -qF -- "auth required pam_google_authenticator.so nullok" "/etc/pam.d/sshd" || \ -sudo sed -i '/^@include common-auth/a # Check OTP 2FA, if configured for the user\nauth required pam_google_authenticator.so nullok' /etc/pam.d/sshd +sudo sed -i '/^@include common-auth/a # Check OTP 2FA, if configured for the user\nauth required pam_google_authenticator.so nullok' /etc/pam.d/sshd # Add 2FA support for local authentication sudo grep -qF -- "auth required pam_google_authenticator.so nullok" "/etc/pam.d/login" || \ -sudo sed -i '/^@include common-auth/a # Check OTP 2FA, if configured for the user\nauth required pam_google_authenticator.so nullok' /etc/pam.d/login +sudo sed -i '/^@include common-auth/a # Check OTP 2FA, if configured for the user\nauth required pam_google_authenticator.so nullok' /etc/pam.d/login # Add RADIUS operator user for RADIUS authenticated users to map to if ! grep -q '^radius_user' /etc/passwd; then diff --git a/interface-definitions/system-login.xml.in b/interface-definitions/system-login.xml.in index 7dd045e6c..def42544a 100644 --- a/interface-definitions/system-login.xml.in +++ b/interface-definitions/system-login.xml.in @@ -8,62 +8,6 @@ 400 - - - Global authentication settings - - - - - 2FA OTP authentication parameters - - - - - Number of attempts. Limit logins to N per every M seconds - - u32:1-10 - Number of attempts. Limit logins to N per every M seconds - - - - - Number of login attempts must me between 1 and 10 - - 3 - - - - Time interval. Limit logins to N per every M seconds - - u32:15-600 - Time interval. Limit logins to N per every M seconds - - - - - Rate limit time interval must be between 15 and 600 seconds - - 30 - - - - Set window of concurrently valid codes - - u32:1-21 - Set window of concurrently valid codes - - - - - Window of concurrently valid codes must be between 1 and 21 - - 3 - - - - - Local user account information @@ -75,7 +19,7 @@ - Password authentication + Authentication settings @@ -94,18 +38,60 @@ - 2FA OTP authentication parameters + One-Time-Pad (two-factor) authentication parameters + + + Limit number of logins (rate-limit) per rate-time + + u32:1-10 + Number of attempts + + + + + Number of login attempts must me between 1 and 10 + + 3 + + + + Limit number of logins (rate-limit) per rate-time + + u32:15-600 + Time interval + + + + + Rate limit time interval must be between 15 and 600 seconds + + 30 + + + + Set window of concurrently valid codes + + u32:1-21 + Window size + + + + + Window of concurrently valid codes must be between 1 and 21 + + 3 + - Token Key Secret key for the token algorithm (see RFC 4226) + Key/secret the token algorithm (see RFC4226) txt - OTP key (base32 encoded secret) + Base32 encoded key/token - [a-zA-Z2-7]{20,10000} + [a-zA-Z2-7]{26,10000} Key must only include base32 characters and be at least 26 characters long diff --git a/smoketest/scripts/cli/test_system_login.py b/smoketest/scripts/cli/test_system_login.py index a99721d66..6006fe0f6 100755 --- a/smoketest/scripts/cli/test_system_login.py +++ b/smoketest/scripts/cli/test_system_login.py @@ -46,6 +46,14 @@ TTSb0X1zPGxPIRFy5GoGtO9Mm5h4OZk= """ class TestSystemLogin(VyOSUnitTestSHIM.TestCase): + @classmethod + def setUpClass(cls): + super(TestSystemLogin, cls).setUpClass() + + # ensure we can also run this test on a live system - so lets clean + # out the current configuration which will break this test + cls.cli_delete(cls, base_path + ['radius']) + def tearDown(self): # Delete individual users from configuration for user in users: diff --git a/src/conf_mode/system-login.py b/src/conf_mode/system-login.py index bd9cc3b89..eea6c7413 100755 --- a/src/conf_mode/system-login.py +++ b/src/conf_mode/system-login.py @@ -64,6 +64,18 @@ def get_config(config=None): login = conf.get_config_dict(base, key_mangling=('-', '_'), no_tag_node_value_mangle=True, get_first_key=True) + # We have gathered the dict representation of the CLI, but there are default + # options which we need to update into the dictionary retrived. + default_values = defaults(base) + # XXX: T2665: we can not safely rely on the defaults() when there are + # tagNodes in place, it is better to blend in the defaults manually. + # defaults for RADIUS and USER will be added later down this function + if 'radius' in default_values: + del default_values['radius'] + if 'user' in default_values: + del default_values['user'] + login = dict_merge(default_values, login) + # users no longer existing in the running configuration need to be deleted local_users = get_local_users() cli_users = [] @@ -253,23 +265,19 @@ def apply(login): user_config, permission=0o600, formater=lambda _: _.replace(""", '"'), user=user, group='users') - #OTP 2FA key file generation - if dict_search('authentication.otp.key', user_config): - user_config['authentication']['otp']['key'] = user_config['authentication']['otp']['key'].upper() - user_config['authentication']['otp']['rate_limit'] = login['authentication']['otp']['rate_limit'] - user_config['authentication']['otp']['rate_time'] = login['authentication']['otp']['rate_time'] - user_config['authentication']['otp']['window_size'] = login['authentication']['otp']['window_size'] - render(f'{home_dir}/.google_authenticator', 'login/pam_otp_ga.conf.j2', - user_config, permission=0o600, - formater=lambda _: _.replace(""", '"'), - user=user, group='users') - #OTP 2FA key file deletion - elif os.path.exists(f'{home_dir}/.google_authenticator'): - os.remove(f'{home_dir}/.google_authenticator') - + except Exception as e: raise ConfigError(f'Adding user "{user}" raised exception: "{e}"') + # Generate 2FA/MFA One-Time-Pad configuration + if dict_search('authentication.otp.key', user_config): + render(f'{home_dir}/.google_authenticator', 'login/pam_otp_ga.conf.j2', + user_config, permission=0o400, user=user, group='users') + else: + # delete configuration as it's not enabled for the user + if os.path.exists(f'{home_dir}/.google_authenticator'): + os.remove(f'{home_dir}/.google_authenticator') + if 'rm_users' in login: for user in login['rm_users']: try: -- cgit v1.2.3