CSRF-Protector-PHP icon indicating copy to clipboard operation
CSRF-Protector-PHP copied to clipboard

Enhancement Requests: Configurable duration and reusable tokens

Open JimmyPruitt opened this issue 7 years ago • 10 comments

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

  1. 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.

  2. 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.

JimmyPruitt avatar Mar 13 '17 20:03 JimmyPruitt

@jimmy-p123 answers:

  1. 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.

  2. 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

  1. https://github.com/mebjas/CSRF-Protector-PHP/blob/master/libs/csrf/csrfprotector.php#L228
  2. https://github.com/mebjas/CSRF-Protector-PHP/blob/master/libs/csrf/csrfprotector.php#L316

mebjas avatar Mar 16 '17 21:03 mebjas

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 avatar Mar 16 '17 22:03 JimmyPruitt

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.

JimmyPruitt avatar Mar 20 '17 23:03 JimmyPruitt

@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 avatar Mar 29 '17 19:03 mebjas

@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.

mebjas avatar Mar 29 '17 20:03 mebjas

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.

JimmyPruitt avatar Mar 31 '17 19:03 JimmyPruitt

@jimmy-p123 status?

mebjas avatar Apr 12 '17 08:04 mebjas

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 avatar Apr 12 '17 16:04 JimmyPruitt

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);

JimmyPruitt avatar Apr 13 '17 16:04 JimmyPruitt

https://github.com/mebjas/CSRF-Protector-PHP/pull/78 closed, will send a new PR with corresponding changes. References mentioned in the PR comment.

mebjas avatar Oct 05 '17 20:10 mebjas