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

[Martin Kosek]

On 10/10/2012 11:07 AM, Petr Viktorin wrote:
> On 10/09/2012 04:11 PM, Martin Kosek wrote:
>> 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
>>
> 
> The upgrade is done even on a replica without a CA.
> On such a system, rpm update just silently fails.
> 
> https://fedorahosted.org/freeipa/ticket/3159
> 

Thanks, this is a very good catch. I will send a patch fixing this issue in few
minutes.

Martin