Re: [PHP-DEV] [VOTE] Increasing the default BCrypt cost

2023-09-25 Thread Craig Francis
On 25 Sep 2023, at 18:07, Tim Düsterhus  wrote:
> I've now did the maths and you really need rate limiting no matter if you use 
> costs 10, 11 or 12, so I believe the DoS argument is a little moot.


Yes, someone being malicious could easily generate enough requests to create an 
Denial of Service Attack, but I was referring to normal users logging in, on a 
small hosting service.

Think of a little web-shop that has just sent out an email to ~30,000 
customers, and initially they get a gentle ~20 customers logins at a time... 
with a cost of 10, that causes the HTML for other all other pages to go from 
0.09 seconds to ~1.1 seconds, not good, but manageable; cost of 11 takes that 
to ~2.1 seconds; cost of 12 goes to ~4.2 seconds.

(I got those numbers with a simple `ab -n 200 -c 20` to call password_hash, and 
`while true; do curl -o /dev/null -s -w '%{time_total}\n'` to request a basic 
page while this is running to get some rough averages).




>> While a high cost might make you *feel* good, the DoS problem is real, 
>> especially on older hardware - 10 is still fine today, 11 is a fair 
>> improvement against brute force guessing, 12 is just burning CPU cycles 
>> today, simply because the difference does not address the problem of 
>> commonly used passwords (like 123456, password1, monkey, etc).
> 
> The attacker does not know which users use less secure passwords and thus 
> will spend effort for "secure" and "insecure" passwords alike. Doubling the 
> costs will mean that each password takes twice as long to crack on average, 
> making cracking twice as expensive. For less secure passwords that can make 
> the difference between "being cracked" and "not being cracked" if the 
> attacker is willing to spend a given amount of CPU time per password.


Yep, and we are defining a baseline, a default that is good enough for 
everyone; this is why I'd consider what is being achieved, think of normal 
customers, choosing passwords that can be found on the 14.3 million record 
RockYou list, to test that at "640 hashes per second", would be 6.2 hours per 
hash, so the 11 vs 12 cost for these people won't really make much of a 
difference to them.

Craig




For those who want a bit of background, while this 3 years old video covers a 
different subject, @chick3nman (of hashcat fame) notes the use/value of bcrypt:

https://www.youtube.com/watch?v=OQD3qDYMyYQ&t=381s

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



Re: [PHP-DEV] [VOTE] Increasing the default BCrypt cost

2023-09-25 Thread Tim Düsterhus

Hi

On 9/25/23 21:43, Levi Morrison via internals wrote:

I did a tiny bit of my own research, and could not find any
recommendations more specific than "10 or more" as the cost factor.
Typically, the advice is "use a more modern system like argon2id".


Please see this email of mine regarding Argon2:

https://news-web.php.net/php.internals/120996

Other than that, the recommendation for BCrypt's cost factor (and 
basically also Argon's tunables) is "as high as feasible for your use case".


See also this post on Fediverse (it's also referenced in the initial 
email of the voting thread):


https://phpc.social/@tychotithonus@infosec.exchange/111025157601179075


However, I did notice some sites mention that systems ought to check
for a maximum length of 72 bytes when using bcrypt:
https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#input-limits

As far as I can tell, PHP does not do this check. I am not sure if the
implementation(s) used suffer(s) from the limitation that is the
source of this recommendation. Perhaps someone has time to investigate
this? Anyway, it's "future work."


BCrypt-as-specified has a limit of at most 72 bytes, none of which may 
be NUL. Additional characters will simply be ignored. Think of it like 
the password would be passed through `substr($password, 0, 72)`. The 
implementation used is crypt_blowfish by Openwall [1]. The limit in the 
source code is given by this loop:


https://github.com/php/php-src/blob/2e8cdd8eecac5d34619bbd03916d0b7bcc2cc023/ext/standard/crypt_blowfish.c#L580-L582

(BF_N (16) + 2) * 4 is 72

The behavior for longer passwords is well-defined, so I'd say that PHP 
doesn't need any additional check. The only thing that could be done 
differently is throwing an exception and this is likely not a good idea 
for such a generic function as 'password_hash'.



I have voted for 11, but will not be hurt in any way if 12 wins.



Best regards
Tim Düsterhus

[1] https://github.com/openwall/crypt_blowfish

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



Re: [PHP-DEV] [VOTE] Increasing the default BCrypt cost

2023-09-25 Thread Kamil Tekiela
Yes, BCrypt uses only the first 72 bytes for hash generation. You can
test it with:

var_dump(password_verify(str_repeat('a', 72).'sdfsdf',
password_hash(str_repeat('a', 80), PASSWORD_BCRYPT)));

But I would not consider this an issue. Users rarely create passwords
longer than 72 bytes. 72 bytes is still a very long password and not
easily guessable. What's more important is to have the minimum limit
check. But why bother checking the 72 maximum if the algorithm won't
complain about longer input? It doesn't impact security in any way.

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



Re: [PHP-DEV] [VOTE] Increasing the default BCrypt cost

2023-09-25 Thread Levi Morrison via internals
> Please find the following resources for your references:
>
> RFC Text: https://wiki.php.net/rfc/bcrypt_cost_2023
> Discussion Thread: https://externals.io/message/121004
> Feedback by a Hashcat team member on Fediverse:
> https://phpc.social/@tychotithonus@infosec.exchange/111025157601179075

I did a tiny bit of my own research, and could not find any
recommendations more specific than "10 or more" as the cost factor.
Typically, the advice is "use a more modern system like argon2id".

However, I did notice some sites mention that systems ought to check
for a maximum length of 72 bytes when using bcrypt:
https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#input-limits

As far as I can tell, PHP does not do this check. I am not sure if the
implementation(s) used suffer(s) from the limitation that is the
source of this recommendation. Perhaps someone has time to investigate
this? Anyway, it's "future work."

I have voted for 11, but will not be hurt in any way if 12 wins.

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



Re: [PHP-DEV] [VOTE] Increasing the default BCrypt cost

2023-09-25 Thread Tim Düsterhus

Hi

On 9/22/23 10:46, Craig Francis wrote:

On 22 Sep 2023, at 08:04, Nicolas Grekas  wrote:

For the record, I voted for 11 because I think it's nicer to end users (I guess 
many don't know they could have a potential DoS vector via password 
submissions), and also because it's going to be easy to raise again in 8.5/9.0.


[...]
I can't vote, but I would urge people to be careful with this.


I've now did the maths and you really need rate limiting no matter if 
you use costs 10, 11 or 12, so I believe the DoS argument is a little moot.


Taking the Xeon(R) E-2246G CPU as the fastest CPU within the RFC itself 
(only beaten by Alexandru's Xeon Gold 5416S):


Verifying a single cost-11 hash keeps a single core busy for ~80ms. This 
allows for calculating 12.7 cost-11 hashes per core and second. The CPU 
is a 6C/12T CPU. SMT likely does not provide a performance benefit, 
because the cores will generally be busy all the time, but let's just 
assume SMT scales linearly for sake of the argument. Rounding up 
generously that means this CPU can calculate ~155 cost-11 hashes per second.


A single computer on a slow Internet connection is easily capable of 
sustaining 155 HTTP requests per second. For reference a single HTTP/2 
TCP connection commonly allows for 100 concurrent streams as per 
https://stackoverflow.com/a/36847527/782822.


The current costs of 10 would allow for roughly twice the amount of 
hashes per second, resulting in the CPU being saturated at just 300 
requests per second, still easily emitted with a single low-bandwidth 
computer.


For the Xeon Gold 5416S which needs 0.1s / cost-12 hash (i.e. 50ms per 
cost-11 hash) you can do 20 cost-11 hashes per core and second. This CPU 
has 16C/32T, resulting in less than 640 hashes per second if you assume 
SMT scales linearly.



While a high cost might make you *feel* good, the DoS problem is real, 
especially on older hardware - 10 is still fine today, 11 is a fair improvement 
against brute force guessing, 12 is just burning CPU cycles today, simply 
because the difference does not address the problem of commonly used passwords 
(like 123456, password1, monkey, etc).


The attacker does not know which users use less secure passwords and 
thus will spend effort for "secure" and "insecure" passwords alike. 
Doubling the costs will mean that each password takes twice as long to 
crack on average, making cracking twice as expensive. For less secure 
passwords that can make the difference between "being cracked" and "not 
being cracked" if the attacker is willing to spend a given amount of CPU 
time per password.



Also, if you want to increase the cost yourself, on a system which blocks too 
many password attempts, you can do that easily - this is about the default, for 
people who are not customising it for their (shared/old) hardware.



I believe it would be correct to assume that higher-traffic websites 
which are at a risk of denial of service (e.g. by folks that envy their 
success) would generally be sufficiently knowledgeable to make an 
explicit choice for their workload. The less experienced folks that will 
rely on the defaults are probably also the folks who are most likely to 
be breached and thus higher costs provide a real value-add against 
hashes being cracked.


Best regards
Tim Düsterhus

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



Re: [PHP-DEV] [VOTE] Increasing the default BCrypt cost

2023-09-25 Thread Tim Düsterhus

Hi

On 9/25/23 06:20, Theodore Brown wrote:

Thanks for your work on this. I think bumping the default BCrypt cost from 10 
to 11 is reasonable, as this typically adds less than 100 milliseconds 
additional latency, which shouldn't be too noticeable for users logging in.

However, I am concerned about changing the default directly from 10 to 12. Per 
the benchmarks in the RFC, even on recent hardware like the Apple M1 Pro this 
adds 179 ms additional time to verify a password (compared to 60 ms for the 
change to 11). This would be a noticeable slowdown for user logins.

It gets even worse on older hardware, with the example of the 2011 Core i5 
adding 247 milliseconds additional time at a cost of 12, vs. 81 ms additional 
time using a cost of 11.


Logging in should generally be a rare thing for a given user, making a 
longer delay more acceptable. All the services I interact with, except 
for my bank, do not ask for a password more than twice per day with the 
majority allowing for indefinite session lengths.


As per 
https://www.nngroup.com/articles/response-times-3-important-limits/, any 
delay above 100ms is perceptible, but as long as it's below 1000ms, it's 
okay without taking any special measures.


As given in the RFC, costs of 12 stay well below 500ms for all tested 
CPUs. The ARM CPUs tested by Remi are slower than the CPUs I tested, but 
even those are below 430ms.


From my personal experience as a developer of a software that uses 12 
since 2021, costs of 12 do not really feel slow even when logging in 
multiple times in a short period to test the login process.



It will be easy to bump the default cost again in the future, so I think a more 
gradual increase will be safer to avoid an obvious degradation to user login 
time.


I'm concerned about this actually happening. Increasing the default from 
10 is *long* overdue and is only happening, because I accidentally 
stumbled over this issue. As far as I can tell there is no procedure to 
perform this kind of periodic reevaluation of defaults.


Best regards
Tim Düsterhus

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



Re: [PHP-DEV] Security Audit Priorities

2023-09-25 Thread Tim Düsterhus

Hi

On 9/25/23 10:49, Derick Rethans wrote:

So, if you can suggest an area where doing an external review would have
high impact, please reply to this email.


Some things from top of my head in arbitrary order. Not all of them are 
necessarily important themselves per se, but rather intended to spark 
additional thoughts.


- Footguns in the default configuration / tunables / php.ini [1]
- MySQL Native Driver
- password_* [1]
- hash_equals()
- ext/json, specifically json_decode()
- The CSPRNG (ext/random/csprng.c)
- bin2hex, base64_encode [2]
- Open-ended: Misuse resistance of existing functions - Is it possible 
for a user to not properly check a return value and would this result in 
harm (i.e. should the function throw, but does not yet)?


Best regards
Tim Düsterhus

[1] These tie a little into my https://wiki.php.net/rfc/bcrypt_cost_2023 
RFC, which is not code but configuration.
[2] Should these be made constant-time / should constant-time 
implementations always be available? See: 
https://github.com/paragonie/constant_time_encoding


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



Re: [PHP-DEV] Security Audit Priorities

2023-09-25 Thread Hans Henrik Bergan
the php-fpm master<->php-fpm worker glue code. php-fpm master usually
runs as *root*, so a compromise in that glue could lead to webserver
rooting

On Mon, 25 Sept 2023 at 10:49, Derick Rethans  wrote:
>
> Hi,
>
> The Foundation is organising an external audit/security check of the PHP
> source code. As part of that, we would like to identify the places in
> the PHP source code where checking this will have the most impact.
>
> Typical areas would be where user input can be (automatically read) remotely, 
> such as
> our RFC 1867 HTTP header parser. But we are sure there are other
> important areas as well, and we would like your input.
>
> So, if you can suggest an area where doing an external review would have
> high impact, please reply to this email.
>
> cheers,
> Derick
>
> --
> https://derickrethans.nl | https://xdebug.org | https://dram.io
>
> Author of Xdebug. Like it? Consider supporting me: https://xdebug.org/support
> Host of PHP Internals News: https://phpinternals.news
>
> mastodon: @derickr@phpc.social @xdebug@phpc.social
> twitter: @derickr and @xdebug
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>

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



Re: [PHP-DEV] Re: [RFC] [Discussion] DOM HTML5 parsing and serialization support

2023-09-25 Thread Derick Rethans
On Sat, 23 Sep 2023, Niels Dossche wrote:

> On 9/2/23 21:41, Niels Dossche wrote:
> > 
> > I'm opening the discussion for my RFC "DOM HTML5 parsing and
> > serialization support".
> > https://wiki.php.net/rfc/domdocument_html5_parser
> 
> Some minor changes after a discussion with Tim:
> 
> * The old class names are now written with a leading backslash to
>   emphasize they are in the global namespace. Hopefully this will resolve
>   confusion around them.
>
> * Fixed unclear wording, i.e. "type hint" -> "type declaration"
>
> * The alias for DOMException will be DOM\DOMException. Because that's the
>   official name per DOM spec, and otherwise importing it and using it
>   would be confusing with the global namespace Exception class
>   (see also
>   https://github.com/php/php-src/pull/9071#issuecomment-1193162754 for
>   precedent).

I have just read the RFC, and I find it both well written and compelling 
to accept. +1 from me.

cheers,
Derick

-- 
https://derickrethans.nl | https://xdebug.org | https://dram.io

Author of Xdebug. Like it? Consider supporting me: https://xdebug.org/support
Host of PHP Internals News: https://phpinternals.news

mastodon: @derickr@phpc.social @xdebug@phpc.social
twitter: @derickr and @xdebug

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



[PHP-DEV] Security Audit Priorities

2023-09-25 Thread Derick Rethans
Hi,

The Foundation is organising an external audit/security check of the PHP 
source code. As part of that, we would like to identify the places in 
the PHP source code where checking this will have the most impact.

Typical areas would be where user input can be (automatically read) remotely, 
such as 
our RFC 1867 HTTP header parser. But we are sure there are other 
important areas as well, and we would like your input.

So, if you can suggest an area where doing an external review would have 
high impact, please reply to this email.

cheers,
Derick

-- 
https://derickrethans.nl | https://xdebug.org | https://dram.io

Author of Xdebug. Like it? Consider supporting me: https://xdebug.org/support
Host of PHP Internals News: https://phpinternals.news

mastodon: @derickr@phpc.social @xdebug@phpc.social
twitter: @derickr and @xdebug

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