# [RFC][VOTE] Add validation functions to filter module - Thread: https://externals.io/message/95159 - Original archive: https://news-web.php.net/php.internals/95159 - Started by Yasuo Ohgaki on 2016-08-15T00:15:40+00:00 - Messages: 75 - Page 1 of 3: messages 1 to 25 (25 per page, add `?page=N` to the URL) - Next page: https://externals.io/message/95159.md?page=2 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #95159 — Yasuo Ohgaki — 2016-08-15T00:15:40+00:00 URL: https://externals.io/message/95159#95159 Hi all, This RFC is to add functions that are suitable for input validations for secure coding. IMHO, these additions are mandatory for PHP. https://wiki.php.net/rfc/add_validate_functions_to_filter Vote ends 2016/08/22 23:59:59 UTC I don't mind suspend vote and continue discussion if there is issue. It's rather long RFC. Thank you for reading and voting! Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #95160 — Yasuo Ohgaki — 2016-08-15T00:20:33+00:00 URL: https://externals.io/message/95159#95160 · In reply to: #95159 > This RFC is to add functions that are suitable for input validations > for secure coding. IMHO, these additions are mandatory for PHP. > > https://wiki.php.net/rfc/add_validate_functions_to_filter > Vote ends 2016/08/22 23:59:59 UTC > > I don't mind suspend vote and continue discussion if there is issue. > It's rather long RFC. Thank you for reading and voting! Note for voting. There are 2 votes for RFC acceptance and target PHP version. https://wiki.php.net/rfc/add_validate_functions_to_filter#proposed_voting_choices You have to vote twice. i.e. It cannot store 2 votes results at once. I do make this mistake. Please be careful! Thank you for voting! -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #95162 — Yasuo Ohgaki — 2016-08-15T00:53:37+00:00 URL: https://externals.io/message/95159#95162 · In reply to: #95160 Hi all, > > > […] > > Note for voting. > There are 2 votes for RFC acceptance and target PHP version. > > https://wiki.php.net/rfc/add_validate_functions_to_filter#proposed_voting_choices > > You have to vote twice. i.e. It cannot store 2 votes results at once. > I do make this mistake. Please be careful! > > Thank you for voting! One more usual request. Please describe reason(s) why you object proposal. I would like to improve proposal even when it is declined. In addition, I see votes based on misconception/misunderstanding on occasions. Thank you! P.S. I've missed last PR update in the RFC. RFC is fixed. Exception can be disabled by a option now. -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #95164 — Dan Ackroyd — 2016-08-15T01:25:15+00:00 URL: https://externals.io/message/95159#95164 · In reply to: #95162 Hi Yasuo, > One more usual request. > Please describe reason(s) why you object proposal. I'm not entirely sure why you ask for reasons when people vote no. The reasons are almost always the same as the reasons given before the voting starts. But for posterity: i) Validation error messages need to specify what is wrong.....which is bespoke to the application. Which is a reason why validation code belongs in userland. ii) Validation error message need to be in the correct language for an application. It is not a good approach for people to be trying to match strings emitted by internal code and trying to convert them to the correct language. iii) The argument that it needs to be fast could be applied to anything and everything, and so is bogus. The RFC doesn't even show that userland implementations are slow enought to be a concern. iv) The RFC makes an assumption that programs should exit when validation fails. "Input data validation should accept only valid and possible inputs. If not, reject it and terminate program." and the code example: > catch (FilterValidateException $e) { > var_dump($e->getMessage()); > die('Invalid input detected!'); // Should terminate execution when input validation fails > } This assumption is bogus. Any program that accepts data from users should provide useful error messages when the data is wrong with someting as simple as a string being too long. v) I don't like the current filter functions, and recommend people avoid using them. Adding to them with an even harder to use API is the wrong way to go. cheers Dan For the record - these are what my input validation functions look like. They are bespoke to the application, and provide useful error messages to the end user when an exception handler catches that specific exception to a 4xx HTTP response. function validateOrderAmount($value) : int { $count = preg_match("/[^0-9]*/", $value); if ($count) { throw new InvalidOrderAmount("Der Wert muss nur Ziffern enthalten."); } $value = intval($value); if ($value < 1) { throw new InvalidOrderAmount("Der Wert muss eine oder mehrere sein ."); } if ($value >= MAX_ORDER_AMOUNT) { throw new InvalidOrderAmount("Sie können nur ".MAX_ORDER_AMOUNT." auf einmal bestellen "); } return $value; } --- ## #95165 — Yasuo Ohgaki — 2016-08-15T02:15:28+00:00 URL: https://externals.io/message/95159#95165 · In reply to: #95164 Hi Dan, Thank you for sharing idea! > > > > > […] > > > I'm not entirely sure why you ask for reasons when people vote no. The > reasons are almost always the same as the reasons given before the > voting starts. Without feedback, there is no clue which way should go or improve. I didn't realize some of your idea and I think your feedback is great. > > But for posterity: > > i) Validation error messages need to specify what is wrong.....which > is bespoke to the application. Which is a reason why validation code > belongs in userland. When exception is enabled, offensive key name is written in exception message. When exception is disabled, your statement is true. This could be improved. Good feedback. > > ii) Validation error message need to be in the correct language for an > application. It is not a good approach for people to be trying to > match strings emitted by internal code and trying to convert them to > the correct language. It seems there is misunderstanding. These new functions are intended for "secure coding input validation" that should never fail. It means something unexpected in input data that cannot/shouldn't keep program running. Why do you need to parse message? All needed info, filter name, key and value, is in exception message and exception object, BTW. This one is good feedback, too. I appreciate better error message suggestions. > > iii) The argument that it needs to be fast could be applied to > anything and everything, and so is bogus. The RFC doesn't even show > that userland implementations are slow enought to be a concern. I thought I don't have to have example of userland implementation, so it's good feedback also. Typical OO implementation uses number of setters to define validation rules. In addition, it validates validation rule is OK for it. e.g. It will check input data type at least. Setters and validation rule validation makes execution slower obviously. One may optimize validation rules to plain array (like I do). In this case, performance is could be better than previously mentioned validators do all in the production environment. I also thought the performance issue is not much important because there is no PHP feature to compare. All of us knew PHP function call overheads are relatively large and proposed almost all in C implementation would be faster than userland. > > iv) The RFC makes an assumption that programs should exit when validation fails. > > "Input data validation should accept only valid and possible inputs. > If not, reject it and terminate program." > > and the code example: > > > […] > > This assumption is bogus. > > Any program that accepts data from users should provide useful error > messages when the data is wrong with someting as simple as a string > being too long. There is misunderstanding on this. As I wrote explicitly in the RFC, input validation and user input mistakes must be handled differently. "The input validation (or think it as assertion or requirement) error" that this RFC is dealing, is should never happen conditions (or think it as contract should never fail). The point of having the input validation is accept only inputs that program expects and can work correctly. Accepting unexpected data that program cannot work correctly is pointless. Don't misunderstood me. I'm not saying "You should reject user input mistakes". "User input mistakes" and "input validation error" is totally different error. > > v) I don't like the current filter functions, and recommend people > avoid using them. Adding to them with an even harder to use API is the > wrong way to go. I didn't recommend it either because it could not be used for input validation easily, escaping or sanitization could be done for dedicated API. Having new module is one of my idea also. However, I realized many of filter module codes could be reused after investigation. That's the reason why I added to filter module. I also named new functions to have "validate_*" prefix, rather than "filter_*" to emphasis it's for validations. I renamed them to "filter_*" to comply CODING_STANDARDS. This feedback is great because I'm worrying about the same thing. Please feedback this kind of things during discussion so that I can do something on issues. Thank you for comments. I think it's very helpful for improvements! Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #95167 — Yasuo Ohgaki — 2016-08-15T03:35:56+00:00 URL: https://externals.io/message/95159#95167 · In reply to: #95165 > Hi Dan, > > Thank you for sharing idea! > > > > > […] > > Without feedback, there is no clue which way should go or improve. > I didn't realize some of your idea and I think your feedback is great. > > > […] > > When exception is enabled, offensive key name is written in exception message. > > When exception is disabled, your statement is true. This could be improved. > Good feedback. > > > […] > > It seems there is misunderstanding. > These new functions are intended for "secure coding input validation" that > should never fail. It means something unexpected in input data that > cannot/shouldn't keep program running. Why do you need to parse > message? > > All needed info, filter name, key and value, is in exception message and > exception object, BTW. > > This one is good feedback, too. > I appreciate better error message suggestions. > > > […] > > I thought I don't have to have example of userland implementation, so > it's good feedback also. > > Typical OO implementation uses number of setters to define validation rules. > In addition, it validates validation rule is OK for it. e.g. It will > check input data type at least. Setters and validation rule validation > makes execution slower obviously. > > One may optimize validation rules to plain array (like I do). > In this case, performance is could be better than previously mentioned > validators do all in the production environment. > > I also thought the performance issue is not much important because > there is no PHP feature to compare. All of us knew PHP function call > overheads are relatively large and proposed almost all in C implementation > would be faster than userland. > > > […] > > There is misunderstanding on this. > As I wrote explicitly in the RFC, input validation and user input > mistakes must be handled differently. > > "The input validation (or think it as assertion or requirement) error" > that this RFC is dealing, is should never happen conditions (or think > it as contract should never fail). > > The point of having the input validation is accept only inputs that > program expects and can work correctly. Accepting unexpected > data that program cannot work correctly is pointless. > > Don't misunderstood me. I'm not saying "You should reject user input mistakes". > "User input mistakes" and "input validation error" is totally different error. > > > […] > > I didn't recommend it either because it could not be used for input > validation easily, escaping or sanitization could be done for > dedicated API. > > Having new module is one of my idea also. However, I realized many of > filter module codes could be reused after investigation. That's the > reason why I added to filter module. I also named new functions to > have "validate_*" prefix, rather than "filter_*" to emphasis it's > for validations. I renamed them to "filter_*" to comply CODING_STANDARDS. > > This feedback is great because I'm worrying about the same thing. > Please feedback this kind of things during discussion so that I can > do something on issues. > > Thank you for comments. > I think it's very helpful for improvements! Hi Dan, I don't mind suspend vote for a while to resolve issues if there should be changes in the RFC. I also don't mind adding missing features, e.g. helpful error messages when exception is disabled, to my todo list. BTW, I'll document basic idea of secure coding and emphasize how it should be done, so misuse would be few. It seems you aren't objecting the idea itself. Could you give more feedback what's missing to change your vote? Thank you! -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #95169 — Marco Pivetta — 2016-08-15T03:46:51+00:00 URL: https://externals.io/message/95159#95169 Hey Yasuo, Besides what reported above by Dan, my reasoning for voting "no" is that this API can be implemented in userland, regardless if trivial or not There is no reason good enough for justifying yet another added endpoint that can even be implemented with simple function composition. In addition to that, the lack of a strongly typed data structure for the validation DSL makes this proposed functionality very error-prone and obnoxious to use and maintain for future additional use-case scenarios that may come up. Performance impact in userland implementations can be mitigated via codegen there (similar to what Nikic's FastRoute lib): still less complicated than relying on the core API, maintaining it in C code, and having it locked onto the installed PHP version. Cheers, Marco Hi Dan, Thank you for sharing idea! On Mon, Aug 15, 2016 at 10:25 AM, Dan Ackroyd wrote: > > > > > […] > > > I'm not entirely sure why you ask for reasons when people vote no. The > reasons are almost always the same as the reasons given before the > voting starts. Without feedback, there is no clue which way should go or improve. I didn't realize some of your idea and I think your feedback is great. > > But for posterity: > > i) Validation error messages need to specify what is wrong.....which > is bespoke to the application. Which is a reason why validation code > belongs in userland. When exception is enabled, offensive key name is written in exception message. When exception is disabled, your statement is true. This could be improved. Good feedback. > > ii) Validation error message need to be in the correct language for an > application. It is not a good approach for people to be trying to > match strings emitted by internal code and trying to convert them to > the correct language. It seems there is misunderstanding. These new functions are intended for "secure coding input validation" that should never fail. It means something unexpected in input data that cannot/shouldn't keep program running. Why do you need to parse message? All needed info, filter name, key and value, is in exception message and exception object, BTW. This one is good feedback, too. I appreciate better error message suggestions. > > iii) The argument that it needs to be fast could be applied to > anything and everything, and so is bogus. The RFC doesn't even show > that userland implementations are slow enought to be a concern. I thought I don't have to have example of userland implementation, so it's good feedback also. Typical OO implementation uses number of setters to define validation rules. In addition, it validates validation rule is OK for it. e.g. It will check input data type at least. Setters and validation rule validation makes execution slower obviously. One may optimize validation rules to plain array (like I do). In this case, performance is could be better than previously mentioned validators do all in the production environment. I also thought the performance issue is not much important because there is no PHP feature to compare. All of us knew PHP function call overheads are relatively large and proposed almost all in C implementation would be faster than userland. > > iv) The RFC makes an assumption that programs should exit when validation fails. > > "Input data validation should accept only valid and possible inputs. > If not, reject it and terminate program." > > and the code example: > > > […] input validation fails > > […] > > This assumption is bogus. > > Any program that accepts data from users should provide useful error > messages when the data is wrong with someting as simple as a string > being too long. There is misunderstanding on this. As I wrote explicitly in the RFC, input validation and user input mistakes must be handled differently. "The input validation (or think it as assertion or requirement) error" that this RFC is dealing, is should never happen conditions (or think it as contract should never fail). The point of having the input validation is accept only inputs that program expects and can work correctly. Accepting unexpected data that program cannot work correctly is pointless. Don't misunderstood me. I'm not saying "You should reject user input mistakes". "User input mistakes" and "input validation error" is totally different error. > > v) I don't like the current filter functions, and recommend people > avoid using them. Adding to them with an even harder to use API is the > wrong way to go. I didn't recommend it either because it could not be used for input validation easily, escaping or sanitization could be done for dedicated API. Having new module is one of my idea also. However, I realized many of filter module codes could be reused after investigation. That's the reason why I added to filter module. I also named new functions to have "validate_*" prefix, rather than "filter_*" to emphasis it's for validations. I renamed them to "filter_*" to comply CODING_STANDARDS. This feedback is great because I'm worrying about the same thing. Please feedback this kind of things during discussion so that I can do something on issues. Thank you for comments. I think it's very helpful for improvements! Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #95170 — Stanislav Malyshev — 2016-08-15T05:17:07+00:00 URL: https://externals.io/message/95159#95170 · In reply to: #95169 Hi! > It seems there is misunderstanding. > These new functions are intended for "secure coding input validation" that > should never fail. It means something unexpected in input data that > cannot/shouldn't keep program running. Why do you need to parse > message? I think the problem here is as follows: assume you accept use input. You want it to conform to some set of rules. If it does not, you may want to inform the user that the input is wrong, in an informative way. Now, if you say these functions "should never fail", it implies that before them, there would be other functions filtering user input (because user input could always violate whatever rules you'd have) - and then the question is, would you really want *two* sets of validators? You'd probably want one. Now, when you have one, you probably want it to validate the data and return some information that would be useful for informing the user what has gone wrong. That seems to be the issue here. I do think having strong input validation is a good thing. However, we'd also need to have them in a way that would make them useful in above scenario - otherwise people would avoid them because they fail "too hard" and the app does not retain enough control over the outcome. > There is misunderstanding on this. > As I wrote explicitly in the RFC, input validation and user input > mistakes must be handled differently. > > "The input validation (or think it as assertion or requirement) error" > that this RFC is dealing, is should never happen conditions (or think > it as contract should never fail). This is what I'm not sure I understand - when this approach would be used? I.e. if I get data from the user, I surely can not claim I can impose any conditions on the data that would never fail. Is it assumed I'd pre-filter the data before passing it to this filter? > The point of having the input validation is accept only inputs that > program expects and can work correctly. Accepting unexpected > data that program cannot work correctly is pointless. Well, that depends on what you mean by "accepting". The program should exhibit sane behavior (i.e., useful error message, not whitescreen or something like that) on bad input. That behavior can be different - i.e., if you are given wrong password, you shouldn't be too helpful and say "this password is wrong, the right password is this: ...." (you'd laugh but there *was* a real application doing this, no, I have no idea what the developers were thinking :) but at least you could say "authentication details are wrong". > Don't misunderstood me. I'm not saying "You should reject user input > mistakes". > "User input mistakes" and "input validation error" is totally different > error. Here, again, I am not sure I understand the difference. -- Stas Malyshev smalyshev@gmail.com --- ## #95176 — Lester Caine — 2016-08-15T08:27:11+00:00 URL: https://externals.io/message/95159#95176 · In reply to: #95170 > > […] > This is what I'm not sure I understand - when this approach would be > used? I.e. if I get data from the user, I surely can not claim I can > impose any conditions on the data that would never fail. Is it assumed > I'd pre-filter the data before passing it to this filter? Keeping things simple ... I like your nice flowchart ... BUT Input logic is a LOT more complex than that. I need to be able to use the rule set that you are hiding in the filter to CREATE the page that your little man is looking at. Those rules create the browser side validation everybody seems to think is pointless, but is essential in modern web apps? Those rules may well flag that if some 'variable' already exists browser side actions can amend the workflow and load the selected data. At the very least input validation once the input array reaches the server may require different rules based on some of the responses, and 'business logic' requires 'sanitized' variables to carry out that process. Sanitation that may vary depending on the workflow. Basically the simplistic view that everything can be reduced to a fixed single chain is not what happens in reality. The output array to amend the stored data WILL have a different set of variables depending on the route through, so any filter needs to be able to be built from the set of rules that the variables define. And in my own storage process, the set of variables being stored are individual records in table who's wrapping transaction must complete or roll back and add new failure flags to the set of variables before deciding what to return to the user screen. You can not assume your output process will complete without errors and those errors will amend the rest of the chain. -- Lester Caine - G8HFL ----------------------------- Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk --- ## #95185 — Tony Marston — 2016-08-15T11:00:13+00:00 URL: https://externals.io/message/95159#95185 · In reply to: #95164 "Dan Ackroyd" wrote in message news:CA+kxMuRiOBQpmTeKqNyV8rX0GKCLrYixi--y5TcYUkdqpT746w@mail.gmail.com... > >Hi Yasuo, > > > > > […] > > >I'm not entirely sure why you ask for reasons when people vote no. The >reasons are almost always the same as the reasons given before the >voting starts. > >But for posterity: > >i) Validation error messages need to specify what is wrong.....which >is bespoke to the application. Which is a reason why validation code >belongs in userland. I agree 100% >ii) Validation error message need to be in the correct language for an >application. It is not a good approach for people to be trying to >match strings emitted by internal code and trying to convert them to >the correct language. I agree 100% >iii) The argument that it needs to be fast could be applied to >anything and everything, and so is bogus. The RFC doesn't even show >that userland implementations are slow enought to be a concern. > >iv) The RFC makes an assumption that programs should exit when validation >fails. > >"Input data validation should accept only valid and possible inputs. >If not, reject it and terminate program." I DISagree 100%. Validation errors should NEVER terminate the program, they should continue by displaying all the error messages to the user so that he/she can correct his/her mistake and try the operation again. >and the code example: > > > […] > >This assumption is bogus. > >Any program that accepts data from users should provide useful error >messages when the data is wrong with someting as simple as a string >being too long. I agree 100% >v) I don't like the current filter functions, and recommend people >avoid using them. Adding to them with an even harder to use API is the >wrong way to go. I agree 100% -- Tony Marston --- ## #95187 — Christoph M. Becker — 2016-08-15T13:13:43+00:00 URL: https://externals.io/message/95159#95187 · In reply to: #95185 > "Dan Ackroyd" wrote in message > news:CA+kxMuRiOBQpmTeKqNyV8rX0GKCLrYixi--y5TcYUkdqpT746w@mail.gmail.com... > > > […] > > I DISagree 100%. Validation errors should NEVER terminate the program, > they should continue by displaying all the error messages to the user so > that he/she can correct his/her mistake and try the operation again. Yasuo (who Dan quoted here) refers to completely invalid input, such as invalid UTF-8 byte sequences. I think, that in this case the app should bail out without even given detailed information, as such grossly invalid input most likely is an attempt to attack (or a severe browser bug). -- Christoph M. Becker --- ## #95225 — Stanislav Malyshev — 2016-08-16T06:42:05+00:00 URL: https://externals.io/message/95159#95225 · In reply to: #95187 Hi! > Yasuo (who Dan quoted here) refers to completely invalid input, such as > invalid UTF-8 byte sequences. I think, that in this case the app should > bail out without even given detailed information, as such grossly > invalid input most likely is an attempt to attack (or a severe browser bug). I personally am not a big fan of "bail out without giving information", unless that information somehow crosses security boundary (e.g. displaying PHP error messages in production) or reveals unnecessary info (this part is super-tricky in crypto, but ouside of crypto common sense is usually not a bad guide). Assume indeed you have a buggy release of Firefox that produces invalid UTF-8 when your language is set to Hindi (this is almost true story btw, I've seen bug not exactly that but somewhat similar). Now assume you get a message from the user "all our office can not use your application since new version was deployed!" and you walk the user through and it indeed bails out, no additional info. How you debug that? You don't know Hindi is the culprit. You may not have access to that office's environment. Your users can't help much but scream "get our app working again, we're losing money here!". And of course it works for you when you try it and best time to talk to them is 4am on your side. Now, how much easier your life would be if you app would just report "invalid UTF-8 sequence encountered in parameter FirstName" before bailing out? How many hours, pulled out hairs and 4am sessions would it save? I think it's worth considering. -- Stas Malyshev smalyshev@gmail.com --- ## #95226 — Dan Ackroyd — 2016-08-16T08:01:46+00:00 URL: https://externals.io/message/95159#95226 · In reply to: #95167 > > > Hi Dan, > > I don't mind suspend vote for a while ... > > It seems you aren't objecting the idea itself. I do object to the concept of the RFC. In my first point I said it belongs in userland. And I strongly object to the idea of stopping and starting voting on RFCS. Please leave the vote open and if it fails take some time to think about the feedback. If then, in at least 3 months time, you think you can improve the RFC significantly, then reintroduce the idea. cheers Dan --- ## #95227 — Christoph M. Becker — 2016-08-16T09:01:40+00:00 URL: https://externals.io/message/95159#95227 · In reply to: #95225 Hi! > > […] > > I personally am not a big fan of "bail out without giving information", > unless that information somehow crosses security boundary (e.g. > displaying PHP error messages in production) or reveals unnecessary info > (this part is super-tricky in crypto, but ouside of crypto common sense > is usually not a bad guide). > > > > Now, how much easier your life would be if you app would just report > "invalid UTF-8 sequence encountered in parameter FirstName" before > bailing out? How many hours, pulled out hairs and 4am sessions would it > save? I think it's worth considering. I once introduced a check erroring with "Malformed UTF-8 detected" to a CMS. Until that was changed to "Bad request. Please try again.", we got a lot of support requests from confused users who had bookmarked URLs with ISO-8859-* query strings. Even pointing out which parameter was the culprit, wouldn't have changed that, I presume. Of course, it makes sense to *log* very detailed information in this case (amongst others, the byte sequence that was malformed), but presenting them to visitors doesn't seem to be helpful – most of these wouldn't even know what UTF-8 is. -- Christoph M. Becker --- ## #95244 — Yasuo Ohgaki — 2016-08-16T23:09:12+00:00 URL: https://externals.io/message/95159#95244 · In reply to: #95170 Hi Stas, > > […] > > I think the problem here is as follows: assume you accept use input. You > want it to conform to some set of rules. If it does not, you may want to > inform the user that the input is wrong, in an informative way. Now, if > you say these functions "should never fail", it implies that before > them, there would be other functions filtering user input (because user > input could always violate whatever rules you'd have) - and then the > question is, would you really want *two* sets of validators? You'd > probably want one. > Now, when you have one, you probably want it to validate the data and > return some information that would be useful for informing the user what > has gone wrong. That seems to be the issue here. > I do think having strong input validation is a good thing. However, we'd > also need to have them in a way that would make them useful in above > scenario - otherwise people would avoid them because they fail "too > hard" and the app does not retain enough control over the outcome. I think this discussion relates to following questions. I'll try to explain there. > > > […] > > This is what I'm not sure I understand - when this approach would be > used? I.e. if I get data from the user, I surely can not claim I can > impose any conditions on the data that would never fail. Is it assumed > I'd pre-filter the data before passing it to this filter? How and what rules could be imposed to inputs varies depending on what kind of data should be sent from outsides of a software including human users. Let's say your app validate user written/chosen "Date" on client side by JavaScript. Then browser must send whatever "Date" format you impose to client. It may be "YYYYMMDD", for example. Then programer should not accept "Date" format other than "YYYYMMDD" because other format is invalid. Accepting format other than "YYYYMMDD" does only bad and increase risks of program malfunctioning. i.e. All kinds of injections like JavaScript, SQL, Null char, Newline, etc. The basic idea of secure coding input validation is to remove all unnecessary security risks at "Input Validation". Even when "Date" field is plain that user can write any chars, Null char, CR/LF, TAB or any CNTRL chars should not be in there. There will be no users type in 100 chars for "Date" field unless they were trying to tamper application. "Input validation" should reject all of them and does not have to inform users (attackers) to "there is invalid input". If you need to tell legitimate users "There is invalid input", then it should be treated by "Business logic", not by "Input validation". > > > […] > > Well, that depends on what you mean by "accepting". The program should > exhibit sane behavior (i.e., useful error message, not whitescreen or > something like that) on bad input. That behavior can be different - > i.e., if you are given wrong password, you shouldn't be too helpful and > say "this password is wrong, the right password is this: ...." (you'd > laugh but there *was* a real application doing this, no, I have no idea > what the developers were thinking :) but at least you could say > "authentication details are wrong". User authentication could do the similar to "Date" field for "User name" and "Password". "User name" and "Password" shouldn't have CNTRL chars or invalid char encoding. Even when fields are plain , there shouldn't be 500 chars long inputs for them. Anything else for "User name" and "Password" should be handled by "Business logic". Logic part should display nice and proper error messages like - User name is too long for 100 chars name. - Password is too long for 100 chars password. - User name and/or Password is wrong and failed to authenticate. > > […] > > Here, again, I am not sure I understand the difference. The reason why I propose to divide input error checks into "Input validation" and "Business logic" is for simplicity and maintainability. "Input validation" should be done not only for human entered inputs, but also automatically generated inputs by system. Generally speaking, developers should not accept request that has Invalid browser headers: - Invalid REFERER contains Illegal/CTNRL chars and/or too many chars. - Invalid ACCEPT-CHARSET contains Illegal/CNTRL chars and/or too many chars. - Invalid ACCEPT-ENCODING contains Illegal/CNTRL chars and/or too many chars. - Invalid ACCEPT-LANGUAGE contains Illegal/CNTRL chars and/or too many chars. - and so on. Invalid POST/GET request: - Lacks required field by your program. e.g. If you set CSRF token for POST always, but it's missing. - Multi page form inputs and lack/have invalid data that should have been validated previously. Note: there is design choice for this where/how to deal with invalid inputs. - Program written data is invalid. e.g. //php.net/show_bug.php?id=[string contains CNTRL chars and/or 100 chars or more] - $_POST/$_GET has more than 20 elements. Note: most apps/code would not have this many elements. Invalid COOKIE: - $_COOKIE has more than 20 elements. Note: normal apps would not have this many cookies. - Lacks required field by your program. - Invalid chars. e.g. CNTRL chars. All of these have history of abuse by attackers and programs should not accept them. Please note that secure coding requires to output securely. Input validation and output sanitization should be treated as individual task. e.g. Escape all variables at "Output" code when you output something to other software. Never assume, "This var is validated at input, so it is safe without escaping." It's developer's choice how to validate inputs, e.g. they don't use "CONNECTION" HTTP header at all and don't care, but all of secure coding related guides that I know of recommends/requires to validate "all inputs". Validating all inputs that are irrelevant to "Business logic" makes programs complicated and hard to maintain. Broken char encoding, too long/short, CNTRL chars for
inputs are better to handled by "Input validation" because the same thing might be done by different s repeatedly. There are many possibility for software design. This RFC is designed to encourage to do certain validation. However, this RFC does not impose developers to do certain validation, but provides tools that are needed for validations. I would not encourage users to disable exception from filter_require_var()/filter_require_var_array(), but I've changed them not to raise exception optionally as a last minute change. This allows developers to use new validator for wider purposes. Regards, P.S. I'll extend vote period because there is ongoing discussion. BTW, ISO 27000/ISMS requires/recommends proposed input validation. Latest ISO 27000 mentioned as "adopt secure programming". Older ISO 27000 explained how to validate inputs. New ISO 27000 removed detailed input validation method explanation because secure programming is widely adopted and standardized. -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #95245 — Yasuo Ohgaki — 2016-08-16T23:16:19+00:00 URL: https://externals.io/message/95159#95245 · In reply to: #95227 Hi Christoph, > > > > […] > > I once introduced a check erroring with "Malformed UTF-8 detected" to a > CMS. Until that was changed to "Bad request. Please try > again.", we got a lot of support requests from confused users who > had bookmarked URLs with ISO-8859-* query strings. Even pointing out > which parameter was the culprit, wouldn't have changed that, I presume. > > Of course, it makes sense to *log* very detailed information in this > case (amongst others, the byte sequence that was malformed), but > presenting them to visitors doesn't seem to be helpful – most of these > wouldn't even know what UTF-8 is. Excellent example of input validation exception! Software has history, therefore certain validation cannot be done automatically. For the record, many security standards/guides require to "Canonicalize" input data _before_ input validation. If anyone would like to validate "String", canonicalize first. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #95246 — Yasuo Ohgaki — 2016-08-16T23:43:13+00:00 URL: https://externals.io/message/95159#95246 · In reply to: #95185 Hi Tony, Allow me to top post. "The input validation" is not for legitimate users, but for attackers. You shouldn't help attackers by explaining what/how wrong in attackers' inputs. I've added discussion "Input validation and User input mistake handling difference" https://wiki.php.net/rfc/add_validate_functions_to_filter#input_validation_and_user_input_mistake_handling_difference Please refer to the section for distinction. BTW, the input validation that I'm proposing here is required/recommended feature by ISO 27000/ISMS. Why shouldn't PHP provide features that is needed to implement ISO 27000/ISMS requirements? > "Dan Ackroyd" wrote in message > news:CA+kxMuRiOBQpmTeKqNyV8rX0GKCLrYixi--y5TcYUkdqpT746w@mail.gmail.com... > > […] > > > I agree 100% > > > […] > > > I agree 100% > > > […] > > > I DISagree 100%. Validation errors should NEVER terminate the program, they > should continue by displaying all the error messages to the user so that > he/she can correct his/her mistake and try the operation again. > > > […] > > > I agree 100% > > > […] > > > I agree 100% -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #95247 — Yasuo Ohgaki — 2016-08-17T00:01:46+00:00 URL: https://externals.io/message/95159#95247 · In reply to: #95185 To those who voted "no" for this, > "Dan Ackroyd" wrote in message > news:CA+kxMuRiOBQpmTeKqNyV8rX0GKCLrYixi--y5TcYUkdqpT746w@mail.gmail.com... > > […] > > > I agree 100% > > > […] > > > I agree 100% > > > […] > > > I DISagree 100%. Validation errors should NEVER terminate the program, they > should continue by displaying all the error messages to the user so that > he/she can correct his/her mistake and try the operation again. > > > […] > > > I agree 100% > > > […] > > > I agree 100% > > > -- > Tony Marston Could you explain why or express "same opinion here"? IMHO, this opinion is based on misunderstanding of secure coding and software security methodology. However, either way is helpful. There are many that seem "the input validation should be implemented by userland fully". If this is true, I'm going to propose "Filter module" deprecation next. It's better not to have misleading and/or half implemented module for security purpose. Thank you! -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #95248 — Pierre Joye — 2016-08-17T00:19:43+00:00 URL: https://externals.io/message/95159#95248 · In reply to: #95167 > I don't mind suspend vote for a while to resolve issues if there > should be changes in the RFC. I also don't mind adding missing > features, e.g. helpful error messages when exception is disabled, to > my todo list. BTW, I'll document basic idea of secure coding and > emphasize how it should be done, so misuse would be few. There is no such thing as suspended vote. The vote has to restart if there are changes. --- ## #95249 — Yasuo Ohgaki — 2016-08-17T00:30:31+00:00 URL: https://externals.io/message/95159#95249 · In reply to: #95248 Hi Pierre, > > > > […] > > There is no such thing as suspended vote. The vote has to restart if there > are changes. Thank you. I'll restart vote if I have to make changes, other than more items in discussion section. Anyone would change votes if I rename functions to avoid possible confusions? There is one opinion for better names so far. It seems either "People misunderstand secure coding" and/or "People consider validation codes should be in userland fully". (Or "No more additions for filter module"?) Although I do think repetitive validations for browser's request headers/inputs in userland is awfully inefficient, efficiency is not 1st priority of security anyway. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #95250 — Yasuo Ohgaki — 2016-08-17T02:59:27+00:00 URL: https://externals.io/message/95159#95250 · In reply to: #95169 Hi Marco, > Besides what reported above by Dan, my reasoning for voting "no" is that > this API can be implemented in userland, regardless if trivial or not > > There is no reason good enough for justifying yet another added endpoint > that can even be implemented with simple function composition. OK. Thank you. You prefer full userland implementation. > > In addition to that, the lack of a strongly typed data structure for the > validation DSL makes this proposed functionality very error-prone and > obnoxious to use and maintain for future additional use-case scenarios that > may come up. This is good argument. Filter module uses definition array already. The RFC adds check function for them, too. New check function does not take care semantics, it may be improved by having class for defining validation rule. If we have to add more complex, yet robust input validation definitions as array, all we have to do is adding version number, totally different array structure or object for it. Do you want me to drop filter_check_definition() proposal? It's easy to write PHP code that does the same, and write it in the manual. It's only there, because I'm expecting comments like you've made. "There is no check function for rule definition array. It's dangerous" or something like this. > Performance impact in userland implementations can be mitigated via codegen > there (similar to what Nikic's FastRoute lib): still less complicated than > relying on the core API, maintaining it in C code, and having it locked onto > the installed PHP version. This is debatable how far PHP should implement mandatory features for web applications. Some may prefer PHP to be like Python or other normal languages that does not have web application support in core at all. Having a router in core is too much to me, too. However, I prefer PHP to have basic features that is mandatory to write simple web forms. For example, 1]); // There is RFC for this. function check_user_input_error($today) { if (strtotime($today) != date('Ymd')) { $err_msg[] = 'You have entered invalid date. '. $today; return $err_msg; } } if ($_POST['submit']) { $err_msg = check_user_input_error($_POST['Today']); if (!(empty($err_msg))) { // Save CSRF protected data into some DB } else { $_POST=array(); } } // We do need shorter/simpler/consistent escape functions somehow. ?> // Display client info and date You're using Yes, today is Enter today's date:
Simple web forms should be able to be written by PHP core feature only. IMHO. It's impossible to teach beginners how to write code for input validations. As a result, the most important security feature, input validation, is omitted in beginner courses/examples/etc. It's great for beginners to understand what's going on Web apps and what developers should do. It's useful for small web service that requires the best performance possible as well. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #95252 — Dan Ackroyd — 2016-08-17T03:23:25+00:00 URL: https://externals.io/message/95159#95252 · In reply to: #95249 > I'll restart vote if I have to make changes, Yasuo, No one is asking you to make changes to the RFC. Just leave the voting open, and then close it on the agreed date. You would be violating the agreed RFC process* by closing a vote early, just so that you an resubmit a proposal straight away. https://wiki.php.net/rfc/voting Additionally, you seem to completely have ignored this: Dan Ackroyd wrote: > And I strongly object to the idea of stopping and starting voting on RFCS. Please leave the vote open and if it fails take some time to think about the feedback. It would benefit everyone if you stopped responding immediately and instead took time to actually think about what people have been saying. This RFC isn't going to be in PHP 7.1, so it is fine to wait 3 months to present a new version of the RFC. cheers Dan * "Resurrecting Rejected Proposals In order to save valuable time, it will not be allowed to bring up a rejected proposal up for another vote, unless one of the following happens: 6 months pass from the time of the previous vote, OR The author(s) make substantial changes to the proposal." --- ## #95253 — Yasuo Ohgaki — 2016-08-17T05:19:48+00:00 URL: https://externals.io/message/95159#95253 · In reply to: #95252 Hi Dan, I understood about RFC process. > Additionally, you seem to completely have ignored this: > > Dan Ackroyd wrote: > > […] > > It would benefit everyone if you stopped responding immediately and > instead took time to actually think about what people have been > saying. This RFC isn't going to be in PHP 7.1, so it is fine to wait 3 > months to present a new version of the RFC. It seems I've marked "already read" by mistake. Thank you for reminding. I got that you prefer userland implementation. I'm planning to propose "Filter module deprecation" when this RFC is declined, because current validation filter is not good enough to do the job and makes situation worse than better... If deprecation RFC is declined also, then I might try to improve this RFC again. BTW, I cannot guess the reason behind "no" votes. I can guess reasons for people participating discussions, though. Even when RFC author could guess the reason, it would be nicer for voters and author if one explains the reason why vote "no" in vote thread. Explicit description is better than guess, IMHO. Besides, unlike you, there are many people do not left any clue. For example, I completely fail to understand the reason why "Enable session.use_strict_mode by default" and "Precise Session Management" RFC is declined. These are _mandatory_ for session security and not a matter of preference, but do it and/or how to do it. If one fails to see why it is mandatory, should ask why. If one think "it must be more efficient", then should insist patch improvement. If one think proposal is wrong, then should point out what's wrong. IMO. If opinion is the same, should mention "Same here"/"Agree" at least. It's okay to say "let's ignore such security issues" or "let it users responsibility to secure session", but his/her opinion should be expressed. It's not a political vote, but technical vote after all. I guess most people voted "no" for "Enable session.use_strict_mode by default" and "Precise Session Management" is based on wrong assumption. For this vote, I'm guessing preferences are strongly affected, filter module nature and patch quality. The code is messy because I didn't refactor code to minimize changes. It's still a guess, though. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #95254 — Lester Caine — 2016-08-17T08:08:03+00:00 URL: https://externals.io/message/95159#95254 · In reply to: #95250 > require_once('my_exception_error_handler.php'); Actually load framework ... and that is the first can of worms? > require_once('my_input_spec_def_for_this_file.php'); THIS is my sticking point ... What it needs to load is the rules for all of fields that were in the page build. For simple forms that is fine, but it does not scale. For more complex form it has to scan the input array and build a list of keys ... why not carry out checks while doing that scan? Makes building an array of rules pointless, just use the individual rules ... except we do not know where to get them. > // Validate general requirement that cannot covered by filter_requrie*() > validate_inputs(); > // These are came from this RFC. Validate inputs. > // What's to validate is design decision, but validating them all is the > // best way. > filter_require_var_array($_GET, $get_spec); > filter_require_var_array($_POST, $post_spec); > filter_require_var_array($_COOKIE, $cookie_spec); > filter_require_var_array($_SERVER, $server_spec); I have less of a problem with the STATIC stuff covered here EXCEPT that would be handled as part of loading the framework and include checking what state of handling the form you ARE at. In my case the session has elements of handling multiple page forms. > session_start(['use_csrf_protection'=>1]); // There is RFC for this. > > function check_user_input_error($today) { > if (strtotime($today) != date('Ymd')) { > $err_msg[] = 'You have entered invalid date. '. $today; > return $err_msg; > } > } I will not waste time going through your long discussion on handling dates it's the bain of my life! Especially when you get given 12/8/2016 and have no idea if it is August or December! So forms use date pickers and browser validation. And the "'You have entered invalid date. '. $today;" forms part of the set of rules used to build the form. Validate that christening date is later than birth date has a suitable error message as part of that rule set. Now cross checking those rules at this stage is probably necessary but only if some hacker is trying to be cleaver? But in my case business rules can be cross checked in the database ... apps other than PHP may also be accessing it ( think pigging phone apps :( ) ... so the same rules are needed in the database business logic. And that is why populating the PHP rules data from the database schema makes sense. > Simple web forms should be able to be written by PHP core feature > only. IMHO. Just how many versions of 'login' or 'register' pages exist and all have checks for valid username, password and email. And all use different styles of managing the rules. > It's impossible to teach beginners how to write code for input validations. > As a result, the most important security feature, input validation, is > omitted in beginner courses/examples/etc. > > It's great for beginners to understand what's going on Web apps and > what developers should do. It's useful for small web service that > requires the best performance possible as well. We are on the same book, just coming at this from different ends. It's that 'my_input_spec_def_for_this_file.php' which is the problem starting with just where you get the set of rules from and not having a simple beginner friendly method of adding those rules to a variable. There are hundreds of legacy examples of how to do a login form, and the vast majority use styles of programming that start a user down the wrong path from day one ... so can we agree on something simple and easily expandable? filter_require_var_array($_POST, $post_spec); could then use those rules to build the $post_spec array, but why not just use the rules direct? -- Lester Caine - G8HFL ----------------------------- Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk --- ## #95255 — Stanislav Malyshev — 2016-08-17T08:33:21+00:00 URL: https://externals.io/message/95159#95255 · In reply to: #95244 Hi! > Let's say your app validate user written/chosen "Date" on client side by > JavaScript. Then browser must send whatever "Date" format you impose > to client. It may be "YYYYMMDD", for example. I'm not sure what Javascript has to do with it. Many apps don't have any client-side and have little to do with Javascript. Assuming that whole world is browser applications running Javascript (controlled by you) would be a big mistake. > Then programer should not accept "Date" format other than "YYYYMMDD" > because other format is invalid. Accepting format other than "YYYYMMDD" > does only bad and increase risks of program malfunctioning. i.e. All kinds > of injections like JavaScript, SQL, Null char, Newline, etc. What you mean by "accept" here? I think you are under impression (please correct me if I'm wrong) that there are only two ways for application to work - either treating all inputs equally, or bailing out immediately when incorrect input is detected. However, this is not the case, there are many other ways for application to handle the situation of invalid input - while knowing it is invalid - and exact manner of this handling is application-dependent. > "Input validation" should reject all of them and does not have to inform users > (attackers) to "there is invalid input". If you need to tell legitimate users I think we disagree here. I think not doing this makes my work as a developer much much harder. > "There is invalid input", then it should be treated by "Business logic", not by > "Input validation". Wait, input validation happens before business logic has a chance to run, so if input validation bails, how business logic can treat anything? > "User name" and "Password" shouldn't have CNTRL chars or invalid char > encoding. Even when fields are plain , there shouldn't be 500 chars > long inputs for them. So your proposal seems to be having two input checking procedures instead of one. I don't think people would find it very useful to have two separate input checking procedures. -- Stas Malyshev smalyshev@gmail.com --- Next page: https://externals.io/message/95159.md?page=2