encoding/asn1: be stricter by reserialising parsed times.

The time package does normalisation of times: for example day zero is
converted to the last day of the previous month and the 31st of February
is moved into March etc. This makes the ASN.1 parsing a little
worryingly lax.

This change causes the parser to reserialise parsed times to ensure that
they round-trip correctly and thus were not normalised.

Fixes #11134.

Change-Id: I3988bb95153a7b33d64ab861fbe51b1a34a359e9
Reviewed-on: https://go-review.googlesource.com/11094
Run-TryBot: Adam Langley <agl@golang.org>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

2 files changed, 64 insertions, 5 deletions

diff --git a/src/encoding/asn1/asn1.go b/src/encoding/asn1/asn1.go
index b4457e02dc..3eab6aa384 100644
--- a/src/encoding/asn1/asn1.go
+++ b/src/encoding/asn1/asn1.go
@@ -288,11 +288,23 @@ func parseBase128Int(bytes []byte, initOffset int) (ret, offset int, err error)
 
 func parseUTCTime(bytes []byte) (ret time.Time, err error) {
 	s := string(bytes)
-	ret, err = time.Parse("0601021504Z0700", s)
+
+	formatStr := "0601021504Z0700"
+	ret, err = time.Parse(formatStr, s)
+	if err != nil {
+		formatStr = "060102150405Z0700"
+		ret, err = time.Parse(formatStr, s)
+	}
 	if err != nil {
-		ret, err = time.Parse("060102150405Z0700", s)
+		return
 	}
-	if err == nil && ret.Year() >= 2050 {
+
+	if serialized := ret.Format(formatStr); serialized != s {
+		err = fmt.Errorf("asn1: time did not serialize back to the original value and may be invalid: given %q, but serialized as %q", s, serialized)
+		return
+	}
+
+	if ret.Year() >= 2050 {
 		// UTCTime only encodes times prior to 2050. See https://tools.ietf.org/html/rfc5280#section-4.1.2.5.1
 		ret = ret.AddDate(-100, 0, 0)
 	}
@@ -303,7 +315,18 @@ func parseUTCTime(bytes []byte) (ret time.Time, err error) {
 // parseGeneralizedTime parses the GeneralizedTime from the given byte slice
 // and returns the resulting time.
 func parseGeneralizedTime(bytes []byte) (ret time.Time, err error) {
-	return time.Parse("20060102150405Z0700", string(bytes))
+	const formatStr = "20060102150405Z0700"
+	s := string(bytes)
+
+	if ret, err = time.Parse(formatStr, s); err != nil {
+		return
+	}
+
+	if serialized := ret.Format(formatStr); serialized != s {
+		err = fmt.Errorf("asn1: time did not serialize back to the original value and may be invalid: given %q, but serialized as %q", s, serialized)
+	}
+
+	return
 }
 
 // PrintableString
diff --git a/src/encoding/asn1/asn1_test.go b/src/encoding/asn1/asn1_test.go
index 32e9ff2b0c..acba0965a2 100644
--- a/src/encoding/asn1/asn1_test.go
+++ b/src/encoding/asn1/asn1_test.go
@@ -258,6 +258,24 @@ var utcTestData = []timeTest{
 	{"91050633444aZ", false, time.Time{}},
 	{"910506334461Z", false, time.Time{}},
 	{"910506334400Za", false, time.Time{}},
+	/* These are invalid times. However, the time package normalises times
+	 * and they were accepted in some versions. See #11134. */
+	{"000100000000Z", false, time.Time{}},
+	{"101302030405Z", false, time.Time{}},
+	{"100002030405Z", false, time.Time{}},
+	{"100100030405Z", false, time.Time{}},
+	{"100132030405Z", false, time.Time{}},
+	{"100231030405Z", false, time.Time{}},
+	{"100102240405Z", false, time.Time{}},
+	{"100102036005Z", false, time.Time{}},
+	{"100102030460Z", false, time.Time{}},
+	{"-100102030410Z", false, time.Time{}},
+	{"10-0102030410Z", false, time.Time{}},
+	{"10-0002030410Z", false, time.Time{}},
+	{"1001-02030410Z", false, time.Time{}},
+	{"100102-030410Z", false, time.Time{}},
+	{"10010203-0410Z", false, time.Time{}},
+	{"1001020304-10Z", false, time.Time{}},
 }
 
 func TestUTCTime(t *testing.T) {
@@ -287,6 +305,24 @@ var generalizedTimeTestData = []timeTest{
 	{"20100102030405", false, time.Time{}},
 	{"20100102030405+0607", true, time.Date(2010, 01, 02, 03, 04, 05, 0, time.FixedZone("", 6*60*60+7*60))},
 	{"20100102030405-0607", true, time.Date(2010, 01, 02, 03, 04, 05, 0, time.FixedZone("", -6*60*60-7*60))},
+	/* These are invalid times. However, the time package normalises times
+	 * and they were accepted in some versions. See #11134. */
+	{"00000100000000Z", false, time.Time{}},
+	{"20101302030405Z", false, time.Time{}},
+	{"20100002030405Z", false, time.Time{}},
+	{"20100100030405Z", false, time.Time{}},
+	{"20100132030405Z", false, time.Time{}},
+	{"20100231030405Z", false, time.Time{}},
+	{"20100102240405Z", false, time.Time{}},
+	{"20100102036005Z", false, time.Time{}},
+	{"20100102030460Z", false, time.Time{}},
+	{"-20100102030410Z", false, time.Time{}},
+	{"2010-0102030410Z", false, time.Time{}},
+	{"2010-0002030410Z", false, time.Time{}},
+	{"201001-02030410Z", false, time.Time{}},
+	{"20100102-030410Z", false, time.Time{}},
+	{"2010010203-0410Z", false, time.Time{}},
+	{"201001020304-10Z", false, time.Time{}},
 }
 
 func TestGeneralizedTime(t *testing.T) {
@@ -297,7 +333,7 @@ func TestGeneralizedTime(t *testing.T) {
 		}
 		if err == nil {
 			if !reflect.DeepEqual(test.out, ret) {
-				t.Errorf("#%d: Bad result: %v (expected %v)", i, ret, test.out)
+				t.Errorf("#%d: Bad result: %q → %v (expected %v)", i, test.in, ret, test.out)
 			}
 		}
 	}