[Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
Martin Kosek
mkosek at redhat.com
Wed Jun 11 11:29:26 UTC 2014
On 06/11/2014 10:58 AM, Jan Cholasta wrote:
> On 10.6.2014 09:55, Martin Kosek wrote:
>> On 06/06/2014 12:50 PM, Jan Cholasta wrote:
>>> On 23.1.2014 14:34, Jan Cholasta wrote:
>>>> On 22.1.2014 16:43, Simo Sorce wrote:
>>>>> On Wed, 2014-01-22 at 16:05 +0100, Jan Cholasta wrote:
>>>>>> On 22.1.2014 15:34, Simo Sorce wrote:
>>>>>>> On Wed, 2014-01-22 at 10:40 +0100, Jan Cholasta wrote:
>>>>>>>> On 21.1.2014 17:12, Simo Sorce wrote:
>>>>>>>>> Later in the patch you seem to be changing from needing
>>>>>>>>> managedby_host
>>>>>>>>> to needing write access to an entry, I am not sure I understand
>>>>>>>>> why that
>>>>>>>>> was changed. not saying it is necessarily wrong, but why the
>>>>>>>>> original
>>>>>>>>> check is not right anymore ?
>>>>>>>>
>>>>>>>> The original check is wrong, see
>>>>>>>> <https://fedorahosted.org/freeipa/ticket/3977#comment:23>.
>>>>>>>>
>>>>>>>> The check in my patch allows SAN only if the requesting host has write
>>>>>>>> access to all of the SAN services. I'm not entirely sure if this is
>>>>>>>> right, but even if it is not, I think we should still check for write
>>>>>>>> access to the SAN services, so that access control can be (partially)
>>>>>>>> handled by ACIs.
>>>>>>>
>>>>>>> Right, I remembered that comment, but it just says to check the right
>>>>>>> object's managed-by, here instead you changed it to check if you can
>>>>>>> write the usercertificate.
>>>>>>>
>>>>>>> I guess it is the same *if* there is an ACI that gives write permission
>>>>>>> when the host is in the managed-by attribute, is that the reasoning ?
>>>>>>
>>>>>> Exactly. The ACIs that allow this by default are named "Hosts can manage
>>>>>> service Certificates and kerberos keys" and "Hosts can manage other host
>>>>>> Certificates and kerberos keys".
>>>>>>
>>>>>> I think the check can be extended to users as well, so that requesting
>>>>>> certificate with SAN is allowed only to users which have write access to
>>>>>> the SAN services.
>>>>
>>>> I have done the modification, see attached patches.
>>>>
>>>>>
>>>>> Sounds good to me then, thanks for explaining.
>>>>>
>>>>> The patches also look good, but I would like someone to give them a try
>>>>> for a formal ack.
>>>>
>>>> OK, thanks.
>>>>
>>>
>>> Bump.
>>>
>>> I have added stricter validation of subject alt names as well as certificate
>>> extensions in general to the second patch.
>>
>> Thanks!
>>
>>> Updated patches attached.
>>>
>>> Note that you will need python-nss 0.15 in order to test, you can get a RPM for
>>> Fedora here: <http://koji.fedoraproject.org/koji/buildinfo?buildID=514739>.
>>
>> John, do you think we could build python-nss 0.15 also for Fedora 20? The fix
>> incorporated in it is pretty important to warrant bugfix update in bodhi. It
>> would also allow FreeIPA 4.0 to be installed on Fedora 20.
>>
>>> Also, resubmitting HTTP and LDAP Server-Cert certmonger requests does not work,
>>> because <https://fedorahosted.org/freeipa/ticket/4370>.
>>
>> This worked for me, I suspect this is a DS bug. More info in the ticket.
>>
>> Now back to review of the patch:
>>
>> 210.4: looks ok
>> 234.4:
>>
>> 1) Virtual operation "request certificate with subjectaltname" should be a
>> member of Certificate Administrators privilege
>
> OK.
>
>>
>>
>> 2) I would write "Request Certificate With SubjectAltName" with "with" instead
>> of "With". I.e.:
>> Request Certificate with SubjectAltName
>
> OK.
>
>>
>>
>> 3) Why is the extension check only enforced for non-hosts?
>>
>> + if not bind_principal.startswith('host/'):
>> + for ext in extensions:
>> + operation = self._allowed_extensions.get(ext)
>> + if operation:
>> + self.check_access(operation)
>>
>> My tricky certmonger request passed:
>>
>> # ipa-getcert request -d /etc/pki/nssdb/ -n test-san-1 -I test-san-1 -g 2048 -r
>> -N "cn=`hostname`" -K host/`hostname` -D test1.example.com -D test2.example.com
>> -E mkosek at redhat.com
>>
>> while when I posted the same CSR as privileged user, it was rejected:
>>
>> $ klist
>> Ticket cache: KEYRING:persistent:962000003:962000003
>> Default principal: fbar at IDM.LAB.ENG.BRQ.REDHAT.COM
>>
>> $ ipa cert-request --principal test/`hostname` certmonger.csr
>> ipa: ERROR: invalid 'csr': subject alt name type RFC822 Name is forbidden
>
> Right, I meant to NACK myself, but you were faster. Fixed.
>
>>
>>
>> 4) Regular users cannot read Virtual Operations, so even if I assign them a
>> permission, command is refused:
>>
>> $ ipa cert-request --principal test/`hostname` openssl.csr
>> ipa: ERROR: Insufficient access: No such virtual command
>>
>> I think we will need to create new managed permission "Read Virtual Operations"
>> and assign it to "Certificate Administrators" privilege by default as this
>> privilege has the virtual operation permissions assigned. Petr3, what is your
>> take on this?
>>
>> Otherwise the patch worked well for me, the authorization part looked OK. My
>> biggest concern was just the host/user differentiation wrt unknown
>> subjectaltname types.
>>
>> Martin
>>
>
> Updated patches attached.
>
1) I could not compile the patch set:
$ make rpms
...
if [ "" != "yes" ]; then \
./makeapi --validate; \
./makeaci --validate; \
fi
Traceback (most recent call last):
File "./makeapi", line 457, in <module>
sys.exit(main())
File "./makeapi", line 428, in main
api.finalize()
File "/root/freeipa-master/ipalib/plugable.py", line 708, in finalize
self.__do_if_not_done('load_plugins')
File "/root/freeipa-master/ipalib/plugable.py", line 482, in __do_if_not_done
getattr(self, name)()
File "/root/freeipa-master/ipalib/plugable.py", line 645, in load_plugins
self.import_plugins('ipalib')
File "/root/freeipa-master/ipalib/plugable.py", line 689, in import_plugins
__import__(fullname)
File "/root/freeipa-master/ipalib/plugins/cert.py", line 30, in <module>
from ipalib import pkcs10
File "/root/freeipa-master/ipalib/pkcs10.py", line 77
.replace('@', '\\@')
...
The rest of the notes are thus only from reading.
2) There are lines like
+ if name_type == 'Other Name (OID.1.3.6.1.5.2.2)':
or
+ if name_type not in ('DNS name',
+ 'Other Name (OID.1.3.6.1.4.1.311.20.2.3)',
+ 'Other Name (OID.1.3.6.1.5.2.2)'):
or
+ elif name_type in ('Other Name (OID.1.3.6.1.4.1.311.20.2.3)',
+ 'Other Name (OID.1.3.6.1.5.2.2)'):
Can we do something better? Getting the extension type based on it's
description seems extremely unstable to me.
Can we for example modify get_subjectaltname to get the type based on
CertificateExtension object's oid or oid_tag attributes?
I would rather see get_subjectaltname return solid type like
CERT_EXTENSION_DNS_NAME defined in pkcs10.py than consumers comparing
descriptions. It would be more readable, too.
Martin
More information about the Freeipa-devel
mailing list