Skip to content

Commit

Permalink
MODSIGN: Fix 32-bit overflow in X.509 certificate validity date checking
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: David Howells <[email protected]>
Acked-by: Josh Boyer <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
  • Loading branch information
dhowells authored and rustyrussell committed Oct 10, 2012
1 parent d5b7193 commit a5752d1
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 20 deletions.
25 changes: 12 additions & 13 deletions crypto/asymmetric_keys/x509_cert_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions crypto/asymmetric_keys/x509_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
42 changes: 37 additions & 5 deletions crypto/asymmetric_keys/x509_public_key.c
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ static int x509_check_signature(const struct public_key *pub,
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;
Expand All @@ -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]);
Expand All @@ -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;
Expand Down

0 comments on commit a5752d1

Please sign in to comment.