From 0d9010e1509d8731aab1186981cb54522623426c Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Tue, 26 Mar 2024 20:47:14 -0500 Subject: [PATCH] chore: fix 30% startup time hit from userpassword (#12769) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pbkdf2 is too expensive to run in init, so this change makes it load lazily. I introduced a lazy package that I hope to use more in my `GODEBUG=inittrace=1` adventure. Benchmark results: ``` $ hyperfine "coder --help" "coder-new --help" Benchmark 1: coder --help Time (mean ± σ): 82.1 ms ± 3.8 ms [User: 93.3 ms, System: 30.4 ms] Range (min … max): 72.2 ms … 90.7 ms 35 runs Benchmark 2: coder-new --help Time (mean ± σ): 52.0 ms ± 4.3 ms [User: 62.4 ms, System: 30.8 ms] Range (min … max): 41.9 ms … 62.2 ms 52 runs Summary coder-new --help ran 1.58 ± 0.15 times faster than coder --help ``` --- coderd/userpassword/userpassword.go | 17 ++++++++++++++--- coderd/util/lazy/value.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 coderd/util/lazy/value.go diff --git a/coderd/userpassword/userpassword.go b/coderd/userpassword/userpassword.go index 6f0da0e9aa..fa16a2c89e 100644 --- a/coderd/userpassword/userpassword.go +++ b/coderd/userpassword/userpassword.go @@ -14,6 +14,8 @@ import ( "golang.org/x/crypto/pbkdf2" "golang.org/x/exp/slices" "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/util/lazy" ) var ( @@ -38,8 +40,15 @@ var ( defaultSaltSize = 16 // The simulated hash is used when trying to simulate password checks for - // users that don't exist. - simulatedHash, _ = Hash("hunter2") + // users that don't exist. It's meant to preserve the timing of the hash + // comparison. + simulatedHash = lazy.New(func() string { + h, err := Hash("hunter2") + if err != nil { + panic(err) + } + return h + }) ) // Make password hashing much faster in tests. @@ -65,7 +74,9 @@ func init() { func Compare(hashed string, password string) (bool, error) { // If the hased password provided is empty, simulate comparing a real hash. if hashed == "" { - hashed = simulatedHash + // TODO: this seems ripe for creating a vulnerability where + // hunter2 can log into any account. + hashed = simulatedHash.Load() } if len(hashed) < hashLength { diff --git a/coderd/util/lazy/value.go b/coderd/util/lazy/value.go new file mode 100644 index 0000000000..f53848a0dd --- /dev/null +++ b/coderd/util/lazy/value.go @@ -0,0 +1,28 @@ +// Package lazy provides a lazy value implementation. +// It's useful especially in global variable initialization to avoid +// slowing down the program startup time. +package lazy + +import ( + "sync" + "sync/atomic" +) + +type Value[T any] struct { + once sync.Once + fn func() T + cached atomic.Pointer[T] +} + +func (v *Value[T]) Load() T { + v.once.Do(func() { + vv := v.fn() + v.cached.Store(&vv) + }) + return *v.cached.Load() +} + +// New creates a new lazy value with the given load function. +func New[T any](fn func() T) *Value[T] { + return &Value[T]{fn: fn} +}