On 09/18/2014 09:42 PM, Martin Kosek wrote:
On 09/18/2014 09:11 PM, Simo Sorce wrote:
On Thu, 18 Sep 2014 14:57:45 -0400
Rob Crittenden <rcrit...@redhat.com> wrote:
Martin Kosek wrote:
On 09/18/2014 04:06 PM, David Kupka wrote:
On 09/18/2014 03:44 PM, Rob Crittenden wrote:
David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/4421
You are removing an ACI in this patch. It is always possible it
is no longer needed. Did you test all the client enrollment
scenarios?
rob
As far as I'm aware I'm not removing any ACI. I'm modifying ACI so
it is possible to add krbPrincipalName to host even when there is
already one (or more). And adding one ACI to allow writing
krbCanonicalName to host. But I'm still not really familiar with
ACI so please correct me if I'm wrong.
What refers to is probably the update in ACI.txt - the ACI
alternative to API.txt. David updated an ACI, not removed it.
On that note, what is the reason for this permission change:
- 'ipapermtargetfilter': [
- '(objectclass=ipahost)',
- '(!(krbprincipalname=*))',
- ],
?
Yes, this is exactly the change I was referring to. Permission changes
within a plugin now translate automatically to ACI changes. Sorry I
wasn't clearer.
This ACI gets replaced with a much simpler one and I'm not 100% sure
it will work with all enrollments:
-aci: (targetattr = "krbprincipalname")(targetfilter =
"(&(!(krbprincipalname=*))(objectclass=ipahost))")(version 3.0;acl
"permission:System: Add krbPrincipalName to a Host";allow (write)
groupdn = "ldap:///cn=System: Add krbPrincipalName to a
Host,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+aci: (targetattr = "krbprincipalname")(targetfilter =
"(objectclass=ipahost)")(version 3.0;acl "permission:System: Add
krbPrincipalName to a Host";allow (write) groupdn =
"ldap:///cn=System: Add krbPrincipalName to a
Host,cn=permissions,cn=pbac,dc=ipa,dc=example";)
The first one restricts writing the attribute only if it isn't already
set. The second lets it be changed unconditionally.
Yeah this is wrong indeed, the point of the ACI is to allow setting the
principal only when it is not already set, which is the OTP enrollment
case. But if krbprincipal is set then this specific permission should
not grant rights to change it.
At least this was my understanding.
Simo.
Right. It seems to me we should add keep this permission intact and add
a new permission allowing adding krbPrincipalName aliases. This would
allow writing both krbPrincipalName and krbCanonicalName.
Martin
Thank you all for explanation and help. I rewrote the patch so it should
work as requested now. Also I added tests to reassure the behavior is
correct.
--
David Kupka
From 3e4a4ce3d951bd83ec046025f5acbfc7a0823f6e Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Tue, 2 Sep 2014 16:11:55 +0200
Subject: [PATCH] Allow multiple krbprincipalnames.
Allow user to specify multiple krbprincipalnames and krbcanonicalname.
User must have "IT specialist" role or "Host Administrators" privilege
assigned.
https://fedorahosted.org/freeipa/ticket/4421
---
ACI.txt | 2 ++
API.txt | 2 +-
ipalib/plugins/host.py | 50 ++++++++++++++++++++++++++++++++++++++++----------
3 files changed, 43 insertions(+), 11 deletions(-)
diff --git a/ACI.txt b/ACI.txt
index 1e6bec0ece554fb2457fae0462c0c673a9b24e41..1ac48c938c86a42082e4eb9a2cbbc42d9eb6fe8d 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -98,6 +98,8 @@ dn: cn=computers,cn=accounts,dc=ipa,dc=example
aci: (targetattr = "ipasshpubkey")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Manage Host SSH Public Keys";allow (write) groupdn = "ldap:///cn=System: Manage Host SSH Public Keys,cn=permissions,cn=pbac,dc=ipa,dc=example";)
dn: cn=computers,cn=accounts,dc=ipa,dc=example
aci: (targetattr = "description || l || macaddress || nshardwareplatform || nshostlocation || nsosversion || userclass")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Modify Hosts";allow (write) groupdn = "ldap:///cn=System: Modify Hosts,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+dn: cn=computers,cn=accounts,dc=ipa,dc=example
+aci: (targetattr = "krbcanonicalname || krbprincipalname")(targetfilter = "(objectclass=ipahost)")(version 3.0;acl "permission:System: Modify krbCanonicalName and krbPrincipalName to a Host";allow (write) groupdn = "ldap:///cn=System: Modify krbCanonicalName and krbPrincipalName to a Host,cn=permissions,cn=pbac,dc=ipa,dc=example";)
dn: dc=ipa,dc=example
aci: (targetattr = "cn || createtimestamp || entryusn || macaddress || modifytimestamp || objectclass")(target = "ldap:///cn=computers,cn=compat,dc=ipa,dc=example";)(version 3.0;acl "permission:System: Read Host Compat Tree";allow (compare,read,search) userdn = "ldap:///anyone";;)
dn: cn=computers,cn=accounts,dc=ipa,dc=example
diff --git a/API.txt b/API.txt
index bbd0f507b2faeec0239920cdcff28fe25d618e02..ef1f70397e7685161821a98a92ea575aa6eff532 100644
--- a/API.txt
+++ b/API.txt
@@ -1884,7 +1884,7 @@ option: Str('description', attribute=True, autofill=False, cli_name='desc', mult
option: Bool('ipakrbokasdelegate', attribute=False, autofill=False, cli_name='ok_as_delegate', multivalue=False, required=False)
option: Bool('ipakrbrequirespreauth', attribute=False, autofill=False, cli_name='requires_pre_auth', multivalue=False, required=False)
option: Str('ipasshpubkey', attribute=True, autofill=False, cli_name='sshpubkey', csv=True, multivalue=True, required=False)
-option: Str('krbprincipalname?', attribute=True, cli_name='principalname')
+option: Str('krbprincipalname?', attribute=True, cli_name='principalname', multivalue=True)
option: Str('l', attribute=True, autofill=False, cli_name='locality', multivalue=False, required=False)
option: Str('macaddress', attribute=True, autofill=False, cli_name='macaddress', csv=True, multivalue=True, pattern='^([a-fA-F0-9]{2}[:|\\-]?){5}[a-fA-F0-9]{2}$', required=False)
option: Flag('no_members', autofill=True, default=False, exclude='webui')
diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 570bbe56aa0a315a031051f9a895702ba7c35076..8a2ab90e737c35ab0981d1257fe0acd3ce76f4ba 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -313,6 +313,11 @@ class host(LDAPObject):
],
'default_privileges': {'Host Administrators', 'Host Enrollment'},
},
+ 'System: Modify krbCanonicalName and krbPrincipalName to a Host': {
+ 'ipapermright': {'write'},
+ 'ipapermdefaultattr': {'krbcanonicalname', 'krbprincipalname'},
+ 'default_privileges': {'Host Administrators'},
+ },
'System: Enroll a Host': {
'ipapermright': {'write'},
'ipapermdefaultattr': {'objectclass', 'enrolledby'},
@@ -761,6 +766,7 @@ class host_mod(LDAPUpdate):
label=_('Principal name'),
doc=_('Kerberos principal name for this host'),
attribute=True,
+ multivalue=True,
),
Flag('updatedns?',
doc=_('Update DNS entries'),
@@ -778,22 +784,46 @@ class host_mod(LDAPUpdate):
if not entry['has_password'] and entry['has_keytab']:
raise errors.ValidationError(name='password', error=_('Password cannot be set on enrolled host.'))
- # Once a principal name is set it cannot be changed
if 'cn' in entry_attrs:
raise errors.ACIError(info=_('cn is immutable'))
if 'locality' in entry_attrs:
entry_attrs['l'] = entry_attrs['locality']
- if 'krbprincipalname' in entry_attrs:
+ # whenever changing canonicalname or pricipalname check if all constrains are met
+ if 'krbcanonicalname' in entry_attrs or 'krbprincipalname' in entry_attrs:
entry_attrs_old = ldap.get_entry(
- dn, ['objectclass', 'krbprincipalname']
+ dn, ['objectclass', 'krbprincipalname', 'krbcanonicalname']
)
- if 'krbprincipalname' in entry_attrs_old:
- msg = 'Principal name already set, it is unchangeable.'
- raise errors.ACIError(info=msg)
- obj_classes = entry_attrs_old['objectclass']
- if 'krbprincipalaux' not in obj_classes:
- obj_classes.append('krbprincipalaux')
- entry_attrs['objectclass'] = obj_classes
+ krbcanonicalname = entry_attrs.get('krbcanonicalname',
+ entry_attrs_old.get('krbcanonicalname', []))
+ krbprincipalnames = entry_attrs.get('krbprincipalname',
+ entry_attrs_old.get('krbprincipalname', []))
+
+ # TODO: remove this workaround when ldap_base is fixed
+ # entry_attrs.get('krbcanonicalname') returns None or
+ # unicode or list
+ if krbcanonicalname is None:
+ krbcanonicalname = []
+ if isinstance(krbcanonicalname, unicode):
+ krbcanonicalname = [krbcanonicalname]
+
+ if len(krbcanonicalname) == 0:
+ if len(krbprincipalnames) > 1:
+ msg = 'must be set when using multiple krbprincipalname values.'
+ raise errors.ValidationError(name='krbcanonicalname', error=msg)
+ elif len(krbcanonicalname) == 1:
+ if len(krbprincipalnames) > 0:
+ if krbcanonicalname[0] not in krbprincipalnames:
+ msg = 'must be one of krbprincipalname values.'
+ raise errors.ValidationError(name='krbcanonicalname', error=msg)
+ else:
+ msg = 'only one value allowed.'
+ raise errors.ValidationError(name='krbcanonicalname', error=msg)
+
+ if 'krbprincipalname' in entry_attrs:
+ obj_classes = entry_attrs_old['objectclass']
+ if 'krbprincipalaux' not in obj_classes:
+ obj_classes.append('krbprincipalaux')
+ entry_attrs['objectclass'] = obj_classes
cert = x509.normalize_certificate(entry_attrs.get('usercertificate'))
if cert:
if self.api.env.enable_ra:
--
1.9.3
From a55989664b7b4b8a441ca91971b349e077c3e186 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 18 Sep 2014 01:53:49 -0400
Subject: [PATCH] Test: Allow multiple krbPrincipalNames
Allow to add multiple krbPrincipalName. krbCanonicalName must be specified
when adding second and next krbPrincipalName.
Related to: https://fedorahosted.org/freeipa/ticket/4421
---
ipatests/test_xmlrpc/test_host_plugin.py | 139 ++++++++++++++++++++++++++++++-
1 file changed, 137 insertions(+), 2 deletions(-)
diff --git a/ipatests/test_xmlrpc/test_host_plugin.py b/ipatests/test_xmlrpc/test_host_plugin.py
index 7c0312bae5e9e652085aac8f1970610044504281..0335e85508070ab0c9cf4c6c4841a9a599a57213 100644
--- a/ipatests/test_xmlrpc/test_host_plugin.py
+++ b/ipatests/test_xmlrpc/test_host_plugin.py
@@ -32,11 +32,12 @@ from nose.tools import raises, assert_raises
from nose.plugins.skip import SkipTest
from ipatests.test_xmlrpc.xmlrpc_test import (Declarative, XMLRPC_test,
fuzzy_uuid, fuzzy_digits, fuzzy_hash, fuzzy_date, fuzzy_issuer,
- fuzzy_hex)
+ fuzzy_hex, fuzzy_string)
from ipatests.test_xmlrpc import objectclasses
from ipatests.test_xmlrpc.testcert import get_testcert
import base64
-
+from ipatests.util import assert_deepequal
+from ipapython.version import API_VERSION
fqdn1 = u'testhost1.%s' % api.env.domain
short1 = u'testhost1'
@@ -136,6 +137,19 @@ ipv6_fromip_ptr_dn = DN(('idnsname', ipv6_fromip_ptr), revipv6zone_dn)
sshpubkey = u'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDGAX3xAeLeaJggwTqMjxNwa6XHBUAikXPGMzEpVrlLDCZtv00djsFTBi38PkgxBJVkgRWMrcBsr/35lq7P6w8KGIwA8GI48Z0qBS2NBMJ2u9WQ2hjLN6GdMlo77O0uJY3251p12pCVIS/bHRSq8kHO2No8g7KA9fGGcagPfQH+ee3t7HUkpbQkFTmbPPN++r3V8oVUk5LxbryB3UIIVzNmcSIn3JrXynlvui4MixvrtX6zx+O/bBo68o8/eZD26QrahVbA09fivrn/4h3TM019Eu/c2jOdckfU3cHUV/3Tno5d6JicibyaoDDK7S/yjdn5jhaz8MSEayQvFkZkiF0L public key test'
sshpubkeyfp = u'13:67:6B:BF:4E:A2:05:8E:AE:25:8B:A1:31:DE:6F:1B public key test (ssh-rsa)'
+
+# multiple krbPrincipalNames test
+host5 = u'test'
+fqdn5 = u'%s.%s' % (host5, api.env.domain)
+dn5 = DN(('fqdn',fqdn5),('cn','computers'),('cn','accounts'),api.env.basedn)
+principalname1 = u'host/%s@%s' % (fqdn5, api.env.realm)
+principalname2 = u'IPA01$@%s' % api.env.realm
+canonicalname = principalname1
+badcanonicalname = u'BADCANONICALNAME@%s' % api.env.realm
+ipaddress = u'172.16.0.1'
+it_spec_role_dn = DN(('cn',u'IT Specialist'),api.env.container_rolegroup,
+ api.env.basedn)
+
class test_host(Declarative):
cleanup_commands = [
@@ -1403,3 +1417,124 @@ class test_host_dns(Declarative):
),
),
]
+
+class test_host_multiple_krbprincipalnames(XMLRPC_test):
+ """
+ Configure multiple krbPrincipalNames to host.
+ """
+ def clean(self):
+ api.Command['host_del'](fqdn5, updatedns=True, **{'continue': True})
+
+ def setUp(self):
+ self.clean()
+ # TODO: remove if admin gets 'IT Specialist' role by default
+ api.Command['role_add_member'](u'IT Specialist', user=[u'admin'])
+
+ def tearDown(self):
+ self.clean()
+ # TODO: remove if admin gets 'IT Specialist' role by default
+ api.Command['role_remove_member'](u'IT Specialist', user=[u'admin'])
+
+
+ def test_host_multiple_krbprincipalnames(self):
+
+ nice = desc='Add host %s' % fqdn5
+ got = api.Command['host_add'](fqdn5, ip_address=ipaddress, no_reverse=True, version=API_VERSION)
+ expected=dict(
+ value=fqdn5,
+ summary=u'Added host "%s"' % fqdn5,
+ result=dict(
+ dn=dn5,
+ fqdn=[fqdn5],
+ has_keytab=False,
+ has_password=False,
+ ipauniqueid=[fuzzy_uuid],
+ krbprincipalname=[principalname1],
+ managedby_host=[fqdn5],
+ objectclass=objectclasses.host,
+ )
+ )
+ assert_deepequal(expected, got, nice)
+
+ nice = 'Add krbCanonicalName %s' % canonicalname
+ got = api.Command['host_mod'](fqdn5, addattr=u'krbcanonicalname=%s' % canonicalname, version=API_VERSION)
+ expected=dict(
+ value=fqdn5,
+ summary=u'Modified host "%s"' % fqdn5,
+ result=dict(
+ fqdn=[fqdn5],
+ has_keytab=False,
+ has_password=False,
+ krbcanonicalname=[canonicalname],
+ krbprincipalname=[principalname1],
+ managedby_host=[fqdn5],
+ )
+ )
+ assert_deepequal(expected, got, nice)
+
+ nice = 'Add kbrPrincipalName %s' % principalname2
+ got = api.Command['host_mod'](fqdn5, addattr=u'krbprincipalname=%s' % principalname2, version=API_VERSION)
+ expected=dict(
+ value=fqdn5,
+ summary=u'Modified host "%s"' % fqdn5,
+ result=dict(
+ fqdn=[fqdn5],
+ has_keytab=False,
+ has_password=False,
+ krbprincipalname=[principalname1, principalname2],
+ managedby_host=[fqdn5],
+ )
+ )
+ assert_deepequal(expected, got, nice)
+
+class test_host_fail_to_add_krbprincipalname_when_krbcanonicalname_missing(XMLRPC_test):
+ """
+ Missing krbCanonicalName.
+ """
+ def clean(self):
+ api.Command['host_del'](fqdn5, updatedns=True, **{'continue': True})
+
+ def setUp(self):
+ self.clean()
+ # TODO: Remove when/if admin has that role by default
+ api.Command['role_add_member'](u'IT Specialist', user=[u'admin'])
+ api.Command['host_add'](fqdn5, ip_address=ipaddress, no_reverse=True)
+
+ def tearDown(self):
+ self.clean()
+ # TODO: Remove when/if admin has that role by default
+ api.Command['role_remove_member'](u'IT Specialist', user=u'admin')
+
+ @raises(errors.ValidationError)
+ def test_try_add_second_krbPrincipalName(self):
+ """
+ Try to add second krbPrincipalName without krbCanonicalName.
+ """
+ api.Command['host_mod'](fqdn5, addattr=u'krbprincipalname=%s' % principalname2)
+
+class test_host_fail_to_have_krbcanonicalname_not_in_krbprincipalname(XMLRPC_test):
+ """
+ krbCanonicalName not in krbPrincipalName.
+ """
+ def clean(self):
+ api.Command['host_del'](fqdn5, updatedns=True, **{'continue': True})
+
+ def setUp(self):
+ self.clean()
+ # TODO: Remove when/if admin has that role by default
+ api.Command['role_add_member'](u'IT Specialist', user=[u'admin'])
+ api.Command['host_add'](fqdn5, ip_address=ipaddress, no_reverse=True)
+
+ def tearDown(self):
+ self.clean()
+ # TODO: Remove when/if admin has that role by default
+ api.Command['role_remove_member'](u'IT Specialist', user=u'admin')
+
+ @raises(errors.ValidationError)
+ def test_try_add_krbcanonicalname_not_in_krbprincipalname(self):
+ api.Command['host_mod'](fqdn5, addattr='krbcanonicalname=%s' % badcanonicalname)
+
+ @raises(errors.ValidationError)
+ def test_try_remove_principalname_equal_to_krbcanonicalname(self):
+ api.Command['host_mod'](fqdn5, addattr='krbcanonicalname=%s' % canonicalname)
+ api.Command['host_mod'](fqdn5, delattr='krbprincipalname=%s' % principalname1)
--
1.9.3
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
