Open Bug 472823 Opened 12 years ago Updated 5 months ago

SHA 256 Digest Authentication


(Core :: Networking, enhancement, P5)






(Reporter: ph, Unassigned)


(Whiteboard: [necko-would-take])


(1 file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/2008121621 Ubuntu/8.04 (hardy) Firefox/3.0.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/2008121621 Ubuntu/8.04 (hardy) Firefox/3.0.5

Firefox implements Digest authentication using the MD5 algorithm, which is the example specified in RFC2617[1].  That RFC allows other algorithms, by using the optional 'algorithm' property:

   An optional header allows the server to specify the algorithm used to
   create the checksum or digest. By default the MD5 algorithm is used
   and that is the only algorithm described in this document.

Since MD5 has been cracked and is now considered vulnerable, I'd like to propose making Firefox recognize this header and implement other algorithms, at the least SHA-1 but later also SHA-2 variants like SHA-256 and SHA-512[2].

Obviously SSL provides a better security model but it is harder to use and fixing the Digest mechanism would give good benefits at little cost afaics.

RFC 2617 allows for a server to issue multiple challenges, using different algorithms, so this allows for negotiation between the browser and server.


Reproducible: Always
Moving to Core->Networking and updating summary.  Not confirming because I actually don't know if we already support other algs for digest auth?
Component: Security → Networking
Product: Firefox → Core
QA Contact: firefox → networking
Summary: Digest Authentication is not secure (MD5 has been broken) → Digest Authentication should support other hash algorithms (MD5 has been broken)
Jason: if you're not otherwise assigned this bug might provide a path into learning about parts of necko that's somewhat more rewarding than just slogging through randomly.
Assignee: nobody → jduell
FWIW, if you need a web server to test against, we'll implement the SHA-1 algorithm in ours (Xitami/5).  Ask me, offlist.
Yes, this might be a good entryway into necko for me.  I've also just assigned myself to a half-dozen other networking bugs, so y'all can feel free to chime in on which ones ought to have highest priority ;)
So we definitely want to fix bug 281851 before this one--improved digest security won't matter if we use Basic auth instead whenever the web server lists it first.
There is a new RFC for Http Digest Authentication:
This specification defines the following algorithms:
   o  SHA2-256 (mandatory to implement)
   o  SHA2-512/256 (as a backup algorithm)
   o  MD5 (for backward compatibility).
This patch adds support for SHA-256 as the digest algorithm.

My C++ skills are very unimpressive so i would advise someone with more skill to review and improve the code.

To test this code, you need a webserver with support for SHA-256.
I used a modified version of which can be found here: (clone it, then `python develop` and then `python -m httpbin.core` should work)
we would want an ietf standard around this.. otherwise add-on territory.
Closed: 5 years ago
Resolution: --- → WONTFIX
(In reply to Patrick McManus [:mcmanus] from comment #8)
> we would want an ietf standard around this.. otherwise add-on territory.

Are you kidding? You've been given the standard, see comment #6, it is RFC 7616 from September 2015.

So please reopen the BUG report. I implemented this into my web-services in less than half an hour, and now I am looking for web browsers supporting it. Firefox could be the first one ;-)
This seems to have been closed in error. RFC 7616 is an IETF standard. The reason stated for closing is bogus.
Ever confirmed: true
Resolution: WONTFIX → ---
(In reply to Dr. Rolf Jansen from comment #9)
> Are you kidding? You've been given the standard, see comment #6, it is RFC
> 7616 from September 2015.

I missed this. Sorry. And the patch wasn't on anyone's radar because it wasn't flagged for review - it was just another attachment. Thanks doubly for the patch.

I'm still not sure if we want this code if there is no interest from other singificant entities on the web.. that's always a tricky thing - sometimes you need to solve the chicken and egg problem, and other times you want to avoid propogating too many 'standards'.

Dan might be in the best place to decide.
Flags: needinfo?(dveditz)
Whiteboard: [necko-would-take]
Attachment #8683095 - Flags: review?(dveditz)
Summary: Digest Authentication should support other hash algorithms (MD5 has been broken) → SHA 256 Digest Authentication
I don't know why we wouldn't want to implement this. MD5 is known crap and this is an approved standard that appears to have been reviewed by the right smart folks (including our own :bagder).
Flags: needinfo?(dveditz)
the reason we wouldn't want to do this is that there is very little interest in digest authentication even with this standard existing and its tough to maintain code or tests without a champion for them.

I'm not saying that's a winning argument - just an argument. If you want to review the code and think its worth including I'm good with that.
The reason why the "old kind of" digest authentication got very little interest nowadays is exactly because of the weakness of MD5, and because everybody has been told to go with TLS anyway so the authentication can be as simple as asking for a password on the web site and generate a session cookie. ****, simply ask the big five suppliers of firewall appliances how many DPI systems with TLS interception they deploy per year. How many people enter their private web-mailer from work, and don't even know that their login credentials would be visible to a dedicated company admin. How many people open the day or the other their WordPress Blog from behind a DPI-FW? The most critical part of this web-traffic are the credentials and these would be safe by SHA256-Digest-Authentication over TLS.

Another advantage of digest authentication compared to other web site authentication methods is, that the credentials are passed in the HTTP message header and not in the HTTP message body (no to be confused with HTML header/body). With digest authentication you could safely utilize HTTP message body compression without having fear about the "BREACH" family of attacks.

That there would be no interest on a better digest authentication with SHA256/SHA512 algorithms can only be very poor assumption of yours, given that yourself learned about this new standard only a few days ago.
I support implementing RFC7616 as well. I've used older HTTP Digest and having the new one work would be excellent.

I've used Digest plenty times before and appreciate the fact that I do not need to come up with my own password hashing scheme (already provided) and don't have to bother to get an SSL set up right and then keep it up to date. For plenty of use cases Digest is perfect.

Thank you!
Hi all,
I waited for this feature for a very long time, hoping that someone should catch on that RFC7616 is no longer optional but critical.
This is not only about the browser but can increase server side security in the case of database stored digested HA1s.
APIs, modems, routers, js based auth clients, all can benefit from this if used corectly together with TLS.

In the era of GPU computing, MD5 is not only just obsolete, but unsecure as well.

HTTP Digest with only MD5 prevents the use of the strong password encryption, meaning that the HA1 strings stored on the server are in the reach of a single machine with enough GPU power.

I don't know how to raise this bug awareness, but this will become critical sooner than you guys might think.

Hopefully someone watches this and Firefox 56 will have support for RFC7616..
Thank you.
Bulk change to priority:
Priority: -- → P5
Comment on attachment 8683095 [details] [diff] [review]

Review of attachment 8683095 [details] [diff] [review]:


::: netwerk/protocol/http/nsHttpDigestAuth.cpp
@@ +60,5 @@
> +  uint32_t hlen = 16;
> +  if (algorithm & ALGO_SHA256)
> +  {
> +    rv = mVerifier->Init(nsICryptoHash::SHA256);
> +    hlen = 32;

Would prefer to see names for these numbers and ones introduced later in this patch.

::: netwerk/protocol/http/nsHttpDigestAuth.h
@@ +88,5 @@
>                                  nsACString & aHeaderLine);
>    protected:
>      nsCOMPtr<nsICryptoHash>        mVerifier;
> +    char                           mHashBuf[32];

could you use EXPANDED_DIGEST_LENGTH or create a #define? It looks like this code is trying to avoid magic numbers.
Attachment #8683095 - Flags: review?(dveditz) → review+

If someone wants to rebase the patch, and is willing to add unit tests for it in netwerk/test/unit/test_authentication.js it would be great to land it.

Assignee: jduell.mcbugs → nobody

Guys... there is a very common use-case for home routers where they want to transfer authentication without tls.

Making them transfer it in MD5 because the privacy-aware and security-aware browser doesn't implement SHA-256 for this is insecure.

Chrome is also broken for this (ignores algorithm= and continues with md5) but if you think about your own home router... it's a missing piece of the security puzzle isn't it?

You need to log in before you can comment on or make changes to this bug.