Skip to content

Commit

Permalink
validate_principal: Don't try to verify that the realm is known
Browse files Browse the repository at this point in the history
The actual value is less important than whether it matches the
regular expression. A number of legal but difficult to know in
context realms could be passed in here (trust for example).

This fixes CVE-2024-1481

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

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
  • Loading branch information
rcritten authored and flo-renaud committed Feb 23, 2024
1 parent b039f30 commit 96a478b
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 11 deletions.
12 changes: 4 additions & 8 deletions ipalib/install/kinit.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
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__)

Expand All @@ -39,7 +38,9 @@ def validate_principal(principal):
if ('/' in principal) and (' ' in principal):
raise RuntimeError('Invalid principal: bad spacing')
else:
realm = None
# For a user match in the regex
# username = match[1]
# realm = match[2]
match = user_pattern.match(principal)
if match is None:
match = service_pattern.match(principal)
Expand All @@ -48,16 +49,11 @@ def validate_principal(principal):
else:
# service = match[1]
hostname = match[2]
realm = match[3]
# 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):
Expand Down
9 changes: 6 additions & 3 deletions ipatests/test_ipalib_install/test_kinit.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@
('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.1example.test@EXAMPLE.TEST', None),
('test /ipa.example,test', RuntimeError),
('testuser@OTHER.TEST', RuntimeError),
('test/ipa.example.test@OTHER.TEST', RuntimeError),
('testuser@OTHER.TEST', None),
('test/ipa.example.test@OTHER.TEST', None)
])
def test_validate_principal(principal, exception):
try:
validate_principal(principal)
except Exception as e:
assert e.__class__ == exception
else:
if exception is not None:
raise RuntimeError('Test should have failed')

0 comments on commit 96a478b

Please sign in to comment.