From 23276ac9101ffd30fc4acf8ca8b318cf93c9625a Mon Sep 17 00:00:00 2001 From: Peter Taoussanis Date: Sun, 6 Jan 2019 12:32:10 +0100 Subject: [PATCH] [#101] NB Change default encryption from AES-CBC to AES-GCM Why? - AES-GCM is faster and can be more secure, Ref. https://goo.gl/Dsc9mL, etc. - AES-GCM is an authenticated[1] encryption mechanism, providing automatic integrity checks. This is relevant to [#101]. What's the issue with #101? - We compress then encrypt on freeze ; Reverse would make compression useless - So we decrypt then decompress on thaw Attempting CBC decryption with the wrong password will often but not *always* throw. Meaning it's possible for decompression could be attempted with a junk ba. And this can cause some decompressors to fail in a destructive way, including large allocations (DDoS) or even taking down the JVM in extreme cases. Possible solutions? - We could add our own HMAC, etc. - And/or we could use something like AES-GCM which offers built-in integrity and will throw an AEADBadTagException on failure. There may indeed be reasons [2,3,4] to consider adding a custom HMAC - and that's still on the cards for later. But in the meantime, the overall balance of pros/cons seems to lean in the direction of choosing AES-GCM as a reasonable default. Note that the change in this commit is done in a backward-compatible way using Nippy's versioned header: new payloads will be written using AES-GCM by default. But old payloads already written using AES-CBC will continue to be read using that scheme. References [1] https://en.wikipedia.org/wiki/Authenticated_encryption [2] https://www.daemonology.net/blog/2009-06-24-encrypt-then-mac.html [3] https://blog.cryptographyengineering.com/2011/12/04/matt-green-smackdown-watch-are-aead/ [4] HMAC vs AEAD integrity, https://crypto.stackexchange.com/q/24379 [5] AES-GCM vs HMAC-SHA256 integrity, https://crypto.stackexchange.com/q/30627 --- src/taoensso/nippy.clj | 61 +++++++++++++++++++----------- src/taoensso/nippy/encryption.clj | 28 ++++++++++---- test/taoensso/nippy/tests/main.clj | 7 +++- 3 files changed, 65 insertions(+), 31 deletions(-) diff --git a/src/taoensso/nippy.clj b/src/taoensso/nippy.clj index 410a1dd6..dec41029 100644 --- a/src/taoensso/nippy.clj +++ b/src/taoensso/nippy.clj @@ -58,25 +58,38 @@ (def ^:private ^:const head-version "Current Nippy header format version" 1) (def ^:private ^:const head-meta "Final byte of 4-byte Nippy header stores version-dependent metadata" + + ;; Currently + ;; - 5 compressors, #{nil :snappy :lz4 :lzma2 :else} + ;; - 4 encryptors, #{nil :aes128-cbc-sha512 :aes128-gcm-sha512 :else} + {(byte 0) {:version 1 :compressor-id nil :encryptor-id nil} + (byte 2) {:version 1 :compressor-id nil :encryptor-id :aes128-cbc-sha512} + (byte 14) {:version 1 :compressor-id nil :encryptor-id :aes128-gcm-sha512} (byte 4) {:version 1 :compressor-id nil :encryptor-id :else} - (byte 5) {:version 1 :compressor-id :else :encryptor-id nil} - (byte 6) {:version 1 :compressor-id :else :encryptor-id :else} - ;; - (byte 2) {:version 1 :compressor-id nil :encryptor-id :aes128-sha512} - ;; + (byte 1) {:version 1 :compressor-id :snappy :encryptor-id nil} - (byte 3) {:version 1 :compressor-id :snappy :encryptor-id :aes128-sha512} + (byte 3) {:version 1 :compressor-id :snappy :encryptor-id :aes128-cbc-sha512} + (byte 15) {:version 1 :compressor-id :snappy :encryptor-id :aes128-gcm-sha512} (byte 7) {:version 1 :compressor-id :snappy :encryptor-id :else} - ;; + ;;; :lz4 used for both lz4 and lz4hc compressor (the two are compatible) (byte 8) {:version 1 :compressor-id :lz4 :encryptor-id nil} - (byte 9) {:version 1 :compressor-id :lz4 :encryptor-id :aes128-sha512} + (byte 9) {:version 1 :compressor-id :lz4 :encryptor-id :aes128-cbc-sha512} + (byte 16) {:version 1 :compressor-id :lz4 :encryptor-id :aes128-gcm-sha512} (byte 10) {:version 1 :compressor-id :lz4 :encryptor-id :else} - ;; + (byte 11) {:version 1 :compressor-id :lzma2 :encryptor-id nil} - (byte 12) {:version 1 :compressor-id :lzma2 :encryptor-id :aes128-sha512} - (byte 13) {:version 1 :compressor-id :lzma2 :encryptor-id :else}}) + (byte 12) {:version 1 :compressor-id :lzma2 :encryptor-id :aes128-cbc-sha512} + (byte 17) {:version 1 :compressor-id :lzma2 :encryptor-id :aes128-gcm-sha512} + (byte 13) {:version 1 :compressor-id :lzma2 :encryptor-id :else} + + (byte 5) {:version 1 :compressor-id :else :encryptor-id nil} + (byte 18) {:version 1 :compressor-id :else :encryptor-id :aes128-cbc-sha512} + (byte 19) {:version 1 :compressor-id :else :encryptor-id :aes128-gcm-sha512} + (byte 6) {:version 1 :compressor-id :else :encryptor-id :else}}) + +(comment (count (sort (keys head-meta)))) (defmacro ^:private when-debug [& body] (when #_true false `(do ~@body))) @@ -242,7 +255,10 @@ (enc/defalias encrypt encryption/encrypt) (enc/defalias decrypt encryption/decrypt) - (enc/defalias aes128-encryptor encryption/aes128-encryptor) + + (enc/defalias aes128-gcm-encryptor encryption/aes128-gcm-encryptor) + (enc/defalias aes128-cbc-encryptor encryption/aes128-cbc-encryptor) + (enc/defalias aes128-encryptor encryption/aes128-gcm-encryptor) ; Default (enc/defalias freezable? utils/freezable?)) @@ -988,7 +1004,7 @@ ([x] (freeze x nil)) ([x {:keys [compressor encryptor password] :or {compressor :auto - encryptor aes128-encryptor} + encryptor aes128-gcm-encryptor} :as opts}] (let [;; Intentionally undocumented: no-header? (or (get opts :no-header?) @@ -1322,18 +1338,19 @@ :lzma2 lzma2-compressor :lz4 lz4-compressor :no-header (throw (ex-info ":auto not supported on headerless data." {})) - :else (throw (ex-info ":auto not supported for non-standard compressors." {})) - (throw (ex-info (str "Unrecognized :auto compressor id: " compressor-id) - {:compressor-id compressor-id})))) + :else (throw (ex-info ":auto not supported for non-standard compressors." {})) + (do (throw (ex-info (str "Unrecognized :auto compressor id: " compressor-id) + {:compressor-id compressor-id}))))) (defn- get-auto-encryptor [encryptor-id] (case encryptor-id - nil nil - :aes128-sha512 aes128-encryptor - :no-header (throw (ex-info ":auto not supported on headerless data." {})) - :else (throw (ex-info ":auto not supported for non-standard encryptors." {})) - (throw (ex-info (str "Unrecognized :auto encryptor id: " encryptor-id) - {:encryptor-id encryptor-id})))) + nil nil + :aes128-gcm-sha512 aes128-gcm-encryptor + :aes128-cbc-sha512 aes128-cbc-encryptor + :no-header (throw (ex-info ":auto not supported on headerless data." {})) + :else (throw (ex-info ":auto not supported for non-standard encryptors." {})) + (do (throw (ex-info (str "Unrecognized :auto encryptor id: " encryptor-id) + {:encryptor-id encryptor-id}))))) (def ^:private err-msg-unknown-thaw-failure "Decryption/decompression failure, or data unfrozen/damaged.") diff --git a/src/taoensso/nippy/encryption.clj b/src/taoensso/nippy/encryption.clj index 2cb9c4af..ce8ae4e3 100644 --- a/src/taoensso/nippy/encryption.clj +++ b/src/taoensso/nippy/encryption.clj @@ -4,7 +4,10 @@ [taoensso.encore :as enc] [taoensso.nippy.crypto :as crypto])) -(def standard-header-ids "These'll support :auto thaw" #{:aes128-sha512}) +(def standard-header-ids + "These'll support :auto thaw" + #{:aes128-cbc-sha512 + :aes128-gcm-sha512}) (defprotocol IEncryptor (header-id [encryptor]) @@ -15,7 +18,7 @@ (throw (ex-info (str "Expected password form: " "[<#{:salted :cached}> ].\n " - "See `default-aes128-encryptor` docstring for details!") + "See `aes128-encryptor` docstring for details!") {:typed-password typed-password}))) (defn- destructure-typed-pwd [typed-password] @@ -28,7 +31,7 @@ (comment (destructure-typed-pwd [:salted "foo"])) -(deftype AES128Encryptor [header-id salted-key-fn cached-key-fn] +(deftype AES128Encryptor [header-id cipher-kit salted-key-fn cached-key-fn] IEncryptor (header-id [_] header-id) (encrypt [_ typed-pwd plain-ba] @@ -42,7 +45,7 @@ (cached-key-fn nil pwd)))] (crypto/encrypt - {:cipher-kit crypto/cipher-kit-aes-cbc + {:cipher-kit cipher-kit :?salt-ba ?salt-ba :key-ba key-ba :plain-ba plain-ba}))) @@ -56,13 +59,13 @@ #(cached-key-fn % pwd))] (crypto/decrypt - {:cipher-kit crypto/cipher-kit-aes-cbc + {:cipher-kit cipher-kit :salt-size (if salt? 16 0) :salt->key-fn salt->key-fn :enc-ba enc-ba})))) -(def aes128-encryptor - "Default 128bit AES encryptor with many-round SHA-512 key-gen. +(def aes128-gcm-encryptor + "Default 128bit AES-GCM encryptor with many-round SHA-512 key-gen. Password form [:salted \"my-password\"] --------------------------------------- @@ -97,7 +100,16 @@ Faster than `aes128-salted`, and harder to attack any particular key - but increased danger if a key is somehow compromised." - (AES128Encryptor. :aes128-sha512 + (AES128Encryptor. :aes128-gcm-sha512 + crypto/cipher-kit-aes-gcm + (do (fn [ salt-ba pwd] (crypto/take-ba 16 (crypto/sha512-key-ba salt-ba pwd (* Short/MAX_VALUE 5))))) + (enc/memoize_ (fn [_salt-ba pwd] (crypto/take-ba 16 (crypto/sha512-key-ba nil pwd (* Short/MAX_VALUE 64))))))) + +(def aes128-cbc-encryptor + "Default 128bit AES-CBC encryptor with many-round SHA-512 key-gen. + See also `aes-128-cbc-encryptor`." + (AES128Encryptor. :aes128-cbc-sha512 + crypto/cipher-kit-aes-cbc (do (fn [ salt-ba pwd] (crypto/take-ba 16 (crypto/sha512-key-ba salt-ba pwd (* Short/MAX_VALUE 5))))) (enc/memoize_ (fn [_salt-ba pwd] (crypto/take-ba 16 (crypto/sha512-key-ba nil pwd (* Short/MAX_VALUE 64))))))) diff --git a/test/taoensso/nippy/tests/main.clj b/test/taoensso/nippy/tests/main.clj index 46ed5d77..c05de43e 100644 --- a/test/taoensso/nippy/tests/main.clj +++ b/test/taoensso/nippy/tests/main.clj @@ -77,7 +77,12 @@ (thaw (org.xerial.snappy.Snappy/uncompress xerial-ba)) (thaw (org.xerial.snappy.Snappy/uncompress iq80-ba)) (thaw (org.iq80.snappy.Snappy/uncompress iq80-ba 0 (alength iq80-ba))) - (thaw (org.iq80.snappy.Snappy/uncompress xerial-ba 0 (alength xerial-ba))))))) + (thaw (org.iq80.snappy.Snappy/uncompress xerial-ba 0 (alength xerial-ba)))))) + + (is ; CBC auto-encryptor compatibility + (= "payload" + (thaw (freeze "payload" {:password [:salted "pwd"] :encryptor nippy/aes128-cbc-encryptor}) + (do {:password [:salted "pwd"]}))))) ;;;; Custom types & records