linux-kernel - Re: [RFC PATCH 4/4] X.509: Handle midnight alternative notation in GeneralizedTime

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABwpRLSHs-pU-djUpoMZ1C_33R_6s5+d5_oqnesXtNv3YhbGXA@mail.gmail.com>
Date:	Wed, 20 Jan 2016 10:08:04 -0500
From:	Rudolf Polzer <rpolzer@...gle.com>
To:	David Howells <dhowells@...hat.com>
Cc:	keyrings@...r.kernel.org,
	David Woodhouse <David.Woodhouse@...el.com>,
	Arnd Bergmann <arnd@...db.de>, linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	John Stultz <john.stultz@...aro.org>
Subject: Re: [RFC PATCH 4/4] X.509: Handle midnight alternative notation in GeneralizedTime

On Mon, Jan 4, 2016 at 5:17 PM, David Howells <dhowells@...hat.com> wrote:
> The ASN.1 GeneralizedTime object carries an ISO 8601 format date and time.
> The time is permitted to show midnight as 00:00 or 24:00 (the latter being
> equivalent of 00:00 of the following day).
>
> The permitted value is checked in x509_decode_time() but the actual
> handling is left to mktime64().
>
> Without this patch, certain X.509 certificates will be rejected and could
> lead to an unbootable kernel.
>
> Note that with this patch we also permit any 24:mm:ss time and extend this
> to UTCTime, which whilst not strictly correct don't permit much leeway in
> fiddling date strings.
>
> Reported-by: Rudolf Polzer <rpolzer@...gle.com>
> Signed-off-by: David Howells <dhowells@...hat.com>
> cc: David Woodhouse <David.Woodhouse@...el.com>
> cc: John Stultz <john.stultz@...aro.org>
> cc: Arnd Bergmann <arnd@...db.de>
> ---
>
>  crypto/asymmetric_keys/x509_cert_parser.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
> index 3379c0ba3988..70ed0852fdb2 100644
> --- a/crypto/asymmetric_keys/x509_cert_parser.c
> +++ b/crypto/asymmetric_keys/x509_cert_parser.c
> @@ -548,7 +548,7 @@ int x509_decode_time(time64_t *_t,  size_t hdrlen,
>         }
>
>         if (day < 1 || day > mon_len ||
> -           hour > 23 ||
> +           hour > 24 || /* ISO 8601 permits 24:00:00 as midnight tomorrow */
>             min > 59 ||
>             sec > 60) /* ISO 8601 permits leap seconds [X.680 46.3] */
>                 goto invalid_time;
>

Looks good.

As for 24:xx:yy times - I'm split about this. This code doesn't
require a bijective decoding anyway (and if it did, 24:00:00 and
00:00:00 mapping to the same time64_t would be problem enough) so this
is sure safe. On the other hand, a cert with a 24:xx:yy time that's
not 24:00:00 probably should be regarded as invalid and not trusted
for that reason alone.

Best regards,

Rudolf Polzer