From 24fc3bb7d893067d6d6f47f4fa7d12e3905537e9 Mon Sep 17 00:00:00 2001
From: Andrew Buss <abuss@ucsd.edu>
Date: Fri, 4 Dec 2015 01:30:18 -0800
Subject: [PATCH] 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        | 15 ++++++++++-----
 keycache/keycache.go      |  8 ++++++--
 keycache/keycache_test.go |  1 -
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/cryptor/cryptor.go b/cryptor/cryptor.go
index 88b27a9..2944fa6 100644
--- a/cryptor/cryptor.go
+++ b/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
-		fullMatch bool = false
-		nameSet        = map[string]bool{}
+		decryptErr error
+		fullMatch  bool = false
+		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))
diff --git a/keycache/keycache.go b/keycache/keycache.go
index 4e6df4f..3aa83d1 100644
--- a/keycache/keycache.go
+++ b/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)
 		}
diff --git a/keycache/keycache_test.go b/keycache/keycache_test.go
index a3b56d3..d4049aa 100644
--- a/keycache/keycache_test.go
+++ b/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)
 	}