Add testing for golang 1.16 (letsencrypt#5313)

andygabby · web-flow · commit ceffe18dfc08 · 2021-03-11T11:47:41.000-08:00
- Add 1.16.1 to the GitHub CI test matrix - Fix tlsalpn tests for go 1.16.1 but maintain compatibility with 1.15.x - Fix integration tests. Fix: letsencrypt#5301 Fix: letsencrypt#5316
diff --git a/.github/workflows/boulder-ci.yml b/.github/workflows/boulder-ci.yml
@@ -30,7 +30,8 @@ jobs:
       matrix:
         # Add additional docker image tags here and all tests will be run with the additional image.
         BOULDER_TOOLS_TAG:
-          - go1.15.7_2021-02-25
+          - go1.15.7_2021-03-11
+          - go1.16.1_2021-03-11
         # Tests command definitions. Use the entire docker-compose command you want to run.
         tests:
           # Run ./test.sh --help for a description of each of the flags
diff --git a/docker-compose.yml b/docker-compose.yml
@@ -2,7 +2,7 @@ version: '3'
 services:
     boulder:
         # To minimize fetching this should be the same version used below
-        image: letsencrypt/boulder-tools:${BOULDER_TOOLS_TAG:-go1.15.7_2021-02-25}
+        image: letsencrypt/boulder-tools:${BOULDER_TOOLS_TAG:-go1.15.7_2021-03-11}
         environment:
             - FAKE_DNS=10.77.77.77
             - BOULDER_CONFIG_DIR=test/config
@@ -76,7 +76,7 @@ services:
         logging:
             driver: none
     netaccess:
-        image: letsencrypt/boulder-tools:${BOULDER_TOOLS_TAG:-go1.15.7_2021-02-25}
+        image: letsencrypt/boulder-tools:${BOULDER_TOOLS_TAG:-go1.15.7_2021-03-11}
         environment:
             GO111MODULE: "on"
             GOFLAGS: "-mod=vendor"
diff --git a/test/boulder-tools/build.sh b/test/boulder-tools/build.sh
@@ -48,7 +48,7 @@ GO111MODULE=on go get \
 
 # Pebble's latest version is v2+, but it's not properly go mod compatible, so we
 # fetch it in GOPATH mode.
-go get github.com/letsencrypt/pebble/cmd/pebble-challtestsrv
+GO111MODULE=off go get github.com/letsencrypt/pebble/cmd/pebble-challtestsrv
 
 go clean -cache
 go clean -modcache
diff --git a/test/boulder-tools/tag_and_upload.sh b/test/boulder-tools/tag_and_upload.sh
@@ -5,7 +5,7 @@ cd $(dirname $0)
 DATESTAMP=$(date +%Y-%m-%d)
 DOCKER_REPO="letsencrypt/boulder-tools"
 
-GO_VERSIONS=( "1.15.7" )
+GO_VERSIONS=( "1.15.7" "1.16.1" )
 
 # Build a tagged image for each GO_VERSION
 for GO_VERSION in "${GO_VERSIONS[@]}"
diff --git a/va/tlsalpn.go b/va/tlsalpn.go
@@ -48,6 +48,10 @@ func certNames(cert *x509.Certificate) []string {
 	}
 	names = append(names, cert.DNSNames...)
 	names = core.UniqueLowerNames(names)
+	// TODO(#5321): This for loop can be deleted after new builds of boulder use
+	// golang 1.16. In 1.16, code was added to crypto/x509 to not allow
+	// invalid unicode into a DNSName in a SAN. An error will be caught in
+	// the standard library before it gets to this point.
 	for i, n := range names {
 		names[i] = replaceInvalidUTF8([]byte(n))
 	}
diff --git a/va/tlsalpn_test.go b/va/tlsalpn_test.go
@@ -114,34 +114,45 @@ func tlsalpn01Srv(
 	chall core.Challenge,
 	oid asn1.ObjectIdentifier,
 	minTLSVersion uint16,
-	names ...string) *httptest.Server {
+	names ...string) (*httptest.Server, error) {
 	template := tlsCertTemplate(names)
-	certBytes, _ := x509.CreateCertificate(rand.Reader, template, template, &TheKey.PublicKey, &TheKey)
+	certBytes, err := x509.CreateCertificate(rand.Reader, template, template, &TheKey.PublicKey, &TheKey)
+	if err != nil {
+		return nil, err
+	}
 	cert := &tls.Certificate{
 		Certificate: [][]byte{certBytes},
 		PrivateKey:  &TheKey,
 	}
 
 	shasum := sha256.Sum256([]byte(chall.ProvidedKeyAuthorization))
-	encHash, _ := asn1.Marshal(shasum[:])
+	encHash, err := asn1.Marshal(shasum[:])
+	if err != nil {
+		return nil, err
+	}
 	acmeExtension := pkix.Extension{
 		Id:       oid,
 		Critical: true,
 		Value:    encHash,
 	}
 	template.ExtraExtensions = []pkix.Extension{acmeExtension}
-	certBytes, _ = x509.CreateCertificate(rand.Reader, template, template, &TheKey.PublicKey, &TheKey)
+	certBytes, err = x509.CreateCertificate(rand.Reader, template, template, &TheKey.PublicKey, &TheKey)
+	if err != nil {
+		return nil, err
+	}
 	acmeCert := &tls.Certificate{
 		Certificate: [][]byte{certBytes},
 		PrivateKey:  &TheKey,
 	}
 
-	return tlsalpn01SrvWithCert(t, chall, oid, names, cert, acmeCert, minTLSVersion)
+	return tlsalpn01SrvWithCert(t, chall, oid, names, cert, acmeCert, minTLSVersion), nil
 }
 
 func TestTLSALPN01FailIP(t *testing.T) {
 	chall := tlsalpnChallenge()
-	hs := tlsalpn01Srv(t, chall, IdPeAcmeIdentifier, 0, "localhost")
+	hs, err := tlsalpn01Srv(t, chall, IdPeAcmeIdentifier, 0, "localhost")
+	test.AssertNotError(t, err, "Error creating test server")
+
 	va, _ := setup(hs, 0, "", nil)
 
 	port := getPort(hs)
@@ -254,7 +265,9 @@ func TestTLSALPN01DialTimeout(t *testing.T) {
 
 func TestTLSALPN01Refused(t *testing.T) {
 	chall := tlsalpnChallenge()
-	hs := tlsalpn01Srv(t, chall, IdPeAcmeIdentifier, 0, "localhost")
+	hs, err := tlsalpn01Srv(t, chall, IdPeAcmeIdentifier, 0, "localhost")
+	test.AssertNotError(t, err, "Error creating test server")
+
 	va, _ := setup(hs, 0, "", nil)
 	// Take down validation server and check that validation fails.
 	hs.Close()
@@ -271,7 +284,9 @@ func TestTLSALPN01Refused(t *testing.T) {
 
 func TestTLSALPN01TalkingToHTTP(t *testing.T) {
 	chall := tlsalpnChallenge()
-	hs := tlsalpn01Srv(t, chall, IdPeAcmeIdentifier, 0, "localhost")
+	hs, err := tlsalpn01Srv(t, chall, IdPeAcmeIdentifier, 0, "localhost")
+	test.AssertNotError(t, err, "Error creating test server")
+
 	va, _ := setup(hs, 0, "", nil)
 	httpOnly := httpSrv(t, "")
 	va.tlsPort = getPort(httpOnly)
@@ -334,13 +349,11 @@ func TestCertNames(t *testing.T) {
 		"hello.world", "goodbye.world",
 		"bonjour.le.monde", "au.revoir.le.monde",
 		"bonjour.le.monde", "au.revoir.le.monde",
-		"f\xffoo", "f\xffoo",
 	}
-	// We expect only unique names, in sorted order and with any invalid utf-8
-	// replaced.
+	// We expect only unique names, in sorted order.
 	expected := []string{
 		"au.revoir.le.monde", "bonjour.le.monde",
-		"f\ufffdoo", "goodbye.world", "hello.world",
+		"goodbye.world", "hello.world",
 	}
 	template := &x509.Certificate{
 		SerialNumber:          big.NewInt(1337),
@@ -358,15 +371,20 @@ func TestCertNames(t *testing.T) {
 	}
 
 	// Create the certificate, check that certNames provides the expected result
-	certBytes, _ := x509.CreateCertificate(rand.Reader, template, template, &TheKey.PublicKey, &TheKey)
-	cert, _ := x509.ParseCertificate(certBytes)
+	certBytes, err := x509.CreateCertificate(rand.Reader, template, template, &TheKey.PublicKey, &TheKey)
+	test.AssertNotError(t, err, "Error creating certificate")
+
+	cert, err := x509.ParseCertificate(certBytes)
+	test.AssertNotError(t, err, "Error parsing certificate")
+
 	actual := certNames(cert)
 	test.AssertDeepEquals(t, actual, expected)
 }
 
 func TestTLSALPN01Success(t *testing.T) {
 	chall := tlsalpnChallenge()
-	hs := tlsalpn01Srv(t, chall, IdPeAcmeIdentifier, 0, "localhost")
+	hs, err := tlsalpn01Srv(t, chall, IdPeAcmeIdentifier, 0, "localhost")
+	test.AssertNotError(t, err, "Error creating test server")
 
 	va, _ := setup(hs, 0, "", nil)
 
@@ -378,7 +396,8 @@ func TestTLSALPN01Success(t *testing.T) {
 
 	hs.Close()
 	chall = tlsalpnChallenge()
-	hs = tlsalpn01Srv(t, chall, IdPeAcmeIdentifierV1Obsolete, 0, "localhost")
+	hs, err = tlsalpn01Srv(t, chall, IdPeAcmeIdentifierV1Obsolete, 0, "localhost")
+	test.AssertNotError(t, err, "Error creating test server")
 
 	va, _ = setup(hs, 0, "", nil)
 
@@ -394,7 +413,9 @@ func TestValidateTLSALPN01BadChallenge(t *testing.T) {
 	chall2 := chall
 	setChallengeToken(&chall2, "bad token")
 
-	hs := tlsalpn01Srv(t, chall2, IdPeAcmeIdentifier, 0, "localhost")
+	hs, err := tlsalpn01Srv(t, chall2, IdPeAcmeIdentifier, 0, "localhost")
+	test.AssertNotError(t, err, "Error creating test server")
+
 	va, _ := setup(hs, 0, "", nil)
 
 	_, prob := va.validateTLSALPN01(ctx, dnsi("localhost"), chall)
@@ -446,7 +467,18 @@ func TestValidateTLSALPN01UnawareSrv(t *testing.T) {
 // will result in a problem with the invalid UTF-8 replaced.
 func TestValidateTLSALPN01BadUTFSrv(t *testing.T) {
 	chall := tlsalpnChallenge()
-	hs := tlsalpn01Srv(t, chall, IdPeAcmeIdentifier, 0, "localhost", "\xf0\x28\x8c\xbc")
+	hs, err := tlsalpn01Srv(t, chall, IdPeAcmeIdentifier, 0, "localhost", "\xf0\x28\x8c\xbc")
+	// TODO(#5321): Remove this comment and the err check below. In go1.16 and
+	// greater tlsalpn01Srv is expected to fail because of invalid unicode
+	// attempted in the certificate creation. If that error occurs, then
+	// the standard library has done it's job and this test is satisfied.
+	// If the error is for any other reason, the unit test will fail. In
+	// 1.15.x this error is not expected and the other test cases will
+	// continue.
+	if err != nil {
+		test.AssertContains(t, err.Error(), "cannot be encoded as an IA5String")
+		return
+	}
 	port := getPort(hs)
 	va, _ := setup(hs, 0, "", nil)
 
@@ -523,7 +555,8 @@ func TestValidateTLSALPN01MalformedExtnValue(t *testing.T) {
 func TestTLSALPN01TLS13(t *testing.T) {
 	chall := tlsalpnChallenge()
 	// Create a server that uses tls.VersionTLS13 as the minimum supported version
-	hs := tlsalpn01Srv(t, chall, IdPeAcmeIdentifier, tls.VersionTLS13, "localhost")
+	hs, err := tlsalpn01Srv(t, chall, IdPeAcmeIdentifier, tls.VersionTLS13, "localhost")
+	test.AssertNotError(t, err, "Error creating test server")
 	defer hs.Close()
 
 	va, _ := setup(hs, 0, "", nil)

Original file line number	Diff line number	Diff line change
`@@ -48,6 +48,10 @@ func certNames(cert *x509.Certificate) []string {`
`48`	`48`	`}`
`49`	`49`	`names = append(names, cert.DNSNames...)`
`50`	`50`	`names = core.UniqueLowerNames(names)`
	`51`	`+ // TODO(#5321): This for loop can be deleted after new builds of boulder use`
	`52`	`+ // golang 1.16. In 1.16, code was added to crypto/x509 to not allow`
	`53`	`+ // invalid unicode into a DNSName in a SAN. An error will be caught in`
	`54`	`+ // the standard library before it gets to this point.`
`51`	`55`	`for i, n := range names {`
`52`	`56`	`names[i] = replaceInvalidUTF8([]byte(n))`
`53`	`57`	`}`