Closed Bug 162520 Opened 22 years ago Closed 22 years ago

possible princeton-style password stealing exploit

Categories

(SeaMonkey :: Passwords & Permissions, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.2beta

People

(Reporter: darin.moz, Assigned: darin.moz)

Details

(Whiteboard: [sg:blocker])

Attachments

(1 file)

the fix for bug 161228 possibly re-introduced the princeton-style exploit for
password manager.

mitch, dveditz and i discussed this issue before choosing to go ahead with the
fix for bug 161228, which was a regression resulting from the original princeton
attack fix (bug 149943).

basically, the problem is that our DNS table now maps host:port -> ipaddress
instead of host -> ipaddress.  see bug 161228 for a description of why this was
necessary.  as a result, we may connect to a different ip node depending on the
desired port.  this is expected, but since in most cases password manager only
stores the hostname of the site corresponding to the remembered password,
mozilla might be vulnerable to giving out a remembered password to a malicious
website.  

i'm not certain that an attack actually exists in this case, because the user
would have to enter their password under a domain controlled by the attacker.
at first glance, that seems unlikely.
Shouldn't the password manager always store the password based on
protocol+hostname? Otherwise an attacker can steal passwords that should be been
protected by SSL by faking up a non-SSL connection.

Consider this scenario:

Alice connects to https://site.com and enters her password.
Later, Alice connects to http://site2.com.
Mallory hijacks Alice's HTTP connection to http://site2.com and injects an
<IFRAME SRC="http://site.com">.
Mallory then hijacks Alice's HTTP connection to http://site.com, injecting a
password-capturing form field and arranging the content so that a button that
appears to be on site2.com actually submits the form.
Alice clicks on the button, and Mallory gets Alice's https://site.com password.
yes, indeed, that is the big problem.  because we used to "pin" the ip-address
of site.com, it was much harder to spoof site.com (although in your example,
another browser session would have defeated our ip-address pinning).  now it
becomes easier since the ip-address of https://site.com and http://site.com are
not necessarily the same.

password manager should store not only the protocol and hostname, but also the
port.  this will be tricky to do without breaking existing password databases. 
i'm not exactly sure how best to migrate old password databases.
Severity: normal → critical
Status: NEW → ASSIGNED
Keywords: mozilla1.2, nsbeta1
Priority: -- → P1
Target Milestone: --- → mozilla1.2beta
When you see a legacy entry for HTTP auth, convert it to a new entry for
HTTPS:667 (or whatever the port is for https). This is nice and conservative
because HTTPS shouldn't be spoofable. This means that people will have to
reenter their passwords for plain HTTP sites, but there should not be so many of
those because those passwords are easily sniffed.

This bug is actually quite serious; the scenario I outlined is not hard to pull
off by someone who is on the same local network as the user. We really need this
fixed for 1.2.
sounds like a good plan... working on a patch.
one possible problem.  a lot of the time, a password form is served up via
http:, but the resulting form submission is to a https: site.  password manager
only knows about the http: site.  so, while the password is meant to be securely
transfered, password manager will make it appear as though it is an insecurely
transfered password.

this is probably still ok, but it screws with our assumption that most password
entries are for https: sites.  in fact, most in my password file are for http:
sites that get POSTed to a https: site.  what to do, what to do.
If we key off the form's protocol+port then those passwords will be stealable.
Can't we change the password engine to key off the form submission URL?
An HTTP form page is just plain insecure against active attacks. Someone can
hijack the connection, and replace the form page with their own form that
uploads the password to their own server (e.g. by generating a GET request using
a carefully constructed dynamically generated IMG SRC). These sites should be
evangelised.

Anyway, this means we might as well key off the form's protocol and port since
anything else is no more secure.

How about this, a little more complicated: leave legacy (host only) entries
alone initially. When looking for a password entry, look up using
host+port+protocol first, and use that if found. If not found, look for a legacy
entry using only host. If we get a legacy entry, then upgrade the entry by
adding the current port+protocol to its key.
yeah, that's sort of what dveditz and i just came up with.  we could try to
phase out old-style password entries as we visit them (possibly inside the code
to remember a password entry).
i thought about this solution some more and i'm starting to think that it might
be acceptable to leave the existing password entries as is.  that way a user
would not lose the ability to go back to an older version of mozilla.  this is
not really a complete solution as existing profiles will be vulnerable, but at
least any new passwords will be stored with the fully qualified key.  does this
sound good to everyone?
Attached patch v1 patchSplinter Review
this patch makes the following changes:

1- password forms now use "scheme://host[:port]" as database key.  the port is
optionally not specified since the scheme implies a default port.  this seems
reasonable since the keys will be shown to users in the password manager UI. 
this format should be familiar to users since it resembles URLs.

2- database lookup (SINGSIGN_RestoreSignonData) will first try the fully
qualified key.	if password entries is not found, then it will try the
old-style host key.

3- database write (SINGSIGN_RememberSignonData) will always write a fully
qualified key.	if password entry exists under old-style key, it will not be
modified or deleted.

i factored the creation of the fully qualified key out into a helper function
called si_ExtractRealm, which takes a nsIURI as an argument.

please review.
Comment on attachment 103020 [details] [diff] [review]
v1 patch

r=morse
Attachment #103020 - Flags: review+
Comment on attachment 103020 [details] [diff] [review]
v1 patch

>+  realm = scheme + NS_LITERAL_CSTRING("://") + hostPort;

Would nsIURI::GetPrePath() have simplified this? I guess you're trying to avoid
any username/password by doing this, but you could argue that if the URI
specifies a username/password on a password form then it might know something
we don't and require different single-signon values.

I'm fine with this, just wanted to raise the question.

sr=dveditz.  Is this worth getting on the branch, too?
Attachment #103020 - Flags: superreview+
I really think we should do something about legacy passwords.

Maybe in six months or so we could land a patch which removes any legacy
passwords :-).
dveditz: i thought about that, but the username/password is parsed out of the
URL by necko before communicating with the site.  in fact, HTTP only sends
username/password to the server after it is challenged.  so, i think we should
be ok filtering out username/password.  yeah, i think this is important for moz
1.0.2.

roc: yeah, that sounds like a good idea to me.  i'll file a follow up bug.
Keywords: adt1.0.2
Attachment #103020 - Flags: approval+
Comment on attachment 103020 [details] [diff] [review]
v1 patch

a=asa for checkin to 1.2 (on behalf of drivers).
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Keywords: mozilla1.0.2
Resolution: --- → FIXED
i filed bug 175120 about the legacy passwords.
adt+ for branch -- have you mailed drivers for approval?
Keywords: adt1.0.2adt1.0.2+
dveditz: no i have not, but i will ;-)

we will also want the fix for bug 175495 on the branch in addition to this patch.
a=rjesup@wgate.com for 1.0 branch; change mozilla1.0.2+ to fixed1.0.2 when
checked in.
fixed1.0.2
Verified fix checked into bonsai.mcom.com, adding verified1.0.2 keyword
Group: security
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: