Allow SPIFFEs to create keys

client/create.go

@@ -1,6 +1,7 @@
 package client
 
 import (
+	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -13,7 +14,7 @@ func init() {
 }
 
 var cmdCreate = &Command{
-	UsageLine: "create [--key-template template_name] <key_identifier>",
+	UsageLine: "create [--acl key_acl] [--key-template template_name] <key_identifier>",
 	Short:     "creates a new key",
 	Long: `
 Create will create a new key in knox with input as the primary key version. Key data should be sent to stdin unless a key-template is specified.
@@ -26,14 +27,43 @@ Please run "knox create --key-template <template_name> <key_identifier>".
 
 The original key version id will be print to stdout.
 
-To create a new key, user credentials are required. The default access list will include the creator of this key and a limited set of site reliablity and security engineers.
+Only users or SPIFFEs can create a new key. For SPIFFEs, an ACL must be provided with at least 2 users/groups set as admins.
+The default ACL will include a limited set of site reliablity and security engineers, and the creator if they are a user.
 
 For more about knox, see https://github.com/pinterest/knox.
 
 See also: knox add, knox get
 	`,
 }
 var createTinkKeyset = cmdCreate.Flag.String("key-template", "", "name of a knox-supported Tink key template")
+var createAcl = cmdCreate.Flag.String("acl", "", "ACL for the created key")
+
+func parseAcl(aclString string) (knox.ACL, error) {
+	var err error
+	var accessList []knox.Access
+
+	if aclString == "" {
+		return knox.ACL{}, nil
+	}
+
+	err = json.Unmarshal([]byte(aclString), &accessList)
+	if err != nil {
+		return nil, err
+	}
+
+	acl := knox.ACL(accessList)
+
+	err = acl.Validate()
+	if err != nil {
+		return nil, err
+	}
+	err = acl.ValidateHasMultipleHumanAdmins()
+	if err != nil {
+		return nil, err
+	}
+
+	return acl, nil
+}
 
 func runCreate(cmd *Command, args []string) *ErrorStatus {
 	if len(args) != 1 {
@@ -55,8 +85,13 @@ func runCreate(cmd *Command, args []string) *ErrorStatus {
 	if err != nil {
 		return &ErrorStatus{err, false}
 	}
-	// TODO(devinlundberg): allow ACL to be entered as input
-	acl := knox.ACL{}
+
+	var acl knox.ACL
+	acl, err = parseAcl(*createAcl)
+	if err != nil {
+		return &ErrorStatus{fmt.Errorf("Error parsing ACL: %s", err.Error()), false}
+	}
+
 	versionID, err := cli.CreateKey(keyID, data, acl)
 	if err != nil {
 		return &ErrorStatus{fmt.Errorf("Error adding version: %s", err.Error()), true}

client/create_test.go

@@ -0,0 +1,77 @@
+package client
+
+import (
+	"testing"
+
+	"github.com/pinterest/knox"
+)
+
+func testAclEq(a, b knox.ACL) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i := range a {
+		if a[i] != b[i] {
+			return false
+		}
+	}
+	return true
+}
+
+func TestParseAcl(t *testing.T) {
+	machineAdmin := knox.Access{ID: "testmachine1", AccessType: knox.Admin, Type: knox.Machine}
+	userWrite := knox.Access{ID: "testuser", AccessType: knox.Write, Type: knox.User}
+	userAdmin := knox.Access{ID: "testuser", AccessType: knox.Admin, Type: knox.User}
+	groupAdmin := knox.Access{ID: "testgroup", AccessType: knox.Admin, Type: knox.UserGroup}
+
+	validAclNoHumanAdmin := knox.ACL([]knox.Access{machineAdmin, userWrite})
+	validAclWithOneHumanAdmin := knox.ACL([]knox.Access{machineAdmin, userAdmin})
+	validAclWithTwoHumanAdmins := knox.ACL([]knox.Access{machineAdmin, userAdmin, groupAdmin})
+	blankAcl := knox.ACL{}
+
+	testCases := []struct {
+		str string
+		acl knox.ACL
+		errMsg string
+	}{
+		{
+			`[{"type":"foo","id":"bar","access":"test"}]`,
+			validAclNoHumanAdmin,  // ACL does not matter here
+			"json: Invalid AccessType to convert",
+		},
+		{
+			`[{"type":"User","id":"testuser","access":"Write"}, {"type":"Machine","id":"testmachine1","access":"Admin"}]`,
+			validAclNoHumanAdmin,
+			"ACL needs to have at least 2 users/groups set as admins", // User only has Write access
+		},
+		{
+			`[{"type":"Machine","id":"testmachine1","access":"Admin"}, {"type":"User","id":"testuser","access":"Admin"}]`,
+			validAclWithOneHumanAdmin,
+			"ACL needs to have at least 2 users/groups set as admins", // Only 1 human admin
+		},
+		{
+			`[{"type":"Machine","id":"testmachine1","access":"Admin"}, {"type":"User","id":"testuser","access":"Admin"}, {"type":"UserGroup","id":"testgroup","access":"Admin"}]`,
+			validAclWithTwoHumanAdmins,
+			"", // Success, no error
+		},
+		// Original, and default, behaviour is a blank ACL
+		{
+			``,
+			blankAcl,
+			"", // Success, no error
+		},
+	}
+
+	for _, tc := range testCases {
+		acl, err := parseAcl(tc.str)
+		if tc.errMsg != "" || err != nil {
+			if err.Error() != tc.errMsg {
+				t.Fatalf("%v should equal %v", tc.errMsg, err.Error())
+			}
+		} else{
+			if !testAclEq(acl, tc.acl) {
+				t.Fatalf("%v should equal %v", acl, tc.acl)
+			}
+		}
+	}
+}

client/register_test.go

@@ -17,12 +17,12 @@ func TestParseTimeout(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		r, err := parseTimeout(tc.str)
+		timeout, err := parseTimeout(tc.str)
 		if err != nil {
 			t.Errorf("error parsing value %s: %s", tc.str, err)
 			continue
 		}
-		if r != tc.dur {
+		if timeout != tc.dur {
 			t.Errorf("mismatch: %s should parse to %s", tc.str, tc.dur.String())
 		}
 	}

client_test.go

@@ -11,6 +11,10 @@ import (
 	"testing"
 )
 
+var DataBytes = []byte("data")
+// I.e. b64encode("data")
+const DataB64Encoded = "ZGF0YQ=="
+
 func TestMockClient(t *testing.T) {
 	p := "primary"
 	a := []string{"active1", "active2"}
@@ -181,8 +185,8 @@ func TestCreateKey(t *testing.T) {
 			t.Fatalf("%s is not %s", r.URL.Path, "/v0/keys/")
 		}
 		r.ParseForm()
-		if r.PostForm["data"][0] != "ZGF0YQ==" {
-			t.Fatalf("%s is not expected: %s", r.PostForm["data"][0], "ZGF0YQ==")
+		if r.PostForm["data"][0] != DataB64Encoded {
+			t.Fatalf("%s is not expected: %s", r.PostForm["data"][0], DataB64Encoded)
 		}
 		if r.PostForm["id"][0] != "testkey" {
 			t.Fatalf("%s is not expected: %s", r.PostForm["id"][0], "testkey")
@@ -195,27 +199,60 @@ func TestCreateKey(t *testing.T) {
 
 	cli := MockClient(srv.Listener.Addr().String(), "")
 
-	acl := ACL([]Access{
+	aclWithMultipleUserAdmins := ACL([]Access{
+		{
+			Type:       User,
+			AccessType: Admin,
+			ID:         "test",
+		},
+		{
+			Type:       User,
+			AccessType: Admin,
+			ID:         "test2",
+		},
+	})
+	aclWithUserRead := ACL([]Access{
 		{
 			Type:       User,
 			AccessType: Read,
 			ID:         "test",
 		},
 	})
 
-	badACL := ACL([]Access{
+	invalidTypeACL := ACL([]Access{
 		{
-			Type:       233,
-			AccessType: 80927,
+			Type:       233,  // Not a principal
+			AccessType: Read,
 			ID:         "test",
 		},
 	})
-	_, err = cli.CreateKey("testkey", []byte("data"), badACL)
-	if err == nil {
-		t.Fatal("error is nil")
+	_, err = cli.CreateKey("testkey", DataBytes, invalidTypeACL)
+	if err.Error() != "json: error calling MarshalJSON for type knox.PrincipalType: json: Invalid PrincipalType to convert" {
+		t.Fatalf("Expected err is %v", err)
+	}
+
+	invalidAccessTypeACL := ACL([]Access{
+		{
+			Type:       User,
+			AccessType: 80927, // Not an access type
+			ID:         "test",
+		},
+	})
+	_, err = cli.CreateKey("testkey", DataBytes, invalidAccessTypeACL)
+	if err.Error() != "json: error calling MarshalJSON for type knox.AccessType: json: Invalid AccessType to convert" {
+		t.Fatalf("Expected err is %v", err)
+	}
+
+	// Note: In `client/create.go` and `server/routes.go`, acl.ValidateHasMultipleHumanAdmins() would be called, but not in `client.go`
+	k, err := cli.CreateKey("testkey", DataBytes, aclWithUserRead)
+	if err != nil {
+		t.Fatalf("%s is not nil", err)
+	}
+	if k != expected {
+		t.Fatalf("%d is not %d", k, expected)
 	}
 
-	k, err := cli.CreateKey("testkey", []byte("data"), acl)
+	k, err = cli.CreateKey("testkey", DataBytes, aclWithMultipleUserAdmins)
 	if err != nil {
 		t.Fatalf("%s is not nil", err)
 	}
@@ -238,15 +275,15 @@ func TestAddVersion(t *testing.T) {
 			t.Fatalf("%s is not %s", r.URL.Path, "/v0/keys/testkey/versions/")
 		}
 		r.ParseForm()
-		if r.PostForm["data"][0] != "ZGF0YQ==" {
-			t.Fatalf("%s is not expected: %s", r.PostForm["data"][0], "ZGF0YQ==")
+		if r.PostForm["data"][0] != DataB64Encoded {
+			t.Fatalf("%s is not expected: %s", r.PostForm["data"][0], DataB64Encoded)
 		}
 	})
 	defer srv.Close()
 
 	cli := MockClient(srv.Listener.Addr().String(), "")
 
-	k, err := cli.AddVersion("testkey", []byte("data"))
+	k, err := cli.AddVersion("testkey", DataBytes)
 	if err != nil {
 		t.Fatalf("%s is not nil", err)
 	}

knox.go

@@ -17,6 +17,7 @@ var (
 	ErrACLDuplicateEntries = fmt.Errorf("Duplicate entries in ACL")
 	ErrACLContainsNone     = fmt.Errorf("ACL contains None access")
 	ErrACLEmptyPrincipal   = fmt.Errorf("Principals of type user, user group, machine, or machine prefix may not be empty.")
+	ErrACLDoesNotContainMultipleHumanAdmins = fmt.Errorf("ACL needs to have at least 2 users/groups set as admins")
 
 	ErrACLInvalidService               = fmt.Errorf("Service is invalid, must conform to 'spiffe://<domain>/<path>' format.")
 	ErrACLInvalidServicePrefixURL      = fmt.Errorf("Service prefix is invalid URL, must conform to 'spiffe://<domain>/<path>/' format.")
@@ -43,7 +44,7 @@ const (
 	spiffeScheme = "spiffe"
 )
 
-// InvalidTypeError is an error for to throw when in the json conversion.
+// An invalidTypeError is an error to throw when in the json conversion.
 type invalidTypeError struct {
 	badType string
 }
@@ -320,6 +321,26 @@ func (acl ACL) Validate() error {
 	return nil
 }
 
+// ValidateHasMultipleHumanAdmins ensures the ACL has at least 2 users/groups set as 'Admin's.
+// This is for when SPIFFEs create keys, we want to ensure there are multiple human owners set.
+// Intended to be called separately from Validate().
+func (acl ACL) ValidateHasMultipleHumanAdmins() error {
+	var humanAdminCount uint64
+	humanAdminCount = 0
+	for _, a := range acl {
+		if a.AccessType != Admin {
+			continue;
+		}
+		if a.Type == User || a.Type == UserGroup {
+			humanAdminCount++
+		}
+		if humanAdminCount > 1 {
+			return nil
+		}
+	}
+	return ErrACLDoesNotContainMultipleHumanAdmins
+}
+
 // Add appends an access to the ACL. It does so by overwriting any existing access
 // that principal or group may have had.
 func (acl ACL) Add(a Access) ACL {
@@ -588,6 +609,8 @@ const (
 	BadRequestDataCode
 	BadKeyFormatCode
 	BadPrincipalIdentifier
+	BadAclCode
+	NoMultipleHumanAdminsInAclCode
 )
 
 // Response is the format for responses from the api server.