[Freeipa-devel] [PATCH] 340 Add OCSP and CRL URIs to certificates

[Rob Crittenden]

Martin Kosek wrote:
> On 12/06/2012 08:35 PM, Rob Crittenden wrote:
>> Martin Kosek wrote:
>>> On 12/06/2012 04:48 PM, Martin Kosek wrote:
>>>> On 12/06/2012 04:45 PM, Martin Kosek wrote:
>>>>> Modify the default IPA CA certificate profile to include CRL and
>>>>> OCSP extensions which will add URIs to IPA CRL&OCSP to published
>>>>> certificates.
>>>>>
>>>>> Both CRL and OCSP extensions have 2 URIs, one pointing directly to
>>>>> the IPA CA which published the certificate and one to a new CNAME
>>>>> ipa-ca.$DOMAIN which was introduced as a general CNAME pointing
>>>>> to all IPA replicas which have CA configured.
>>>>>
>>>>> The new CNAME is added either during new IPA server/replica/CA
>>>>> installation or during upgrade.
>>>>>
>>>>> https://fedorahosted.org/freeipa/ticket/3074
>>>>> https://fedorahosted.org/freeipa/ticket/1431
>>>>>
>>>>> ----
>>>>>
>>>>> This patch originates in Rob's WIP OCSP patch, which I had to rewrite to make
>>>>> things working as we want to :-)
>>>>>
>>>>> Martin
>>>>>
>>>>
>>>> I knew the subject is wrong the moment I clicked the Send button... Sending a
>>>> fixed patch.
>>>>
>>>> Martin
>>>
>>> Found a crash in ipa-replica-install, sending a fixed patch.
>>>
>>> Martin
>>
>> It looks good. I tested with and without DNS, with and without CAs, adding CAs,
>> upgrades, no problems.
>>
>> The only question I have is this effectively makes the CRL optional. If it
>> isn't found configured in the profile it won't add it in. Was that intentional?
>>
>> rob
>>
>
> It certainly wasn't intentional. Did it happen for you? It did not reproduce it
> in my tests as I always tried to request a cert and then checked for CRL record
> in it, but I found a glitch in my patch, pasting an interdiff:
>
> diff -u b/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
> --- b/ipaserver/install/cainstance.py
> +++ b/ipaserver/install/cainstance.py
> @@ -1350,7 +1350,7 @@
>           new_set_list = None
>
>           if setlist == '1,2,3,4,5,6,7,8':
> -            new_set_list = '1,2,3,4,5,6,7,8,10'
> +            new_set_list = '1,2,3,4,5,6,7,8,9'
>           elif setlist == '1,2,3,4,5,6,7,8,10':
>               new_set_list = '1,2,3,4,5,6,7,8,9,10'
>
> This may have caused an issue in your case. I checked a profile we use either
> with dogtag9 and dogtag10, and both have one empty CRL extension template, so
>
> if crl_profile_count == '1':
>
> should pass and CRL should be filled. Following sections should then enable it:
>
> +        if setlist == '1,2,3,4,5,6,7,8':
> +            new_set_list = '1,2,3,4,5,6,7,8,10'
> +        elif setlist == '1,2,3,4,5,6,7,8,10':
> +            new_set_list = '1,2,3,4,5,6,7,8,9,10'
> +
> +        if new_set_list:
> +            installutils.set_directive(self.dogtag_constants.IPA_SERVICE_PROFILE,
> +                'policyset.serverCertSet.list',
> +                new_set_list, quotes=False, separator='=')
>
>
> Updated patch (with the interdiff) attached).
>
> Martin
>

Ok, makes sense.

ACK, pushed to master

rob