Merge branch 'master' into context-authority

authority/policy.go

@@ -137,7 +137,7 @@ func (a *Authority) checkAuthorityPolicy(ctx context.Context, currentAdmin *link
 	return a.checkPolicy(ctx, currentAdmin, allAdmins, p)
 }
 
-func (a *Authority) checkProvisionerPolicy(ctx context.Context, currentAdmin *linkedca.Admin, provName string, p *linkedca.Policy) error {
+func (a *Authority) checkProvisionerPolicy(ctx context.Context, provName string, p *linkedca.Policy) error {
 
 	// no policy and thus nothing to evaluate; return early
 	if p == nil {
@@ -147,7 +147,11 @@ func (a *Authority) checkProvisionerPolicy(ctx context.Context, currentAdmin *li
 	// get all admins for the provisioner; ignoring case in which they're not found
 	allProvisionerAdmins, _ := a.admins.LoadByProvisioner(provName)
 
-	return a.checkPolicy(ctx, currentAdmin, allProvisionerAdmins, p)
+	// check the policy; pass in nil as the current admin, as all admins for the
+	// provisioner will be checked by looping through allProvisionerAdmins. Also,
+	// the current admin may be a super admin not belonging to the provisioner, so
+	// can't be blocked, but is not required to be in the policy, either.
+	return a.checkPolicy(ctx, nil, allProvisionerAdmins, p)
 }
 
 // checkPolicy checks if a new or updated policy configuration results in the user
@@ -178,10 +182,13 @@ func (a *Authority) checkPolicy(ctx context.Context, currentAdmin *linkedca.Admi
 
 	// check if the admin user that instructed the authority policy to be
 	// created or updated, would still be allowed when the provided policy
-	// would be applied.
+	// would be applied. This case is skipped when current admin is nil, which
-	sans := []string{currentAdmin.GetSubject()}
+	// is the case when a provisioner policy is checked.
-	if err := isAllowed(engine, sans); err != nil {
+	if currentAdmin != nil {
-		return err
+		sans := []string{currentAdmin.GetSubject()}
+		if err := isAllowed(engine, sans); err != nil {
+			return err
+		}
 	}
 
 	// loop through admins to verify that none of them would be
@@ -189,7 +196,7 @@ func (a *Authority) checkPolicy(ctx context.Context, currentAdmin *linkedca.Admi
 	// an error with a message that includes the admin subject that
 	// would be locked out.
 	for _, adm := range otherAdmins {
-		sans = []string{adm.GetSubject()}
+		sans := []string{adm.GetSubject()}
 		if err := isAllowed(engine, sans); err != nil {
 			return err
 		}

authority/policy_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"gopkg.in/square/go-jose.v2"
 
 	"go.step.sm/linkedca"
 
@@ -917,13 +918,58 @@ func TestAuthority_checkAuthorityPolicy(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "fail/policy",
+			fields: fields{
+				admins: administrator.NewCollection(nil),
+				adminDB: &admin.MockDB{
+					MockGetAdmins: func(ctx context.Context) ([]*linkedca.Admin, error) {
+						return []*linkedca.Admin{
+							{
+								Id:      "adminID1",
+								Subject: "anotherAdmin",
+							},
+							{
+								Id:      "adminID2",
+								Subject: "step",
+							},
+							{
+								Id:      "adminID3",
+								Subject: "otherAdmin",
+							},
+						}, nil
+					},
+				},
+			},
+			args: args{
+				currentAdmin: &linkedca.Admin{Subject: "step"},
+				provName:     "prov",
+				p: &linkedca.Policy{
+					X509: &linkedca.X509Policy{
+						Allow: &linkedca.X509Names{
+							Dns: []string{"step", "otherAdmin"},
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
 		{
 			name: "ok",
 			fields: fields{
 				admins: administrator.NewCollection(nil),
 				adminDB: &admin.MockDB{
 					MockGetAdmins: func(ctx context.Context) ([]*linkedca.Admin, error) {
-						return []*linkedca.Admin{}, nil
+						return []*linkedca.Admin{
+							{
+								Id:      "adminID2",
+								Subject: "step",
+							},
+							{
+								Id:      "adminID3",
+								Subject: "otherAdmin",
+							},
+						}, nil
 					},
 				},
 			},
@@ -957,6 +1003,20 @@ func TestAuthority_checkAuthorityPolicy(t *testing.T) {
 }
 
 func TestAuthority_checkProvisionerPolicy(t *testing.T) {
+	jwkProvisioner := &provisioner.JWK{
+		ID:   "jwkID",
+		Type: "JWK",
+		Name: "jwkProv",
+		Key:  &jose.JSONWebKey{KeyID: "jwkKeyID"},
+	}
+	provisioners := provisioner.NewCollection(testAudiences)
+	provisioners.Store(jwkProvisioner)
+	admins := administrator.NewCollection(provisioners)
+	admins.Store(&linkedca.Admin{
+		Id:            "adminID",
+		Subject:       "step",
+		ProvisionerId: "jwkID",
+	}, jwkProvisioner)
 	type fields struct {
 		provisioners *provisioner.Collection
 		admins       *administrator.Collection
@@ -964,10 +1024,9 @@ func TestAuthority_checkProvisionerPolicy(t *testing.T) {
 		adminDB      admin.DB
 	}
 	type args struct {
-		ctx          context.Context
+		ctx      context.Context
-		currentAdmin *linkedca.Admin
+		provName string
-		provName     string
+		p        *linkedca.Policy
-		p            *linkedca.Policy
 	}
 	tests := []struct {
 		name    string
@@ -979,20 +1038,37 @@ func TestAuthority_checkProvisionerPolicy(t *testing.T) {
 			name:   "no policy",
 			fields: fields{},
 			args: args{
-				currentAdmin: nil,
+				provName: "prov",
-				provName:     "prov",
+				p:        nil,
-				p:            nil,
 			},
 			wantErr: false,
 		},
 		{
-			name: "ok",
+			name: "fail/policy",
 			fields: fields{
-				admins: administrator.NewCollection(nil),
+				provisioners: provisioners,
+				admins:       admins,
 			},
 			args: args{
-				currentAdmin: &linkedca.Admin{Subject: "step"},
+				provName: "jwkProv",
-				provName:     "prov",
+				p: &linkedca.Policy{
+					X509: &linkedca.X509Policy{
+						Allow: &linkedca.X509Names{
+							Dns: []string{"otherAdmin"}, // step not in policy
+						},
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "ok",
+			fields: fields{
+				provisioners: provisioners,
+				admins:       admins,
+			},
+			args: args{
+				provName: "jwkProv",
 				p: &linkedca.Policy{
 					X509: &linkedca.X509Policy{
 						Allow: &linkedca.X509Names{
@@ -1012,7 +1088,7 @@ func TestAuthority_checkProvisionerPolicy(t *testing.T) {
 				db:           tt.fields.db,
 				adminDB:      tt.fields.adminDB,
 			}
-			if err := a.checkProvisionerPolicy(tt.args.ctx, tt.args.currentAdmin, tt.args.provName, tt.args.p); (err != nil) != tt.wantErr {
+			if err := a.checkProvisionerPolicy(tt.args.ctx, tt.args.provName, tt.args.p); (err != nil) != tt.wantErr {
 				t.Errorf("Authority.checkProvisionerPolicy() error = %v, wantErr %v", err, tt.wantErr)
 			}
 		})

authority/provisioners.go

@@ -173,9 +173,7 @@ func (a *Authority) StoreProvisioner(ctx context.Context, prov *linkedca.Provisi
 		return admin.WrapErrorISE(err, "error generating provisioner config")
 	}
 
-	adm := linkedca.MustAdminFromContext(ctx)
+	if err := a.checkProvisionerPolicy(ctx, prov.Name, prov.Policy); err != nil {
-
-	if err := a.checkProvisionerPolicy(ctx, adm, prov.Name, prov.Policy); err != nil {
 		return err
 	}
 
@@ -224,9 +222,7 @@ func (a *Authority) UpdateProvisioner(ctx context.Context, nu *linkedca.Provisio
 		return admin.WrapErrorISE(err, "error generating provisioner config")
 	}
 
-	adm := linkedca.MustAdminFromContext(ctx)
+	if err := a.checkProvisionerPolicy(ctx, nu.Name, nu.Policy); err != nil {
-
-	if err := a.checkProvisionerPolicy(ctx, adm, nu.Name, nu.Policy); err != nil {
 		return err
 	}
 

pki/helm.go

@@ -68,6 +68,10 @@ inject:
         federateRoots: []
         crt: {{ .Intermediate }}
         key: {{ .IntermediateKey }}
+	{{- if .Kms }}
+	kms:
+	  type: {{ lower (.Kms.Type | toString) }}
+	{{- end }}
         {{- if .EnableSSH }}
         ssh:
           hostKey: {{ .Ssh.HostKey }}

scep/api/api.go

@@ -109,7 +109,7 @@ func Get(w http.ResponseWriter, r *http.Request) {
 	case opnGetCACaps:
 		res, err = GetCACaps(ctx)
 	case opnPKIOperation:
-		// TODO: implement the GET for PKI operation? Default CACAPS doesn't specify this is in use, though
+		res, err = h.PKIOperation(ctx, req)
 	default:
 		err = fmt.Errorf("unknown operation: %s", req.Operation)
 	}
@@ -170,8 +170,8 @@ func decodeRequest(r *http.Request) (request, error) {
 			if _, ok := query["message"]; ok {
 				message = query.Get("message")
 			}
-			// TODO: verify this; it seems like it should be StdEncoding instead of URLEncoding
+			// TODO: verify this; right type of encoding? Needs additional transformations?
-			decodedMessage, err := base64.URLEncoding.DecodeString(message)
+			decodedMessage, err := base64.StdEncoding.DecodeString(message)
 			if err != nil {
 				return request{}, err
 			}

scep/api/api_test.go

@@ -0,0 +1,113 @@
+// Package api implements a SCEP HTTP server.
+package api
+
+import (
+	"bytes"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"reflect"
+	"testing"
+	"testing/iotest"
+)
+
+func Test_decodeRequest(t *testing.T) {
+	type args struct {
+		r *http.Request
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    request
+		wantErr bool
+	}{
+		{
+			name: "fail/unsupported-method",
+			args: args{
+				r: httptest.NewRequest(http.MethodPatch, "http://scep:8080/?operation=AnUnsupportOperation", nil),
+			},
+			want:    request{},
+			wantErr: true,
+		},
+		{
+			name: "fail/get-unsupported-operation",
+			args: args{
+				r: httptest.NewRequest(http.MethodGet, "http://scep:8080/?operation=AnUnsupportOperation", nil),
+			},
+			want:    request{},
+			wantErr: true,
+		},
+		{
+			name: "fail/get-PKIOperation",
+			args: args{
+				r: httptest.NewRequest(http.MethodGet, "http://scep:8080/?operation=PKIOperation&message='somewronginput'", nil),
+			},
+			want:    request{},
+			wantErr: true,
+		},
+		{
+			name: "fail/post-PKIOperation",
+			args: args{
+				r: httptest.NewRequest(http.MethodPost, "http://scep:8080/?operation=PKIOperation", iotest.ErrReader(errors.New("a read error"))),
+			},
+			want:    request{},
+			wantErr: true,
+		},
+		{
+			name: "ok/get-GetCACert",
+			args: args{
+				r: httptest.NewRequest(http.MethodGet, "http://scep:8080/?operation=GetCACert", nil),
+			},
+			want: request{
+				Operation: "GetCACert",
+				Message:   []byte{},
+			},
+			wantErr: false,
+		},
+		{
+			name: "ok/get-GetCACaps",
+			args: args{
+				r: httptest.NewRequest(http.MethodGet, "http://scep:8080/?operation=GetCACaps", nil),
+			},
+			want: request{
+				Operation: "GetCACaps",
+				Message:   []byte{},
+			},
+			wantErr: false,
+		},
+		{
+			name: "ok/get-PKIOperation",
+			args: args{
+				r: httptest.NewRequest(http.MethodGet, "http://scep:8080/?operation=PKIOperation&message=MTIzNA==", nil),
+			},
+			want: request{
+				Operation: "PKIOperation",
+				Message:   []byte("1234"),
+			},
+			wantErr: false,
+		},
+		{
+			name: "ok/post-PKIOperation",
+			args: args{
+				r: httptest.NewRequest(http.MethodPost, "http://scep:8080/?operation=PKIOperation", bytes.NewBufferString("1234")),
+			},
+			want: request{
+				Operation: "PKIOperation",
+				Message:   []byte("1234"),
+			},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := decodeRequest(tt.args.r)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("decodeRequest() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("decodeRequest() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}