Use of weak MD5 algorithm

The OSSA tasks is set to Incomplete pending additional security details.

Isn't the StreamReader object already consuming a secured protocol (SSL) ?
It can be a good security hardening but is there a real threat here ?

Tristan, python-swiftclient that is used to interact with Swift doesn't use SSL (see https://github.com/openstack/trove/blob/24b1f25204ae0f3c149851e2b6275a39c4da6768/trove/common/remote.py#L148-L162) due to missing ca_cert attribute.

Actually, i don't think that it's threat since Trove uses MD5 to verify that income stream is not corrupted/modified. But i do agree about MD5 - it's already hacked, but as i can see SHA3 (Keccak) is not a part of Python standard lib. Thoughts?

The only current known weakness in MD5 is a hash collision--the ability for an attacker to pick (with some effort) two inputs which hash to the same value. In what way do you see this posing a risk to Trove's use of MD5 for stream validation?

Also, I agree with your bug description calling this out specifically as a hardening measure, something for which we should not issue a security advisory.

@Jeremy there are more issues than just collisions in MD5, for a start the search space for an MD5 hash is completely tractable due to rainbow tables, pre-image attacks are also theoretically possible in addition to the collisions you describe.

In summary, it's appropriate to use MD5 for corruption checks where there's no possibility of malicious attempts to breach file integrity but this is rarely the case and SHA hashes should probably just replace md5 across the board.

+1 to Rob's comment.

Okay, I took a quick look at the code in Trove, and this _is_ one of the cases where we're using an MD5 hash check to ensure no data corruption has occurred with backup data stored in swift. Since it's not actually being used for any crypto (i.e. encryption / decryption) I don't think this is a high priority security issue, but it would be good for hardening. Triaging the bug, appropriately based on this information. Thanks!

Right, so risky in places where collision and chosen-prefix attacks can be mounted... just trying to ascertain whether the static analysis which highlighted this bug identified an exploitable security vulnerability or just another hardening opportunity. Sounds like the latter.

It's worth noting however that similar issues were just pointed out yesterday in rsync (it uses MD5 for identifying alterations to blocks rather than stream integrity, but perhaps a tangentially similar problem space?). http://openwall.com/lists/oss-security/2014/07/28/1http://openwall.com/lists/oss-security/2014/07/28/1

Gah, my paste button is bouncing lately... http://openwall.com/lists/oss-security/2014/07/28/1

Something to consider...when a crypto algorithm/function is used for what is perceived to be a non-crypto use, it introduces a bunch of baggage and should be part of the overall decision process. Some organizations are taking a VERY hard line when it comes to the use of things like MD5, SHA-1, RC4, etc. A generic question gets asked as to whether the code/application uses certain banned algorithms. If the answer is "yes" then its use is not permitted within the organization unless a waiver is approved (may not be an option). In such a scenario, the person wanting to use the code is put in the position of justifying the waiver and "accepting" the risks...often considered a career limiting move. I've also see crypto issues used as a way of down-selecting choices (vendor, code bases etc.).

It seems prudent to just make this problem go away by replacing MD5 with SHA-2, especially when we've found it. Coming back later to find these because of a data breach or other problem can be a massive waste of time.

I pretty much agree with all that has been said. We should fix it. SHA-2 makes the most sense today. And fixing it as a general hardening measure, rather than an OSSA makes sense.

To the point of wanting to just get all bad crypto algorithms out of these code bases... the OSSG is working on some gate tools that would catch such things. Once those are put in place, it should be much easier to prevent this kind of thing from happening in the future :-)

+1 to Bryan and Eric

Many enterprise customers are requesting to see our crypto audits - there's only so many times you can explain "Yes, but, MD5 is pretty much o.k in this context" before you start looking silly.

I don't think we can upgrade the code in question from MD5 to SHA256 at the moment.

The Swift put_object() call's response has an etag field. This etag field is populated with the MD5 hash for the data segment received by Swift, and it is calculated by Swift.

I took a quick look at the Swift code, and don't see evidence of etag hash algorithm being configurable on Swift.

This posting is also saying etag is MD5 only:
https://answers.launchpad.net/swift/+question/217171

Since MD5 is a constraint imposed by Swift, unless Swift starts to support SHA etags, I don't believe we have other choice at the moment but to stick with MD5 here.

Can you make it configurable, and have it default to md5?

So I looked into this a bit more, and schang is correct.
Trove is limited by the implementation that swift supports which is currently only MD5.
If we want to support file verification based on SHA256 or something else, we will need to have this added to swift.