XSRF?

[ringmaster]

Thinking about implementing the changepassword method in the username-
password form profile, wouldn't this open up the opportunity for XSRF?

Trying to implement this in Habari brings us to the system's "change
password" form where a user can enter a username and password (must be
entered twice for confirmation), but the form also contains a nonce to
prevent XSRF, which changes with high frequency. The Account Manager
can't generate the nonce, and submitting the request to the server
without it will generate an error.

The Account Manager could obtain the nonce from the form in advance
for forwarding to the POST action that changes the password, but I
believe that is not the current behavior.

With the current behavior, in my understanding, the form must exist
without additional fields (or those fields must be optional). In that
case, such a form would be vulnerable to XSRF, since an external site
could cause a visitor who is already logged in to the target site to
post data to the open (and published via this spec!) changepassword
method URL. This is undesirable.

Is there some other system in place that would prevent this from
leaving the form vulnerable? Is the expectation that the lack of the
nonce would generate a confirmation via a web UI? Or have I
completely misunderstood how the Account Manager gathers and submits
data to the URL and its role in changing a password?

I admit to not having seen an interface provided in the Account
Manager for changing a password, nor have I seen any reaction to the
changepassword method from the Firefox plugin so far. Still, I'm
curious about how the plugin is expected to function.

Owen

[Edward Lee]

On Wed, Apr 28, 2010 at 1:44 PM, ringmaster <epi...@gmail.com> wrote:
> The Account Manager could obtain the nonce from the form in advance
> for forwarding to the POST action that changes the password

Would providing the nonce as part of the account status be enough for
this use case?
https://groups.google.com/group/mozilla-labs-online-identity/browse_thread/thread/cf4be006297eb8f6

Also, Account Manager isn't taking any existing web form and filling
out data then submitting. Notice that the connect/disconnect methods
are targets of a POST. So one can think of it as a separate login page
with only username and password submitting to an endpoint.

Ed

[Owen Winkler]

On 4/28/2010 4:51 PM, Edward Lee wrote:
> On Wed, Apr 28, 2010 at 1:44 PM, ringmaster<epi...@gmail.com> wrote:
>> The Account Manager could obtain the nonce from the form in advance
>> for forwarding to the POST action that changes the password
> Would providing the nonce as part of the account status be enough for
> this use case?
> https://groups.google.com/group/mozilla-labs-online-identity/browse_thread/thread/cf4be006297eb8f6

As long as the extension manages holding the token value and passes it
with the request as per the parameters on the method, this could work.
One additional concern - and I can't remember how the WordPress nonces
are generated, but it might affect WP, too - is that Habari's nonces are
time-sensitive. The Firefox extension would need to update the token
from the response header more frequently than the current refresh only
on restart of Firefox.

Is this implemented in the WordPress plugin/Firefox extension already?
I'm curious about the workaround I saw mentioned over Twitter earlier.

> Also, Account Manager isn't taking any existing web form and filling
> out data then submitting. Notice that the connect/disconnect methods
> are targets of a POST. So one can think of it as a separate login page
> with only username and password submitting to an endpoint.

Right, and the spec seems written such that you could use your existing
login forms with a couple of static files used to indicate those
endpoints, rather than come up with whole new endpoints just for this
purpose. I was simply wondering if reading an existing form first might
be a possibility or the expectation other than what I had read in the
spec, since there could potentially be other data required when
submitting to these forms.

Owen

[Mike Hanson]

We definitely have had prototypes that read a value from an existing form and used that as part of the POST. Technically, it is quite doable.

The tricky part is simply going to be defining how to read that value in a very standard way -- we can settle on something very simple and deterministic, like "read the element with this ID", or something far more general and potentially problematic, like "evaluate this XPath".

I'd be interested in your feedback on that point.

-Mike
--
Michael Hanson, Mozilla Labs (@michaelrhanson)

[Edward Lee]

On Wed, Apr 28, 2010 at 2:02 PM, Owen Winkler <epi...@gmail.com> wrote:
> need to update the token from the response header more frequently

Assuming the pages are using the X-Account-Management-Status header,
it would update on every page visit. The Account Manager uses that to
figure out if the user is logged in or not and what name to show.
However, for pages that use the sessionstatus method, the target page
providing the status would need to correctly set cache headers to have
Firefox refetch the status periodically.

However, why would the nonces need to be continuously cycled for a
given user if it hasn't been used yet? I suppose the nonces might be
used for non-password related actions such as even loading "the next
page," and each successful page load has its own nonce.

Ed

[Dan Mills]

Just implemented Ed's proposal, though I haven't tested it:

http://hg.mozilla.org/labs/weave-identity/rev/1ab1f098d9a6

Basically, if you add a "token" parameter to the status header, you can then use it for connect/disconnect by setting the "token" param in the AMCD to whatever you want.  For example:

AMCD:
{
  connect: {
    ...
    params: {
      ...
      token: "csrf"
    }
  }
}

And this gets returned with the page, or the status ping:

X-Account-Management-Status: none; token="fskdmk32irm2w3kv9a"

Then when the user clicks 'connect' we will send a POST with:

username=...&password=...&csrf=fskdmk32irm2w3kv9a

As the body.

Comments welcome, I'm not sure if this is a good solution.  It might lead to many more tokens being valid at any given time, specially for sites that return the header with every page (and a different token every time).

Dan

[Fred Wenzel]

Did you package up the extension with this change so we can try it
out? The method of providing a CSRF token in the X-AM-Status header
seems to work for Django (and I assume it'd also work for most other
frameworks that provide a CSRF mechanism), but version 0.0.12 of the
extension does not seem to respect it.

Also, after clicking "connect", the extension sends a POST request to
the login form, as expected resulting in a 403 response due to the
missing token (this response contains no X-AM-Status header). The page
reloads (also expected) but when I click on the key symbol again, the
"bubble" is empty. There might be a bug in the error handling code
there.

~F

On Apr 29, 2:54 am, Dan Mills <thun...@mozilla.com> wrote:
> Just implemented Ed's proposal, though I haven't tested it:
>
> http://hg.mozilla.org/labs/weave-identity/rev/1ab1f098d9a6
>
> Basically, if you add a "token" parameter to the status header, you can then
> use it for connect/disconnect by setting the "token" param in the AMCD to
> whatever you want.  For example:
>
> AMCD:
> {
>   connect: {
>     ...
>     params: {
>       ...
>       token: "csrf"
>     }
>   }
>
> }
>
> And this gets returned with the page, or the status ping:
>
> X-Account-Management-Status: none; token="fskdmk32irm2w3kv9a"
>
> Then when the user clicks 'connect' we will send a POST with:
>
> username=...&password=...&csrf=fskdmk32irm2w3kv9a
>
> As the body.
>
> Comments welcome, I'm not sure if this is a good solution.  It might lead to
> many more tokens being valid at any given time, specially for sites that
> return the header with every page (and a different token every time).
>
> Dan
>
> On Wed, Apr 28, 2010 at 2:12 PM, Edward Lee <edi...@mozilla.com> wrote:

[François de Metz]

Hi,

On 04/05/2010 17:30, Fred Wenzel wrote:
> Did you package up the extension with this change so we can try it
> out? The method of providing a CSRF token in the X-AM-Status header
> seems to work for Django (and I assume it'd also work for most other
> frameworks that provide a CSRF mechanism), but version 0.0.12 of the
> extension does not seem to respect it.

Yes it works. I use it. See
http://github.com/francois2metz/django-account-manager
(http://github.com/francois2metz/django-account-manager/commit/698d5c40ffb7b6b874215b73b461bc0dbcdf03ab)

> Also, after clicking "connect", the extension sends a POST request to
> the login form, as expected resulting in a 403 response due to the
> missing token (this response contains no X-AM-Status header). The page
> reloads (also expected) but when I click on the key symbol again, the
> "bubble" is empty. There might be a bug in the error handling code
> there.

works for me :)

[Fred Wenzel]

On May 4, 5:44 pm, François de Metz <franc...@2metz.fr> wrote:
> Yes it works. I use it. Seehttp://github.com/francois2metz/django-account-manager

> (http://github.com/francois2metz/django-account-manager/commit/698d5c4...)

Very nice, thank you. I am thinking the problem lies in the fact that
I do not send the header with every request, while you are. I am
trying to rely on the extension querying the session status URL.

Account Manger should remember the token from when it last queried /
sessionstatus or, if it doesn't know it anymore, it should re-query
that URL to obtain a CSRF token, then issue the POST request. That
doesn't seem to happen (unless it's my mistake, which is completely
possible :)).

> > Also, after clicking "connect", the extension sends a POST request to
> > the login form, as expected resulting in a 403 response due to the
> > missing token (this response contains no X-AM-Status header). The page
> > reloads (also expected) but when I click on the key symbol again, the
> > "bubble" is empty. There might be a bug in the error handling code
> > there.
>
> works for me :)

Again, when it does not send the token correctly, the extension seems
to kill itself. It seems, while handling the header on every request
works well, the fallback to querying /sessionstatus doesn't work quite
right yet.

~F

[Fred Wenzel]

After discussing this further with the AMO team, I don't think it's a
good solution to send the header on every request, and furthermore
providing the token as part of the header (even only on the
sessionstatus page) seems less than ideal as well, as it still
requires us to create CSRF tokens for all anonymous users, whether
they will eventually log in or not. In Django, this also results in a
cookie being set (as that's how their CSRF middleware works), and
therefore has a negative impact on caching.

While we don't do anything crazy with the login form on AMO, Dan
already mentioned some of the bigger players on the Web do all sorts
of obscure things with their login forms that cannot be reflected with
the current specification.

I suggest the following:
- keep the logged-out header simple: X-AM-Status: none
- when the user clicks on the login button, GET the connect URL,
execute an xpath expression to get to the *form* in question. Fill out
only the username and password, then POST it back to what its action
attribute specifies.

That'd allow CSRF cookies to be set beforehand (due to the GET
request) and allows the site to do whatever it pleases in its login
form.

Admittedly, there are ways here to break this as well (on sites that
do crazy things with JS on the login page, for example), but I think
that's a much rarer problem than having to serve CSRF tokens to every
anonymous user. Remember, if this becomes a default Firefox feature,
there would be prohibitively many (read: millions) of those tokens we
needed to serve, for the benefit of auto-login for a small minority.

[Dan Mills]

Haven't packaged that up yet, I can do that today so you can test.

As for the bug you found, I think the client is getting stuck in the "signing in" state, forgetting to clear that after the failure.  I'll look into it.

Dan