diff --git a/internal/auth/password/query.go b/internal/auth/password/query.go index 20d40f472c..ec3802ed95 100644 --- a/internal/auth/password/query.go +++ b/internal/auth/password/query.go @@ -17,13 +17,16 @@ select acct.name, -- Account.Name conf.key_length, -- Argon2Configuration.KeyLength conf.iterations, -- Argon2Configuration.Iterations conf.memory, -- Argon2Configuration.Memory - conf.threads -- Argon2Configuration.Threads + conf.threads, -- Argon2Configuration.Threads + meth.password_conf_id = cred.password_conf_id as is_current_conf from auth_password_argon2_cred cred, auth_password_argon2_conf conf, - auth_password_account acct + auth_password_account acct, + auth_password_method meth where acct.auth_method_id = $1 and acct.user_name = $2 and cred.password_conf_id = conf.private_id - and cred.password_account_id = acct.public_id; + and cred.password_account_id = acct.public_id + and acct.auth_method_id = meth.public_id ; ` ) diff --git a/internal/auth/password/repository_password.go b/internal/auth/password/repository_password.go index 8e48f9136a..73148797e3 100644 --- a/internal/auth/password/repository_password.go +++ b/internal/auth/password/repository_password.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/hashicorp/watchtower/internal/db" + "github.com/hashicorp/watchtower/internal/oplog" "golang.org/x/crypto/argon2" ) @@ -35,6 +36,7 @@ func (r *Repository) Authenticate(ctx context.Context, authMethodId string, user *Account *Argon2Credential *Argon2Configuration + IsCurrentConf bool } var accts []AuthAccount @@ -76,8 +78,42 @@ func (r *Repository) Authenticate(ctx context.Context, authMethodId string, user return nil, nil } - // TODO(mgaffney) 07/2020: update stored credential if acct config is - // not the current config for authMethodId + if !acct.IsCurrentConf { + cc, err := r.currentConfig(ctx, authMethodId) + if err != nil { + return acct.Account, fmt.Errorf("authenticate: retrieve current password configuration: %w", err) + } + cred, err := newArgon2Credential(acct.PublicId, password, cc.argon2()) + if err != nil { + return acct.Account, fmt.Errorf("authenticate: rehash current password: %w", err) + } + cred.PrivateId = acct.CredentialId + cred.PasswordMethodId = cc.PasswordMethodId + + var newCred *Argon2Credential + _, err = r.writer.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, + func(_ db.Reader, w db.Writer) error { + newCred = cred.clone() + if err := newCred.encrypt(ctx, r.wrapper); err != nil { + return err + } + rowsUpdated, err := w.Update( + ctx, + newCred, + []string{"CtSalt", "DerivedKey", "PasswordConfId"}, + nil, + db.WithOplog(r.wrapper, cred.oplog(oplog.OpType_OP_TYPE_UPDATE)), + ) + if err == nil && rowsUpdated > 1 { + return db.ErrMultipleRecords + } + return err + }, + ) + if err != nil { + return acct.Account, fmt.Errorf("authenticate: update rehashed password: %w", err) + } + } return acct.Account, nil } diff --git a/internal/auth/password/repository_password_test.go b/internal/auth/password/repository_password_test.go index 799eb32e49..acc2b9e99e 100644 --- a/internal/auth/password/repository_password_test.go +++ b/internal/auth/password/repository_password_test.go @@ -115,3 +115,116 @@ func TestRepository_Authenticate(t *testing.T) { }) } } + +func TestRepository_AuthenticateRehash(t *testing.T) { + conn, _ := db.TestSetup(t, "postgres") + rw := db.New(conn) + wrapper := db.TestWrapper(t) + assert, require := assert.New(t), require.New(t) + + authMethods := testAuthMethods(t, conn, 1) + authMethod := authMethods[0] + authMethodId := authMethod.GetPublicId() + userName := "kazmierczak" + passwd := "12345678" + ctx := context.Background() + + repo, err := NewRepository(rw, rw, wrapper) + assert.NoError(err) + require.NotNil(repo) + + // Get the default (original) argon2 configuration + origConf, err := repo.GetConfiguration(ctx, authMethodId) + assert.NoError(err) + require.NotNil(origConf) + origArgonConf, ok := origConf.(*Argon2Configuration) + require.True(ok, "want *Argon2Configuration") + require.NotEmpty(origArgonConf.PrivateId, "original configuration PrivateId") + origConfId := origArgonConf.PrivateId + + // Create an account with a password + inAcct := &Account{ + Account: &store.Account{ + AuthMethodId: authMethod.PublicId, + UserName: userName, + }, + } + + origAcct, err := repo.CreateAccount(ctx, inAcct, WithPassword(passwd)) + require.NoError(err) + require.NotNil(origAcct) + assert.NotEmpty(origAcct.PublicId) + + // Get the credential for the new account and verify the KDF used the + // original argon2 configuration + origCred := &Argon2Credential{Argon2Credential: &store.Argon2Credential{}} + require.NoError(rw.LookupWhere(ctx, origCred, "password_account_id = ?", origAcct.PublicId)) + assert.Equal(origAcct.PublicId, origCred.PasswordAccountId) + assert.Equal(origConfId, origCred.PasswordConfId) + assert.Equal(origCred.CreateTime, origCred.UpdateTime, "create and update times are equal") + origCredId := origCred.PrivateId + + // Authenticate and verify the credential ID + authAcct, err := repo.Authenticate(ctx, authMethodId, userName, passwd) + require.NoError(err) + require.NotNil(authAcct, "auth account") + assert.Equal(origAcct.PublicId, authAcct.PublicId) + assert.Equal(origCredId, authAcct.CredentialId) + auth1CredId := authAcct.CredentialId + + // Get the credential and verify the call to Authenticate did not + // change anything + auth1Cred := &Argon2Credential{ + Argon2Credential: &store.Argon2Credential{ + PrivateId: auth1CredId, + }, + } + require.NoError(rw.LookupById(ctx, auth1Cred)) + assert.Equal(authAcct.PublicId, auth1Cred.PasswordAccountId) + assert.Equal(origConfId, auth1Cred.PasswordConfId) + assert.Equal(origCred.PasswordConfId, auth1Cred.PasswordConfId, "same configuration ID") + assert.Equal(origCred.Salt, auth1Cred.Salt, "same salt") + assert.Equal(origCred.DerivedKey, auth1Cred.DerivedKey, "same derived key") + assert.Equal(origCred.UpdateTime, auth1Cred.UpdateTime, "same update time") + + // Change the argon2 configuration + inArgonConf := origArgonConf.clone() + inArgonConf.Threads = origArgonConf.Threads + 1 + + upConf, err := repo.SetConfiguration(ctx, inArgonConf) + require.NoError(err) + require.NotNil(upConf) + assert.NotSame(inArgonConf, upConf) + + upArgonConf, ok := upConf.(*Argon2Configuration) + require.True(ok, "want *Argon2Configuration") + assert.NotEqual(origConfId, upArgonConf.PrivateId) + + // Authenticate and verify the credential ID has not changed + auth2Acct, err := repo.Authenticate(ctx, authMethodId, userName, passwd) + require.NoError(err) + require.NotNil(auth2Acct, "auth2 account") + assert.Equal(origAcct.PublicId, auth2Acct.PublicId) + require.Equal(origCredId, auth2Acct.CredentialId) + auth2CredId := auth2Acct.CredentialId + + // Get the credential and verify the call to Authenticate changed the + // appropriate fields + auth2Cred := &Argon2Credential{ + Argon2Credential: &store.Argon2Credential{ + PrivateId: auth2CredId, + }, + } + require.NoError(rw.LookupById(ctx, auth2Cred)) + // Verify fields that should not change + assert.Equal(auth2Acct.PublicId, auth2Cred.PasswordAccountId) + assert.Equal(origCred.PrivateId, auth2Cred.PrivateId, "the credential Id should not change") + assert.Equal(origCred.CreateTime, auth2Cred.CreateTime, "the create time should not change") + + // Verify fields that should change + require.NoError(auth2Cred.decrypt(ctx, wrapper)) + assert.NotEqual(origCred.UpdateTime, auth2Cred.UpdateTime, "the update time should be different") + assert.NotEqual(origCred.PasswordConfId, auth2Cred.PasswordConfId, "the configuration Id should be different") + assert.NotEqual(origCred.Salt, auth2Cred.Salt, "a new salt value should be generated") + assert.NotEqual(origCred.DerivedKey, auth2Cred.DerivedKey, "the derived key should be different") +} diff --git a/internal/db/migrations/postgres.gen.go b/internal/db/migrations/postgres.gen.go index 7e32bac15c..1641fcd786 100644 --- a/internal/db/migrations/postgres.gen.go +++ b/internal/db/migrations/postgres.gen.go @@ -1816,6 +1816,26 @@ begin; end; $$ language plpgsql; + -- update_auth_password_credential_subtype() is an after update trigger + -- function for subtypes of auth_password_credential + create or replace function + update_auth_password_credential_subtype() + returns trigger + as $$ + begin + /* + The configuration id of a credential is updated when a credential is + rehashed during authentication. + */ + if new.password_conf_id is distinct from old.password_conf_id then + update auth_password_credential + set password_conf_id = new.password_conf_id + where private_id = new.private_id; + end if; + return null; -- result is ignored since this is an after trigger + end; + $$ language plpgsql; + -- -- triggers for time columns -- @@ -1960,6 +1980,7 @@ begin; references auth_password_credential (password_method_id, password_conf_id, password_account_id) on delete cascade on update cascade + deferrable initially deferred ); create trigger @@ -1967,6 +1988,11 @@ begin; before insert on auth_password_argon2_cred for each row execute procedure insert_auth_password_credential_subtype(); + create trigger + update_auth_password_credential_subtype + after update on auth_password_argon2_cred + for each row execute procedure update_auth_password_credential_subtype(); + -- -- triggers for time columns -- diff --git a/internal/db/migrations/postgres/12_auth_password.up.sql b/internal/db/migrations/postgres/12_auth_password.up.sql index 80d76cda62..deae259854 100644 --- a/internal/db/migrations/postgres/12_auth_password.up.sql +++ b/internal/db/migrations/postgres/12_auth_password.up.sql @@ -184,6 +184,26 @@ begin; end; $$ language plpgsql; + -- update_auth_password_credential_subtype() is an after update trigger + -- function for subtypes of auth_password_credential + create or replace function + update_auth_password_credential_subtype() + returns trigger + as $$ + begin + /* + The configuration id of a credential is updated when a credential is + rehashed during authentication. + */ + if new.password_conf_id is distinct from old.password_conf_id then + update auth_password_credential + set password_conf_id = new.password_conf_id + where private_id = new.private_id; + end if; + return null; -- result is ignored since this is an after trigger + end; + $$ language plpgsql; + -- -- triggers for time columns -- diff --git a/internal/db/migrations/postgres/13_auth_password_argon.up.sql b/internal/db/migrations/postgres/13_auth_password_argon.up.sql index cacc5727dd..920a989ad9 100644 --- a/internal/db/migrations/postgres/13_auth_password_argon.up.sql +++ b/internal/db/migrations/postgres/13_auth_password_argon.up.sql @@ -75,6 +75,7 @@ begin; references auth_password_credential (password_method_id, password_conf_id, password_account_id) on delete cascade on update cascade + deferrable initially deferred ); create trigger @@ -82,6 +83,11 @@ begin; before insert on auth_password_argon2_cred for each row execute procedure insert_auth_password_credential_subtype(); + create trigger + update_auth_password_credential_subtype + after update on auth_password_argon2_cred + for each row execute procedure update_auth_password_credential_subtype(); + -- -- triggers for time columns --