From 559c73abef27ae14040fd7fbc12454dc65849f61 Mon Sep 17 00:00:00 2001 From: Peter Taoussanis Date: Sun, 16 Jun 2013 17:10:19 +0700 Subject: [PATCH] Fix core dump issue (important), simplify API PROBLEM: :legacy-mode :auto/true thawing was resulting in JVM core dumps when attempting to use Snappy to decompress encrypted data. CAUSE: The org.iq80.snappy implementation seems to choke on the random IV byte data being generated by the AES128 encrypter. This may or may not be a bug (still awaiting feedback from lib's authors). SOLUTION: We're only susceptible to this issue when trying to decompress data that is: a) encrypted, b) being thawed in legacy mode. In particular, we're _not_ susceptible to this issue when thawing in non-legacy mode because in that case we have a header explicitly warning us that the data is encrypted. An obvious work-around, therefore, is just to disable decryption when attempting to thaw legacy-mode data. In practice this isn't a problem because older versions of Nippy didn't support encryption anyway. --- README.md | 2 +- project.clj | 2 +- src/taoensso/nippy.clj | 124 +++++++++++------------------ src/taoensso/nippy/encryption.clj | 2 +- test/taoensso/nippy/tests/main.clj | 26 ++---- 5 files changed, 56 insertions(+), 100 deletions(-) diff --git a/README.md b/README.md index aaa93ce..8aa12ee 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ Current [semantic](http://semver.org/) version: ```clojure [com.taoensso/nippy "1.2.1"] ; Stable -[com.taoensso/nippy "2.0.0-alpha6"] ; Development (notes below) +[com.taoensso/nippy "2.0.0-alpha7"] ; Development (notes below) ``` 2.x adds pluggable compression, crypto support (also pluggable), an improved API (including much better error messages), and hugely improved performance. It **is backwards compatible**, but please note that the old `freeze-to-bytes`/`thaw-from-bytes` API has been **deprecated** in favor of `freeze`/`thaw`. **PLEASE REPORT ANY PROBLEMS!** diff --git a/project.clj b/project.clj index 5d7eac0..8299258 100644 --- a/project.clj +++ b/project.clj @@ -1,4 +1,4 @@ -(defproject com.taoensso/nippy "2.0.0-alpha6" +(defproject com.taoensso/nippy "2.0.0-alpha7" :description "Clojure serialization library" :url "https://github.com/ptaoussanis/nippy" :license {:name "Eclipse Public License" diff --git a/src/taoensso/nippy.clj b/src/taoensso/nippy.clj index 926e34e..f802ce4 100644 --- a/src/taoensso/nippy.clj +++ b/src/taoensso/nippy.clj @@ -13,7 +13,6 @@ IPersistentSet IPersistentCollection])) ;;;; Nippy 2.x+ header spec (4 bytes) - (def ^:private ^:const head-version 1) (def ^:private head-sig (.getBytes "NPY" "UTF-8")) (def ^:private head-meta "Final byte stores version-dependent metadata." @@ -162,6 +161,8 @@ (comment (wrap-header (.getBytes "foo") {:compressed? true :encrypted? false})) +(declare assert-legacy-args) + (defn freeze "Serializes arg (any Clojure data type) to a byte array. Set :legacy-mode to true to produce bytes readble by Nippy < 2.x." @@ -169,7 +170,7 @@ :or {print-dup? true compressor snappy-compressor encryptor aes128-encryptor}}]] - + (when legacy-mode (assert-legacy-args compressor password)) (let [ba (ByteArrayOutputStream.) stream (DataOutputStream. ba)] (binding [*print-dup* print-dup?] (freeze-to-stream x stream)) @@ -259,88 +260,56 @@ (when (utils/ba= head-sig* head-sig) [data-ba (head-meta meta-id {:unrecognized-header? true})])))) - -(defn throw-thaw-ex [msg & [e]] (throw (Exception. (str "Thaw failed: " msg) e))) - (defn thaw - "Deserializes frozen bytes to their original Clojure data type. - - :legacy-mode options: - false - Nippy >= 2.x data only (best). - true - Nippy < 2.x data only (deprecated). - :auto - Mixed data (default, migrating). - - In most cases you'll want :auto if you're using a preexisting data set, and - `false` otherwise. + "Deserializes frozen bytes to their original Clojure data type. Supports data + frozen with current and all previous versions of Nippy. WARNING: Enabling `:read-eval?` can lead to security vulnerabilities unless you are sure you know what you're doing." - [^bytes ba & [{:keys [read-eval? password compressor encryptor legacy-mode - strict?] - :or {legacy-mode :auto + [^bytes ba & [{:keys [read-eval? password compressor encryptor legacy-opts] + :or {legacy-opts {:compressed? true} compressor snappy-compressor encryptor aes128-encryptor}}]] - (let [try-thaw-data - (fn [data-ba {decompress? :compressed? decrypt? :encrypted? - :or {decompress? compressor - decrypt? password} - :as head-meta}] - (let [apparent-header? (not (empty? head-meta))] + (let [ex (fn [msg & [e]] (throw (Exception. (str "Thaw failed: " msg) e))) + try-thaw-data + (fn [data-ba {:keys [compressed? encrypted?] :as head-meta}] + (let [password (when encrypted? password) ; => also head-meta + compressor (if head-meta + (when compressed? compressor) + (when (:compressed? legacy-opts) snappy-compressor))] (try (let [ba data-ba - ba (if decrypt? (encryption/decrypt encryptor password ba) ba) - ba (if decompress? (compression/decompress compressor ba) ba) + ba (if password (encryption/decrypt encryptor password ba) ba) + ba (if compressor (compression/decompress compressor ba) ba) stream (DataInputStream. (ByteArrayInputStream. ba))] (binding [*read-eval* read-eval?] (thaw-from-stream stream))) (catch Exception e (cond - decrypt? (throw-thaw-ex "Wrong password/encryptor?" e) - decompress? (throw-thaw-ex "Encrypted data or wrong compressor?" e) - :else - (if apparent-header? - (throw-thaw-ex "Corrupt data?" e) - (throw-thaw-ex "Encrypted and/or compressed data?" e)))))))] + password (ex "Wrong password/encryptor?" e) + compressor (if head-meta (ex "Encrypted data or wrong compressor?" e) + (ex "Uncompressed data?" e)) + :else (if head-meta (ex "Corrupt data?" e) + (ex "Compressed data?" e)))))))] - (if (= legacy-mode true) - (try-thaw-data ba nil) - (if-let [[data-ba {:keys [unrecognized-header? compressed? encrypted?] - :as head-meta}] (try-parse-header ba)] - (if (= legacy-mode :auto) - (try - ;; Header seems okay, but we won't trust its metadata for - ;; error-reporting purposes - (try-thaw-data data-ba head-meta) - (catch Exception _ (try-thaw-data ba nil))) + (if-let [[data-ba {:keys [unrecognized-header? compressed? encrypted?] + :as head-meta}] (try-parse-header ba)] - (cond ; Trust metadata, give fancy error messages - unrecognized-header? - (throw-thaw-ex - "Unrecognized header. Data frozen with newer Nippy version?") - (and strict? (not encrypted?) password) - (throw-thaw-ex (str "Unencrypted data. Try again w/o password.\n" - "Disable `:strict?` option to ignore this error. ")) - (and strict? (not compressed?) compressor) - (throw-thaw-ex (str "Uncompressed data. Try again w/o compressor.\n" - "Disable `:strict?` option to ignore this error.")) - (and compressed? (not compressor)) - (throw-thaw-ex "Compressed data. Try again with compressor.") - (and encrypted? (not password)) - (throw-thaw-ex "Encrypted data. Try again with password.") - :else (try-thaw-data data-ba head-meta))) + (cond ; Header appears okay + (and (not legacy-opts) unrecognized-header?) ; Conservative + (ex "Unrecognized header. Data frozen with newer Nippy version?") + (and compressed? (not compressor)) + (ex "Compressed data. Try again with compressor.") + (and encrypted? (not password)) + (ex "Encrypted data. Try again with password.") + :else (try (try-thaw-data data-ba head-meta) + (catch Exception _ (try-thaw-data ba nil)))) - ;; Header definitely not okay - (if (= legacy-mode :auto) - (try-thaw-data ba nil) ; Legacy thaw - (throw-thaw-ex - (str "Not Nippy data, data frozen with Nippy < 2.x, " - "or corrupt data?\n" - "See `:legacy-mode` option for data frozen with Nippy < 2.x."))))))) + ;; Header definitely not okay + (try-thaw-data ba nil)))) (comment (thaw (freeze "hello")) (thaw (freeze "hello" {:compressor nil})) - (thaw (freeze "hello" {:compressor nil}) {:legacy-mode false - :strict? true}) ; ex (thaw (freeze "hello" {:password [:salted "p"]})) ; ex (thaw (freeze "hello") {:password [:salted "p"]})) @@ -394,21 +363,24 @@ ;;;; Deprecated API +(defn- assert-legacy-args [compressor password] + (when password + (throw (AssertionError. "Encryption not supported in legacy mode."))) + (when (and compressor (not= compressor snappy-compressor)) + (throw (AssertionError. "Only Snappy compressor supported in legacy mode.")))) + (defn freeze-to-bytes "DEPRECATED: Use `freeze` instead." - ^bytes [x & {:keys [print-dup? compress? password] + ^bytes [x & {:keys [print-dup? compress?] :or {print-dup? true compress? true}}] - (freeze x {:print-dup? print-dup? + (freeze x {:legacy-mode true + :print-dup? print-dup? :compressor (when compress? snappy-compressor) - :encryptor nil - :password password - :legacy-mode true})) + :password nil})) (defn thaw-from-bytes "DEPRECATED: Use `thaw` instead." - [ba & {:keys [read-eval? compressed? password] + [ba & {:keys [read-eval? compressed?] :or {compressed? true}}] - (thaw ba {:read-eval? read-eval? - :compressor (when compressed? snappy-compressor) - :encryptor nil - :password password - :legacy-mode true})) + (thaw ba {:legacy-opts {:compressed? compressed?} + :read-eval? read-eval? + :password nil})) \ No newline at end of file diff --git a/src/taoensso/nippy/encryption.clj b/src/taoensso/nippy/encryption.clj index e0d9b14..25ac362 100644 --- a/src/taoensso/nippy/encryption.clj +++ b/src/taoensso/nippy/encryption.clj @@ -51,7 +51,7 @@ (defn- destructure-typed-pwd [typed-password] (letfn [(throw-ex [] - (throw (Exception. + (throw (AssertionError. (str "Expected password form: " "[<#{:salted :cached}> ].\n " "See `default-aes128-encryptor` docstring for details!"))))] diff --git a/test/taoensso/nippy/tests/main.clj b/test/taoensso/nippy/tests/main.clj index 86cc95a..2ef8213 100644 --- a/test/taoensso/nippy/tests/main.clj +++ b/test/taoensso/nippy/tests/main.clj @@ -6,13 +6,17 @@ ;; Remove stuff from stress-data that breaks roundtrip equality (def test-data (dissoc nippy/stress-data :bytes)) -;;;; Basic data integrity (expect test-data ((comp thaw freeze) test-data)) (expect test-data ((comp thaw #(freeze % {:legacy-mode true})) test-data)) (expect test-data ((comp #(thaw % {:password [:salted "p"]}) #(freeze % {:password [:salted "p"]})) test-data)) +(expect AssertionError (thaw (freeze test-data {:password "malformed"}))) +(expect Exception (thaw (freeze test-data {:password [:salted "p"]}))) +(expect Exception (thaw (freeze test-data {:password [:salted "p"]}) + {:compressor nil})) + (expect ; Snappy lib compatibility (for legacy versions of Nippy) (let [^bytes raw-ba (freeze test-data {:compressor nil}) ^bytes xerial-ba (org.xerial.snappy.Snappy/compress raw-ba) @@ -23,24 +27,4 @@ (thaw (org.iq80.snappy.Snappy/uncompress iq80-ba 0 (alength iq80-ba))) (thaw (org.iq80.snappy.Snappy/uncompress xerial-ba 0 (alength xerial-ba)))))) -;;;; API stuff - -;;; Strict/auto mode - compression -(expect test-data (thaw (freeze test-data {:compressor nil}))) -(expect Exception (thaw (freeze test-data {:compressor nil}) - {:legacy-mode false - :strict? true})) - -;;; Strict/auto mode - encryption -(expect test-data (thaw (freeze test-data) {:password [:salted "p"]})) -(expect Exception (thaw (freeze test-data) {:password [:salted "p"] - :legacy-mode false - :strict? true})) - -;;; Encryption - passwords -(expect Exception (thaw (freeze test-data {:password "malformed"}))) -(expect Exception (thaw (freeze test-data {:password [:salted "p"]}))) -(expect test-data (thaw (freeze test-data {:password [:salted "p"]}) - {:password [:salted "p"]})) - (expect (benchmarks/autobench)) ; Also tests :cached passwords \ No newline at end of file