2013-03-27 18:42:27

by Alexander Holler

[permalink] [raw]
Subject: [PATCH] crypto: Ignore validity dates of X.509 certificates at loading/parsing time

I don't see any real use case where checking the validity dates of X.509
certificates at parsing time adds any security gain. In contrast, doing so
makes MODSIGN unusable on systems without a RTC (or systems with a possible
wrong date in a existing RTC, or systems where the RTC is read after the keys
got loaded).

If something really cares about the dates, it should check them at the time
when the certificates are used, not when they are loaded and parsed.

So just remove the validity check of the dates in the parser.

Signed-off-by: Alexander Holler <[email protected]>
Cc: [email protected]

Sidenote (not for necessarily for inclusion into the patch):
There were already 3 variations of this patch.
The first removed the check by adding a config option:
https://lkml.org/lkml/2013/2/25/171
The second just removed the return of errors:
https://lkml.org/lkml/2013/3/14/247
And the third wich removed parsing of the dates too:
https://lkml.org/lkml/2013/3/14/257
Maybe this one will find the approval of a maintainer.
Here is a thread which explains the motivation for this patch a bit further:
https://lkml.org/lkml/2013/2/13/90
---
crypto/asymmetric_keys/x509_public_key.c | 38 --------------------------------
1 file changed, 38 deletions(-)

diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 06007f0..52222a2 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;
@@ -137,43 +136,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;

--
1.8.1.4


2013-05-02 14:09:06

by Alexander Holler

[permalink] [raw]
Subject: [PATCH RESEND/V2] crypto: Ignore validity dates of X.509 certificates at loading/parsing time

I don't see any real use case where checking the validity dates of X.509
certificates at parsing time adds any security gain. In contrast, doing so
makes MODSIGN unusable on systems without a RTC (or systems with a possible
wrong date in a existing RTC, or systems where the RTC is read after the keys
got loaded).

If something really cares about the dates, it should check them at the time
when the certificates are used, not when they are loaded and parsed.

So just remove the validity check of the dates in the parser.

Signed-off-by: Alexander Holler <[email protected]>
Cc: [email protected]
---

Updates to V1:
I've moved the below comment from the git commit message to this section so
that this patch can be applied without any further modifications.
Maybe this raises the statistical possibility that someone presses a button
or key to feed this simple patch into the mainline kernel as it fixes a real
bug which renders MODSIGN unusable on system without an RTC (which needs to
set the time before modsign loads the crypto keys). Because I think modsign
is a nice security feature e.g. to prevent rootkits which do modify or add
modules, I care about it and try it a last time.

There were already 4 variations of this patch (I know about).

The first removed the check by adding a config option:
https://lkml.org/lkml/2013/2/25/171
The second just removed the return of errors:
https://lkml.org/lkml/2013/3/14/247
The third removed parsing of the dates too:
https://lkml.org/lkml/2013/3/14/257
And finally V1 of this patch:
https://lkml.org/lkml/2013/3/27/449

Here is a thread which explains the motivation for this patch a bit further:
https://lkml.org/lkml/2013/2/13/90

crypto/asymmetric_keys/x509_public_key.c | 38 --------------------------------
1 file changed, 38 deletions(-)

diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 06007f0..52222a2 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;
@@ -137,43 +136,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;

--
1.8.1.4

2013-06-06 11:18:17

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH RESEND/V2] crypto: Ignore validity dates of X.509 certificates at loading/parsing time

Am 02.05.2013 16:09, schrieb Alexander Holler:
> I don't see any real use case where checking the validity dates of X.509
> certificates at parsing time adds any security gain. In contrast, doing so
> makes MODSIGN unusable on systems without a RTC (or systems with a possible
> wrong date in a existing RTC, or systems where the RTC is read after the keys
> got loaded).
>
> If something really cares about the dates, it should check them at the time
> when the certificates are used, not when they are loaded and parsed.
>
> So just remove the validity check of the dates in the parser.
>
> Signed-off-by: Alexander Holler <[email protected]>
> Cc: [email protected]

As it just happened to me again and I've recently posted some patches
which do make it possible to experience the problem on x86 systems too,
here is a reminder.

To replay the problem (on x86 or any other arch), apply the 3 patches in
this series:

https://lkml.org/lkml/2013/6/5/430

build a kernel with CONFIG_MODULE_SIG_FORCE=y and start that kernel with
hctosys=none as kernel command line parameter.

This will disable the "persistent" clock (and any RTC), thus the kernel
will refuse to load modules because it doesn't has a valid time when
loading the certificate.

Regards,

Alexander Holler

2013-06-07 02:13:41

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH RESEND/V2] crypto: Ignore validity dates of X.509 certificates at loading/parsing time

Alexander Holler <[email protected]> writes:
> Am 02.05.2013 16:09, schrieb Alexander Holler:
>> I don't see any real use case where checking the validity dates of X.509
>> certificates at parsing time adds any security gain. In contrast, doing so
>> makes MODSIGN unusable on systems without a RTC (or systems with a possible
>> wrong date in a existing RTC, or systems where the RTC is read after the keys
>> got loaded).
>>
>> If something really cares about the dates, it should check them at the time
>> when the certificates are used, not when they are loaded and parsed.
>>
>> So just remove the validity check of the dates in the parser.
>>
>> Signed-off-by: Alexander Holler <[email protected]>
>> Cc: [email protected]
>
> As it just happened to me again and I've recently posted some patches
> which do make it possible to experience the problem on x86 systems too,
> here is a reminder.
>
> To replay the problem (on x86 or any other arch), apply the 3 patches in
> this series:
>
> https://lkml.org/lkml/2013/6/5/430
>
> build a kernel with CONFIG_MODULE_SIG_FORCE=y and start that kernel with
> hctosys=none as kernel command line parameter.
>
> This will disable the "persistent" clock (and any RTC), thus the kernel
> will refuse to load modules because it doesn't has a valid time when
> loading the certificate.
>
> Regards,
>
> Alexander Holler

David?

Thanks,
Rusty.