Merge pull request #16 from google/fix

Use Description when placing keys in the keyring

2 files changed, 28 insertions, 26 deletions

diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go
index 8b63457..5655fef 100644
--- a/crypto/crypto_test.go
+++ b/crypto/crypto_test.go
@@ -237,27 +237,36 @@ func TestKeyLargeResize(t *testing.T) {
 // Adds and removes a key with various services.
 func TestAddRemoveKeys(t *testing.T) {
 	for _, service := range []string{DefaultService, "ext4:", "f2fs:"} {
-		if err := InsertPolicyKey(fakeValidPolicyKey, fakeValidDescriptor, service); err != nil {
+		validDescription := service + fakeValidDescriptor
+		if err := InsertPolicyKey(fakeValidPolicyKey, validDescription); err != nil {
 			t.Error(err)
 		}
-		if err := RemovePolicyKey(fakeValidDescriptor, service); err != nil {
+		if err := RemovePolicyKey(validDescription); err != nil {
 			t.Error(err)
 		}
 	}
 }
 
-// Makes sure a key fails with bad descriptor, policy, or service
+// Adds a key twice (both should succeed)
+func TestAddTwice(t *testing.T) {
+	validDescription := DefaultService + fakeValidDescriptor
+	InsertPolicyKey(fakeValidPolicyKey, validDescription)
+	if InsertPolicyKey(fakeValidPolicyKey, validDescription) != nil {
+		t.Error("InsertPolicyKey should not fail if key already exists")
+	}
+	RemovePolicyKey(validDescription)
+}
+
+// Makes sure a key fails with bad policy or service
 func TestBadAddKeys(t *testing.T) {
-	if InsertPolicyKey(fakeInvalidPolicyKey, fakeValidDescriptor, DefaultService) == nil {
-		RemovePolicyKey(fakeValidDescriptor, DefaultService)
+	validDescription := DefaultService + fakeValidDescriptor
+	if InsertPolicyKey(fakeInvalidPolicyKey, validDescription) == nil {
+		RemovePolicyKey(validDescription)
 		t.Error("InsertPolicyKey should fail with bad policy key")
 	}
-	if InsertPolicyKey(fakeValidPolicyKey, fakeInvalidDescriptor, DefaultService) == nil {
-		RemovePolicyKey(fakeInvalidDescriptor, DefaultService)
-		t.Error("InsertPolicyKey should fail with bad descriptor")
-	}
-	if InsertPolicyKey(fakeValidPolicyKey, fakeValidDescriptor, "ext4") == nil {
-		RemovePolicyKey(fakeValidDescriptor, "ext4")
+	invalidDescription := "ext4" + fakeValidDescriptor
+	if InsertPolicyKey(fakeValidPolicyKey, invalidDescription) == nil {
+		RemovePolicyKey(invalidDescription)
 		t.Error("InsertPolicyKey should fail with bad service")
 	}
 }
diff --git a/crypto/key.go b/crypto/key.go
index 6781c1d..cffe2b4 100644
--- a/crypto/key.go
+++ b/crypto/key.go
@@ -240,15 +240,14 @@ func getKeyring() (int, error) {
 }
 
 // FindPolicyKey tries to locate a policy key in the kernel keyring with the
-// provided descriptor and service. The keyring and key ids are returned if we
-// can find the key. An error is returned if the key does not exist.
-func FindPolicyKey(descriptor, service string) (keyringID, keyID int, err error) {
+// provided description. The keyring and key ids are returned if we can find the
+// key. An error is returned if the key does not exist.
+func FindPolicyKey(description string) (keyringID, keyID int, err error) {
 	keyringID, err = getKeyring()
 	if err != nil {
 		return
 	}
 
-	description := service + descriptor
 	keyID, err = unix.KeyctlSearch(keyringID, keyType, description, 0)
 	log.Printf("unix.KeyctlSearch(%d, %s, %s) = %d, %v", keyringID, keyType, description, keyID, err)
 	if err != nil {
@@ -258,10 +257,9 @@ func FindPolicyKey(descriptor, service string) (keyringID, keyID int, err error)
 }
 
 // RemovePolicyKey tries to remove a policy key from the kernel keyring with the
-// provided descriptor and service. An error is returned if the key does not
-// exist.
-func RemovePolicyKey(descriptor, service string) error {
-	keyringID, keyID, err := FindPolicyKey(descriptor, service)
+// provided description. An error is returned if the key does not exist.
+func RemovePolicyKey(description string) error {
+	keyringID, keyID, err := FindPolicyKey(description)
 	if err != nil {
 		return err
 	}
@@ -275,15 +273,11 @@ func RemovePolicyKey(descriptor, service string) error {
 }
 
 // InsertPolicyKey puts the provided policy key into the kernel keyring with the
-// provided descriptor, provided service prefix, and type logon. The key and
-// descriptor must have the appropriate lengths.
-func InsertPolicyKey(key *Key, descriptor, service string) error {
+// provided description, and type logon. The key must be a policy key.
+func InsertPolicyKey(key *Key, description string) error {
 	if err := util.CheckValidLength(metadata.PolicyKeyLen, key.Len()); err != nil {
 		return errors.Wrap(err, "policy key")
 	}
-	if err := util.CheckValidLength(metadata.DescriptorLen, len(descriptor)); err != nil {
-		return errors.Wrap(err, "descriptor")
-	}
 
 	// Create our payload (containing an FscryptKey)
 	payload, err := newBlankKey(int(unsafe.Sizeof(unix.FscryptKey{})))
@@ -304,7 +298,6 @@ func InsertPolicyKey(key *Key, descriptor, service string) error {
 		return err
 	}
 
-	description := service + descriptor
 	keyID, err := unix.AddKey(keyType, description, payload.data, keyringID)
 	log.Printf("unix.AddKey(%s, %s, <payload>, %d) = %d, %v",
 		keyType, description, keyringID, keyID, err)