Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql

2012-06-15 Thread Ulf Wendel

Am 15.06.2012 03:01, schrieb Anthony Ferrara:

I raised this topic on list over a year ago (
http://marc.info/?l=php-internalsm=130417646507744w=2 ). It was
determined that it wasn't time yet to disable prepared statement
emulation for MySQL yet. However, Rasmus did mention that it was a
possibility for 5.4 (
http://marc.info/?l=php-internalsm=130418875017027w=2 ). Since that
ship has sailed, I submitted a pull request for trunk to change the
default value of prepared statement emulation for MySQL.

https://github.com/php/php-src/pull/108

https://bugs.php.net/bug.php?id=54638

Does this need to be an RFC (should I draft one)? Or can it just be
pulled as-is?


Please, be aware of the consequences of this move and don't break any 
tests.  Also, please, do no break it down to prepared statement === 
security as any such _short_ statement draws an incomplete picture and 
bares more risks than it does good.


Back then, Johannes already pointed out some consequences: 
http://marc.info/?l=php-internalsm=130423522001744w=2 . Here's another 
iteration on the topic. There are assorted others in my blog from the 
days PDO_MySQL has seen its last major update.



*Security*

It is claimed that native prepared statements are more secure because:

 - statement and parameter are send to the server independently
 - the server builds the final statement string

Whereas for a non-prepared statement:

 - the client builds the final statement string
 - the client has to escape parameter values
 - the client sends statement and parameter in one

As long as client-side escaping is done properly, there is no practical 
difference between the two approaches. There are, however, arguments 
that boil down to limiting the possiblity of user errors:


 - bind-style API
 - messing up on charsets

A bind-style API may be convenient and prevent forgetting to use 
escaping but does not prevent to shoot yourself doing something foolish 
such as:


   PDO::query(... . $_REQUEST['killme']);

When using non-prepared statements it must be ensured to use the correct 
charset for (client-side) escaping. As long as charset changes happen 
through API calls, the client library is always aware of the current 
charset and everything is fine. But, the charset can be changes through 
SQL as well:


  SET NAMES ...

The fact that PDO lacks API calls to set the charset, which enforces the 
use of SET NAMES, is not helpful. However, even with appropriate API 
calls users can always fool themselves using:


  PDO::query(SET NAMES)

If building the final statement is done on the server or SET NAMES is 
disallowed, fooling yourself becomes much harder.



*Security in the context of PDO*

Actually, the change of the default can give you a false feeling of 
improved security. This is due to the design of PDO and the existance 
of a prepared statement emulation in the core of PDO.


A statement like this will use the prepared statement emulation:

 SELECT something FROM table WHERE cond = :placeholder

MySQL does not support named bind parameter such as :placeholder. The 
PDO parser kicks in and does replace :placeholder with the bound 
value. Which then, is no different from:


 sprintf(SELECT something FROM table WHERE cond = '%s', escape($param))

Of course, escape() can't be forgotten. It happens inside PHP, inside 
PDO on the C level. Messing up with charsets is still possible. Thus, 
users must be educated as ever since. The message prepared statement 
=== security is too short to tell the full story.


BTW, there's the classic of:

  SELECT something FROM table LIMIT :placeholder

What will happen if forget to hint a data type for :placeholer? Search 
the bug database, if you can't answer. As you are at it, try the reverse 
with ? and another database system that does not support ? as a 
placeholder...



*Performance (and impact on server load)*

After switching to native prepared statements users may see an increase 
of MySQL - PHP round trips:


  prepare() client -- server
  execute() client -- server

Great, if statements get executed multiple times. Bad, if statements are 
not executed multiple times, such as:


  SELECT title, received FROM news WHERE  =
  SELECT name, price, special_price FROM specials

Those statements will take two round trips, thus become slower. Things 
become slower although there is no other win. Note, that none of the 
example statements has any parameters.


A statement such as:

   INSERT INTO test(col) VALUES (?)

May become faster if multiple rows are inserted. The statement string is 
transferred only once during prepare. Later only the parameter value is 
transferred. But, of course, you'd use MySQL multi-insert syntax or 
other tricks here anyway, wouldn't you?


Back in the 70's, when prepared statements have been introduced, the 
world was a different one. Prepared statements make the assumption that 
its benefitial to cache various things, such as query execution plans, 
inside the database server. 

Re: [PHP-DEV] Adding a simple API for secure password hashing?

2012-06-15 Thread Herman Radtke
This userland library already solves all the issues you outlined with
bcrypt: http://www.openwall.com/phpass/

The API is very easy to use, has been around for a while and has a
number of detailed articles that explain how to use it.

-- 
Herman Radtke
hermanrad...@gmail.com | http://hermanradtke.com

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql

2012-06-15 Thread Christopher Jones




Am 15.06.2012 03:01, schrieb Anthony Ferrara:

I raised this topic on list over a year ago (
http://marc.info/?l=php-internalsm=130417646507744w=2 ). It was
determined that it wasn't time yet to disable prepared statement
emulation for MySQL yet.



Does this need to be an RFC (should I draft one)? Or can it just be
pulled as-is?


It needs an RFC because it needs to document whether or not the other
DB drivers should also be changed.

On 06/15/2012 08:34 AM, Ulf Wendel wrote:

As long as client-side escaping is done properly, there is no
practical difference between the [client vs server -prepare]
approaches.


The big problem with this line of reasoning is that the client must
know exactly the same dialect of SQL/XQUERY/whatever that the server
does.  Since we can't predict the future, and so a new DB might
introduce some funky syntax, then client preparing could break visibly
or invisibly.  The DB client  DB server code architecture needs to
head to a model where server side prepares work (and are easy etc).

Chris

--
christopher.jo...@oracle.com
http://twitter.com/#!/ghrd

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql

2012-06-15 Thread Anthony Ferrara
Chris,

 Does this need to be an RFC (should I draft one)? Or can it just be
 pulled as-is?


 It needs an RFC because it needs to document whether or not the other
 DB drivers should also be changed.

It's a PDO driver-specific change. So to me it's fairly straight
forward to see that no other drivers need changing... It doesn't even
hit the mysql API level...

Not saying an RFC isn't needed, just that justification isn't clear to me...

 The big problem with this line of reasoning is that the client must
 know exactly the same dialect of SQL/XQUERY/whatever that the server
 does.  Since we can't predict the future, and so a new DB might
 introduce some funky syntax, then client preparing could break visibly
 or invisibly.  The DB client  DB server code architecture needs to
 head to a model where server side prepares work (and are easy etc).

We're talking about the MySQL specific API, not all APIs. We're not
talking about removing emulation support either. Just disabling it by
default for MySQL just like it is for almost every other driver
supported by PDO...

Anthony

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql

2012-06-15 Thread Christopher Jones



On 06/15/2012 09:36 AM, Anthony Ferrara wrote:

Chris,


Does this need to be an RFC (should I draft one)? Or can it just be
pulled as-is?



It needs an RFC because it needs to document whether or not the other
DB drivers should also be changed.


It's a PDO driver-specific change. So to me it's fairly straight
forward to see that no other drivers need changing... It doesn't even
hit the mysql API level...

Not saying an RFC isn't needed, just that justification isn't clear to me...


PDO is supposed to be a generic layer. An RFC can clarify the project
scope and examine what happens in other drivers.

Chris

--
christopher.jo...@oracle.com
http://twitter.com/#!/ghrd

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql

2012-06-15 Thread Ulf Wendel

Am 15.06.2012 18:36, schrieb Anthony Ferrara:

Chris,


Does this need to be an RFC (should I draft one)? Or can it just be
pulled as-is?



It needs an RFC because it needs to document whether or not the other
DB drivers should also be changed.


It's a PDO driver-specific change. So to me it's fairly straight
forward to see that no other drivers need changing... It doesn't even
hit the mysql API level...

Not saying an RFC isn't needed, just that justification isn't clear to me...


Go commit if you feel like the change is widely accepted, you are 
willing to explain the differences to users and no test breaks. After 
all its a tiny, little PDO driver default setting.


Ulf

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql

2012-06-15 Thread Anthony Ferrara
Ulf,
 It needs an RFC because it needs to document whether or not the other
 DB drivers should also be changed.


 It's a PDO driver-specific change. So to me it's fairly straight
 forward to see that no other drivers need changing... It doesn't even
 hit the mysql API level...

 Not saying an RFC isn't needed, just that justification isn't clear to
 me...


 Go commit if you feel like the change is widely accepted, you are willing to
 explain the differences to users and no test breaks. After all its a tiny,
 little PDO driver default setting.

I think you interpreted me wrong. I was just pointing out that this
change would be localized to the PDO_MySQL driver. And that
documenting whether other DB drivers should be changed is a moot
point, since they already aren't using emulation by default...

And where did the passive-agressive thing hit off? I posted it to list
as a question and a suggestion. I didn't say let's just do it, who
cares. I didn't say anything about any of the points that were
raised, I just wanted to clarify something that I thought was
miscommunicated. You brought up some very valid points. Points that
shouldn't be ignored. I just haven't addressed them because I haven't
wrapped my head around them fully yet...

So in short, can we please take it down a notch?

Anthony

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql

2012-06-15 Thread Ulf Wendel

Am 15.06.2012 19:31, schrieb Anthony Ferrara:

Ulf,

It needs an RFC because it needs to document whether or not the other
DB drivers should also be changed.



It's a PDO driver-specific change. So to me it's fairly straight
forward to see that no other drivers need changing... It doesn't even
hit the mysql API level...

Not saying an RFC isn't needed, just that justification isn't clear to
me...



Go commit if you feel like the change is widely accepted, you are willing to
explain the differences to users and no test breaks. After all its a tiny,
little PDO driver default setting.


I think you interpreted me wrong. I was just pointing out that this
change would be localized to the PDO_MySQL driver. And that
documenting whether other DB drivers should be changed is a moot
point, since they already aren't using emulation by default...

And where did the passive-agressive thing hit off? I posted it to list


There's a bug/feature request. Not just one, many came up over the 
years. You wrote a patch. You proposed a change. Nobody rejected the 
change - for a year. Its a tiny change of a driver default setting. It 
does not require an RFC.


One of the maintainers, me, didn't bring up any objections. I said what 
any maintainer has to say:


 - educate users, be aware of the consequences
 - make sure not to break any tests

Another maintainer, Johannes, didn't say no a year ago. Nobody says 
no, !no ~ go - its your call. Stand in for your proposal, take care of 
the tests and commit.


Ulf







--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql

2012-06-15 Thread Anthony Ferrara
Ulf,

My apologies. I mis-interpreted that reply.

I would commit, but I do not have commit access (no karma at all). I'm
going away for a few days, but when I get back I will send a separate
request to the list for karma.

Thanks,

Anthony

On Fri, Jun 15, 2012 at 1:55 PM, Ulf Wendel ulf.wen...@oracle.com wrote:
 Am 15.06.2012 19:31, schrieb Anthony Ferrara:

 Ulf,

 It needs an RFC because it needs to document whether or not the other
 DB drivers should also be changed.



 It's a PDO driver-specific change. So to me it's fairly straight
 forward to see that no other drivers need changing... It doesn't even
 hit the mysql API level...

 Not saying an RFC isn't needed, just that justification isn't clear to
 me...



 Go commit if you feel like the change is widely accepted, you are willing
 to
 explain the differences to users and no test breaks. After all its a
 tiny,
 little PDO driver default setting.


 I think you interpreted me wrong. I was just pointing out that this
 change would be localized to the PDO_MySQL driver. And that
 documenting whether other DB drivers should be changed is a moot
 point, since they already aren't using emulation by default...

 And where did the passive-agressive thing hit off? I posted it to list


 There's a bug/feature request. Not just one, many came up over the years.
 You wrote a patch. You proposed a change. Nobody rejected the change - for a
 year. Its a tiny change of a driver default setting. It does not require an
 RFC.

 One of the maintainers, me, didn't bring up any objections. I said what any
 maintainer has to say:

  - educate users, be aware of the consequences
  - make sure not to break any tests

 Another maintainer, Johannes, didn't say no a year ago. Nobody says no,
 !no ~ go - its your call. Stand in for your proposal, take care of the tests
 and commit.


 Ulf







 --
 PHP Internals - PHP Runtime Development Mailing List
 To unsubscribe, visit: http://www.php.net/unsub.php


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql

2012-06-15 Thread Ulf Wendel

No worries, Anthony!

Am 15.06.2012 21:15, schrieb Anthony Ferrara:

I would commit, but I do not have commit access (no karma at all). I'm
going away for a few days, but when I get back I will send a separate
request to the list for karma.


I see.  It is not up to me to grant karma. I am not that active in the 
PHP project and wouldn't even want to have the power to give you karma. 
Those that have the power, shall decide.


The key for me is:

 ... you stand in for the change
 ... you are aware of the technical aspects
 ... you are willing to help
 ... you waited for a long time and drove discussion - you must have a 
reason...


Who does the commit is of little interest to me. You, I or Johannes can 
do it the other day. You can expect us to support the new default and 
not fall in your back but regarding the change itself, I'm no more than 
+/- 0. You'll have to continue to stand in if a discussion comes up.


Deal?

n8  enjoy your weekend,
Ulf








--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php