-
Notifications
You must be signed in to change notification settings - Fork 15
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
add support for PBKDF2 for passphrases #288
Conversation
7decb61
to
9a2d46c
Compare
1746a1d
to
0cab4ed
Compare
0cab4ed
to
fe8b71d
Compare
Passphrase support currently hardcodes the use of Argon2. This adds support for specifying PBKDF2, for use in environments where FIPS140 compliance is required.
6d89a46
to
1f3ff7a
Compare
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.
did a pass, couple of remarks/questions
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) | ||
} |
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.
this bit of code in itself is not reached by tests
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've added a test that doesn't mock this now.
case !params.HashAlg.Available(): | ||
return nil, errors.New("unavailable digest algorithm") | ||
case keyLen > math.MaxInt: | ||
return nil, errors.New("invalid key length") |
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.
isn't there anything to check between keyLen and HashAlg?
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.
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.
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.
thanks
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.