Fix "invalid key size 0" when decrypting after a delegation expires

The keycache does not remove active delegations when uses drops to zero;
rather it only removes these when Refresh is called.

So Valid returns true even if the user's delegation has expired,
so fullMatch is not set to false in unwrapKey,
so DecryptKey fails since the keycache refreshes and finds the delegation has expired,
so tmpKeyValue is left empty and decryptErr is set.

Since decryptErr is only used to break out of the inner loop, and
fullMatch wasn't set to false, no error is returned from unwrapKey.
So aesKey in DecryptKey is an empty string, causing an error when
passed to aes.NewCipher.

This commit actively removes a delegation from the keycache when it
is used for the last time, and properly handles errors thrown by
DecryptKey in unwrapKey.

cryptor/cryptor.go

@@ -396,9 +396,9 @@ func (encrypted *EncryptedData) wrapKey(records *passvault.Records, clearKey []b
 // unwrapKey decrypts first key in keys whose encryption keys are in keycache
 func (encrypted *EncryptedData) unwrapKey(cache *keycache.Cache, user string) (unwrappedKey []byte, names []string, err error) {
 	var (
-		keyFound  error
+		decryptErr error
-		fullMatch bool = false
+		fullMatch  bool = false
-		nameSet        = map[string]bool{}
+		nameSet         = map[string]bool{}
 	)
 
 	if len(encrypted.Predicate) == 0 {
@@ -427,7 +427,7 @@ func (encrypted *EncryptedData) unwrapKey(cache *keycache.Cache, user string) (u
 				tmpKeyValue := mwKey.Key
 				for _, mwName := range mwKey.Name {
 					pubEncrypted := encrypted.KeySetRSA[mwName]
-					if tmpKeyValue, keyFound = cache.DecryptKey(tmpKeyValue, mwName, user, encrypted.Labels, pubEncrypted.Key); keyFound != nil {
+					if tmpKeyValue, decryptErr = cache.DecryptKey(tmpKeyValue, mwName, user, encrypted.Labels, pubEncrypted.Key); decryptErr != nil {
 						break
 					}
 				}
@@ -438,7 +438,12 @@ func (encrypted *EncryptedData) unwrapKey(cache *keycache.Cache, user string) (u
 
 		if !fullMatch {
 			err = errors.New("Need more delegated keys")
-			names = nil
+			return
+		}
+
+		if decryptErr != nil {
+			err = errors.New("Failed to decrypt with all keys in keyset")
+			return
 		}
 
 		names = make([]string, 0, len(nameSet))

keycache/keycache.go

@@ -133,7 +133,11 @@ func (cache *Cache) MatchUser(name, user string, labels []string) (ActiveUser, s
 func (cache *Cache) useKey(name, user, slot string, labels []string) {
 	if val, slot, present := cache.MatchUser(name, user, labels); present {
 		val.Usage.Uses -= 1
-		cache.setUser(val, name, slot)
+		if val.Usage.Uses <= 0 {
+			delete(cache.UserKeys, DelegateIndex{name, slot})
+		} else {
+			cache.setUser(val, name, slot)
+		}
 	}
 }
 
@@ -160,7 +164,7 @@ func (cache *Cache) FlushCache() {
 // Refresh purges all expired or used up keys.
 func (cache *Cache) Refresh() {
 	for d, active := range cache.UserKeys {
-		if active.Usage.Expiry.Before(time.Now()) || active.Usage.Uses <= 0 {
+		if active.Usage.Expiry.Before(time.Now()) {
 			log.Println("Record expired", d.Name, d.Slot, active.Usage.Users, active.Usage.Labels, active.Usage.Expiry)
 			delete(cache.UserKeys, d)
 		}

keycache/keycache_test.go

@@ -70,7 +70,6 @@ func TestUsesFlush(t *testing.T) {
 		t.Fatalf("%v", err)
 	}
 
-	cache.Refresh()
 	if len(cache.UserKeys) != 0 {
 		t.Fatalf("Error in number of live keys %v", cache.UserKeys)
 	}