diff --git a/config.go b/config.go index bd4a71b..53eba6a 100644 --- a/config.go +++ b/config.go @@ -27,6 +27,23 @@ type WorkFactor interface { Unmarshal([]int) error } +// cloneWorkFactor returns an independent copy of the provided WorkFactor +func cloneWorkFactor(wf WorkFactor) (WorkFactor, error) { + switch v := wf.(type) { + case *Pbkdf2WorkFactor: + c := *v + return &c, nil + case *BcryptWorkFactor: + c := *v + return &c, nil + case *ScryptWorkFactor: + c := *v + return &c, nil + default: + return nil, fmt.Errorf("unsupported WorkFactor type: %T", wf) + } +} + // WorkFactorsEqual determines if 2 WorkFactors are equivalent func WorkFactorsEqual(a, b WorkFactor) bool { if reflect.TypeOf(a) != reflect.TypeOf(b) { @@ -135,5 +152,9 @@ func (c Config) NewCredential(userID UserID, password string) (*Credential, erro if err != nil { return nil, err } - return &Credential{UserID: userID, Kdf: c.Kdf, WorkFactor: c.WorkFactor, Salt: salt, Hash: hash}, nil + wfCopy, err := cloneWorkFactor(c.WorkFactor) + if err != nil { + return nil, err + } + return &Credential{UserID: userID, Kdf: c.Kdf, WorkFactor: wfCopy, Salt: salt, Hash: hash}, nil } diff --git a/credential_test.go b/credential_test.go index b301e2a..8776641 100644 --- a/credential_test.go +++ b/credential_test.go @@ -4,9 +4,9 @@ import ( _ "crypto/sha256" _ "crypto/sha512" "testing" -) -import ( + "golang.org/x/crypto/bcrypt" + "github.com/dhui/passhash" ) @@ -180,7 +180,7 @@ func TestMatchesPasswordMatchNoUpdate(t *testing.T) { if credential.Kdf != origKdf { t.Errorf("Original credential Kdf changed. %v != %v", credential.Kdf, origKdf) } - if credential.WorkFactor != origWorkFactor { + if !passhash.WorkFactorsEqual(credential.WorkFactor, origWorkFactor) { t.Errorf("Original credential WorkFactor changed. %v != %v", credential.WorkFactor, origWorkFactor) } @@ -195,7 +195,7 @@ func TestMatchesPasswordMatchNoUpdate(t *testing.T) { t.Errorf("Credential Kdf unexpectedly updated from safe recommended Kdf. %v != %v", credential.Kdf, passhash.DefaultConfig.Kdf) } - if credential.WorkFactor != passhash.DefaultWorkFactor[passhash.DefaultConfig.Kdf] { + if !passhash.WorkFactorsEqual(credential.WorkFactor, passhash.DefaultWorkFactor[passhash.DefaultConfig.Kdf]) { t.Errorf("Credential WorkFactor unexpected updated from safe recommended Kdf WorkFactor. %v != %v", credential.WorkFactor, passhash.DefaultWorkFactor[passhash.DefaultConfig.Kdf]) } @@ -223,7 +223,7 @@ func TestMatchesPasswordUpdateKdfAndWorkFactor(t *testing.T) { if credential.Kdf != origKdf { t.Errorf("Original credential Kdf changed. %v != %v", credential.Kdf, origKdf) } - if credential.WorkFactor != origWorkFactor { + if !passhash.WorkFactorsEqual(credential.WorkFactor, origWorkFactor) { t.Errorf("Original credential WorkFactor changed. %v != %v", credential.WorkFactor, origWorkFactor) } @@ -238,7 +238,7 @@ func TestMatchesPasswordUpdateKdfAndWorkFactor(t *testing.T) { t.Errorf("Updated credential Kdf did not update to safe recommended Kdf. %v != %v", credential.Kdf, passhash.DefaultConfig.Kdf) } - if credential.WorkFactor != passhash.DefaultWorkFactor[passhash.DefaultConfig.Kdf] { + if !passhash.WorkFactorsEqual(credential.WorkFactor, passhash.DefaultWorkFactor[passhash.DefaultConfig.Kdf]) { t.Errorf("Updated credential WorkFactor did not update to safe recommended Kdf WorkFactor. %v != %v", credential.WorkFactor, passhash.DefaultWorkFactor[passhash.DefaultConfig.Kdf]) } @@ -278,12 +278,81 @@ func TestMatchesPasswordUpdateWorkFactor(t *testing.T) { if credential.Kdf != kdf { t.Errorf("Updated credential Kdf changed. %v != %v", credential.Kdf, kdf) } - if credential.WorkFactor != passhash.DefaultWorkFactor[passhash.DefaultConfig.Kdf] { + if !passhash.WorkFactorsEqual(credential.WorkFactor, passhash.DefaultWorkFactor[passhash.DefaultConfig.Kdf]) { t.Errorf("Updated credential WorkFactor did not update to safe recommended Kdf WorkFactor. %v != %v", credential.WorkFactor, passhash.DefaultWorkFactor[passhash.DefaultConfig.Kdf]) } } +func TestScryptWorkFactorMutationDoesNotBreakExistingCredential(t *testing.T) { + cfg := passhash.Config{ + Kdf: passhash.Scrypt, + WorkFactor: &passhash.ScryptWorkFactor{N: 1024, R: 16, P: 1}, + SaltSize: 16, + KeyLength: 32, + AuditLogger: &passhash.DummyAuditLogger{}, + Store: passhash.DummyCredentialStore{}, + } + userID := passhash.UserID(1) + + cred, err := cfg.NewCredential(userID, testPassword) + if err != nil { + t.Fatalf("Unable to create new Credential: %v", err) + } + + if matched, _ := cred.MatchesPasswordWithConfig(cfg, testPassword); !matched { + t.Fatalf("Password did not match before work factor change") + } + + wf := cfg.WorkFactor.(*passhash.ScryptWorkFactor) + wf.N *= 2 + + if matched, _ := cred.MatchesPasswordWithConfig(cfg, testPassword); !matched { + t.Fatalf("Password did not match after work factor change; credential should be stable") + } +} + +func TestBcryptWorkFactorMutationTriggersUpgrade(t *testing.T) { + cfg := passhash.Config{ + Kdf: passhash.Bcrypt, + WorkFactor: &passhash.BcryptWorkFactor{Cost: 10}, + SaltSize: 16, + KeyLength: 32, + AuditLogger: &passhash.DummyAuditLogger{}, + Store: passhash.DummyCredentialStore{}, + } + userID := passhash.UserID(2) + + cred, err := cfg.NewCredential(userID, testPassword) + if err != nil { + t.Fatalf("Unable to create new Credential: %v", err) + } + + beforeCost, err := bcrypt.Cost(cred.Hash) + if err != nil { + t.Fatalf("Unable to read bcrypt cost: %v", err) + } + + wf := cfg.WorkFactor.(*passhash.BcryptWorkFactor) + wf.Cost = beforeCost + 2 + + matched, updated := cred.MatchesPasswordWithConfig(cfg, testPassword) + if !matched { + t.Fatalf("Password did not match after bcrypt work factor change") + } + if !updated { + t.Fatalf("Expected credential to be upgraded after bcrypt work factor change") + } + + afterCost, err := bcrypt.Cost(cred.Hash) + if err != nil { + t.Fatalf("Unable to read bcrypt cost after upgrade: %v", err) + } + if afterCost <= beforeCost { + t.Fatalf("Expected bcrypt cost to increase, got before=%d after=%d", beforeCost, afterCost) + } +} + func TestChangePassword(t *testing.T) { userID := passhash.UserID(0) credential, err := passhash.NewCredential(userID, testPassword)