Menu

#101 "Revocation" column shows "CRL Expires: <date>" even if no CRL has been generated

v1.0_(example)
closed-fixed
nobody
None
1
2015-03-21
2015-02-19
saperski
No

If you:

1) import a CA certificate (I have used https://github.com/cryptostorm/leakblock/blob/master/superfish.com/rootcrt.crt)

At this point "Revocation" field in this certificate in the "Certificates" tab is empty.

2) import a private key (I have used https://github.com/cryptostorm/leakblock/blob/master/superfish.com/encrypted_key.txt)

At this point "Revocation" field in this certificate says "CRL expires: 2015-02-19" (current date at the time of the writing) and the background is red.

This is done despite not having any CRL imported, created or otherwise available.

This is because in lib/pki_x509.c we have:

831                 case HD_cert_revokation:
832                         if (isRevoked())
833                                 return QVariant(getRevoked().toSortable());
834                         else if (canSign())
835                                 return QVariant(tr("CRL expires: %1").
836                                         arg(crlExpiry.toSortable()));
837                         return QVariant();

and crlExpiry field gets initialized to "today" in the same file:

130 void pki_x509::init()
131 {
132         psigner = NULL;  
133         trust = 0;
134         efftrust = 0;
135         revoked = a1time::now();
136         caSerial = 1;
137         caTemplate = "";   
138         crlDays = 30;   
139         crlExpiry = a1time::now();
140         class_name = "pki_x509";

Since isRevoked() is false and canSign() returns true because we have a private key, a bogus revocation information is shown.

The above code is taken from git commit 70b4f42f3677d25501295bcf4bde068cc4fb2366

1 Attachments

Discussion

  • saperski

    saperski - 2015-02-19

    Attached please find a simple (possibly incomplete) patch to fix the issue.

    From 9762ad901d4d8028cbf6854c74742c4b67d6e272 Mon Sep 17 00:00:00 2001
    From: =?UTF-8?q?Marcin=20Cie=C5=9Blak?= <saper@saper.info>
    Date: Thu, 19 Feb 2015 22:27:43 +0100
    Subject: [PATCH] bug 101: Don't trust pre-initialized crlExpiry
    
    Explicitly set crlExpiry to be undefined
    and do not accept the default value if the certificate
    private key is available.
    
    https://sourceforge.net/p/xca/bugs/101/
    ---
     lib/pki_x509.cpp | 20 ++++++++++++--------
     1 file changed, 12 insertions(+), 8 deletions(-)
    
    diff --git a/lib/pki_x509.cpp b/lib/pki_x509.cpp
    index caa35c8..562bbbf 100644
    --- a/lib/pki_x509.cpp
    +++ b/lib/pki_x509.cpp
    @@ -136,7 +136,8 @@ void pki_x509::init()
        caSerial = 1;
        caTemplate = "";
        crlDays = 30;
    
    -   crlExpiry = a1time::now();
    +   crlExpiry = a1time();
    +   crlExpiry.setUndefined();
        class_name = "pki_x509";
        cert = NULL;
        isrevoked = false;
    @@ -831,7 +832,7 @@ QVariant pki_x509::column_data(dbheader *hd)
            case HD_cert_revokation:
                if (isRevoked())
                    return QVariant(getRevoked().toSortable());
    -           else if (canSign())
    +           else if (canSign() && !crlExpiry.isUndefined())
                    return QVariant(tr("CRL expires: %1").
                        arg(crlExpiry.toSortable()));
                return QVariant();
    @@ -954,11 +955,13 @@ QVariant pki_x509::bg_color(dbheader *hd)
                if (canSign()) {
                    QDateTime crlwarn, crlex;
                    crlex = crlExpiry;
    -               crlwarn = crlex.addSecs(-2 *60*60*24);
    -               if (crlex < now)
    -                   return QVariant(BG_RED);
    -               if (crlwarn < now || !crlex.isValid())
    -                   return QVariant(BG_YELLOW);
    +               if (!crlExpiry.isUndefined()) {
    +                   crlwarn = crlex.addSecs(-2 *60*60*24);
    +                   if (crlex < now)
    +                       return QVariant(BG_RED);
    +                   if (crlwarn < now || !crlex.isValid())
    +                       return QVariant(BG_YELLOW);
    +               }
                }
        }
        return QVariant();
    @@ -988,7 +991,8 @@ void pki_x509::oldFromData(unsigned char *p, int size)
            if (version == 1) {
                caTemplate="";
                caSerial=1;
    -           crlExpiry=a1time::now();
    +           crlExpiry=a1time();
    +           crlExpiry.setUndefined();
                crlDays=30;
            }
    
    -- 
    2.1.0
    
     

    Last edit: saperski 2015-02-19
  • Christian Hohnstaedt

    Patch looks reasonable.
    Will be applied to the next version

     
  • Christian Hohnstaedt

    Fixed by applaing the patch in XCA 1.2.0

     
  • Christian Hohnstaedt

    • status: open --> closed-fixed
     
MongoDB Logo MongoDB