MB-61292: Change proof calculation for encr at rest keys

Current algorithm for calculating proof for a key uses this
very key for encryption. This may cause issues when key is
can't be used for encryption (e.g. there is no connection
to AWS). Inability to calculate proof causes inability
to save that key on disk.
This leads to several usability problems and confusions, such as:
1. constant save retries causes pollution of logs with errors even
   when the key is not used.
2. integrity tokens can't be rotated until all keys can be saved
   successfully.
3. when the key is modified and can not be saved, the previous
   version of this key will be used for encryption of other keys,
   which may lead to confusion (e.g. AWS credentials file is
   changed, but the system keeps using prev creds file).

This change's goal is to only use those parts of the key that
are available locally for proof calculation.

Change-Id: Ie1cdab90a81545a3a650910e58c658e16b17e00c
Reviewed-on: https://review.couchbase.org/c/ns_server/+/232626
Reviewed-by: Navdeep S Boparai <[email protected]>
Tested-by: Timofey Barmin <[email protected]>
Well-Formed: Build Bot <[email protected]>

Author: timofey-barmin

apps/ns_babysitter/src/cb_gosecrets_runner.erl

@@ -1543,7 +1543,7 @@ mac_test() ->
                              verify_mac(Pid, <<0, 1, 2, 3>>, Data)),
                 ?assertMatch({error, "invalid utf-8 in uuid: " ++ _},
                              verify_mac(Pid, RandomUUIDMac, Data)),
-                ?assertEqual({error, "invalid mac"},
+                ?assertMatch({error, "invalid mac (token uuid: " ++ _},
                              verify_mac(Pid, WrongMac, Data)),
 
                 %% Rotate tokens and make sure that old mac is still valid

apps/ns_server/src/cb_cluster_secrets.erl

@@ -2634,7 +2634,7 @@ maybe_reencrypt_data(#{type := encrypted, data := Bin,
                encrypted_by => {NewSecretId, NewKekId}}}
     else
         {error, Error} ->
-            {error, encryption_service:maybe_map_not_found_key_error(
+            {error, encryption_service:maybe_wrap_encryption_error(
                       Error, failed_to_encrypt_or_decrypt_key)}
     end;
 %% Encrypted, but we want it to be unencrypted (encrypted by node SM actually)
@@ -2657,7 +2657,7 @@ maybe_reencrypt_data(#{type := sensitive, data := Bin,
                encrypted_by => {NewSecretId, NewKekId}}}
     else
         {error, Error} ->
-            {error, encryption_service:maybe_map_not_found_key_error(
+            {error, encryption_service:maybe_wrap_encryption_error(
                       Error, failed_to_encrypt_or_decrypt_key)}
     end;
 %% Not encrypted, and that's right

apps/ns_server/src/encryption_service.erl

@@ -32,7 +32,7 @@
          encrypt_key/3,
          decrypt_key/3,
          test_existing_key/1,
-         maybe_map_not_found_key_error/2,
+         maybe_wrap_encryption_error/2,
          change_password/1,
          get_keys_ref/0,
          rotate_data_key/0,
@@ -278,20 +278,17 @@ test_existing_key(KekId) when is_binary(KekId) ->
         end
     else
         {error, Error} ->
-            {error, maybe_map_not_found_key_error(Error,
-                                                  invalid_key_settings)}
+            {error, maybe_wrap_encryption_error(Error, invalid_key_settings)}
     end.
 
-maybe_map_not_found_key_error({T, "no files found matching:" ++ _ = Error},
-                              Wrapper)
-                                                when T == encrypt_key_error;
-                                                     T == decrypt_key_error ->
+maybe_wrap_encryption_error({T, Error}, Wrapper) when T == encrypt_key_error;
+                                                      T == decrypt_key_error ->
     %% We know that if this function is called the secret actually
     %% exists, so if we get this error, it means that we have problems
     %% saving the key on disk (likely because something is wrong with secret
     %% settings). Changing the error to avoid confusion.
     {Wrapper, Error};
-maybe_map_not_found_key_error(Error, _Wrapper) ->
+maybe_wrap_encryption_error(Error, _Wrapper) ->
     Error.
 
 maybe_rotate_integrity_tokens(undefined) ->

deps/gocode/gosecrets/stored_keys.go

@@ -61,6 +61,7 @@ type storedKeyIface interface {
 	kind() string
 	needRewrite(*storedKeyConfig, *StoredKeysState, *storedKeysCtx) (bool, int, error)
 	ad() []byte
+	asBytes() ([]byte, error)
 	encryptMe(*StoredKeysState, *storedKeysCtx) error
 	decryptMe(bool, *StoredKeysState, *storedKeysCtx) error
 	encryptData([]byte, []byte) ([]byte, error)
@@ -91,6 +92,7 @@ const (
 	encryptedFileHeaderSize     = 80
 	encryptedFileKeyNameLength  = byte(36)
 	macLen                      = 101 // Vsn: 1B, UUID: 36B, MAC: 64B
+	intTokenSize                = 64
 )
 
 // Struct for marshalling/unmarshalling of a raw aes-gcm stored key
@@ -286,11 +288,14 @@ func (state *StoredKeysState) maybeRegenerateProof(path, name string, ctx *store
 		logDbg("Failed to read key %s from file %s: %s", name, filePathPrefix, err.Error())
 		return err
 	}
-	parts := strings.Split(proof, ":")
-	if len(parts) != 2 {
-		return fmt.Errorf("file %s has invalid proof format", filePathPrefix)
+	proofBytes, err := base64.StdEncoding.DecodeString(proof)
+	if err != nil {
+		return fmt.Errorf("file %s has invalid proof format: %s", filePathPrefix, err.Error())
+	}
+	uuid, _, err := parseMac(proofBytes)
+	if err != nil {
+		return fmt.Errorf("file %s has invalid proof format: %s", filePathPrefix, err.Error())
 	}
-	uuid := parts[0]
 	if uuid == state.intTokens[0].uuid {
 		// Proof is still valid, no need to regenerate proof
 		return nil
@@ -312,7 +317,7 @@ func (state *StoredKeysState) maybeRegenerateProof(path, name string, ctx *store
 func (state *StoredKeysState) generateIntToken(ctx *storedKeysCtx) error {
 	newToken := intToken{
 		uuid:  uuid.New().String(),
-		token: createRandomKey(),
+		token: generateRandomBytes(intTokenSize),
 	}
 	prevTokens := state.intTokens
 	state.intTokens = append([]intToken{newToken}, state.intTokens...)
@@ -415,47 +420,52 @@ func (state *StoredKeysState) mac(data []byte) ([]byte, error) {
 		return nil, fmt.Errorf("no keys")
 	}
 	token := state.intTokens[0]
-	h := hmac.New(sha512.New, token.token)
-	h.Write(data)
-	mac := h.Sum(nil)
+	hmacBytes := hmacSHA512(token.token, data)
 	// Format: Version: 1 byte, TokenUUID, MAC
 	res := append([]byte{0}, token.uuid...)
-	res = append(res, mac...)
+	res = append(res, hmacBytes...)
 	if len(res) != macLen {
 		return nil, fmt.Errorf("unexpected mac length: %d", len(res))
 	}
 	return res, nil
 }
 
 func (state *StoredKeysState) verifyMac(mac, data []byte) error {
+	uuid, hmacBytes, err := parseMac(mac)
+	if err != nil {
+		return err
+	}
+	for _, token := range state.intTokens {
+		if token.uuid == uuid {
+			expectedHmacBytes := hmacSHA512(token.token, data)
+			if hmac.Equal(hmacBytes, expectedHmacBytes) {
+				return nil
+			}
+			return fmt.Errorf("invalid mac (token uuid: %s)", uuid)
+		}
+	}
+	return fmt.Errorf("unknown token: %s", uuid)
+}
+
+// Format: Version = 0: 1 byte, TokenUUID (36 bytes), MAC (64 bytes)
+func parseMac(mac []byte) (string, []byte, error) {
 	if len(mac) == 0 {
-		return fmt.Errorf("mac is empty")
+		return "", nil, fmt.Errorf("mac is empty")
 	}
 	if mac[0] != 0 {
-		return fmt.Errorf("unknown mac version: %v", mac[0])
+		return "", nil, fmt.Errorf("unknown mac version: %v", mac[0])
 	}
 
 	if len(mac) != macLen {
-		return fmt.Errorf("unexpected mac length: %d", len(mac))
+		return "", nil, fmt.Errorf("unexpected mac length: %d", len(mac))
 	}
 	uuidBytes := mac[1:37]
 	if !utf8.Valid(uuidBytes) {
-		return fmt.Errorf("invalid utf-8 in uuid: %q", uuidBytes)
+		return "", nil, fmt.Errorf("invalid utf-8 in uuid: %q", uuidBytes)
 	}
 	uuid := string(uuidBytes)
-	mac = mac[37:]
-	for _, token := range state.intTokens {
-		if token.uuid == uuid {
-			h := hmac.New(sha512.New, token.token)
-			h.Write(data)
-			expectedMac := h.Sum(nil)
-			if hmac.Equal(mac, expectedMac) {
-				return nil
-			}
-			return fmt.Errorf("invalid mac")
-		}
-	}
-	return fmt.Errorf("unknown token: %s", uuid)
+	hmacBytes := mac[37:]
+	return uuid, hmacBytes, nil
 }
 
 func (state *StoredKeysState) revalidateKeyCache(ctx *storedKeysCtx) {
@@ -850,9 +860,9 @@ func (state *StoredKeysState) decryptKey(keyIface storedKeyIface, validateProof
 		return err
 	}
 	if validateProof {
-		err = validateKeyProof(keyIface, proof, state.intTokens)
+		err = state.validateKeyProof(keyIface, proof)
 		if err != nil {
-			return err
+			return fmt.Errorf("key integrity check failed for key %s: %s", keyIface.name(), err.Error())
 		}
 	}
 	if keyIface.canBeCached() {
@@ -872,7 +882,7 @@ func (state *StoredKeysState) writeKeyToFile(keyIface storedKeyIface, curVsn int
 	if err != nil {
 		return err
 	}
-	proof, err := generateKeyProof(keyIface, state.intTokens)
+	proof, err := state.generateKeyProof(keyIface)
 	if err != nil {
 		return err
 	}
@@ -908,48 +918,40 @@ func (state *StoredKeysState) writeKeyToFile(keyIface storedKeyIface, curVsn int
 	return nil
 }
 
-func generateKeyProof(keyIface storedKeyIface, intTokens []intToken) (string, error) {
-	if len(intTokens) == 0 {
-		return "", fmt.Errorf("failed to generate proof: empty integrity tokens")
+func (state *StoredKeysState) generateKeyProof(keyIface storedKeyIface) (string, error) {
+	keyBytes, err := keyIface.asBytes()
+	if err != nil {
+		return "", fmt.Errorf("failed to convert key to bytes: %s", err.Error())
 	}
-	tokenHash := tokenHash(intTokens[0], keyIface)
-	encryptedTokenHash, err := keyIface.encryptData(tokenHash[:], []byte(keyIface.name()))
+	proofBytes, err := state.mac(keyBytes)
 	if err != nil {
-		return "", fmt.Errorf("failed to use key to generate proof: %s", err.Error())
+		return "", err
 	}
-	proof := fmt.Sprintf("%s:%s", intTokens[0].uuid, base64.StdEncoding.EncodeToString(encryptedTokenHash))
-	return proof, nil
+	proofStr := base64.StdEncoding.EncodeToString(proofBytes)
+	return proofStr, nil
 }
 
-func validateKeyProof(keyIface storedKeyIface, proof string, intTokens []intToken) error {
-	parts := strings.Split(proof, ":")
-	if len(parts) != 2 {
-		return fmt.Errorf("key integrity check failed: invalid proof format")
+func (state *StoredKeysState) validateKeyProof(keyIface storedKeyIface, proof string) error {
+	proofBytes, err := base64.StdEncoding.DecodeString(proof)
+	if err != nil {
+		return fmt.Errorf("failed to decode proof: %s", err.Error())
 	}
-	uuid := parts[0]
-	encryptedTokenHashBase64 := parts[1]
-	for _, token := range intTokens {
-		if token.uuid == uuid {
-			encryptedTokenHash, err := base64.StdEncoding.DecodeString(encryptedTokenHashBase64)
-			if err != nil {
-				return fmt.Errorf("key integrity check failed: failed to decode encrypted token hash: %s", err.Error())
-			}
-			decryptedTokenHash, err := keyIface.decryptData(encryptedTokenHash, []byte(keyIface.name()))
-			if err != nil {
-				return fmt.Errorf("key integrity check failed: failed to decrypt proof: %s", err.Error())
-			}
-			tokenHash := tokenHash(token, keyIface)
-			if bytes.Equal(tokenHash[:], decryptedTokenHash) {
-				return nil
-			}
-			return fmt.Errorf("key integrity check failed: invalid integrity token (token uuid: %s, key name: %s)", uuid, keyIface.name())
-		}
+	keyBytes, err := keyIface.asBytes()
+	if err != nil {
+		return fmt.Errorf("failed to convert key to bytes: %s", err.Error())
 	}
-	return fmt.Errorf("key integrity check failed: unknown token: %s (key name: %s)", uuid, keyIface.name())
+	err = state.verifyMac(proofBytes, keyBytes)
+	if err != nil {
+		return err
+	}
+	return nil
 }
 
-func tokenHash(token intToken, keyIface storedKeyIface) [64]byte {
-	return sha512.Sum512(append(token.token, []byte(keyIface.name())...))
+func hmacSHA512(key []byte, data []byte) []byte {
+	h := hmac.New(sha512.New, key)
+	h.Write(data)
+	mac := h.Sum(nil)
+	return mac
 }
 
 func (state *StoredKeysState) encryptWithKey(keyKind, keyName string, data, AD []byte, ctx *storedKeysCtx) ([]byte, error) {
@@ -1313,6 +1315,13 @@ func (k *rawAesGcmStoredKey) ad() []byte {
 	return []byte(string(rawAESGCMKey) + k.Name + k.Kind + k.CreationTime + k.EncryptionKeyName + strconv.FormatBool(k.CanBeCached))
 }
 
+func (k *rawAesGcmStoredKey) asBytes() ([]byte, error) {
+	if k.DecryptedKey == nil {
+		return nil, fmt.Errorf("key %s is encrypted", k.Name)
+	}
+	return append(k.ad(), k.DecryptedKey...), nil
+}
+
 func (k *rawAesGcmStoredKey) encryptMe(state *StoredKeysState, ctx *storedKeysCtx) error {
 	if k.EncryptedKey != nil {
 		// Seems like it is already encrypted
@@ -1449,6 +1458,20 @@ func (k *awsStoredKey) ad() []byte {
 	return []byte("")
 }
 
+func (k *awsStoredKey) asBytes() ([]byte, error) {
+	return []byte(
+		string(awskmKey) +
+			k.Name +
+			k.Kind +
+			k.KeyArn +
+			k.Region +
+			k.ConfigFile +
+			k.CredsFile +
+			k.Profile +
+			strconv.FormatBool(k.UseIMDS) +
+			k.CreationTime), nil
+}
+
 func (k *awsStoredKey) encryptMe(state *StoredKeysState, ctx *storedKeysCtx) error {
 	// Nothing to encrypt here, configuration should contain no secrets
 	return nil
@@ -1625,6 +1648,13 @@ func (k *kmipStoredKey) ad() []byte {
 			k.CreationTime)
 }
 
+func (k *kmipStoredKey) asBytes() ([]byte, error) {
+	if k.decryptedPassphrase == nil {
+		return nil, fmt.Errorf("key %s is encrypted", k.Name)
+	}
+	return append(k.ad(), k.decryptedPassphrase...), nil
+}
+
 func (k *kmipStoredKey) encryptMe(state *StoredKeysState, ctx *storedKeysCtx) error {
 	if k.EncryptedPassphrase != nil {
 		// Seems like it is already encrypted