Skip to content

Commit 414a94b

Browse files
dopeydopey
committed
Instrument getIdentity func for OIDC ssh provisioner
1 parent 3d970b4 commit 414a94b

File tree

9 files changed

+198
-45
lines changed

9 files changed

+198
-45
lines changed

‎authority/authority.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ type Authority struct {
4141
initOnce bool
4242
// Custom functions
4343
sshBastionFunc func(user, hostname string) (*Bastion, error)
44-
getIdentityFunc func(p provisioner.Interface, email string) (*provisioner.Identity, error)
44+
getIdentityFunc provisioner.GetIdentityFunc
4545
}
4646

4747
// New creates and initiates a new Authority type.
@@ -192,6 +192,7 @@ func (a *Authority) init() error {
192192
UserKeys: sshKeys.UserKeys,
193193
HostKeys: sshKeys.HostKeys,
194194
},
195+
GetIdentityFunc: a.getIdentityFunc,
195196
}
196197
// Store all the provisioners
197198
for _, p := range a.config.AuthorityConfig.Provisioners {

‎authority/provisioner/jwk.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,10 +197,10 @@ func (p *JWK) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption,
197197
// Add modifiers from custom claims
198198
// FIXME: this is also set in the sign method using SSHOptions.Modify.
199199
if opts.CertType != "" {
200-
signOptions = append(signOptions, sshCertificateCertTypeModifier(opts.CertType))
200+
signOptions = append(signOptions, sshCertTypeModifier(opts.CertType))
201201
}
202202
if len(opts.Principals) > 0 {
203-
signOptions = append(signOptions, sshCertificatePrincipalsModifier(opts.Principals))
203+
signOptions = append(signOptions, sshCertPrincipalsModifier(opts.Principals))
204204
}
205205
if !opts.ValidAfter.IsZero() {
206206
signOptions = append(signOptions, sshCertificateValidAfterModifier(opts.ValidAfter.RelativeTime(t).Unix()))

‎authority/provisioner/oidc.go

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ type OIDC struct {
6464
configuration openIDConfiguration
6565
keyStore *keyStore
6666
claimer *Claimer
67+
getIdentityFunc GetIdentityFunc
6768
}
6869

6970
// IsAdmin returns true if the given email is in the Admins whitelist, false
@@ -169,6 +170,13 @@ func (o *OIDC) Init(config Config) (err error) {
169170
if err != nil {
170171
return err
171172
}
173+
174+
// Set the identity getter if it exists, otherwise use the default.
175+
if config.GetIdentityFunc == nil {
176+
o.getIdentityFunc = DefaultIdentityFunc
177+
} else {
178+
o.getIdentityFunc = config.GetIdentityFunc
179+
}
172180
return nil
173181
}
174182

@@ -326,23 +334,26 @@ func (o *OIDC) AuthorizeSSHSign(ctx context.Context, token string) ([]SignOption
326334
sshCertificateKeyIDModifier(claims.Email),
327335
}
328336

329-
name := SanitizeSSHUserPrincipal(claims.Email)
330-
if !sshUserRegex.MatchString(name) {
331-
return nil, errors.Errorf("invalid principal '%s' from email address '%s'", name, claims.Email)
337+
// Get the identity using either the default identityFunc or one injected
338+
// externally.
339+
iden, err := o.getIdentityFunc(o, claims.Email)
340+
if err != nil {
341+
return nil, errors.Wrap(err, "authorizeSSHSign")
332342
}
333-
334-
// Admin users will default to user + name but they can be changed by the
335-
// user options. Non-admins are only able to sign user certificates.
336343
defaults := SSHOptions{
337344
CertType: SSHUserCert,
338-
Principals: []string{name},
345+
Principals: iden.Usernames,
339346
}
340347

348+
// Admin users can use any principal, and can sign user and host certificates.
349+
// Non-admin users can only use principals returned by the identityFunc, and
350+
// can only sign user certificates.
341351
if !o.IsAdmin(claims.Email) {
342352
signOptions = append(signOptions, sshCertificateOptionsValidator(defaults))
343353
}
344354

345-
// Default to a user with name as principal if not set
355+
// Default to a user certificate with usernames as principals if those options
356+
// are not set.
346357
signOptions = append(signOptions, sshCertificateDefaultsModifier(defaults))
347358

348359
return append(signOptions,

‎authority/provisioner/oidc_test.go

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,10 @@ func TestOIDC_AuthorizeSSHSign(t *testing.T) {
347347
assert.FatalError(t, err)
348348
p3, err := generateOIDC()
349349
assert.FatalError(t, err)
350+
p4, err := generateOIDC()
351+
assert.FatalError(t, err)
352+
p5, err := generateOIDC()
353+
assert.FatalError(t, err)
350354
// Admin + Domains
351355
p3.Admins = []string{"name@smallstep.com", "root@example.com"}
352356
p3.Domains = []string{"smallstep.com"}
@@ -356,12 +360,27 @@ func TestOIDC_AuthorizeSSHSign(t *testing.T) {
356360
p1.ConfigurationEndpoint = srv.URL + "/.well-known/openid-configuration"
357361
p2.ConfigurationEndpoint = srv.URL + "/.well-known/openid-configuration"
358362
p3.ConfigurationEndpoint = srv.URL + "/.well-known/openid-configuration"
363+
p4.ConfigurationEndpoint = srv.URL + "/.well-known/openid-configuration"
364+
p5.ConfigurationEndpoint = srv.URL + "/.well-known/openid-configuration"
359365
assert.FatalError(t, p1.Init(config))
360366
assert.FatalError(t, p2.Init(config))
361367
assert.FatalError(t, p3.Init(config))
368+
assert.FatalError(t, p4.Init(config))
369+
assert.FatalError(t, p5.Init(config))
370+
371+
p4.getIdentityFunc = func(p Interface, email string) (*Identity, error) {
372+
return &Identity{Usernames: []string{"max", "mariano"}}, nil
373+
}
374+
p5.getIdentityFunc = func(p Interface, email string) (*Identity, error) {
375+
return nil, errors.New("force")
376+
}
362377

363378
t1, err := generateSimpleToken("the-issuer", p1.ClientID, &keys.Keys[0])
364379
assert.FatalError(t, err)
380+
okGetIdentityToken, err := generateSimpleToken("the-issuer", p4.ClientID, &keys.Keys[0])
381+
assert.FatalError(t, err)
382+
failGetIdentityToken, err := generateSimpleToken("the-issuer", p5.ClientID, &keys.Keys[0])
383+
assert.FatalError(t, err)
365384
// Admin email not in domains
366385
okAdmin, err := generateToken("subject", "the-issuer", p3.ClientID, "root@example.com", []string{}, time.Now(), &keys.Keys[0])
367386
assert.FatalError(t, err)
@@ -384,11 +403,11 @@ func TestOIDC_AuthorizeSSHSign(t *testing.T) {
384403
userDuration := p1.claimer.DefaultUserSSHCertDuration()
385404
hostDuration := p1.claimer.DefaultHostSSHCertDuration()
386405
expectedUserOptions := &SSHOptions{
387-
CertType: "user", Principals: []string{"name"},
406+
CertType: "user", Principals: []string{"name", "name@smallstep.com"},
388407
ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration)),
389408
}
390409
expectedAdminOptions := &SSHOptions{
391-
CertType: "user", Principals: []string{"root"},
410+
CertType: "user", Principals: []string{"root", "root@example.com"},
392411
ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration)),
393412
}
394413
expectedHostOptions := &SSHOptions{
@@ -412,17 +431,32 @@ func TestOIDC_AuthorizeSSHSign(t *testing.T) {
412431
{"ok", p1, args{t1, SSHOptions{}, pub}, expectedUserOptions, false, false},
413432
{"ok-rsa2048", p1, args{t1, SSHOptions{}, rsa2048.Public()}, expectedUserOptions, false, false},
414433
{"ok-user", p1, args{t1, SSHOptions{CertType: "user"}, pub}, expectedUserOptions, false, false},
415-
{"ok-principals", p1, args{t1, SSHOptions{Principals: []string{"name"}}, pub}, expectedUserOptions, false, false},
416-
{"ok-options", p1, args{t1, SSHOptions{CertType: "user", Principals: []string{"name"}}, pub}, expectedUserOptions, false, false},
434+
{"ok-principals", p1, args{t1, SSHOptions{Principals: []string{"name"}}, pub},
435+
&SSHOptions{CertType: "user", Principals: []string{"name"},
436+
ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration))}, false, false},
437+
{"ok-principals-getIdentity", p4, args{okGetIdentityToken, SSHOptions{Principals: []string{"mariano"}}, pub},
438+
&SSHOptions{CertType: "user", Principals: []string{"mariano"},
439+
ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration))}, false, false},
440+
{"ok-emptyPrincipals-getIdentity", p4, args{okGetIdentityToken, SSHOptions{}, pub},
441+
&SSHOptions{CertType: "user", Principals: []string{"max", "mariano"},
442+
ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration))}, false, false},
443+
{"ok-options", p1, args{t1, SSHOptions{CertType: "user", Principals: []string{"name"}}, pub},
444+
&SSHOptions{CertType: "user", Principals: []string{"name"},
445+
ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration))}, false, false},
417446
{"admin", p3, args{okAdmin, SSHOptions{}, pub}, expectedAdminOptions, false, false},
418447
{"admin-user", p3, args{okAdmin, SSHOptions{CertType: "user"}, pub}, expectedAdminOptions, false, false},
419-
{"admin-principals", p3, args{okAdmin, SSHOptions{Principals: []string{"root"}}, pub}, expectedAdminOptions, false, false},
420-
{"admin-options", p3, args{okAdmin, SSHOptions{CertType: "user", Principals: []string{"name"}}, pub}, expectedUserOptions, false, false},
448+
{"admin-principals", p3, args{okAdmin, SSHOptions{Principals: []string{"root"}}, pub},
449+
&SSHOptions{CertType: "user", Principals: []string{"root"},
450+
ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration))}, false, false},
451+
{"admin-options", p3, args{okAdmin, SSHOptions{CertType: "user", Principals: []string{"name"}}, pub},
452+
&SSHOptions{CertType: "user", Principals: []string{"name"},
453+
ValidAfter: NewTimeDuration(tm), ValidBefore: NewTimeDuration(tm.Add(userDuration))}, false, false},
421454
{"admin-host", p3, args{okAdmin, SSHOptions{CertType: "host", Principals: []string{"smallstep.com"}}, pub}, expectedHostOptions, false, false},
422455
{"fail-rsa1024", p1, args{t1, SSHOptions{}, rsa1024.Public()}, expectedUserOptions, false, true},
423456
{"fail-user-host", p1, args{t1, SSHOptions{CertType: "host"}, pub}, nil, false, true},
424457
{"fail-user-principals", p1, args{t1, SSHOptions{Principals: []string{"root"}}, pub}, nil, false, true},
425458
{"fail-email", p3, args{failEmail, SSHOptions{}, pub}, nil, true, false},
459+
{"fail-getIdentity", p5, args{failGetIdentityToken, SSHOptions{}, pub}, nil, true, false},
426460
}
427461
for _, tt := range tests {
428462
t.Run(tt.name, func(t *testing.T) {

‎authority/provisioner/provisioner.go

Lines changed: 67 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,9 @@ type Config struct {
185185
DB db.AuthDB
186186
// SSHKeys are the root SSH public keys
187187
SSHKeys *SSHKeys
188+
// GetIdentityFunc is a function that returns an identity that will be
189+
// used by the provisioner to populate certificate attributes.
190+
GetIdentityFunc GetIdentityFunc
188191
}
189192

190193
type provisioner struct {
@@ -314,7 +317,7 @@ func (b *base) AuthorizeSSHRenew(ctx context.Context, token string) (*ssh.Certif
314317
}
315318

316319
// AuthorizeSSHRekey returns an unimplmented error. Provisioners should overwrite
317-
// this method if they will support authorizing tokens for renewing SSH Certificates.
320+
// this method if they will support authorizing tokens for rekeying SSH Certificates.
318321
func (b *base) AuthorizeSSHRekey(ctx context.Context, token string) (*ssh.Certificate, []SignOption, error) {
319322
return nil, nil, errors.New("not implemented; provisioner does not implement AuthorizeSSHRekey")
320323
}
@@ -325,6 +328,23 @@ type Identity struct {
325328
Usernames []string `json:"usernames"`
326329
}
327330

331+
// GetIdentityFunc is a function that returns an identity.
332+
type GetIdentityFunc func(p Interface, email string) (*Identity, error)
333+
334+
// DefaultIdentityFunc return a default identity depending on the provisioner type.
335+
func DefaultIdentityFunc(p Interface, email string) (*Identity, error) {
336+
switch k := p.(type) {
337+
case *OIDC:
338+
name := SanitizeSSHUserPrincipal(email)
339+
if !sshUserRegex.MatchString(name) {
340+
return nil, errors.Errorf("invalid principal '%s' from email '%s'", name, email)
341+
}
342+
return &Identity{Usernames: []string{name, email}}, nil
343+
default:
344+
return nil, errors.Errorf("provisioner type '%T' not supported by identity function", k)
345+
}
346+
}
347+
328348
// MockProvisioner for testing
329349
type MockProvisioner struct {
330350
Mret1, Mret2, Mret3 interface{}
@@ -335,9 +355,13 @@ type MockProvisioner struct {
335355
MgetType func() Type
336356
MgetEncryptedKey func() (string, string, bool)
337357
Minit func(Config) error
338-
MauthorizeRevoke func(ott string) error
339358
MauthorizeSign func(ctx context.Context, ott string) ([]SignOption, error)
340-
MauthorizeRenewal func(*x509.Certificate) error
359+
MauthorizeRenew func(ctx context.Context, cert *x509.Certificate) error
360+
MauthorizeRevoke func(ctx context.Context, ott string) error
361+
MauthorizeSSHSign func(ctx context.Context, ott string) ([]SignOption, error)
362+
MauthorizeSSHRenew func(ctx context.Context, ott string) (*ssh.Certificate, error)
363+
MauthorizeSSHRekey func(ctx context.Context, ott string) (*ssh.Certificate, []SignOption, error)
364+
MauthorizeSSHRevoke func(ctx context.Context, ott string) error
341365
}
342366

343367
// GetID mock
@@ -391,26 +415,58 @@ func (m *MockProvisioner) Init(c Config) error {
391415
return m.Merr
392416
}
393417

418+
// AuthorizeSign mock
419+
func (m *MockProvisioner) AuthorizeSign(ctx context.Context, ott string) ([]SignOption, error) {
420+
if m.MauthorizeSign != nil {
421+
return m.MauthorizeSign(ctx, ott)
422+
}
423+
return m.Mret1.([]SignOption), m.Merr
424+
}
425+
394426
// AuthorizeRevoke mock
395-
func (m *MockProvisioner) AuthorizeRevoke(ott string) error {
427+
func (m *MockProvisioner) AuthorizeRevoke(ctx context.Context, ott string) error {
396428
if m.MauthorizeRevoke != nil {
397-
return m.MauthorizeRevoke(ott)
429+
return m.MauthorizeRevoke(ctx, ott)
398430
}
399431
return m.Merr
400432
}
401433

402-
// AuthorizeSign mock
403-
func (m *MockProvisioner) AuthorizeSign(ctx context.Context, ott string) ([]SignOption, error) {
434+
// AuthorizeRenew mock
435+
func (m *MockProvisioner) AuthorizeRenew(ctx context.Context, c *x509.Certificate) error {
436+
if m.MauthorizeRenew != nil {
437+
return m.MauthorizeRenew(ctx, c)
438+
}
439+
return m.Merr
440+
}
441+
442+
// AuthorizeSSHSign mock
443+
func (m *MockProvisioner) AuthorizeSSHSign(ctx context.Context, ott string) ([]SignOption, error) {
404444
if m.MauthorizeSign != nil {
405445
return m.MauthorizeSign(ctx, ott)
406446
}
407447
return m.Mret1.([]SignOption), m.Merr
408448
}
409449

410-
// AuthorizeRenewal mock
411-
func (m *MockProvisioner) AuthorizeRenewal(c *x509.Certificate) error {
412-
if m.MauthorizeRenewal != nil {
413-
return m.MauthorizeRenewal(c)
450+
// AuthorizeSSHRenew mock
451+
func (m *MockProvisioner) AuthorizeSSHRenew(ctx context.Context, ott string) (*ssh.Certificate, error) {
452+
if m.MauthorizeRenew != nil {
453+
return m.MauthorizeSSHRenew(ctx, ott)
454+
}
455+
return m.Mret1.(*ssh.Certificate), m.Merr
456+
}
457+
458+
// AuthorizeSSHRekey mock
459+
func (m *MockProvisioner) AuthorizeSSHRekey(ctx context.Context, ott string) (*ssh.Certificate, []SignOption, error) {
460+
if m.MauthorizeSSHRekey != nil {
461+
return m.MauthorizeSSHRekey(ctx, ott)
462+
}
463+
return m.Mret1.(*ssh.Certificate), m.Mret2.([]SignOption), m.Merr
464+
}
465+
466+
// AuthorizeSSHRevoke mock
467+
func (m *MockProvisioner) AuthorizeSSHRevoke(ctx context.Context, ott string) error {
468+
if m.MauthorizeSSHRevoke != nil {
469+
return m.MauthorizeSSHRevoke(ctx, ott)
414470
}
415471
return m.Merr
416472
}

‎authority/provisioner/provisioner_test.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ package provisioner
22

33
import (
44
"testing"
5+
6+
"github.com/pkg/errors"
7+
"github.com/smallstep/assert"
58
)
69

710
func TestType_String(t *testing.T) {
@@ -52,3 +55,49 @@ func TestSanitizeSSHUserPrincipal(t *testing.T) {
5255
})
5356
}
5457
}
58+
59+
func TestDefaultIdentityFunc(t *testing.T) {
60+
type test struct {
61+
p Interface
62+
email string
63+
err error
64+
identity *Identity
65+
}
66+
tests := map[string]func(*testing.T) test{
67+
"fail/unsupported-provisioner": func(t *testing.T) test {
68+
return test{
69+
p: &X5C{},
70+
err: errors.New("provisioner type '*provisioner.X5C' not supported by identity function"),
71+
}
72+
},
73+
"fail/bad-ssh-regex": func(t *testing.T) test {
74+
return test{
75+
p: &OIDC{},
76+
email: "$%^#_>@smallstep.com",
77+
err: errors.New("invalid principal '______' from email '$%^#_>@smallstep.com'"),
78+
}
79+
},
80+
"ok": func(t *testing.T) test {
81+
return test{
82+
p: &OIDC{},
83+
email: "max.furman@smallstep.com",
84+
identity: &Identity{Usernames: []string{"maxfurman", "max.furman@smallstep.com"}},
85+
}
86+
},
87+
}
88+
for name, get := range tests {
89+
t.Run(name, func(t *testing.T) {
90+
tc := get(t)
91+
identity, err := DefaultIdentityFunc(tc.p, tc.email)
92+
if err != nil {
93+
if assert.NotNil(t, tc.err) {
94+
assert.Equals(t, tc.err.Error(), err.Error())
95+
}
96+
} else {
97+
if assert.Nil(t, tc.err) {
98+
assert.Equals(t, identity.Usernames, tc.identity.Usernames)
99+
}
100+
}
101+
})
102+
}
103+
}

0 commit comments

Comments
 (0)