[Freeipa-devel] [PATCH] 210 Allow SAN in IPA certificate profile

[Martin Kosek]

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


2) I would write "Request Certificate With SubjectAltName" with "with" instead
of "With". I.e.:
Request Certificate with SubjectAltName


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


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