920030 - Identity Forgery in BrowserID-Sideshow

Status: RESOLVED FIXED

(Cloud Services :: Server: Identity, defect)

Description

[g.schmitz]

It is possible to forge the email address reported by Gmail to Sideshow. Thus, an attacker can impersonate any Gmail user in BrowserID.

In a typical run, Sideshow redirects the user to the Gmail OpenID endpoint. Such a request looks as follows:

https://www.google.com/accounts/o8/ud?openid.mode=checkid_setup&openid.ns=http%3A%2F%2Fspecs.openid.net%2Fauth%2F2.0&openid.ns.ax=http%3A%2F%2Fopenid.net%2Fsrv%2Fax%2F1.0&openid.ax.mode=fetch_request&openid.ax.type.email=http%3A%2F%2Faxschema.org%2Fcontact%2Femail&openid.ax.required=email&openid.ns.ui=http%3A%2F%2Fspecs.openid.net%2Fextensions%2Fui%2F1.0&openid.ui.mode=popup&openid.identity=http%3A%2F%2Fspecs.openid.net%2Fauth%2F2.0%2Fidentifier_select&openid.claimed_id=http%3A%2F%2Fspecs.openid.net%2Fauth%2F2.0%2Fidentifier_select&openid.return_to=https%3A%2F%2Fgmail.login.persona.org%2Fauthenticate%2Fverify&openid.realm=https%3A%2F%2F*.persona.org

In this request, the email address of the user is requested from Gmail by the following URL parameters:

openid.ax.type.email=http%3A%2F%2Faxschema.org%2Fcontact%2Femail
openid.ax.required=email

The key "email" is arbitrarily chosen according to the OpenID specification. A malicous user can easily change this key to be something else, e.g. emailx:

openid.ax.type.emailx=http%3A%2F%2Faxschema.org%2Fcontact%2Femail
openid.ax.required=emailx

The OpenID endpoint at Gmail will redirect the user after successful authentication to the following URL at Sideshow:

https://gmail.login.persona.org/authenticate/verify?openid.ns=http%3A%2F%2Fspecs.openid.net%2Fauth%2F2.0&openid.mode=id_res&openid.op_endpoint=https%3A%2F%2Fwww.google.com%2Faccounts%2Fo8%2Fud&openid.response_nonce=2013-09-24T11%3A46%3A11Z479iYHqAdS054A&openid.return_to=https%3A%2F%2Fgmail.login.persona.org%2Fauthenticate%2Fverify&openid.assoc_handle=1.AMlYA9WY-N_y7bgaBQJ1NPIlDKor3MSPJjpyL2-ID8Svm3AGp9zVVbjtRubxCOqB&openid.signed=op_endpoint%2Cclaimed_id%2Cidentity%2Creturn_to%2Cresponse_nonce%2Cassoc_handle%2Cns.ext1%2Cext1.mode%2Cext1.type.emailx%2Cext1.value.emailx&openid.sig=BIPe1PIwitMp365MUEMd34IJLUs%3D&openid.identity=https%3A%2F%2Fwww.google.com%2Faccounts%2Fo8%2Fid%3Fid%3DAItOawnpe2gwVe563V5tt1yUqsE4Db-uMsLfSiQ&openid.claimed_id=https%3A%2F%2Fwww.google.com%2Faccounts%2Fo8%2Fid%3Fid%3DAItOawnpe2gwVe563V5tt1yUqsE4Db-uMsLfSiQ&openid.ns.ext1=http%3A%2F%2Fopenid.net%2Fsrv%2Fax%2F1.0&openid.ext1.mode=fetch_response&openid.ext1.type.emailx=http%3A%2F%2Faxschema.org%2Fcontact%2Femail&openid.ext1.value.emailx=ATTACKER%40gmail.com&openid.ns.ext2=http%3A%2F%2Fspecs.openid.net%2Fextensions%2Fui%2F1.0&openid.ext2.mode=popup

In this URL the OpenID assertion is encoded in the parameters. It correctly contains the user's email address, but stored in the value of the key openid.ext1.value.emailx. This value is correctly signed by OpenID. Sideshow expects the email address of the user in the value of the key openid.ext1.value.email. An attacker can now attach the following paramters:

openid.ext1.value.email=VICTIM%40gmail.com
openid.ext1.type.email=http%3A%2F%2Faxschema.org%2Fcontact%2Femail

As in OpenID not every attribute has to be signed, the modified request is still a correctly signed OpenID assertion. Sideshow now sends the assertion to Gmail's OpenID verify service, which says that the signatures are still correct. Sideshow now reads the email value from the key openid.ext1.value.email, which is now [e-mail] and not the attacker's address.

As you can see, this attack is easy to carry out and allows any user to login as ANY Gmail user. We have tested this aginst the current live system. Maybe we have crashed some of your servers there (we got some 502 Bad Gateway responses when the email value was actually not present), sorry for that.

To prevent this attack, Sideshow has to verify that the email address in the OpenID assertion is actually signed.

Comment 1

[Lloyd Hilaiel [:lloyd]]

callahad and seanmonstar - sorry for another fire drill today, but this is a serious flaw in sideshow.  Who has availability to jump on this.  please privately advice jrgm and gene to prepare for a rapid deployment of a fix.

Comment 2

[Lloyd Hilaiel [:lloyd]]

note also 502 - bad gateway responses.

Comment 3

[Lloyd Hilaiel [:lloyd]]

widening the scope to include warner.  larger scope than we want, but these are all trusted team members and we need someone who can jump on this.

Comment 4

[Sean McArthur [:seanmonstar]]

I'll jump on it right after lunch.

Comment 5

[David Chan [:dchan]]

I was able to reproduce this bug though not it's not that clean.

1. Log into a Persona site
2. Go through the account selection flow and choose your account
3. Sign out of the site / clear cookies for site.
4. Start signup flow
5. Select "This isn't me" if prompted
6. Enter the victim's email
7. Intercept request to 
https://www.google.com/accounts/o8/ud?openid.mode=checkid_setup&openid.ns=http%3A%2F%2Fspecs.openid.net%2Fauth%2F2.0&openid.ns.ax=http%3A%2F%2Fopenid.net%2Fsrv%2Fax%2F1.0&openid.ax.mode=fetch_request&openid.ax.type.emailx=http%3A%2F%2Faxschema.org%2Fcontact%2Femail&openid.ax.required=emailx&openid.ns.ui=http%3A%2F%2Fspecs.openid.net%2Fextensions%2Fui%2F1.0&openid.ui.mode=popup&openid.identity=http%3A%2F%2Fspecs.openid.net%2Fauth%2F2.0%2Fidentifier_select&openid.claimed_id=http%3A%2F%2Fspecs.openid.net%2Fauth%2F2.0%2Fidentifier_select&openid.return_to=https%3A%2F%2Fgmail.login.persona.org%2Fauthenticate%2Fverify&openid.realm=https%3A%2F%2F*.persona.org

8. Change email to emailx as in original comment. I've already replaced the email with emailx
9. Continue with request, intercepting the one to
https://gmail.login.persona.org/authenticate/verify?....

10. add
openid.ext1.type.email=http%3A%2F%2Faxschema.org%2Fcontact%2Femail&openid.ext1.value.email=VICTIM%40gmail.com

11. Complete request

If I tried to sign in with my email at step 6 and added the VICTIM at step 10, I would get a 409 Conflict

Accounts don't match

You're currently signed into Google as [e-mail]

If you want to use [e-mail], log out of Google and sign in again.

Comment 6

[Sean McArthur [:seanmonstar]]

Nevermind, I'll be on this in 15.

Comment 7

[Sean McArthur [:seanmonstar]]

Dchan, what if you use VICTIM at step 6? Starting the process with the target email, authing with Google with your own email?

Comment 8

[David Chan [:dchan]]

(In reply to Sean McArthur [:seanmonstar] from comment #7)
> Dchan, what if you use VICTIM at step 6? Starting the process with the
> target email, authing with Google with your own email?

It looks like you get the 409 Conflict error starting at step 6. I was completely logged out of my google account when trying this. Some magic happens if you go through the account chooser process once then re-login to Identity in the same browser session. The account chooser is not presented the second time.

Comment 9

[Sean McArthur [:seanmonstar]]

Attachment: 0001-check-that-ext1.value.email-is-signed.patch

Checks that `ext1.value.email` is part of the signature, fails if it is not.

Comment 10

[Lloyd Hilaiel [:lloyd]]

Comment on attachment 809373 [details] [diff] [review]
0001-check-that-ext1.value.email-is-signed.patch

Review of attachment 809373 [details] [diff] [review]:
-----------------------------------------------------------------

r+

Comment 11

[Lloyd Hilaiel [:lloyd]]

sean and jrgm are going to work with gene to deploy this patch to production.

warner and ckarlof are verifying the fix by reproducing in a test environment, then applying the above patch.

I'm going to bed!

Comment 12

[Dan Callahan [:callahad]]

I'm back on a network and awake for another couple of hours. Want me to roll up an RPM with this? How can I help?

ALSO: Have we verified that bigtent is unaffected?

Comment 13

[Brian Warner [:warner :bwarner]]

Attachment: mitmproxy exploit script

Here's a quick script to doctor the URLs. If you run this under mitmproxy (http://mitmproxy.org) (which involves importing the CA cert and setting your proxy config to deliver HTTP and HTTPS to the mitmproxy port), then go to 123done.org and sign in as "[e-mail]", then when you land on the gmail login page, login as any valid gmail account. You should get bounced back to 123done.org as "[e-mail]" and have full control over that account (on 123done.org).

Comment 14

[Dan Callahan [:callahad]]

RPM: http://people.mozilla.org/~dcallahan/tmp/browserid-bridge-gmail-0.9.3-0.1.20130924SHA183f3a9R118029.el6.x86_64.rpm

shasum  a34b4e9cd5dbe761fc059d86991ae60b4b949a44

Previous RPM: http://people.mozilla.org/~dcallahan/tmp/browserid-bridge-gmail-0.9.2-0.1.20130718SHAb18e22eR118029.el6.x86_64.rpm

Comment 15

[Sean McArthur [:seanmonstar]]

ccing ozten because this likely is also in bigtent (yahoo)

Comment 16

[Austin King [:ozten]]

Does this reproduce for yahoo.com email addresses (aka Bigtent)?

It is a different codepath than sideshow.

(I'm still reading and getting setup to try to reproduce).

Comment 17

[Brian Warner [:warner :bwarner]]

yes, it also hits yahoo.com. bug 920301 is for that one.

Comment 18

[Lloyd Hilaiel [:lloyd]]

What's our deployment timing on the gmail fix?

Comment 19

[John Morrison [:jrgm]]

QA has signed off on the fix. It can go live when Gene can build prod stacks.

Comment 20

[Lloyd Hilaiel [:lloyd]]

tore down ephemeral instances where dev/testing occured (gpersona and gsideshow)

Comment 21

[Gene Wood [:gene]]

Deployment is being tracked in Bug 920260

Comment 22

[Brian Warner [:warner :bwarner]]

I should ask the same question as on bug 920301: what does our web framework do if the query string contains two copies of "openid.signed="? If it doesn't reject such a thing, we could be open to another form of attack (depending upon how the IdP's verifier parses that same string).

Comment 23

[Austin King [:ozten]]

An array of strings is returned instead of a string.

Example: some_url?openid.signed=foo,bar&openid.signed=buzz,baz

req.query['openid.signed'] gives us ["foo,bar","buzz,baz"]

Comment 24

[Daniel Veditz [:dveditz]]

["See also" is generally for linking to other bug systems; moving these to "depends on"]

If we've deployed the fix (bug 920260) can this be closed FIXED now? The Bigtent version is still open but they seem to be independent fixes.

Comment 25

[John Morrison [:jrgm]]

The bigtent fix was deployed @18:15 PDT. Closing.

Comment 26

[John Morrison [:jrgm]]

And, to be clear, both sideshow and bigtent fixes are deployed.

Comment 27

[g.schmitz]

The fix is not sufficient as it tests for the parameter ext1.value.email to be signed. However an attacker can inject a parameter with a different prefix (instead of ext1). Sideshow always takes the parameter containing an email address using the first prefix configured for email addresses it finds. See also https://github.com/havard/node-openid/blob/master/openid.js#L1475

To reproduce this:

We intercepted the redirect from the OpenID endpoint back to Sideshow and injected the following parameters in front of all other URL parameters:

openid.ns.foo=http%3A%2F%2Fopenid.net%2Fsrv%2Fax%2F1.0
openid.foo.mode=fetch_response
openid.foo.type.email=http%3A%2F%2Faxschema.org%2Fcontact%2Femail
openid.foo.value.email=VICTIM%40gmail.com


The implementation should check the prefix which it takes the email address from.

Comment 28

[Sean McArthur [:seanmonstar]]

Attachment: 0001-protect-agaisnt-email-property-not-being-signed.patch

Updated patch that checks for namespace first.

Comment 29

[Austin King [:ozten]]

Reading your second patch, I it looks good, but some additional thoughts:

Please see Bug#920301 Comment#29 for an alternative approach.

I think that in sideshow, we are trusting the result object from

openidRP.verifyAssertion(req, function (error, result)

It will be populated with email or emails... depending on the query string on the openid return url.

If it is just email, then you're good since you've checked the req.query has the proper email value in 'openid.signed'. If result has emails... then you know that you have additional unsigned emails and you should bail. This will effectively happen in sideshow, assuming I'm reading the code correctly.

Comment 30

[Dan Callahan [:callahad]]

Oh wow. I'm an idiot. I finally was able to reproduce this issue, after three hours of making the same stupid mistake with mitmproxy.

Verifying Sean's patch now...

Comment 31

[Dan Callahan [:callahad]]

I wrote a different, more verbose, more paranoid patch. Review, please?

Comment 32

[Dan Callahan [:callahad]]

Attachment: 0001-Be-extremely-cautious-when-using-OpenID-params.patch

Alternative, verbose patch.

Comment 33

[Dan Callahan [:callahad]]

Attachment: mitmproxy exploit script

Updated warner's exploit script to allow testing each of the two discovered bugs.

Comment 34

[Dan Callahan [:callahad]]

Attachment: mitmproxy exploit script

Heh. Whoops. Upload a *working* mitmproxy exploit script.

I've used this to iteratively test that 0.9.2 was vulnerable to both, 0.9.3 is only vulnerable to the namespace issue, and my patch is vulnerable to neither.

Beningn logins also work successfully for me.

Comment 35

[Dan Callahan [:callahad]]

Attachment: 0002-Make-sure-op_endpoint-is-not-altered-in-flight.patch

Also bail out if the OpenID endpoint changes to anything funny.

Comment 36

[Dan Callahan [:callahad]]

Attachment: 0003-More-OpenID-tomfoolery.patch

Add a third patch guarding against claimed_id being tampered with

Comment 37

[Dan Callahan [:callahad]]

RPM containing patches 1 through 3 above, mitigating:

1. Unsigned email fields.
2. Malicious namespace alterations.
3. Endpoint tampering.
4. Claimed ID spoofing.

RPM: http://people.mozilla.org/~dcallahan/tmp/browserid-bridge-gmail-0.9.6-0.1.20130926SHAde46b0dR118029.el6.x86_64.rpm
SHA: dae27b0367d3a1ad8ff8af9fb492b51a96cbde98

Comment 38

[Dan Callahan [:callahad]]

Attachment: Diff between 0.9.3 and 0.9.6

Added a diff showing the net change from 0.9.3 (current production) to 0.9.6 (RPM in the comment above)

Comment 39

[Brian Warner [:warner :bwarner]]

The vulnerable places I've seen so far:

1: set [openid.claimed_id] to attacker.com/anything, which always returns OK
2: set [openid.op_endpoint] to attacker.com/something
3: just include [email]=victim in the response
4: modify response to use an ax extension with regexp metachars (like "ext1."
   instead of "ext1", somehow take advantage of the regexp injection in
   openid.AttributeExchange.fillResult to let the signature get verified on
   [openid.ext1.value.email] but copy a different [openid.ext1A.email] into
   the response
5: modify request to get an "ax*" extension, also target the regexp

I'm testing current prod (which only has seanmonster's first patch), and
2/3/4 are safe (blocked for various reasons). I suspect 1 will be
exploitable. 5 will depend upon how yahoo responds.

Comment 40

[Austin King [:ozten]]

> 1: set [openid.claimed_id] to attacker.com/anything, which always returns OK
> I suspect 1 will be exploitable.

Wouldn't our limiting of outbound http/https requests block this attack?
Does production not limit outbound connections, or am I misunderstanding the attack?

Comment 41

[Brian Warner [:warner :bwarner]]

(In reply to Austin King [:ozten] from comment #40)
> Wouldn't our limiting of outbound http/https requests block this attack?

I believe you're right.

None of the sneaky things I've tried so far were able to work. For #1, I didn't see any hits to the external server I set up as a provider: jrgm and I could see the /auth/yahoo/return come back in with my own provider, and then we saw an error ("Failed to verify assertion: No OpenID provider was discovered for the asserted claimed identifier"), but we were unable to tell whether this was due to a firewall blocking the discovery request, or some additional software check that I haven't located yet. (I'd feel better if it were both).

#2 is blocked by the openid module comparing the response's endpoint against the provider's value. #3 doesn't work because the AttributeExchange extension overwrites .email (if those extended attributes are present, so this depends upon our other extra validation code working correctly). #4 was invalid: it's not the attribute *names* that get injected into a regexg, it's the *namespace* value. So #5 could be valid (which is about getting a namespace with metachars, e.g. q[openid.ns.ax*]="http://openid.net/srv/ax/1.0"), but from what I can tell, neither yahoo nor gmail copy the requested namespace into the response (the response namespace is always "ext1" for gmail and "ax" for yahoo).

There's a #6 where you add q[openid.ns.ax*] to the response, so you get one set of names that are signed (openid.ax.value.email) and a second set which are not (openid.axFOO.value.email), if the axFOO form comes first, the fillResult() function might grab it as the email instead of the signed one. I'm still looking into this one.

Comment 42

[Brian Warner [:warner :bwarner]]

My #6 is thwarted by callahad's excellent validation script: to work, we'd need two keys of the email-type, and his function rejects responses that have more than one. If it weren't for you meddling kids, I mean, for that thorough check, I might be able to pull it off.

I have one more sneaky trick up my sleeve, but so far it hasn't worked anywhere, probably doesn't affect our specific deployment, and should probably go into a separate bug.

Comment 43

[Dan Callahan [:callahad]]

I believe your #6 is identical to the namespace bug that caused us to re-open this in the first place?

Comment 44

[Brian Warner [:warner :bwarner]]

It overlaps, but the original namespace problem didn't depend upon that
regexp injection bug.

There are a bunch of partial problems here: all the exploits I've been able
to think of are blocked by one defense (sometimes accidentally), but it'd be
safest if all of them were blocked by everything. If somebody changes
something down the line, two of those barriers might cease to overlap, and a
hole could open up. Defense in depth.

E.g. yahoo/gmail fortunately don't let you control the namespace that they
use for their signed parameters. That policy, plus your validation function
which asserts that the namespace is in the signed list, prevents metachars
from getting into the namespace field. But if they accidentally changed that
policy (maybe by copying the namespace of the request into the response),
then we could get validly-signed responses that contain one email field that
gets signature-checked and a second which doesn't (but gets used by
fillResult). *Then* the remaining line of defense would be the part of your
validation function which insists upon a single email-type field, and if some
other hole were to develop, that might not be good enough.

Some other changes I'd recommend for your validParams() function:

* googleAccountRegex admits claimed_ids from "wwwXgoogle.com". Every regexp
  metacharacter must be escaped. Even safer avoid regexps in favor of however
  you spell .startswith() in JS.
* I'd move the claimed_id check to the top. If it's not a google provider, we
  can't trust anything else. I like to do checks from the most coarse to the
  most fine.
* check all params to make sure they're single strings, not arrays. There
  should be no duplicate keys anyways, and that will protect against
  incautious operations on the params (not that you're using any, but someone
  in the future might do param[foo].indexOf). Do this at the top, before
  letting any code examine the params.
* assert that all param values contain only printable characters

But that's really an awesome validation function.. I love it.

Comment 45

[Michael Coates [:mcoates] (acct no longer active)]

What actions are remaining to close out this bug?

Comment 47

[Austin King [:ozten]]

I believe we're finished here. Please reopen if I'm wrong.

Comment 48

[Gene Wood [:gene]]

Sean, should we re-open this ticket (just noticed your attachment and review request)?

Comment 49

[Sean McArthur [:seanmonstar]]

No, sorry. Bugzilla kept bugging me about an open review request, so I just marked an old attachment reviewed to shut it up.

Comment 51

[Dan Callahan [:callahad]]

Comment on attachment 810616 [details] [diff] [review]
0001-protect-agaisnt-email-property-not-being-signed.patch

Review of attachment 810616 [details] [diff] [review]:
-----------------------------------------------------------------

Clearing really old review flag.

Also, fixed my filters so I actually see these nags from Bugzilla.