[PATCH 0/5] X.509: Fix time handling

[PATCH 0/5] X.509: Fix time handling

[David Howells]

Here's a set of patches that fix X.509 time handling in three ways:

 (1) Fix leap year handling.

 (2) Add leap second handling (where you get a time of 23:59:60).

 (3) Add end-of-day midnight encoding (where you get a time of 24:00:00).

David
---
David Howells (5):
      X.509: Fix leap year handling again
      Handle leap seconds in mktime64()
      X.509: Support leap seconds
      Handle both ISO 8601 encodings of midnight in mktime64()
      X.509: Handle midnight alternative notation in GeneralizedTime


 crypto/asymmetric_keys/x509_cert_parser.c |   24 +++++++++++++++++-------
 include/linux/time.h                      |   13 ++++++-------
 kernel/time/time.c                        |   19 +++++++++++++++----
 3 files changed, 38 insertions(+), 18 deletions(-)

[PATCH 1/5] X.509: Fix leap year handling again

[David Howells]

There are still a couple of minor issues in the X.509 leap year handling:

 (1) To avoid doing a modulus-by-400 in addition to a modulus-by-100 when
     determining whether the year is a leap year or not, I divided the year
     by 100 after doing the modulus-by-100, thereby letting the compiler do
     one instruction for both, and then did a modulus-by-4.

     Unfortunately, I then passed the now-modified year value to mktime64()
     to construct a time value.

     Since this isn't a fast path and since mktime64() does a bunch of
     divisions, just condense down to "% 400".  It's also easier to read.

 (2) The default month length for any February where the year doesn't
     divide by four exactly is obtained from the month_length[] array where
     the value is 29, not 28.

     This is fixed by altering the table.

Reported-by: Rudolf Polzer <rpolzer@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-By: David Woodhouse <David.Woodhouse@intel.com>
cc: stable@vger.kernel.org
---

 crypto/asymmetric_keys/x509_cert_parser.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 021d39c0ba75..13c4e5a5fe8c 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -494,7 +494,7 @@ int x509_decode_time(time64_t *_t,  size_t hdrlen,
 		     unsigned char tag,
 		     const unsigned char *value, size_t vlen)
 {
-	static const unsigned char month_lengths[] = { 31, 29, 31, 30, 31, 30,
+	static const unsigned char month_lengths[] = { 31, 28, 31, 30, 31, 30,
 						       31, 31, 30, 31, 30, 31 };
 	const unsigned char *p = value;
 	unsigned year, mon, day, hour, min, sec, mon_len;
@@ -540,9 +540,9 @@ int x509_decode_time(time64_t *_t,  size_t hdrlen,
 		if (year % 4 == 0) {
 			mon_len = 29;
 			if (year % 100 == 0) {
-				year /= 100;
-				if (year % 4 != 0)
-					mon_len = 28;
+				mon_len = 28;
+				if (year % 400 == 0)
+					mon_len = 29;
 			}
 		}
 	}

[PATCH 2/5] Handle leap seconds in mktime64()

[David Howells]

Handle leap seconds in mktime64() - where the seconds parameter is the
value 60 - by treating it the same as 59.

This facility will be used by the X.509 parser.  Doing it in mktime64()
makes the policy common to the whole kernel and easier to find.

Whilst we're at it, remove the const markers from all the parameters since
they don't really achieve anything and we do need to alter the sec
parameter.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: John Stultz <john.stultz@linaro.org>
cc: Arnd Bergmann <arnd@arndb.de>
cc: stable@vger.kernel.org
---

 include/linux/time.h |   13 ++++++-------
 kernel/time/time.c   |   14 +++++++++++---
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/include/linux/time.h b/include/linux/time.h
index beebe3a02d43..35384f0c0aa2 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -39,17 +39,16 @@ static inline int timeval_compare(const struct timeval *lhs, const struct timeva
 	return lhs->tv_usec - rhs->tv_usec;
 }
 
-extern time64_t mktime64(const unsigned int year, const unsigned int mon,
-			const unsigned int day, const unsigned int hour,
-			const unsigned int min, const unsigned int sec);
+extern time64_t mktime64(unsigned int year, unsigned int mon,
+			 unsigned int day, unsigned int hour,
+			 unsigned int min, unsigned int sec);
 
 /**
  * Deprecated. Use mktime64().
  */
-static inline unsigned long mktime(const unsigned int year,
-			const unsigned int mon, const unsigned int day,
-			const unsigned int hour, const unsigned int min,
-			const unsigned int sec)
+static inline unsigned long mktime(unsigned int year, unsigned int mon,
+				   unsigned int day, unsigned int hour,
+				   unsigned int min, unsigned int sec)
 {
 	return mktime64(year, mon, day, hour, min, sec);
 }
diff --git a/kernel/time/time.c b/kernel/time/time.c
index 86751c68e08d..1858b10602f5 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -322,10 +322,14 @@ EXPORT_SYMBOL(timespec_trunc);
  * -year/100+year/400 terms, and add 10.]
  *
  * This algorithm was first published by Gauss (I think).
+ *
+ * A leap second can be indicated by calling this function with sec as
+ * 60 (allowable under ISO 8601).  The leap second is treated the same
+ * as the preceding second since they don't exist in UNIX time.
  */
-time64_t mktime64(const unsigned int year0, const unsigned int mon0,
-		const unsigned int day, const unsigned int hour,
-		const unsigned int min, const unsigned int sec)
+time64_t mktime64(unsigned int year0, unsigned int mon0,
+		  unsigned int day, unsigned int hour,
+		  unsigned int min, unsigned int sec)
 {
 	unsigned int mon = mon0, year = year0;
 
@@ -335,6 +339,10 @@ time64_t mktime64(const unsigned int year0, const unsigned int mon0,
 		year -= 1;
 	}
 
+	/* Handle leap seconds */
+	if (sec == 60)
+		sec = 59;
+
 	return ((((time64_t)
 		  (year/4 - year/100 + year/400 + 367*mon/12 + day) +
 		  year*365 - 719499

Re: [PATCH 2/5] Handle leap seconds in mktime64()

[Arnd Bergmann]

On Friday 18 December 2015 00:02:02 David Howells wrote:
> Handle leap seconds in mktime64() - where the seconds parameter is the
> value 60 - by treating it the same as 59.
> 
> This facility will be used by the X.509 parser.  Doing it in mktime64()
> makes the policy common to the whole kernel and easier to find.
> 
> Whilst we're at it, remove the const markers from all the parameters since
> they don't really achieve anything and we do need to alter the sec
> parameter.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: John Stultz <john.stultz@linaro.org>
> cc: Arnd Bergmann <arnd@arndb.de>
> cc: stable@vger.kernel.org
> 

Acked-by: Arnd Bergmann <arnd@arndb.de>

[PATCH 3/5] X.509: Support leap seconds

[David Howells]

The format of ASN.1 GeneralizedTime seems to be specified by ISO 8601
[X.680 46.3] and this apparently supports leap seconds (ie. the seconds
field is 60).  It's not entirely clear that ASN.1 expects it, but we can
relax the seconds check slightly for GeneralizedTime.

This, however, results in us passing a time with sec as 60 to mktime64()
which, unpatched, doesn't really handle such things.  What it will do is
equate the 60th second of a minute to the 0th second of the next minute.

We can't really do otherwise without giving the kernel much greater
knowledge of where all the leap seconds are.  Unfortunately, this would
require change the mapping of the kernel's current-time-in-seconds.

UTCTime, however, only supports a seconds value in the range 00-59.

Without this patch, certain X.509 certificates will be rejected,
potentially making a kernel unbootable.

Reported-by: Rudolf Polzer <rpolzer@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: David Woodhouse <David.Woodhouse@intel.com>
cc: John Stultz <john.stultz@linaro.org>
cc: Arnd Bergmann <arnd@arndb.de>
cc: stable@vger.kernel.org
---

 crypto/asymmetric_keys/x509_cert_parser.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 13c4e5a5fe8c..9be2caebc57b 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -497,7 +497,7 @@ int x509_decode_time(time64_t *_t,  size_t hdrlen,
 	static const unsigned char month_lengths[] = { 31, 28, 31, 30, 31, 30,
 						       31, 31, 30, 31, 30, 31 };
 	const unsigned char *p = value;
-	unsigned year, mon, day, hour, min, sec, mon_len;
+	unsigned year, mon, day, hour, min, sec, mon_len, max_sec;
 
 #define dec2bin(X) ({ unsigned char x = (X) - '0'; if (x > 9) goto invalid_time; x; })
 #define DD2bin(P) ({ unsigned x = dec2bin(P[0]) * 10 + dec2bin(P[1]); P += 2; x; })
@@ -511,6 +511,7 @@ int x509_decode_time(time64_t *_t,  size_t hdrlen,
 			year += 1900;
 		else
 			year += 2000;
+		max_sec = 59;
 	} else if (tag == ASN1_GENTIM) {
 		/* GenTime: YYYYMMDDHHMMSSZ */
 		if (vlen != 15)
@@ -518,6 +519,7 @@ int x509_decode_time(time64_t *_t,  size_t hdrlen,
 		year = DD2bin(p) * 100 + DD2bin(p);
 		if (year >= 1950 && year <= 2049)
 			goto invalid_time;
+		max_sec = 60; /* ISO 8601 permits leap seconds [X.680 46.3] */
 	} else {
 		goto unsupported_time;
 	}
@@ -550,7 +552,7 @@ int x509_decode_time(time64_t *_t,  size_t hdrlen,
 	if (day < 1 || day > mon_len ||
 	    hour > 23 ||
 	    min > 59 ||
-	    sec > 59)
+	    sec > max_sec)
 		goto invalid_time;
 
 	*_t = mktime64(year, mon, day, hour, min, sec);

Re: [PATCH 3/5] X.509: Support leap seconds

[Arnd Bergmann]

On Friday 18 December 2015 00:02:09 David Howells wrote:
> The format of ASN.1 GeneralizedTime seems to be specified by ISO 8601
> [X.680 46.3] and this apparently supports leap seconds (ie. the seconds
> field is 60).  It's not entirely clear that ASN.1 expects it, but we can
> relax the seconds check slightly for GeneralizedTime.
> 
> This, however, results in us passing a time with sec as 60 to mktime64()
> which, unpatched, doesn't really handle such things.  What it will do is
> equate the 60th second of a minute to the 0th second of the next minute.
> 
> We can't really do otherwise without giving the kernel much greater
> knowledge of where all the leap seconds are.  Unfortunately, this would
> require change the mapping of the kernel's current-time-in-seconds.
> 
> UTCTime, however, only supports a seconds value in the range 00-59.
> 
> Without this patch, certain X.509 certificates will be rejected,
> potentially making a kernel unbootable.
> 
> Reported-by: Rudolf Polzer <rpolzer@google.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: David Woodhouse <David.Woodhouse@intel.com>
> cc: John Stultz <john.stultz@linaro.org>
> cc: Arnd Bergmann <arnd@arndb.de>
> cc: stable@vger.kernel.org

Acked-by: Arnd Bergmann <arnd@arndb.de>

[PATCH 4/5] Handle both ISO 8601 encodings of midnight in mktime64()

[David Howells]

ISO 8601 format dates permit two different encodings of midnight - 00:00:00
and 24:00:00 - the first is midnight today and the second is midnight
tomorrow and is exactly equivalent to the first with tomorrow's date.

Note that the implementation of mktime64() doesn't actually need to be
changed to handle this - the multiplication by 3600 of the hour will take
care of it automatically.  However, we should document that this handling
is done in mktime64() and is thus in a common place in the kernel.

This handling is required for X.509 certificate parsing which can be given
ISO 8601 dates.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: John Stultz <john.stultz@linaro.org>
cc: Arnd Bergmann <arnd@arndb.de>
cc: stable@vger.kernel.org
---

 kernel/time/time.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/time/time.c b/kernel/time/time.c
index 1858b10602f5..56e7ada38471 100644
--- a/kernel/time/time.c
+++ b/kernel/time/time.c
@@ -326,6 +326,9 @@ EXPORT_SYMBOL(timespec_trunc);
  * A leap second can be indicated by calling this function with sec as
  * 60 (allowable under ISO 8601).  The leap second is treated the same
  * as the preceding second since they don't exist in UNIX time.
+ *
+ * An encoding of midnight at the end of the day as 24:00:00 - ie. midnight
+ * tomorrow - (allowable under ISO 8601) is supported.
  */
 time64_t mktime64(unsigned int year0, unsigned int mon0,
 		  unsigned int day, unsigned int hour,
@@ -346,7 +349,7 @@ time64_t mktime64(unsigned int year0, unsigned int mon0,
 	return ((((time64_t)
 		  (year/4 - year/100 + year/400 + 367*mon/12 + day) +
 		  year*365 - 719499
-	    )*24 + hour /* now have hours */
+	    )*24 + hour /* now have hours - midnight tomorrow handled here */
 	  )*60 + min /* now have minutes */
 	)*60 + sec; /* finally seconds */
 }

Re: [PATCH 4/5] Handle both ISO 8601 encodings of midnight in mktime64()

[Arnd Bergmann]

On Friday 18 December 2015 00:02:17 David Howells wrote:
> ISO 8601 format dates permit two different encodings of midnight - 00:00:00
> and 24:00:00 - the first is midnight today and the second is midnight
> tomorrow and is exactly equivalent to the first with tomorrow's date.
> 
> Note that the implementation of mktime64() doesn't actually need to be
> changed to handle this - the multiplication by 3600 of the hour will take
> care of it automatically.  However, we should document that this handling
> is done in mktime64() and is thus in a common place in the kernel.
> 
> This handling is required for X.509 certificate parsing which can be given
> ISO 8601 dates.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: John Stultz <john.stultz@linaro.org>
> cc: Arnd Bergmann <arnd@arndb.de>
> cc: stable@vger.kernel.org
> 

Acked-by: Arnd Bergmann <arnd@arndb.de>

[PATCH 5/5] X.509: Handle midnight alternative notation in GeneralizedTime

[David Howells]

The ASN.1 GeneralizedTime object carries an ISO8601 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.

Reported-by: Rudolf Polzer <rpolzer@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: David Woodhouse <David.Woodhouse@intel.com>
cc: John Stultz <john.stultz@linaro.org>
cc: Arnd Bergmann <arnd@arndb.de>
cc: stable@vger.kernel.org
---

 crypto/asymmetric_keys/x509_cert_parser.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 9be2caebc57b..b9de251c419c 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -497,7 +497,7 @@ int x509_decode_time(time64_t *_t,  size_t hdrlen,
 	static const unsigned char month_lengths[] = { 31, 28, 31, 30, 31, 30,
 						       31, 31, 30, 31, 30, 31 };
 	const unsigned char *p = value;
-	unsigned year, mon, day, hour, min, sec, mon_len, max_sec;
+	unsigned year, mon, day, hour, min, sec, mon_len, max_sec, max_hour;
 
 #define dec2bin(X) ({ unsigned char x = (X) - '0'; if (x > 9) goto invalid_time; x; })
 #define DD2bin(P) ({ unsigned x = dec2bin(P[0]) * 10 + dec2bin(P[1]); P += 2; x; })
@@ -512,6 +512,7 @@ int x509_decode_time(time64_t *_t,  size_t hdrlen,
 		else
 			year += 2000;
 		max_sec = 59;
+		max_hour = 23;
 	} else if (tag == ASN1_GENTIM) {
 		/* GenTime: YYYYMMDDHHMMSSZ */
 		if (vlen != 15)
@@ -520,6 +521,7 @@ int x509_decode_time(time64_t *_t,  size_t hdrlen,
 		if (year >= 1950 && year <= 2049)
 			goto invalid_time;
 		max_sec = 60; /* ISO 8601 permits leap seconds [X.680 46.3] */
+		max_hour = 24;
 	} else {
 		goto unsupported_time;
 	}
@@ -550,11 +552,17 @@ int x509_decode_time(time64_t *_t,  size_t hdrlen,
 	}
 
 	if (day < 1 || day > mon_len ||
-	    hour > 23 ||
+	    hour > max_hour ||
 	    min > 59 ||
 	    sec > max_sec)
 		goto invalid_time;
 
+	/* GeneralizedTime, encoded as ISO 8601, also permits 24:00 today as an
+	 * alternative for 00:00 tomorrow.
+	 */
+	if (hour == 24 && (min != 0 || sec != 0))
+		goto invalid_time;
+
 	*_t = mktime64(year, mon, day, hour, min, sec);
 	return 0;