From 43f4ca710bfcf2e4076f95b70e8cfc292becec7f Mon Sep 17 00:00:00 2001 From: Martin Kosek Date: Tue, 2 Oct 2012 17:06:10 +0200 Subject: [PATCH] Only use service PAC type as an override PAC type (ipakrbauthzdata attribute) was being filled for all new service automatically. However, the PAC type attribute was designed to serve only as an override to default PAC type configured in IPA config. With PAC type set in all services, users would have to update all services to get new PAC types configured in IPA config. Do not set PAC type for new services. Add new NONE value meaning that we do not want any PAC for the service (empty/missing attribute means that the default PAC type list from IPA config is read). https://fedorahosted.org/freeipa/ticket/2184 --- API.txt | 6 +-- VERSION | 2 +- ipalib/plugins/config.py | 4 +- ipalib/plugins/service.py | 37 +++++++++++++----- tests/test_xmlrpc/test_host_plugin.py | 1 - tests/test_xmlrpc/test_service_plugin.py | 48 ++++++++++++++++++------ 6 files changed, 70 insertions(+), 28 deletions(-) diff --git a/API.txt b/API.txt index 1906e22fe..7bd046c8d 100644 --- a/API.txt +++ b/API.txt @@ -2738,7 +2738,7 @@ command: service_add args: 1,8,3 arg: Str('krbprincipalname', attribute=True, cli_name='principal', multivalue=False, primary_key=True, required=True) option: Bytes('usercertificate', attribute=True, cli_name='certificate', multivalue=False, required=False) -option: StrEnum('ipakrbauthzdata', attribute=True, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD')) +option: StrEnum('ipakrbauthzdata', attribute=True, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD', u'NONE')) option: Str('setattr*', cli_name='setattr', exclude='webui') option: Str('addattr*', cli_name='addattr', exclude='webui') option: Flag('force', autofill=True, default=False) @@ -2775,7 +2775,7 @@ command: service_find args: 1,10,4 arg: Str('criteria?', noextrawhitespace=False) option: Str('krbprincipalname', attribute=True, autofill=False, cli_name='principal', multivalue=False, primary_key=True, query=True, required=False) -option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, query=True, required=False, values=(u'MS-PAC', u'PAD')) +option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, query=True, required=False, values=(u'MS-PAC', u'PAD', u'NONE')) option: Int('timelimit?', autofill=False, minvalue=0) option: Int('sizelimit?', autofill=False, minvalue=0) option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') @@ -2792,7 +2792,7 @@ command: service_mod args: 1,9,3 arg: Str('krbprincipalname', attribute=True, cli_name='principal', multivalue=False, primary_key=True, query=True, required=True) option: Bytes('usercertificate', attribute=True, autofill=False, cli_name='certificate', multivalue=False, required=False) -option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD')) +option: StrEnum('ipakrbauthzdata', attribute=True, autofill=False, cli_name='pac_type', csv=True, multivalue=True, required=False, values=(u'MS-PAC', u'PAD', u'NONE')) option: Str('setattr*', cli_name='setattr', exclude='webui') option: Str('addattr*', cli_name='addattr', exclude='webui') option: Str('delattr*', cli_name='delattr', exclude='webui') diff --git a/VERSION b/VERSION index 962d476e7..c1f1bceff 100644 --- a/VERSION +++ b/VERSION @@ -79,4 +79,4 @@ IPA_DATA_VERSION=20100614120000 # # ######################################################## IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=43 +IPA_API_VERSION_MINOR=44 diff --git a/ipalib/plugins/config.py b/ipalib/plugins/config.py index 1c62e0d94..5f916903b 100644 --- a/ipalib/plugins/config.py +++ b/ipalib/plugins/config.py @@ -192,8 +192,8 @@ class config(LDAPObject): ), StrEnum('ipakrbauthzdata*', cli_name='pac_type', - label=_('PAC type'), - doc=_('Default types of PAC for new services'), + label=_('Default PAC types'), + doc=_('Default types of PAC supported for services'), values=(u'MS-PAC', u'PAD'), csv=True, ), diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py index a7201f525..120eb6076 100644 --- a/ipalib/plugins/service.py +++ b/ipalib/plugins/service.py @@ -60,8 +60,11 @@ EXAMPLES: ipa service-add HTTP/web.example.com Allow a host to manage an IPA service certificate: - ipa service-add-host --hosts=web.example.com HTTP/web.example.com - ipa role-add-member --hosts=web.example.com certadmin + ipa service-add-host --hosts=web.example.com HTTP/web.example.com + ipa role-add-member --hosts=web.example.com certadmin + + Override a default list of supported PAC types for the service: + ipa service-mod HTTP/web.example.com --pac-type=MS-PAC Delete an IPA service: ipa service-del HTTP/web.example.com @@ -253,12 +256,28 @@ class service(LDAPObject): StrEnum('ipakrbauthzdata*', cli_name='pac_type', label=_('PAC type'), - doc=_('Types of PAC this service supports'), - values=(u'MS-PAC', u'PAD'), + doc=_("Override default list of supported PAC types." + " Use 'NONE' to disable PAC support for this service"), + values=(u'MS-PAC', u'PAD', u'NONE'), csv=True, ), ) + def validate_ipakrbauthzdata(self, entry): + new_value = entry.get('ipakrbauthzdata', []) + + if not new_value: + return + + if not isinstance(new_value, (list, tuple)): + new_value = set([new_value]) + else: + new_value = set(new_value) + + if u'NONE' in new_value and len(new_value) > 1: + raise errors.ValidationError(name='ipakrbauthzdata', + error=_('NONE value cannot be combined with other PAC types')) + api.register(service) @@ -287,6 +306,8 @@ class service_add(LDAPCreate): reason=_("The host '%s' does not exist to add a service to.") % hostname) + self.obj.validate_ipakrbauthzdata(entry_attrs) + cert = options.get('usercertificate') if cert: dercert = x509.normalize_certificate(cert) @@ -300,11 +321,6 @@ class service_add(LDAPCreate): util.validate_host_dns(self.log, hostname) if not 'managedby' in entry_attrs: entry_attrs['managedby'] = hostresult['dn'] - if 'ipakrbauthzdata' not in entry_attrs: - config = ldap.get_ipa_config()[1] - default_pac_type = config.get('ipakrbauthzdata', []) - if default_pac_type: - entry_attrs['ipakrbauthzdata'] = default_pac_type # Enforce ipaKrbPrincipalAlias to aid case-insensitive searches # as krbPrincipalName/krbCanonicalName are case-sensitive in Kerberos @@ -372,6 +388,9 @@ class service_mod(LDAPUpdate): def pre_callback(self, ldap, dn, entry_attrs, *keys, **options): assert isinstance(dn, DN) + + self.obj.validate_ipakrbauthzdata(entry_attrs) + if 'usercertificate' in options: (service, hostname, realm) = split_principal(keys[-1]) cert = options.get('usercertificate') diff --git a/tests/test_xmlrpc/test_host_plugin.py b/tests/test_xmlrpc/test_host_plugin.py index 2010af8a3..37b7e407d 100644 --- a/tests/test_xmlrpc/test_host_plugin.py +++ b/tests/test_xmlrpc/test_host_plugin.py @@ -654,7 +654,6 @@ class test_host(Declarative): krbprincipalname=[service1], objectclass=objectclasses.service, managedby_host=[fqdn1], - ipakrbauthzdata=[u'MS-PAC'], ipauniqueid=[fuzzy_uuid], ), ), diff --git a/tests/test_xmlrpc/test_service_plugin.py b/tests/test_xmlrpc/test_service_plugin.py index a76bc9184..29c94e310 100644 --- a/tests/test_xmlrpc/test_service_plugin.py +++ b/tests/test_xmlrpc/test_service_plugin.py @@ -181,7 +181,6 @@ class test_service(Declarative): krbprincipalname=[service1], objectclass=objectclasses.service, ipauniqueid=[fuzzy_uuid], - ipakrbauthzdata=[u'MS-PAC'], managedby_host=[fqdn1], ), ), @@ -210,7 +209,6 @@ class test_service(Declarative): dn=service1dn, krbprincipalname=[service1], has_keytab=False, - ipakrbauthzdata=[u'MS-PAC'], managedby_host=[fqdn1], ), ), @@ -230,7 +228,6 @@ class test_service(Declarative): objectclass=objectclasses.service, ipauniqueid=[fuzzy_uuid], managedby_host=[fqdn1], - ipakrbauthzdata=[u'MS-PAC'], has_keytab=False ), ), @@ -249,7 +246,6 @@ class test_service(Declarative): dn=service1dn, krbprincipalname=[service1], managedby_host=[fqdn1], - ipakrbauthzdata=[u'MS-PAC'], has_keytab=False, ), ], @@ -271,7 +267,6 @@ class test_service(Declarative): ipakrbprincipalalias=[service1], objectclass=objectclasses.service, ipauniqueid=[fuzzy_uuid], - ipakrbauthzdata=[u'MS-PAC'], has_keytab=False, managedby_host=[fqdn1], ), @@ -289,7 +284,6 @@ class test_service(Declarative): result=dict( dn=service1dn, krbprincipalname=[service1], - ipakrbauthzdata=[u'MS-PAC'], managedby_host=[fqdn1], ), ), @@ -305,7 +299,6 @@ class test_service(Declarative): result=dict( dn=service1dn, krbprincipalname=[service1], - ipakrbauthzdata=[u'MS-PAC'], managedby_host=[fqdn1], ), ), @@ -321,7 +314,6 @@ class test_service(Declarative): result=dict( dn=service1dn, krbprincipalname=[service1], - ipakrbauthzdata=[u'MS-PAC'], managedby_host=[fqdn1, fqdn2], ), ), @@ -337,7 +329,6 @@ class test_service(Declarative): result=dict( dn=service1dn, krbprincipalname=[service1], - ipakrbauthzdata=[u'MS-PAC'], managedby_host=[fqdn1], ), ), @@ -353,7 +344,6 @@ class test_service(Declarative): result=dict( dn=service1dn, krbprincipalname=[service1], - ipakrbauthzdata=[u'MS-PAC'], managedby_host=[fqdn1, fqdn3.lower()], ), ), @@ -369,7 +359,6 @@ class test_service(Declarative): result=dict( dn=service1dn, krbprincipalname=[service1], - ipakrbauthzdata=[u'MS-PAC'], managedby_host=[fqdn1], ), ), @@ -394,7 +383,6 @@ class test_service(Declarative): result=dict( usercertificate=[base64.b64decode(servercert)], krbprincipalname=[service1], - ipakrbauthzdata=[u'MS-PAC'], managedby_host=[fqdn1], valid_not_before=fuzzy_date, valid_not_after=fuzzy_date, @@ -409,6 +397,42 @@ class test_service(Declarative): ), + dict( + desc='Try to update %r with invalid ipakrbauthz data ' + 'combination' % service1, + command=('service_mod', [service1], + dict(ipakrbauthzdata=[u'MS-PAC', u'NONE'])), + expected=errors.ValidationError(name='ipakrbauthzdata', + error=u'NONE value cannot be combined with other PAC types') + ), + + + dict( + desc='Update %r with valid ipakrbauthz data ' + 'combination' % service1, + command=('service_mod', [service1], + dict(ipakrbauthzdata=[u'MS-PAC'])), + expected=dict( + value=service1, + summary=u'Modified service "%s"' % service1, + result=dict( + usercertificate=[base64.b64decode(servercert)], + krbprincipalname=[service1], + managedby_host=[fqdn1], + ipakrbauthzdata=[u'MS-PAC'], + valid_not_before=fuzzy_date, + valid_not_after=fuzzy_date, + subject=DN(('CN',api.env.host),x509.subject_base()), + serial_number=fuzzy_digits, + serial_number_hex=fuzzy_hex, + md5_fingerprint=fuzzy_hash, + sha1_fingerprint=fuzzy_hash, + issuer=fuzzy_issuer, + ), + ), + ), + + dict( desc='Retrieve %r to verify update' % service1, command=('service_show', [service1], {}),