Enhancement Requests: Configurable duration and reusable tokens #68

JimmyPruitt · 2017-03-13T20:09:00Z

I have two requests for enhancements that would suit my needs quite nicely.

Could you possibly make the duration of the cookie configurable? My site has a setting that gives some elevated users the option to change the duration of their session, so it would be nice to have the duration of the CSRFP cookie match the duration of the user session.
Could you make it possible to reuse the same token for the entire duration of the session? Everything I've read on the subject says that using one token per session is adequate, and having tokens that change every request has caused me a lot of headache. I can elaborate if you need, but I've essentially encountered a race condition where the expected token in the SESSION array doesn't match the token in the cookie, even when it should, and having one token per session would fix the problem.

The text was updated successfully, but these errors were encountered:

mebjas · 2017-03-16T21:54:28Z

@jimmy-p123 answers:

it's 30mins by default. You can change it here: https://github.com/mebjas/CSRF-Protector-PHP/blob/master/libs/csrf/csrfprotector.php#L32 in your code. If you want to make it configurable, you can alter config file, and load it in constructor. Feel free to send a PR.
If this is happening then it's a bug. A lot of effort has been put to make it per request token based model to make it safe to MITM attacks. I'd like to know when it's failing. Maybe some way to reproduce.

Now if you want to make it per session token: you need to alter these methods

JimmyPruitt · 2017-03-16T22:14:55Z

I made a post on stackoverflow that explains the problem, but I didn't explain it as well as I could have. I thought there would be a way around it that didn't involve modifying your library, but nobody had any ideas.

Since you obviously understand your own library, let me just pose a simple question to you, to see if you can understand where the race condition occurs. Assume that session_write_close is called immediately after I initialize the CSRFP in my own .php file.

Two requests come into the server from the same session at or near the same time. Request 1 is received first but takes longer to respond than request 2 because it's more computationally intensive. From which request would you expect the values A) in the cookie client side and B) in the SESSION array server side to be?

I'm finding that A) the cookie is the value set by the first request, but B) the SESSION array's value is from the second request.

JimmyPruitt · 2017-03-20T23:16:26Z

Here's a more concrete example:

<?php
    include_once 'csrf-protector/libs/csrf/csrfprotector.php';
    csrfProtector::init();
    session_write_close();

    if (!$_POST['request'])
    {
        echo
            '<html>
                <head>
                <script src="https://code.jquery.com/jquery-1.12.4.min.js" integrity="sha256-ZosEbRLbNQzLpnKIkEdrPv7lOy9C27hHQ+Xp8a4MxAQ=" crossorigin="anonymous"></script>
                </head>
                <body>
                    <script>
                        $(document).ready(function() {
                            $.post("index.php", "request=1", () => $.post("index.php", "request=4"));
                            $.post("index.php", "request=2", () => $.post("index.php", "request=3"));
                        });
                    </script>
                </body>
            </html>';

    }

    if ($_POST['request'] == 1)
        sleep(2);

Request #4 will be blocked.

mebjas · 2017-03-29T19:21:51Z

@jimmy-p123 : for the cookie duration (1st part of it), could you resend a PR?

As for other, amazing example. I get the issue: if there is a causality inconsistency between the server and client the model fails. Will try to find a solution for this, in existing model.

mebjas · 2017-03-29T20:38:36Z

@jimmy-p123 it is an interesting problem. Can you try replacing libs/csrf/csrfprotector.php with this https://gist.github.com/mebjas/df33433ec5584341d2f8b22d6fc67ccb and see if it works well for your case.

JimmyPruitt · 2017-03-31T19:07:49Z

It appears to be fixed using variations of the example I provided. I'll apply it to where I initially found the problem and get back to you if I find any problems there.

mebjas · 2017-04-12T08:19:33Z

@jimmy-p123 status?

JimmyPruitt · 2017-04-12T16:38:58Z

I found one instance where we're sending three requests at the same time, and then a fourth request that starts shortly after the first three start, but finishes before any of the others finish, gets blocked. I'm still working on how to simplify the problem, as it seems to be slightly more complicated than that.

I'll try to get that pull request sent over today as well.

JimmyPruitt · 2017-04-13T16:31:15Z

I'm still getting blocks on requests number 7 and 8 in the following example, but only sometimes. I have to refresh the page a few times to get it to happen:

<?php
    include_once 'csrf-protector/libs/csrf/csrfprotector.php';
    csrfProtector::init();
    session_write_close();

    if (!$_POST['request'])
    {
        echo
            '<html>
                <head>
                    <script src="https://code.jquery.com/jquery-1.12.4.min.js" integrity="sha256-ZosEbRLbNQzLpnKIkEdrPv7lOy9C27hHQ+Xp8a4MxAQ=" crossorigin="anonymous"></script>
                </head>
                <body>
                    <script>
                        $(document).ready(() =>
                        {
                            setTimeout(() => $.post("index.php", "request=4", () => $.post("index.php", "request=5")));
                            $.post("index.php", "request=1", () => $.post("index.php", "request=6"));
                            $.post("index.php", "request=2", () => $.post("index.php", "request=7"));
                            $.post("index.php", "request=3", () => $.post("index.php", "request=8"));
                        });
                    </script>
                </body>
            </html>';
    }

    if ($_POST['request'] == 1 || $_POST['request'] == 2 || $_POST['request'] == 3)
        sleep(1);

mebjas · 2017-10-05T20:54:28Z

#78 closed, will send a new PR with corresponding changes.
References mentioned in the PR comment.

mebjas added the enhancement request label Mar 16, 2017

JimmyPruitt mentioned this issue Mar 17, 2017

Make cookie duration configurable and add option to reuse valid tokens #70

Closed

mebjas added the bug label Mar 29, 2017

mebjas self-assigned this Mar 29, 2017

mebjas added this to the v0.3.0 milestone Mar 29, 2017

mebjas removed this from the v0.3.0 milestone Sep 17, 2017

mebjas modified the milestone: v1.0.0 Oct 5, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhancement Requests: Configurable duration and reusable tokens #68

Enhancement Requests: Configurable duration and reusable tokens #68

JimmyPruitt commented Mar 13, 2017

mebjas commented Mar 16, 2017

JimmyPruitt commented Mar 16, 2017

JimmyPruitt commented Mar 20, 2017 •

edited

Loading

mebjas commented Mar 29, 2017

mebjas commented Mar 29, 2017

JimmyPruitt commented Mar 31, 2017

mebjas commented Apr 12, 2017

JimmyPruitt commented Apr 12, 2017

JimmyPruitt commented Apr 13, 2017 •

edited

Loading

mebjas commented Oct 5, 2017

Enhancement Requests: Configurable duration and reusable tokens #68

Enhancement Requests: Configurable duration and reusable tokens #68

Comments

JimmyPruitt commented Mar 13, 2017

mebjas commented Mar 16, 2017

JimmyPruitt commented Mar 16, 2017

JimmyPruitt commented Mar 20, 2017 • edited Loading

mebjas commented Mar 29, 2017

mebjas commented Mar 29, 2017

JimmyPruitt commented Mar 31, 2017

mebjas commented Apr 12, 2017

JimmyPruitt commented Apr 12, 2017

JimmyPruitt commented Apr 13, 2017 • edited Loading

mebjas commented Oct 5, 2017

JimmyPruitt commented Mar 20, 2017 •

edited

Loading

JimmyPruitt commented Apr 13, 2017 •

edited

Loading