From d9eaa15a471fa3369ed13a664687dc7255188d63 Mon Sep 17 00:00:00 2001 From: Alan King Date: Thu, 28 Sep 2023 15:56:31 -0400 Subject: [PATCH 1/4] [ 7274] Add password_extend_lifetime tests for pam_password --- scripts/core_tests_list.json | 2 +- ...py => test_pam_password_authentication.py} | 253 +++++++++++++++++- 2 files changed, 247 insertions(+), 8 deletions(-) rename scripts/irods/test/{test_pam_password.py => test_pam_password_authentication.py} (55%) diff --git a/scripts/core_tests_list.json b/scripts/core_tests_list.json index 7a7d688041..5caf08e021 100644 --- a/scripts/core_tests_list.json +++ b/scripts/core_tests_list.json @@ -98,7 +98,7 @@ "test_misc", "test_native_rule_engine_plugin", "test_negotiation", - "test_pam_password.test_configurations", + "test_pam_password_authentication.test_configurations", "test_prep_genquery_iterator", "test_python_rule_engine_plugin", "test_quotas", diff --git a/scripts/irods/test/test_pam_password.py b/scripts/irods/test/test_pam_password_authentication.py similarity index 55% rename from scripts/irods/test/test_pam_password.py rename to scripts/irods/test/test_pam_password_authentication.py index f22d11c327..4ccd6740db 100644 --- a/scripts/irods/test/test_pam_password.py +++ b/scripts/irods/test/test_pam_password_authentication.py @@ -66,27 +66,27 @@ def setUpClass(self): cfg = lib.open_and_load_json( os.path.join(IrodsConfig().irods_directory, 'test', 'test_framework_configuration.json')) - auth_user = cfg['irods_authuser_name'] - auth_pass = cfg['irods_authuser_password'] + self.auth_user = cfg['irods_authuser_name'] + self.auth_pass = cfg['irods_authuser_password'] # Requires existence of OS account 'irodsauthuser' with password ';=iamnotasecret' try: import pwd - pwd.getpwnam(auth_user) + pwd.getpwnam(self.auth_user) except KeyError: # This is a requirement in order to run these tests and running the tests is required for our test suite, so # we always fail here when the prerequisites are not being met on the test-running host. self.fail('OS user "[{}]" with password "[{}]" must exist in order to run these tests.'.format( - auth_user, auth_pass)) + self.auth_user, self.auth_pass)) - self.auth_session = session.mkuser_and_return_session('rodsuser', auth_user, auth_pass, lib.get_hostname()) + self.auth_session = session.mkuser_and_return_session('rodsuser', self.auth_user, self.auth_pass, lib.get_hostname()) self.service_account_environment_file_path = os.path.join( os.path.expanduser('~'), '.irods', 'irods_environment.json') self.server_key_path, self.chain_pem_path, self.dhparams_pem_path = test_configurations.generate_default_ssl_files() self.authentication_scheme = 'pam_password' - self.configuration_namespace = 'authentication::pam_password' + self.configuration_namespace = 'authentication' @classmethod def tearDownClass(self): @@ -301,8 +301,10 @@ def test_password_expires_appropriately_based_on_grid_configuration_value(self): 'STDOUT', original_max_time) # When no TTL is specified, the default value is the minimum password lifetime as configured in - # R_GRID_CONFIGURATION. + # R_GRID_CONFIGURATION. This value should be higher than 3 seconds to ensure steps in the test + # have enough time to complete. ttl = 4 + self.assertGreater(ttl, 3) option_value = str(ttl) self.admin.assert_icommand( ['iadmin', 'set_grid_configuration', self.configuration_namespace, min_time_option_name, option_value]) @@ -343,4 +345,241 @@ def test_password_expires_appropriately_based_on_grid_configuration_value(self): self.admin.assert_icommand( ['iadmin', 'set_grid_configuration', self.configuration_namespace, min_time_option_name, original_min_time]) + # Re-authenticate as the session user to make sure things can be cleaned up. + self.auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'iRODS password', input=f'{self.auth_session.password}\n') + + def test_password_extend_lifetime_set_to_true_extends_other_authentications_past_expiration(self): + import time + + min_time_option_name = 'password_min_time' + extend_lifetime_option_name = 'password_extend_lifetime' + + # Stash away the original configuration for later... + original_min_time = self.admin.assert_icommand( + ['iadmin', 'get_grid_configuration', self.configuration_namespace, min_time_option_name], + 'STDOUT')[1].strip() + + original_extend_lifetime = self.admin.assert_icommand( + ['iadmin', 'get_grid_configuration', self.configuration_namespace, extend_lifetime_option_name], + 'STDOUT')[1].strip() + + # Set password_extend_lifetime to 1 so that the same randomly generated password is used for all sessions. + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, extend_lifetime_option_name, '1']) + + # Make a new session of the existing auth_user. The data is "managed" in the session, so the session + # collection shall be shared with the other session. + temp_auth_session = session.make_session_for_existing_user( + self.auth_user, self.auth_pass, lib.get_hostname(), self.auth_session.zone_name) + temp_auth_session.assert_icommand(['icd', self.auth_session.session_collection]) + + IrodsController().stop() + + auth_session_env_backup = copy.deepcopy(self.auth_session.environment_file_contents) + admin_session_env_backup = copy.deepcopy(self.admin.environment_file_contents) + + try: + service_account_environment_file_path = os.path.join( + os.path.expanduser('~'), '.irods', 'irods_environment.json') + with lib.file_backed_up(service_account_environment_file_path): + client_update = test_configurations.make_dict_for_ssl_client_environment( + self.server_key_path, self.chain_pem_path, self.dhparams_pem_path) + + lib.update_json_file_from_dict(service_account_environment_file_path, client_update) + + self.admin.environment_file_contents.update(client_update) + + client_update['irods_authentication_scheme'] = self.authentication_scheme + self.auth_session.environment_file_contents.update(client_update) + temp_auth_session.environment_file_contents.update(client_update) + + with temporary_core_file() as core: + core.add_rule(test_configurations.get_pep_for_ssl(self.plugin_name)) + + IrodsController().start() + + # Make sure the settings applied correctly... + self.admin.assert_icommand( + ['iadmin', 'get_grid_configuration', self.configuration_namespace, extend_lifetime_option_name], + 'STDOUT', '1') + + # Set the minimum time to a very short value so that the password expires in a reasonable amount of + # time for testing purposes. This value should be higher than 3 seconds to ensure steps in the test + # have enough time to complete. + ttl = 4 + self.assertGreater(ttl, 3) + option_value = str(ttl) + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, min_time_option_name, option_value]) + + # Authenticate with both sessions and run a command... + self.auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'PAM password', input=f'{self.auth_session.password}\n') + temp_auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'PAM password', input=f'{self.auth_session.password}\n') + + self.auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + temp_auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + + # Sleep until just before password is expired... + time.sleep(ttl - 1) + + # Re-authenticate as one of the sessions such that the random password lifetime is extended. This + # will allow the other session to continue without re-authenticating. + self.auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'PAM password', input=f'{self.auth_session.password}\n') + + # We want to sleep 1 second past the timeout (ttl + 1) to ensure that the original expiration time + # has passed. We already slept ttl - 1 seconds, so the remaining time is calculated like this: + # remaining_sleep_time = total_time_to_sleep - time_already_slept = (ttl + 1) - (ttl - 1) = 2 + time.sleep(2) + + # Run a command as the other session to show that the existing password is still valid. + temp_auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + + # The re-authenticated session should also be able to run commands, of course. + self.auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + + # Sleep again to let the password time out. + time.sleep(ttl + 1) + + # Password should be expired now... + temp_auth_session.assert_icommand( + ["ils"], 'STDERR', 'CAT_PASSWORD_EXPIRED: failed to perform request') + # The sessions are using the same password, so the second response will be different + # TODO: irods/irods#7344 - This should emit a better error message. + self.auth_session.assert_icommand( + ["ils"], 'STDERR', 'CAT_INVALID_AUTHENTICATION: failed to perform request') + + finally: + temp_auth_session.environment_file_contents = auth_session_env_backup + self.auth_session.environment_file_contents = auth_session_env_backup + self.admin.environment_file_contents = admin_session_env_backup + + IrodsController().restart() + + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, extend_lifetime_option_name, original_extend_lifetime]) + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, min_time_option_name, original_min_time]) + + # Re-authenticate as the session user to make sure things can be cleaned up. + self.auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'iRODS password', input=f'{self.auth_session.password}\n') + + def test_password_extend_lifetime_set_to_false_invalidates_other_authentications_on_expiration(self): + import time + + min_time_option_name = 'password_min_time' + extend_lifetime_option_name = 'password_extend_lifetime' + + # Stash away the original configuration for later... + original_min_time = self.admin.assert_icommand( + ['iadmin', 'get_grid_configuration', self.configuration_namespace, min_time_option_name], + 'STDOUT')[1].strip() + + original_extend_lifetime = self.admin.assert_icommand( + ['iadmin', 'get_grid_configuration', self.configuration_namespace, extend_lifetime_option_name], + 'STDOUT')[1].strip() + + # Set password_extend_lifetime to 1 so that the same randomly generated password is used for all sessions. + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, extend_lifetime_option_name, '1']) + + # Make a new session of the existing auth_user. The data is "managed" in the session, so the session + # collection shall be shared with the other session. + temp_auth_session = session.make_session_for_existing_user( + self.auth_user, self.auth_pass, lib.get_hostname(), self.auth_session.zone_name) + temp_auth_session.assert_icommand(['icd', self.auth_session.session_collection]) + + IrodsController().stop() + + auth_session_env_backup = copy.deepcopy(self.auth_session.environment_file_contents) + admin_session_env_backup = copy.deepcopy(self.admin.environment_file_contents) + + try: + service_account_environment_file_path = os.path.join( + os.path.expanduser('~'), '.irods', 'irods_environment.json') + with lib.file_backed_up(service_account_environment_file_path): + client_update = test_configurations.make_dict_for_ssl_client_environment( + self.server_key_path, self.chain_pem_path, self.dhparams_pem_path) + + lib.update_json_file_from_dict(service_account_environment_file_path, client_update) + + self.admin.environment_file_contents.update(client_update) + + client_update['irods_authentication_scheme'] = self.authentication_scheme + self.auth_session.environment_file_contents.update(client_update) + temp_auth_session.environment_file_contents.update(client_update) + + with temporary_core_file() as core: + core.add_rule(test_configurations.get_pep_for_ssl(self.plugin_name)) + + IrodsController().start() + + # Make sure the settings applied correctly... + self.admin.assert_icommand( + ['iadmin', 'get_grid_configuration', self.configuration_namespace, extend_lifetime_option_name], + 'STDOUT', '1') + + # Set the minimum time to a very short value so that the password expires in a reasonable amount of + # time for testing purposes. This value should be higher than 3 seconds to ensure steps in the test + # have enough time to complete. + ttl = 4 + self.assertGreater(ttl, 3) + option_value = str(ttl) + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, min_time_option_name, option_value]) + + # Authenticate with both sessions and run a command... + self.auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'PAM password', input=f'{self.auth_session.password}\n') + temp_auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'PAM password', input=f'{temp_auth_session.password}\n') + + self.auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + temp_auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + + # Disable password_extend_lifetime so that on the next authentication, the expiration time of the + # existing password will not be extended. + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, extend_lifetime_option_name, '0']) + + # Sleep until just before password is expired... + time.sleep(ttl - 1) + + # Re-authenticate as one of the sessions - the random password lifetime will not be extended for + # either session. + self.auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'PAM password', input=f'{self.auth_session.password}\n') + + # We want to sleep 1 second past the timeout (ttl + 1) to ensure that the original expiration time + # has passed. We already slept ttl - 1 seconds, so the remaining time is calculated like this: + # remaining_sleep_time = total_time_to_sleep - time_already_slept = (ttl + 1) - (ttl - 1) = 2 + time.sleep(2) + + # Password should be expired for both sessions despite one having re-authenticated past the + # expiry time. + temp_auth_session.assert_icommand( + ["ils"], 'STDERR', 'CAT_PASSWORD_EXPIRED: failed to perform request') + # The sessions are using the same password, so the second response will be different + # TODO: irods/irods#7344 - This should emit a better error message. + self.auth_session.assert_icommand( + ["ils"], 'STDERR', 'CAT_INVALID_AUTHENTICATION: failed to perform request') + + finally: + temp_auth_session.environment_file_contents = auth_session_env_backup + self.auth_session.environment_file_contents = auth_session_env_backup + self.admin.environment_file_contents = admin_session_env_backup + IrodsController().restart() + + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, extend_lifetime_option_name, original_extend_lifetime]) + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, min_time_option_name, original_min_time]) + + # Re-authenticate as the session user to make sure things can be cleaned up. + self.auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'iRODS password', input=f'{self.auth_session.password}\n') From 9d8e69f69eba17bbc94e75f0b6a62037c20235a2 Mon Sep 17 00:00:00 2001 From: Alan King Date: Thu, 28 Sep 2023 18:31:09 -0400 Subject: [PATCH 2/4] [ 7274] Add configuration tests for native auth These tests are based entirely off of the pam_password equivalents because they should behave in the same way. Tests for password_extend_lifetime have been written but are skipped because that configuration is not supported for native authentication. Tests for password expiration are also being skipped because the minimum TTL we can specify is 1 hour which is not feasible for automated testing. --- scripts/core_tests_list.json | 1 + .../irods/test/test_native_authentication.py | 356 ++++++++++++++++++ 2 files changed, 357 insertions(+) create mode 100644 scripts/irods/test/test_native_authentication.py diff --git a/scripts/core_tests_list.json b/scripts/core_tests_list.json index 5caf08e021..3630505e48 100644 --- a/scripts/core_tests_list.json +++ b/scripts/core_tests_list.json @@ -96,6 +96,7 @@ "test_izonereport", "test_load_balanced_suite", "test_misc", + "test_native_authentication.test_configurations", "test_native_rule_engine_plugin", "test_negotiation", "test_pam_password_authentication.test_configurations", diff --git a/scripts/irods/test/test_native_authentication.py b/scripts/irods/test/test_native_authentication.py new file mode 100644 index 0000000000..67009bcfed --- /dev/null +++ b/scripts/irods/test/test_native_authentication.py @@ -0,0 +1,356 @@ +from __future__ import print_function + +import os +import unittest + +from . import session +from .. import lib +from ..configuration import IrodsConfig + + +class test_configurations(unittest.TestCase): + plugin_name = IrodsConfig().default_rule_engine_plugin + + @classmethod + def setUpClass(self): + self.admin = session.mkuser_and_return_session('rodsadmin', 'otherrods', 'rods', lib.get_hostname()) + + cfg = lib.open_and_load_json( + os.path.join(IrodsConfig().irods_directory, 'test', 'test_framework_configuration.json')) + self.auth_user = cfg['irods_authuser_name'] + self.auth_pass = cfg['irods_authuser_password'] + + self.auth_session = session.mkuser_and_return_session('rodsuser', self.auth_user, self.auth_pass, lib.get_hostname()) + self.service_account_environment_file_path = os.path.join( + os.path.expanduser('~'), '.irods', 'irods_environment.json') + + self.configuration_namespace = 'authentication' + + @classmethod + def tearDownClass(self): + self.auth_session.__exit__() + + self.admin.assert_icommand(['iadmin', 'rmuser', self.auth_session.username]) + self.admin.__exit__() + with session.make_session_for_existing_admin() as admin_session: + admin_session.assert_icommand(['iadmin', 'rmuser', self.admin.username]) + + def do_test_invalid_password_time_configurations(self, _option_name): + # Stash away the original configuration for later... + original_config = self.admin.assert_icommand( + ['iadmin', 'get_grid_configuration', self.configuration_namespace, _option_name], 'STDOUT')[1].strip() + + try: + self.auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'iRODS password', input=f'{self.auth_session.password}\n') + self.auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + + for option_value in [' ', 'nope', str(-1), str(18446744073709552000), str(-18446744073709552000)]: + with self.subTest(f'test with value [{option_value}]'): + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', '--', self.configuration_namespace, _option_name, option_value]) + + # These invalid configurations will not cause any errors, but default values will be used. + self.auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'iRODS password', input=f'{self.auth_session.password}\n') + self.auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + + finally: + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, _option_name, original_config]) + + def test_invalid_password_max_time(self): + self.do_test_invalid_password_time_configurations('password_max_time') + + def test_invalid_password_min_time(self): + self.do_test_invalid_password_time_configurations('password_min_time') + + def test_password_max_time_less_than_password_min_time_makes_ttl_constraints_unsatisfiable(self): + min_time_option_name = 'password_min_time' + max_time_option_name = 'password_max_time' + + # Stash away the original configuration for later... + original_min_time = self.admin.assert_icommand( + ['iadmin', 'get_grid_configuration', self.configuration_namespace, min_time_option_name], 'STDOUT')[1].strip() + + original_max_time = self.admin.assert_icommand( + ['iadmin', 'get_grid_configuration', self.configuration_namespace, max_time_option_name], 'STDOUT')[1].strip() + + try: + # Try a few different values here that are in the range of overall acceptable values: + # - 2 hours allows us to go up OR down by 1 hour (boundary case). + # - 336 hours is 1209600 seconds (or 2 weeks) which is the default maximum allowed TTL value. + for base_ttl_in_hours in [2, 336]: + with self.subTest(f'test with TTL of [{base_ttl_in_hours}] hours'): + base_ttl_in_seconds = base_ttl_in_hours * 3600 + + option_value = str(base_ttl_in_seconds + 10) + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, min_time_option_name, option_value]) + + # Set password_max_time to a value less than the password_min_time. + option_value = str(base_ttl_in_seconds - 10) + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, max_time_option_name, option_value]) + + # Note: The min/max check does not occur when no TTL parameter is passed, for some reason. + # We must pass TTL explicitly for each test. Even though this is native authentication, + # PAM_AUTH_PASSWORD_INVALID_TTL is the returned error. + + # This is lower than the minimum and higher than the maximum. The TTL is invalid. + self.auth_session.assert_icommand( + ['iinit', '--ttl', str(base_ttl_in_hours)], + 'STDERR', 'rcGetLimitedPassword failed with error [-994000]', + input=f'{self.auth_session.password}\n') + + # This is lower than the maximum but also lower than the minimum. The TTL is invalid. + self.auth_session.assert_icommand( + ['iinit', '--ttl', str(base_ttl_in_hours - 1)], + 'STDERR', 'rcGetLimitedPassword failed with error [-994000]', + input=f'{self.auth_session.password}\n') + + # This is higher than the minimum but also higher than the maximum. The TTL is invalid. + self.auth_session.assert_icommand( + ['iinit', '--ttl', str(base_ttl_in_hours + 1)], + 'STDERR', 'rcGetLimitedPassword failed with error [-994000]', + input=f'{self.auth_session.password}\n') + + # Restore grid configuration and try again, with success. + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, max_time_option_name, original_max_time]) + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, min_time_option_name, original_min_time]) + + self.auth_session.assert_icommand( + ['iinit', '--ttl', str(1)], 'STDOUT', 'iRODS password', input=f'{self.auth_session.password}\n') + self.auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + + finally: + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, max_time_option_name, original_max_time]) + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, min_time_option_name, original_min_time]) + + @unittest.skip('iinit does not allow TTL to be under 1 hour for native authentication. Skip this test for now.') + def test_password_expires_appropriately_based_on_grid_configuration_value(self): + import time + + min_time_option_name = 'password_min_time' + max_time_option_name = 'password_max_time' + + # Stash away the original configuration for later... + original_min_time = self.admin.assert_icommand( + ['iadmin', 'get_grid_configuration', self.configuration_namespace, min_time_option_name], 'STDOUT')[1].strip() + + original_max_time = self.admin.assert_icommand( + ['iadmin', 'get_grid_configuration', self.configuration_namespace, max_time_option_name], 'STDOUT')[1].strip() + + try: + # When no TTL is specified, the default value is the minimum password lifetime as configured in + # R_GRID_CONFIGURATION. This value should be higher than 3 seconds to ensure steps in the test + # have enough time to complete. + ttl = 4 + self.assertGreater(ttl, 3) + option_value = str(ttl) + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, min_time_option_name, option_value]) + + # Authenticate and run a command... + self.auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'iRODS password', input=f'{self.auth_session.password}\n') + + self.auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + + # Sleep until the password is expired... + time.sleep(ttl + 1) + + # Password should be expired now... + self.auth_session.assert_icommand(["ils"], 'STDERR', 'CAT_PASSWORD_EXPIRED: failed to perform request') + + # ...and stays expired. + # TODO: irods/irods#7344 - This should emit a better error message. + self.auth_session.assert_icommand(["ils"], 'STDERR', 'CAT_INVALID_AUTHENTICATION: failed to perform request') + + # Restore grid configuration and try again, with success. + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, max_time_option_name, original_max_time]) + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, min_time_option_name, original_min_time]) + + self.auth_session.assert_icommand(['iinit'], 'STDOUT', input=f'{self.auth_session.password}\n') + self.auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + + finally: + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, max_time_option_name, original_max_time]) + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, min_time_option_name, original_min_time]) + + # Re-authenticate as the session user to make sure things can be cleaned up. + self.auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'iRODS password', input=f'{self.auth_session.password}\n') + + @unittest.skip('password_extend_lifetime is not supported for native authentication.') + def test_password_extend_lifetime_set_to_true_extends_other_authentications_past_expiration(self): + import time + + min_time_option_name = 'password_min_time' + extend_lifetime_option_name = 'password_extend_lifetime' + + # Stash away the original configuration for later... + original_min_time = self.admin.assert_icommand( + ['iadmin', 'get_grid_configuration', self.configuration_namespace, min_time_option_name], + 'STDOUT')[1].strip() + + original_extend_lifetime = self.admin.assert_icommand( + ['iadmin', 'get_grid_configuration', self.configuration_namespace, extend_lifetime_option_name], + 'STDOUT')[1].strip() + + # Set password_extend_lifetime to 1 so that the same randomly generated password is used for all sessions. + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, extend_lifetime_option_name, '1']) + + # Make a new session of the existing auth_user. The data is "managed" in the session, so the session + # collection shall be shared with the other session. + temp_auth_session = session.make_session_for_existing_user( + self.auth_user, self.auth_pass, lib.get_hostname(), self.auth_session.zone_name) + temp_auth_session.assert_icommand(['icd', self.auth_session.session_collection]) + + try: + # Set the minimum time to a very short value so that the password expires in a reasonable amount of + # time for testing purposes. This value should be higher than 3 seconds to ensure steps in the test + # have enough time to complete. + ttl = 4 + self.assertGreater(ttl, 3) + option_value = str(ttl) + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, min_time_option_name, option_value]) + + # Authenticate with both sessions and run a command... + self.auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'iRODS password', input=f'{self.auth_session.password}\n') + temp_auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'iRODS password', input=f'{self.auth_session.password}\n') + + self.auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + temp_auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + + # Sleep until just before password is expired... + time.sleep(ttl - 1) + + # Re-authenticate as one of the sessions such that the random password lifetime is extended. This + # will allow the other session to continue without re-authenticating. + self.auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'iRODS password', input=f'{self.auth_session.password}\n') + + # We want to sleep 1 second past the timeout (ttl + 1) to ensure that the original expiration time + # has passed. We already slept ttl - 1 seconds, so the remaining time is calculated like this: + # remaining_sleep_time = total_time_to_sleep - time_already_slept = (ttl + 1) - (ttl - 1) = 2 + time.sleep(2) + + # Run a command as the other session to show that the existing password is still valid. + temp_auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + + # The re-authenticated session should also be able to run commands, of course. + self.auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + + # Sleep again to let the password time out. + time.sleep(ttl + 1) + + # Password should be expired now... + temp_auth_session.assert_icommand( + ["ils"], 'STDERR', 'CAT_PASSWORD_EXPIRED: failed to perform request') + # The sessions are using the same password, so the second response will be different + # TODO: irods/irods#7344 - This should emit a better error message. + self.auth_session.assert_icommand( + ["ils"], 'STDERR', 'CAT_INVALID_AUTHENTICATION: failed to perform request') + + finally: + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, extend_lifetime_option_name, original_extend_lifetime]) + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, min_time_option_name, original_min_time]) + + # Re-authenticate as the session user to make sure things can be cleaned up. + self.auth_session.assert_icommand(['iinit'], 'STDOUT', input=f'{self.auth_session.password}\n') + + @unittest.skip('password_extend_lifetime is not supported for native authentication.') + def test_password_extend_lifetime_set_to_false_invalidates_other_authentications_on_expiration(self): + import time + + min_time_option_name = 'password_min_time' + extend_lifetime_option_name = 'password_extend_lifetime' + + # Stash away the original configuration for later... + original_min_time = self.admin.assert_icommand( + ['iadmin', 'get_grid_configuration', self.configuration_namespace, min_time_option_name], + 'STDOUT')[1].strip() + + original_extend_lifetime = self.admin.assert_icommand( + ['iadmin', 'get_grid_configuration', self.configuration_namespace, extend_lifetime_option_name], + 'STDOUT')[1].strip() + + # Set password_extend_lifetime to 1 so that the same randomly generated password is used for all sessions. + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, extend_lifetime_option_name, '1']) + + # Make a new session of the existing auth_user. The data is "managed" in the session, so the session + # collection shall be shared with the other session. + temp_auth_session = session.make_session_for_existing_user( + self.auth_user, self.auth_pass, lib.get_hostname(), self.auth_session.zone_name) + temp_auth_session.assert_icommand(['icd', self.auth_session.session_collection]) + + try: + # Set the minimum time to a very short value so that the password expires in a reasonable amount of + # time for testing purposes. This value should be higher than 3 seconds to ensure steps in the test + # have enough time to complete. + ttl = 4 + self.assertGreater(ttl, 3) + option_value = str(ttl) + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, min_time_option_name, option_value]) + + # Authenticate with both sessions and run a command... + self.auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'iRODS password', input=f'{self.auth_session.password}\n') + temp_auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'iRODS password', input=f'{temp_auth_session.password}\n') + + self.auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + temp_auth_session.assert_icommand(["ils"], 'STDOUT', self.auth_session.session_collection) + + # Disable password_extend_lifetime so that on the next authentication, the expiration time of the + # existing password will not be extended. + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, extend_lifetime_option_name, '0']) + + # Sleep until just before password is expired... + time.sleep(ttl - 1) + + # Re-authenticate as one of the sessions - the random password lifetime will not be extended for + # either session. + self.auth_session.assert_icommand( + ['iinit'], 'STDOUT', 'iRODS password', input=f'{self.auth_session.password}\n') + + # We want to sleep 1 second past the timeout (ttl + 1) to ensure that the original expiration time + # has passed. We already slept ttl - 1 seconds, so the remaining time is calculated like this: + # remaining_sleep_time = total_time_to_sleep - time_already_slept = (ttl + 1) - (ttl - 1) = 2 + time.sleep(2) + + # Password should be expired for both sessions despite one having re-authenticated past the + # expiry time. + temp_auth_session.assert_icommand( + ["ils"], 'STDERR', 'CAT_PASSWORD_EXPIRED: failed to perform request') + # The sessions are using the same password, so the second response will be different + # TODO: irods/irods#7344 - This should emit a better error message. + self.auth_session.assert_icommand( + ["ils"], 'STDERR', 'CAT_INVALID_AUTHENTICATION: failed to perform request') + + finally: + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, extend_lifetime_option_name, original_extend_lifetime]) + self.admin.assert_icommand( + ['iadmin', 'set_grid_configuration', self.configuration_namespace, min_time_option_name, original_min_time]) + + # Re-authenticate as the session user to make sure things can be cleaned up. + self.auth_session.assert_icommand(['iinit'], 'STDOUT', 'iRODS password', input=f'{self.auth_session.password}\n') From a802b083211ec3bcaf02442de755e59d46289780 Mon Sep 17 00:00:00 2001 From: Alan King Date: Fri, 29 Sep 2023 11:32:45 -0400 Subject: [PATCH 3/4] [ 7274] Fixes for native authentication configs TTL needs to be converted to seconds before comparing against the min/max password time configurations. clientLogin needs to return a better error message when a failure occurs in rcGetLimitedPassword. --- lib/core/src/clientLogin.cpp | 3 ++- plugins/database/src/db_plugin.cpp | 12 +++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/core/src/clientLogin.cpp b/lib/core/src/clientLogin.cpp index 87c59e34c1..ad83a2930a 100644 --- a/lib/core/src/clientLogin.cpp +++ b/lib/core/src/clientLogin.cpp @@ -253,7 +253,8 @@ int clientLoginTTL( rcComm_t *Conn, int ttl ) { if ( int status = rcGetLimitedPassword( Conn, &getLimitedPasswordInp, &getLimitedPasswordOut ) ) { - allocate_if_necessary_and_add_rError_msg(&Conn->rError, status, "rcGetLimitedPassword"); + const auto msg = fmt::format("rcGetLimitedPassword failed with error [{}]", status); + allocate_if_necessary_and_add_rError_msg(&Conn->rError, status, msg.c_str()); memset( userPassword, 0, sizeof( userPassword ) ); return status; } diff --git a/plugins/database/src/db_plugin.cpp b/plugins/database/src/db_plugin.cpp index 6509ca12e4..fb6d49ec14 100644 --- a/plugins/database/src/db_plugin.cpp +++ b/plugins/database/src/db_plugin.cpp @@ -7100,15 +7100,17 @@ irods::error db_make_limited_pw_op( return err; } - if (_ttl < ac.password_min_time || _ttl > ac.password_max_time) { - log_db::error( - "Invalid TTL - min time: [{}] max time:[{}] ttl: [{}]", ac.password_min_time, ac.password_max_time, _ttl); + // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) + int timeToLive = _ttl * 3600; /* convert input hours to seconds */ + if (timeToLive < ac.password_min_time || timeToLive > ac.password_max_time) { + log_db::error("Invalid TTL - min time: [{}] max time:[{}] ttl: [{}]", + ac.password_min_time, + ac.password_max_time, + timeToLive); return ERROR( PAM_AUTH_PASSWORD_INVALID_TTL, "invalid ttl" ); } /* Insert the limited password */ - // NOLINTNEXTLINE(cppcoreguidelines-avoid-magic-numbers,readability-magic-numbers) - int timeToLive = _ttl * 3600; /* convert input hours to seconds */ snprintf( expTime, sizeof expTime, "%d", timeToLive ); cllBindVars[cllBindVarCount++] = _ctx.comm()->clientUser.userName; // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-constant-array-index) From d036bb8327086c252d6ad36fcfbc4b243b973e10 Mon Sep 17 00:00:00 2001 From: Alan King Date: Fri, 29 Sep 2023 11:32:09 -0400 Subject: [PATCH 4/4] [ 7274] Replace plugin-specific auth configs with general one The plugin-specific auth configurations (authentication::pam_password and authentication::native) have been replaced by a general config for all auth schemes. The string in R_GRID_CONFIGURATION is now just "authentication". --- plugins/database/src/db_plugin.cpp | 10 ++++---- scripts/irods/database_upgrade.py | 4 ++-- .../test_iadmin_set_grid_configuration.py | 24 +++++++++---------- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/plugins/database/src/db_plugin.cpp b/plugins/database/src/db_plugin.cpp index fb6d49ec14..963b4cbf0b 100644 --- a/plugins/database/src/db_plugin.cpp +++ b/plugins/database/src/db_plugin.cpp @@ -6596,9 +6596,7 @@ irods::error db_check_auth_op( expireTime = atoll( goodPwExpiry ); getNowStr( myTime ); - /* Check for PAM_AUTH type passwords */ - - if (const auto err = get_auth_config("authentication::pam_password", ac); !err.ok()) { + if (const auto err = get_auth_config("authentication", ac); !err.ok()) { log_db::error("Failed to get auth configuration. [{}]", err.result()); return err; } @@ -7095,7 +7093,7 @@ irods::error db_make_limited_pw_op( getNowStr( myTime ); auth_config ac{}; - if (const auto err = get_auth_config("authentication::native", ac); !err.ok()) { + if (const auto err = get_auth_config("authentication", ac); !err.ok()) { log_db::error("Failed to get auth configuration. [{}]", err.result()); return err; } @@ -7235,7 +7233,7 @@ auto db_update_pam_password_op(irods::plugin_context& _ctx, getNowStr( myTime ); auth_config ac{}; - if (const auto err = get_auth_config("authentication::pam_password", ac); !err.ok()) { + if (const auto err = get_auth_config("authentication", ac); !err.ok()) { log_db::error("Failed to get auth configuration. [{}]", err.result()); return err; } @@ -7573,7 +7571,7 @@ irods::error db_mod_user_op( if ( strncmp( _option, "rmPamPw", 9 ) == 0 ) { auth_config ac{}; - if (const auto err = get_auth_config("authentication::pam_password", ac); !err.ok()) { + if (const auto err = get_auth_config("authentication", ac); !err.ok()) { log_db::error("Failed to get auth configuration. [{}]", err.result()); return err; } diff --git a/scripts/irods/database_upgrade.py b/scripts/irods/database_upgrade.py index ea58f67c8b..fa296b94bc 100644 --- a/scripts/irods/database_upgrade.py +++ b/scripts/irods/database_upgrade.py @@ -175,10 +175,10 @@ def run_update(irods_config, cursor): 'password_max_time': str(pam_password_config.get('password_max_time', 1209600)) } - scheme_namespaces = ['authentication::pam_password', 'authentication::native'] + scheme_namespaces = ['authentication'] statement_str = "insert into R_GRID_CONFIGURATION (namespace, option_name, option_value) values ('{}','{}','{}');" # pam_password configurations for password lifetime have always been used with native authentication as well. - # The configurations are now separately configurable. + # The new configurations shall continue to configure both schemes, but under a more generic namespace. for scheme in scheme_namespaces: for option in password_config_dict: database_connect.execute_sql_statement(cursor, statement_str.format(scheme, option, password_config_dict[option])) diff --git a/scripts/irods/test/test_iadmin_set_grid_configuration.py b/scripts/irods/test/test_iadmin_set_grid_configuration.py index 07a198d0d8..3f6cfe0943 100644 --- a/scripts/irods/test/test_iadmin_set_grid_configuration.py +++ b/scripts/irods/test/test_iadmin_set_grid_configuration.py @@ -42,7 +42,7 @@ def test_nonexistent_namespace(self): f'Failed to get grid configuration for namespace [{bad_namespace}] and option [{option_name}] [ec=-808000]') def test_no_option_name(self): - namespace = 'authentication::native' + namespace = 'authentication' self.admin.assert_icommand( ['iadmin', 'get_grid_configuration', namespace], 'STDERR', 'Error: option name must be between 1 and 2699 characters.') @@ -51,7 +51,7 @@ def test_really_long_option_name(self): # The input buffer to set_grid_configuration_value API is only 2700 characters long. If a value of 2700 # characters or more is fed to the input struct for the set_pam_password_config API, packstruct gives an error. # iadmin will catch this case and show a slightly more presentable error, which is checked in this test. - namespace = 'authentication::native' + namespace = 'authentication' really_long_option_name = 'this_is_27_characters_long_' * 100 self.admin.assert_icommand( @@ -59,7 +59,7 @@ def test_really_long_option_name(self): 'STDERR', 'Error: option name must be between 1 and 2699 characters.') def test_nonexistent_option_name(self): - namespace = 'authentication::native' + namespace = 'authentication' bad_option_name = 'nopes' self.admin.assert_icommand( @@ -67,7 +67,7 @@ def test_nonexistent_option_name(self): f'Failed to get grid configuration for namespace [{namespace}] and option [{bad_option_name}] [ec=-808000]') def test_get_grid_configuration_valid(self): - namespace = 'authentication::native' + namespace = 'authentication' option_name = 'password_max_time' # Assert that a value is returned and that there are no errors. @@ -112,7 +112,7 @@ def test_nonexistent_namespace(self): f'Failed to set grid configuration for namespace [{bad_namespace}] and option [{option_name}] [ec=-808000]') def test_no_option_name(self): - namespace = 'authentication::native' + namespace = 'authentication' self.admin.assert_icommand( ['iadmin', 'set_grid_configuration', namespace], 'STDERR', 'Error: option name must be between 1 and 2699 characters.') @@ -121,7 +121,7 @@ def test_really_long_option_name(self): # The input buffer to set_grid_configuration_value API is only 2700 characters long. If a value of 2700 # characters or more is fed to the input struct for the set_pam_password_config API, packstruct gives an error. # iadmin will catch this case and show a slightly more presentable error, which is checked in this test. - namespace = 'authentication::native' + namespace = 'authentication' really_long_option_name = 'this_is_27_characters_long_' * 100 option_value = '1000' @@ -130,7 +130,7 @@ def test_really_long_option_name(self): 'STDERR', 'Error: option name must be between 1 and 2699 characters.') def test_nonexistent_option_name(self): - namespace = 'authentication::native' + namespace = 'authentication' bad_option_name = 'nopes' option_value = '1000' @@ -139,14 +139,14 @@ def test_nonexistent_option_name(self): f'Failed to set grid configuration for namespace [{namespace}] and option [{bad_option_name}] [ec=-808000]') def test_no_option_value(self): - namespace = 'authentication::native' + namespace = 'authentication' option_name = 'password_max_time' self.admin.assert_icommand( ['iadmin', 'set_grid_configuration', namespace, option_name], 'STDERR', 'Error: option value must be between 1 and 2699 characters.') def test_really_long_option_value(self): - namespace = 'authentication::native' + namespace = 'authentication' option_name = 'password_max_time' # The input buffer to set_grid_configuration_value API is only 2700 characters long. If a value of 2700 @@ -167,7 +167,7 @@ def test_really_long_option_value(self): self.admin.assert_icommand(['iadmin', 'get_grid_configuration', namespace, option_name], 'STDOUT')[1]) def test_set_grid_configuration_valid(self): - namespace = 'authentication::native' + namespace = 'authentication' option_name = 'password_max_time' original_value = self.admin.assert_icommand( @@ -188,7 +188,7 @@ def test_set_grid_configuration_valid(self): self.admin.run_icommand(['iadmin', 'set_grid_configuration', namespace, option_name, original_value]) def test_set_invalid_grid_configuration_with_option_name_that_is_protected_in_another_namespace(self): - namespace = 'authentication::native' + namespace = 'authentication' option_name = 'schema_version' # Make sure this namespace doesn't have the option_name used in the test... @@ -274,7 +274,7 @@ def test_set_delay_server_namespace_is_protected_even_with_invalid_option_name(s def test_set_delay_server_namespace_is_protected_even_with_option_name_from_unprotected_namespaces(self): namespace = 'delay_server' - other_namespace = 'authentication::native' + other_namespace = 'authentication' option_name = 'password_max_time' option_value = 'shenanigans!'