From df1f7e5a2e98ca442e2b74b58977445a4382fda9 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 29 Jul 2020 14:29:12 -0700 Subject: [PATCH] Use CertificateRequest type as input for ssh NewCertificate. SSH does not have a real concept of ssh certificate request, but we are using the type to encapsulate the parameters coming in the request. --- sshutil/certificate.go | 6 ++--- sshutil/certificate_request.go | 17 ++++++++++++++ sshutil/certificate_test.go | 33 ++++++++++++++------------ sshutil/options.go | 19 ++++++++------- sshutil/options_test.go | 42 ++++++++++++++++++++-------------- sshutil/templates.go | 41 +++++++++++++++++++++------------ sshutil/templates_test.go | 32 +++++++++++++++----------- 7 files changed, 118 insertions(+), 72 deletions(-) create mode 100644 sshutil/certificate_request.go diff --git a/sshutil/certificate.go b/sshutil/certificate.go index 473344a4..1b58882c 100644 --- a/sshutil/certificate.go +++ b/sshutil/certificate.go @@ -29,8 +29,8 @@ type Certificate struct { // NewCertificate creates a new certificate with the given key after parsing a // template given in the options. -func NewCertificate(key ssh.PublicKey, opts ...Option) (*Certificate, error) { - o, err := new(Options).apply(key, opts) +func NewCertificate(cr CertificateRequest, opts ...Option) (*Certificate, error) { + o, err := new(Options).apply(cr, opts) if err != nil { return nil, err } @@ -46,7 +46,7 @@ func NewCertificate(key ssh.PublicKey, opts ...Option) (*Certificate, error) { } // Complete with public key - cert.Key = key + cert.Key = cr.Key return &cert, nil } diff --git a/sshutil/certificate_request.go b/sshutil/certificate_request.go new file mode 100644 index 00000000..d4081dc3 --- /dev/null +++ b/sshutil/certificate_request.go @@ -0,0 +1,17 @@ +package sshutil + +import "golang.org/x/crypto/ssh" + +// CertificateRequests simulates a certificate request for SSH. SSH does not +// have a concept of certificate requests, but the CA accepts the key and some +// other parameters in the requests that are part of the certificate. This +// struct will hold these parameters. +// +// CertificateRequests object will be used in the templates to set parameters +// passed with the API instead of the validated ones. +type CertificateRequest struct { + Key ssh.PublicKey + Type CertType + KeyID string + Principals []string +} diff --git a/sshutil/certificate_test.go b/sshutil/certificate_test.go index ed8c5f9a..8042cc6b 100644 --- a/sshutil/certificate_test.go +++ b/sshutil/certificate_test.go @@ -36,9 +36,12 @@ func mustGeneratePublicKey(t *testing.T) ssh.PublicKey { func TestNewCertificate(t *testing.T) { key := mustGeneratePublicKey(t) + cr := CertificateRequest{ + Key: key, + } type args struct { - key ssh.PublicKey + cr CertificateRequest opts []Option } tests := []struct { @@ -47,7 +50,7 @@ func TestNewCertificate(t *testing.T) { want *Certificate wantErr bool }{ - {"user", args{key, []Option{WithTemplate(DefaultCertificate, CreateTemplateData(UserCert, "jane@doe.com", []string{"jane"}))}}, &Certificate{ + {"user", args{cr, []Option{WithTemplate(DefaultCertificate, CreateTemplateData(UserCert, "jane@doe.com", []string{"jane"}))}}, &Certificate{ Nonce: nil, Key: key, Serial: 0, @@ -68,7 +71,7 @@ func TestNewCertificate(t *testing.T) { SignatureKey: nil, Signature: nil, }, false}, - {"host", args{key, []Option{WithTemplate(DefaultCertificate, CreateTemplateData(HostCert, "foobar", []string{"foo.internal", "bar.internal"}))}}, &Certificate{ + {"host", args{cr, []Option{WithTemplate(DefaultCertificate, CreateTemplateData(HostCert, "foobar", []string{"foo.internal", "bar.internal"}))}}, &Certificate{ Nonce: nil, Key: key, Serial: 0, @@ -83,12 +86,12 @@ func TestNewCertificate(t *testing.T) { SignatureKey: nil, Signature: nil, }, false}, - {"file", args{key, []Option{WithTemplateFile("./testdata/github.tpl", TemplateData{ + {"file", args{cr, []Option{WithTemplateFile("./testdata/github.tpl", TemplateData{ TypeKey: UserCert, KeyIDKey: "john@doe.com", PrincipalsKey: []string{"john", "john@doe.com"}, ExtensionsKey: DefaultExtensions(UserCert), - InsecureKey: map[string]interface{}{ + InsecureKey: TemplateData{ "User": map[string]interface{}{"username": "john"}, }, })}}, &Certificate{ @@ -102,18 +105,18 @@ func TestNewCertificate(t *testing.T) { ValidBefore: 0, CriticalOptions: nil, Extensions: map[string]string{ + "login@github.com": "john", "permit-X11-forwarding": "", "permit-agent-forwarding": "", "permit-port-forwarding": "", "permit-pty": "", "permit-user-rc": "", - "login@github.com": "john", }, Reserved: nil, SignatureKey: nil, Signature: nil, }, false}, - {"base64", args{key, []Option{WithTemplateBase64(base64.StdEncoding.EncodeToString([]byte(DefaultCertificate)), CreateTemplateData(HostCert, "foo.internal", nil))}}, &Certificate{ + {"base64", args{cr, []Option{WithTemplateBase64(base64.StdEncoding.EncodeToString([]byte(DefaultCertificate)), CreateTemplateData(HostCert, "foo.internal", nil))}}, &Certificate{ Nonce: nil, Key: key, Serial: 0, @@ -128,22 +131,22 @@ func TestNewCertificate(t *testing.T) { SignatureKey: nil, Signature: nil, }, false}, - {"failNilOptions", args{key, nil}, nil, true}, - {"failEmptyOptions", args{key, nil}, nil, true}, - {"badBase64Template", args{key, []Option{WithTemplateBase64("foobar", TemplateData{})}}, nil, true}, - {"badFileTemplate", args{key, []Option{WithTemplateFile("./testdata/missing.tpl", TemplateData{})}}, nil, true}, - {"badJsonTemplate", args{key, []Option{WithTemplate(`{"type":{{ .Type }}}`, TemplateData{})}}, nil, true}, - {"failTemplate", args{key, []Option{WithTemplate(`{{ fail "an error" }}`, TemplateData{})}}, nil, true}, + {"failNilOptions", args{cr, nil}, nil, true}, + {"failEmptyOptions", args{cr, nil}, nil, true}, + {"badBase64Template", args{cr, []Option{WithTemplateBase64("foobar", TemplateData{})}}, nil, true}, + {"badFileTemplate", args{cr, []Option{WithTemplateFile("./testdata/missing.tpl", TemplateData{})}}, nil, true}, + {"badJsonTemplate", args{cr, []Option{WithTemplate(`{"type":{{ .Type }}}`, TemplateData{})}}, nil, true}, + {"failTemplate", args{cr, []Option{WithTemplate(`{{ fail "an error" }}`, TemplateData{})}}, nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := NewCertificate(tt.args.key, tt.args.opts...) + got, err := NewCertificate(tt.args.cr, tt.args.opts...) if (err != nil) != tt.wantErr { t.Errorf("NewCertificate() error = %v, wantErr %v", err, tt.wantErr) return } if !reflect.DeepEqual(got, tt.want) { - t.Errorf("NewCertificate() = %v, want %v", got, tt.want) + t.Errorf("NewCertificate() = \n%+v, want \n%+v", got, tt.want) } }) } diff --git a/sshutil/options.go b/sshutil/options.go index 58562ba9..e6f094e0 100644 --- a/sshutil/options.go +++ b/sshutil/options.go @@ -9,7 +9,6 @@ import ( "github.com/Masterminds/sprig/v3" "github.com/pkg/errors" "github.com/smallstep/cli/config" - "golang.org/x/crypto/ssh" ) func getFuncMap(failMessage *string) template.FuncMap { @@ -26,9 +25,9 @@ type Options struct { CertBuffer *bytes.Buffer } -func (o *Options) apply(key ssh.PublicKey, opts []Option) (*Options, error) { +func (o *Options) apply(cr CertificateRequest, opts []Option) (*Options, error) { for _, fn := range opts { - if err := fn(key, o); err != nil { + if err := fn(cr, o); err != nil { return o, err } } @@ -36,12 +35,12 @@ func (o *Options) apply(key ssh.PublicKey, opts []Option) (*Options, error) { } // Option is the type used as a variadic argument in NewCertificate. -type Option func(key ssh.PublicKey, o *Options) error +type Option func(cr CertificateRequest, o *Options) error // WithTemplate is an options that executes the given template text with the // given data. func WithTemplate(text string, data TemplateData) Option { - return func(key ssh.PublicKey, o *Options) error { + return func(cr CertificateRequest, o *Options) error { terr := new(TemplateError) funcMap := getFuncMap(&terr.Message) @@ -51,7 +50,7 @@ func WithTemplate(text string, data TemplateData) Option { } buf := new(bytes.Buffer) - data.SetPublicKey(key) + data.SetCertificateRequest(cr) if err := tmpl.Execute(buf, data); err != nil { if terr.Message != "" { return terr @@ -66,26 +65,26 @@ func WithTemplate(text string, data TemplateData) Option { // WithTemplateBase64 is an options that executes the given template base64 // string with the given data. func WithTemplateBase64(s string, data TemplateData) Option { - return func(key ssh.PublicKey, o *Options) error { + return func(cr CertificateRequest, o *Options) error { b, err := base64.StdEncoding.DecodeString(s) if err != nil { return errors.Wrap(err, "error decoding template") } fn := WithTemplate(string(b), data) - return fn(key, o) + return fn(cr, o) } } // WithTemplateFile is an options that reads the template file and executes it // with the given data. func WithTemplateFile(path string, data TemplateData) Option { - return func(key ssh.PublicKey, o *Options) error { + return func(cr CertificateRequest, o *Options) error { filename := config.StepAbs(path) b, err := ioutil.ReadFile(filename) if err != nil { return errors.Wrapf(err, "error reading %s", path) } fn := WithTemplate(string(b), data) - return fn(key, o) + return fn(cr, o) } } diff --git a/sshutil/options_test.go b/sshutil/options_test.go index ac4da67d..1c9e4aa4 100644 --- a/sshutil/options_test.go +++ b/sshutil/options_test.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/pkg/errors" - "golang.org/x/crypto/ssh" ) func Test_getFuncMap_fail(t *testing.T) { @@ -28,11 +27,14 @@ func Test_getFuncMap_fail(t *testing.T) { func TestWithTemplate(t *testing.T) { key := mustGeneratePublicKey(t) + cr := CertificateRequest{ + Key: key, + } type args struct { text string data TemplateData - key ssh.PublicKey + cr CertificateRequest } tests := []struct { name string @@ -45,7 +47,7 @@ func TestWithTemplate(t *testing.T) { KeyIDKey: "jane@doe.com", PrincipalsKey: []string{"jane", "jane@doe.com"}, ExtensionsKey: DefaultExtensions(UserCert), - }, key}, Options{ + }, cr}, Options{ CertBuffer: bytes.NewBufferString(`{ "type": "user", "keyId": "jane@doe.com", @@ -56,24 +58,24 @@ func TestWithTemplate(t *testing.T) { TypeKey: "host", KeyIDKey: "foo", PrincipalsKey: []string{"foo.internal"}, - }, key}, Options{ + }, cr}, Options{ CertBuffer: bytes.NewBufferString(`{ "type": "host", "keyId": "foo", "principals": ["foo.internal"], "extensions": null }`)}, false}, - {"fail", args{`{{ fail "a message" }}`, TemplateData{}, key}, Options{}, true}, - {"failTemplate", args{`{{ fail "fatal error }}`, TemplateData{}, key}, Options{}, true}, + {"fail", args{`{{ fail "a message" }}`, TemplateData{}, cr}, Options{}, true}, + {"failTemplate", args{`{{ fail "fatal error }}`, TemplateData{}, cr}, Options{}, true}, {"error", args{`{{ mustHas 3 .Data }}`, TemplateData{ "Data": 3, - }, key}, Options{}, true}, + }, cr}, Options{}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { var got Options fn := WithTemplate(tt.args.text, tt.args.data) - if err := fn(tt.args.key, &got); (err != nil) != tt.wantErr { + if err := fn(tt.args.cr, &got); (err != nil) != tt.wantErr { t.Errorf("WithTemplate() error = %v, wantErr %v", err, tt.wantErr) } if !reflect.DeepEqual(got, tt.want) { @@ -85,11 +87,14 @@ func TestWithTemplate(t *testing.T) { func TestWithTemplateBase64(t *testing.T) { key := mustGeneratePublicKey(t) + cr := CertificateRequest{ + Key: key, + } type args struct { s string data TemplateData - key ssh.PublicKey + cr CertificateRequest } tests := []struct { name string @@ -102,20 +107,20 @@ func TestWithTemplateBase64(t *testing.T) { KeyIDKey: "foo.internal", PrincipalsKey: []string{"foo.internal", "bar.internal"}, ExtensionsKey: map[string]interface{}{"foo": "bar"}, - }, key}, Options{ + }, cr}, Options{ CertBuffer: bytes.NewBufferString(`{ "type": "host", "keyId": "foo.internal", "principals": ["foo.internal","bar.internal"], "extensions": {"foo":"bar"} }`)}, false}, - {"badBase64", args{"foobar", TemplateData{}, key}, Options{}, true}, + {"badBase64", args{"foobar", TemplateData{}, cr}, Options{}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { var got Options fn := WithTemplateBase64(tt.args.s, tt.args.data) - if err := fn(tt.args.key, &got); (err != nil) != tt.wantErr { + if err := fn(tt.args.cr, &got); (err != nil) != tt.wantErr { t.Errorf("WithTemplateBase64() error = %v, wantErr %v", err, tt.wantErr) } if !reflect.DeepEqual(got, tt.want) { @@ -127,13 +132,16 @@ func TestWithTemplateBase64(t *testing.T) { func TestWithTemplateFile(t *testing.T) { key := mustGeneratePublicKey(t) + cr := CertificateRequest{ + Key: key, + } data := TemplateData{ TypeKey: "user", KeyIDKey: "jane@doe.com", PrincipalsKey: []string{"jane", "jane@doe.com"}, ExtensionsKey: DefaultExtensions(UserCert), - InsecureKey: map[string]interface{}{ + InsecureKey: TemplateData{ UserKey: map[string]interface{}{ "username": "jane", }, @@ -143,7 +151,7 @@ func TestWithTemplateFile(t *testing.T) { type args struct { path string data TemplateData - key ssh.PublicKey + cr CertificateRequest } tests := []struct { name string @@ -151,7 +159,7 @@ func TestWithTemplateFile(t *testing.T) { want Options wantErr bool }{ - {"github.com", args{"./testdata/github.tpl", data, key}, Options{ + {"github.com", args{"./testdata/github.tpl", data, cr}, Options{ CertBuffer: bytes.NewBufferString(`{ "type": "user", "keyId": "jane@doe.com", @@ -159,13 +167,13 @@ func TestWithTemplateFile(t *testing.T) { "extensions": {"login@github.com":"jane","permit-X11-forwarding":"","permit-agent-forwarding":"","permit-port-forwarding":"","permit-pty":"","permit-user-rc":""} }`), }, false}, - {"missing", args{"./testdata/missing.tpl", data, key}, Options{}, true}, + {"missing", args{"./testdata/missing.tpl", data, cr}, Options{}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { var got Options fn := WithTemplateFile(tt.args.path, tt.args.data) - if err := fn(tt.args.key, &got); (err != nil) != tt.wantErr { + if err := fn(tt.args.cr, &got); (err != nil) != tt.wantErr { t.Errorf("WithTemplateFile() error = %v, wantErr %v", err, tt.wantErr) } if !reflect.DeepEqual(got, tt.want) { diff --git a/sshutil/templates.go b/sshutil/templates.go index c925da3a..8f0265ab 100644 --- a/sshutil/templates.go +++ b/sshutil/templates.go @@ -1,16 +1,14 @@ package sshutil -import "golang.org/x/crypto/ssh" - const ( - TypeKey = "Type" - KeyIDKey = "KeyID" - PrincipalsKey = "Principals" - ExtensionsKey = "Extensions" - TokenKey = "Token" - InsecureKey = "Insecure" - UserKey = "User" - PublicKey = "PublicKey" + TypeKey = "Type" + KeyIDKey = "KeyID" + PrincipalsKey = "Principals" + ExtensionsKey = "Extensions" + TokenKey = "Token" + InsecureKey = "Insecure" + UserKey = "User" + CertificateRequestKey = "CR" ) // TemplateError represents an error in a template produced by the fail @@ -117,10 +115,10 @@ func (t TemplateData) SetUserData(v interface{}) { t.SetInsecure(UserKey, v) } -// SetUserData sets the given user provided object in the insecure template -// data. -func (t TemplateData) SetPublicKey(v ssh.PublicKey) { - t.Set(PublicKey, v) +// SetCertificateRequest sets the simulated ssh certificate request the insecure +// template data. +func (t TemplateData) SetCertificateRequest(cr CertificateRequest) { + t.SetInsecure(CertificateRequestKey, cr) } // DefaultCertificate is the default template for an SSH certificate. @@ -130,3 +128,18 @@ const DefaultCertificate = `{ "principals": {{ toJson .Principals }}, "extensions": {{ toJson .Extensions }} }` + +const DefaultIIDCertificate = `{ + "type": "{{ .Type }}", +{{- if .Insecure.CR.KeyID }} + "keyId": "{{ .Insecure.CR.KeyID }}", +{{- else }} + "keyId": "{{ .KeyID }}", +{{- end}} +{{- if .Insecure.CR.Principals }} + "principals": {{ toJson .Insecure.CR.Principals }}, +{{- else }} + "principals": {{ toJson .Principals }}, +{{- end }} + "extensions": {{ toJson .Extensions }} +}` diff --git a/sshutil/templates_test.go b/sshutil/templates_test.go index ad550ccb..0d411c32 100644 --- a/sshutil/templates_test.go +++ b/sshutil/templates_test.go @@ -3,8 +3,6 @@ package sshutil import ( "reflect" "testing" - - "golang.org/x/crypto/ssh" ) func TestTemplateError_Error(t *testing.T) { @@ -374,11 +372,11 @@ func TestTemplateData_SetUserData(t *testing.T) { } } -func TestTemplateData_SetPublicKey(t *testing.T) { - k1 := mustGeneratePublicKey(t) - k2 := mustGeneratePublicKey(t) +func TestTemplateData_SetCertificateRequest(t *testing.T) { + cr1 := CertificateRequest{Key: mustGeneratePublicKey(t)} + cr2 := CertificateRequest{Key: mustGeneratePublicKey(t)} type args struct { - v ssh.PublicKey + cr CertificateRequest } tests := []struct { name string @@ -386,20 +384,28 @@ func TestTemplateData_SetPublicKey(t *testing.T) { args args want TemplateData }{ - {"ok", TemplateData{}, args{k1}, TemplateData{ - PublicKey: k1, + {"ok", TemplateData{}, args{cr1}, TemplateData{ + InsecureKey: TemplateData{ + CertificateRequestKey: cr1, + }, }}, {"overwrite", TemplateData{ - PublicKey: k1, - }, args{k2}, TemplateData{ - PublicKey: k2, + InsecureKey: TemplateData{ + UserKey: "data", + CertificateRequestKey: cr1, + }, + }, args{cr2}, TemplateData{ + InsecureKey: TemplateData{ + UserKey: "data", + CertificateRequestKey: cr2, + }, }}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - tt.t.SetPublicKey(tt.args.v) + tt.t.SetCertificateRequest(tt.args.cr) if !reflect.DeepEqual(tt.t, tt.want) { - t.Errorf("TemplateData.SetPublicKey() = %v, want %v", tt.t, tt.want) + t.Errorf("TemplateData.SetCertificateRequest() = %v, want %v", tt.t, tt.want) } }) }