Skip to content
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

add support for PBKDF2 for passphrases #288

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

chrisccoulson
Copy link
Collaborator

@chrisccoulson chrisccoulson commented Mar 9, 2024

Passphrase support currently hardcodes the use of Argon2. This adds
support for specifying PBKDF2, for use in environments where FIPS140
compliance is required.

PBKDF2 will also be a requirement for PIN support, where Argon2
will not be an option.

@chrisccoulson chrisccoulson force-pushed the add-pbkdf2-support branch 10 times, most recently from 7decb61 to 9a2d46c Compare March 16, 2024 01:14
@chrisccoulson chrisccoulson force-pushed the add-pbkdf2-support branch 4 times, most recently from 1746a1d to 0cab4ed Compare March 19, 2024 22:16
Passphrase support currently hardcodes the use of Argon2. This adds
support for specifying PBKDF2, for use in environments where FIPS140
compliance is required.
Copy link
Collaborator

@pedronis pedronis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did a pass, couple of remarks/questions

Comment on lines +39 to +45
var timeExecution = func(params *Params) time.Duration {
start := time.Now()
if _, err := Key(benchmarkPassword, benchmarkSalt, params, uint(params.HashAlg.Size())); err != nil {
panic(err)
}
return time.Now().Sub(start)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this bit of code in itself is not reached by tests

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a test that doesn't mock this now.

Comment on lines +139 to +142
case !params.HashAlg.Available():
return nil, errors.New("unavailable digest algorithm")
case keyLen > math.MaxInt:
return nil, errors.New("invalid key length")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't there anything to check between keyLen and HashAlg?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There isn't really any need to do that - one could use any hash algorithm to produce a key of any length, eg, if you request a key of 64 bytes with SHA-256 as the digest and 10000 iterations, it runs 10000 iterations twice to produce 2 blocks of 32-bytes and there's nothing in the design of PBKDF2 that disallows this.

Copy link
Collaborator

@pedronis pedronis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@chrisccoulson chrisccoulson merged commit e57c85d into canonical:master Sep 19, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants