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

[Rudolf Polzer]

On Mon, Jan 4, 2016 at 5:17 PM, David Howells <dhowells@xxxxxxxxxx> 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@xxxxxxxxxx>
> Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
> cc: David Woodhouse <David.Woodhouse@xxxxxxxxx>
> cc: John Stultz <john.stultz@xxxxxxxxxx>
> cc: Arnd Bergmann <arnd@xxxxxxxx>
> ---
>
>  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