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

[Jan Cholasta]

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.

-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-210.5-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/fa9b9415/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-234.5-Support-requests-with-SAN-in-cert-request.patch
Type: text/x-patch
Size: 14117 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140611/fa9b9415/attachment-0001.bin>