Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:93206
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.192.41 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <CAEZPtU7TeMxMnxBVA4BKdrsHba=4mxQCz_8VvEM528jadNyzVg@mail.gmail.com>
References: <CAGa2bXZytbD1H-Rg5eWfiPwEM0p47JSQH-GC-ZayDSLbmbsGVA@mail.gmail.com>
 <CAEZPtU7Wv1EQ_GsFSNwmzJnaACD91Dej_pWS_TUvjnj-wCRB5g@mail.gmail.com>
 <CAGa2bXZNtokRG8e73XCbhFsaqXZEfTM64zEdWGc25zYwYWTbWg@mail.gmail.com>
 <CAEZPtU7Lr+OA0Zn+qCc6h2ftpnRDE4t7YH=mUymXJu7XdR=79w@mail.gmail.com>
 <CAEZPtU6g-U5fbzcy=KD_OGH2BEukYnMPqrpDisTN5wbGkkbB2w@mail.gmail.com> <CAEZPtU7TeMxMnxBVA4BKdrsHba=4mxQCz_8VvEM528jadNyzVg@mail.gmail.com>
Date: Wed, 11 May 2016 15:54:07 +0900
Message-ID: <CAGa2bXZ3k7banuhgH=VNiSuowmdfenMn5bY1jdQKiyYEq2LhHw@mail.gmail.com>
To: Pierre Joye <pierre.php@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [RFC DRAFT] Automatic CSRF Protection
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

Hi Pierre,

On Wed, May 11, 2016 at 2:19 PM, Pierre Joye <pierre.php@gmail.com> wrote:
> On May 11, 2016 11:46 AM, "Yasuo Ohgaki" <yohgaki@ohgaki.net> wrote:
>
>> Thank you for your comments. I've updated the RFC. You might like this
>> version.
>>
>
> I still think we should not have that in core. If we do, it should be
> controlled by the application implementation and not ini settings (some
> routes may have it, other not, some route may have different ttl etc). I am
> not even sure it should be part of the session module.
>
> Sessions are per definiton easy. Implement them correctly (whatever that
> means) is hard. Adding csrf to ext/session feels like adding auth methods as
> well. Both csrf ans auth may need sessions but they are not part of the
> session features.

Session, authenticity, authentication cannot divide into independent
parts, but they are dependent by its nature. Whether we like it or
not, session is a part of authenticity/authentication. (I'm not going
to add authentication feature. Session's task is to store
authentication "flag", but I think authenticity is part of session)
Current session module provide authenticity partially and it could
provide good authenticity check with relatively simple change. This is
what I would like to change by this RFC.

I fully agree that implementations of proper session, authenticity,
authentication are hard. They are not simple tasks and could be
implemented badly like many applications in the wild. If these tasks
are easy and almost all developers/applications do them correctly, I
would not bother. Unfortunately, it's not. PHP has to be provided
basic utility to web developers for the task. IMO.

PHP is made for easy web application development. This RFC could
provide easy, yet good enough, CSRF protection when only POST is used.
It's just a matter of

<?php
session_start([
  'csrf_rewrite'=>SESSION_CSRF_POST,
  'csrf_validate'=>SESSION_CSRF_POST]);
// Do something useful w/o caring CSRF if data modification is
// done by POST only.
?>

We are in micro service era. Full featured frameworks should not be
required for simple request/response applications. It should be
something like

<?php
require 'mylib.php';

session_start([
  'csrf_rewrite'=>SESSION_CSRF_POST,
  'csrf_validate'=>SESSION_CSRF_POST]);

// Do something useful w/o caring CSRF if data modification is
// done by POST only.
check_authentication_and_authorization_if_not_redirect_to_appropriate_page_and_exit();
output_json_response(do_some_usuful_data_update_by($_POST));
?>

This is simple, fast and maintainable. There are many simple data
retrieval/modification APIs now. This is what I would like to achieve.

What is good about this RFC is applications that use only POST for
data modifications could be protected w/o single line of code
modification. And most importantly, it's easy to implement this :)
So I don't see reasons let it developers burden.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net