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.
smallstep · Oct 25, 2024 · 3c29288 · 3c29288
1 parent 8d0b4d7
commit 3c29288
Show file tree

Hide file tree

Showing 9 changed files with 62 additions and 6 deletions.
diff --git a/authority/authority.go b/authority/authority.go
@@ -151,6 +151,8 @@ func New(cfg *config.Config, opts ...Option) (*Authority, error) {
 		}
 	}
 
+	GlobalVersion.RequireClientAuthentication = true
+
 	return a, nil
 }
 

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 names SAN of a certificate request.
+type dnsNamesSubsetValidator []string
+
+// Valid checks that all DNS Names 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]bool)
+	for _, s := range v {
+		allowed[s] = true
+	}
+	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)