[Esapi-dev] Bad news / good news and recommendation for ESAPI 1.4.x patch release

[Kevin W. Wall]

Jim,

*BAD NEWS*
==========
As I mentioned before, the first time that I looked at ESAPI was ESAPI 2.0rc2,
back in August 2009.

At the time, the symmetric encryption was using "AES" for the default
with a 256-bit key and as I've explained elsewhere, specifying "AES"
used the weak ECB cipher mode.

It wasn't until perhaps late Dec / early Jan, when you notified me of an
ESAPI blog bashing, that I went back and looked at ESAPI 1.4 crypto to find
out what was really going on there. (You know what they say about when you
assume. I stand [or should that be 'sit'?] guilty as charged.)

To my dismay, the bad news was that the default encryption algorithm
was (and still is, as of 1.4.5) chosen to be "PBEWithMD5AndDES".  I had never
really bothered looking at the ESAPI 1.4 code because my assumption up until
that point was always that the ESAPI 1.4 default encryption could not possibly
be worse than AES in ECB mode. I thought that AES in ECB mode was what
ESAPI 1.4 was using as well. It wasn't. That started sometime in 2.0.

Unfortunately, I was wrong about thinking that things could not be worse in 1.4.
The "PBE" part in "PBEWithMD5AndDES", refers to Password (or passphrase)
Based Encryption that is created by taking a password (the default of
which was "esapi1" and it is delivered that way!). That then gets hashed
with a salt taken from MasterSalt (delivered as the string "testtest"!)
using MD5, a now broken message digest.  (The whole point of the hashing
20 times is to add a work factor that makes it take significantly longer
to mount a dictionary attack against the password / passphrase.) Finally,
the plaintext is then encrypted using DES.

Now the bad news here is that:
1) the MasterKey and MasterSalt is delivered set to something and may
   be used as-is. (Don't we tell people not to do that in other OWASP docs?
   Oops!)
2) the DES encryption key is based on a password / passphrase and _still_
   is likely to be subject to dictionary attacks because humans have a tendency
   to do a very poor job at this. (I'd wager no one has set it to anything
   longer than 16 characters.)
3) The worse news is that no matter how strong your passphrase is--even if
   it were 100 random characters--one can still attack the DES encryption
   key directly. After all, it is only 56-bits and is now crackable in less
   than 24 hrs with modest equipment of modest cost.

*GOOD NEWS*
===========
I feared that things were even worse than they were because I just figured
that DES used with "PBEWithMD5AndDES" was also using ECB cipher mode as well.

Further research on my part has revealed in fact this algorithm that Sun's
JCE implements is RSA's PKCS#5 (version 1.5), which means that _CBC_ and *not*
ECB is used as the cipher mode, and PKCS5Padding (obviously, huh?) is used
as the padding scheme.

So... this is *good* news, at least relatively speaking. Why? Well, because we
are going to put out a 1.4.x patch (or minimally, a 'security announcement')
that at least bolsters the encryption to something somewhat reasonable. How?
Simple.

First, we change EncrpyptionAlgorithm in ESAPI.properties to be set thusly:

	EncryptionAlgorithm=PBEWithHmacSHA1AndDESede

and secondly, we set MasterKey and MasterSalt to empty values and/or just
leave them commented out in ESAPI.properties. (I think either of these will
cause a NPE if used w/out changing anything in such cases, but need to test to
confirm.)

We do the latter to ensure that people change them. In a comment in
ESAPI.properties and/or in the user documentation, we tell people to choose
a *random* 20-byte or longer MasterSalt and to set MasterKey to a pass *phrase*
to be *at least* 45 bytes or so. (That's roughly equivalent to a 128-bit random
key if you figure the _worst_ case of about 3 bits of entropy per byte for
normal printable ASCII characters.) We would need to add a JavaEncryptor.main()
or have a standalone program somewhere if we want to generate a base64-encoded,
random 20-byte salt, but that's not too hard. Short enough to put into a
security bulletin really. Just gather 20 bytes from SecureRandom and then
base64 encode them and print to System.out with "MasterSalt=" prepended. Piece
of cake, really.

If you want to be paranoid, we could also add a simple change to
DefaultSecurityConfiguration to reject any MasterKey less than 45 characters
or so and log it and then throw an exception if it is considered too short.

PBEWithHmacSHA1AndDESede implements PKCS#5, version 2.0 and as such, it
uses 168-bit DESede with CBC cipher mode and PKCS5Padding. This means that
making the passphrase sufficiently long and using a sufficiently long
random salt removes many of the weaknesses (specifically dictionary attacks,
and brute force attacks on key space). And because it is not using ECB mode,
it has none of those problems usually associated with ECB. (Note: Key size
here is the *effective* key size, ignoring parity bits. Actual key size is
192-bits.)

So, used right--with sufficiently long/random passphrase and salt, this can
still be a solid enough algorithm for most uses. (Whether it would pass muster
for PCI DSS compliance, I'm not sure. That might depend on the whims of your
particular PCI auditor. *Anyone know?* Probably not FIPS 140 compliant though
as I seriously doubt that PBE is considered a secure way of generating a key.)

*MORE BAD NEWS???*
==================
As my son is apt to say, this one would fall under the category of "whine,
whine, whine". (Not to be confused with "wine, wine, beer", which clearly
would fall under the "*more good news*" category. ;-)

The bad news is _I_ have *extensive* changes to rewrite in the document
"Why Is OWASP Changing ESAPI Encryption?". Several of the things are still
relevant...e.g., the whole thing about doing all encryptions with a
single key, but many others have to go. Specifically the lovable
encrypted Tux image has got to go--or at least move to a less prominent
position later in the document. (Recall that I wrote this when I had only
looked at ESAPI 2.0rc2 and saw that AES in ECB mode was being used.)
No matter--it just will take me a few days longer.

Secondly, perhaps we should have not been quite so hasty to remove
LegacyJavaEncryptor as PBEWithHmacSHA1AndDESede could be used
and with appropriate passphrase length, the legacy encrypt/decrypt
mechanism could be sufficiently bolstered to at least be acceptable.
OTOH, I shall shed no tears for it. It simplifies things and in my
mind was dead baggage. Plus, because no one else raised any objection
either, I doubt that anyone else is going to shed any tears either.

I think I am going to drastically shorten up the "Why Is OWASP
Changing ESAPI Encryption" and move the part that illustrates
how to use encryption in ESAPI 2.0 to an ESAPI 2.0 Users Guide.
(*Mike*: Can you tell me where this would go? Do you have a template
to follow for this? Please let me know.)

*MORE GOOD NEWS???*
===================
Oddly enough perhaps, the reason I decided to start working on ESAPI was bacause
I felt that the symmetric encryption was done so poorly. It was somehting
I thought I could fix and I was concerned that developers from my company would
start using it, so I was motivated to get it fixed ASAP. But had I known last
August what I know now, I probably would have stopped at suggesting that OWASP
just revert back to what they had in ESAPI 1.4 but change it to use
PBEWithHmacSHA1AndDESede and ensure a sufficiently long and random passphrase
and salt. Chances are I might have left it at that. Of course, had that
happened, I would have missed out on all the fun. Funny how things work out
like that.


Anyhow, enough rambling. We now return you to your regularly scheduled
broadcast.

-kevin
P.S.- Jim: Please xpost this to ESAPI-Users list if you feel it is relevant
      there too. But they probably can wait until we just post a new
      1.4 patch release or a security bulletin (if you are ready to
      test out that process).
--
Kevin W. Wall
"The most likely way for the world to be destroyed, most experts agree,
is by accident. That's where we come in; we're computer professionals.
We cause accidents."        -- Nathaniel Borenstein, co-creator of MIME