bugSavannah Static web pages of project:
jra1mdw

bugbugs #29559: -->fetch-crl is not robust enough



Submitted by:  Maarten Litmaath <maart>
Submitted on:  2007-09-13 20:09
 
Status:  Ready for Review Open/Closed:  Closed
Category:  * Security Severity:  * 5 - Major
Baseline Release (where bug has been observed):  * gLite 3.0.2 Release (where bug fix will be available: EMI 1, EMI 2, EMI 3, All):  *
OS:  None Architecture:  None
Bug detection area:  * Production Assigned to:  None
Privacy:  Public Priority:  Medium
Associated Test:  None
GGUS reference URL: https://gus.fzk.de/ws/ticket_info.php?ticket=36191
Component tag(s): 
Subsystem tag(s): 
Discussion Lock:  Unlocked Build environment:  None
Summary:  *fetch-crl is not robust enough
* Mandatory Fields

(Jump to the original submission Jump to the original submission)

2009-07-28 09:00, comment #16:

***
This bug has been automatically closed as it has been inactive in the
'Ready for Review' state for too long. If the issue is still relevant,
please reopen the bug with a comment explaining the situation.
***

Savannah WatchDog <savannahwatchdog>
2009-03-03 12:25, comment #15:

works as explained

Alvaro Fernandez <afernand>



2009-02-02 16:10, comment #14:

Resolved in version 2.7.0 by setting
FORCE_OVERWRITE=yes
in the configuration file (/etc/sysconfig/fetch-crl). The default setting is off, but it's now easy to specify "yes" if you configure the system (with Yaim or so).
The previous option
I_TAKE_FULL_RESPONSIBILITY_FOR_OVERWRITING_ANY_EXISTING_FILE_THAT_HAS_A_CRL_LIKE_FILENAME_BUT_CONTAINS_NON_CRL_DATA=yes_i_really_do
also still works for compatibility reasons.

An upgrade to >2.7.0 with this setting enabled will solve this issue. New versions of the tool can be obtained from
https://dist.eugridpma.info/distrib...

Now: whoever wants to own this bug: please take it :-)

David Groep <groep>
2008-05-07 21:13, comment #13:

Hi Joni, David,
I would rather see the option get a less annoying name first,
before it gets documented. For example:

OVERWRITE_BAD_CRL_FILES=yes

I also note that there should be no risk of accidentally zapping
some other file that happens to have a name like *.r0, since there
would be no corresponding *.crl_url file for it.

BTW, version 2.6.6 has been in use on the CERN AFS UI since
Sep 19, 2007 and has worked fine.

Maarten Litmaath <maart>
2008-05-07 21:02, comment #12:

But should the default be yes_i_really_do, as at least I agree with Graeme that the danger of overwriting .r0 files that would be important non CRL files in the certificates directory seems very academic.

Joni Hahkala <hahkala>
2008-05-07 20:57, comment #11:

The man page of 2.6.6 didn't seem to mention the magic option, nor did the README, should it be documented?

Otherwise we could push for a patch for it.

Joni Hahkala <hahkala>
2008-05-07 20:37, comment #10:

Well, his problem was caused by a full file system...

Maarten Litmaath <maart>
2008-05-07 20:35, comment #9:

Hallo David,
Graeme Stewart now also was hit by the problem:

https://gus.fzk.de/ws/ticket_info.p...

A new rpm would be welcome.

Maarten Litmaath <maart>
2007-09-17 10:48, comment #8:

Hi David,
will you submit a patch with the new rpm, or should I do that?

Maarten Litmaath <maart>
2007-09-16 15:08, comment #7:

It works, but you may want to improve the indentation of the added line... :-)

Maarten Litmaath <maart>
2007-09-16 12:56, comment #6:

Verison 2.6.6 is now available at

https://dist.eugridpma.info/distrib...

with this patch.

Enjoy!
DavidG.

David Groep <groep>
2007-09-15 23:27, comment #5:

Hi David,
version 2.6.5 is unusable due to a syntax error in the script:

------------------------------------------------------------------------
/usr/sbin/fetch-crl: line 675: syntax error near unexpected token `else'
/usr/sbin/fetch-crl: line 675: ` else '
------------------------------------------------------------------------

It worked fine after I applied this patch:

------------------------------------------------------------------------
--- fetch-crl-265 2007-09-16 01:04:32.000000000 +0200
+++ fetch-crl 2007-09-16 01:14:35.000000000 +0200
@@ -91,7 +91,7 @@
FETCH_CRL_SYSCONFIG="${FETCH_CRL_SYSCONFIG:-/etc/sysconfig/fetch-crl}"

# specific work-around for incidental filesystem corruption
-# (please enable only in case you have broken wardware or a broken
+# (please enable only in case you have broken hardware or a broken
# filesystem implementation. In that case, set this variable to
# the value "yes_i_really_do") ...
I_TAKE_FULL_RESPONSIBILITY_FOR_OVERWRITING_ANY_EXISTING_FILE_THAT_HAS_A_CRL_LIKE_FILENAME_BUT_CONTAINS_NON_CRL_DATA=no
@@ -672,6 +672,7 @@
${mv} "${outputDirectory}/${finalCrlFileName}" "${savefile}"
PrintMessage "As you specified the old file will be replaced"
PrintMessage "Previous data contents saved in $savefile"
+ fi
else
PrintMessage "Ignoring this CRL download for ${issuer}"
continue
------------------------------------------------------------------------

Looking forward to version 2.6.6.

Maarten Litmaath <maart>
2007-09-14 14:08, comment #4:

Hoi Maarten,

OK, I've added the magic option. I'm running out of time now, so could you please test the new version 2.6.5 that's now available
at:

https://dist.eugridpma.info/distrib...

pick either the tar.gz one or the RPM.
Oh ja, en zet dus:
I_TAKE_FULL_RESPONSIBILITY_FOR_OVERWRITING_ANY_EXISTING_FILE_THAT_HAS_A_CRL_LIKE_FILENAME_BUT_CONTAINS_NON_CRL_DATA=yes_i_really_do

in /etc/sysconfig/fetch-crl, of waar je dan ook FETCH_CRL_SYSCONFIG naartoe hebt laten wijzen :-)

Succes ermee!

Grtn,
DavidG.

David Groep <groep>
2007-09-14 13:39, comment #3:

Hi David,
we have an unworkable situation on the AFS UI: a problem with a
single file may cause all CRLs to become out of date!

At the very least the "exit 1" should be replaced with "continue".
Please comment.

The bug in srmcp/srmls/... is that they require all CRLs to be
parsable, instead of just the one or two that are needed.

We may have to resort to a customized version of fetch-crl...

Maarten Litmaath <maart>
2007-09-14 11:00, comment #2:

Actually I disagree, on both issues... CRL is robust and secure in its current way of doing things.
It is not the job of fetch-crl to do away with basic sanity checking because of bugs in the underlying filesystem or hardware.

fetch-crl checks the integrity of the existing CRL with a reason: if you put a file there which by accident is named just like a CRL, I don't want a process to overwrite such a file, or move it away, and break other things. If you find that, the setup is seriously broken, and fetch-crl should complain bitterly (it does, with PrintError), and ignore it.
Having a look at the error messages of fetch-crl is a worthwhile effort, and catches these errors immediately.
Note that this does not affect the download of other CRLs.

And on the use of CRLs by crmcp/srmls: their current behaviour is the CORRECT one. If you find a file that should be the CRL but is not, you should treat it as an invalid CA, and not trust anything coming from that CA. That's the only safe default -- or errors would go unnoticed.

I think it is good and proper baheviour if people see errors when their underlying filesystem or hardware is corrupt.


If you really, really, insist, I could comment out the check in a version that will only be distributed to those interested in it, by making a configuration option like

I_TAKE_FULL_RESPONSIBILITY_FOR_OVERWRITING_ANY_EXISTING_FILE_THAT_HAS_A_CRL_LIKE_FILENAME_BUT_CONTAINS_NON_CRL_DATA=yes_i_really_do

in /etc/sysconfig/fetch-crl...

David Groep <groep>
2007-09-14 10:15, comment #1:

Hi,

Can you also put the version of the fetch-crl in?

Joni Hahkala <hahkala>
2007-09-13 20:09, original submission:

The fetch-crl script contains this block:

-------------------------------------------------------------------
if [ x"${crlHashFileIsValid}" != x"yes" ]; then
PrintError "Attempt to overwrite" \
${outputDirectory}/${finalCrlFileName} \
"failed since the original is not a valid CRL file"
exit 1
fi
-------------------------------------------------------------------

On the CERN AFS UI we have observed that a CRL file may become
corrupted after it was successfully installed; this probably is
due to some issue with AFS or its underlying hardware, which may
be hard to track down.

In each case we had to discover that most of the CRLs stopped
getting updated after the incident. This is not acceptable.

Furthermore, the bad file was left in place, thereby causing
utilities like "srmcp" and "srmls" to fail, since they insist
that all CRLs are parsable (which is a bug in those utilities).

The upshot is that fetch-crl must not require the original file
be in good shape. Instead, it should log any problems it sees
and rename any bad file with a time stamp suffix.

Maarten Litmaath <maart>

 

No files currently attached

 

Depends on the following items: None found

Carbon-Copy List
  • Savannah WatchDog <savannahwatchdog> added by (savannahwatchdog) (Posted a comment)
  • DeleteMaria Alandes Pradillo <malandes> added by malandes (Updated the item)
  • Delete added by (Alvaro Fernandez <afernand>) added by afernand (Posted a comment)
  • Jan Just Keijser <janjust> added by (janjust) (Updated the item)
  • helpdesk@ggus.org added by (ype) (automatically added by cronjob)
  • Graeme Stewart <graemestewart> added by (graemestewart)
  • DeleteStephen Burke <sburke> added by sburke
  • Delete added by (David Groep <groep>) added by groep (Posted a comment)
  • Joni Hahkala <hahkala> added by (hahkala) (Posted a comment)
  • Maarten Litmaath <maart> added by (maart) (Submitted the item)
  •  

    Follow 17 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    2009-07-28 09:00savannahwatchdogOpen/ClosedOpen=>Closed
      Closed on2009-07-28 09:00=>2009-07-28 09:00
    2009-06-25 08:38malandesStatusFix Certified=>Ready for Review
      Assigned toegeetest=>None
    2009-03-03 12:25afernandStatusReady for Test=>Fix Certified
    2009-02-10 11:01janjustStatusIntegration Candidate=>Ready for Test
      Assigned to-Automatic update due to transitions settings-=>egeetest
    2009-02-10 11:00janjustStatusIn progress=>Integration Candidate
    2009-02-09 16:11groepAssigned toNone=>janjust
    2009-02-02 16:10groepAssigned togroep=>None
    2008-05-07 20:42graemestewartCarbon-Copy-=>Added graemestewart
    2008-05-07 20:35maartGGUS reference URL=>https://gus.fzk.de/ws/ticket_info.p...
    2007-09-19 16:37sburkeCarbon-Copy-=>Added sburke
    2007-09-14 14:09groepStatusAccepted=>In progress
    2007-09-14 14:08groepStatusNone=>Accepted
    2007-09-14 10:15hahkalaAssigned tohahkala=>groep
    2007-09-13 20:09maartAssigned to-Automatic update due to transitions settings-=>hahkala
    Show feedback again

    Back to the top