IntenseWebs/freeipa
3
0
Fork 0
mirror of https://salsa.debian.org/freeipa-team/freeipa.git synced 2025-02-25 18:55:28 -06:00
Code Issues Packages Projects Releases Wiki Activity

rpcserver: validate Kerberos principal name before running kinit

Browse Source 
Do minimal validation of the Kerberos principal name when passing it to
kinit command line tool. Also pass it as the final argument to prevent
option injection.

Accepted Kerberos principals are:
 - user names, using the following regexp
   (username with optional @realm, no spaces or slashes in the name):
   "(?!^[0-9]+$)^[a-zA-Z0-9_.][a-zA-Z0-9_.-]*[a-zA-Z0-9_.$-]?@?[a-zA-Z0-9.-]*$"

 - service names (with slash in the name but no spaces). Validation of
   the hostname is done. There is no validation of the service name.

The regular expression above also covers cases where a principal name
starts with '-'. This prevents option injection as well.

This fixes CVE-2024-1481

Fixes: https://pagure.io/freeipa/issue/9541

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
This commit is contained in:
Alexander Bokovoy 2024-02-07 13:09:54 +02:00 committed by Rob Crittenden
parent dc3e902b0b
commit 404fe1018e
13 changed files with 172 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
ipalib/install/kinit.py
View File

@ -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

9
ipaserver/rpcserver.py
View File

@ -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):

12
ipatests/prci_definitions/gating.yaml
View File

@ -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

12
ipatests/prci_definitions/nightly_latest.yaml
View File

@ -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

13
ipatests/prci_definitions/nightly_latest_selinux.yaml
View File

@ -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

14
ipatests/prci_definitions/nightly_latest_testing.yaml
View File

@ -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

15
ipatests/prci_definitions/nightly_latest_testing_selinux.yaml
View File

@ -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

12
ipatests/prci_definitions/nightly_previous.yaml
View File

@ -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

13
ipatests/prci_definitions/nightly_rawhide.yaml
View File

@ -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

1
ipatests/setup.py
View File

@ -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",

0
ipatests/test_ipalib_install/__init__.py Normal file
View File

29
ipatests/test_ipalib_install/test_kinit.py Normal file
View File

@ -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

2
tox.ini
View File

@ -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