Skip to content

Commit

Permalink
Use dnsNamesSubsetValidator for IID provisioners
Browse files Browse the repository at this point in the history
... 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.
  • Loading branch information
dopey committed Oct 25, 2024
1 parent 8d0b4d7 commit d9a3cc6
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 6 deletions.
2 changes: 1 addition & 1 deletion authority/provisioner/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}),
Expand Down
2 changes: 1 addition & 1 deletion authority/provisioner/aws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion authority/provisioner/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion authority/provisioner/azure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion authority/provisioner/gcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion authority/provisioner/gcp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
21 changes: 21 additions & 0 deletions authority/provisioner/sign_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
33 changes: 33 additions & 0 deletions authority/provisioner/sign_options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit d9a3cc6

Please sign in to comment.