Add pbkdf2-sha256 as a FIPS 140 approved password hashing algorithm - #2295
Closed
iAbhishek91 wants to merge 1 commit into
Closed
iAbhishek91 wants to merge 1 commit into
iAbhishek91 wants to merge 1 commit into
Conversation
SFTPGo currently only creates password hashes using bcrypt and argon2id, neither of which is a NIST/FIPS 140 approved algorithm. This blocks building FIPS-compliant SFTPGo images (see the FIPS build discussion in drakkan#1272). SFTPGo already implements PBKDF2 verification for migrating passwords from other systems (comparePbkdf2PasswordAndHash), so this change reuses that code and adds pbkdf2-sha256 (PBKDF2-HMAC-SHA256, NIST SP 800-132) as a third selectable value for password_hashing.algo, alongside a pbkdf2_options.iterations setting (default 600000, minimum enforced 10000). Admin, API key and share password hashing/verification, which duplicated the bcrypt/argon2id branching, now call the shared hashPlainPassword helper so all four credential types support the new algorithm consistently.
|
|
Owner
|
see #2294 (comment). Thanks anyway |
5 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Closes #2294.
password_hashing.algocurrently only supportsbcryptandargon2id, neither of which is a NIST/FIPS 140 approved algorithm.This prevents building a FIPS-compliant SFTPGo image, since there's
no FIPS-approved algorithm to select even with a FIPS-enabled Go
toolchain.
This PR adds
pbkdf2-sha256(PBKDF2-HMAC-SHA256, per NIST SP 800-132)as a third selectable value for
password_hashing.algo, plus a newpassword_hashing.pbkdf2_options.iterationssetting (default600000, minimum enforced10000per NIST SP 800-132).SFTPGo already implements PBKDF2 verification, used for migrating
password hashes from other systems (
comparePbkdf2PasswordAndHash,supporting
$pbkdf2-b64salt-sha256$,$pbkdf2-sha256$,$pbkdf2-sha512$,$pbkdf2-sha1$). This change reuses that existingverification path for the new
pbkdf2-sha256creation option, insteadof adding a parallel implementation.
Changes
internal/dataprovider/dataprovider.go: addHashingAlgoPBKDF2SHA256constant and
Pbkdf2Optionsconfig struct;hashPlainPasswordnowcreates a
$pbkdf2-b64salt-sha256$<iterations>$<salt>$<hash>hashwhen selected, using
util.GenerateRandomBytesfor the salt and theexisting
pbkdf2SHA256B64SaltPrefixformat so it round-trips throughthe existing
comparePbkdf2PasswordAndHashverifier unchanged;initializeHashingAlgovalidates the configured iteration count.internal/config/config.go: defaultpbkdf2_options.iterationsto600000and register the corresponding viper default.internal/dataprovider/admin.go,apikey.go,share.go: theseduplicated the bcrypt/argon2id branching for hashing admin passwords,
API keys and share passwords. Replaced with a call to the shared
hashPlainPasswordhelper so all four credential types pick uppbkdf2-sha256consistently, and added the matchingcomparePbkdf2PasswordAndHashbranch to each type's verificationpath (previously only
dataprovider.go's user-password verificationhandled pbkdf2 hashes; admin/API key/share verification assumed
bcrypt-or-argon2id).
This is intentionally scoped to just the hashing algorithm; it doesn't
touch transport/TLS crypto, which is a separate FIPS concern already
addressed by building with a FIPS-enabled Go toolchain
(
GOEXPERIMENT=boringcrypto), as noted in #1272.Test plan
go build ./...go vet ./internal/dataprovider/... ./internal/config/...gofmt -lon all changed files (no output)pbkdf2-sha256(correct password matches, wrong password is rejected, hash has
the expected
$pbkdf2-b64salt-sha256$format) with a local test;removed before submitting since this package has no existing
test file to extend
dataprovider_test.goshould be introduced for this, since nonecurrently exists in this package