273 lines
11 KiB
Diff
273 lines
11 KiB
Diff
|
From a5d2857297cfcf87ed8973df96e89ebcef22850d Mon Sep 17 00:00:00 2001
|
||
|
From: Antonio Torres <antorres@redhat.com>
|
||
|
Date: Mon, 8 Mar 2021 18:15:50 +0100
|
||
|
Subject: [PATCH] Add checks to prevent adding auth indicators to internal IPA
|
||
|
services
|
||
|
|
||
|
Authentication indicators should not be enforced against internal
|
||
|
IPA services, since not all users of those services are able to produce
|
||
|
Kerberos tickets with all the auth indicator options. This includes
|
||
|
host, ldap, HTTP and cifs in IPA server and cifs in IPA clients.
|
||
|
If a client that is being promoted to replica has an auth indicator
|
||
|
in its host principal then the promotion is aborted.
|
||
|
|
||
|
Fixes: https://pagure.io/freeipa/issue/8206
|
||
|
Signed-off-by: Antonio Torres <antorres@redhat.com>
|
||
|
---
|
||
|
ipaserver/install/server/replicainstall.py | 13 ++++++++++++
|
||
|
ipaserver/plugins/host.py | 5 ++++-
|
||
|
ipaserver/plugins/service.py | 24 ++++++++++++++++++++++
|
||
|
3 files changed, 41 insertions(+), 1 deletion(-)
|
||
|
|
||
|
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
|
||
|
index 73967a224..f1fb91036 100644
|
||
|
--- a/ipaserver/install/server/replicainstall.py
|
||
|
+++ b/ipaserver/install/server/replicainstall.py
|
||
|
@@ -770,6 +770,15 @@ def promotion_check_ipa_domain(master_ldap_conn, basedn):
|
||
|
))
|
||
|
|
||
|
|
||
|
+def promotion_check_host_principal_auth_ind(conn, hostdn):
|
||
|
+ entry = conn.get_entry(hostdn, ['krbprincipalauthind'])
|
||
|
+ if 'krbprincipalauthind' in entry:
|
||
|
+ raise RuntimeError(
|
||
|
+ "Client cannot be promoted to a replica if the host principal "
|
||
|
+ "has an authentication indicator set."
|
||
|
+ )
|
||
|
+
|
||
|
+
|
||
|
@common_cleanup
|
||
|
@preserve_enrollment_state
|
||
|
def promote_check(installer):
|
||
|
@@ -956,6 +965,10 @@ def promote_check(installer):
|
||
|
config.master_host_name, None)
|
||
|
|
||
|
promotion_check_ipa_domain(conn, remote_api.env.basedn)
|
||
|
+ hostdn = DN(('fqdn', api.env.host),
|
||
|
+ api.env.container_host,
|
||
|
+ api.env.basedn)
|
||
|
+ promotion_check_host_principal_auth_ind(conn, hostdn)
|
||
|
|
||
|
# Make sure that domain fulfills minimal domain level
|
||
|
# requirement
|
||
|
diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py
|
||
|
index eb1f8ef04..41fa933e2 100644
|
||
|
--- a/ipaserver/plugins/host.py
|
||
|
+++ b/ipaserver/plugins/host.py
|
||
|
@@ -38,7 +38,7 @@ from .baseldap import (LDAPQuery, LDAPObject, LDAPCreate,
|
||
|
LDAPAddAttributeViaOption,
|
||
|
LDAPRemoveAttributeViaOption)
|
||
|
from .service import (
|
||
|
- validate_realm, normalize_principal,
|
||
|
+ validate_realm, validate_auth_indicator, normalize_principal,
|
||
|
set_certificate_attrs, ticket_flags_params, update_krbticketflags,
|
||
|
set_kerberos_attrs, rename_ipaallowedtoperform_from_ldap,
|
||
|
rename_ipaallowedtoperform_to_ldap, revoke_certs)
|
||
|
@@ -735,6 +735,8 @@ class host_add(LDAPCreate):
|
||
|
update_krbticketflags(ldap, entry_attrs, attrs_list, options, False)
|
||
|
if 'krbticketflags' in entry_attrs:
|
||
|
entry_attrs['objectclass'].append('krbticketpolicyaux')
|
||
|
+ validate_auth_indicator(entry_attrs)
|
||
|
+
|
||
|
return dn
|
||
|
|
||
|
def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
|
||
|
@@ -993,6 +995,7 @@ class host_mod(LDAPUpdate):
|
||
|
if 'krbprincipalaux' not in (item.lower() for item in
|
||
|
entry_attrs['objectclass']):
|
||
|
entry_attrs['objectclass'].append('krbprincipalaux')
|
||
|
+ validate_auth_indicator(entry_attrs)
|
||
|
|
||
|
add_sshpubkey_to_attrs_pre(self.context, attrs_list)
|
||
|
|
||
|
diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py
|
||
|
index 1c9347804..cfbbff3c6 100644
|
||
|
--- a/ipaserver/plugins/service.py
|
||
|
+++ b/ipaserver/plugins/service.py
|
||
|
@@ -201,6 +201,28 @@ def validate_realm(ugettext, principal):
|
||
|
raise errors.RealmMismatch()
|
||
|
|
||
|
|
||
|
+def validate_auth_indicator(entry):
|
||
|
+ new_value = entry.get('krbprincipalauthind', None)
|
||
|
+ if not new_value:
|
||
|
+ return
|
||
|
+ # The following services are considered internal IPA services
|
||
|
+ # and shouldn't be allowed to have auth indicators.
|
||
|
+ # https://pagure.io/freeipa/issue/8206
|
||
|
+ pkey = api.Object['service'].get_primary_key_from_dn(entry.dn)
|
||
|
+ principal = kerberos.Principal(pkey)
|
||
|
+ server = api.Command.server_find(principal.hostname)['result']
|
||
|
+ if server:
|
||
|
+ prefixes = ("host", "cifs", "ldap", "HTTP")
|
||
|
+ else:
|
||
|
+ prefixes = ("cifs",)
|
||
|
+ if principal.service_name in prefixes:
|
||
|
+ raise errors.ValidationError(
|
||
|
+ name='krbprincipalauthind',
|
||
|
+ error=_('authentication indicators not allowed '
|
||
|
+ 'in service "%s"' % principal.service_name)
|
||
|
+ )
|
||
|
+
|
||
|
+
|
||
|
def normalize_principal(value):
|
||
|
"""
|
||
|
Ensure that the name in the principal is lower-case. The realm is
|
||
|
@@ -652,6 +674,7 @@ class service_add(LDAPCreate):
|
||
|
hostname)
|
||
|
|
||
|
self.obj.validate_ipakrbauthzdata(entry_attrs)
|
||
|
+ validate_auth_indicator(entry_attrs)
|
||
|
|
||
|
if not options.get('force', False):
|
||
|
# We know the host exists if we've gotten this far but we
|
||
|
@@ -846,6 +869,7 @@ class service_mod(LDAPUpdate):
|
||
|
assert isinstance(dn, DN)
|
||
|
|
||
|
self.obj.validate_ipakrbauthzdata(entry_attrs)
|
||
|
+ validate_auth_indicator(entry_attrs)
|
||
|
|
||
|
# verify certificates
|
||
|
certs = entry_attrs.get('usercertificate') or []
|
||
|
--
|
||
|
2.31.1
|
||
|
|
||
|
From 28484c3dee225662e41acc691bfe6b1c1cee99c8 Mon Sep 17 00:00:00 2001
|
||
|
From: Antonio Torres <antorres@redhat.com>
|
||
|
Date: Mon, 8 Mar 2021 18:20:35 +0100
|
||
|
Subject: [PATCH] ipatests: ensure auth indicators can't be added to internal
|
||
|
IPA services
|
||
|
|
||
|
Authentication indicators should not be added to internal IPA services,
|
||
|
since this can lead to a broken IPA setup. In case a client with
|
||
|
an auth indicator set in its host principal, promoting it to a replica
|
||
|
should fail.
|
||
|
|
||
|
Related: https://pagure.io/freeipa/issue/8206
|
||
|
Signed-off-by: Antonio Torres <antorres@redhat.com>
|
||
|
---
|
||
|
.../test_replica_promotion.py | 38 +++++++++++++++++++
|
||
|
ipatests/test_xmlrpc/test_host_plugin.py | 10 +++++
|
||
|
ipatests/test_xmlrpc/test_service_plugin.py | 21 ++++++++++
|
||
|
3 files changed, 69 insertions(+)
|
||
|
|
||
|
diff --git a/ipatests/test_integration/test_replica_promotion.py b/ipatests/test_integration/test_replica_promotion.py
|
||
|
index 0a137dbdc..b9c56f775 100644
|
||
|
--- a/ipatests/test_integration/test_replica_promotion.py
|
||
|
+++ b/ipatests/test_integration/test_replica_promotion.py
|
||
|
@@ -101,6 +101,44 @@ class TestReplicaPromotionLevel1(ReplicaPromotionBase):
|
||
|
assert result.returncode == 1
|
||
|
assert expected_err in result.stderr_text
|
||
|
|
||
|
+ @replicas_cleanup
|
||
|
+ def test_install_with_host_auth_ind_set(self):
|
||
|
+ """ A client shouldn't be able to be promoted if it has
|
||
|
+ any auth indicator set in the host principal.
|
||
|
+ https://pagure.io/freeipa/issue/8206
|
||
|
+ """
|
||
|
+
|
||
|
+ client = self.replicas[0]
|
||
|
+ # Configure firewall first
|
||
|
+ Firewall(client).enable_services(["freeipa-ldap",
|
||
|
+ "freeipa-ldaps"])
|
||
|
+
|
||
|
+ client.run_command(['ipa-client-install', '-U',
|
||
|
+ '--domain', self.master.domain.name,
|
||
|
+ '--realm', self.master.domain.realm,
|
||
|
+ '-p', 'admin',
|
||
|
+ '-w', self.master.config.admin_password,
|
||
|
+ '--server', self.master.hostname,
|
||
|
+ '--force-join'])
|
||
|
+
|
||
|
+ tasks.kinit_admin(client)
|
||
|
+
|
||
|
+ client.run_command(['ipa', 'host-mod', '--auth-ind=otp',
|
||
|
+ client.hostname])
|
||
|
+
|
||
|
+ res = client.run_command(['ipa-replica-install', '-U', '-w',
|
||
|
+ self.master.config.dirman_password],
|
||
|
+ raiseonerr=False)
|
||
|
+
|
||
|
+ client.run_command(['ipa', 'host-mod', '--auth-ind=',
|
||
|
+ client.hostname])
|
||
|
+
|
||
|
+ expected_err = ("Client cannot be promoted to a replica if the host "
|
||
|
+ "principal has an authentication indicator set.")
|
||
|
+ assert res.returncode == 1
|
||
|
+ assert expected_err in res.stderr_text
|
||
|
+
|
||
|
+
|
||
|
@replicas_cleanup
|
||
|
def test_one_command_installation(self):
|
||
|
"""
|
||
|
diff --git a/ipatests/test_xmlrpc/test_host_plugin.py b/ipatests/test_xmlrpc/test_host_plugin.py
|
||
|
index c66bbc865..9cfde3565 100644
|
||
|
--- a/ipatests/test_xmlrpc/test_host_plugin.py
|
||
|
+++ b/ipatests/test_xmlrpc/test_host_plugin.py
|
||
|
@@ -605,6 +605,16 @@ class TestProtectedMaster(XMLRPC_test):
|
||
|
error=u'An IPA master host cannot be deleted or disabled')):
|
||
|
command()
|
||
|
|
||
|
+ def test_try_add_auth_ind_master(self, this_host):
|
||
|
+ command = this_host.make_update_command({
|
||
|
+ u'krbprincipalauthind': u'radius'})
|
||
|
+ with raises_exact(errors.ValidationError(
|
||
|
+ name='krbprincipalauthind',
|
||
|
+ error=u'authentication indicators not allowed '
|
||
|
+ 'in service "host"'
|
||
|
+ )):
|
||
|
+ command()
|
||
|
+
|
||
|
|
||
|
@pytest.mark.tier1
|
||
|
class TestValidation(XMLRPC_test):
|
||
|
diff --git a/ipatests/test_xmlrpc/test_service_plugin.py b/ipatests/test_xmlrpc/test_service_plugin.py
|
||
|
index 4c845938c..ed634a045 100644
|
||
|
--- a/ipatests/test_xmlrpc/test_service_plugin.py
|
||
|
+++ b/ipatests/test_xmlrpc/test_service_plugin.py
|
||
|
@@ -25,6 +25,7 @@ from ipalib import api, errors
|
||
|
from ipatests.test_xmlrpc.xmlrpc_test import Declarative, fuzzy_uuid, fuzzy_hash
|
||
|
from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_digits, fuzzy_date, fuzzy_issuer
|
||
|
from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_hex, XMLRPC_test
|
||
|
+from ipatests.test_xmlrpc.xmlrpc_test import raises_exact
|
||
|
from ipatests.test_xmlrpc import objectclasses
|
||
|
from ipatests.test_xmlrpc.testcert import get_testcert, subject_base
|
||
|
from ipatests.test_xmlrpc.test_user_plugin import get_user_result, get_group_dn
|
||
|
@@ -1552,6 +1553,15 @@ def indicators_host(request):
|
||
|
return tracker.make_fixture(request)
|
||
|
|
||
|
|
||
|
+@pytest.fixture(scope='function')
|
||
|
+def this_host(request):
|
||
|
+ """Fixture for the current master"""
|
||
|
+ tracker = HostTracker(name=api.env.host.partition('.')[0],
|
||
|
+ fqdn=api.env.host)
|
||
|
+ tracker.exists = True
|
||
|
+ return tracker
|
||
|
+
|
||
|
+
|
||
|
@pytest.fixture(scope='function')
|
||
|
def indicators_service(request):
|
||
|
tracker = ServiceTracker(
|
||
|
@@ -1587,6 +1597,17 @@ class TestAuthenticationIndicators(XMLRPC_test):
|
||
|
expected_updates={u'krbprincipalauthind': [u'radius']}
|
||
|
)
|
||
|
|
||
|
+ def test_update_indicator_internal_service(self, this_host):
|
||
|
+ command = this_host.make_command('service_mod',
|
||
|
+ 'ldap/' + this_host.fqdn,
|
||
|
+ **dict(krbprincipalauthind='otp'))
|
||
|
+ with raises_exact(errors.ValidationError(
|
||
|
+ name='krbprincipalauthind',
|
||
|
+ error=u'authentication indicators not allowed '
|
||
|
+ 'in service "ldap"'
|
||
|
+ )):
|
||
|
+ command()
|
||
|
+
|
||
|
|
||
|
@pytest.fixture(scope='function')
|
||
|
def managing_host(request):
|
||
|
--
|
||
|
2.31.1
|
||
|
|