diff --git a/api/types/authentication.go b/api/types/authentication.go index 61ea3e1cb20..e90a217c66b 100644 --- a/api/types/authentication.go +++ b/api/types/authentication.go @@ -48,6 +48,12 @@ var ( ErrPasswordlessDisabledBySettings = &trace.BadParameterError{ Message: "passwordless disabled by cluster settings", } + + // ErrPassswordlessLoginBySSOUser is issued if an SSO user tries to login + // using passwordless. + ErrPassswordlessLoginBySSOUser = &trace.AccessDeniedError{ + Message: "SSO user cannot login using passwordless", + } ) // AuthPreference defines the authentication preferences for a specific diff --git a/api/utils/keys/policy.go b/api/utils/keys/policy.go index 70585666700..78029c99a17 100644 --- a/api/utils/keys/policy.go +++ b/api/utils/keys/policy.go @@ -194,5 +194,8 @@ func ParsePrivateKeyPolicyError(err error) (PrivateKeyPolicy, error) { // IsPrivateKeyPolicyError returns true if the given error is a private key policy error. func IsPrivateKeyPolicyError(err error) bool { + if err == nil { + return false + } return privateKeyPolicyErrRegex.MatchString(err.Error()) } diff --git a/lib/auth/auth.go b/lib/auth/auth.go index e880a3a1ca4..6b7f88571a7 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -6414,6 +6414,18 @@ func (a *Server) validateMFAAuthResponseInternal( Identity: a.Services, } loginData, err = webLogin.Finish(ctx, assertionResp) + + // Disallow non-local users from logging in with passwordless. + if err == nil { + u, getErr := a.GetUser(ctx, loginData.User, false /* withSecrets */) + if getErr != nil { + err = trace.Wrap(getErr) + } else if u.GetUserType() != types.UserTypeLocal { + // Return the error unmodified, without the "MFA response validation + // failed" prefix. + return nil, trace.Wrap(types.ErrPassswordlessLoginBySSOUser) + } + } } else { webLogin := &wanlib.LoginFlow{ U2F: u2f, diff --git a/lib/auth/auth_login_test.go b/lib/auth/auth_login_test.go index 59ccca14fc5..8939d0a202c 100644 --- a/lib/auth/auth_login_test.go +++ b/lib/auth/auth_login_test.go @@ -955,9 +955,6 @@ func TestServer_Authenticate_passwordless(t *testing.T) { }) require.NoError(t, err, "Failed to register passwordless device") - // userWebID is what identifies the user for usernameless/passwordless. - userWebID := registerChallenge.GetWebauthn().PublicKey.User.Id - // Use a proxy client for now on; the user's identity isn't established yet. proxyClient, err := svr.NewClient(TestBuiltin(types.RoleProxy)) require.NoError(t, err) @@ -1067,7 +1064,6 @@ func TestServer_Authenticate_passwordless(t *testing.T) { // Sign challenge (mocks user interaction). assertionResp, err := pwdKey.SignAssertion(origin, wantypes.CredentialAssertionFromProto(mfaChallenge.GetWebauthnChallenge())) require.NoError(t, err) - assertionResp.AssertionResponse.UserHandle = userWebID // identify user, a real device would set this // Complete login procedure (SSH or Web). test.authenticate(t, assertionResp) @@ -1083,6 +1079,94 @@ func TestServer_Authenticate_passwordless(t *testing.T) { } } +// TestPasswordlessProhibitedForSSO tests a scenario where an SSO user bypasses +// SSO by using a passwordless login. +func TestPasswordlessProhibitedForSSO(t *testing.T) { + t.Parallel() + + testServer := newTestTLSServer(t) + authServer := testServer.Auth() + clock := testServer.Clock() + + mfa := configureForMFA(t, testServer) + ctx := context.Background() + + // Register a passwordless device. + userClient, err := testServer.NewClient(TestUser(mfa.User)) + require.NoError(t, err, "NewClient failed") + pwdlessDev, err := RegisterTestDevice(ctx, userClient, "pwdless", proto.DeviceType_DEVICE_TYPE_WEBAUTHN, mfa.WebDev, WithPasswordless()) + require.NoError(t, err, "RegisterTestDevice failed") + + // Edit the configured user so it looks like an SSO user attempting local + // logins. This isn't exactly like an SSO user, but it's close enough. + _, err = authServer.UpdateAndSwapUser(ctx, mfa.User, true /* withSecrets */, func(user types.User) (changed bool, err error) { + user.SetCreatedBy(types.CreatedBy{ + Connector: &types.ConnectorRef{ + Type: constants.Github, + ID: "github", + Identity: mfa.User, + }, + Time: clock.Now(), + User: types.UserRef{ + Name: teleport.UserSystem, + }, + }) + return true, nil + }) + require.NoError(t, err, "UpdateAndSwapUser failed") + + // Authentication happens through the Proxy identity. + proxyClient, err := testServer.NewClient(TestBuiltin(types.RoleProxy)) + require.NoError(t, err) + + tests := []struct { + name string + authenticate func(*wantypes.CredentialAssertionResponse) error + }{ + { + name: "web not allowed", + authenticate: func(car *wantypes.CredentialAssertionResponse) error { + _, err := proxyClient.AuthenticateWebUser(ctx, AuthenticateUserRequest{ + Webauthn: car, + }) + return err + }, + }, + { + name: "ssh not allowed", + authenticate: func(car *wantypes.CredentialAssertionResponse) error { + _, err := proxyClient.AuthenticateSSHUser(ctx, AuthenticateSSHRequest{ + AuthenticateUserRequest: AuthenticateUserRequest{ + PublicKey: []byte(sshPubKey), + Webauthn: car, + }, + TTL: 12 * time.Hour, + }) + return err + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + chal, err := proxyClient.CreateAuthenticateChallenge(ctx, &proto.CreateAuthenticateChallengeRequest{ + Request: &proto.CreateAuthenticateChallengeRequest_Passwordless{ + Passwordless: &proto.Passwordless{}, + }, + ChallengeExtensions: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN, + }, + }) + require.NoError(t, err, "CreateAuthenticateChallenge failed") + + chalResp, err := pwdlessDev.SolveAuthn(chal) + require.NoError(t, err, "SolveAuthn failed") + + err = test.authenticate(wantypes.CredentialAssertionResponseFromProto(chalResp.GetWebauthn())) + assert.ErrorIs(t, err, types.ErrPassswordlessLoginBySSOUser, "authentication error mismatch") + }) + } +} + func TestServer_Authenticate_nonPasswordlessRequiresUsername(t *testing.T) { t.Parallel() svr := newTestTLSServer(t) diff --git a/lib/auth/helpers_mfa.go b/lib/auth/helpers_mfa.go index ddadf6e56a9..b168e426168 100644 --- a/lib/auth/helpers_mfa.go +++ b/lib/auth/helpers_mfa.go @@ -221,8 +221,7 @@ func (d *TestDevice) solveRegisterWebauthn(c *proto.MFARegisterChallenge) (*prot d.Key.PreferRPID = true if d.passwordless { - d.Key.AllowResidentKey = true - d.Key.SetUV = true + d.Key.SetPasswordless() } resp, err := d.Key.SignCredentialCreation(d.Origin(), wantypes.CredentialCreationFromProto(c.GetWebauthn())) diff --git a/lib/auth/methods.go b/lib/auth/methods.go index 425d15d37f8..102aac89e64 100644 --- a/lib/auth/methods.go +++ b/lib/auth/methods.go @@ -506,7 +506,11 @@ func (a *Server) authenticatePasswordless(ctx context.Context, req AuthenticateU requiredExt := &mfav1.ChallengeExtensions{Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN} mfaData, err := a.ValidateMFAAuthResponse(ctx, mfaResponse, "" /* user */, requiredExt) - if err != nil { + switch { + // Don't obfuscate the SSO error. + case errors.Is(err, types.ErrPassswordlessLoginBySSOUser): + return nil, "", trace.Wrap(err) + case err != nil: log.Debugf("Passwordless authentication failed: %v", err) return nil, "", trace.Wrap(authenticateWebauthnError) } diff --git a/lib/auth/mocku2f/mocku2f.go b/lib/auth/mocku2f/mocku2f.go index 2299abddcd9..fe722e3414e 100644 --- a/lib/auth/mocku2f/mocku2f.go +++ b/lib/auth/mocku2f/mocku2f.go @@ -50,6 +50,11 @@ type Key struct { // Cert is the Key attestation certificate. Cert []byte + // UserHandle is the WebAuthn User ID. + // Saved from passwordless registrations and set on passwordless assertions. + // Requires a passwordless-configured Key (see [Key.SetPasswordless]). + UserHandle []byte + // PreferRPID instructs the Key to use favor using the RPID for Webauthn // ceremonies, even if the U2F App ID extension is present. PreferRPID bool diff --git a/lib/auth/mocku2f/webauthn.go b/lib/auth/mocku2f/webauthn.go index 93f164406a0..c98e4dfbcbe 100644 --- a/lib/auth/mocku2f/webauthn.go +++ b/lib/auth/mocku2f/webauthn.go @@ -79,6 +79,12 @@ func (muk *Key) SignAssertion(origin string, assertion *wantypes.CredentialAsser return nil, trace.Wrap(err) } + // If passwordless, then relay our WebAuthn UserHandle. + var userHandle []byte + if len(assertion.Response.AllowedCredentials) == 0 && len(muk.UserHandle) > 0 { + userHandle = muk.UserHandle + } + return &wantypes.CredentialAssertionResponse{ PublicKeyCredential: wantypes.PublicKeyCredential{ Credential: wantypes.Credential{ @@ -95,7 +101,8 @@ func (muk *Key) SignAssertion(origin string, assertion *wantypes.CredentialAsser }, AuthenticatorData: res.AuthData, // Signature starts after user presence (1byte) and counter (4 bytes). - Signature: res.SignData[5:], + Signature: res.SignData[5:], + UserHandle: userHandle, }, }, nil } @@ -128,7 +135,11 @@ func (muk *Key) SignCredentialCreation(origin string, cc *wantypes.CredentialCre if aa := cc.Response.AuthenticatorSelection.AuthenticatorAttachment; aa == protocol.Platform { return nil, trace.BadParameter("platform attachment required by authenticator selection") } - if rrk := cc.Response.AuthenticatorSelection.RequireResidentKey; rrk != nil && *rrk && !muk.AllowResidentKey { + rrk, err := cc.RequireResidentKey() + if err != nil { + return nil, trace.Wrap(err) + } + if rrk && !muk.AllowResidentKey { return nil, trace.BadParameter("resident key required by authenticator selection") } if uv := cc.Response.AuthenticatorSelection.UserVerification; uv == protocol.VerificationRequired && !muk.SetUV { @@ -184,6 +195,11 @@ func (muk *Key) SignCredentialCreation(origin string, cc *wantypes.CredentialCre return nil, trace.Wrap(err) } + // Save the WebAuthn UserHandle if this is a resident key creation. + if rrk && len(cc.Response.User.ID) > 0 && len(muk.UserHandle) == 0 { + muk.UserHandle = cc.Response.User.ID + } + return &wantypes.CredentialCreationResponse{ PublicKeyCredential: wantypes.PublicKeyCredential{ Credential: wantypes.Credential{ diff --git a/lib/auth/webauthn/login_test.go b/lib/auth/webauthn/login_test.go index 8e88caa5e6e..42acb7b7855 100644 --- a/lib/auth/webauthn/login_test.go +++ b/lib/auth/webauthn/login_test.go @@ -452,11 +452,6 @@ func TestPasswordlessFlow_BeginAndFinish(t *testing.T) { // User interaction would happen here. assertionResp, err := test.key.SignAssertion(test.origin, assertion) require.NoError(t, err) - // Fetch the stored user handle; in a real-world the scenario the - // authenticator knows it, as passwordless requires a resident credential. - wla, err := identity.GetWebauthnLocalAuth(ctx, test.user) - require.NoError(t, err) - assertionResp.AssertionResponse.UserHandle = wla.UserID // 2nd and last step of the login ceremony. loginData, err := webLogin.Finish(ctx, assertionResp) diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index e377353b7a3..7aef60d9e5f 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -2536,13 +2536,18 @@ func (h *Handler) mfaLoginFinishSession(w http.ResponseWriter, r *http.Request, clientMeta := clientMetaFromReq(r) session, err := h.auth.AuthenticateWebUser(r.Context(), req, clientMeta) - if err != nil { - // Since checking for private key policy meant that they passed authn, - // return policy error as is to help direct user. - if keys.IsPrivateKeyPolicyError(err) { - return nil, trace.Wrap(err) - } - // Obscure all other errors. + switch { + // Since checking for private key policy meant that they passed authn, + // return policy error as is to help direct user. + case keys.IsPrivateKeyPolicyError(err): + return nil, trace.Wrap(err) + + // Return a friendlier error if an SSO user tried to do passwordless. + case errors.Is(err, types.ErrPassswordlessLoginBySSOUser): + return nil, trace.Wrap(err) + + // Obscure all other errors. + case err != nil: return nil, trace.AccessDenied("invalid credentials") } diff --git a/lib/web/apiserver_login_test.go b/lib/web/apiserver_login_test.go index f7355388665..11eefb57325 100644 --- a/lib/web/apiserver_login_test.go +++ b/lib/web/apiserver_login_test.go @@ -33,6 +33,7 @@ import ( "github.com/stretchr/testify/require" "golang.org/x/crypto/ssh" + "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/constants" devicepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/devicetrust/v1" @@ -314,6 +315,115 @@ func TestAuthenticate_passwordless(t *testing.T) { }) } +// TestPasswordlessProhibitedForSSO is rather similar to +// lib/auth.TestPasswordlessProhibitedForSSO, but here our main concern is that +// error messages aren't obfuscated along the way. +func TestPasswordlessProhibitedForSSO(t *testing.T) { + env := newWebPack(t, 1) + + testServer := env.server + authServer := testServer.Auth() + proxyServer := env.proxies[0] + clock := env.clock + + // Prepare user and default devices. + mfa := configureClusterForMFA(t, env, &types.AuthPreferenceSpecV2{ + Type: constants.Local, + SecondFactor: constants.SecondFactorOn, + Webauthn: &types.Webauthn{ + RPID: testServer.ClusterName(), + }, + }) + user := mfa.User + ctx := context.Background() + + // Register a passwordless device. + userClient, err := testServer.NewClient(auth.TestUser(user)) + require.NoError(t, err, "NewClient failed") + pwdlessDev, err := auth.RegisterTestDevice( + ctx, userClient, "pwdless", proto.DeviceType_DEVICE_TYPE_WEBAUTHN, mfa.WebDev, auth.WithPasswordless()) + require.NoError(t, err, "RegisterTestDevice failed") + + // Update the user so it looks like an SSO user. + _, err = authServer.UpdateAndSwapUser(ctx, user, false /* withSecrets */, func(u types.User) (changed bool, err error) { + u.SetCreatedBy(types.CreatedBy{ + Connector: &types.ConnectorRef{ + Type: constants.Github, + ID: "github", + Identity: user, + }, + Time: clock.Now(), + User: types.UserRef{ + Name: teleport.UserSystem, + }, + }) + return true, nil + }) + require.NoError(t, err, "UpdateAndSwapUser failed") + + // Prepare SSH key to be signed. + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err, "GenerateKey failed") + pub, err := ssh.NewPublicKey(&priv.PublicKey) + require.NoError(t, err, "NewPublicKey failed") + pubBytes := ssh.MarshalAuthorizedKey(pub) + + webClient, err := client.NewWebClient( + proxyServer.webURL.String(), + roundtrip.HTTPClient(client.NewInsecureWebClient()), + ) + require.NoError(t, err, "NewWebClient failed") + + tests := []struct { + name string + login func(chalResp *wantypes.CredentialAssertionResponse) error + }{ + { + name: "web", + login: func(chalResp *wantypes.CredentialAssertionResponse) error { + ep := webClient.Endpoint("webapi", "mfa", "login", "finishsession") + _, err := webClient.PostJSON(ctx, ep, &client.AuthenticateWebUserRequest{ + WebauthnAssertionResponse: chalResp, + }) + return err + }, + }, + { + name: "ssh", + login: func(chalResp *wantypes.CredentialAssertionResponse) error { + ep := webClient.Endpoint("webapi", "mfa", "login", "finish") + _, err := webClient.PostJSON(ctx, ep, &client.AuthenticateSSHUserRequest{ + WebauthnChallengeResponse: chalResp, + PubKey: pubBytes, + TTL: 12 * time.Hour, + }) + return err + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // Issue passwordless challenge. + ep := webClient.Endpoint("webapi", "mfa", "login", "begin") + beginResp, err := webClient.PostJSON(ctx, ep, &client.MFAChallengeRequest{ + Passwordless: true, + }) + require.NoError(t, err, "POST /webapi/mfa/login/begin") + mfaChallenge := &client.MFAAuthenticateChallenge{} + require.NoError(t, json.Unmarshal(beginResp.Bytes(), mfaChallenge), "Unmarshal MFA challenge failed") + + // Sign it. + origin := "https://" + testServer.ClusterName() + chalResp, err := pwdlessDev.Key.SignAssertion(origin, mfaChallenge.WebauthnChallenge) + require.NoError(t, err, "SignAssertion failed") + + // Login and verify that the passwordless/SSO error was not obfuscated. + err = test.login(chalResp) + assert.ErrorIs(t, err, types.ErrPassswordlessLoginBySSOUser, "Login error mismatch") + }) + } +} + func TestAuthenticate_rateLimiting(t *testing.T) { ctx := context.Background() diff --git a/tool/tctl/common/admin_action_test.go b/tool/tctl/common/admin_action_test.go index b3d8443a6f0..d23d9ad740f 100644 --- a/tool/tctl/common/admin_action_test.go +++ b/tool/tctl/common/admin_action_test.go @@ -1187,8 +1187,6 @@ func setupWebAuthn(t *testing.T, authServer *auth.Server, username string) libcl require.NoError(t, err) cc := wantypes.CredentialCreationFromProto(res.GetWebauthn()) - userWebID := res.GetWebauthn().PublicKey.User.Id - ccr, err := device.SignCredentialCreation(origin, cc) require.NoError(t, err) _, err = authServer.ChangeUserAuthentication(ctx, &proto.ChangeUserAuthenticationRequest{ @@ -1206,7 +1204,6 @@ func setupWebAuthn(t *testing.T, authServer *auth.Server, username string) libcl if err != nil { return nil, "", err } - car.AssertionResponse.UserHandle = userWebID return &proto.MFAAuthenticateResponse{ Response: &proto.MFAAuthenticateResponse_Webauthn{