Merge pull request #1030 from smallstep/herman/fix-template-validation

Add provisioner template validation
2022-08-25 14:51:39 -07:00 · 2022-08-25 14:51:39 -07:00 · 432477aa91
commit 432477aa91
parent dd6f59b538 1938b1bb34
5 changed files with 236 additions and 20 deletions
--- a/acme/api/middleware_test.go
+++ b/acme/api/middleware_test.go
@ -19,6 +19,7 @@ import (
 	"github.com/smallstep/certificates/acme"
 	"github.com/smallstep/nosql/database"
 	"go.step.sm/crypto/jose"
+	"go.step.sm/crypto/keyutil"
 )

 var testBody = []byte("foo")
@ -1136,6 +1137,8 @@ func TestHandler_validateJWS(t *testing.T) {
 			}
 		},
 		"fail/rsa-key-too-small": func(t *testing.T) test {
+			revert := keyutil.Insecure()
+			defer revert()
 			jwk, err := jose.GenerateJWK("RSA", "", "", "sig", "", 1024)
 			assert.FatalError(t, err)
 			pub := jwk.Public()
--- a/authority/admin/api/provisioner.go
+++ b/authority/admin/api/provisioner.go
@ -1,10 +1,13 @@
 package api

 import (
+	"fmt"
 	"net/http"

 	"github.com/go-chi/chi"

+	"go.step.sm/crypto/sshutil"
+	"go.step.sm/crypto/x509util"
 	"go.step.sm/linkedca"

 	"github.com/smallstep/certificates/api"
@ -89,6 +92,12 @@ func CreateProvisioner(w http.ResponseWriter, r *http.Request) {
 		return
 	}

+	// validate the templates and template data
+	if err := validateTemplates(prov.X509Template, prov.SshTemplate); err != nil {
+		render.Error(w, admin.WrapError(admin.ErrorBadRequestType, err, "invalid template"))
+		return
+	}
+
 	if err := mustAuthority(r.Context()).StoreProvisioner(r.Context(), prov); err != nil {
 		render.Error(w, admin.WrapErrorISE(err, "error storing provisioner %s", prov.Name))
 		return
@ -179,9 +188,47 @@ func UpdateProvisioner(w http.ResponseWriter, r *http.Request) {
 		return
 	}

+	// validate the templates and template data
+	if err := validateTemplates(nu.X509Template, nu.SshTemplate); err != nil {
+		render.Error(w, admin.WrapError(admin.ErrorBadRequestType, err, "invalid template"))
+		return
+	}
+
 	if err := auth.UpdateProvisioner(r.Context(), nu); err != nil {
 		render.Error(w, err)
 		return
 	}
 	render.ProtoJSON(w, nu)
 }
+
+// validateTemplates validates the X.509 and SSH templates and template data if set.
+func validateTemplates(x509, ssh *linkedca.Template) error {
+	if x509 != nil {
+		if len(x509.Template) > 0 {
+			if err := x509util.ValidateTemplate(x509.Template); err != nil {
+				return fmt.Errorf("invalid X.509 template: %w", err)
+			}
+		}
+		if len(x509.Data) > 0 {
+			if err := x509util.ValidateTemplateData(x509.Data); err != nil {
+				return fmt.Errorf("invalid X.509 template data: %w", err)
+			}
+		}
+	}
+
+	if ssh != nil {
+		if len(ssh.Template) > 0 {
+			if err := sshutil.ValidateTemplate(ssh.Template); err != nil {
+				return fmt.Errorf("invalid SSH template: %w", err)
+			}
+		}
+
+		if len(ssh.Data) > 0 {
+			if err := sshutil.ValidateTemplateData(ssh.Data); err != nil {
+				return fmt.Errorf("invalid SSH template data: %w", err)
+			}
+		}
+	}
+
+	return nil
+}
--- a/authority/admin/api/provisioner_test.go
+++ b/authority/admin/api/provisioner_test.go
@ -18,9 +18,9 @@ import (
 	"google.golang.org/protobuf/encoding/protojson"
 	"google.golang.org/protobuf/types/known/timestamppb"

+	"github.com/smallstep/assert"
 	"go.step.sm/linkedca"

-	"github.com/smallstep/assert"
 	"github.com/smallstep/certificates/authority/admin"
 	"github.com/smallstep/certificates/authority/provisioner"
 )
@ -349,6 +349,29 @@ func TestHandler_CreateProvisioner(t *testing.T) {
 		// "fail/authority.ValidateClaims": func(t *testing.T) test {
 		// 	return test{}
 		// },
+		"fail/validateTemplates": func(t *testing.T) test {
+			prov := &linkedca.Provisioner{
+				Id:   "provID",
+				Type: linkedca.Provisioner_OIDC,
+				Name: "provName",
+				X509Template: &linkedca.Template{
+					Template: []byte(`{ {{missingFunction "foo"}} }`),
+				},
+			}
+			body, err := protojson.Marshal(prov)
+			assert.FatalError(t, err)
+			return test{
+				ctx:        context.Background(),
+				body:       body,
+				statusCode: 400,
+				err: &admin.Error{
+					Type:    "badRequest",
+					Status:  400,
+					Detail:  "bad request",
+					Message: "invalid template: invalid X.509 template: error parsing template: template: template:1: function \"missingFunction\" not defined",
+				},
+			}
+		},
 		"fail/auth.StoreProvisioner": func(t *testing.T) test {
 			prov := &linkedca.Provisioner{
 				Id:   "provID",
@ -936,6 +959,61 @@ func TestHandler_UpdateProvisioner(t *testing.T) {
 		},
 		// TODO(hs): ValidateClaims can't be mocked atm
 		//"fail/ValidateClaims": func(t *testing.T) test { return test{} },
+		"fail/validateTemplates": func(t *testing.T) test {
+			chiCtx := chi.NewRouteContext()
+			chiCtx.URLParams.Add("name", "provName")
+			ctx := context.WithValue(context.Background(), chi.RouteCtxKey, chiCtx)
+			createdAt := time.Now()
+			var deletedAt time.Time
+			prov := &linkedca.Provisioner{
+				Id:          "provID",
+				Type:        linkedca.Provisioner_OIDC,
+				Name:        "provName",
+				AuthorityId: "authorityID",
+				CreatedAt:   timestamppb.New(createdAt),
+				DeletedAt:   timestamppb.New(deletedAt),
+				X509Template: &linkedca.Template{
+					Template: []byte("{ {{ missingFunction }} }"),
+				},
+			}
+			body, err := protojson.Marshal(prov)
+			assert.FatalError(t, err)
+			auth := &mockAdminAuthority{
+				MockLoadProvisionerByName: func(name string) (provisioner.Interface, error) {
+					assert.Equals(t, "provName", name)
+					return &provisioner.OIDC{
+						ID:   "provID",
+						Name: "provName",
+					}, nil
+				},
+			}
+			db := &admin.MockDB{
+				MockGetProvisioner: func(ctx context.Context, id string) (*linkedca.Provisioner, error) {
+					assert.Equals(t, "provID", id)
+					return &linkedca.Provisioner{
+						Id:          "provID",
+						Name:        "provName",
+						Type:        linkedca.Provisioner_OIDC,
+						AuthorityId: "authorityID",
+						CreatedAt:   timestamppb.New(createdAt),
+						DeletedAt:   timestamppb.New(deletedAt),
+					}, nil
+				},
+			}
+			return test{
+				ctx:        ctx,
+				body:       body,
+				auth:       auth,
+				adminDB:    db,
+				statusCode: 400,
+				err: &admin.Error{
+					Type:    "badRequest",
+					Status:  400,
+					Detail:  "bad request",
+					Message: "invalid template: invalid X.509 template: error parsing template: template: template:1: function \"missingFunction\" not defined",
+				},
+			}
+		},
 		"fail/auth.UpdateProvisioner": func(t *testing.T) test {
 			chiCtx := chi.NewRouteContext()
 			chiCtx.URLParams.Add("name", "provName")
@ -1107,3 +1185,87 @@ func TestHandler_UpdateProvisioner(t *testing.T) {
 		})
 	}
 }
+
+func Test_validateTemplates(t *testing.T) {
+	type args struct {
+		x509 *linkedca.Template
+		ssh  *linkedca.Template
+	}
+	tests := []struct {
+		name string
+		args args
+		err  error
+	}{
+		{
+			name: "ok",
+			args: args{},
+			err:  nil,
+		},
+		{
+			name: "ok/x509",
+			args: args{
+				x509: &linkedca.Template{
+					Template: []byte(`{"x": 1}`),
+				},
+			},
+			err: nil,
+		},
+		{
+			name: "ok/ssh",
+			args: args{
+				ssh: &linkedca.Template{
+					Template: []byte(`{"x": 1}`),
+				},
+			},
+			err: nil,
+		},
+		{
+			name: "fail/x509-template-missing-quote",
+			args: args{
+				x509: &linkedca.Template{
+					Template: []byte(`{ {{printf "%q" "quoted}} }`),
+				},
+			},
+			err: errors.New("invalid X.509 template: error parsing template: template: template:1: unterminated quoted string"),
+		},
+		{
+			name: "fail/x509-template-data",
+			args: args{
+				x509: &linkedca.Template{
+					Data: []byte(`{!?}`),
+				},
+			},
+			err: errors.New("invalid X.509 template data: error validating json template data"),
+		},
+		{
+			name: "fail/ssh-template-unknown-function",
+			args: args{
+				ssh: &linkedca.Template{
+					Template: []byte(`{ {{unknownFunction "foo"}} }`),
+				},
+			},
+			err: errors.New("invalid SSH template: error parsing template: template: template:1: function \"unknownFunction\" not defined"),
+		},
+		{
+			name: "fail/ssh-template-data",
+			args: args{
+				ssh: &linkedca.Template{
+					Data: []byte(`{!?}`),
+				},
+			},
+			err: errors.New("invalid SSH template data: error validating json template data"),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := validateTemplates(tt.args.x509, tt.args.ssh)
+			if tt.err != nil {
+				assert.Error(t, err)
+				assert.Equals(t, tt.err.Error(), err.Error())
+				return
+			}
+
+			assert.Nil(t, err)
+		})
+	}
+}
--- a/go.mod
+++ b/go.mod
@ -5,8 +5,19 @@ go 1.18
 require (
 	cloud.google.com/go v0.100.2
 	cloud.google.com/go/security v1.3.0
+	github.com/Azure/azure-sdk-for-go v65.0.0+incompatible // indirect
+	github.com/Azure/go-autorest/autorest v0.11.27 // indirect
+	github.com/Azure/go-autorest/autorest/azure/auth v0.5.11 // indirect
+	github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect
 	github.com/Masterminds/sprig/v3 v3.2.2
+	github.com/ThalesIgnite/crypto11 v1.2.5 // indirect
+	github.com/aws/aws-sdk-go v1.44.37 // indirect
+	github.com/dgraph-io/ristretto v0.0.4-0.20200906165740-41ebdbffecfd // indirect
+	github.com/fatih/color v1.9.0 // indirect
 	github.com/go-chi/chi v4.1.2+incompatible
+	github.com/go-kit/kit v0.10.0 // indirect
+	github.com/go-piv/piv-go v1.10.0 // indirect
+	github.com/go-sql-driver/mysql v1.6.0 // indirect
 	github.com/golang/mock v1.6.0
 	github.com/google/go-cmp v0.5.8
 	github.com/google/uuid v1.3.0
@ -14,6 +25,10 @@ require (
 	github.com/hashicorp/vault/api v1.3.1
 	github.com/hashicorp/vault/api/auth/approle v0.1.1
 	github.com/hashicorp/vault/api/auth/kubernetes v0.1.0
+	github.com/jhump/protoreflect v1.9.0 // indirect
+	github.com/kr/pretty v0.3.0 // indirect
+	github.com/mattn/go-colorable v0.1.8 // indirect
+	github.com/mattn/go-isatty v0.0.13 // indirect
 	github.com/micromdm/scep/v2 v2.1.0
 	github.com/newrelic/go-agent/v3 v3.18.0
 	github.com/pkg/errors v0.9.1
@ -26,10 +41,11 @@ require (
 	github.com/urfave/cli v1.22.4
 	go.mozilla.org/pkcs7 v0.0.0-20210826202110-33d05740a352
 	go.step.sm/cli-utils v0.7.3
-	go.step.sm/crypto v0.17.1
+	go.step.sm/crypto v0.18.0
 	go.step.sm/linkedca v0.18.0
 	golang.org/x/crypto v0.0.0-20211215153901-e495a2d5b3d3
 	golang.org/x/net v0.0.0-20220607020251-c690dde0001d
+	golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba // indirect
 	google.golang.org/api v0.84.0
 	google.golang.org/genproto v0.0.0-20220617124728-180714bec0ad
 	google.golang.org/grpc v1.47.0
@ -43,23 +59,17 @@ require (
 	cloud.google.com/go/kms v1.4.0 // indirect
 	filippo.io/edwards25519 v1.0.0-rc.1 // indirect
 	github.com/AndreasBriese/bbloom v0.0.0-20190825152654-46b345b51c96 // indirect
-	github.com/Azure/azure-sdk-for-go v65.0.0+incompatible // indirect
 	github.com/Azure/go-autorest v14.2.0+incompatible // indirect
-	github.com/Azure/go-autorest/autorest v0.11.27 // indirect
 	github.com/Azure/go-autorest/autorest/adal v0.9.18 // indirect
-	github.com/Azure/go-autorest/autorest/azure/auth v0.5.11 // indirect
 	github.com/Azure/go-autorest/autorest/azure/cli v0.4.5 // indirect
-	github.com/Azure/go-autorest/autorest/date v0.3.0 // indirect
 	github.com/Azure/go-autorest/autorest/to v0.4.0 // indirect
 	github.com/Azure/go-autorest/autorest/validation v0.3.1 // indirect
 	github.com/Azure/go-autorest/logger v0.2.1 // indirect
 	github.com/Azure/go-autorest/tracing v0.6.0 // indirect
 	github.com/Masterminds/goutils v1.1.1 // indirect
 	github.com/Masterminds/semver/v3 v3.1.1 // indirect
-	github.com/ThalesIgnite/crypto11 v1.2.5 // indirect
 	github.com/armon/go-metrics v0.3.9 // indirect
 	github.com/armon/go-radix v1.0.0 // indirect
-	github.com/aws/aws-sdk-go v1.44.37 // indirect
 	github.com/cenkalti/backoff/v3 v3.0.0 // indirect
 	github.com/cespare/xxhash v1.1.0 // indirect
 	github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e // indirect
@ -67,15 +77,10 @@ require (
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/dgraph-io/badger v1.6.2 // indirect
 	github.com/dgraph-io/badger/v2 v2.2007.4 // indirect
-	github.com/dgraph-io/ristretto v0.0.4-0.20200906165740-41ebdbffecfd // indirect
 	github.com/dgryski/go-farm v0.0.0-20200201041132-a6ae2369ad13 // indirect
 	github.com/dimchansky/utfbom v1.1.1 // indirect
 	github.com/dustin/go-humanize v1.0.0 // indirect
-	github.com/fatih/color v1.9.0 // indirect
-	github.com/go-kit/kit v0.10.0 // indirect
 	github.com/go-logfmt/logfmt v0.5.0 // indirect
-	github.com/go-piv/piv-go v1.10.0 // indirect
-	github.com/go-sql-driver/mysql v1.6.0 // indirect
 	github.com/golang-jwt/jwt/v4 v4.2.0 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
@ -109,12 +114,9 @@ require (
 	github.com/jackc/pgservicefile v0.0.0-20200714003250-2b9c44734f2b // indirect
 	github.com/jackc/pgtype v1.9.0 // indirect
 	github.com/jackc/pgx/v4 v4.14.0 // indirect
-	github.com/jhump/protoreflect v1.9.0 // indirect
 	github.com/jmespath/go-jmespath v0.4.0 // indirect
 	github.com/klauspost/compress v1.12.3 // indirect
 	github.com/manifoldco/promptui v0.9.0 // indirect
-	github.com/mattn/go-colorable v0.1.8 // indirect
-	github.com/mattn/go-isatty v0.0.13 // indirect
 	github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d // indirect
 	github.com/miekg/pkcs11 v1.1.1 // indirect
 	github.com/mitchellh/copystructure v1.2.0 // indirect
@ -137,7 +139,6 @@ require (
 	golang.org/x/oauth2 v0.0.0-20220608161450-d0670ef3b1eb // indirect
 	golang.org/x/sys v0.0.0-20220610221304-9f5ed59c137d // indirect
 	golang.org/x/text v0.3.8-0.20211004125949-5bd84dd9b33b // indirect
-	golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba // indirect
 	google.golang.org/appengine v1.6.7 // indirect
 	gopkg.in/yaml.v3 v3.0.0 // indirect
 )
--- a/go.sum
+++ b/go.sum
@ -526,8 +526,9 @@ github.com/konsorten/go-windows-terminal-sequences v1.0.2/go.mod h1:T0+1ngSBFLxv
 github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc=
 github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
 github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
-github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI=
 github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
+github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0=
+github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk=
 github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/pty v1.1.8/go.mod h1:O1sed60cT9XZ5uDucP5qwvh+TE3NnUj51EiZO/lmSfw=
 github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
@ -664,6 +665,8 @@ github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a/go.mod h1:bCqn
 github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg=
 github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ=
 github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
+github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k=
+github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=
 github.com/rs/xid v1.2.1 h1:mhH9Nq+C1fY2l1XIpgxIiUOfNpRBYH1kKcr+qfKgjRc=
 github.com/rs/xid v1.2.1/go.mod h1:+uKXf+4Djp6Md1KODXJxgGQPKngRmWyn10oCKFzNHOQ=
 github.com/rs/zerolog v1.13.0/go.mod h1:YbFCdg8HfsridGWAh22vktObvhZbQsZXe4/zB0OKkWU=
@ -767,8 +770,8 @@ go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqe
 go.step.sm/cli-utils v0.7.3 h1:IA12IaiXVCI18yOFVQuvMpyvjL8wuwUn1yO+KhAVAr0=
 go.step.sm/cli-utils v0.7.3/go.mod h1:RJRwbBLqzs5nrepQLAV9FuT3fVpWz66tKzLIB7Izpfk=
 go.step.sm/crypto v0.9.0/go.mod h1:+CYG05Mek1YDqi5WK0ERc6cOpKly2i/a5aZmU1sfGj0=
-go.step.sm/crypto v0.17.1 h1:uKpJNvzVy/GKR28hJbW8VCbfcKKBDnGNBYCKhAp2TSg=
-go.step.sm/crypto v0.17.1/go.mod h1:FXFiLBUsoE0OGz8JTjxhYU1rwKKNgVIb5izZTUMdc/8=
+go.step.sm/crypto v0.18.0 h1:saD/tMG7uKJmUIPyOyudidVTHPnozTU02CDd+oqwKn0=
+go.step.sm/crypto v0.18.0/go.mod h1:qZ+pNU1nV+THwP7TPTNCRMRr9xrRURhETTAK7U5psfw=
 go.step.sm/linkedca v0.18.0 h1:uxRBd2WDvJNZ2i0nJm/QmG4lkRxWoebYKJinchX7T7o=
 go.step.sm/linkedca v0.18.0/go.mod h1:qSuYlIIhvPmA2+DSSS03E2IXhbXWTLW61Xh9zDQJ3VM=
 go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=