fix: Forbid SSO users from logging in using passwordless (#41062)

* Save the WebAuthn UserHandle in the mock Key

* Simplify passwordless tests

* fix: Forbid SSO users from logging in using passwordless

* Rename tests to TestPasswordlessProhibitedForSSO
This commit is contained in:
Alan Parra
2024-04-30 19:07:57 +00:00
committed by GitHub
parent 3ee2c51564
commit 72780dce68
12 changed files with 260 additions and 24 deletions
+6
View File
@@ -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
+3
View File
@@ -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())
}
+12
View File
@@ -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,
+88 -4
View File
@@ -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)
+1 -2
View File
@@ -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()))
+5 -1
View File
@@ -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)
}
+5
View File
@@ -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
+18 -2
View File
@@ -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{
-5
View File
@@ -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)
+12 -7
View File
@@ -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")
}
+110
View File
@@ -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()
-3
View File
@@ -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{