Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v14] fix: Enforce allow_passwordless server-side #41058

Merged
merged 2 commits into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions api/types/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,21 @@ import (
"github.com/gravitational/teleport/api/utils/tlsutils"
)

var (
// ErrPasswordlessRequiresWebauthn is issued if a passwordless challenge is
// requested but WebAuthn isn't enabled.
ErrPasswordlessRequiresWebauthn = &trace.BadParameterError{
Message: "passwordless requires WebAuthn",
}

// ErrPasswordlessDisabledBySettings is issued if a passwordless challenge is
// requested but passwordless is disabled by cluster settings.
// See AuthPreferenceV2.AuthPreferenceV2.
ErrPasswordlessDisabledBySettings = &trace.BadParameterError{
Message: "passwordless disabled by cluster settings",
}
)

// AuthPreference defines the authentication preferences for a specific
// cluster. It defines the type (local, oidc) and second factor (off, otp, oidc).
// AuthPreference is a configuration resource, never create more than one instance
Expand Down
12 changes: 11 additions & 1 deletion lib/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -3102,9 +3102,15 @@ func (a *Server) CreateAuthenticateChallenge(ctx context.Context, req *proto.Cre

challenges, err := a.mfaAuthChallenge(ctx, username, passwordless)
if err != nil {
// Do not obfuscate config-related errors.
if errors.Is(err, types.ErrPasswordlessRequiresWebauthn) || errors.Is(err, types.ErrPasswordlessDisabledBySettings) {
return nil, trace.Wrap(err)
}

log.Error(trace.DebugReport(err))
return nil, trace.AccessDenied("unable to create MFA challenges")
}

return challenges, nil
}

Expand Down Expand Up @@ -5782,8 +5788,12 @@ func (a *Server) mfaAuthChallenge(ctx context.Context, user string, passwordless
// Handle passwordless separately, it works differently from MFA.
if passwordless {
if !enableWebauthn {
return nil, trace.BadParameter("passwordless requires WebAuthn")
return nil, trace.Wrap(types.ErrPasswordlessRequiresWebauthn)
}
if !apref.GetAllowPasswordless() {
return nil, trace.Wrap(types.ErrPasswordlessDisabledBySettings)
}

webLogin := &wanlib.PasswordlessFlow{
Webauthn: webConfig,
Identity: a.Services,
Expand Down
77 changes: 58 additions & 19 deletions lib/auth/auth_login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,40 @@ func TestServer_CreateAuthenticateChallenge_authPreference(t *testing.T) {
t.Parallel()
ctx := context.Background()

reqUserPassword := func(user, pass string) *proto.CreateAuthenticateChallengeRequest {
return &proto.CreateAuthenticateChallengeRequest{
Request: &proto.CreateAuthenticateChallengeRequest_UserCredentials{
UserCredentials: &proto.UserCredentials{
Username: user,
Password: []byte(pass),
},
},
}
}

reqPasswordless := func(_, _ string) *proto.CreateAuthenticateChallengeRequest {
return &proto.CreateAuthenticateChallengeRequest{
Request: &proto.CreateAuthenticateChallengeRequest_Passwordless{
Passwordless: &proto.Passwordless{},
},
}
}

makeWebauthnSpec := func() *types.AuthPreferenceSpecV2 {
return &types.AuthPreferenceSpecV2{
Type: constants.Local,
SecondFactor: constants.SecondFactorWebauthn,
Webauthn: &types.Webauthn{
RPID: "localhost",
},
}
}

tests := []struct {
name string
spec *types.AuthPreferenceSpecV2
createReq func(user, pass string) *proto.CreateAuthenticateChallengeRequest
wantErr error
assertChallenge func(*proto.MFAAuthenticateChallenge)
}{
{
Expand All @@ -50,6 +81,7 @@ func TestServer_CreateAuthenticateChallenge_authPreference(t *testing.T) {
Type: constants.Local,
SecondFactor: constants.SecondFactorOff,
},
createReq: reqUserPassword,
assertChallenge: func(challenge *proto.MFAAuthenticateChallenge) {
require.Empty(t, challenge.GetTOTP())
require.Empty(t, challenge.GetWebauthnChallenge())
Expand All @@ -61,6 +93,7 @@ func TestServer_CreateAuthenticateChallenge_authPreference(t *testing.T) {
Type: constants.Local,
SecondFactor: constants.SecondFactorOTP,
},
createReq: reqUserPassword,
assertChallenge: func(challenge *proto.MFAAuthenticateChallenge) {
require.NotNil(t, challenge.GetTOTP())
require.Empty(t, challenge.GetWebauthnChallenge())
Expand All @@ -75,20 +108,16 @@ func TestServer_CreateAuthenticateChallenge_authPreference(t *testing.T) {
AppID: "https://localhost",
},
},
createReq: reqUserPassword,
assertChallenge: func(challenge *proto.MFAAuthenticateChallenge) {
require.Empty(t, challenge.GetTOTP())
require.NotEmpty(t, challenge.GetWebauthnChallenge())
},
},
{
name: "OK second_factor:webauthn (standalone)",
spec: &types.AuthPreferenceSpecV2{
Type: constants.Local,
SecondFactor: constants.SecondFactorWebauthn,
Webauthn: &types.Webauthn{
RPID: "localhost",
},
},
name: "OK second_factor:webauthn (standalone)",
spec: makeWebauthnSpec(),
createReq: reqUserPassword,
assertChallenge: func(challenge *proto.MFAAuthenticateChallenge) {
require.Empty(t, challenge.GetTOTP())
require.NotEmpty(t, challenge.GetWebauthnChallenge())
Expand All @@ -106,6 +135,7 @@ func TestServer_CreateAuthenticateChallenge_authPreference(t *testing.T) {
RPID: "localhost",
},
},
createReq: reqUserPassword,
assertChallenge: func(challenge *proto.MFAAuthenticateChallenge) {
require.Empty(t, challenge.GetTOTP())
require.NotEmpty(t, challenge.GetWebauthnChallenge())
Expand All @@ -121,6 +151,7 @@ func TestServer_CreateAuthenticateChallenge_authPreference(t *testing.T) {
RPID: "localhost",
},
},
createReq: reqUserPassword,
assertChallenge: func(challenge *proto.MFAAuthenticateChallenge) {
require.NotNil(t, challenge.GetTOTP())
require.NotEmpty(t, challenge.GetWebauthnChallenge())
Expand All @@ -135,11 +166,22 @@ func TestServer_CreateAuthenticateChallenge_authPreference(t *testing.T) {
RPID: "localhost",
},
},
createReq: reqUserPassword,
assertChallenge: func(challenge *proto.MFAAuthenticateChallenge) {
require.NotNil(t, challenge.GetTOTP())
require.NotEmpty(t, challenge.GetWebauthnChallenge())
},
},
{
name: "allow_passwordless=false and passwordless challenge",
spec: func() *types.AuthPreferenceSpecV2 {
spec := makeWebauthnSpec()
spec.AllowPasswordless = &types.BoolOption{Value: false}
return spec
}(),
createReq: reqPasswordless,
wantErr: types.ErrPasswordlessDisabledBySettings,
},
}
for _, test := range tests {
test := test
Expand All @@ -149,22 +191,19 @@ func TestServer_CreateAuthenticateChallenge_authPreference(t *testing.T) {
svr := newTestTLSServer(t)
authServer := svr.Auth()
mfa := configureForMFA(t, svr)
username := mfa.User
password := mfa.Password
user := mfa.User
pass := mfa.Password

authPreference, err := types.NewAuthPreference(*test.spec)
require.NoError(t, err)
require.NoError(t, authServer.SetAuthPreference(ctx, authPreference))

challenge, err := authServer.CreateAuthenticateChallenge(ctx, &proto.CreateAuthenticateChallengeRequest{
Request: &proto.CreateAuthenticateChallengeRequest_UserCredentials{
UserCredentials: &proto.UserCredentials{
Username: username,
Password: []byte(password),
},
},
})
require.NoError(t, err)
challenge, err := authServer.CreateAuthenticateChallenge(ctx, test.createReq(user, pass))
if test.wantErr != nil {
assert.ErrorIs(t, err, test.wantErr, "CreateAuthenticateChallenge error mismatch")
return
}
require.NoError(t, err, "CreateAuthenticateChallenge errored unexpectedly")
test.assertChallenge(challenge)
})
}
Expand Down
4 changes: 4 additions & 0 deletions lib/web/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -2464,6 +2464,10 @@ func (h *Handler) mfaLoginBegin(w http.ResponseWriter, r *http.Request, p httpro

mfaChallenge, err := h.auth.proxyClient.CreateAuthenticateChallenge(r.Context(), mfaReq)
if err != nil {
// Do not obfuscate config-related errors.
if errors.Is(err, types.ErrPasswordlessRequiresWebauthn) || errors.Is(err, types.ErrPasswordlessDisabledBySettings) {
return nil, trace.Wrap(err)
}
return nil, trace.AccessDenied("invalid credentials")
}

Expand Down
40 changes: 40 additions & 0 deletions lib/web/apiserver_login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,46 @@ func TestAuthenticate_passwordless(t *testing.T) {
test.login(t, assertionResp)
})
}

// Test a couple of config-mismatch scenarios.
// They progressively alter the cluster's auth preference.

t.Run("allow_passwordless=false", func(t *testing.T) {
// Set allow_passwordless=false
authPref, err := authServer.GetAuthPreference(ctx)
require.NoError(t, err, "GetAuthPreference failed")
authPref.SetAllowPasswordless(false)
require.NoError(t,
authServer.SetAuthPreference(ctx, authPref),
"UpsertAuthPreference failed",
)

// GET /webapi/mfa/login/begin.
ep := clt.Endpoint("webapi", "mfa", "login", "begin")
_, err = clt.PostJSON(ctx, ep, &client.MFAChallengeRequest{
Passwordless: true, // no username and password
})
assert.ErrorIs(t, err, types.ErrPasswordlessDisabledBySettings, "/webapi/mfa/login/begin error mismatch")
})

t.Run("webauthn disabled", func(t *testing.T) {
authPref, err := types.NewAuthPreference(types.AuthPreferenceSpecV2{
Type: constants.Local,
SecondFactor: constants.SecondFactorOTP, // disable webauthn
})
require.NoError(t, err, "NewAuthPreference failed")
require.NoError(t,
authServer.SetAuthPreference(ctx, authPref),
"UpsertAuthPreference failed",
)

// GET /webapi/mfa/login/begin.
ep := clt.Endpoint("webapi", "mfa", "login", "begin")
_, err = clt.PostJSON(ctx, ep, &client.MFAChallengeRequest{
Passwordless: true, // no username and password
})
assert.ErrorIs(t, err, types.ErrPasswordlessRequiresWebauthn, "/webapi/mfa/login/begin error mismatch")
})
}

func TestAuthenticate_rateLimiting(t *testing.T) {
Expand Down
Loading