Remove the certificate date checks that are performed when a certificate is
parsed. There are two checks: a valid from and a valid to. The first check is
causing a lot of problems with system clocks that don't keep good time and the
second places an implicit expiry date upon the kernel when used for module
signing, so do we really need them?
Signed-off-by: David Howells <[email protected]>
cc: David Woodhouse <[email protected]>
cc: Rusty Russell <[email protected]>
cc: Josh Boyer <[email protected]>
cc: Alexander Holler <[email protected]>
cc: [email protected]
---
crypto/asymmetric_keys/x509.asn1 | 4 +-
crypto/asymmetric_keys/x509_cert_parser.c | 64 -----------------------------
crypto/asymmetric_keys/x509_parser.h | 2 -
crypto/asymmetric_keys/x509_public_key.c | 46 ---------------------
4 files changed, 2 insertions(+), 114 deletions(-)
diff --git a/crypto/asymmetric_keys/x509.asn1 b/crypto/asymmetric_keys/x509.asn1
index bf32b3d..b448099 100644
--- a/crypto/asymmetric_keys/x509.asn1
+++ b/crypto/asymmetric_keys/x509.asn1
@@ -35,8 +35,8 @@ AttributeValueAssertion ::= SEQUENCE {
}
Validity ::= SEQUENCE {
- notBefore Time ({ x509_note_not_before }),
- notAfter Time ({ x509_note_not_after })
+ notBefore Time,
+ notAfter Time
}
Time ::= CHOICE {
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 7fabc4c..3987508 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -430,67 +430,3 @@ int x509_process_extension(void *context, size_t hdrlen,
return 0;
}
-
-/*
- * Record a certificate time.
- */
-static int x509_note_time(struct tm *tm, size_t hdrlen,
- unsigned char tag,
- const unsigned char *value, size_t vlen)
-{
- const unsigned char *p = value;
-
-#define dec2bin(X) ((X) - '0')
-#define DD2bin(P) ({ unsigned x = dec2bin(P[0]) * 10 + dec2bin(P[1]); P += 2; x; })
-
- if (tag == ASN1_UNITIM) {
- /* UTCTime: YYMMDDHHMMSSZ */
- if (vlen != 13)
- goto unsupported_time;
- tm->tm_year = DD2bin(p);
- if (tm->tm_year >= 50)
- tm->tm_year += 1900;
- else
- tm->tm_year += 2000;
- } else if (tag == ASN1_GENTIM) {
- /* GenTime: YYYYMMDDHHMMSSZ */
- if (vlen != 15)
- goto unsupported_time;
- tm->tm_year = DD2bin(p) * 100 + DD2bin(p);
- } else {
- goto unsupported_time;
- }
-
- 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;
-
- return 0;
-
-unsupported_time:
- pr_debug("Got unsupported time [tag %02x]: '%*.*s'\n",
- tag, (int)vlen, (int)vlen, value);
- return -EBADMSG;
-}
-
-int x509_note_not_before(void *context, size_t hdrlen,
- unsigned char tag,
- const void *value, size_t vlen)
-{
- struct x509_parse_context *ctx = context;
- return x509_note_time(&ctx->cert->valid_from, hdrlen, tag, value, vlen);
-}
-
-int x509_note_not_after(void *context, size_t hdrlen,
- unsigned char tag,
- const void *value, size_t vlen)
-{
- struct x509_parse_context *ctx = context;
- return x509_note_time(&ctx->cert->valid_to, hdrlen, tag, value, vlen);
-}
diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index f86dc5f..8ee6f8e 100644
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -18,8 +18,6 @@ struct x509_certificate {
char *subject; /* Name of certificate subject */
char *fingerprint; /* Key fingerprint as hex */
char *authority; /* Authority key fingerprint as hex */
- 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 */
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 06007f0..0d1fcfb 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -106,7 +106,6 @@ error_no_sig:
static int x509_key_preparse(struct key_preparsed_payload *prep)
{
struct x509_certificate *cert;
- struct tm now;
size_t srlen, sulen;
char *desc = NULL;
int ret;
@@ -118,14 +117,6 @@ 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 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);
- pr_devel("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]);
@@ -137,43 +128,6 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
goto error_free_cert;
}
- time_to_tm(CURRENT_TIME.tv_sec, 0, &now);
- pr_devel("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.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;
- }
-
cert->pub->algo = x509_public_key_algorithms[cert->pkey_algo];
cert->pub->id_type = PKEY_ID_X509;
On Thu, 2013-03-14 at 12:34 +0000, David Howells wrote:
> Remove the certificate date checks that are performed when a certificate is
> parsed. There are two checks: a valid from and a valid to. The first check is
> causing a lot of problems with system clocks that don't keep good time and the
> second places an implicit expiry date upon the kernel when used for module
> signing, so do we really need them?
While the date check is entirely bogus for the specific case of module
signing, I don't think we necessarily ought to rip it out of our generic
X.509 support entirely.
Some use cases *might* want to check the dates, and should be permitted
to do so. Just don't refuse to even *parse* the key outside its valid
date range... :)
--
David Woodhouse Open Source Technology Centre
[email protected] Intel Corporation
Am 14.03.2013 13:48, schrieb David Woodhouse:
> On Thu, 2013-03-14 at 12:34 +0000, David Howells wrote:
>> Remove the certificate date checks that are performed when a certificate is
>> parsed. There are two checks: a valid from and a valid to. The first check is
>> causing a lot of problems with system clocks that don't keep good time and the
>> second places an implicit expiry date upon the kernel when used for module
>> signing, so do we really need them?
>
> While the date check is entirely bogus for the specific case of module
> signing, I don't think we necessarily ought to rip it out of our generic
> X.509 support entirely.
>
> Some use cases *might* want to check the dates, and should be permitted
> to do so. Just don't refuse to even *parse* the key outside its valid
> date range... :)
Agreed (thats what my patch did).
I've introduced a new config option because I don't know if something (a
use case I don't know) relies on the validity check of the dates in the
parser. If there currently isn't such a user, just removing the validity
check in the parser might be enough. Offering the parsed dates for later
usage is still a good idea.
Regards,
Alexander
On Thu, 2013-03-14 at 17:22 +0100, Alexander Holler wrote:
>
> Agreed (thats what my patch did).
>
> I've introduced a new config option because I don't know if something (a
> use case I don't know) relies on the validity check of the dates in the
> parser. If there currently isn't such a user, just removing the validity
> check in the parser might be enough.
Is there *is* such a user, it's broken already. The key could have been
loaded (and passed the existing check) *months* ago, expired seconds
after it was loaded, and your hypothetical user could still be happily
trusting it.
> Offering the parsed dates for later usage is still a good idea.
Right.
--
dwmw2
Am 14.03.2013 18:09, schrieb David Woodhouse:
> On Thu, 2013-03-14 at 17:22 +0100, Alexander Holler wrote:
>>
>> Agreed (thats what my patch did).
>>
>> I've introduced a new config option because I don't know if something (a
>> use case I don't know) relies on the validity check of the dates in the
>> parser. If there currently isn't such a user, just removing the validity
>> check in the parser might be enough.
>
> Is there *is* such a user, it's broken already. The key could have been
> loaded (and passed the existing check) *months* ago, expired seconds
> after it was loaded, and your hypothetical user could still be happily
> trusting it.
As the user (program or whatever) calls the parser, he knows if he can
trust it to validate dates. So there might be something for which the
current implementation works (parsing date = using date).
I just don't know, because I've only discovered that glitch while trying
to use modsign to be sure no unsigned module (I've compiled myself) will
be become loaded (I compile the kernel and delete the keys right
afterwards). So I don't know anything if and how the crypto-api to load
x.509 keys is used besides modsign. ;)
Regards,
Alexander