luciano/nippy
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

[#101] NB Change default encryption from AES-CBC to AES-GCM

Browse source 
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
This commit is contained in:
Peter Taoussanis 2019-01-06 12:32:10 +01:00
parent ae8baa639d
commit 23276ac910
3 changed files with 65 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

61
src/taoensso/nippy.clj
View file

@ -58,25 +58,38 @@
(def ^:private ^:const head-version "Current Nippy header format version" 1) (def ^:private ^:const head-version "Current Nippy header format version" 1)
(def ^:private ^:const head-meta (def ^:private ^:const head-meta
"Final byte of 4-byte Nippy header stores version-dependent metadata" "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 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 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 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} (byte 7) {:version 1 :compressor-id :snappy :encryptor-id :else}
;;
;;; :lz4 used for both lz4 and lz4hc compressor (the two are compatible) ;;; :lz4 used for both lz4 and lz4hc compressor (the two are compatible)
(byte 8) {:version 1 :compressor-id :lz4 :encryptor-id nil} (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 10) {:version 1 :compressor-id :lz4 :encryptor-id :else}
;;
(byte 11) {:version 1 :compressor-id :lzma2 :encryptor-id nil} (byte 11) {:version 1 :compressor-id :lzma2 :encryptor-id nil}
(byte 12) {:version 1 :compressor-id :lzma2 :encryptor-id :aes128-sha512} (byte 12) {:version 1 :compressor-id :lzma2 :encryptor-id :aes128-cbc-sha512}
(byte 13) {:version 1 :compressor-id :lzma2 :encryptor-id :else}}) (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))) (defmacro ^:private when-debug [& body] (when #_true false `(do ~@body)))
@ -242,7 +255,10 @@
(enc/defalias encrypt encryption/encrypt) (enc/defalias encrypt encryption/encrypt)
(enc/defalias decrypt encryption/decrypt) (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?)) (enc/defalias freezable? utils/freezable?))
@ -988,7 +1004,7 @@
([x] (freeze x nil)) ([x] (freeze x nil))
([x {:keys [compressor encryptor password] ([x {:keys [compressor encryptor password]
:or {compressor :auto :or {compressor :auto
encryptor aes128-encryptor} encryptor aes128-gcm-encryptor}
:as opts}] :as opts}]
(let [;; Intentionally undocumented: (let [;; Intentionally undocumented:
no-header? (or (get opts :no-header?) no-header? (or (get opts :no-header?)
@ -1322,18 +1338,19 @@
:lzma2 lzma2-compressor :lzma2 lzma2-compressor
:lz4 lz4-compressor :lz4 lz4-compressor
:no-header (throw (ex-info ":auto not supported on headerless data." {})) :no-header (throw (ex-info ":auto not supported on headerless data." {}))
:else (throw (ex-info ":auto not supported for non-standard compressors." {})) :else (throw (ex-info ":auto not supported for non-standard compressors." {}))
(throw (ex-info (str "Unrecognized :auto compressor id: " compressor-id) (do (throw (ex-info (str "Unrecognized :auto compressor id: " compressor-id)
{:compressor-id compressor-id})))) {:compressor-id compressor-id})))))
(defn- get-auto-encryptor [encryptor-id] (defn- get-auto-encryptor [encryptor-id]
(case encryptor-id (case encryptor-id
nil nil nil nil
:aes128-sha512 aes128-encryptor :aes128-gcm-sha512 aes128-gcm-encryptor
:no-header (throw (ex-info ":auto not supported on headerless data." {})) :aes128-cbc-sha512 aes128-cbc-encryptor
:else (throw (ex-info ":auto not supported for non-standard encryptors." {})) :no-header (throw (ex-info ":auto not supported on headerless data." {}))
(throw (ex-info (str "Unrecognized :auto encryptor id: " encryptor-id) :else (throw (ex-info ":auto not supported for non-standard encryptors." {}))
{:encryptor-id encryptor-id})))) (do (throw (ex-info (str "Unrecognized :auto encryptor id: " encryptor-id)
{:encryptor-id encryptor-id})))))
(def ^:private err-msg-unknown-thaw-failure (def ^:private err-msg-unknown-thaw-failure
"Decryption/decompression failure, or data unfrozen/damaged.") "Decryption/decompression failure, or data unfrozen/damaged.")

28
src/taoensso/nippy/encryption.clj
View file

@ -4,7 +4,10 @@
[taoensso.encore :as enc] [taoensso.encore :as enc]
[taoensso.nippy.crypto :as crypto])) [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 (defprotocol IEncryptor
(header-id [encryptor]) (header-id [encryptor])
@ -15,7 +18,7 @@
(throw (ex-info (throw (ex-info
(str "Expected password form: " (str "Expected password form: "
"[<#{:salted :cached}> <password-string>].\n " "[<#{:salted :cached}> <password-string>].\n "
"See `default-aes128-encryptor` docstring for details!") "See `aes128-encryptor` docstring for details!")
{:typed-password typed-password}))) {:typed-password typed-password})))
(defn- destructure-typed-pwd [typed-password] (defn- destructure-typed-pwd [typed-password]
@ -28,7 +31,7 @@
(comment (destructure-typed-pwd [:salted "foo"])) (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 IEncryptor
(header-id [_] header-id) (header-id [_] header-id)
(encrypt [_ typed-pwd plain-ba] (encrypt [_ typed-pwd plain-ba]
@ -42,7 +45,7 @@
(cached-key-fn nil pwd)))] (cached-key-fn nil pwd)))]
(crypto/encrypt (crypto/encrypt
{:cipher-kit crypto/cipher-kit-aes-cbc {:cipher-kit cipher-kit
:?salt-ba ?salt-ba :?salt-ba ?salt-ba
:key-ba key-ba :key-ba key-ba
:plain-ba plain-ba}))) :plain-ba plain-ba})))
@ -56,13 +59,13 @@
#(cached-key-fn % pwd))] #(cached-key-fn % pwd))]
(crypto/decrypt (crypto/decrypt
{:cipher-kit crypto/cipher-kit-aes-cbc {:cipher-kit cipher-kit
:salt-size (if salt? 16 0) :salt-size (if salt? 16 0)
:salt->key-fn salt->key-fn :salt->key-fn salt->key-fn
:enc-ba enc-ba})))) :enc-ba enc-ba}))))
(def aes128-encryptor (def aes128-gcm-encryptor
"Default 128bit AES encryptor with many-round SHA-512 key-gen. "Default 128bit AES-GCM encryptor with many-round SHA-512 key-gen.
Password form [:salted \"my-password\"] Password form [:salted \"my-password\"]
--------------------------------------- ---------------------------------------
@ -97,7 +100,16 @@
Faster than `aes128-salted`, and harder to attack any particular key - but Faster than `aes128-salted`, and harder to attack any particular key - but
increased danger if a key is somehow compromised." 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))))) (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))))))) (enc/memoize_ (fn [_salt-ba pwd] (crypto/take-ba 16 (crypto/sha512-key-ba nil pwd (* Short/MAX_VALUE 64)))))))

7
test/taoensso/nippy/tests/main.clj
View file

@ -77,7 +77,12 @@
(thaw (org.xerial.snappy.Snappy/uncompress xerial-ba)) (thaw (org.xerial.snappy.Snappy/uncompress xerial-ba))
(thaw (org.xerial.snappy.Snappy/uncompress iq80-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 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 ;;;; Custom types & records