From ca1962f6e487e95ddf8573027afbc44f219524f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julio=20L=C3=B3pez?= <1953782+julio-lopez@users.noreply.github.com> Date: Fri, 26 Apr 2024 23:30:56 -0700 Subject: [PATCH] refactor(general): user password hashing and key derivation helpers (#3821) Code movement and simplification, no functional changes. Objectives: - Allow callers specifying the needed key (or hash) size, instead of hard-coding it in the registered PBK derivers. Conceptually, the caller needs to specify the key size, since that is a requirement of the (encryption) algorithm being used in the caller. Now, the code changes here do not result in any functional changes since the key size is always 32 bytes. - Remove a global definition for the default PB key deriver to use. Instead, each of the 3 use case sets the default value. Changes: - `crypto.DeriveKeyFromPassword` now takes a key size. - Adds new constants for the key sizes at the callers. - Removes the global `crypto.MasterKeySize` const. - Removes the global `crypto.DefaultKeyDerivationAlgorithm` const. - Adds const for the default derivation algorithms for each use case. - Adds a const for the salt length in the `internal/user` package, to ensure the same salt length is used in both hash versions. - Unexports various functions, variables and constants in the `internal/crypto` & `internal/user` packages. - Renames various constants for consistency. - Removes unused functions and symbols. - Renames files to be consistent and better reflect the structure of the code. - Adds a couple of tests to ensure the const values are in sync and supported. - Fixes a couple of typos Followups to: - #3725 - #3770 - #3779 - #3799 - #3816 The individual commits show the code transformations to simplify the review of the changes. --- cli/command_repository_connect_server.go | 5 +- cli/command_repository_create.go | 3 +- cli/command_user_add_set.go | 3 +- internal/crypto/key_derivation_nontest.go | 41 ++++--------- internal/crypto/key_derivation_testing.go | 20 +----- .../{pbkdf.go => pb_key_deriver_pbkdf2.go} | 30 ++++----- internal/crypto/pb_key_deriver_scrypt.go | 52 ++++++++++++++++ internal/crypto/scrypt.go | 61 ------------------- internal/servertesting/servertesting.go | 2 +- internal/user/password_hashings.go | 9 +++ internal/user/password_hashings_test.go | 32 ++++++++++ internal/user/user_profile.go | 33 +++++----- ...ile_hash_v1.go => user_profile_pw_hash.go} | 22 ++----- repo/format/format_blob.go | 6 +- repo/format/format_blob_key_derivation.go | 13 ++++ repo/open.go | 7 +-- repo/server_repo_cache_enc_key.go | 15 +++++ .../api_server_repository_test.go | 6 +- .../repository_connect_test.go | 5 +- 19 files changed, 184 insertions(+), 181 deletions(-) rename internal/crypto/{pbkdf.go => pb_key_deriver_pbkdf2.go} (54%) create mode 100644 internal/crypto/pb_key_deriver_scrypt.go delete mode 100644 internal/crypto/scrypt.go create mode 100644 internal/user/password_hashings.go create mode 100644 internal/user/password_hashings_test.go rename internal/user/{user_profile_hash_v1.go => user_profile_pw_hash.go} (61%) create mode 100644 repo/format/format_blob_key_derivation.go create mode 100644 repo/server_repo_cache_enc_key.go diff --git a/cli/command_repository_connect_server.go b/cli/command_repository_connect_server.go index 65a047d00..aeb771fce 100644 --- a/cli/command_repository_connect_server.go +++ b/cli/command_repository_connect_server.go @@ -6,7 +6,6 @@ "github.com/pkg/errors" - "github.com/kopia/kopia/internal/crypto" "github.com/kopia/kopia/internal/passwordpersist" "github.com/kopia/kopia/repo" ) @@ -31,14 +30,14 @@ func (c *commandRepositoryConnectServer) setup(svc advancedAppServices, parent c cmd.Flag("url", "Server URL").Required().StringVar(&c.connectAPIServerURL) cmd.Flag("server-cert-fingerprint", "Server certificate fingerprint").StringVar(&c.connectAPIServerCertFingerprint) //nolint:lll - cmd.Flag("local-cache-key-derivation-algorithm", "Key derivation algorithm used to derive the local cache encryption key").Hidden().Default(repo.DefaultKeyDerivationAlgorithm).EnumVar(&c.connectAPIServerLocalCacheKeyDerivationAlgorithm, crypto.AllowedKeyDerivationAlgorithms()...) + cmd.Flag("local-cache-key-derivation-algorithm", "Key derivation algorithm used to derive the local cache encryption key").Hidden().Default(repo.DefaultServerRepoCacheKeyDerivationAlgorithm).EnumVar(&c.connectAPIServerLocalCacheKeyDerivationAlgorithm, repo.SupportedLocalCacheKeyDerivationAlgorithms()...) cmd.Action(svc.noRepositoryAction(c.run)) } func (c *commandRepositoryConnectServer) run(ctx context.Context) error { localCacheKeyDerivationAlgorithm := c.connectAPIServerLocalCacheKeyDerivationAlgorithm if localCacheKeyDerivationAlgorithm == "" { - localCacheKeyDerivationAlgorithm = repo.DefaultKeyDerivationAlgorithm + localCacheKeyDerivationAlgorithm = repo.DefaultServerRepoCacheKeyDerivationAlgorithm } as := &repo.APIServerInfo{ diff --git a/cli/command_repository_create.go b/cli/command_repository_create.go index f92084f43..bc889ac66 100644 --- a/cli/command_repository_create.go +++ b/cli/command_repository_create.go @@ -7,7 +7,6 @@ "github.com/alecthomas/kingpin/v2" "github.com/pkg/errors" - "github.com/kopia/kopia/internal/crypto" "github.com/kopia/kopia/repo" "github.com/kopia/kopia/repo/blob" "github.com/kopia/kopia/repo/ecc" @@ -54,7 +53,7 @@ func (c *commandRepositoryCreate) setup(svc advancedAppServices, parent commandP cmd.Flag("retention-mode", "Set the blob retention-mode for supported storage backends.").EnumVar(&c.retentionMode, blob.Governance.String(), blob.Compliance.String()) cmd.Flag("retention-period", "Set the blob retention-period for supported storage backends.").DurationVar(&c.retentionPeriod) //nolint:lll - cmd.Flag("format-block-key-derivation-algorithm", "Algorithm to derive the encryption key for the format block from the repository password").Default(crypto.DefaultKeyDerivationAlgorithm).EnumVar(&c.createBlockKeyDerivationAlgorithm, crypto.AllowedKeyDerivationAlgorithms()...) + cmd.Flag("format-block-key-derivation-algorithm", "Algorithm to derive the encryption key for the format block from the repository password").Default(format.DefaultKeyDerivationAlgorithm).EnumVar(&c.createBlockKeyDerivationAlgorithm, format.SupportedFormatBlobKeyDerivationAlgorithms()...) c.co.setup(svc, cmd) c.svc = svc diff --git a/cli/command_user_add_set.go b/cli/command_user_add_set.go index abc7ff2b5..d5f00f448 100644 --- a/cli/command_user_add_set.go +++ b/cli/command_user_add_set.go @@ -7,7 +7,6 @@ "github.com/alecthomas/kingpin/v2" "github.com/pkg/errors" - "github.com/kopia/kopia/internal/crypto" "github.com/kopia/kopia/internal/user" "github.com/kopia/kopia/repo" ) @@ -37,7 +36,7 @@ func (c *commandServerUserAddSet) setup(svc appServices, parent commandParent, i cmd.Flag("ask-password", "Ask for user password").BoolVar(&c.userAskPassword) cmd.Flag("user-password", "Password").StringVar(&c.userSetPassword) cmd.Flag("user-password-hash", "Password hash").StringVar(&c.userSetPasswordHash) - cmd.Flag("user-password-hashing-algorithm", "[Experimental] Password hashing algorithm").Hidden().Default(crypto.DefaultKeyDerivationAlgorithm).EnumVar(&c.userSetPasswordHashAlgorithm, crypto.AllowedKeyDerivationAlgorithms()...) + cmd.Flag("user-password-hashing-algorithm", "[Experimental] Password hashing algorithm").Hidden().Default(user.DefaultPasswordHashingAlgorithm).EnumVar(&c.userSetPasswordHashAlgorithm, user.PasswordHashingAlgorithms()...) cmd.Arg("username", "Username").Required().StringVar(&c.userSetName) cmd.Action(svc.repositoryWriterAction(c.runServerUserAddSet)) diff --git a/internal/crypto/key_derivation_nontest.go b/internal/crypto/key_derivation_nontest.go index ce546eacd..9aee6ad6f 100644 --- a/internal/crypto/key_derivation_nontest.go +++ b/internal/crypto/key_derivation_nontest.go @@ -9,25 +9,16 @@ "github.com/pkg/errors" ) -const ( - // MasterKeyLength describes the length of the master key. - MasterKeyLength = 32 -) - -// DefaultKeyDerivationAlgorithm is the key derivation algorithm for new configurations. -const DefaultKeyDerivationAlgorithm = ScryptAlgorithm - -// KeyDeriver is an interface that contains methods for deriving a key from a password. -type KeyDeriver interface { - DeriveKeyFromPassword(password string, salt []byte) ([]byte, error) - RecommendedSaltLength() int +// passwordBasedKeyDeriver is an interface that contains methods for deriving a key from a password. +type passwordBasedKeyDeriver interface { + deriveKeyFromPassword(password string, salt []byte, keySize int) ([]byte, error) } //nolint:gochecknoglobals -var keyDerivers = map[string]KeyDeriver{} +var keyDerivers = map[string]passwordBasedKeyDeriver{} -// RegisterKeyDerivers registers various key derivation functions. -func RegisterKeyDerivers(name string, keyDeriver KeyDeriver) { +// registerPBKeyDeriver registers a password-based key deriver. +func registerPBKeyDeriver(name string, keyDeriver passwordBasedKeyDeriver) { if _, ok := keyDerivers[name]; ok { panic(fmt.Sprintf("key deriver (%s) is already registered", name)) } @@ -36,28 +27,18 @@ func RegisterKeyDerivers(name string, keyDeriver KeyDeriver) { } // DeriveKeyFromPassword derives encryption key using the provided password and per-repository unique ID. -func DeriveKeyFromPassword(password string, salt []byte, algorithm string) ([]byte, error) { +func DeriveKeyFromPassword(password string, salt []byte, keySize int, algorithm string) ([]byte, error) { kd, ok := keyDerivers[algorithm] if !ok { - return nil, errors.Errorf("unsupported key derivation algorithm: %v, supported algorithms %v", algorithm, AllowedKeyDerivationAlgorithms()) + return nil, errors.Errorf("unsupported key derivation algorithm: %v, supported algorithms %v", algorithm, supportedPBKeyDerivationAlgorithms()) } //nolint:wrapcheck - return kd.DeriveKeyFromPassword(password, salt) + return kd.deriveKeyFromPassword(password, salt, keySize) } -// RecommendedSaltLength returns the recommended salt length of a given key derivation algorithm. -func RecommendedSaltLength(algorithm string) (int, error) { - kd, ok := keyDerivers[algorithm] - if !ok { - return 0, errors.Errorf("failed to get salt length for unsupported key derivation algorithm: %v, supported algorithms %v", algorithm, AllowedKeyDerivationAlgorithms()) - } - - return kd.RecommendedSaltLength(), nil -} - -// AllowedKeyDerivationAlgorithms returns a slice of the allowed key derivation algorithms. -func AllowedKeyDerivationAlgorithms() []string { +// supportedPBKeyDerivationAlgorithms returns a slice of the allowed key derivation algorithms. +func supportedPBKeyDerivationAlgorithms() []string { kdAlgorithms := make([]string, 0, len(keyDerivers)) for k := range keyDerivers { kdAlgorithms = append(kdAlgorithms, k) diff --git a/internal/crypto/key_derivation_testing.go b/internal/crypto/key_derivation_testing.go index 9a30554e0..7107d4f58 100644 --- a/internal/crypto/key_derivation_testing.go +++ b/internal/crypto/key_derivation_testing.go @@ -10,23 +10,13 @@ ) const ( - // MasterKeyLength describes the length of the master key. - MasterKeyLength = 32 - - V1SaltLength = 32 - ScryptAlgorithm = "scrypt-65536-8-1" Pbkdf2Algorithm = "pbkdf2-sha256-600000" - - // DefaultKeyDerivationAlgorithm is the key derivation algorithm for new configurations. - DefaultKeyDerivationAlgorithm = ScryptAlgorithm ) // DeriveKeyFromPassword derives encryption key using the provided password and per-repository unique ID. -func DeriveKeyFromPassword(password string, salt []byte, algorithm string) ([]byte, error) { - const masterKeySize = 32 - +func DeriveKeyFromPassword(password string, salt []byte, keySize int, algorithm string) ([]byte, error) { switch algorithm { case ScryptAlgorithm, Pbkdf2Algorithm: h := sha256.New() @@ -41,11 +31,3 @@ func DeriveKeyFromPassword(password string, salt []byte, algorithm string) ([]by return nil, errors.Errorf("unsupported key algorithm: %v", algorithm) } } - -func RecommendedSaltLength(algorithm string) (int, error) { - return V1SaltLength, nil -} - -func AllowedKeyDerivationAlgorithms() []string { - return []string{DefaultKeyDerivationAlgorithm} -} diff --git a/internal/crypto/pbkdf.go b/internal/crypto/pb_key_deriver_pbkdf2.go similarity index 54% rename from internal/crypto/pbkdf.go rename to internal/crypto/pb_key_deriver_pbkdf2.go index 818b39138..ebcf00d99 100644 --- a/internal/crypto/pbkdf.go +++ b/internal/crypto/pb_key_deriver_pbkdf2.go @@ -11,43 +11,37 @@ ) const ( + // Pbkdf2Algorithm is the key for the pbkdf algorithm. + Pbkdf2Algorithm = "pbkdf2-sha256-600000" + // The NIST recommended minimum size for a salt for pbkdf2 is 16 bytes. // - // TBD: However, a good rule of thumb is to use a salt that is the same size + // However, a good rule of thumb is to use a salt that is the same size // as the output of the hash function. For example, the output of SHA256 // is 256 bits (32 bytes), so the salt should be at least 32 random bytes. // See: https://crackstation.net/hashing-security.htm - minPbkdfSha256SaltSize = 32 // size in bytes == 128 bits + pbkdf2Sha256MinSaltLength = 32 // 256 bits // The NIST recommended iterations for PBKDF2 with SHA256 hash is 600,000. pbkdf2Sha256Iterations = 600_000 - - // Pbkdf2Algorithm is the key for the pbkdf algorithm. - Pbkdf2Algorithm = "pbkdf2-sha256-600000" ) func init() { - RegisterKeyDerivers(Pbkdf2Algorithm, &pbkdf2KeyDeriver{ - iterations: pbkdf2Sha256Iterations, - recommendedSaltLength: minPbkdfSha256SaltSize, - minSaltLength: minPbkdfSha256SaltSize, + registerPBKeyDeriver(Pbkdf2Algorithm, &pbkdf2KeyDeriver{ + iterations: pbkdf2Sha256Iterations, + minSaltLength: pbkdf2Sha256MinSaltLength, }) } type pbkdf2KeyDeriver struct { - iterations int - recommendedSaltLength int - minSaltLength int + iterations int + minSaltLength int } -func (s *pbkdf2KeyDeriver) DeriveKeyFromPassword(password string, salt []byte) ([]byte, error) { +func (s *pbkdf2KeyDeriver) deriveKeyFromPassword(password string, salt []byte, keySize int) ([]byte, error) { if len(salt) < s.minSaltLength { return nil, errors.Errorf("required salt size is atleast %d bytes", s.minSaltLength) } - return pbkdf2.Key([]byte(password), salt, s.iterations, MasterKeyLength, sha256.New), nil -} - -func (s *pbkdf2KeyDeriver) RecommendedSaltLength() int { - return s.recommendedSaltLength + return pbkdf2.Key([]byte(password), salt, s.iterations, keySize, sha256.New), nil } diff --git a/internal/crypto/pb_key_deriver_scrypt.go b/internal/crypto/pb_key_deriver_scrypt.go new file mode 100644 index 000000000..f1a2a6cc6 --- /dev/null +++ b/internal/crypto/pb_key_deriver_scrypt.go @@ -0,0 +1,52 @@ +//go:build !testing +// +build !testing + +package crypto + +import ( + "github.com/pkg/errors" + "golang.org/x/crypto/scrypt" +) + +const ( + // ScryptAlgorithm is the registration name for the scrypt algorithm instance. + ScryptAlgorithm = "scrypt-65536-8-1" + + // The recommended minimum size for a salt to be used for scrypt. + // Currently set to 16 bytes (128 bits). + // + // A good rule of thumb is to use a salt that is the same size + // as the output of the hash function. For example, the output of SHA256 + // is 256 bits (32 bytes), so the salt should be at least 32 random bytes. + // Scrypt uses a SHA256 hash function. + // https://crackstation.net/hashing-security.htm + scryptMinSaltLength = 16 // 128 bits +) + +func init() { + registerPBKeyDeriver(ScryptAlgorithm, &scryptKeyDeriver{ + n: 65536, //nolint:gomnd + r: 8, //nolint:gomnd + p: 1, + minSaltLength: scryptMinSaltLength, + }) +} + +type scryptKeyDeriver struct { + // n scryptCostParameterN is scrypt's CPU/memory cost parameter. + n int + // r scryptCostParameterR is scrypt's work factor. + r int + // p scryptCostParameterP is scrypt's parallelization parameter. + p int + + minSaltLength int +} + +func (s *scryptKeyDeriver) deriveKeyFromPassword(password string, salt []byte, keySize int) ([]byte, error) { + if len(salt) < s.minSaltLength { + return nil, errors.Errorf("required salt size is at least %d bytes", s.minSaltLength) + } + //nolint:wrapcheck + return scrypt.Key([]byte(password), salt, s.n, s.r, s.p, keySize) +} diff --git a/internal/crypto/scrypt.go b/internal/crypto/scrypt.go deleted file mode 100644 index b7e71eb9b..000000000 --- a/internal/crypto/scrypt.go +++ /dev/null @@ -1,61 +0,0 @@ -//go:build !testing -// +build !testing - -package crypto - -import ( - "github.com/pkg/errors" - "golang.org/x/crypto/scrypt" -) - -// The recommended minimum size for a salt to be used for scrypt. -// Currently set to 16 bytes (128 bits). -// -// TBD: A good rule of thumb is to use a salt that is the same size -// as the output of the hash function. For example, the output of SHA256 -// is 256 bits (32 bytes), so the salt should be at least 32 random bytes. -// Scrypt uses a SHA256 hash function. -// https://crackstation.net/hashing-security.htm -const ( - minScryptSha256SaltSize = 16 // size in bytes == 128 bits - - // ScryptAlgorithm is the key for the scrypt algorithm. - ScryptAlgorithm = "scrypt-65536-8-1" - - // Legacy hash version salt length. - V1SaltLength = 32 -) - -func init() { - RegisterKeyDerivers(ScryptAlgorithm, &scryptKeyDeriver{ - n: 65536, //nolint:gomnd - r: 8, //nolint:gomnd - p: 1, - recommendedSaltLength: V1SaltLength, - minSaltLength: minScryptSha256SaltSize, - }) -} - -type scryptKeyDeriver struct { - // n scryptCostParameterN is scrypt's CPU/memory cost parameter. - n int - // r scryptCostParameterR is scrypt's work factor. - r int - // p scryptCostParameterP is scrypt's parallelization parameter. - p int - - recommendedSaltLength int - minSaltLength int -} - -func (s *scryptKeyDeriver) DeriveKeyFromPassword(password string, salt []byte) ([]byte, error) { - if len(salt) < s.minSaltLength { - return nil, errors.Errorf("required salt size is atleast %d bytes", s.minSaltLength) - } - //nolint:wrapcheck - return scrypt.Key([]byte(password), salt, s.n, s.r, s.p, MasterKeyLength) -} - -func (s *scryptKeyDeriver) RecommendedSaltLength() int { - return s.recommendedSaltLength -} diff --git a/internal/servertesting/servertesting.go b/internal/servertesting/servertesting.go index 0fa211835..898e2a1fe 100644 --- a/internal/servertesting/servertesting.go +++ b/internal/servertesting/servertesting.go @@ -79,7 +79,7 @@ func StartServer(t *testing.T, env *repotesting.Environment, tls bool) *repo.API asi.BaseURL = hs.URL } - asi.LocalCacheKeyDerivationAlgorithm = repo.DefaultKeyDerivationAlgorithm + asi.LocalCacheKeyDerivationAlgorithm = repo.DefaultServerRepoCacheKeyDerivationAlgorithm t.Cleanup(hs.Close) diff --git a/internal/user/password_hashings.go b/internal/user/password_hashings.go new file mode 100644 index 000000000..1a9a2aeca --- /dev/null +++ b/internal/user/password_hashings.go @@ -0,0 +1,9 @@ +package user + +// DefaultPasswordHashingAlgorithm is the default password hashing scheme for user profiles. +const DefaultPasswordHashingAlgorithm = scryptHashAlgorithm + +// PasswordHashingAlgorithms returns the supported algorithms for user password hashing. +func PasswordHashingAlgorithms() []string { + return []string{scryptHashAlgorithm, pbkdf2HashAlgorithm} +} diff --git a/internal/user/password_hashings_test.go b/internal/user/password_hashings_test.go new file mode 100644 index 000000000..fcf24d842 --- /dev/null +++ b/internal/user/password_hashings_test.go @@ -0,0 +1,32 @@ +package user + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/kopia/kopia/internal/crypto" +) + +// The password hashing constants defined in this package are used as "lookup +// keys" for the register password-based key derivers in the crypto package. +// This trivial test is a change detector to ensure that the constants defined +// in the user package match those defined in the crypto package. +func TestPasswordHashingConstantMatchCryptoPackage(t *testing.T) { + require.Equal(t, crypto.ScryptAlgorithm, scryptHashAlgorithm) + require.Equal(t, crypto.Pbkdf2Algorithm, pbkdf2HashAlgorithm) +} + +// The passwordHashSaltLength constant defines the salt length used in this +// package for password hashing. This trivial test ensures that this hash length +// meets the minimum requirement for the instantiations of the registered +// password hashers (PB key derivers in the crypto package). +func TestSaltLengthIsSupported(t *testing.T) { + const badPwd = "password" + var salt [passwordHashSaltLength]byte + + for _, h := range PasswordHashingAlgorithms() { + _, err := computePasswordHash(badPwd, salt[:], h) + require.NoError(t, err) + } +} diff --git a/internal/user/user_profile.go b/internal/user/user_profile.go index 5065385ea..be0cedd56 100644 --- a/internal/user/user_profile.go +++ b/internal/user/user_profile.go @@ -3,7 +3,6 @@ import ( "math/rand" - "github.com/kopia/kopia/internal/crypto" "github.com/kopia/kopia/repo/manifest" "github.com/pkg/errors" @@ -12,13 +11,16 @@ const ( // ScryptHashVersion is the version representation of the scrypt algorithm. ScryptHashVersion = 1 - // ScryptHashAlgorithm is the scrypt password hashing algorithm. This must match crypto.ScryptAlgorithm. - ScryptHashAlgorithm = "scrypt-65536-8-1" + // scryptHashAlgorithm is the scrypt password hashing algorithm. This must match crypto.ScryptAlgorithm. + scryptHashAlgorithm = "scrypt-65536-8-1" // Pbkdf2HashVersion is the version representation of the pbkdf2 algorithm. Pbkdf2HashVersion = 2 - // Pbkdf2HashAlgorithm is the pbkdf2 password hashing algorithm. This must match crypto.Pbkdf2Algorithm. - Pbkdf2HashAlgorithm = "pbkdf2-sha256-600000" + // pbkdf2HashAlgorithm is the pbkdf2 password hashing algorithm. This must match crypto.Pbkdf2Algorithm. + pbkdf2HashAlgorithm = "pbkdf2-sha256-600000" + + passwordHashLength = 32 + passwordHashSaltLength = 32 ) // Profile describes information about a single user. @@ -46,17 +48,18 @@ func (p *Profile) IsValidPassword(password string) bool { if p == nil { invalidProfile = true } else { - passwordHashAlgorithm, err = GetPasswordHashAlgorithm(p.PasswordHashVersion) + passwordHashAlgorithm, err = getPasswordHashAlgorithm(p.PasswordHashVersion) if err != nil { invalidProfile = true } } if invalidProfile { - algorithms := crypto.AllowedKeyDerivationAlgorithms() - // if the Username is invalid, return false but use the same amount of time as when we + algorithms := PasswordHashingAlgorithms() + // if the user profile is invalid, either a non-existing user name or password + // hash version, then return false but use the same amount of time as when we // compare against valid user to avoid revealing whether the user account exists. - isValidPassword(password, dummyV1HashThatNeverMatchesAnyPassword, algorithms[rand.Intn(len(algorithms))]) //nolint:gosec + isValidPassword(password, dummyHashThatNeverMatchesAnyPassword, algorithms[rand.Intn(len(algorithms))]) //nolint:gosec return false } @@ -64,13 +67,13 @@ func (p *Profile) IsValidPassword(password string) bool { return isValidPassword(password, p.PasswordHash, passwordHashAlgorithm) } -// GetPasswordHashAlgorithm returns the password hash algorithm given a version. -func GetPasswordHashAlgorithm(passwordHashVersion int) (string, error) { +// getPasswordHashAlgorithm returns the password hash algorithm given a version. +func getPasswordHashAlgorithm(passwordHashVersion int) (string, error) { switch passwordHashVersion { case ScryptHashVersion: - return ScryptHashAlgorithm, nil + return scryptHashAlgorithm, nil case Pbkdf2HashVersion: - return Pbkdf2HashAlgorithm, nil + return pbkdf2HashAlgorithm, nil default: return "", errors.Errorf("unsupported hash version (%d)", passwordHashVersion) } @@ -79,9 +82,9 @@ func GetPasswordHashAlgorithm(passwordHashVersion int) (string, error) { // GetPasswordHashVersion returns the password hash version given an algorithm. func GetPasswordHashVersion(passwordHashAlgorithm string) (int, error) { switch passwordHashAlgorithm { - case ScryptHashAlgorithm: + case scryptHashAlgorithm: return ScryptHashVersion, nil - case Pbkdf2HashAlgorithm: + case pbkdf2HashAlgorithm: return Pbkdf2HashVersion, nil default: return 0, errors.Errorf("unsupported hash algorithm (%s)", passwordHashAlgorithm) diff --git a/internal/user/user_profile_hash_v1.go b/internal/user/user_profile_pw_hash.go similarity index 61% rename from internal/user/user_profile_hash_v1.go rename to internal/user/user_profile_pw_hash.go index 379604e08..c01a8502f 100644 --- a/internal/user/user_profile_hash_v1.go +++ b/internal/user/user_profile_pw_hash.go @@ -11,20 +11,15 @@ ) //nolint:gochecknoglobals -var dummyV1HashThatNeverMatchesAnyPassword = make([]byte, crypto.MasterKeyLength+crypto.V1SaltLength) +var dummyHashThatNeverMatchesAnyPassword = make([]byte, passwordHashSaltLength+passwordHashLength) func (p *Profile) setPassword(password string) error { - passwordHashAlgorithm, err := GetPasswordHashAlgorithm(p.PasswordHashVersion) + passwordHashAlgorithm, err := getPasswordHashAlgorithm(p.PasswordHashVersion) if err != nil { return err } - saltLength, err := crypto.RecommendedSaltLength(passwordHashAlgorithm) - if err != nil { - return errors.Wrap(err, "error getting recommended salt length") - } - - salt := make([]byte, saltLength) + salt := make([]byte, passwordHashSaltLength) if _, err := io.ReadFull(rand.Reader, salt); err != nil { return errors.Wrap(err, "error generating salt") } @@ -35,7 +30,7 @@ func (p *Profile) setPassword(password string) error { } func computePasswordHash(password string, salt []byte, keyDerivationAlgorithm string) ([]byte, error) { - key, err := crypto.DeriveKeyFromPassword(password, salt, keyDerivationAlgorithm) + key, err := crypto.DeriveKeyFromPassword(password, salt, passwordHashLength, keyDerivationAlgorithm) if err != nil { return nil, errors.Wrap(err, "error deriving key from password") } @@ -46,16 +41,11 @@ func computePasswordHash(password string, salt []byte, keyDerivationAlgorithm st } func isValidPassword(password string, hashedPassword []byte, keyDerivationAlgorithm string) bool { - saltLength, err := crypto.RecommendedSaltLength(keyDerivationAlgorithm) - if err != nil { - panic(err) - } - - if len(hashedPassword) != saltLength+crypto.MasterKeyLength { + if len(hashedPassword) != passwordHashSaltLength+passwordHashLength { return false } - salt := hashedPassword[0:saltLength] + salt := hashedPassword[0:passwordHashSaltLength] h, err := computePasswordHash(password, salt, keyDerivationAlgorithm) if err != nil { diff --git a/repo/format/format_blob.go b/repo/format/format_blob.go index 0dcb94351..def049a6b 100644 --- a/repo/format/format_blob.go +++ b/repo/format/format_blob.go @@ -17,9 +17,6 @@ // DefaultFormatEncryption is the identifier of the default format blob encryption algorithm. const DefaultFormatEncryption = "AES256_GCM" -// DefaultKeyDerivationAlgorithm is the key derivation algorithm for new configurations. -const DefaultKeyDerivationAlgorithm = crypto.DefaultKeyDerivationAlgorithm - const ( aes256GcmEncryption = "AES256_GCM" lengthOfRecoverBlockLength = 2 // number of bytes used to store recover block length @@ -27,6 +24,7 @@ maxRecoverChunkLength = 65536 minRecoverableChunkLength = lengthOfRecoverBlockLength + 2 formatBlobChecksumSize = sha256.Size + formatBlobEncryptionKeySize = 32 ) // KopiaRepositoryBlobID is the identifier of a BLOB that describes repository format. @@ -72,7 +70,7 @@ func ParseKopiaRepositoryJSON(b []byte) (*KopiaRepositoryJSON, error) { // DeriveFormatEncryptionKeyFromPassword derives encryption key using the provided password and per-repository unique ID. func (f *KopiaRepositoryJSON) DeriveFormatEncryptionKeyFromPassword(password string) ([]byte, error) { - res, err := crypto.DeriveKeyFromPassword(password, f.UniqueID, f.KeyDerivationAlgorithm) + res, err := crypto.DeriveKeyFromPassword(password, f.UniqueID, formatBlobEncryptionKeySize, f.KeyDerivationAlgorithm) if err != nil { return nil, errors.Wrap(err, "Failed to derive format encryption key") } diff --git a/repo/format/format_blob_key_derivation.go b/repo/format/format_blob_key_derivation.go new file mode 100644 index 000000000..7d0141c97 --- /dev/null +++ b/repo/format/format_blob_key_derivation.go @@ -0,0 +1,13 @@ +package format + +import "github.com/kopia/kopia/internal/crypto" + +// DefaultKeyDerivationAlgorithm is the derivation algorithm for format encryption for new repositories. +const DefaultKeyDerivationAlgorithm = crypto.ScryptAlgorithm + +// SupportedFormatBlobKeyDerivationAlgorithms returns the supported algorithms +// for deriving the local cache encryption key when connecting to a repository +// via the kopia API server. +func SupportedFormatBlobKeyDerivationAlgorithms() []string { + return []string{crypto.ScryptAlgorithm, crypto.Pbkdf2Algorithm} +} diff --git a/repo/open.go b/repo/open.go index 577250ffd..2c55e8001 100644 --- a/repo/open.go +++ b/repo/open.go @@ -58,9 +58,6 @@ // localCacheIntegrityHMACSecretLength length of HMAC secret protecting local cache items. const localCacheIntegrityHMACSecretLength = 16 -// DefaultKeyDerivationAlgorithm is the default key derivation algorithm used to derive a cache encryption key. -const DefaultKeyDerivationAlgorithm = crypto.DefaultKeyDerivationAlgorithm - //nolint:gochecknoglobals var localCacheIntegrityPurpose = []byte("local-cache-integrity") @@ -146,7 +143,9 @@ func getContentCacheOrNil(ctx context.Context, si *APIServerInfo, opt *content.C // derive content cache key from the password & HMAC secret saltWithPurpose := append([]byte("content-cache-protection"), opt.HMACSecret...) - cacheEncryptionKey, err := crypto.DeriveKeyFromPassword(password, saltWithPurpose, si.LocalCacheKeyDerivationAlgorithm) + const cacheEncryptionKeySize = 32 + + cacheEncryptionKey, err := crypto.DeriveKeyFromPassword(password, saltWithPurpose, cacheEncryptionKeySize, si.LocalCacheKeyDerivationAlgorithm) if err != nil { return nil, errors.Wrap(err, "unable to derive cache encryption key from password") } diff --git a/repo/server_repo_cache_enc_key.go b/repo/server_repo_cache_enc_key.go new file mode 100644 index 000000000..1ce062adc --- /dev/null +++ b/repo/server_repo_cache_enc_key.go @@ -0,0 +1,15 @@ +package repo + +import "github.com/kopia/kopia/internal/crypto" + +// DefaultServerRepoCacheKeyDerivationAlgorithm is the default algorithm used to +// derive an encryption key for the local cache when connecting to a repository +// through the kopia API server. +const DefaultServerRepoCacheKeyDerivationAlgorithm = crypto.ScryptAlgorithm + +// SupportedLocalCacheKeyDerivationAlgorithms returns the supported algorithms +// for deriving the local cache encryption key when connecting to a repository +// via the kopia API server. +func SupportedLocalCacheKeyDerivationAlgorithms() []string { + return []string{crypto.ScryptAlgorithm, crypto.Pbkdf2Algorithm} +} diff --git a/tests/end_to_end_test/api_server_repository_test.go b/tests/end_to_end_test/api_server_repository_test.go index 0d2ac1e83..f76592629 100644 --- a/tests/end_to_end_test/api_server_repository_test.go +++ b/tests/end_to_end_test/api_server_repository_test.go @@ -136,7 +136,7 @@ func testAPIServerRepository(t *testing.T, allowRepositoryUsers bool) { rep, err := servertesting.ConnectAndOpenAPIServer(t, ctx2, &repo.APIServerInfo{ BaseURL: sp.BaseURL, TrustedServerCertificateFingerprint: sp.SHA256Fingerprint, - LocalCacheKeyDerivationAlgorithm: repo.DefaultKeyDerivationAlgorithm, + LocalCacheKeyDerivationAlgorithm: repo.DefaultServerRepoCacheKeyDerivationAlgorithm, }, repo.ClientOptions{ Username: "foo", Hostname: "bar", @@ -259,7 +259,7 @@ func testAPIServerRepository(t *testing.T, allowRepositoryUsers bool) { servertesting.ConnectAndOpenAPIServer(t, ctx, &repo.APIServerInfo{ BaseURL: sp.BaseURL, TrustedServerCertificateFingerprint: sp.SHA256Fingerprint, - LocalCacheKeyDerivationAlgorithm: repo.DefaultKeyDerivationAlgorithm, + LocalCacheKeyDerivationAlgorithm: repo.DefaultServerRepoCacheKeyDerivationAlgorithm, }, repo.ClientOptions{ Username: "foo", Hostname: "bar", @@ -330,7 +330,7 @@ func TestFindManifestsPaginationOverGRPC(t *testing.T) { rep, err := servertesting.ConnectAndOpenAPIServer(t, ctx, &repo.APIServerInfo{ BaseURL: sp.BaseURL, TrustedServerCertificateFingerprint: sp.SHA256Fingerprint, - LocalCacheKeyDerivationAlgorithm: repo.DefaultKeyDerivationAlgorithm, + LocalCacheKeyDerivationAlgorithm: repo.DefaultServerRepoCacheKeyDerivationAlgorithm, }, repo.ClientOptions{ Username: "foo", Hostname: "bar", diff --git a/tests/end_to_end_test/repository_connect_test.go b/tests/end_to_end_test/repository_connect_test.go index d70b321c8..ff218c070 100644 --- a/tests/end_to_end_test/repository_connect_test.go +++ b/tests/end_to_end_test/repository_connect_test.go @@ -10,7 +10,6 @@ "github.com/google/uuid" "github.com/stretchr/testify/require" - "github.com/kopia/kopia/internal/crypto" "github.com/kopia/kopia/internal/testutil" "github.com/kopia/kopia/repo/format" "github.com/kopia/kopia/tests/testenv" @@ -110,7 +109,7 @@ func TestReconnectUsingToken(t *testing.T) { func TestRepoConnectKeyDerivationAlgorithm(t *testing.T) { t.Parallel() - for _, algorithm := range crypto.AllowedKeyDerivationAlgorithms() { + for _, algorithm := range format.SupportedFormatBlobKeyDerivationAlgorithms() { runner := testenv.NewInProcRunner(t) e := testenv.NewCLITest(t, testenv.RepoFormatNotImportant, runner) @@ -133,7 +132,7 @@ func TestRepoConnectBadKeyDerivationAlgorithm(t *testing.T) { runner := testenv.NewInProcRunner(t) e := testenv.NewCLITest(t, testenv.RepoFormatNotImportant, runner) - e.RunAndExpectSuccess(t, "repo", "create", "filesystem", "--path", e.RepoDir, "--format-block-key-derivation-algorithm", crypto.DefaultKeyDerivationAlgorithm) + e.RunAndExpectSuccess(t, "repo", "create", "filesystem", "--path", e.RepoDir, "--format-block-key-derivation-algorithm", format.DefaultKeyDerivationAlgorithm) e.RunAndExpectSuccess(t, "repo", "disconnect") kopiaRepoPath := filepath.Join(e.RepoDir, "kopia.repository.f")