Closed Bug 226278 Opened 21 years ago Closed 20 years ago

Password cache for http auth should remember if the site was secure

Categories

(Core :: Networking: HTTP, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.7final

People

(Reporter: cneberg, Assigned: darin.moz)

Details

(Keywords: fixed1.4.3, Whiteboard: [sg:fix]security)

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5a) Gecko/20030728 Mozilla Firebird/0.6.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5a) Gecko/20030728 Mozilla Firebird/0.6.1

Forgive me if I'm missing something, but browsing through the code it appears
that the password cache for HTTP, remembers realm, hostname and port, to
determine if it should replay a password to a site that it had seen previously.
 I believe the list should actually be realm, hostname, port, and whether it was
a secure site.  Just because a site is on port 443 it does not mean that the
site is under SSL.   

An attacker could allow you to enter your password to your secure site, then
latter during the same browsing session spoof DNS for a non-ssl version of the
same url you hit previously on the same port.  Your browser would happily send 
your password to them, no SSL required.

I haven't had time to test this theory.

Reproducible: Didn't try

Steps to Reproduce:
I've done some more testing. I configured a web server using SSL on on port 
8443 and Mozilla successfully authenticates using basic authentication.   Then 
I changed the webserver so port 8443 was not a secure port.  Mozilla sent my 
password to it in the clear, without re-prompting me. 

DNS spoofing is one way to cause this to occur, another is that your DNS 
servers are compromised, but the case I am most concerned about is a modified 
or compromised proxy server. It would be simple to modify a proxy server to 
randomly insert an image link into a later document (like http://www.somesite-
you-have-previously-sent-your-password-to.html:443/image.jpg). The image could 
be 1 pixel wide, so you would not notice.  If you previously had authenticated 
to the site, the proxy server could become a password harvestor by saving your 
response.
cneberg: good catch!  this is a serious bug.
Severity: major → critical
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: blocking1.6?
Target Milestone: --- → mozilla1.6beta
from RFC 2617:

  The realm value (case-sensitive), in combination with the canonical root URL 
  (the absoluteURI for the server whose abs_path is empty; see section 5.1.2
  of [2]) of the server being accessed, defines the protection space.

so, http://foo.com:443/ and https://foo.com:443/ are different absoluteURI values.
so, http://foo.com:443/ and https://foo.com:443/ are different *protection spaces*
Are there any other places where bugs like this might live?  Cookies maybe?
actually, cookies are different.  individual cookies can be tagged with a secure
flag by the server.  this indicates that they should not be shared between HTTPS
and HTTP sites.  otherwise, they can be shared.

the password manager database (for single signon -- not the same thing as HTTP
auth), stores passwords by scheme + host + port.  so, i think we are covered there.
cneberg: do you think it makes sense to apply the same protocol distinction to
proxy auth?  right now if a proxy server supports HTTP and HTTPS proxy, the user
will only have to enter their username+password once.  should we prompt them
once for each proxy server?  i'm thinking that we must in order to avoid the
security risk of not doing so.  what do you think?
it turns out that IE makes no distinction between basic auth over HTTP and basic
auth over HTTPS to the same proxy server.  it happily sends the same credentials
to for other protocols to the same proxy server.

here's what i did:

1) configured IE to use a squid proxy under my control
2) configured squid to require basic auth for HTTP and HTTPS connections
3) loaded https:// URL in IE, and entered my username+password
4) loaded http:// URL in IE.  was not prompted for my username+password.

on one hand this seems reasonable since it is supposed to be the same server. 
on the other hand, it is pretty risky since the HTTP proxy server could be
spoofed.  hmm... though that is probably true of a HTTPS proxy as well, since
there is no SSL communication between the HTTPS proxy and the browser.  hmm...

i'm leaning toward only fixing this bug for non-proxy traffic.
Questions:

Is it even possible to encrypt control traffic to a proxy server?  I was under 
the impression that control traffic was always unencrypted no matter what site 
you where going to http or https.

For multiple protocols do you normally configure the same proxy machine and 
port no matter what the protocol? Or do you normally configure a different port 
for each protocol?

If I'm right in assuming that control traffic is always unencrypted, then it 
probably makes sense to only prompt the user once for proxy server no matter 
what protocol that they are using through it.

If you actually have different proxy machines configured, then you would want 
to authenticate to each of them seperately.
>Is it even possible to encrypt control traffic to a proxy server?  I was under 
>the impression that control traffic was always unencrypted no matter what site 
>you where going to http or https.

the only "control traffic" (not sure i understand the term) seen by the proxy in
the case of HTTPS is a request by the browser for the proxy server to open a
forwarding port to a particular host:port.  the browser says "CONNECT host:port
HTTP/1.1" and the proxy either returns 200 OK or some error, which can include a
407.  after a 200, any bytes sent to the proxy get forwarded directly to the
origin server.  in other words, the proxy cannot see any of the traffic.


>For multiple protocols do you normally configure the same proxy machine and 
>port no matter what the protocol? Or do you normally configure a different port 
>for each protocol?

no, not necessarily.  each protocol can use a different proxy server.  in fact,
with PAC (proxy auto config), it is possible to configure the browser to use a
different proxy server for each URL.


>If I'm right in assuming that control traffic is always unencrypted, then it 
>probably makes sense to only prompt the user once for proxy server no matter 
>what protocol that they are using through it.

i agree.


>If you actually have different proxy machines configured, then you would want 
>to authenticate to each of them seperately.

yes, this is what we do today.


so, the take-away from this is that we should not use the protocol scheme as
part of the auth key for proxy auth caching.
>>the only "control traffic" (not sure i understand the term) ... "CONNECT
>> host:port HTTP/1.1" 

Yeah that's what I meant, sorry about the confusion in terminology.

>>we should not use the protocol scheme as part of the auth key for proxy auth
caching.

Sounds good.
Darin, any progress on a patch for this problem?
darin's looking at ways to fix...  1.6blocking +
Flags: blocking1.6? → blocking1.6+
Target Milestone: mozilla1.6beta → mozilla1.6final
one more thing complicates this bug:

  nsIJVMAuthTools

is what the Java plugin uses to interact with necko's authentication cache.  it
can set or get auth entries.  unfortunately, the Java plugin does not
distinguish origin server authentication from proxy server authentication.

this means that if we make the change required to fix this bug that we run the
risk of breaking the nsIJVMAuthTools contract.  that interface cannot change, so
i'm a bit unsure how to get around this problem.
i think a low-risk patch for this bug is unlikely.  it would involve reworking a
good amount of code.

this security threat depends on the following:

1) attacker needs to be able to spoof DNS
2) attacker needs to trick user into visiting a specific URL

next steps:

test java plugin to determine what it sends for the "protocol" parameter when
doing proxy authentication.  i suspect it probably sends "http" when doing a
normal proxy connection and "https" for SSL-tunnel.  but, this would make it
impossible for us to distinguish proxy auth from non-proxy auth, which means
prompting once for each protocol type used with a proxy server.  that may be the
right tradeoff, but it would be a regression :(
Is this also a problem for forms containing usernames and passwords (in addition
to http auth)?
jesse:

no, it should not be.  if you look at password manager, you will see that some
"sites" are prefixed by the URI scheme, and some are not.  the ones that are not
correspond to HTTP auth.

come to think of it, password manager further complicates this problem.  we will
need to implement some code to transition password database over to the new key
format.  this will be tricky too... especially since HTTP currently interacts
with password manager indirectly via nsIAuthPrompt.  perhaps some password
manager changes will help.  we'd want it to try using the old key if a value
cannot be found under the new key, but we'd want it to always create entries
using the new key format.  we've had to do things like this before in the past
when changing the key format for other password manager entries.

considering what's involved here, i think a fix for 1.6 would be very risky :(
ok.  let's try and get something for 1.7a
Flags: blocking1.7a+
Flags: blocking1.6-
Flags: blocking1.6+
Target Milestone: mozilla1.6final → mozilla1.7alpha
Summary: Password cache should remember if the site was secure → Password cache for http auth should remember if the site was secure
Whiteboard: security
Priority: -- → P1
Target Milestone: mozilla1.7alpha → mozilla1.7beta
Flags: blocking1.7b+
Flags: blocking1.7a-
Flags: blocking1.7a+
try for 1.7 final
Flags: blocking1.7b-
Flags: blocking1.7b+
Flags: blocking1.7+
sounds like the migration password entries from "host:port (realm)" to
"scheme://host:port (realm)"  could really use a around of testing in an alpha
to catch all the possible corner cases.  might be beter in 1.8a
It's a security bug which has now been known about for sometime.  Isn't that 
sufficient reason to try and get it into the next release rather than waiting? 
It's already been pushed back serveral times.
This is a minor security bug IMO mainly because the user has to OK a prompt
before their stored username and password gets sent to the attacker.  Unlike
forms on a webpage, there's no way for the attacker to get at the prefilled
authentication credentials unless the user presses the OK button.

That said, I think this is an important bug to fix.  The problem is that it is
difficult to do correctly.

We have to be careful not to cause dataloss when we migrate the keys used to
store the passwords.
(In reply to comment #22)
> This is a minor security bug IMO mainly because the user has to OK a prompt
> before their stored username and password gets sent to the attacker.  Unlike
> forms on a webpage, there's no way for the attacker to get at the prefilled
> authentication credentials unless the user presses the OK button.
 
There are at least two ways this security problem can occur.

1. A user is asked to click OK to submit his saved credentials to a non-ssl
version of the website (when you orginally saved your password, it was an ssl
site).  Presumably the user might notice that the site is not SSL and click
cancel.  I believe this is the problem you are referring to, and I agree that it
may not be that big of a problem.

2. You enter your password into an HTTP basic auth prompt (it could be from
saved credentials or not) and click OK.  The site you are entering your password
to is actually SSL protected and legitamet. Later during the same browser
session you browse to a hacker's site (or you go through a compromised proxy
server or comprompised DNS).  They send you a link to an image file on a spoofed
non-ssl protected version of the site where you orginally entered your password.
  They steal your password - no password prompt, no OK button, nothing.  There
is no reason you would know that your password was stolen. If anyone accesses a
network a hacker controls or configures (including a wireless accesspoint), he
can steal all of the user's password's sent over HTTP basic auth, and SSL will
not protect the user.  This is the problem I'm really worried about and it does
not involve migrating password entries, but I do understand from your previous
postings fixing java password cache integration may still be difficult.  

summary:  Access a network I control, and I could configure it to steal all of
your HTTP basic passwords and SSL will not help you, period.

> We have to be careful not to cause dataloss when we migrate the keys used to
> store the passwords.

Understood.
I did some investigation with the Java 1.5.0 beta plugin to see what it passes
to nsIJVMAuthTools::GetAuthenticationInfo.

What I found is the following:

>> GetAuthInfo [protocol=http host=localhost port=4040 scheme=basic realm=Squid
proxy-caching web server]
>> GetAuthInfo [protocol=http host=gogi port=80 scheme=basic realm=FriedFish]

The first case is for proxy authentication, and the second case is for origin
server authentication.  Notice that there's no way to determine from these
parameters that the first is for proxy authentication.  This suggests that it
will be difficult to share authentication credentials between HTTP and HTTPS
proxies.

However, when I point the Java plugin at a https:// URL, it tries to open a SSL
tunnel to the proxy server, and it queries Gecko's auth cache like so:

>> GetAuthInfo [protocol=http host=localhost port=4040 scheme=basic realm=Squid
proxy-caching web server]

Notice that it passes the protocol as "http" :-)

This means, I think, that we can solve this bug without breaking the Java
plugin.  That's a relief.
(In reply to comment #23)
> 2. You enter your password into an HTTP basic auth prompt (it could be from
> saved credentials or not) and click OK.  The site you are entering your password
> to is actually SSL protected and legitamet. Later during the same browser
> session you browse to a hacker's site (or you go through a compromised proxy
> server or comprompised DNS).  They send you a link to an image file on a spoofed
> non-ssl protected version of the site where you orginally entered your password.
>   They steal your password - no password prompt, no OK button, nothing.

You are correct.  I did not consider this case earlier.
Here's a thought:

We could start by fixing the more serious problem.  We fix the HTTP auth cache
so that it uses the full base URI as the key.  We can fixup nsIHttpAuthManager
and make nsJVMAuthTools pass the protocol scheme field to it.  Then we'll have
solved the more severe problem.

The second problem, which involves what the password manager automatically
suggests could be solved at a later time.  Since it is the more risky part of
the total solution, it might make sense to break things up like this.
(In reply to comment #26)

Sounds good.
Attached patch v1 patchSplinter Review
straightforward patch for part 1.
Attachment #144079 - Flags: review?(cneberg)
I applied the patch to 1.7a and tried the orginal test I had performed previously.

http://foo.com:443/ and https://foo.com:443/ are now correctly considered
different protection spaces by the Auth cache. 

I liked the addition of the protocol to the password prompt.

I didn't test proxy auth, or Java HTTP basic auth, but the changes looked
reasonable.  One confusing point I saw in the code was that in most places you
consider a function argument named named scheme to refer to HTTP or HTTPS, and
authType to mean Basic, but in the Java conectivity parts you use scheme to mean
Basic, and protocol to mean HTTP or HTTPS. Using using the word Scheme in two
completely different ways may lead to confusion later. 

For whatever reason, I don't seem to have access to the flags that would let me
mark a reviewer flag, but if my r counts for anything r=cneberg.
good point about JVM parameter names being different.  that has to do with the
names for those parameters used by Java in its HTTP authentication callback
mechanism.  i'm torn between using the necko terminology vs. the Java
terminology in the OJI module.  i figure java developers will be looking at the
OJI code, so perhaps its better to leave it Java friendly :-/

cneberg: thanks for looking over the patch.  i really appreciate it!  i suspect
your bugzilla account simply has limited privileges.  i'll talk to someone at
the foundation about increasing your privileges.  thanks again!
Comment on attachment 144079 [details] [diff] [review]
v1 patch

dveditz: can you please sr= this security fix.	thanks!
Attachment #144079 - Flags: superreview?(dveditz+bmo)
Attachment #144079 - Flags: review?(cneberg)
Attachment #144079 - Flags: review+
Target Milestone: mozilla1.7beta → mozilla1.7final
Comment on attachment 144079 [details] [diff] [review]
v1 patch

>Index: netwerk/protocol/http/src/nsHttpAuthCache.cpp
>===================================================================
>+GetAuthKey(const char *scheme, const char *host, PRInt32 port, nsCString &key)
> {
>-    key.Assign(host);
>+    key.Assign(scheme);
>+    key.Append(NS_LITERAL_CSTRING("://"));
>+    key.Append(host);
>     key.Append(':');
>     key.AppendInt(port);

This routine probably doesn't get a high-volume of calls, but it can be more
efficient (saves multiple reallocs as a string grows) to take advantage of
string's operator+

    key = nsDependentString(scheme) +
	  NS_LITERAL_CSTRING("://") +
	  nsDependentString(host)   +
	  NS_LITERAL_CSTRING(":");
    key.AppendInt(port);

Is it valid for consumers to pass in a null scheme or host? Seems dodgy but the
original code would handle it. nsDependentString() MUST NOT be used on null
pointers so if that's a legal possibility you can't use what I've suggested
without adding error checks.

>     // |host| and |port| are required
>     // |path| can be null
>     // |entry| is either null or a weak reference
>-    nsresult GetAuthEntryForPath(const char *host,
>+    nsresult GetAuthEntryForPath(const char *scheme,
>+                                 const char *host,

Add |scheme| to the comments in this section. Required, I assume?

>Index: modules/oji/src/nsJVMAuthTools.cpp
>===================================================================
>     
>+    nsDependentCString protocolString(protocol);
>     nsDependentCString hostString(host);
>+    nsDependentCString schemeString(scheme);

See earlier comments about nsDependentString -- Add "|| !scheme" to the first
error check in this routine (where protocol and host are already checked).

>@@ -190,23 +193,27 @@ nsJVMAuthTools::SetAuthenticationInfo(co
>     if (!protocol || !host || !realm)
>         return NS_ERROR_INVALID_ARG;
>     
>     
>+    nsDependentCString protocolString(protocol);
>     nsDependentCString hostString(host);
>+    nsDependentCString schemeString(scheme);

Ditto here

sr=dveditz with those nits fixed.
Attachment #144079 - Flags: superreview?(dveditz+bmo)
Attachment #144079 - Flags: superreview+
Attachment #144079 - Flags: approval1.7?
Comment on attachment 144079 [details] [diff] [review]
v1 patch

a=chofmann for 1.7
Attachment #144079 - Flags: approval1.7? → approval1.7+
Thank you for the review, Dan!
dveditz: host and scheme must be non-null.  i'll fix up the comments.
fixed-on-trunk for 1.7 final.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
(In reply to comment #26)
> Here's a thought:
> We could start by fixing the more serious problem.  We fix the HTTP auth cache
> so that it uses the full base URI as the key.  We can fixup nsIHttpAuthManager
> and make nsJVMAuthTools pass the protocol scheme field to it.  Then we'll have
> solved the more severe problem.

> The second problem, which involves what the password manager automatically
> suggests could be solved at a later time.  Since it is the more risky part of
> the total solution, it might make sense to break things up like this.

Should I open a new security sensitive bug for the second problem? 
Bug 240058 added to address password manager issues.
Adding Jon Granrose to CC list to help round up QA resources for verification
Whiteboard: security → [sg:fix]security
Removing security-sensitive flag for bugs on the known-vulnerabilities list
Group: security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: