From 72bbe533763be336153d5be970e5f0cec6a70d19 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Tue, 19 Apr 2022 14:41:36 +0200 Subject: [PATCH] Add additional policy options --- authority/admin/api/policy.go | 2 +- authority/admin/api/policy_test.go | 2 + authority/policy.go | 135 ++++++++++++++------------ authority/policy/options_test.go | 93 ++++++++++++++++++ authority/policy_test.go | 35 ++++++- authority/provisioner/options_test.go | 88 +++++++++++++++++ 6 files changed, 291 insertions(+), 64 deletions(-) create mode 100644 authority/policy/options_test.go diff --git a/authority/admin/api/policy.go b/authority/admin/api/policy.go index d04147b3..fc6ab1d9 100644 --- a/authority/admin/api/policy.go +++ b/authority/admin/api/policy.go @@ -390,7 +390,7 @@ func applyConditionalDefaults(p *linkedca.Policy) { if p.GetX509() == nil { return } - if p.GetX509().VerifySubjectCommonName == nil { + if p.GetX509().GetVerifySubjectCommonName() == nil { p.X509.VerifySubjectCommonName = &wrapperspb.BoolValue{Value: true} } } diff --git a/authority/admin/api/policy_test.go b/authority/admin/api/policy_test.go index 41fe05ae..e3cc6b65 100644 --- a/authority/admin/api/policy_test.go +++ b/authority/admin/api/policy_test.go @@ -286,6 +286,7 @@ func TestPolicyAdminResponder_CreateAuthorityPolicy(t *testing.T) { Allow: &linkedca.X509Names{ Dns: []string{"*.local"}, }, + VerifySubjectCommonName: &wrapperspb.BoolValue{Value: true}, }, } body, err := protojson.Marshal(policy) @@ -971,6 +972,7 @@ func TestPolicyAdminResponder_CreateProvisionerPolicy(t *testing.T) { Allow: &linkedca.X509Names{ Dns: []string{"*.local"}, }, + VerifySubjectCommonName: &wrapperspb.BoolValue{Value: true}, }, } body, err := protojson.Marshal(policy) diff --git a/authority/policy.go b/authority/policy.go index b7d5e4ec..e626eaed 100644 --- a/authority/policy.go +++ b/authority/policy.go @@ -243,97 +243,108 @@ func policyToCertificates(p *linkedca.Policy) *authPolicy.Options { return nil } - // prepare full policy struct - opts := &authPolicy.Options{ - X509: &authPolicy.X509PolicyOptions{ - AllowedNames: &authPolicy.X509NameOptions{}, - DeniedNames: &authPolicy.X509NameOptions{}, - }, - SSH: &authPolicy.SSHPolicyOptions{ - Host: &authPolicy.SSHHostCertificateOptions{ - AllowedNames: &authPolicy.SSHNameOptions{}, - DeniedNames: &authPolicy.SSHNameOptions{}, - }, - User: &authPolicy.SSHUserCertificateOptions{ - AllowedNames: &authPolicy.SSHNameOptions{}, - DeniedNames: &authPolicy.SSHNameOptions{}, - }, - }, + // return early if x509 nor SSH is set + if p.GetX509() == nil && p.GetSsh() == nil { + return nil } + opts := &authPolicy.Options{} + // fill x509 policy configuration - if p.X509 != nil { - if p.X509.Allow != nil { - if p.X509.Allow.Dns != nil { - opts.X509.AllowedNames.DNSDomains = p.X509.Allow.Dns + if p.GetX509() != nil { + opts.X509 = &authPolicy.X509PolicyOptions{} + if p.GetX509().GetAllow() != nil { + opts.X509.AllowedNames = &authPolicy.X509NameOptions{} + allow := p.GetX509().GetAllow() + if allow.Dns != nil { + opts.X509.AllowedNames.DNSDomains = allow.Dns } - if p.X509.Allow.Ips != nil { - opts.X509.AllowedNames.IPRanges = p.X509.Allow.Ips + if allow.Ips != nil { + opts.X509.AllowedNames.IPRanges = allow.Ips } - if p.X509.Allow.Emails != nil { - opts.X509.AllowedNames.EmailAddresses = p.X509.Allow.Emails + if allow.Emails != nil { + opts.X509.AllowedNames.EmailAddresses = allow.Emails } - if p.X509.Allow.Uris != nil { - opts.X509.AllowedNames.URIDomains = p.X509.Allow.Uris + if allow.Uris != nil { + opts.X509.AllowedNames.URIDomains = allow.Uris } } - if p.X509.Deny != nil { - if p.X509.Deny.Dns != nil { - opts.X509.DeniedNames.DNSDomains = p.X509.Deny.Dns + if p.GetX509().GetDeny() != nil { + opts.X509.DeniedNames = &authPolicy.X509NameOptions{} + deny := p.GetX509().GetDeny() + if deny.Dns != nil { + opts.X509.DeniedNames.DNSDomains = deny.Dns } - if p.X509.Deny.Ips != nil { - opts.X509.DeniedNames.IPRanges = p.X509.Deny.Ips + if deny.Ips != nil { + opts.X509.DeniedNames.IPRanges = deny.Ips } - if p.X509.Deny.Emails != nil { - opts.X509.DeniedNames.EmailAddresses = p.X509.Deny.Emails + if deny.Emails != nil { + opts.X509.DeniedNames.EmailAddresses = deny.Emails } - if p.X509.Deny.Uris != nil { - opts.X509.DeniedNames.URIDomains = p.X509.Deny.Uris + if deny.Uris != nil { + opts.X509.DeniedNames.URIDomains = deny.Uris } } + if p.GetX509().GetAllowWildcardLiteral() != nil { + opts.X509.AllowWildcardLiteral = &p.GetX509().GetAllowWildcardLiteral().Value + } + if p.GetX509().GetVerifySubjectCommonName() != nil { + opts.X509.VerifySubjectCommonName = &p.GetX509().VerifySubjectCommonName.Value + } } // fill ssh policy configuration - if p.Ssh != nil { - if p.Ssh.Host != nil { - if p.Ssh.Host.Allow != nil { - if p.Ssh.Host.Allow.Dns != nil { - opts.SSH.Host.AllowedNames.DNSDomains = p.Ssh.Host.Allow.Dns + if p.GetSsh() != nil { + opts.SSH = &authPolicy.SSHPolicyOptions{} + if p.GetSsh().GetHost() != nil { + opts.SSH.Host = &authPolicy.SSHHostCertificateOptions{} + if p.GetSsh().GetHost().GetAllow() != nil { + opts.SSH.Host.AllowedNames = &authPolicy.SSHNameOptions{} + allow := p.GetSsh().GetHost().GetAllow() + if allow.Dns != nil { + opts.SSH.Host.AllowedNames.DNSDomains = allow.Dns } - if p.Ssh.Host.Allow.Ips != nil { - opts.SSH.Host.AllowedNames.IPRanges = p.Ssh.Host.Allow.Ips + if allow.Ips != nil { + opts.SSH.Host.AllowedNames.IPRanges = allow.Ips } - if p.Ssh.Host.Allow.Principals != nil { - opts.SSH.Host.AllowedNames.Principals = p.Ssh.Host.Allow.Principals + if allow.Principals != nil { + opts.SSH.Host.AllowedNames.Principals = allow.Principals } } - if p.Ssh.Host.Deny != nil { - if p.Ssh.Host.Deny.Dns != nil { - opts.SSH.Host.DeniedNames.DNSDomains = p.Ssh.Host.Deny.Dns + if p.GetSsh().GetHost().GetDeny() != nil { + opts.SSH.Host.DeniedNames = &authPolicy.SSHNameOptions{} + deny := p.GetSsh().GetHost().GetDeny() + if deny.Dns != nil { + opts.SSH.Host.DeniedNames.DNSDomains = deny.Dns } - if p.Ssh.Host.Deny.Ips != nil { - opts.SSH.Host.DeniedNames.IPRanges = p.Ssh.Host.Deny.Ips + if deny.Ips != nil { + opts.SSH.Host.DeniedNames.IPRanges = deny.Ips } - if p.Ssh.Host.Deny.Principals != nil { - opts.SSH.Host.DeniedNames.Principals = p.Ssh.Host.Deny.Principals + if deny.Principals != nil { + opts.SSH.Host.DeniedNames.Principals = deny.Principals } } } - if p.Ssh.User != nil { - if p.Ssh.User.Allow != nil { - if p.Ssh.User.Allow.Emails != nil { - opts.SSH.User.AllowedNames.EmailAddresses = p.Ssh.User.Allow.Emails + if p.GetSsh().GetUser() != nil { + opts.SSH.User = &authPolicy.SSHUserCertificateOptions{} + if p.GetSsh().GetUser().GetAllow() != nil { + opts.SSH.User.AllowedNames = &authPolicy.SSHNameOptions{} + allow := p.GetSsh().GetUser().GetAllow() + if allow.Emails != nil { + opts.SSH.User.AllowedNames.EmailAddresses = allow.Emails } - if p.Ssh.User.Allow.Principals != nil { - opts.SSH.User.AllowedNames.Principals = p.Ssh.User.Allow.Principals + if allow.Principals != nil { + opts.SSH.User.AllowedNames.Principals = allow.Principals } } - if p.Ssh.User.Deny != nil { - if p.Ssh.User.Deny.Emails != nil { - opts.SSH.User.DeniedNames.EmailAddresses = p.Ssh.User.Deny.Emails + if p.GetSsh().GetUser().GetDeny() != nil { + opts.SSH.User.DeniedNames = &authPolicy.SSHNameOptions{} + deny := p.GetSsh().GetUser().GetDeny() + if deny.Emails != nil { + opts.SSH.User.DeniedNames.EmailAddresses = deny.Emails } - if p.Ssh.User.Deny.Principals != nil { - opts.SSH.User.DeniedNames.Principals = p.Ssh.User.Deny.Principals + if deny.Principals != nil { + opts.SSH.User.DeniedNames.Principals = deny.Principals } } } diff --git a/authority/policy/options_test.go b/authority/policy/options_test.go new file mode 100644 index 00000000..49330f08 --- /dev/null +++ b/authority/policy/options_test.go @@ -0,0 +1,93 @@ +package policy + +import ( + "testing" +) + +func TestX509PolicyOptions_IsWildcardLiteralAllowed(t *testing.T) { + trueValue := true + falseValue := false + tests := []struct { + name string + options *X509PolicyOptions + want bool + }{ + { + name: "nil-options", + options: nil, + want: true, + }, + { + name: "nil", + options: &X509PolicyOptions{ + AllowWildcardLiteral: nil, + }, + want: false, + }, + { + name: "set-true", + options: &X509PolicyOptions{ + AllowWildcardLiteral: &trueValue, + }, + want: true, + }, + { + name: "set-false", + options: &X509PolicyOptions{ + AllowWildcardLiteral: &falseValue, + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.options.IsWildcardLiteralAllowed(); got != tt.want { + t.Errorf("X509PolicyOptions.IsWildcardLiteralAllowed() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestX509PolicyOptions_ShouldVerifySubjectCommonName(t *testing.T) { + trueValue := true + falseValue := false + tests := []struct { + name string + options *X509PolicyOptions + want bool + }{ + { + name: "nil-options", + options: nil, + want: false, + }, + { + name: "nil", + options: &X509PolicyOptions{ + VerifySubjectCommonName: nil, + }, + want: true, + }, + { + name: "set-true", + options: &X509PolicyOptions{ + VerifySubjectCommonName: &trueValue, + }, + want: true, + }, + { + name: "set-false", + options: &X509PolicyOptions{ + VerifySubjectCommonName: &falseValue, + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.options.ShouldVerifySubjectCommonName(); got != tt.want { + t.Errorf("X509PolicyOptions.ShouldVerifySubjectCommonName() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/authority/policy_test.go b/authority/policy_test.go index 38132a7c..24fe5840 100644 --- a/authority/policy_test.go +++ b/authority/policy_test.go @@ -7,6 +7,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" + "google.golang.org/protobuf/types/known/wrapperspb" "go.step.sm/linkedca" @@ -190,16 +191,44 @@ func TestAuthority_checkPolicy(t *testing.T) { } func Test_policyToCertificates(t *testing.T) { + trueValue := true + falseValue := false tests := []struct { name string policy *linkedca.Policy want *authPolicy.Options }{ { - name: "no-policy", + name: "nil", policy: nil, want: nil, }, + { + name: "no-policy", + policy: &linkedca.Policy{}, + want: nil, + }, + { + name: "partial-policy", + policy: &linkedca.Policy{ + X509: &linkedca.X509Policy{ + Allow: &linkedca.X509Names{ + Dns: []string{"*.local"}, + }, + AllowWildcardLiteral: &wrapperspb.BoolValue{Value: false}, + VerifySubjectCommonName: &wrapperspb.BoolValue{Value: true}, + }, + }, + want: &authPolicy.Options{ + X509: &authPolicy.X509PolicyOptions{ + AllowedNames: &authPolicy.X509NameOptions{ + DNSDomains: []string{"*.local"}, + }, + AllowWildcardLiteral: &falseValue, + VerifySubjectCommonName: &trueValue, + }, + }, + }, { name: "full-policy", policy: &linkedca.Policy{ @@ -216,6 +245,8 @@ func Test_policyToCertificates(t *testing.T) { Emails: []string{"badhost.example.com"}, Uris: []string{"https://badhost.local"}, }, + AllowWildcardLiteral: &wrapperspb.BoolValue{Value: true}, + VerifySubjectCommonName: &wrapperspb.BoolValue{Value: true}, }, Ssh: &linkedca.SSHPolicy{ Host: &linkedca.SSHHostPolicy{ @@ -256,6 +287,8 @@ func Test_policyToCertificates(t *testing.T) { EmailAddresses: []string{"badhost.example.com"}, URIDomains: []string{"https://badhost.local"}, }, + AllowWildcardLiteral: &trueValue, + VerifySubjectCommonName: &trueValue, }, SSH: &authPolicy.SSHPolicyOptions{ Host: &authPolicy.SSHHostCertificateOptions{ diff --git a/authority/provisioner/options_test.go b/authority/provisioner/options_test.go index 8f411aca..32bea92b 100644 --- a/authority/provisioner/options_test.go +++ b/authority/provisioner/options_test.go @@ -287,3 +287,91 @@ func Test_unsafeParseSigned(t *testing.T) { }) } } + +func TestX509Options_IsWildcardLiteralAllowed(t *testing.T) { + trueValue := true + falseValue := false + tests := []struct { + name string + options *X509Options + want bool + }{ + { + name: "nil-options", + options: nil, + want: true, + }, + { + name: "nil", + options: &X509Options{ + AllowWildcardLiteral: nil, + }, + want: false, + }, + { + name: "set-true", + options: &X509Options{ + AllowWildcardLiteral: &trueValue, + }, + want: true, + }, + { + name: "set-false", + options: &X509Options{ + AllowWildcardLiteral: &falseValue, + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.options.IsWildcardLiteralAllowed(); got != tt.want { + t.Errorf("X509PolicyOptions.IsWildcardLiteralAllowed() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestX509Options_ShouldVerifySubjectCommonName(t *testing.T) { + trueValue := true + falseValue := false + tests := []struct { + name string + options *X509Options + want bool + }{ + { + name: "nil-options", + options: nil, + want: false, + }, + { + name: "nil", + options: &X509Options{ + VerifySubjectCommonName: nil, + }, + want: true, + }, + { + name: "set-true", + options: &X509Options{ + VerifySubjectCommonName: &trueValue, + }, + want: true, + }, + { + name: "set-false", + options: &X509Options{ + VerifySubjectCommonName: &falseValue, + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.options.ShouldVerifySubjectCommonName(); got != tt.want { + t.Errorf("X509PolicyOptions.ShouldVerifySubjectCommonName() = %v, want %v", got, tt.want) + } + }) + } +}