Merge pull request #1910 from smallstep/mariano/dns

Do strict DNS lookup on ACME

acme/challenge.go

@@ -110,7 +110,8 @@ func (ch *Challenge) Validate(ctx context.Context, db DB, jwk *jose.JSONWebKey,
 }
 
 func http01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSONWebKey) error {
-	u := &url.URL{Scheme: "http", Host: http01ChallengeHost(ch.Value), Path: fmt.Sprintf("/.well-known/acme-challenge/%s", ch.Token)}
+	u := &url.URL{Scheme: "http", Host: ch.Value, Path: fmt.Sprintf("/.well-known/acme-challenge/%s", ch.Token)}
+	challengeURL := &url.URL{Scheme: "http", Host: http01ChallengeHost(ch.Value), Path: fmt.Sprintf("/.well-known/acme-challenge/%s", ch.Token)}
 
 	// Append insecure port if set.
 	// Only used for testing purposes.
@@ -119,7 +120,7 @@ func http01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSONWeb
 	}
 
 	vc := MustClientFromContext(ctx)
-	resp, err := vc.Get(u.String())
+	resp, err := vc.Get(challengeURL.String())
 	if err != nil {
 		return storeError(ctx, db, ch, false, WrapError(ErrorConnectionType, err,
 			"error doing http GET for url %s", u))
@@ -157,15 +158,40 @@ func http01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSONWeb
 	return nil
 }
 
+// rootedName adds a trailing "." to a given domain name.
+func rootedName(name string) string {
+	if name == "" || name[len(name)-1] != '.' {
+		return name + "."
+	}
+	return name
+}
+
 // http01ChallengeHost checks if a Challenge value is an IPv6 address
 // and adds square brackets if that's the case, so that it can be used
 // as a hostname. Returns the original Challenge value as the host to
 // use in other cases.
 func http01ChallengeHost(value string) string {
-	if ip := net.ParseIP(value); ip != nil && ip.To4() == nil {
-		value = "[" + value + "]"
+	if ip := net.ParseIP(value); ip != nil {
+		if ip.To4() == nil {
+			value = "[" + value + "]"
+		}
+		return value
 	}
-	return value
+	return rootedName(value)
+}
+
+// tlsAlpn01ChallengeHost returns the rooted DNS used on TLS-ALPN-01
+// validations.
+func tlsAlpn01ChallengeHost(name string) string {
+	if ip := net.ParseIP(name); ip != nil {
+		return name
+	}
+	return rootedName(name)
+}
+
+// dns01ChallengeHost returns the TXT record used in DNS-01 validations.
+func dns01ChallengeHost(domain string) string {
+	return "_acme-challenge." + rootedName(domain)
 }
 
 func tlsAlert(err error) uint8 {
@@ -190,13 +216,12 @@ func tlsalpn01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSON
 		InsecureSkipVerify: true, //nolint:gosec // we expect a self-signed challenge certificate
 	}
 
-	var hostPort string
-
 	// Allow to change TLS port for testing purposes.
+	hostPort := tlsAlpn01ChallengeHost(ch.Value)
 	if port := InsecurePortTLSALPN01; port == 0 {
-		hostPort = net.JoinHostPort(ch.Value, "443")
+		hostPort = net.JoinHostPort(hostPort, "443")
 	} else {
-		hostPort = net.JoinHostPort(ch.Value, strconv.Itoa(port))
+		hostPort = net.JoinHostPort(hostPort, strconv.Itoa(port))
 	}
 
 	vc := MustClientFromContext(ctx)
@@ -211,7 +236,7 @@ func tlsalpn01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSON
 				"cannot negotiate ALPN acme-tls/1 protocol for tls-alpn-01 challenge"))
 		}
 		return storeError(ctx, db, ch, false, WrapError(ErrorConnectionType, err,
-			"error doing TLS dial for %s", hostPort))
+			"error doing TLS dial for %s", ch.Value))
 	}
 	defer conn.Close()
 
@@ -307,7 +332,7 @@ func dns01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSONWebK
 	domain := strings.TrimPrefix(ch.Value, "*.")
 
 	vc := MustClientFromContext(ctx)
-	txtRecords, err := vc.LookupTxt("_acme-challenge." + domain)
+	txtRecords, err := vc.LookupTxt(dns01ChallengeHost(domain))
 	if err != nil {
 		return storeError(ctx, db, ch, false, WrapError(ErrorDNSType, err,
 			"error looking up TXT records for domain %s", domain))
@@ -1058,14 +1083,10 @@ func doStepAttestationFormat(_ context.Context, prov Provisioner, ch *Challenge,
 // 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
+	if ip := net.ParseIP(ch.Value); ip != nil {
+		return reverseAddr(ip)
 	}
-	return serverName
+	return ch.Value
 }
 
 // reverseaddr returns the in-addr.arpa. or ip6.arpa. hostname of the IP

acme/challenge_test.go

@@ -646,7 +646,7 @@ func TestChallenge_Validate(t *testing.T) {
 						assert.Equal(t, "zap.internal", updch.Value)
 						assert.Equal(t, StatusPending, updch.Status)
 
-						err := NewError(ErrorConnectionType, "error doing TLS dial for %v:443: force", ch.Value)
+						err := NewError(ErrorConnectionType, "error doing TLS dial for %v: force", ch.Value)
 
 						assert.EqualError(t, updch.Error.Err, err.Err.Error())
 						assert.Equal(t, err.Type, updch.Error.Type)
@@ -1723,7 +1723,7 @@ func TestTLSALPN01Validate(t *testing.T) {
 						assert.Equal(t, ChallengeType("tls-alpn-01"), updch.Type)
 						assert.Equal(t, "zap.internal", updch.Value)
 
-						err := NewError(ErrorConnectionType, "error doing TLS dial for %v:443: force", ch.Value)
+						err := NewError(ErrorConnectionType, "error doing TLS dial for %v: force", ch.Value)
 
 						assert.EqualError(t, updch.Error.Err, err.Err.Error())
 						assert.Equal(t, err.Type, updch.Error.Type)
@@ -1754,7 +1754,7 @@ func TestTLSALPN01Validate(t *testing.T) {
 						assert.Equal(t, ChallengeType("tls-alpn-01"), updch.Type)
 						assert.Equal(t, "zap.internal", updch.Value)
 
-						err := NewError(ErrorConnectionType, "error doing TLS dial for %v:443: force", ch.Value)
+						err := NewError(ErrorConnectionType, "error doing TLS dial for %v: force", ch.Value)
 
 						assert.EqualError(t, updch.Error.Err, err.Err.Error())
 						assert.Equal(t, err.Type, updch.Error.Type)
@@ -1786,7 +1786,7 @@ func TestTLSALPN01Validate(t *testing.T) {
 						assert.Equal(t, ChallengeType("tls-alpn-01"), updch.Type)
 						assert.Equal(t, "zap.internal", updch.Value)
 
-						err := NewError(ErrorConnectionType, "error doing TLS dial for %v:443: context deadline exceeded", ch.Value)
+						err := NewError(ErrorConnectionType, "error doing TLS dial for %v: context deadline exceeded", ch.Value)
 
 						assert.EqualError(t, updch.Error.Err, err.Err.Error())
 						assert.Equal(t, err.Type, updch.Error.Type)
@@ -2774,7 +2774,12 @@ func Test_http01ChallengeHost(t *testing.T) {
 		{
 			name:  "dns",
 			value: "www.example.com",
-			want:  "www.example.com",
+			want:  "www.example.com.",
+		},
+		{
+			name:  "rooted dns",
+			value: "www.example.com.",
+			want:  "www.example.com.",
 		},
 		{
 			name:  "ipv4",
@@ -4301,3 +4306,43 @@ func createSubjectAltNameExtension(dnsNames, emailAddresses x509util.MultiString
 		Value:    rawBytes,
 	}, nil
 }
+
+func Test_tlsAlpn01ChallengeHost(t *testing.T) {
+	type args struct {
+		name string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{"dns", args{"smallstep.com"}, "smallstep.com."},
+		{"rooted dns", args{"smallstep.com."}, "smallstep.com."},
+		{"ipv4", args{"1.2.3.4"}, "1.2.3.4"},
+		{"ipv6", args{"2607:f8b0:4023:1009::71"}, "2607:f8b0:4023:1009::71"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, tlsAlpn01ChallengeHost(tt.args.name))
+		})
+	}
+}
+
+func Test_dns01ChallengeHost(t *testing.T) {
+	type args struct {
+		domain string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{"dns", args{"smallstep.com"}, "_acme-challenge.smallstep.com."},
+		{"rooted dns", args{"smallstep.com."}, "_acme-challenge.smallstep.com."},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, dns01ChallengeHost(tt.args.domain))
+		})
+	}
+}

go.mod

@@ -27,7 +27,7 @@ require (
 	github.com/slackhq/nebula v1.6.1
 	github.com/smallstep/assert v0.0.0-20200723003110-82e2b9b3b262
 	github.com/smallstep/go-attestation v0.4.4-0.20240109183208-413678f90935
-	github.com/smallstep/nosql v0.6.1
+	github.com/smallstep/nosql v0.7.0
 	github.com/smallstep/pkcs7 v0.0.0-20231024181729-3b98ecc1ca81
 	github.com/smallstep/scep v0.0.0-20231024192529-aee96d7ad34d
 	github.com/stretchr/testify v1.9.0
@@ -93,7 +93,7 @@ require (
 	github.com/go-logr/logr v1.4.1 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/go-piv/piv-go v1.11.0 // indirect
-	github.com/go-sql-driver/mysql v1.7.1 // indirect
+	github.com/go-sql-driver/mysql v1.8.1 // indirect
 	github.com/golang-jwt/jwt/v5 v5.2.1 // indirect
 	github.com/golang/glog v1.2.0 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
@@ -116,14 +116,10 @@ require (
 	github.com/hashicorp/hcl v1.0.0 // indirect
 	github.com/huandu/xstrings v1.3.3 // indirect
 	github.com/imdario/mergo v0.3.12 // indirect
-	github.com/jackc/chunkreader/v2 v2.0.1 // indirect
-	github.com/jackc/pgconn v1.14.3 // indirect
-	github.com/jackc/pgio v1.0.0 // indirect
 	github.com/jackc/pgpassfile v1.0.0 // indirect
-	github.com/jackc/pgproto3/v2 v2.3.3 // indirect
 	github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a // indirect
-	github.com/jackc/pgtype v1.14.0 // indirect
-	github.com/jackc/pgx/v4 v4.18.3 // indirect
+	github.com/jackc/pgx/v5 v5.6.0 // indirect
+	github.com/jackc/puddle/v2 v2.2.1 // indirect
 	github.com/klauspost/compress v1.16.3 // indirect
 	github.com/kylelemons/godebug v1.1.0 // indirect
 	github.com/manifoldco/promptui v0.9.0 // indirect
@@ -149,7 +145,7 @@ require (
 	github.com/spf13/cast v1.4.1 // indirect
 	github.com/thales-e-security/pool v0.0.2 // indirect
 	github.com/x448/float16 v0.8.4 // indirect
-	go.etcd.io/bbolt v1.3.9 // indirect
+	go.etcd.io/bbolt v1.3.10 // indirect
 	go.opencensus.io v0.24.0 // indirect
 	go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0 // indirect
 	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.49.0 // indirect