Use dnsNamesSubsetValidator for IID provisioners (#2044)

* Use dnsNamesSubsetValidator for IID provisioners ... when disableCustomSANs is set to 'true'. The DNS names in the certificate request must be a subset of the authorized set of DNS names (from the IID token). The previous functionality required that the DNS names in the certificate request exactly matched the authorized DNS names. * Update authority/provisioner/sign_options.go Co-authored-by: Herman Slatman <hslatman@users.noreply.github.com> * Update authority/provisioner/sign_options.go Co-authored-by: Herman Slatman <hslatman@users.noreply.github.com> * Use map[string]struct rather than map[string]bool for clarity --------- Co-authored-by: Herman Slatman <hslatman@users.noreply.github.com>
smallstep · Oct 25, 2024 · 88443dd · 88443dd
1 parent 946bba2
commit 88443dd
Show file tree

Hide file tree

Showing 8 changed files with 60 additions and 6 deletions.
diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go
@@ -358,7 +358,7 @@ func (p *AWS) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 	if p.DisableCustomSANs {
 		dnsName := fmt.Sprintf("ip-%s.%s.compute.internal", strings.ReplaceAll(doc.PrivateIP, ".", "-"), doc.Region)
 		so = append(so,
-			dnsNamesValidator([]string{dnsName}),
+			dnsNamesSubsetValidator([]string{dnsName}),
 			ipAddressesValidator([]net.IP{
 				net.ParseIP(doc.PrivateIP),
 			}),

diff --git a/authority/provisioner/aws_test.go b/authority/provisioner/aws_test.go
@@ -698,7 +698,7 @@ func TestAWS_AuthorizeSign(t *testing.T) {
 					case *urisValidator:
 						assert.Equals(t, v.uris, nil)
 						assert.Equals(t, MethodFromContext(v.ctx), SignMethod)
-					case dnsNamesValidator:
+					case dnsNamesSubsetValidator:
 						assert.Equals(t, []string(v), []string{"ip-127-0-0-1.us-west-1.compute.internal"})
 					case *x509NamePolicyValidator:
 						assert.Equals(t, nil, v.policyEngine)

diff --git a/authority/provisioner/azure.go b/authority/provisioner/azure.go
@@ -379,7 +379,7 @@ func (p *Azure) AuthorizeSign(ctx context.Context, token string) ([]SignOption,
 		// name will work only inside the virtual network
 		so = append(so,
 			commonNameValidator(name),
-			dnsNamesValidator([]string{name}),
+			dnsNamesSubsetValidator([]string{name}),
 			ipAddressesValidator(nil),
 			emailAddressesValidator(nil),
 			newURIsValidator(ctx, nil),

diff --git a/authority/provisioner/azure_test.go b/authority/provisioner/azure_test.go
@@ -563,7 +563,7 @@ func TestAzure_AuthorizeSign(t *testing.T) {
 					case *urisValidator:
 						assert.Equals(t, v.uris, nil)
 						assert.Equals(t, MethodFromContext(v.ctx), SignMethod)
-					case dnsNamesValidator:
+					case dnsNamesSubsetValidator:
 						assert.Equals(t, []string(v), []string{"virtualMachine"})
 					case *x509NamePolicyValidator:
 						assert.Equals(t, nil, v.policyEngine)

diff --git a/authority/provisioner/gcp.go b/authority/provisioner/gcp.go
@@ -265,7 +265,7 @@ func (p *GCP) AuthorizeSign(ctx context.Context, token string) ([]SignOption, er
 			commonNameSliceValidator([]string{
 				ce.InstanceName, ce.InstanceID, dnsName1, dnsName2,
 			}),
-			dnsNamesValidator([]string{
+			dnsNamesSubsetValidator([]string{
 				dnsName1, dnsName2,
 			}),
 			ipAddressesValidator(nil),

diff --git a/authority/provisioner/gcp_test.go b/authority/provisioner/gcp_test.go
@@ -579,7 +579,7 @@ func TestGCP_AuthorizeSign(t *testing.T) {
 					case *urisValidator:
 						assert.Equals(t, v.uris, nil)
 						assert.Equals(t, MethodFromContext(v.ctx), SignMethod)
-					case dnsNamesValidator:
+					case dnsNamesSubsetValidator:
 						assert.Equals(t, []string(v), []string{"instance-name.c.project-id.internal", "instance-name.zone.c.project-id.internal"})
 					case *x509NamePolicyValidator:
 						assert.Equals(t, nil, v.policyEngine)

diff --git a/authority/provisioner/sign_options.go b/authority/provisioner/sign_options.go
@@ -190,6 +190,27 @@ func (v dnsNamesValidator) Valid(req *x509.CertificateRequest) error {
 	return nil
 }
 
+// dnsNamesSubsetValidator validates the DNS name SANs of a certificate request.
+type dnsNamesSubsetValidator []string
+
+// Valid checks that all DNS name SANs in the certificate request are present in
+// the allowed list of DNS names.
+func (v dnsNamesSubsetValidator) Valid(req *x509.CertificateRequest) error {
+	if len(req.DNSNames) == 0 {
+		return nil
+	}
+	allowed := make(map[string]struct{}, len(v))
+	for _, s := range v {
+		allowed[s] = struct{}{}
+	}
+	for _, s := range req.DNSNames {
+		if _, ok := allowed[s]; !ok {
+			return errs.Forbidden("certificate request contains unauthorized DNS names - got %v, allowed %v", req.DNSNames, v)
+		}
+	}
+	return nil
+}
+
 // ipAddressesValidator validates the IP addresses SAN of a certificate request.
 type ipAddressesValidator []net.IP
 

diff --git a/authority/provisioner/sign_options_test.go b/authority/provisioner/sign_options_test.go
@@ -186,6 +186,39 @@ func Test_dnsNamesValidator_Valid(t *testing.T) {
 	}
 }
 
+func Test_dnsNamesSubsetValidator_Valid(t *testing.T) {
+	type args struct {
+		req *x509.CertificateRequest
+	}
+	tests := []struct {
+		name    string
+		v       dnsNamesSubsetValidator
+		args    args
+		wantErr bool
+	}{
+		{"ok0", []string{}, args{&x509.CertificateRequest{DNSNames: []string{}}}, false},
+		{"ok1", []string{"foo.bar.zar"}, args{&x509.CertificateRequest{DNSNames: []string{"foo.bar.zar"}}}, false},
+		{"ok2", []string{"foo.bar.zar", "bar.zar"}, args{&x509.CertificateRequest{DNSNames: []string{"foo.bar.zar", "bar.zar"}}}, false},
+		{"ok3", []string{"foo.bar.zar", "bar.zar"}, args{&x509.CertificateRequest{DNSNames: []string{"bar.zar", "foo.bar.zar"}}}, false},
+		{"ok4", []string{"foo.bar.zar", "bar.zar"}, args{&x509.CertificateRequest{}}, false},
+		{"ok5", []string{"foo.bar.zar", "bar.zar"}, args{&x509.CertificateRequest{DNSNames: []string{"bar.zar"}}}, false},
+		{"ok6", []string{"foo", "bar", "baz", "zar", "zap"}, args{&x509.CertificateRequest{DNSNames: []string{"zap", "baz", "foo"}}}, false},
+		{"fail1", []string{"foo.bar.zar"}, args{&x509.CertificateRequest{DNSNames: []string{"bar.zar"}}}, true},
+		{"fail2", []string{"foo.bar.zar"}, args{&x509.CertificateRequest{DNSNames: []string{"bar.zar", "foo.bar.zar"}}}, true},
+		{"fail3", []string{"foo.bar.zar", "bar.zar"}, args{&x509.CertificateRequest{DNSNames: []string{"foo.bar.zar", "zar.bar"}}}, true},
+		{"fail4", []string{"foo", "bar", "baz", "zar", "zap"}, args{&x509.CertificateRequest{DNSNames: []string{"zap", "baz", "foO"}}}, true},
+		{"fail5", []string{"foo", "bar", "baz", "zar", "zap"}, args{&x509.CertificateRequest{DNSNames: []string{"zap", "baz", "fax", "foo"}}}, true},
+		{"fail6", []string{}, args{&x509.CertificateRequest{DNSNames: []string{"zap", "baz", "fax", "foo"}}}, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := tt.v.Valid(tt.args.req); (err != nil) != tt.wantErr {
+				t.Errorf("dnsNamesSubsetValidator.Valid() error = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
 func Test_ipAddressesValidator_Valid(t *testing.T) {
 	ip1 := net.IPv4(10, 3, 2, 1)
 	ip2 := net.IPv4(10, 3, 2, 2)