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

[Rob Crittenden]

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?

ACK

rob