Details:
- Nippy will continue to support thawing OLD data that was originally compressed with Snappy.
- But Nippy will no longer support freezing NEW data with Snappy.
Motivation:
- The current Snappy implementation can cause JVM crashes in some cases [1].
- The only alternative JVM implementation that seems to be safe [2] uses JNI and
so would introduce possible incompatibility issues even for folks not using Snappy.
- Nippy already moved to the superior LZ4 as its default compression scheme in v2.7.0,
more than 9 years ago.
[1] Ref. <https://github.com/airlift/aircompressor/issues/183>
[2] Ref. <https://github.com/xerial/snappy-java>
BREAKING for the very small minority of folks that use `nippy/stress-data`.
Changes:
1. Make `nippy/stress-data` a function
It's unnecessarily wasteful to generate and store all this data when it's not
being used in the common case.
2. Make data deterministic
The stress data will now generally be stable by default between different versions
of Nippy, etc. This will help support an upcoming test for stable serialized output.
Note: also considered (but ultimately rejected) idea of a separate
`*thaw-mapfn*` opt that operates directly on every `thaw-from-in!`
result.
This (transducer) approach is more flexible, and covers the most
common use cases just fine. Having both seems excessive.
Before:
Longs in [ -128, 127] use 1 byte
Longs in [-32768, 32767] use 2 bytes
etc.
After:
Longs in [ -255, 255] use 1 byte
Longs in [-65535, 65535] use 2 bytes
etc.
I.e. doubles the range of longs that can be stored by 1, 2, and 4 bytes.
This changes saves:
- 1 byte per long in [ 128, 255], or [ -129, -255]
- 2 bytes per long in [32768, 65535], or [-32769, -65535]
- 4 bytes per long ...
Is this advantage worth the extra complexity? Probably yes, given how
common longs (and colls of longs) are in Clojure.
This reverts commit f1af0cae674f7dea29d460c5b630a58c59c7dcab.
Motivation for revert:
At least 1 user has reported depending on the current `cache`
feature, and implementing it manually (i.e. outside of Nippy) can
be non-trivial.
Rather than risk breaking folks, I'll take some more time to
consider alternatives. There's no urgency on this.
This commit is BREAKING for those still actively using `nippy/cache`.
Data previously frozen using `nippy/cache` can still be thawed, though
support for thawing may also be removed in a future Nippy version.
Motivation for removal:
This cache feature (marked as experimental) was always a bit dubious.
The use cases were very limited, and the complexity quite significant.
I don't believe that the feature has ever had much (any?) public
adoption, so I'm removing it here.
PLEASE LET ME KNOW if this removal negatively impacts you.
Removed 2x vars:
- *serializable-whitelist*
- swap-serializable-whitelist!
Added 4x vars:
- *freeze-serializable-allowlist*
- *thaw-serializable-allowlist*
- swap-freeze-serializable-allowlist!
- swap-thaw-serializable-allowlist!
Deprecated 2x JVM properties:
- taoensso.nippy.serializable-whitelist-base
- taoensso.nippy.serializable-whitelist-add
Deprecated 2x ENV vars:
- TAOENSSO_NIPPY_SERIALIZABLE_WHITELIST_BASE
- TAOENSSO_NIPPY_SERIALIZABLE_WHITELIST_ADD
API is otherwise identical.
MOTIVATION
An API break is unfortunate- but the break here is small, and the
benefit significant.
By separating the freeze/thaw lists, it becomes possible to safely
allow *any* classes to be frozen - and so effectively make the
allowlist a purely thaw-time concern in the common case.
This has several advantages including:
- No risk of Nippy calls unexpectedly throwing where they didn't
before.
- The ability to adjust or bypass the thaw allowlist *after*
seeing which class objects have been quarantined.
In general: this change eases migration to RCE-safe Nippy from
RCE-vulnerable versions. This is especially useful in cases where
Nippy is being used as an ~implementation detail for another
library/application/service.
We have 2 options:
A: Default to Serializable whitelist checks on both freeze and thaw
B: Default to Serializable whitelist checks only on thaw
Before this commit, Nippy was taking option A.
As of this commit, Nippy is taking option B.
Both are equally safe re: the risk of Remote Code Execution in #130:
- Freezing a malicious payload is *not* a security risk
- Thawing a frozen malicious payload *is* a security risk.
But option B has the benefit of not throwing exceptions by default
against a whitelist that has not [yet] been properly configured.
This is especially helpful for other libraries or applications that
may be using Nippy as an underlying dependency.
Behaviour under our two options against a whitelist that has not
[yet] been properly configured:
A: Throw exception on freeze
B: Freeze successfully, and thaw successully as
{:nippy/unthawable {:class-name <> :content <quarantined-ba> :cause :quarantined}}
I think this is probably less of a nuissance, and so a better default.
Fix a Remote Code Execution (RCE) vulnerability identified in an
excellent report by Timo Mihaljov (@solita-timo-mihaljov).
You are vulnerable iff both:
1. You are using Nippy to serialize and deserialize data from an
UNTRUSTED SOURCE.
2. You have a vulnerable ("gadget") class on your classpath.
Notably Clojure <= 1.8 includes such a class [1].
Many other libraries do too, some examples at [2].
To prevent this risk, a Serialization whitelist has been added.
Any classes not *explicitly* authorized by the whitelist to use
Serialization will NOT be permitted to.
The default whitelist is EMPTY, meaning this is a BREAKING
change iff you make use of Nippy's Serialization support. In
this case, you'll need to update the whitelist for your needs.
For more info see the `*serializable-whitelist*` docstring.
[1] https://clojure.atlassian.net/browse/CLJ-2204
[2] https://github.com/frohoff/ysoserial
Further info below provided by Timo:
------------------------------------
Deserialization vulnerabilities are exploited by constructing objects of classes
whose constructors perform some action that's useful to the attacker. A class like
this is called a gadget, and a collection of such classes that can be combined to
reach the attacker's goal is called a gadget chain.
There are three prerequisites for exploiting a deserialization vulnerability:
1) The attacker must be able to control the deserialized data, for example,
by gaining write access to the data store where trusted parties serialize
data or by exploiting some other vulnerability on the other end of a
communications channel.
2) The deserializer must construct objects of classes specified in the
serialized data. In other words, the attacker must have full control over
which classes get instantiated.
3) The classpath must contain gadgets that can be combined into a gadget chain.
The vulnerable code is in Nippy's function `read-serializable`, which calls the
`readObject` method of `ObjectInputStream`.
I have only tested the PoC with the latest stable version, 2.14.0, but looking at
Nippy's Git history, I believe all versions starting with the following commit
are vulnerable:
commit 9448d2b3ce
[Thu Oct 24 13:47:25 2013 +0700]
For a user to be affected, they must:
1) use Nippy to serialize untrusted input, and
2) have a gadget chain on their classpath.
I suspect (but haven't verified) that using Nippy's encryption feature prevents
exploitation in some cases, but if it's used to encrypt the communications between
two systems, one compromised endpoint could send encrypted but
attacker-controlled data to the other.
Ysoserial [4] contains a list of some Java libraries with known gadget chains.
If any of those libraries can be found on the user's classpath, they are known
to be vulnerable. (Ysoserial's list is not exhaustive, so even if a user doesn't
have these particular libraries on their classpath, they may still have some
other gadget chains loaded.)
Unfortunately Clojure versions before 1.9 contained a gadget chain in the
standard library [5][6], so all Nippy users running Clojure 1.8 or earlier
are vulnerable. (Note that users of later Clojure versions may or may not
be vulnerable, depending on whether they have gadget chains from other
libraries on their classpath.)
[4] https://github.com/frohoff/ysoserial
[5] https://groups.google.com/forum/#!msg/clojure/WaL3hHzsevI/7zHU-L7LBQAJ
[6] https://clojure.atlassian.net/browse/CLJ-2204
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
