feat(general): user profile hashing version to algorithm translation (#3816)

Reverts to using the `PasswordHashVersion` in the user profile.
Adds a simple mechanism for translating between password hash
version and the corresponding password hashing algorithm (key
derivation algorithm).

cli/command_user_add_set.go

@@ -13,11 +13,11 @@
 )
 
 type commandServerUserAddSet struct {
-	userAskPassword        bool
-	userSetName            string
-	userSetPassword        string
-	keyDerivationAlgorithm string
-	userSetPasswordHash    string
+	userAskPassword              bool
+	userSetName                  string
+	userSetPassword              string
+	userSetPasswordHashAlgorithm string
+	userSetPasswordHash          string
 
 	isNew bool // true == 'add', false == 'update'
 	out   textOutput
@@ -37,7 +37,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.keyDerivationAlgorithm, crypto.AllowedKeyDerivationAlgorithms()...)
+	cmd.Flag("user-password-hashing-algorithm", "[Experimental] Password hashing algorithm").Hidden().Default(crypto.DefaultKeyDerivationAlgorithm).EnumVar(&c.userSetPasswordHashAlgorithm, crypto.AllowedKeyDerivationAlgorithms()...)
 	cmd.Arg("username", "Username").Required().StringVar(&c.userSetName)
 	cmd.Action(svc.repositoryWriterAction(c.runServerUserAddSet))
 
@@ -53,9 +53,14 @@ func (c *commandServerUserAddSet) getExistingOrNewUserProfile(ctx context.Contex
 			return nil, errors.Errorf("user %q already exists", username)
 
 		case errors.Is(err, user.ErrUserNotFound):
+			passwordHashVersion, err := user.GetPasswordHashVersion(c.userSetPasswordHashAlgorithm)
+			if err != nil {
+				return nil, errors.Wrap(err, "failed to get password hash version")
+			}
+
 			return &user.Profile{
-				Username:               username,
-				KeyDerivationAlgorithm: c.keyDerivationAlgorithm,
+				Username:            username,
+				PasswordHashVersion: passwordHashVersion,
 			}, nil
 		}
 	}

internal/auth/authn_repo_test.go

@@ -7,7 +7,6 @@
 	"github.com/stretchr/testify/require"
 
 	"github.com/kopia/kopia/internal/auth"
-	"github.com/kopia/kopia/internal/crypto"
 	"github.com/kopia/kopia/internal/repotesting"
 	"github.com/kopia/kopia/internal/user"
 	"github.com/kopia/kopia/repo"
@@ -26,14 +25,14 @@ func(ctx context.Context, w repo.RepositoryWriter) error {
 				{
 					profile: &user.Profile{
 						Username:            "user1@host1",
-						PasswordHashVersion: crypto.HashVersion1,
+						PasswordHashVersion: user.ScryptHashVersion,
 					},
 					password: "password1",
 				},
 				{
 					profile: &user.Profile{
-						Username:               "user2@host2",
-						KeyDerivationAlgorithm: crypto.ScryptAlgorithm,
+						Username:            "user2@host2",
+						PasswordHashVersion: user.ScryptHashVersion,
 					},
 					password: "password2",
 				},
@@ -45,8 +44,8 @@ func(ctx context.Context, w repo.RepositoryWriter) error {
 				},
 				{
 					profile: &user.Profile{
-						Username:               "user4@host4",
-						KeyDerivationAlgorithm: crypto.Pbkdf2Algorithm,
+						Username:            "user4@host4",
+						PasswordHashVersion: user.Pbkdf2HashVersion,
 					},
 					password: "password4",
 				},

internal/crypto/key_derivation_testing.go

@@ -10,16 +10,17 @@
 )
 
 const (
-	// DefaultKeyDerivationAlgorithm is the key derivation algorithm for new configurations.
-	DefaultKeyDerivationAlgorithm = "testing-only-insecure"
-
 	// MasterKeyLength describes the length of the master key.
 	MasterKeyLength = 32
 
-	V1SaltLength    = 32
-	HashVersion1    = 1 // this translates to Scrypt KeyDerivationAlgorithm
+	V1SaltLength = 32
+
 	ScryptAlgorithm = "scrypt-65536-8-1"
-	Pbkdf2Algorithm = "pbkdf2"
+
+	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.
@@ -27,7 +28,7 @@ func DeriveKeyFromPassword(password string, salt []byte, algorithm string) ([]by
 	const masterKeySize = 32
 
 	switch algorithm {
-	case DefaultKeyDerivationAlgorithm, ScryptAlgorithm, Pbkdf2Algorithm:
+	case ScryptAlgorithm, Pbkdf2Algorithm:
 		h := sha256.New()
 		// Adjust password so that we get a different key for each algorithm
 		if _, err := h.Write([]byte(password + algorithm)); err != nil {

internal/crypto/scrypt.go

@@ -24,10 +24,6 @@
 
 	// Legacy hash version salt length.
 	V1SaltLength = 32
-
-	// Legacy hash version system translates to KeyDerivationAlgorithm.
-	HashVersion1 = 1 // this translates to Scrypt KeyDerivationAlgorithm
-
 )
 
 func init() {

internal/user/user_profile.go

@@ -5,16 +5,29 @@
 
 	"github.com/kopia/kopia/internal/crypto"
 	"github.com/kopia/kopia/repo/manifest"
+
+	"github.com/pkg/errors"
+)
+
+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"
+
+	// 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"
 )
 
 // Profile describes information about a single user.
 type Profile struct {
 	ManifestID manifest.ID `json:"-"`
 
-	Username               string `json:"username"`
-	PasswordHashVersion    int    `json:"passwordHashVersion,omitempty"` // indicates how password is hashed, deprecated in favor of KeyDerivationAlgorithm
-	KeyDerivationAlgorithm string `json:"keyDerivationAlgorithm,omitempty"`
-	PasswordHash           []byte `json:"passwordHash"`
+	Username            string `json:"username"`
+	PasswordHashVersion int    `json:"passwordHashVersion,omitempty"`
+	PasswordHash        []byte `json:"passwordHash"`
 }
 
 // SetPassword changes the password for a user profile.
@@ -24,23 +37,53 @@ func (p *Profile) SetPassword(password string) error {
 
 // IsValidPassword determines whether the password is valid for a given user.
 func (p *Profile) IsValidPassword(password string) bool {
+	var invalidProfile bool
+
+	var passwordHashAlgorithm string
+
+	var err error
+
 	if p == nil {
+		invalidProfile = true
+	} else {
+		passwordHashAlgorithm, err = GetPasswordHashAlgorithm(p.PasswordHashVersion)
+		if err != nil {
+			invalidProfile = true
+		}
+	}
+
+	if invalidProfile {
 		algorithms := crypto.AllowedKeyDerivationAlgorithms()
-		// if the user is invalid, return false but use the same amount of time as when we
+		// if the Username is invalid, 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
 
 		return false
 	}
 
-	// Legacy case where password hash version is set
-	if p.PasswordHashVersion == crypto.HashVersion1 {
-		return isValidPassword(password, p.PasswordHash, crypto.ScryptAlgorithm)
-	}
-
-	if p.KeyDerivationAlgorithm != "" {
-		return isValidPassword(password, p.PasswordHash, p.KeyDerivationAlgorithm)
-	}
-
-	return false
+	return isValidPassword(password, p.PasswordHash, passwordHashAlgorithm)
+}
+
+// GetPasswordHashAlgorithm returns the password hash algorithm given a version.
+func GetPasswordHashAlgorithm(passwordHashVersion int) (string, error) {
+	switch passwordHashVersion {
+	case ScryptHashVersion:
+		return ScryptHashAlgorithm, nil
+	case Pbkdf2HashVersion:
+		return Pbkdf2HashAlgorithm, nil
+	default:
+		return "", errors.Errorf("unsupported hash version (%d)", passwordHashVersion)
+	}
+}
+
+// GetPasswordHashVersion returns the password hash version given an algorithm.
+func GetPasswordHashVersion(passwordHashAlgorithm string) (int, error) {
+	switch passwordHashAlgorithm {
+	case ScryptHashAlgorithm:
+		return ScryptHashVersion, nil
+	case Pbkdf2HashAlgorithm:
+		return Pbkdf2HashVersion, nil
+	default:
+		return 0, errors.Errorf("unsupported hash algorithm (%s)", passwordHashAlgorithm)
+	}
 }

internal/user/user_profile_hash_v1.go

@@ -14,18 +14,12 @@
 var dummyV1HashThatNeverMatchesAnyPassword = make([]byte, crypto.MasterKeyLength+crypto.V1SaltLength)
 
 func (p *Profile) setPassword(password string) error {
-	keyDerivationAlgorithm := p.KeyDerivationAlgorithm
-	if keyDerivationAlgorithm == "" {
-		if p.PasswordHashVersion == 0 {
-			return errors.New("key derivation algorithm and password hash version not set")
-		}
-		// Setup to handle legacy hashVersion.
-		if p.PasswordHashVersion == crypto.HashVersion1 {
-			keyDerivationAlgorithm = crypto.ScryptAlgorithm
-		}
+	passwordHashAlgorithm, err := GetPasswordHashAlgorithm(p.PasswordHashVersion)
+	if err != nil {
+		return err
 	}
 
-	saltLength, err := crypto.RecommendedSaltLength(keyDerivationAlgorithm)
+	saltLength, err := crypto.RecommendedSaltLength(passwordHashAlgorithm)
 	if err != nil {
 		return errors.Wrap(err, "error getting recommended salt length")
 	}
@@ -35,7 +29,7 @@ func (p *Profile) setPassword(password string) error {
 		return errors.Wrap(err, "error generating salt")
 	}
 
-	p.PasswordHash, err = computePasswordHash(password, salt, keyDerivationAlgorithm)
+	p.PasswordHash, err = computePasswordHash(password, salt, passwordHashAlgorithm)
 
 	return err
 }

internal/user/user_profile_test.go

@@ -3,41 +3,12 @@
 import (
 	"testing"
 
-	"github.com/kopia/kopia/internal/crypto"
 	"github.com/kopia/kopia/internal/user"
 )
 
-func TestLegacyUserProfile(t *testing.T) {
-	p := &user.Profile{
-		PasswordHashVersion: 1, // hashVersion1
-	}
-
-	if p.IsValidPassword("bar") {
-		t.Fatalf("password unexpectedly valid!")
-	}
-
-	p.SetPassword("foo")
-
-	if !p.IsValidPassword("foo") {
-		t.Fatalf("password not valid!")
-	}
-
-	if p.IsValidPassword("bar") {
-		t.Fatalf("password unexpectedly valid!")
-	}
-
-	// Setting the key derivation to scrypt and unsetting PasswordHashVersion
-	// Legacy profile should translate to scrypt
-	p.KeyDerivationAlgorithm = crypto.ScryptAlgorithm
-	p.PasswordHashVersion = 0
-	if !p.IsValidPassword("foo") {
-		t.Fatalf("password not valid!")
-	}
-}
-
 func TestUserProfile(t *testing.T) {
 	p := &user.Profile{
-		KeyDerivationAlgorithm: crypto.ScryptAlgorithm,
+		PasswordHashVersion: user.ScryptHashVersion,
 	}
 
 	if p.IsValidPassword("bar") {
@@ -55,26 +26,24 @@ func TestUserProfile(t *testing.T) {
 	}
 
 	// Different key derivation algorithm besides the original should fail
-	p.KeyDerivationAlgorithm = crypto.Pbkdf2Algorithm
+	p.PasswordHashVersion = user.Pbkdf2HashVersion
 	if p.IsValidPassword("foo") {
 		t.Fatalf("password unexpectedly valid!")
 	}
 }
 
-func TestBadKeyDerivationAlgorithmPanic(t *testing.T) {
-	defer func() { _ = recover() }()
-	func() {
-		// mock a valid password
-		p := &user.Profile{
-			KeyDerivationAlgorithm: crypto.ScryptAlgorithm,
-		}
-		p.SetPassword("foo")
-		// Assume the key derivation algorithm is bad. This will cause
-		// a panic when validating
-		p.KeyDerivationAlgorithm = "some bad algorithm"
-		p.IsValidPassword("foo")
-	}()
-	t.Errorf("should have panicked")
+func TestBadPasswordHashVersion(t *testing.T) {
+	// mock a valid password
+	p := &user.Profile{
+		PasswordHashVersion: user.ScryptHashVersion,
+	}
+	p.SetPassword("foo")
+	// Assume the key derivation algorithm is bad. This will cause
+	// a panic when validating
+	p.PasswordHashVersion = 0
+	if p.IsValidPassword("foo") {
+		t.Fatalf("password unexpectedly valid!")
+	}
 }
 
 func TestNilUserProfile(t *testing.T) {
@@ -92,7 +61,10 @@ func TestInvalidPasswordHash(t *testing.T) {
 	}
 
 	for _, tc := range cases {
-		p := &user.Profile{PasswordHash: tc}
+		p := &user.Profile{
+			PasswordHash:        tc,
+			PasswordHashVersion: 1,
+		}
 		if p.IsValidPassword("some-password") {
 			t.Fatalf("password unexpectedly valid for %v", tc)
 		}