[Owasp-board] Fwd: [Esapi-dev] ESAPI Java and Authenticated encryption implementation

Jim Manico jim.manico at owasp.org
Thu Aug 22 05:54:54 UTC 2013


Applied crypto is tough. The crypto in our flagship project, ESAPI for
Java, is broken, see below. The NSA donated a review for us and missed this
(big surprise). There is no hiding this, it was publicly reported on the
ESAPI-dev list.

It's going to take a major release to fix this. I would like to suggest
that we pay Kevin Wall to push a new revision for us.

We also need to disclose this somehow to warn the many users of this
project.

You can read about the details below as we as on the ESAPI-dev list.

Thoughts?

--
Jim Manico
@Manicode
(808) 652-3805

Begin forwarded message:

*From:* "Kevin W. Wall" <kevin.w.wall at gmail.com>
*Date:* August 22, 2013, 7:44:50 AM GMT+02:00
*To:* Jeff Walton <noloader at gmail.com>
*Cc:* Jim Manico <jim.manico at owasp.org>, Chris Schmidt <
chris.schmidt at owasp.org>,  Dave Wichers <dave.wichers at owasp.org>, Jeff
Williams <Jeff.Williams at owasp.org>
*Subject:* *Re: [Esapi-dev] ESAPI Java and Authenticated encryption
implementation*

I like the idea of the labels on the KDF. You and I have even discussed
it in the past.

The problem is how to extract a label to apply automagically.
Only thing I could think of is to use the IP address or the MAC
address (much harder to get on the web; client IP is hard even
given proxies, NAT, etc.). Otherwise we 1) break backward
compatibility and 2) make people specify it, which means an
interface change which means clients have to change their
code not just deploy an new ESAPI jar.

There is a timestamp in serialized cipher text. That's the only other
simple solution I can think of that wouldn't require interface changes.
Ideally, one would like to see some sort of user identity or purpose
string used for the label, but I can't see how to do that w/out changing
encrypt() / decrypt() interfaces (or adding new ones to an already,
overbloated Encryptor interface).

Long term, best thing would be to start from scratch. If I had to do it
all over again, I would have bit the bullet and implemented CMS
(Cryptographic Message Syntax as defined in RFC 5652).
At least it is standard. There are lots of implementations in C/C++,
but none that I saw (at least at the time) for .NET or PHP or
any of the other implementations that we had at the time.

I'm beginning to see the wisdom of Jim's pleading with me to
implement ESAPI crypto w/out ESAPI. At least I could
change the interfaces to better fit the needs of the crypto
community. I have been reluctant to do so because if I
forked it I would have 2 projects to apply bug fixes to when
stuff like this inevitably happens.  If I do that, then Chris is
going to have to take over the ESAPI Java project full time
or Jim or someone new has to step up and drive it.

Damn, why couldn't have this happened closer to the
ESAPI hack-a-thon at AppSecUSA? Then at least I would
have some interesting stuff to work on.

Sigh. I can tell that I'm not going to get a lot of sleep tonight.
Insomnia for 2 nights in a row. Woohoo!

-kevin


On Thu, Aug 22, 2013 at 1:03 AM, Jeffrey Walton <noloader at gmail.com> wrote:

> On Wed, Aug 21, 2013 at 11:16 PM, Kevin W. Wall <kevin.w.wall at gmail.com>
> wrote:
> > ...
> >
> > Also, there is a high-level design document at:
> >
> http://owasp-esapi-java.googlecode.com/svn/trunk/documentation/esapi4java-core-2.0-crypto-design-goals.doc
> > (Sorry; it's a MS Word doc. I can convert if to PDF if need be.)
> Page 9, Section 3.1.2: "... Providing a MAC key that is different for
> each unique encryption key is the best policy ...".
>
> If you look at, for example, some of the RFCs, you will see the use of
> a label. The label ensures a key derived under the same password with
> different parameters is different. So, if using the KDF for
> AES/CBC/PKCS5, then you might do:
>
>     key = KDF( "AES/CBC/PKCS5", <password>, <salt>, <count>)
>
> If you use it with TripleDES, then it might look like:
>
>     key = KDF( "3DES/CBC/PKCS5", <password>, <salt>, <count>)
>
> And with an HMAC:
>
>     key = KDF( "AES/CBC/PKCS5" || "HMAC-SHA1", <password>, <salt>, <count>)
>
> Because the HmacLen may be tampered, go ahead and do:
>
>     key = KDF( "AES/CBC/PKCS5" || "HMAC-SHA1" || HmacLen, <password>,
> <salt>, <count>)
>
> The labels ensure the same password produces different results under
> different uses.
>
> If Phillip lops off your HMAC and tampers with the serialization, then
> the different labels ensure different keys are derived, so the plain
> text should remain unrecoverable.
>
> Jeff
>
> >> On Wed, Aug 21, 2013 at 10:14 PM, Jeffrey Walton <noloader at gmail.com>
> >> wrote:
> >> > On Wed, Aug 21, 2013 at 9:01 PM, Kevin W. Wall <
> kevin.w.wall at gmail.com>
> >> > wrote:
> >> >> ...
> >> >>>
> >> >>> The problem is that the MAC validation can be bypassed under certain
> >> >>> conditions.
> >> >>>
> >> >>> Condition #1: if the Ciphertext is tampered to contains a MAC that
> is
> >> >>> null
> >> >>> ...
> >> >>> Condition #2: if the cipherSpec is tampered to use a different mode
> >> >>> that
> >> >>> is not part of combinedCipherModes
> >> > This was very clever! I wonder how many other systems suffer the same.
> >> >
> >> > I'd recommend throwing if the tag is less than 64-bits. Recommend
> >> > 96-bit tag for data in transit; and a 128-bit tag for data at rest.
> >> > Its consistent with NIST SP 800-38D.
> >> >
> >> >>> ....
> >> >>> The design to compute the mac for only a portion of the message is
> >> >>> kind of
> >> >>> broken. The mac should cover all parameters serialized.
> Authenticated
> >> >>> encryption implementation should not use logic that support optional
> >> >>> MAC.
> >> > Another good find.
> >> >
> >> >> I had thought of including EVERYTHING, but those on the crypto list
> >> >> said
> >> >> that you really only needed to MAC the
> >> >> IV+ciphertext.
> >> > I may have lead you astray here. My apologies.
> >> >
> >> > In the systems I build, there's only {IV, CT, MAC} - and no optional
> >> > parameters. So my rule is to MAC everything, which for me is IV and
> >> > Ciphertext. On occasion I also need to include the version.
> >> >
> >> > For reference, the rule to MAC everything is called "Semantic
> >> > Authentication", a.k.a "The Horton Principle" from Horton Hears a Who.
> >> > It was introduced by Wagner and Schneier in "Analysis of the SSL 3.0
> >> > protocol" back in 1995,
> >> > https://www.schneier.com/paper-ssl-revised.pdf.
> >> >
> >> > The Horton Principle is great at finding protocol cruft: if a field
> >> > has value then it must be authenticated. The contrapositive ferrets
> >> > the cruft: if a field does not need authentication, then its useless
> >> > and can be removed. Start removing fields and see which ones really
> >> > have value ;)
> >> >
> >> > Don't apply The Horton Principle to code signing in Apple, BSD and
> >> > Android (probably et al). You will be horrified at what you find. I
> >> > warned Android Security about the problems a year or so before Bluebox
> >> > released their "Master Key" exploit.
> >> >
> >> >> OK, here's what I'm not getting about Condition #2. In your PoC code,
> >> >> you
> >> >> had ESAPI.properties set to:
> >> >>
> >> >>     Encryptor.CipherTransformation=AES/OFB/NoPadding
> >> >>     Encryptor.cipher_modes.combined_modes=OFB
> >> >>     Encryptor.cipher_modes.additional_allowed=CBC
> >> >>  ...
> >> >> So, what I am not understanding about your Condition #2 is how your
> >> >> exploit
> >> >> will get past this first test (right after checking if either of the
> >> >> parameters are null)?
> >> > So, my [uneducated] opinion: don't waste time debating the ways a
> >> > Turing machine does or does not stop. Validate the parameters and
> >> > throw on failure. Validate the MAC length is at least 64-bits and
> >> > throw otherwise.
> >> >
> >> > Or, put another way: Meh... fix it and move on.
> >> >
> >> >> Anyhow, I thank you again for looking hard at this. I was never real
> >> >> comfortable
> >> >> with the way I was doing the serialization and where the MAC was
> >> >> applied,
> >> >> but tried to catch other issues in other manners (such as the earlier
> >> >> test
> >> >> for cipher mode).
> >> > So, when the changes are cut-in, be sure the new version derives to a
> >> > new set of keys. That is, the same password or shared secret in V3
> >> > should derive to a different key than in V4. Otherwise you'll be
> >> > susceptible to a downgrade attack like Winzip.
> >> > http://cse.ucsd.edu/node/1340.
>



-- 
Blog: http://off-the-wall-security.blogspot.com/
"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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.owasp.org/pipermail/owasp-board/attachments/20130822/a608ec13/attachment.html>


More information about the Owasp-board mailing list