Quantcast

Re: CSRF implementation take 1 (now take 2)

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: CSRF implementation take 1 (now take 2)

John P. Rouillard
Hi all:

Here is the updated patch:

   http://www.cs.umb.edu/~rouilj/roundup_patches/csrf_patch2.patch

The first round patch can be gotten from

   http://www.cs.umb.edu/~rouilj/roundup_patches

Updates:

  a basic test for csrf and some fixes for existing tests that csrf broke.

  a workaround for csrf token failure when user is changing identity
    (i.e. logs out and tries to log back in again immediately without
     refreshing screen)

  configuration settings to control the csrf header and nonce tests

  starter docs in upgrading and CHANGES.txt

  checking HOST http header as well to defend against some dns attacks

  the ability to require one or more successful header checks before
       accepting the post.

Still open consolidating all the csrf token generation code into one
function.

Comments on the configuration settings and doc welcome.

--
                                -- rouilj
John Rouillard
===========================================================================
My employers don't acknowledge my existence much less my opinions.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Roundup-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/roundup-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: CSRF implementation take 1 (now take 2)

John P. Rouillard
Hi Anthony:

In message <[hidden email]>,
Anthony Pankov writes:

>There is a some case that is not clear for me.
>
>If I have a tracker with no anonymous access and a people who click
>link in emails will go to "Access forbidden" page if they not logged
>in already.  To be more user friendly I want to add redirection to
>login page with "remembering" the target URL. After successful login
>they will go to page they initially want.
>
>I understand that such "delayed URL" must be applied only to view
>action.  But the question is: does the CSRF implementation make
>basically impossible "delayed URL" feature?

I did a test on a csrf enabled tracker I am running. For me the
anonymous user has no access to any user page.

So my first connection is to:

   https://tracker.example.com/tracker/user1

I see a screen with the left hand menu (including a login form) and
the message in the main area "Please login with your username and
password.".

The CSRF token in the login form is associated with the None session
and the uid of 2 (anonymous). So it will be accepted when submitted.

Now also part of that form is the hidden field:

<input type="hidden" name="__came_from"
    value="https://tracker.example.com/tracker/user1">

I then enter my username/password and click on the login button in the
login form.

If the login is successful, roundup will redirect to the url in the
__came_from field.

I think this implements what you want. Note the __came_from field
handling has changed in roundup 1.6. See upgrading.txt for 1.6 for the
full info, but an excerpt reads:

    Login from a search or after logout works better
    ------------------------------------------------

    The login form has been improved to work with some back end code
    changes. Now when a user logs in they stay on the same page where they
    started the login. To make this work, you must change the tal that is
    used to set the ``__came_from`` form variable. Note that the url
    assigned to __came_from must be url encoded/quoted and be under the
    tracker's base url. If the base_url uses http, you can set the url to
    https.

    Replace the existing code in the tracker's html/page.html page that
    looks similar to (look for name="__came_from")::


There are some other instructions on changing the login form to make
things work but you get the idea.

There is one thing that may be an issue. If a user is logged in and
then logs out, I had to add a redirect at the end of the logout
procedure.  If I didn't the csrf token was still associated with the
user who logged out and not with the anonymous user. As a result the
login failed due to a bogus csrf token.  (This may be a bug because
the generation of the new csrf token should occur after logout has
reset the session token and the user.)

Adding the redirect to the home page as the last step of the logout
cleans things up and lets the user log in again with new
credentials. However it means the user will be on the home page when
they log in and not on the page where they logged out.

I don't think this is an issue in this case as the user explicitly
logged out and probably doesn't want to leave an indication of the
page they were looking at before they logged out.

On the good news side I think I finished coverage tests for the csrf
stuff last night. So if things look good tonight I can push the
changes this weekend.

--
                                -- rouilj
John Rouillard
===========================================================================
My employers don't acknowledge my existence much less my opinions.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Roundup-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/roundup-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: CSRF implementation take 1 (now take 2)

Anthony Pankov

Thank you very much for the explanation.


> Hi Anthony:

> In message <[hidden email]>,
> Anthony Pankov writes:
>>There is a some case that is not clear for me.
>>
>>If I have a tracker with no anonymous access and a people who click
>>link in emails will go to "Access forbidden" page if they not logged
>>in already.  To be more user friendly I want to add redirection to
>>login page with "remembering" the target URL. After successful login
>>they will go to page they initially want.
>>
>>I understand that such "delayed URL" must be applied only to view
>>action.  But the question is: does the CSRF implementation make
>>basically impossible "delayed URL" feature?

> I did a test on a csrf enabled tracker I am running. For me the
> anonymous user has no access to any user page.

> So my first connection is to:

>    https://tracker.example.com/tracker/user1

> I see a screen with the left hand menu (including a login form) and
> the message in the main area "Please login with your username and
> password.".

> The CSRF token in the login form is associated with the None session
> and the uid of 2 (anonymous). So it will be accepted when submitted.

> Now also part of that form is the hidden field:

> <input type="hidden" name="__came_from"
>     value="https://tracker.example.com/tracker/user1">

> I then enter my username/password and click on the login button in the
> login form.

> If the login is successful, roundup will redirect to the url in the
> __came_from field.

> I think this implements what you want. Note the __came_from field
> handling has changed in roundup 1.6. See upgrading.txt for 1.6 for the
> full info, but an excerpt reads:

>     Login from a search or after logout works better
>     ------------------------------------------------

>     The login form has been improved to work with some back end code
>     changes. Now when a user logs in they stay on the same page where they
>     started the login. To make this work, you must change the tal that is
>     used to set the ``__came_from`` form variable. Note that the url
>     assigned to __came_from must be url encoded/quoted and be under the
>     tracker's base url. If the base_url uses http, you can set the url to
>     https.

>     Replace the existing code in the tracker's html/page.html page that
>     looks similar to (look for name="__came_from")::


> There are some other instructions on changing the login form to make
> things work but you get the idea.

> There is one thing that may be an issue. If a user is logged in and
> then logs out, I had to add a redirect at the end of the logout
> procedure.  If I didn't the csrf token was still associated with the
> user who logged out and not with the anonymous user. As a result the
> login failed due to a bogus csrf token.  (This may be a bug because
> the generation of the new csrf token should occur after logout has
> reset the session token and the user.)

> Adding the redirect to the home page as the last step of the logout
> cleans things up and lets the user log in again with new
> credentials. However it means the user will be on the home page when
> they log in and not on the page where they logged out.

> I don't think this is an issue in this case as the user explicitly
> logged out and probably doesn't want to leave an indication of the
> page they were looking at before they logged out.

> On the good news side I think I finished coverage tests for the csrf
> stuff last night. So if things look good tonight I can push the
> changes this weekend.

> --
>                                 -- rouilj
> John Rouillard
> ===========================================================================
> My employers don't acknowledge my existence much less my opinions.



--
Best regards,
 Anthony                          mailto:[hidden email]


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Roundup-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/roundup-devel
Loading...