From d9a3cc6ce8e9e8c958b47f6bad28c7f0634d4b50 Mon Sep 17 00:00:00 2001 From: max furman Date: Thu, 24 Oct 2024 21:20:29 -0700 Subject: [PATCH] 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. --- authority/provisioner/aws.go | 2 +- authority/provisioner/aws_test.go | 2 +- authority/provisioner/azure.go | 2 +- authority/provisioner/azure_test.go | 2 +- authority/provisioner/gcp.go | 2 +- authority/provisioner/gcp_test.go | 2 +- authority/provisioner/sign_options.go | 21 ++++++++++++++ authority/provisioner/sign_options_test.go | 33 ++++++++++++++++++++++ 8 files changed, 60 insertions(+), 6 deletions(-) diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go index e95feedd4..9e1d9b7c6 100644 --- 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 index 5af0ec673..eb57a57cb 100644 --- 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 index b930f89f1..0a86b6490 100644 --- 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 index f262ffbca..c0438231f 100644 --- 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 index eca9ae39f..c63a29cf7 100644 --- 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 index 5f5587a44..2841f2e8e 100644 --- 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 index b7cf0dbca..8057e20bf 100644 --- 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 index 4663c3343..71d1ebfeb 100644 --- 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)