[Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile
Jan Cholasta
jcholast at redhat.com
Wed Jun 11 12:59:14 UTC 2014
On 11.6.2014 13:29, Martin Kosek wrote:
> 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.
Sorry, last minute change gone wrong.
>
> 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.
These are SAN types, not extension types. Unfortunately the textual
descriptions are the only SAN type representation available in
python-nss which includes OIDs of other names.
>
> Can we for example modify get_subjectaltname to get the type based on
> CertificateExtension object's oid or oid_tag attributes?
No, see above.
>
> 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.
Done.
>
> Martin
>
Updated patches attached.
--
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-210.6-Allow-SAN-in-IPA-certificate-profile.patch
Type: text/x-patch
Size: 5003 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140611/219e6323/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-234.6-Support-requests-with-SAN-in-cert-request.patch
Type: text/x-patch
Size: 14427 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140611/219e6323/attachment-0001.bin>
More information about the Freeipa-devel
mailing list