Closed Bug 956437 Opened 10 years ago Closed 10 years ago

Add tests with valid "sha384" / "sha512" values for hash-source, along with invalid "sha1"/"md5" values

Categories

(Core :: Security, defect)

defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 883975

People

(Reporter: dholbert, Unassigned)

References

Details

The patches in bug 883975 currently only have tests for "sha256" hashes.

We should also test that we accept the other hash values called out in the spec --  "sha384" and "sha512".  (From the code, it looks like we do, but it'd be worth having tests to be sure & to be sure that we don't accidentally break these.)

Also: We should test that we *don't* accept other hash algorithms listed in
 http://mxr.mozilla.org/mozilla-central/source/netwerk/base/public/nsICryptoHash.idl#16
(e.g. sha1 or md5)

It looks like we shouldn't accept other algorithms, based on R_HASH_ALGOS in bug 883975's patch, but if we misapplied that regexp or messed it up somehow, it's definitely conceivable that we could end up accepting any arbitrary string three, and then I think we'd end up implicitly relying on nsICryptoHash::init to tell us whether it was a valid algorithm or not. (and that would accept "md5" and "sha1" as valid, incorrectly)
(Might be worth having this bug block the bug for enabling security.csp.experimentalEnabled (if there is such a bug).)

(And if there isn't a bug tracking the pref-flip, it'd probably be good to file one, so that we can keep track of the things we need to take care of before flipping the pref -- e.g. this bug and bug 936097)
(Looks like Garret's addressed this in the patches for bug 883975, per bug 883975 comment 34. Garret, feel free to dupe this to bug 883975, or alternately we can just resolve this as FIXED (by bug 883975) when that patch lands.)
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.