Skip to content

Commit 3fa96eb

Browse files
hslatmanhslatman
committedApr 24, 2022
Improve policy errors returned to client
1 parent a3c5188 commit 3fa96eb

File tree

4 files changed

+161
-35
lines changed

4 files changed

+161
-35
lines changed
 

‎authority/ssh.go

+19-3
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,17 @@ import (
1010
"strings"
1111
"time"
1212

13+
"golang.org/x/crypto/ssh"
14+
15+
"go.step.sm/crypto/randutil"
16+
"go.step.sm/crypto/sshutil"
17+
1318
"github.com/smallstep/certificates/authority/config"
1419
"github.com/smallstep/certificates/authority/provisioner"
1520
"github.com/smallstep/certificates/db"
1621
"github.com/smallstep/certificates/errs"
22+
policy "github.com/smallstep/certificates/policy"
1723
"github.com/smallstep/certificates/templates"
18-
"go.step.sm/crypto/randutil"
19-
"go.step.sm/crypto/sshutil"
20-
"golang.org/x/crypto/ssh"
2124
)
2225

2326
const (
@@ -252,6 +255,12 @@ func (a *Authority) SignSSH(ctx context.Context, key ssh.PublicKey, opts provisi
252255
if a.sshUserPolicy != nil {
253256
allowed, err := a.sshUserPolicy.IsSSHCertificateAllowed(certTpl)
254257
if err != nil {
258+
var pe *policy.NamePolicyError
259+
if errors.As(err, &pe) && pe.Reason == policy.NotAuthorizedForThisName {
260+
return nil, errs.ApplyOptions(
261+
errs.ForbiddenErr(errors.New("authority not allowed to sign"), "authority.SignSSH: %s", err.Error()),
262+
)
263+
}
255264
return nil, errs.InternalServerErr(err,
256265
errs.WithMessage("authority.SignSSH: error creating ssh user certificate"),
257266
)
@@ -269,6 +278,13 @@ func (a *Authority) SignSSH(ctx context.Context, key ssh.PublicKey, opts provisi
269278
if a.sshHostPolicy != nil {
270279
allowed, err := a.sshHostPolicy.IsSSHCertificateAllowed(certTpl)
271280
if err != nil {
281+
var pe *policy.NamePolicyError
282+
if errors.As(err, &pe) && pe.Reason == policy.NotAuthorizedForThisName {
283+
return nil, errs.ApplyOptions(
284+
// TODO: show which names were not allowed; they are in the err
285+
errs.ForbiddenErr(errors.New("authority not allowed to sign"), "authority.SignSSH: %s", err.Error()),
286+
)
287+
}
272288
return nil, errs.InternalServerErr(err,
273289
errs.WithMessage("authority.SignSSH: error creating ssh host certificate"),
274290
)

‎authority/ssh_test.go

+65-27
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020

2121
"github.com/smallstep/assert"
2222
"github.com/smallstep/certificates/api/render"
23+
"github.com/smallstep/certificates/authority/policy"
2324
"github.com/smallstep/certificates/authority/provisioner"
2425
"github.com/smallstep/certificates/db"
2526
"github.com/smallstep/certificates/templates"
@@ -159,6 +160,14 @@ func TestAuthority_SignSSH(t *testing.T) {
159160
assert.FatalError(t, err)
160161
hostTemplateWithHosts, err := provisioner.TemplateSSHOptions(nil, sshutil.CreateTemplateData(sshutil.HostCert, "key-id", []string{"foo.test.com", "bar.test.com"}))
161162
assert.FatalError(t, err)
163+
userTemplateWithRoot, err := provisioner.TemplateSSHOptions(nil, sshutil.CreateTemplateData(sshutil.UserCert, "key-id", []string{"root"}))
164+
assert.FatalError(t, err)
165+
hostTemplateWithExampleDotCom, err := provisioner.TemplateSSHOptions(nil, sshutil.CreateTemplateData(sshutil.HostCert, "key-id", []string{"example.com"}))
166+
assert.FatalError(t, err)
167+
badUserTemplate, err := provisioner.TemplateSSHOptions(nil, sshutil.CreateTemplateData(sshutil.UserCert, "key-id", []string{"127.0.0.1"}))
168+
assert.FatalError(t, err)
169+
badHostTemplate, err := provisioner.TemplateSSHOptions(nil, sshutil.CreateTemplateData(sshutil.HostCert, "key-id", []string{"host...local"}))
170+
assert.FatalError(t, err)
162171
userCustomTemplate, err := provisioner.TemplateSSHOptions(&provisioner.Options{
163172
SSH: &provisioner.SSHOptions{Template: `{
164173
"type": "{{ .Type }}",
@@ -182,11 +191,30 @@ func TestAuthority_SignSSH(t *testing.T) {
182191
}, sshutil.CreateTemplateData(sshutil.UserCert, "key-id", []string{"user"}))
183192
assert.FatalError(t, err)
184193

194+
policyOptions := &policy.SSHPolicyOptions{
195+
User: &policy.SSHUserCertificateOptions{
196+
AllowedNames: &policy.SSHNameOptions{
197+
Principals: []string{"user"},
198+
},
199+
},
200+
Host: &policy.SSHHostCertificateOptions{
201+
AllowedNames: &policy.SSHNameOptions{
202+
DNSDomains: []string{"*.test.com"},
203+
},
204+
},
205+
}
206+
userPolicy, err := policy.NewSSHUserPolicyEngine(policyOptions)
207+
assert.FatalError(t, err)
208+
hostPolicy, err := policy.NewSSHHostPolicyEngine(policyOptions)
209+
assert.FatalError(t, err)
210+
185211
now := time.Now()
186212

187213
type fields struct {
188214
sshCAUserCertSignKey ssh.Signer
189215
sshCAHostCertSignKey ssh.Signer
216+
sshUserPolicy policy.UserPolicy
217+
sshHostPolicy policy.HostPolicy
190218
}
191219
type args struct {
192220
key ssh.PublicKey
@@ -206,39 +234,49 @@ func TestAuthority_SignSSH(t *testing.T) {
206234
want want
207235
wantErr bool
208236
}{
209-
{"ok-user", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions}}, want{CertType: ssh.UserCert}, false},
210-
{"ok-host", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{hostTemplate, hostOptions}}, want{CertType: ssh.HostCert}, false},
211-
{"ok-user-only", fields{signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions}}, want{CertType: ssh.UserCert}, false},
212-
{"ok-host-only", fields{nil, signer}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{hostTemplate, hostOptions}}, want{CertType: ssh.HostCert}, false},
213-
{"ok-opts-type-user", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{CertType: "user"}, []provisioner.SignOption{userTemplate}}, want{CertType: ssh.UserCert}, false},
214-
{"ok-opts-type-host", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{CertType: "host"}, []provisioner.SignOption{hostTemplate}}, want{CertType: ssh.HostCert}, false},
215-
{"ok-opts-principals", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{CertType: "user", Principals: []string{"user"}}, []provisioner.SignOption{userTemplateWithUser}}, want{CertType: ssh.UserCert, Principals: []string{"user"}}, false},
216-
{"ok-opts-principals", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{CertType: "host", Principals: []string{"foo.test.com", "bar.test.com"}}, []provisioner.SignOption{hostTemplateWithHosts}}, want{CertType: ssh.HostCert, Principals: []string{"foo.test.com", "bar.test.com"}}, false},
217-
{"ok-opts-valid-after", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{CertType: "user", ValidAfter: provisioner.NewTimeDuration(now)}, []provisioner.SignOption{userTemplate}}, want{CertType: ssh.UserCert, ValidAfter: uint64(now.Unix())}, false},
218-
{"ok-opts-valid-before", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{CertType: "host", ValidBefore: provisioner.NewTimeDuration(now)}, []provisioner.SignOption{hostTemplate}}, want{CertType: ssh.HostCert, ValidBefore: uint64(now.Unix())}, false},
219-
{"ok-cert-validator", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestCertValidator("")}}, want{CertType: ssh.UserCert}, false},
220-
{"ok-cert-modifier", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestCertModifier("")}}, want{CertType: ssh.UserCert}, false},
221-
{"ok-opts-validator", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestOptionsValidator("")}}, want{CertType: ssh.UserCert}, false},
222-
{"ok-opts-modifier", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestOptionsModifier("")}}, want{CertType: ssh.UserCert}, false},
223-
{"ok-custom-template", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userCustomTemplate, userOptions}}, want{CertType: ssh.UserCert, Principals: []string{"user", "admin"}}, false},
224-
{"fail-opts-type", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{CertType: "foo"}, []provisioner.SignOption{userTemplate}}, want{}, true},
225-
{"fail-cert-validator", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestCertValidator("an error")}}, want{}, true},
226-
{"fail-cert-modifier", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestCertModifier("an error")}}, want{}, true},
227-
{"fail-opts-validator", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestOptionsValidator("an error")}}, want{}, true},
228-
{"fail-opts-modifier", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestOptionsModifier("an error")}}, want{}, true},
229-
{"fail-bad-sign-options", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, "wrong type"}}, want{}, true},
230-
{"fail-no-user-key", fields{nil, signer}, args{pub, provisioner.SignSSHOptions{CertType: "user"}, []provisioner.SignOption{userTemplate}}, want{}, true},
231-
{"fail-no-host-key", fields{signer, nil}, args{pub, provisioner.SignSSHOptions{CertType: "host"}, []provisioner.SignOption{hostTemplate}}, want{}, true},
232-
{"fail-bad-type", fields{signer, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, sshTestModifier{CertType: 100}}}, want{}, true},
233-
{"fail-custom-template", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userFailTemplate, userOptions}}, want{}, true},
234-
{"fail-custom-template-syntax-error-file", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userJSONSyntaxErrorTemplateFile, userOptions}}, want{}, true},
235-
{"fail-custom-template-syntax-value-file", fields{signer, signer}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userJSONValueErrorTemplateFile, userOptions}}, want{}, true},
237+
{"ok-user", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions}}, want{CertType: ssh.UserCert}, false},
238+
{"ok-host", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{hostTemplate, hostOptions}}, want{CertType: ssh.HostCert}, false},
239+
{"ok-user-only", fields{signer, nil, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions}}, want{CertType: ssh.UserCert}, false},
240+
{"ok-host-only", fields{nil, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{hostTemplate, hostOptions}}, want{CertType: ssh.HostCert}, false},
241+
{"ok-opts-type-user", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user"}, []provisioner.SignOption{userTemplate}}, want{CertType: ssh.UserCert}, false},
242+
{"ok-opts-type-host", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "host"}, []provisioner.SignOption{hostTemplate}}, want{CertType: ssh.HostCert}, false},
243+
{"ok-opts-principals", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user", Principals: []string{"user"}}, []provisioner.SignOption{userTemplateWithUser}}, want{CertType: ssh.UserCert, Principals: []string{"user"}}, false},
244+
{"ok-opts-principals", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "host", Principals: []string{"foo.test.com", "bar.test.com"}}, []provisioner.SignOption{hostTemplateWithHosts}}, want{CertType: ssh.HostCert, Principals: []string{"foo.test.com", "bar.test.com"}}, false},
245+
{"ok-opts-valid-after", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user", ValidAfter: provisioner.NewTimeDuration(now)}, []provisioner.SignOption{userTemplate}}, want{CertType: ssh.UserCert, ValidAfter: uint64(now.Unix())}, false},
246+
{"ok-opts-valid-before", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "host", ValidBefore: provisioner.NewTimeDuration(now)}, []provisioner.SignOption{hostTemplate}}, want{CertType: ssh.HostCert, ValidBefore: uint64(now.Unix())}, false},
247+
{"ok-cert-validator", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestCertValidator("")}}, want{CertType: ssh.UserCert}, false},
248+
{"ok-cert-modifier", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestCertModifier("")}}, want{CertType: ssh.UserCert}, false},
249+
{"ok-opts-validator", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestOptionsValidator("")}}, want{CertType: ssh.UserCert}, false},
250+
{"ok-opts-modifier", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestOptionsModifier("")}}, want{CertType: ssh.UserCert}, false},
251+
{"ok-custom-template", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userCustomTemplate, userOptions}}, want{CertType: ssh.UserCert, Principals: []string{"user", "admin"}}, false},
252+
{"ok-user-policy", fields{signer, signer, userPolicy, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user", Principals: []string{"user"}}, []provisioner.SignOption{userTemplateWithUser}}, want{CertType: ssh.UserCert, Principals: []string{"user"}}, false},
253+
{"ok-host-policy", fields{signer, signer, nil, hostPolicy}, args{pub, provisioner.SignSSHOptions{CertType: "host", Principals: []string{"foo.test.com", "bar.test.com"}}, []provisioner.SignOption{hostTemplateWithHosts}}, want{CertType: ssh.HostCert, Principals: []string{"foo.test.com", "bar.test.com"}}, false},
254+
{"fail-opts-type", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "foo"}, []provisioner.SignOption{userTemplate}}, want{}, true},
255+
{"fail-cert-validator", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestCertValidator("an error")}}, want{}, true},
256+
{"fail-cert-modifier", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestCertModifier("an error")}}, want{}, true},
257+
{"fail-opts-validator", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestOptionsValidator("an error")}}, want{}, true},
258+
{"fail-opts-modifier", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, sshTestOptionsModifier("an error")}}, want{}, true},
259+
{"fail-bad-sign-options", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, userOptions, "wrong type"}}, want{}, true},
260+
{"fail-no-user-key", fields{nil, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user"}, []provisioner.SignOption{userTemplate}}, want{}, true},
261+
{"fail-no-host-key", fields{signer, nil, nil, nil}, args{pub, provisioner.SignSSHOptions{CertType: "host"}, []provisioner.SignOption{hostTemplate}}, want{}, true},
262+
{"fail-bad-type", fields{signer, nil, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userTemplate, sshTestModifier{CertType: 100}}}, want{}, true},
263+
{"fail-custom-template", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userFailTemplate, userOptions}}, want{}, true},
264+
{"fail-custom-template-syntax-error-file", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userJSONSyntaxErrorTemplateFile, userOptions}}, want{}, true},
265+
{"fail-custom-template-syntax-value-file", fields{signer, signer, nil, nil}, args{pub, provisioner.SignSSHOptions{}, []provisioner.SignOption{userJSONValueErrorTemplateFile, userOptions}}, want{}, true},
266+
{"fail-user-policy", fields{signer, signer, userPolicy, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user", Principals: []string{"root"}}, []provisioner.SignOption{userTemplateWithRoot}}, want{}, true},
267+
{"fail-user-policy-with-host-cert", fields{signer, signer, userPolicy, nil}, args{pub, provisioner.SignSSHOptions{CertType: "host", Principals: []string{"foo.test.com"}}, []provisioner.SignOption{hostTemplateWithExampleDotCom}}, want{}, true},
268+
{"fail-user-policy-with-bad-user", fields{signer, signer, userPolicy, nil}, args{pub, provisioner.SignSSHOptions{CertType: "user", Principals: []string{"user"}}, []provisioner.SignOption{badUserTemplate}}, want{}, true},
269+
{"fail-host-policy", fields{signer, signer, nil, hostPolicy}, args{pub, provisioner.SignSSHOptions{CertType: "host", Principals: []string{"example.com"}}, []provisioner.SignOption{hostTemplateWithExampleDotCom}}, want{}, true},
270+
{"fail-host-policy-with-user-cert", fields{signer, signer, nil, hostPolicy}, args{pub, provisioner.SignSSHOptions{CertType: "user", Principals: []string{"user"}}, []provisioner.SignOption{userTemplateWithUser}}, want{}, true},
271+
{"fail-host-policy-with-bad-host", fields{signer, signer, nil, hostPolicy}, args{pub, provisioner.SignSSHOptions{CertType: "host", Principals: []string{"example.com"}}, []provisioner.SignOption{badHostTemplate}}, want{}, true},
236272
}
237273
for _, tt := range tests {
238274
t.Run(tt.name, func(t *testing.T) {
239275
a := testAuthority(t)
240276
a.sshCAUserCertSignKey = tt.fields.sshCAUserCertSignKey
241277
a.sshCAHostCertSignKey = tt.fields.sshCAHostCertSignKey
278+
a.sshUserPolicy = tt.fields.sshUserPolicy
279+
a.sshHostPolicy = tt.fields.sshHostPolicy
242280

243281
got, err := a.SignSSH(context.Background(), tt.args.key, tt.args.opts, tt.args.signOpts...)
244282
if (err != nil) != tt.wantErr {

‎authority/tls.go

+15-5
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,19 @@ import (
1616
"time"
1717

1818
"github.com/pkg/errors"
19+
"golang.org/x/crypto/ssh"
20+
21+
"go.step.sm/crypto/jose"
22+
"go.step.sm/crypto/keyutil"
23+
"go.step.sm/crypto/pemutil"
24+
"go.step.sm/crypto/x509util"
25+
1926
"github.com/smallstep/certificates/authority/config"
2027
"github.com/smallstep/certificates/authority/provisioner"
2128
casapi "github.com/smallstep/certificates/cas/apiv1"
2229
"github.com/smallstep/certificates/db"
2330
"github.com/smallstep/certificates/errs"
24-
"go.step.sm/crypto/jose"
25-
"go.step.sm/crypto/keyutil"
26-
"go.step.sm/crypto/pemutil"
27-
"go.step.sm/crypto/x509util"
28-
"golang.org/x/crypto/ssh"
31+
"github.com/smallstep/certificates/policy"
2932
)
3033

3134
// GetTLSOptions returns the tls options configured.
@@ -199,6 +202,13 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Sign
199202
// Check if authority is allowed to sign the certificate
200203
var allowedToSign bool
201204
if allowedToSign, err = a.isAllowedToSign(leaf); err != nil {
205+
var pe *policy.NamePolicyError
206+
if errors.As(err, &pe) && pe.Reason == policy.NotAuthorizedForThisName {
207+
return nil, errs.ApplyOptions(
208+
errs.ForbiddenErr(errors.New("authority not allowed to sign"), err.Error()),
209+
opts...,
210+
)
211+
}
202212
return nil, errs.InternalServerErr(err,
203213
errs.WithKeyVal("csr", csr),
204214
errs.WithKeyVal("signOptions", signOpts),

0 commit comments

Comments
 (0)