Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:93235
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.53 as permitted sender)
To: internals@lists.php.net
References: <CAGa2bXZytbD1H-Rg5eWfiPwEM0p47JSQH-GC-ZayDSLbmbsGVA@mail.gmail.com>
 <c21d1d4b-a790-e1a9-7c93-2c5af4baf5d1@gmail.com>
 <CAGa2bXboop3KDFnYtcdO-K+hL0To5te_yeHeFEFO-kCVuzNOJQ@mail.gmail.com>
 <3b115b37-d399-0b69-24b4-de5c95c4a069@gmail.com>
 <CAGa2bXZwRK0GuXbT53B4w+h5ySiVJnJpo-d87Uiw-reYHi80zQ@mail.gmail.com>
 <CANUQDChxOeydBRLPrvuumf+edqJNKV+TOstGz66X_tg+Y7knPQ@mail.gmail.com>
 <CAGa2bXZC8Qp8-Ggyfj5+0X5aiktSX_AyfRdKAPXFZ=x4+A1XDQ@mail.gmail.com>
 <CANUQDCgi-P4=rWA5H+53_BcWyqeWaC_QOADUNMb_jV70KuhEPw@mail.gmail.com>
 <CAL0xaBFTF28h7ZTCkk=gn2=kepv0xe_Gk0+zjhmZzDmLJYU3-w@mail.gmail.com>
 <CAGa2bXbX_YhpJEr+9XjTb3xxgb2KTw=LEjC97A0vM4_WNiSvnA@mail.gmail.com>
 <CAGa2bXZrqvzNSQY=f3xM-czo4bAFZSF4cN0xLpBo6V0K91bivQ@mail.gmail.com>
Message-ID: <ae235ace-7d9c-8b75-ed5a-10f1f264dfd5@gmail.com>
Date: Wed, 11 May 2016 13:29:12 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101
 Thunderbird/45.0
MIME-Version: 1.0
In-Reply-To: <CAGa2bXZrqvzNSQY=f3xM-czo4bAFZSF4cN0xLpBo6V0K91bivQ@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [RFC DRAFT] Automatic CSRF Protection
From: rowan.collins@gmail.com (Rowan Collins)

On 11/05/2016 09:13, Yasuo Ohgaki wrote:
> Why
>
> <?php
> session_start(['csrf_rewrite'=>SESSION_CSRF_POST,
> 'csrf_validate'=>SESSION_CSRF_POST]);
> ?>
>
> and protect CSRF attacks against POST requests are bad for PHP and its users?

1) Because it gives users a false sense of security. If you say "turn 
this on and don't think about CSRF", and then somebody is caught by a 
CSRF vulnerability because the HTML rewriter wasn't smart enough to 
inject the parameter in the right place, they will feel you misled them. 
If you can't say that, then it's not really as simple as you implied.

2) Because it treats all users as malicious actors, rather than fallible 
ones. If you take too long writing a comment, you shouldn't get a 500 
error when you submit it, you should get a friendly message and a 
pre-filled form. That requires integration with the application 
framework, not with the session manager.

3) Because it complicates the session code with extra responsibilities, 
rather than concentrating on safely persisting arbitrary state data.

4) Because it adds yet another global setting that would be essential to 
be turned on for some applications, off for others.

The current proposal has effectively 4 features:

- Automatic generation and storage of a CSRF token on session start; a 
fairly trivial task to implement in plain PHP.

- Optional automatic rewriting of HTML to include the token. This is the 
only part that is actually hard to do without the core's involvement, 
but see point 1 above.

- A method to validate the CSRF token. Again, this is fairly simple to 
implement in PHP, and a quick glance at Packagist shows there are plenty 
of off-the-shelf implementations with various features.

- Optional automatic enforcement of the CSRF check. This is also fairly 
easy to implement in plain PHP, and since the user is likely to need to 
implement user code to make use of it anyway (e.g. set_error_handler is 
mentioned in the current RFC), it may actually be easier to perform the 
check manually.


Thinking about it, the only part that has some value being in core is 
the HTML rewriting. Perhaps what is actually needed is a lower-level 
function that PHP libraries can use to hook into this with whatever 
parameters they want, e.g.

register_html_rewrite_callback(
     function() { return [ 'csrf_token' => MyFramework\CSRF::getToken() 
]; },
     REWRITE_POST_FORMS | REWRITE_URL
);


Regards,
-- 
Rowan Collins
[IMSoP]