[Freeipa-devel] [PATCH] 321 Move CRL publish directory to IPA owned directory

[Martin Kosek]

On 10/09/2012 03:48 PM, Rob Crittenden wrote:
> Martin Kosek wrote:
>> On 10/08/2012 09:29 PM, Rob Crittenden wrote:
>>> Martin Kosek wrote:
>>>> ----- Original Message -----
>>>>> From: "Rob Crittenden" <rcritten at redhat.com>
>>>>> To: "Martin Kosek" <mkosek at redhat.com>
>>>>> Cc: freeipa-devel at redhat.com
>>>>> Sent: Monday, October 8, 2012 8:18:47 PM
>>>>> Subject: Re: [Freeipa-devel] [PATCH] 321 Move CRL publish directory to IPA
>>>>> owned directory
>>>>>
>>>>> Martin Kosek wrote:
>>>>>> Currently, CRL files are being exported to /var/lib/pki-ca
>>>>>> sub-directory, which is then served by httpd to clients. However,
>>>>>> this approach has several disadvantages:
>>>>>>     * We depend on pki-ca directory structure and relevant
>>>>>>     permissions.
>>>>>>       If pki-ca changes directory structure or permissions on
>>>>>>       upgrade,
>>>>>>       IPA may break. This is also a root cause of the latest error,
>>>>>>       where
>>>>>>       the pki-ca directory does not have X permission for others and
>>>>>>       CRL
>>>>>>       publishing by httpd breaks.
>>>>>>     * Since the directory is not static and is generated during
>>>>>>       ipa-server-install, RPM upgrade of IPA packages report errors
>>>>>>       when
>>>>>>       defining SELinux policy for these directories.
>>>>>>
>>>>>> Move CRL publish directory to /var/lib/ipa/pki-ca/publish (common
>>>>>> for
>>>>>> both dogtag 9 and 10) which is created on RPM upgrade, i.e. SELinux
>>>>>> policy
>>>>>> configuration does not report any error. The new CRL publish
>>>>>> directory
>>>>>> is used for both new IPA installs and upgrades, where contents of
>>>>>> the directory (CRLs) is first migrated to the new location and then
>>>>>> the
>>>>>> actual configuration change is made.
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/3144
>>>>>>
>>>>>>
>>>>>> -------
>>>>>>
>>>>>> We may choose to postpone this patch to later version, it is quite
>>>>>> disruptive.
>>>>>> I can produce a hotfix in that case, which would only fix the
>>>>>> permission of the
>>>>>> pki-ca directory.
>>>>>>
>>>>>> Martin
>>>>>
>>>>> This looks good, just a couple of questions.
>>>>>
>>>>> Should the old files be removed?
>>>>
>>>> Yeah, this is something I wonder about too. One one side, users may be get
>>>> confused if there are 2 publish directories with the same content (a least
>>>> until a next CRL generation). But on the other side, I was cautious not to
>>>> delete something important in a case when something goes wrong. But maybe I
>>>> am just being over-cautious and we can delete it, or maybe move directory to
>>>> "publish.deleted". In a worst case scenario, CRLs should be generated again,
>>>> in about 4 hours...
>>>>
>>>>>
>>>>> Should some error handling be added around the copy to ensure it is
>>>>> successful? This would blow up if the disk was full, for example.
>>>>
>>>> I think this is covered, I have there a try..except clause for this case:
>>>>
>>>> +        for f in crl_files:
>>>> +            try:
>>>> +                copy_crl_file(f)
>>>> +            except Exception, e:
>>>> +                root_logger.error('Cannot move CRL file to new directory:
>>>> %s', e)
>>>>
>>>> Just the exception is caught on a higher level and error is reported to user.
>>>> Not sure what to do in this case - report error to user, do not change the
>>>> location and ask user to resolve the error? Or just report error and continue
>>>> with CRL publish directory change as I do currently?
>>>>
>>>> Martin
>>>>
>>>
>>> Ah, I was looking at too low a level.
>>>
>>> I think this is fine. I wonder if we should log/document somewhere in big
>>> flashing letters that the stuff has been moved. It is rather clear in the logs
>>> now, I suppose, if you look carefully.
>>>
>>> I guess it isn't really that big a deal now since we just have server certs. It
>>> would take a whole ton of revocations to grow the CRLs that quickly, so maybe
>>> my concerns are overblown.
>>>
>>> rob
>>
>> CRL size may quickly grown for someone who rapidly adds/removes client hosts or
>> services as we revoke such certificates with 4 - superseded.. About the user
>> info - maybe we should also allow output of ipa-upgradeconfig's stderr stream
>> as we did with ipa-ldap-updater. This way, user would receive error message
>> when CRL copy operation fails.
>>
>> Martin
>>
> 
> I think the patch is fine as-is. Can you open a new ticket on a mechanism to
> alert the user to upgrade things to review?

https://fedorahosted.org/freeipa/ticket/3157

> 
> ACK
> 
> rob
> 

Rebased and pushed to master, ipa-3-0.

Martin