MODSIGN: Fix 32-bit overflow in X.509 certificate validity date checking
authorDavid Howells <dhowells@redhat.com>
Tue, 2 Oct 2012 13:36:16 +0000 (14:36 +0100)
committerRusty Russell <rusty@rustcorp.com.au>
Wed, 10 Oct 2012 09:36:37 +0000 (20:06 +1030)
The current choice of lifetime for the autogenerated X.509 of 100 years,
putting the validTo date in 2112, causes problems on 32-bit systems where a
32-bit time_t wraps in 2106.  64-bit x86_64 systems seem to be unaffected.

This can result in something like:

Loading module verification certificates
X.509: Cert 6e03943da0f3b015ba6ed7f5e0cac4fe48680994 has expired
MODSIGN: Problem loading in-kernel X.509 certificate (-127)

Or:

X.509: Cert 6e03943da0f3b015ba6ed7f5e0cac4fe48680994 is not yet valid
MODSIGN: Problem loading in-kernel X.509 certificate (-129)

Instead of turning the dates into time_t values and comparing, turn the system
clock and the ASN.1 dates into tm structs and compare those piecemeal instead.

Reported-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Josh Boyer <jwboyer@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
crypto/asymmetric_keys/x509_cert_parser.c
crypto/asymmetric_keys/x509_parser.h
crypto/asymmetric_keys/x509_public_key.c

index 8fcac94..db07e8c 100644 (file)
@@ -434,11 +434,10 @@ int x509_process_extension(void *context, size_t hdrlen,
 /*
  * Record a certificate time.
  */
-static int x509_note_time(time_t *_time,  size_t hdrlen,
+static int x509_note_time(struct tm *tm,  size_t hdrlen,
                          unsigned char tag,
                          const unsigned char *value, size_t vlen)
 {
-       unsigned YY, MM, DD, hh, mm, ss;
        const unsigned char *p = value;
 
 #define dec2bin(X) ((X) - '0')
@@ -448,30 +447,30 @@ static int x509_note_time(time_t *_time,  size_t hdrlen,
                /* UTCTime: YYMMDDHHMMSSZ */
                if (vlen != 13)
                        goto unsupported_time;
-               YY = DD2bin(p);
-               if (YY > 50)
-                       YY += 1900;
+               tm->tm_year = DD2bin(p);
+               if (tm->tm_year >= 50)
+                       tm->tm_year += 1900;
                else
-                       YY += 2000;
+                       tm->tm_year += 2000;
        } else if (tag == ASN1_GENTIM) {
                /* GenTime: YYYYMMDDHHMMSSZ */
                if (vlen != 15)
                        goto unsupported_time;
-               YY = DD2bin(p) * 100 + DD2bin(p);
+               tm->tm_year = DD2bin(p) * 100 + DD2bin(p);
        } else {
                goto unsupported_time;
        }
 
-       MM = DD2bin(p);
-       DD = DD2bin(p);
-       hh = DD2bin(p);
-       mm = DD2bin(p);
-       ss = DD2bin(p);
+       tm->tm_year -= 1900;
+       tm->tm_mon  = DD2bin(p) - 1;
+       tm->tm_mday = DD2bin(p);
+       tm->tm_hour = DD2bin(p);
+       tm->tm_min  = DD2bin(p);
+       tm->tm_sec  = DD2bin(p);
 
        if (*p != 'Z')
                goto unsupported_time;
 
-       *_time = mktime(YY, MM, DD, hh, mm, ss);
        return 0;
 
 unsupported_time:
index 635053f..f86dc5f 100644 (file)
@@ -18,8 +18,8 @@ struct x509_certificate {
        char            *subject;               /* Name of certificate subject */
        char            *fingerprint;           /* Key fingerprint as hex */
        char            *authority;             /* Authority key fingerprint as hex */
-       time_t          valid_from;
-       time_t          valid_to;
+       struct tm       valid_from;
+       struct tm       valid_to;
        enum pkey_algo  pkey_algo : 8;          /* Public key algorithm */
        enum pkey_algo  sig_pkey_algo : 8;      /* Signature public key algorithm */
        enum pkey_hash_algo sig_hash_algo : 8;  /* Signature hash algorithm */
index 716917c..5ab736d 100644 (file)
@@ -106,7 +106,7 @@ error_no_sig:
 static int x509_key_preparse(struct key_preparsed_payload *prep)
 {
        struct x509_certificate *cert;
-       time_t now;
+       struct tm now;
        size_t srlen, sulen;
        char *desc = NULL;
        int ret;
@@ -118,7 +118,14 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
        pr_devel("Cert Issuer: %s\n", cert->issuer);
        pr_devel("Cert Subject: %s\n", cert->subject);
        pr_devel("Cert Key Algo: %s\n", pkey_algo[cert->pkey_algo]);
-       pr_devel("Cert Valid: %lu - %lu\n", cert->valid_from, cert->valid_to);
+       printk("Cert Valid From: %04ld-%02d-%02d %02d:%02d:%02d\n",
+                cert->valid_from.tm_year + 1900, cert->valid_from.tm_mon + 1,
+                cert->valid_from.tm_mday, cert->valid_from.tm_hour,
+                cert->valid_from.tm_min,  cert->valid_from.tm_sec);
+       printk("Cert Valid To: %04ld-%02d-%02d %02d:%02d:%02d\n",
+                cert->valid_to.tm_year + 1900, cert->valid_to.tm_mon + 1,
+                cert->valid_to.tm_mday, cert->valid_to.tm_hour,
+                cert->valid_to.tm_min,  cert->valid_to.tm_sec);
        pr_devel("Cert Signature: %s + %s\n",
                 pkey_algo[cert->sig_pkey_algo],
                 pkey_hash_algo[cert->sig_hash_algo]);
@@ -130,13 +137,38 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
                goto error_free_cert;
        }
 
-       now = CURRENT_TIME.tv_sec;
-       if (now < cert->valid_from) {
+       time_to_tm(CURRENT_TIME.tv_sec, 0, &now);
+       printk("Now: %04ld-%02d-%02d %02d:%02d:%02d\n",
+                now.tm_year + 1900, now.tm_mon + 1, now.tm_mday,
+                now.tm_hour, now.tm_min,  now.tm_sec);
+       if (now.tm_year < cert->valid_from.tm_year ||
+           (now.tm_year == cert->valid_from.tm_year &&
+            (now.tm_mon < cert->valid_from.tm_mon ||
+             (now.tm_mon == cert->valid_from.tm_mon &&
+              (now.tm_mday < cert->valid_from.tm_mday ||
+               (now.tm_mday == cert->valid_from.tm_mday &&
+                (now.tm_hour < cert->valid_from.tm_hour ||
+                 (now.tm_hour == cert->valid_from.tm_hour &&
+                  (now.tm_min < cert->valid_from.tm_min ||
+                   (now.tm_min == cert->valid_from.tm_min &&
+                    (now.tm_sec < cert->valid_from.tm_sec
+                     ))))))))))) {
                pr_warn("Cert %s is not yet valid\n", cert->fingerprint);
                ret = -EKEYREJECTED;
                goto error_free_cert;
        }
-       if (now >= cert->valid_to) {
+       if (now.tm_year > cert->valid_to.tm_year ||
+           (now.tm_year == cert->valid_to.tm_year &&
+            (now.tm_mon > cert->valid_to.tm_mon ||
+             (now.tm_mon == cert->valid_to.tm_mon &&
+              (now.tm_mday > cert->valid_to.tm_mday ||
+               (now.tm_mday == cert->valid_to.tm_mday &&
+                (now.tm_hour > cert->valid_to.tm_hour ||
+                 (now.tm_hour == cert->valid_to.tm_hour &&
+                  (now.tm_min > cert->valid_to.tm_min ||
+                   (now.tm_min == cert->valid_to.tm_min &&
+                    (now.tm_sec > cert->valid_to.tm_sec
+                     ))))))))))) {
                pr_warn("Cert %s has expired\n", cert->fingerprint);
                ret = -EKEYEXPIRED;
                goto error_free_cert;