proposal: x/crypto/bcrypt: add WithContext-style functions

[hf]

(This is my first time contributing to Go, so please let me know if I'm doing something wrong.)

What's the proposed change?

golang.org/x/crypto/bcrypt has two main functions:

• GenerateFromPassword([]byte, int)([]byte, error)
• CompareHashAndPassword([]byte, []byte) error

This proposal retains these functions, and adds two more:

• GenerateFromPasswordWithContext(context.Context, []byte, int)([]byte, error)
• CompareHashAndPasswordWithContext(context.Context, []byte, []byte) error

This is a common Go pattern for functions that obey semantics of a provided context. This context can be used to terminate password hashing early.

Why is this necessary?

Password hashing functions like bcrypt are incredibly CPU intensive. For example, hashing a password at the current bcrypt.DefaultCost of 10 on my Apple m1 chip takes about 75ms. Furthermore, the implementation of the algorithm is non-cooperative. Go does not preempt Goroutines. Thus, once a few of these Goroutines are scheduled on the majority of threads, all the other Goroutines will be penalized and will incur a latency of 75ms.

By providing this extended API, the algorithm can be changed to cooperate with the rest of the Goroutines by calling runtime.Gosched() periodically. The provided context can be thus used to check for cancellation or timeout, and terminate hashing early.

Who is most likely to benefit?

Any web server that does some sort of password hashing, typically authentication servers, suffers from unpredictable (and sometimes catastrophic) latencies due to the non-cooperative nature of the current bcrypt implementation. By giving access to this API which uses a cooperative algorithm, these web servers will no longer experience unpredictable latencies. In fact, non-password hashing workloads will be given their fair share of CPU time.

Who is going to implement this?

I've already done this in golang/crypto PR #236.

Further Notes

I've seen issues with this in previous experience. I'm also on the Supabase Auth team.

I wrote a large note some time ago. This isn't very well reviewed, so take all the math with a grain of salt, but I believe it illustrates the problem and solutions.

[FiloSottile]

Shouldn't async preemption handle this, with no need for Gosched?

[hf]

Shouldn't async preemption handle this, with no need for Gosched?

It should, though through some experience I've found the 10ms preemption too generous and not consistent. Not being able to cancel the workload (for example to time-out all password hashing at 2s) which in turn relieves CPU resources is the main issue that can't be dealt with outside of the current package.

I'm open to removing runtime.Gosched(). In fact it may just be that the select to check if the context is done is enough to schedule other Goroutines.