From 2e891f1da876b8f8ac9099304122232bba21736a Mon Sep 17 00:00:00 2001
From: Jeff Evans <jeff303@users.noreply.github.com>
Date: Tue, 19 Oct 2021 21:18:17 -0500
Subject: [PATCH] Support encryption of binary data (#17646)

Update encryption code to be able to operate on byte arrays directly (without dealing in base64 String representation), delegating existing methods to point to these new ones

Adding test
---
 src/metabase/util/encryption.clj       | 88 +++++++++++++++++++-------
 test/metabase/util/encryption_test.clj |  8 ++-
 2 files changed, 72 insertions(+), 24 deletions(-)

diff --git a/src/metabase/util/encryption.clj b/src/metabase/util/encryption.clj
index 8d468dcc720..c5569bbad57 100644
--- a/src/metabase/util/encryption.clj
+++ b/src/metabase/util/encryption.clj
@@ -45,18 +45,43 @@
    (trs "For more information, see")
    "https://metabase.com/docs/latest/operations-guide/encrypting-database-details-at-rest.html"))
 
+(defn encrypt-bytes
+  "Encrypt bytes `b` using a `secret-key` (a 64-byte byte array), by default is the hashed value of
+  `MB_ENCRYPTION_SECRET_KEY`."
+  {:added "0.41.0"}
+  (^String [^bytes b]
+   (encrypt-bytes default-secret-key b))
+  (^String [^String secret-key, ^bytes b]
+   (let [initialization-vector (nonce/random-bytes 16)]
+     (->> (crypto/encrypt b
+            secret-key
+            initialization-vector
+            {:algorithm :aes256-cbc-hmac-sha512})
+       (concat initialization-vector)
+       byte-array))))
+
 (defn encrypt
-  "Encrypt string `s` as hex bytes using a `secret-key` (a 64-byte byte array), by default the hashed value of
+  "Encrypt string `s` as hex bytes using a `secret-key` (a 64-byte byte array), which by default is the hashed value of
   `MB_ENCRYPTION_SECRET_KEY`."
   (^String [^String s]
    (encrypt default-secret-key s))
   (^String [^String secret-key, ^String s]
-   (let [initialization-vector (nonce/random-bytes 16)]
-     (codec/base64-encode
-      (byte-array
-       (concat initialization-vector
-               (crypto/encrypt (codecs/to-bytes s) secret-key initialization-vector
-                               {:algorithm :aes256-cbc-hmac-sha512})))))))
+   (->> (codecs/to-bytes s)
+        (encrypt-bytes secret-key)
+        codec/base64-encode)))
+
+(defn decrypt-bytes
+  "Decrypt bytes `b` using a `secret-key` (a 64-byte byte array), which by default is the hashed value of
+  `MB_ENCRYPTION_SECRET_KEY`."
+  {:added "0.41.0"}
+  (^String [^bytes b]
+   (decrypt-bytes default-secret-key b))
+  (^String [secret-key, ^bytes b]
+   (let [[initialization-vector message] (split-at 16 b)]
+     (crypto/decrypt (byte-array message)
+                     secret-key
+                     (byte-array initialization-vector)
+                     {:algorithm :aes256-cbc-hmac-sha512}))))
 
 (defn decrypt
   "Decrypt string `s` using a `secret-key` (a 64-byte byte array), by default the hashed value of
@@ -64,11 +89,7 @@
   (^String [^String s]
    (decrypt default-secret-key s))
   (^String [secret-key, ^String s]
-   (let [bytes                           (codec/base64-decode s)
-         [initialization-vector message] (split-at 16 bytes)]
-     (codecs/bytes->str (crypto/decrypt (byte-array message) secret-key (byte-array initialization-vector)
-                                        {:algorithm :aes256-cbc-hmac-sha512})))))
-
+   (codecs/bytes->str (decrypt-bytes secret-key (codec/base64-decode s)))))
 
 (defn maybe-encrypt
   "If `MB_ENCRYPTION_SECRET_KEY` is set, return an encrypted version of `s`; otherwise return `s` as-is."
@@ -80,23 +101,44 @@
        (encrypt secret-key s))
      s)))
 
+(defn maybe-encrypt-bytes
+  "If `MB_ENCRYPTION_SECRET_KEY` is set, return an encrypted version of the given bytes `b`; otherwise return `b`
+  as-is."
+  {:added "0.41.0"}
+  (^bytes [^bytes b]
+   (maybe-encrypt-bytes default-secret-key b))
+  (^bytes [secret-key, ^bytes b]
+   (if secret-key
+     (when (seq b)
+       (encrypt-bytes secret-key b))
+     b)))
+
 (def ^:private ^:const aes256-tag-length 32)
 (def ^:private ^:const aes256-block-size 16)
 
+(defn possibly-encrypted-bytes?
+  "Returns true if it's likely that `b` is an encrypted byte array.  To compute this, we need the number of bytes in
+  the input, subtract the bytes used by the cipher type tag (`aes256-tag-length`) and what is left should be divisible
+  by the cipher's block size (`aes256-block-size`). If it's not divisible by that number it is either not encrypted or
+  it has been corrupted as it must always have a multiple of the block size or it won't decrypt."
+  [^bytes b]
+  (if (nil? b)
+    false
+    (u/ignore-exceptions
+      (when-let [byte-length (alength b)]
+        (zero? (mod (- byte-length aes256-tag-length)
+                 aes256-block-size))))))
+
 (defn possibly-encrypted-string?
   "Returns true if it's likely that `s` is an encrypted string. Specifically we need `s` to be a non-blank, base64
-  encoded string of the correct length. The correct length is determined by the cipher's type tag and the cipher's
-  block size (AES+CBC). To compute this, we need the number of bytes in the input, subtract the bytes used by the
-  cipher type tag (`aes256-tag-length`) and what is left should be divisible by the cipher's block size
-  (`aes256-block-size`). If it's not divisible by that number it is either not encrypted or it has been corrupted as
-  it must always have a multiple of the block size or it won't decrypt."
-  [s]
+  encoded string of the correct length. See docstring for `possibly-encrypted-bytes?` for an explanation of correct
+  length."
+  [^String s]
   (u/ignore-exceptions
-    (when-let [str-byte-length (and (not (str/blank? s))
-                                    (u/base64-string? s)
-                                    (alength ^bytes (codec/base64-decode s)))]
-      (zero? (mod (- str-byte-length aes256-tag-length)
-                  aes256-block-size)))))
+    (when-let [b (and (not (str/blank? s))
+                      (u/base64-string? s)
+                      (codec/base64-decode s))]
+      (possibly-encrypted-bytes? b))))
 
 (defn maybe-decrypt
   "If `MB_ENCRYPTION_SECRET_KEY` is set and `s` is encrypted, decrypt `s`; otherwise return `s` as-is."
diff --git a/test/metabase/util/encryption_test.clj b/test/metabase/util/encryption_test.clj
index 2d73d10adda..4aedd43faab 100644
--- a/test/metabase/util/encryption_test.clj
+++ b/test/metabase/util/encryption_test.clj
@@ -47,11 +47,17 @@
     (is (not= (encryption/encrypt secret "Hello!")
               (encryption/encrypt secret "Hello!")))))
 
-(deftest  decrypt-test
+(deftest decrypt-test
   (testing "test that we can decrypt something"
     (is (= "Hello!"
            (encryption/decrypt secret (encryption/encrypt secret "Hello!"))))))
 
+(deftest decrypt-bytes-test
+  (testing "test that we can decrypt binary data"
+    (let [data (byte-array (range 0 100))]
+      (is (= (seq data)
+             (seq (encryption/decrypt-bytes secret (encryption/encrypt-bytes secret data))))))))
+
 (deftest exception-with-wrong-decryption-key-test
   (testing "trying to decrypt something with the wrong key with `decrypt` should throw an Exception"
     (is (thrown-with-msg?
-- 
GitLab