Skip to content

Commit

Permalink
feat: gracefully handle failing password rehashing during login (#4235)
Browse files Browse the repository at this point in the history
This fixes an issue where we would successfully import long passwords (>72 chars), but fail when the user attempts to login with the correct password because we can't rehash it. In this case, we simply issue a warning to the logs, keep the old hash intact, and continue logging in the user.
  • Loading branch information
alnr authored Dec 4, 2024
1 parent 816ea44 commit 3905787
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 4 deletions.
5 changes: 3 additions & 2 deletions hash/hash_comparator.go
Original file line number Diff line number Diff line change
Expand Up @@ -551,10 +551,11 @@ func compareCryptHelper(password []byte, hash string) error {
return errors.WithStack(ErrMismatchedHashAndPassword)
}

var regexSSHA = regexp.MustCompile(`\{([^}]*)\}`)

// decodeSSHAHash decodes SSHA[1|256|512] encoded password hash in usual {SSHA...} format.
func decodeSSHAHash(encodedHash string) (hasher string, salt, hash []byte, err error) {
re := regexp.MustCompile(`\{([^}]*)\}`)
match := re.FindStringSubmatch(string(encodedHash))
match := regexSSHA.FindStringSubmatch(string(encodedHash))

var index_of_salt_begin int
var index_of_hash_begin int
Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/password/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow,

if !s.d.Hasher(ctx).Understands([]byte(o.HashedPassword)) {
if err := s.migratePasswordHash(ctx, i.ID, []byte(p.Password)); err != nil {
return nil, s.handleLoginError(r, f, p, err)
s.d.Logger().Warnf("Unable to migrate password hash for identity %s: %s Keeping existing password hash and continuing.", i.ID, err)
}
}
}
Expand Down
64 changes: 63 additions & 1 deletion selfservice/strategy/password/login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@ package password_test
import (
"bytes"
"context"
"crypto/sha256"
_ "embed"
"encoding/base64"
"encoding/json"
"fmt"
"io"
"net/http"
"net/http/httptest"
"net/url"
"slices"
"strings"
"testing"
"time"
Expand All @@ -21,6 +24,7 @@ import (

configtesthelpers "github.com/ory/kratos/driver/config/testhelpers"

"github.com/ory/x/randx"
"github.com/ory/x/snapshotx"

"github.com/ory/kratos/driver"
Expand Down Expand Up @@ -903,6 +907,63 @@ func TestCompleteLogin(t *testing.T) {
assert.Equal(t, identifier, gjson.Get(body, "identity.traits.email").String(), "%s", body)
})

t.Run("suite=password rehashing degrades gracefully during login", func(t *testing.T) {
identifier := x.NewUUID().String() + "@google.com"
// pwd := "Kd9hUV4Xkcq87VSca6A4fq1iBijrMScBFhkpIPEwBtvTDsBwfqJCqXPPr4TkhOhsd9wFGeB3MzS4bJuesLCAjJc5s1GKJ51zW7F"
pwd := randx.MustString(100, randx.AlphaNum) // longer than bcrypt max length
require.Greater(t, len(pwd), 72) // bcrypt max length
salt := randx.MustString(32, randx.AlphaNum)
sha := sha256.Sum256([]byte(pwd + salt))
hashed := "{SSHA256}" + base64.StdEncoding.EncodeToString(slices.Concat(sha[:], []byte(salt)))
iId := x.NewUUID()
require.NoError(t, reg.PrivilegedIdentityPool().CreateIdentity(context.Background(), &identity.Identity{
ID: iId,
SchemaID: "migration",
Traits: identity.Traits(fmt.Sprintf(`{"email":%q}`, identifier)),
Credentials: map[identity.CredentialsType]identity.Credentials{
identity.CredentialsTypePassword: {
Type: identity.CredentialsTypePassword,
Identifiers: []string{identifier},
Config: sqlxx.JSONRawMessage(`{"hashed_password":"` + hashed + `"}`),
},
},
VerifiableAddresses: []identity.VerifiableAddress{
{
ID: x.NewUUID(),
Value: identifier,
Verified: true,
CreatedAt: time.Now(),
IdentityID: iId,
},
},
}))

values := func(v url.Values) {
v.Set("identifier", identifier)
v.Set("method", identity.CredentialsTypePassword.String())
v.Set("password", pwd)
}

browserClient := testhelpers.NewClientWithCookies(t)

body := testhelpers.SubmitLoginForm(t, false, browserClient, publicTS, values,
false, false, http.StatusOK, redirTS.URL)

assert.Equal(t, identifier, gjson.Get(body, "identity.traits.email").String(), "%s", body)

// check that the password hash algorithm is unchanged
_, c, err := reg.PrivilegedIdentityPool().FindByCredentialsIdentifier(context.Background(), identity.CredentialsTypePassword, identifier)
require.NoError(t, err)
var o identity.CredentialsPassword
require.NoError(t, json.NewDecoder(bytes.NewBuffer(c.Config)).Decode(&o))
assert.Equal(t, hashed, o.HashedPassword)

// login still works
body = testhelpers.SubmitLoginForm(t, false, browserClient, publicTS, values,
false, true, http.StatusOK, redirTS.URL)
assert.Equal(t, identifier, gjson.Get(body, "identity.traits.email").String(), "%s", body)
})

t.Run("suite=password migration hook", func(t *testing.T) {
ctx := context.Background()

Expand Down Expand Up @@ -948,7 +1009,8 @@ func TestCompleteLogin(t *testing.T) {

require.NoError(t, reg.Config().Set(ctx, config.ViperKeyPasswordMigrationHook, map[string]any{
"config": map[string]any{"url": ts.URL},
"enabled": true}))
"enabled": true,
}))

for _, tc := range []struct {
name string
Expand Down

0 comments on commit 3905787

Please sign in to comment.