-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
argon2: Add helpers for running the KDF remotely #328
Merged
chrisccoulson
merged 43 commits into
canonical:master
from
chrisccoulson:remote-argon2-kdf
Jan 23, 2025
Merged
Changes from 9 commits
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
017287b
argon2: Add helpers for running the KDF remotely
chrisccoulson bb46af8
argon2: address some review comments
chrisccoulson 93292cd
argon2: add in-process test for WaitForAndRunArgon2OutOfProcessRequest
chrisccoulson 844021f
[WIP] address review comments
chrisccoulson fd31de5
Merge branch 'master' into remote-argon2-kdf
chrisccoulson fe291c5
Argon2 remoting cleanups
chrisccoulson a097997
Fix a couplke of data races
chrisccoulson ef2c8e4
argon2: increase timeout for github
chrisccoulson dce2200
reduce memory usage of tests that fail in github to see if they will …
chrisccoulson 8f57b45
argon2: add a missing comment
chrisccoulson 3c64825
argon2: replace the use of sync.WaitGroup with a channel
chrisccoulson 35fd6f1
argon2: reduce memory usage for github
chrisccoulson 8b5c902
reduce memory usage in a couple more argon2 tests
chrisccoulson 9d6d0e4
Change the backoff time to 100ms and refactor the code to make the te…
chrisccoulson d1f97b3
close the channel to unblock the receiving routine
chrisccoulson fabe880
complete a doc comment
chrisccoulson 0c869e8
Update x/crypto
chrisccoulson e18c47b
Don't use json.Decoder.DisallowUnknownFields
chrisccoulson 513d782
Fix various race conditions with the Argon2 remoting
chrisccoulson b803ecf
Close both ends of the request channel on error
chrisccoulson d38ab1c
Remove a bit of unnecessary test code
chrisccoulson 093b6ee
Test WaitForAndRunArgon2OutOfProcessRequest closes its end of the res…
chrisccoulson c8d71ba
Improve the argon2 remoting system-lock tests
chrisccoulson a1e15a7
Make use of strings.Builder
chrisccoulson 9c9d5ca
remove some unnecessary channel usage
chrisccoulson 327f235
Try running some tests with a memory consumption of 2GiB again
chrisccoulson afca4f4
Simplify the HMACArgon2OutOfProcessWatchdogMonitor function
chrisccoulson ba7c52f
Move all tests that run Argon2 to the expensive suites
chrisccoulson 6270d98
Add some debugging fmt.Printf statements
chrisccoulson 88d8658
Reenable the watchdog timeout again
chrisccoulson d6cabb3
try disabling the GC during the failing test
chrisccoulson 4cdec5f
remove debug fmt.Printfs
chrisccoulson 57a5694
Ensure WaitForAndRunArgon2OutOfProcessRequest will only process a sin…
chrisccoulson 586bc6b
argon2: Add a comment about not launching sub-processes
chrisccoulson 7a87d2b
argon2: Ensure a panic in the KDF is handled correctly
chrisccoulson 625db26
internal/paths: Use a snapd agnostic path for the Argon2 lock file
chrisccoulson 8883d04
argon2: Add a note explaining why we don't validate the MemoryKiB cos…
chrisccoulson e094dfa
argon2: Don't use type aliases for the watchdog function templates
chrisccoulson a264de8
argon2: Pass O_NOFOLLOW when opening the lock file
chrisccoulson 0e6e386
argon2: make sure the lock file is a regular file
chrisccoulson 71a9050
Merge branch 'master' into remote-argon2-kdf
chrisccoulson d11584e
Merge branch 'master' into remote-argon2-kdf
chrisccoulson 77dcb36
internal/paths: update the Argon2 lock filename
chrisccoulson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
./run_argon2 | ||
vendor/*/ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -33,19 +33,26 @@ import ( | |||||
) | ||||||
|
||||||
var ( | ||||||
argon2Mu sync.Mutex | ||||||
argon2Impl Argon2KDF = nullArgon2KDFImpl{} | ||||||
argon2Mu sync.Mutex // Protects access to argon2Impl | ||||||
argon2Impl Argon2KDF = nullArgon2KDFImpl{} // The Argon2KDF implementation used by functions in this package | ||||||
|
||||||
runtimeNumCPU = runtime.NumCPU | ||||||
) | ||||||
|
||||||
// SetArgon2KDF sets the KDF implementation for Argon2. The default here is | ||||||
// the null implementation which returns an error, so this will need to be | ||||||
// configured explicitly in order to use Argon2. | ||||||
// SetArgon2KDF sets the KDF implementation for Argon2 use from within secboot. | ||||||
// The default here is a null implementation which returns an error, so this | ||||||
// will need to be configured explicitly in order to be able to use Argon2 from | ||||||
// within secboot. | ||||||
// | ||||||
// Passing nil will configure the null implementation as well. | ||||||
// | ||||||
// This returns the currently set implementation. | ||||||
// This function returns the previously configured Argon2KDF instance. | ||||||
// | ||||||
// This exists to facilitate running Argon2 operations in short-lived helper | ||||||
// processes (see [InProcessArgon2KDF]), because Argon2 doesn't interact very | ||||||
// well with Go's garbage collector, and is an algorithm that is only really | ||||||
// suited to languages / runtimes with explicit memory allocation and | ||||||
// de-allocation primitves. | ||||||
func SetArgon2KDF(kdf Argon2KDF) Argon2KDF { | ||||||
argon2Mu.Lock() | ||||||
defer argon2Mu.Unlock() | ||||||
|
@@ -59,13 +66,17 @@ func SetArgon2KDF(kdf Argon2KDF) Argon2KDF { | |||||
return orig | ||||||
} | ||||||
|
||||||
// argon2KDF returns the global [Argon2KDF] implementation set for this process. This | ||||||
// can be set via calls to [SetArgon2KDF]. | ||||||
func argon2KDF() Argon2KDF { | ||||||
argon2Mu.Lock() | ||||||
defer argon2Mu.Unlock() | ||||||
return argon2Impl | ||||||
} | ||||||
|
||||||
// Argon2Mode describes the Argon2 mode to use. | ||||||
// Argon2Mode describes the Argon2 mode to use. Note that the | ||||||
// fully data-dependent mode is not supported because the underlying | ||||||
// argon2 implementation lacks support for it. | ||||||
type Argon2Mode = argon2.Mode | ||||||
|
||||||
const ( | ||||||
|
@@ -119,14 +130,15 @@ func (o *Argon2Options) kdfParams(keyLen uint32) (*kdfParams, error) { | |||||
|
||||||
mode := o.Mode | ||||||
if mode == Argon2Default { | ||||||
// | ||||||
mode = Argon2id | ||||||
} | ||||||
|
||||||
switch { | ||||||
case o.ForceIterations > 0: | ||||||
// The non-benchmarked path. Ensure that ForceIterations | ||||||
// and MemoryKiB fit into an int32 so that it always fits | ||||||
// into an int | ||||||
// into an int, because the retuned kdfParams uses ints. | ||||||
switch { | ||||||
case o.ForceIterations > math.MaxInt32: | ||||||
return nil, fmt.Errorf("invalid iterations count %d", o.ForceIterations) | ||||||
|
@@ -157,12 +169,15 @@ func (o *Argon2Options) kdfParams(keyLen uint32) (*kdfParams, error) { | |||||
|
||||||
return params, nil | ||||||
default: | ||||||
// The benchmarked path, where we determing what cost paramters to | ||||||
// use in order to obtain the desired execution time. | ||||||
benchmarkParams := &argon2.BenchmarkParams{ | ||||||
MaxMemoryCostKiB: 1 * 1024 * 1024, // the default maximum memory cost is 1GiB. | ||||||
TargetDuration: 2 * time.Second, // the default target duration is 2s. | ||||||
} | ||||||
|
||||||
if o.MemoryKiB != 0 { | ||||||
// The memory cost has been spcified expli | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've fixed this. |
||||||
benchmarkParams.MaxMemoryCostKiB = o.MemoryKiB // this is capped to 4GiB by internal/argon2. | ||||||
} | ||||||
if o.TargetDuration != 0 { | ||||||
|
@@ -172,6 +187,7 @@ func (o *Argon2Options) kdfParams(keyLen uint32) (*kdfParams, error) { | |||||
benchmarkParams.Threads = o.Parallel // this is capped to 4 by internal/argon2. | ||||||
} | ||||||
|
||||||
// Run the benchmark, which relies on the global Argon2KDF implementation. | ||||||
params, err := argon2.Benchmark(benchmarkParams, func(params *argon2.CostParams) (time.Duration, error) { | ||||||
return argon2KDF().Time(mode, &Argon2CostParams{ | ||||||
Time: params.Time, | ||||||
|
@@ -217,7 +233,9 @@ func (p *Argon2CostParams) internalParams() *argon2.CostParams { | |||||
} | ||||||
|
||||||
// Argon2KDF is an interface to abstract use of the Argon2 KDF to make it possible | ||||||
// to delegate execution to a short-lived utility process where required. | ||||||
// to delegate execution to a short-lived handler process where required. See | ||||||
// [SetArgon2KDF] and [InProcessArgon2KDF]. Implementations should be thread-safe | ||||||
// (ie, they should be able to handle calls from different goroutines). | ||||||
type Argon2KDF interface { | ||||||
// Derive derives a key of the specified length in bytes, from the supplied | ||||||
// passphrase and salt and using the supplied mode and cost parameters. | ||||||
|
@@ -246,10 +264,27 @@ func (_ inProcessArgon2KDFImpl) Time(mode Argon2Mode, params *Argon2CostParams) | |||||
return argon2.KeyDuration(argon2.Mode(mode), params.internalParams()) | ||||||
} | ||||||
|
||||||
// InProcessArgon2KDF is the in-process implementation of the Argon2 KDF. This | ||||||
// shouldn't be used in long-lived system processes - these processes should | ||||||
// instead provide their own KDF implementation which delegates to a short-lived | ||||||
// utility process which will use the in-process implementation. | ||||||
// InProcessArgon2KDF is the in-process implementation of the Argon2 KDF. | ||||||
// | ||||||
// This shouldn't be used in long-lived system processes. As Argon2 intentionally | ||||||
// allocates a lot of memory and go is garbage collected, it may be some time before | ||||||
// the large amounts of memory it allocates are freed and made available to other code | ||||||
// or other processes on the system. Consecutive calls can rapidly result in the | ||||||
// application being unable to allocate more memory, and even worse, may trigger the | ||||||
// kernel's OOM killer. Whilst implementations can call [runtime.GC] between calls, | ||||||
// go's sweep implementation stops the world, which makes interaction with goroutines | ||||||
// and the scheduler poor, and will likely result in noticeable periods of | ||||||
// unresponsiveness. Rather than using this directly, it's better to pass requests to | ||||||
// a short-lived helper process where this can be used, and let the kernel deal with | ||||||
// reclaiming memory when the short-lived process exits instead. | ||||||
// | ||||||
// This package provides APIs to support this architecture already - | ||||||
// [NewOutOfProcessArgon2KDF] for the parent side, and [WaitForAndRunArgon2OutOfProcessRequest] | ||||||
// for the remote side, which runs in a short-lived process. In order to save storage | ||||||
// space that would be consumed by another go binary, it is reasonable that the parent | ||||||
// side (the one that calls [SetArgon2KDF]) and the remote side (which calls | ||||||
// [WaitForAndRunArgon2OutOfProcessRequest]) could live in the same executable that | ||||||
// is invoked with different arguments depending on which function is required. | ||||||
var InProcessArgon2KDF = inProcessArgon2KDFImpl{} | ||||||
|
||||||
type nullArgon2KDFImpl struct{} | ||||||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forgot the comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did. I've added a comment now.