Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
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?
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
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
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
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
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
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
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
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
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