From ada8bea3c291f7cb8cb07ac19e0c0eed59efc4b3 Mon Sep 17 00:00:00 2001 From: Alan Parra Date: Mon, 29 Apr 2024 16:11:30 -0300 Subject: [PATCH] fix: Enforce allow_passwordless server-side --- api/types/authentication.go | 15 +++++++ lib/auth/auth.go | 12 ++++- lib/auth/auth_login_test.go | 80 +++++++++++++++++++++++++-------- lib/web/apiserver.go | 4 ++ lib/web/apiserver_login_test.go | 36 +++++++++++++++ 5 files changed, 127 insertions(+), 20 deletions(-) diff --git a/api/types/authentication.go b/api/types/authentication.go index 2227aaad80983..445575421cfe4 100644 --- a/api/types/authentication.go +++ b/api/types/authentication.go @@ -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 diff --git a/lib/auth/auth.go b/lib/auth/auth.go index 5b3e84cd8a33f..921155f448f65 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -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 } @@ -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, diff --git a/lib/auth/auth_login_test.go b/lib/auth/auth_login_test.go index b7fd957b9e6db..7a60701430ee7 100644 --- a/lib/auth/auth_login_test.go +++ b/lib/auth/auth_login_test.go @@ -39,9 +39,43 @@ 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{}, + }, + ChallengeExtensions: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN, + }, + } + } + + 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) }{ { @@ -50,6 +84,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()) @@ -61,6 +96,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()) @@ -75,20 +111,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()) @@ -106,6 +138,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()) @@ -121,6 +154,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()) @@ -135,11 +169,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 @@ -149,22 +194,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) }) } diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 51358021518e3..770df7ffe68f1 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -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") } diff --git a/lib/web/apiserver_login_test.go b/lib/web/apiserver_login_test.go index 1f2d1541711e5..130476bc0ab8d 100644 --- a/lib/web/apiserver_login_test.go +++ b/lib/web/apiserver_login_test.go @@ -271,6 +271,42 @@ 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) + _, err = authServer.UpsertAuthPreference(ctx, authPref) + require.NoError(t, err, "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") + _, err = authServer.UpsertAuthPreference(ctx, authPref) + require.NoError(t, err, "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) {