diff --git a/ipalib/install/kinit.py b/ipalib/install/kinit.py index cc839ec38..4ad4eaa1c 100644 --- a/ipalib/install/kinit.py +++ b/ipalib/install/kinit.py @@ -6,12 +6,16 @@ from __future__ import absolute_import import logging import os +import re import time import gssapi from ipaplatform.paths import paths from ipapython.ipautil import run +from ipalib.constants import PATTERN_GROUPUSER_NAME +from ipalib.util import validate_hostname +from ipalib import api logger = logging.getLogger(__name__) @@ -21,6 +25,40 @@ KRB5_KDC_UNREACH = 2529639068 # A service is not available that s required to process the request KRB5KDC_ERR_SVC_UNAVAILABLE = 2529638941 +PATTERN_REALM = '@?([a-zA-Z0-9.-]*)$' +PATTERN_PRINCIPAL = '(' + PATTERN_GROUPUSER_NAME[:-1] + ')' + PATTERN_REALM +PATTERN_SERVICE = '([a-zA-Z0-9.-]+)/([a-zA-Z0-9.-]+)' + PATTERN_REALM + +user_pattern = re.compile(PATTERN_PRINCIPAL) +service_pattern = re.compile(PATTERN_SERVICE) + + +def validate_principal(principal): + if not isinstance(principal, str): + raise RuntimeError('Invalid principal: not a string') + if ('/' in principal) and (' ' in principal): + raise RuntimeError('Invalid principal: bad spacing') + else: + realm = None + match = user_pattern.match(principal) + if match is None: + match = service_pattern.match(principal) + if match is None: + raise RuntimeError('Invalid principal: cannot parse') + else: + # service = match[1] + hostname = match[2] + realm = match[3] + try: + validate_hostname(hostname) + except ValueError as e: + raise RuntimeError(str(e)) + else: # user match, validate realm + # username = match[1] + realm = match[2] + if realm and 'realm' in api.env and realm != api.env.realm: + raise RuntimeError('Invalid principal: realm mismatch') + def kinit_keytab(principal, keytab, ccache_name, config=None, attempts=1): """ @@ -29,6 +67,7 @@ def kinit_keytab(principal, keytab, ccache_name, config=None, attempts=1): The optional parameter 'attempts' specifies how many times the credential initialization should be attempted in case of non-responsive KDC. """ + validate_principal(principal) errors_to_retry = {KRB5KDC_ERR_SVC_UNAVAILABLE, KRB5_KDC_UNREACH} logger.debug("Initializing principal %s using keytab %s", @@ -65,6 +104,7 @@ def kinit_keytab(principal, keytab, ccache_name, config=None, attempts=1): return None + def kinit_password(principal, password, ccache_name, config=None, armor_ccache_name=None, canonicalize=False, enterprise=False, lifetime=None): @@ -73,8 +113,9 @@ def kinit_password(principal, password, ccache_name, config=None, web-based authentication, use armor_ccache_path to specify http service ccache. """ + validate_principal(principal) logger.debug("Initializing principal %s using password", principal) - args = [paths.KINIT, principal, '-c', ccache_name] + args = [paths.KINIT, '-c', ccache_name] if armor_ccache_name is not None: logger.debug("Using armor ccache %s for FAST webauth", armor_ccache_name) @@ -91,6 +132,7 @@ def kinit_password(principal, password, ccache_name, config=None, logger.debug("Using enterprise principal") args.append('-E') + args.extend(['--', principal]) env = {'LC_ALL': 'C'} if config is not None: env['KRB5_CONFIG'] = config @@ -154,6 +196,7 @@ def kinit_pkinit( :raises: CalledProcessError if PKINIT fails """ + validate_principal(principal) logger.debug( "Initializing principal %s using PKINIT %s", principal, user_identity ) @@ -168,7 +211,7 @@ def kinit_pkinit( assert pkinit_anchor.startswith(("FILE:", "DIR:", "ENV:")) args.extend(["-X", f"X509_anchors={pkinit_anchor}"]) args.extend(["-X", f"X509_user_identity={user_identity}"]) - args.append(principal) + args.extend(['--', principal]) # this workaround enables us to capture stderr and put it # into the raised exception in case of unsuccessful authentication diff --git a/ipaserver/rpcserver.py b/ipaserver/rpcserver.py index 198fc9e7d..4f65b7e05 100644 --- a/ipaserver/rpcserver.py +++ b/ipaserver/rpcserver.py @@ -1135,10 +1135,6 @@ class login_password(Backend, KerberosSession): canonicalize=True, lifetime=self.api.env.kinit_lifetime) - if armor_path: - logger.debug('Cleanup the armor ccache') - ipautil.run([paths.KDESTROY, '-A', '-c', armor_path], - env={'KRB5CCNAME': armor_path}, raiseonerr=False) except RuntimeError as e: if ('kinit: Cannot read password while ' 'getting initial credentials') in str(e): @@ -1156,6 +1152,11 @@ class login_password(Backend, KerberosSession): raise KrbPrincipalWrongFAST(principal=principal) raise InvalidSessionPassword(principal=principal, message=unicode(e)) + finally: + if armor_path: + logger.debug('Cleanup the armor ccache') + ipautil.run([paths.KDESTROY, '-A', '-c', armor_path], + env={'KRB5CCNAME': armor_path}, raiseonerr=False) class change_password(Backend, HTTP_Status): diff --git a/ipatests/prci_definitions/gating.yaml b/ipatests/prci_definitions/gating.yaml index f208eb8cc..5921c8c3f 100644 --- a/ipatests/prci_definitions/gating.yaml +++ b/ipatests/prci_definitions/gating.yaml @@ -310,3 +310,15 @@ jobs: template: *ci-master-latest timeout: 3600 topology: *master_1repl_1client + + fedora-latest/test_ipalib_install: + requires: [fedora-latest/build] + priority: 100 + job: + class: RunPytest + args: + build_url: '{fedora-latest/build_url}' + test_suite: test_ipalib_install/test_kinit.py + template: *ci-master-latest + timeout: 600 + topology: *master_1repl diff --git a/ipatests/prci_definitions/nightly_latest.yaml b/ipatests/prci_definitions/nightly_latest.yaml index 189203783..8c39b451c 100644 --- a/ipatests/prci_definitions/nightly_latest.yaml +++ b/ipatests/prci_definitions/nightly_latest.yaml @@ -1897,3 +1897,15 @@ jobs: template: *ci-master-latest timeout: 10800 topology: *master_1repl + + fedora-latest/test_ipalib_install: + requires: [fedora-latest/build] + priority: 50 + job: + class: RunPytest + args: + build_url: '{fedora-latest/build_url}' + test_suite: test_ipalib_install/test_kinit.py + template: *ci-master-latest + timeout: 600 + topology: *master_1repl diff --git a/ipatests/prci_definitions/nightly_latest_selinux.yaml b/ipatests/prci_definitions/nightly_latest_selinux.yaml index 887a5d95c..cbbdeb8af 100644 --- a/ipatests/prci_definitions/nightly_latest_selinux.yaml +++ b/ipatests/prci_definitions/nightly_latest_selinux.yaml @@ -2048,3 +2048,16 @@ jobs: template: *ci-master-latest timeout: 10800 topology: *master_1repl + + fedora-latest/test_ipalib_install: + requires: [fedora-latest/build] + priority: 50 + job: + class: RunPytest + args: + build_url: '{fedora-latest/build_url}' + selinux_enforcing: True + test_suite: test_ipalib_install/test_kinit.py + template: *ci-master-latest + timeout: 600 + topology: *master_1repl diff --git a/ipatests/prci_definitions/nightly_latest_testing.yaml b/ipatests/prci_definitions/nightly_latest_testing.yaml index c525a3580..25805b047 100644 --- a/ipatests/prci_definitions/nightly_latest_testing.yaml +++ b/ipatests/prci_definitions/nightly_latest_testing.yaml @@ -2200,3 +2200,17 @@ jobs: template: *ci-master-latest timeout: 10800 topology: *master_1repl + + testing-fedora/test_ipalib_install: + requires: [testing-fedora/build] + priority: 50 + job: + class: RunPytest + args: + build_url: '{testing-fedora/build_url}' + update_packages: True + enable_testing_repo: True + test_suite: test_ipalib_install/test_kinit.py + template: *ci-master-latest + timeout: 600 + topology: *master_1repl diff --git a/ipatests/prci_definitions/nightly_latest_testing_selinux.yaml b/ipatests/prci_definitions/nightly_latest_testing_selinux.yaml index 40150b2bb..e4cf61d23 100644 --- a/ipatests/prci_definitions/nightly_latest_testing_selinux.yaml +++ b/ipatests/prci_definitions/nightly_latest_testing_selinux.yaml @@ -2351,3 +2351,18 @@ jobs: template: *ci-master-latest timeout: 10800 topology: *master_1repl + + testing-fedora/test_ipalib_install: + requires: [testing-fedora/build] + priority: 50 + job: + class: RunPytest + args: + build_url: '{testing-fedora/build_url}' + update_packages: True + selinux_enforcing: True + enable_testing_repo: True + test_suite: test_ipalib_install/test_kinit.py + template: *ci-master-latest + timeout: 600 + topology: *master_1repl diff --git a/ipatests/prci_definitions/nightly_previous.yaml b/ipatests/prci_definitions/nightly_previous.yaml index 0971888d6..d955c3600 100644 --- a/ipatests/prci_definitions/nightly_previous.yaml +++ b/ipatests/prci_definitions/nightly_previous.yaml @@ -1897,3 +1897,15 @@ jobs: template: *ci-master-previous timeout: 10800 topology: *master_1repl + + fedora-previous/test_ipalib_install: + requires: [fedora-previous/build] + priority: 50 + job: + class: RunPytest + args: + build_url: '{fedora-previous/build_url}' + test_suite: test_ipalib_install/test_kinit.py + template: *ci-master-previous + timeout: 600 + topology: *master_1repl diff --git a/ipatests/prci_definitions/nightly_rawhide.yaml b/ipatests/prci_definitions/nightly_rawhide.yaml index dca5abb40..3dbf312b5 100644 --- a/ipatests/prci_definitions/nightly_rawhide.yaml +++ b/ipatests/prci_definitions/nightly_rawhide.yaml @@ -2048,3 +2048,16 @@ jobs: template: *ci-master-frawhide timeout: 10800 topology: *master_1repl + + fedora-rawhide/test_ipalib_install: + requires: [fedora-rawhide/build] + priority: 50 + job: + class: RunPytest + args: + build_url: '{fedora-rawhide/build_url}' + update_packages: True + test_suite: test_ipalib_install/test_kinit.py + template: *ci-master-frawhide + timeout: 600 + topology: *master_1repl diff --git a/ipatests/setup.py b/ipatests/setup.py index 6217a1ba5..0aec4a70d 100644 --- a/ipatests/setup.py +++ b/ipatests/setup.py @@ -41,6 +41,7 @@ if __name__ == '__main__': "ipatests.test_integration", "ipatests.test_ipaclient", "ipatests.test_ipalib", + "ipatests.test_ipalib_install", "ipatests.test_ipaplatform", "ipatests.test_ipapython", "ipatests.test_ipaserver", diff --git a/ipatests/test_ipalib_install/__init__.py b/ipatests/test_ipalib_install/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/ipatests/test_ipalib_install/test_kinit.py b/ipatests/test_ipalib_install/test_kinit.py new file mode 100644 index 000000000..f89ea17d7 --- /dev/null +++ b/ipatests/test_ipalib_install/test_kinit.py @@ -0,0 +1,29 @@ +# +# Copyright (C) 2024 FreeIPA Contributors see COPYING for license +# +"""Tests for ipalib.install.kinit module +""" + +import pytest + +from ipalib.install.kinit import validate_principal + + +# None means no exception is expected +@pytest.mark.parametrize('principal, exception', [ + ('testuser', None), + ('testuser@EXAMPLE.TEST', None), + ('test/ipa.example.test', None), + ('test/ipa.example.test@EXAMPLE.TEST', None), + ('test/ipa@EXAMPLE.TEST', RuntimeError), + ('test/-ipa.example.test@EXAMPLE.TEST', RuntimeError), + ('test/ipa.1example.test@EXAMPLE.TEST', RuntimeError), + ('test /ipa.example,test', RuntimeError), + ('testuser@OTHER.TEST', RuntimeError), + ('test/ipa.example.test@OTHER.TEST', RuntimeError), +]) +def test_validate_principal(principal, exception): + try: + validate_principal(principal) + except Exception as e: + assert e.__class__ == exception diff --git a/tox.ini b/tox.ini index 274d352e8..607f96244 100644 --- a/tox.ini +++ b/tox.ini @@ -19,7 +19,7 @@ deps= ipatests commands= {envbindir}/ipa --help - {envbindir}/ipa-run-tests --junitxml={envdir}/junit-{envname}.xml {posargs:--ipaclient-unittests} + {envbindir}/ipa-run-tests --junitxml={envdir}/junit-{envname}.xml {posargs:--ipaclient-unittests} --ignore test_ipalib_install [testenv:pylint3] basepython=python3