Skip to content

Commit

Permalink
fix: Enforce allow_passwordless server-side
Browse files Browse the repository at this point in the history
  • Loading branch information
codingllama authored and github-actions committed Apr 30, 2024
1 parent b18a158 commit 81ce8c3
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 20 deletions.
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 @@ -3219,9 +3219,15 @@ func (a *Server) CreateAuthenticateChallenge(ctx context.Context, req *proto.Cre

challenges, err := a.mfaAuthChallenge(ctx, username, challengeExtensions)
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 @@ -6032,8 +6038,12 @@ func (a *Server) mfaAuthChallenge(ctx context.Context, user string, challengeExt
// Handle passwordless separately, it works differently from MFA.
if isPasswordless {
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
80 changes: 61 additions & 19 deletions lib/auth/auth_login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,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)
}{
{
Expand All @@ -56,6 +90,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 @@ -67,6 +102,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 @@ -81,20 +117,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 @@ -112,6 +144,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 @@ -127,6 +160,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 @@ -141,11 +175,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 @@ -155,23 +200,20 @@ 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)
_, err = authServer.UpsertAuthPreference(ctx, authPreference)
require.NoError(t, err)

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 @@ -2482,6 +2482,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
36 changes: 36 additions & 0 deletions lib/web/apiserver_login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,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) {
Expand Down

0 comments on commit 81ce8c3

Please sign in to comment.