[CHEF-3858] ensure invalid key always fails to decrypt

In Ci, we occasionally see test failures when decryption with an
incorrect key does not raise an error, but instead returns garbage.

This fixes that issue by adding an HMAC-SHA2-256 of the encrypted data
to the version 1 format. For backwards compatibility, decryption will
continue if the hmac is missing; therefore, this does not increase the
security of encrypted data bag items.

1 files changed, 29 insertions, 1 deletions

diff --git a/lib/chef/encrypted_data_bag_item.rb b/lib/chef/encrypted_data_bag_item.rb
index 816ea13864..4f96b171fa 100644
--- a/lib/chef/encrypted_data_bag_item.rb
+++ b/lib/chef/encrypted_data_bag_item.rb
@@ -86,6 +86,7 @@ class Chef::EncryptedDataBagItem
     def for_encrypted_item
       {
         "encrypted_data" => encrypted_data,
+        "hmac" => hmac,
         "iv" => Base64.encode64(iv),
         "version" => 1,
         "cipher" => ALGORITHM
@@ -122,6 +123,15 @@ class Chef::EncryptedDataBagItem
       end
     end
 
+    # Generates an HMAC-SHA2-256 of the encrypted data (encrypt-then-mac)
+    def hmac
+      @hmac ||= begin
+        digest = OpenSSL::Digest::Digest.new("sha256")
+        raw_hmac = OpenSSL::HMAC.digest(digest, key, encrypted_data)
+        Base64.encode64(raw_hmac)
+      end
+    end
+
     # Wraps the data in a single key Hash (JSON Object) and converts to JSON.
     # The wrapper is required because we accept values (such as Integers or
     # Strings) that do not produce valid JSON when serialized without the
@@ -182,7 +192,6 @@ class Chef::EncryptedDataBagItem
         raise DecryptionFailure, "Error decrypting data bag value. Most likely the provided key is incorrect"
       end
 
-
       def encrypted_bytes
         Base64.decode64(@encrypted_data["encrypted_data"])
       end
@@ -193,6 +202,7 @@ class Chef::EncryptedDataBagItem
 
       def decrypted_data
         @decrypted_data ||= begin
+          validate_hmac!
           plaintext = openssl_decryptor.update(encrypted_bytes)
           plaintext << openssl_decryptor.final
         rescue OpenSSL::Cipher::CipherError => e
@@ -200,6 +210,24 @@ class Chef::EncryptedDataBagItem
         end
       end
 
+      def validate_hmac!
+        return nil unless @encrypted_data.key?("hmac")
+
+        digest = OpenSSL::Digest::Digest.new("sha256")
+        raw_hmac = OpenSSL::HMAC.digest(digest, key, @encrypted_data["encrypted_data"])
+        expected_bytes = raw_hmac.bytes.to_a
+        actual_bytes = Base64.decode64(@encrypted_data["hmac"]).bytes.to_a
+        valid = expected_bytes.size ^ actual_bytes.size
+        expected_bytes.zip(actual_bytes) { |x, y| valid |= x ^ y.to_i }
+        if valid == 0
+          # correct hmac
+          true
+        else
+          raise DecryptionFailure, "Error decrypting data bag value: invalid hmac. Most likely the provided key is incorrect"
+        end
+      end
+
+
       def openssl_decryptor
         @openssl_decryptor ||= begin
           assert_valid_cipher!