Improve coverage for TLS-ALPN-01 challenge

acme/challenge.go

@@ -115,25 +115,13 @@ func http01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSONWeb
 }
 
 func tlsalpn01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSONWebKey, vo *ValidateChallengeOptions) error {
-
-	// RFC8738 states that, if HostName is IP, it should be the ARPA
-	// address https://datatracker.ietf.org/doc/html/rfc8738#section-6.
-	// It also references TLS Extensions [RFC6066].
-	var serverName string
-	ip := net.ParseIP(ch.Value)
-	if ip != nil {
-		serverName = reverseAddr(ip)
-	} else {
-		serverName = ch.Value
-	}
-
 	config := &tls.Config{
 		NextProtos: []string{"acme-tls/1"},
 		// https://tools.ietf.org/html/rfc8737#section-4
 		// ACME servers that implement "acme-tls/1" MUST only negotiate TLS 1.2
 		// [RFC5246] or higher when connecting to clients for validation.
 		MinVersion:         tls.VersionTLS12,
-		ServerName:         serverName,
+		ServerName:         serverName(ch),
 		InsecureSkipVerify: true, // we expect a self-signed challenge certificate
 	}
 
@@ -163,7 +151,7 @@ func tlsalpn01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSON
 
 	// if no DNS names present, look for IP address and verify that exactly one exists
 	if len(leafCert.DNSNames) == 0 {
-		if len(leafCert.IPAddresses) != 1 || !leafCert.IPAddresses[0].Equal(ip) {
+		if len(leafCert.IPAddresses) != 1 || !leafCert.IPAddresses[0].Equal(net.ParseIP(ch.Value)) {
 			return storeError(ctx, db, ch, true, NewError(ErrorRejectedIdentifierType,
 				"incorrect certificate for tls-alpn-01 challenge: leaf certificate must contain a single IP address or DNS name, %v", ch.Value))
 		}
@@ -272,6 +260,21 @@ func dns01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSONWebK
 	return nil
 }
 
+// serverName determines the SNI HostName to set based on an acme.Challenge
+// for TLS-ALPN-01 challenges. RFC8738 states that, if HostName is an IP, it
+// should be the ARPA address https://datatracker.ietf.org/doc/html/rfc8738#section-6.
+// It also references TLS Extensions [RFC6066].
+func serverName(ch *Challenge) string {
+	var serverName string
+	ip := net.ParseIP(ch.Value)
+	if ip != nil {
+		serverName = reverseAddr(ip)
+	} else {
+		serverName = ch.Value
+	}
+	return serverName
+}
+
 // reverseaddr returns the in-addr.arpa. or ip6.arpa. hostname of the IP
 // address addr suitable for rDNS (PTR) record lookup or an error if it fails
 // to parse the IP address.

acme/challenge_test.go

@@ -2187,6 +2187,43 @@ func TestTLSALPN01Validate(t *testing.T) {
 			srv, tlsDial := newTestTLSALPNServer(cert)
 			srv.Start()
 
+			return test{
+				ch: ch,
+				vo: &ValidateChallengeOptions{
+					TLSDial: tlsDial,
+				},
+				db: &MockDB{
+					MockUpdateChallenge: func(ctx context.Context, updch *Challenge) error {
+						assert.Equals(t, updch.ID, ch.ID)
+						assert.Equals(t, updch.Token, ch.Token)
+						assert.Equals(t, updch.Status, StatusValid)
+						assert.Equals(t, updch.Type, ch.Type)
+						assert.Equals(t, updch.Value, ch.Value)
+						assert.Equals(t, updch.Error, nil)
+						return nil
+					},
+				},
+				srv: srv,
+				jwk: jwk,
+			}
+		},
+		"ok/ip": func(t *testing.T) test {
+			ch := makeTLSCh()
+			ch.Value = "127.0.0.1"
+
+			jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0)
+			assert.FatalError(t, err)
+
+			expKeyAuth, err := KeyAuthorization(ch.Token, jwk)
+			assert.FatalError(t, err)
+			expKeyAuthHash := sha256.Sum256([]byte(expKeyAuth))
+
+			cert, err := newTLSALPNValidationCert(expKeyAuthHash[:], false, true, ch.Value)
+			assert.FatalError(t, err)
+
+			srv, tlsDial := newTestTLSALPNServer(cert)
+			srv.Start()
+
 			return test{
 				ch: ch,
 				vo: &ValidateChallengeOptions{
@@ -2234,4 +2271,84 @@ func TestTLSALPN01Validate(t *testing.T) {
 			}
 		})
 	}
+	t.Fail()
+}
+
+func Test_reverseAddr(t *testing.T) {
+	type args struct {
+		ip net.IP
+	}
+	tests := []struct {
+		name     string
+		args     args
+		wantArpa string
+	}{
+		{
+			name: "ok/ipv4",
+			args: args{
+				ip: net.ParseIP("127.0.0.1"),
+			},
+			wantArpa: "1.0.0.127.in-addr.arpa.",
+		},
+		{
+			name: "ok/ipv6",
+			args: args{
+				ip: net.ParseIP("2001:db8::567:89ab"),
+			},
+			wantArpa: "b.a.9.8.7.6.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if gotArpa := reverseAddr(tt.args.ip); gotArpa != tt.wantArpa {
+				t.Errorf("reverseAddr() = %v, want %v", gotArpa, tt.wantArpa)
+			}
+		})
+	}
+}
+
+func Test_serverName(t *testing.T) {
+	type args struct {
+		ch *Challenge
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "ok/dns",
+			args: args{
+				ch: &Challenge{
+					Value: "example.com",
+				},
+			},
+			want: "example.com",
+		},
+		{
+			name: "ok/ipv4",
+			args: args{
+				ch: &Challenge{
+					Value: "127.0.0.1",
+				},
+			},
+			want: "1.0.0.127.in-addr.arpa.",
+		},
+		{
+			name: "ok/ipv4",
+			args: args{
+				ch: &Challenge{
+					Value: "2001:db8::567:89ab",
+				},
+			},
+			want: "b.a.9.8.7.6.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := serverName(tt.args.ch); got != tt.want {
+				t.Errorf("serverName() = %v, want %v", got, tt.want)
+			}
+		})
+	}
 }