From 6e1955a67c63ae0fba20aae8474f7886b54ab180 Mon Sep 17 00:00:00 2001 From: Brian Joerger Date: Fri, 19 Jan 2024 16:07:50 -0800 Subject: [PATCH] Scoped WebAuthn: MFA extension flow (#36667) * Use SessionData with extensions in Webauthn flow. * Pass MFAChallengeExtensions through webauthn flow. * Opportunistically enforce Webauthn challenge scope. * Don't delete webauthn session data when reuse is allowed. * Return more login data from webauthn flow. * Enforce reuse when provided by the caller. * Address comments. * Fix test. * Add unit test for scope and reuse. * use pointer for challenge extension parameters. * Address comments. --- lib/auth/auth.go | 23 +- lib/auth/webauthn/login.go | 94 ++++++-- lib/auth/webauthn/login_mfa.go | 24 +- lib/auth/webauthn/login_passwordless.go | 15 +- lib/auth/webauthn/login_test.go | 284 ++++++++++++++++++++++-- lib/auth/webauthncli/u2f_login_test.go | 29 ++- 6 files changed, 400 insertions(+), 69 deletions(-) diff --git a/lib/auth/auth.go b/lib/auth/auth.go index a66967ba27739..1dbbe9678e5a7 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -66,6 +66,7 @@ import ( "github.com/gravitational/teleport/api/constants" apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/gen/proto/go/assist/v1" + mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/internalutils/stream" "github.com/gravitational/teleport/api/metadata" "github.com/gravitational/teleport/api/types" @@ -5806,7 +5807,12 @@ func (a *Server) mfaAuthChallenge(ctx context.Context, user string, passwordless Webauthn: webConfig, Identity: wanlib.WithDevices(a.Services, groupedDevs.Webauthn), } - assertion, err := webLogin.Begin(ctx, user) + // TODO(Joerger): Get extensions from caller. + ext := &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + } + assertion, err := webLogin.Begin(ctx, user, ext) if err != nil { return nil, trace.Wrap(err) } @@ -5919,25 +5925,32 @@ func (a *Server) ValidateMFAAuthResponse(ctx context.Context, resp *proto.MFAAut } assertionResp := wantypes.CredentialAssertionResponseFromProto(res.Webauthn) - var dev *types.MFADevice + var loginData *wanlib.LoginData if passwordless { webLogin := &wanlib.PasswordlessFlow{ Webauthn: webConfig, Identity: a.Services, } - dev, user, err = webLogin.Finish(ctx, assertionResp) + loginData, err = webLogin.Finish(ctx, assertionResp) } else { webLogin := &wanlib.LoginFlow{ U2F: u2f, Webauthn: webConfig, Identity: a.Services, } - dev, err = webLogin.Finish(ctx, user, wantypes.CredentialAssertionResponseFromProto(res.Webauthn)) + // TODO(Joerger): Get extensions from caller. + ext := &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + } + loginData, err = webLogin.Finish(ctx, user, wantypes.CredentialAssertionResponseFromProto(res.Webauthn), ext) } if err != nil { return nil, "", trace.AccessDenied("MFA response validation failed: %v", err) } - return dev, user, nil + // TODO(Joerger): Refactor Validate to also return AllowReuse + // for further validation by caller when necessary. + return loginData.Device, loginData.User, nil case *proto.MFAAuthenticateResponse_TOTP: dev, err := a.checkOTP(user, res.TOTP.Code) diff --git a/lib/auth/webauthn/login.go b/lib/auth/webauthn/login.go index ae4e71cbb5c5d..7d185b027c957 100644 --- a/lib/auth/webauthn/login.go +++ b/lib/auth/webauthn/login.go @@ -32,6 +32,7 @@ import ( "github.com/gravitational/trace" log "github.com/sirupsen/logrus" + mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/types" wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes" ) @@ -66,7 +67,16 @@ type loginFlow struct { sessionData sessionIdentity } -func (f *loginFlow) begin(ctx context.Context, user string, passwordless bool) (*wantypes.CredentialAssertion, error) { +func (f *loginFlow) begin(ctx context.Context, user string, challengeExtensions *mfav1.ChallengeExtensions) (*wantypes.CredentialAssertion, error) { + if challengeExtensions == nil { + return nil, trace.BadParameter("requested challenge extensions must be supplied.") + } + + if challengeExtensions.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES && challengeExtensions.Scope != mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION { + return nil, trace.BadParameter("mfa challenges with scope %s cannot allow reuse", challengeExtensions.Scope) + } + + passwordless := challengeExtensions.Scope == mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN if user == "" && !passwordless { return nil, trace.BadParameter("user required") } @@ -166,7 +176,7 @@ func (f *loginFlow) begin(ctx context.Context, user string, passwordless bool) ( if err != nil { return nil, trace.Wrap(err) } - // TODO(Joerger): set challenge extensions from caller + sd.ChallengeExtensions = challengeExtensions if err := f.sessionData.Upsert(ctx, user, sd); err != nil { return nil, trace.Wrap(err) @@ -186,44 +196,61 @@ func (f *loginFlow) getWebID(ctx context.Context, user string) ([]byte, error) { return wla.UserID, nil } -func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse, passwordless bool) (*types.MFADevice, string, error) { +// LoginData is data gathered from a successful webauthn login. +type LoginData struct { + // User is the Teleport user. + User string + // Device is the MFA device used to authenticate the user. + Device *types.MFADevice + // AllowReuse is whether the webauthn challenge used for this login + // can be reused by the user for subsequent logins, until it expires. + AllowReuse mfav1.ChallengeAllowReuse +} + +func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse, requiredExtensions *mfav1.ChallengeExtensions) (*LoginData, error) { + if requiredExtensions == nil { + return nil, trace.BadParameter("requested challenge extensions must be supplied.") + } + + passwordless := requiredExtensions.Scope == mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN + switch { case user == "" && !passwordless: - return nil, "", trace.BadParameter("user required") + return nil, trace.BadParameter("user required") case resp == nil: // resp != nil is good enough to proceed, we leave remaining validations to // duo-labs/webauthn. - return nil, "", trace.BadParameter("credential assertion response required") + return nil, trace.BadParameter("credential assertion response required") } parsedResp, err := parseCredentialResponse(resp) if err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } origin := parsedResp.Response.CollectedClientData.Origin if err := validateOrigin(origin, f.Webauthn.RPID); err != nil { log.WithError(err).Debugf("WebAuthn: origin validation failed") - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } var webID []byte if passwordless { webID = parsedResp.Response.UserHandle if len(webID) == 0 { - return nil, "", trace.BadParameter("webauthn user handle required for passwordless") + return nil, trace.BadParameter("webauthn user handle required for passwordless") } // Fetch user from WebAuthn UserHandle (aka User ID). teleportUser, err := f.identity.GetTeleportUserByWebauthnID(ctx, webID) if err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } user = teleportUser } else { webID, err = f.getWebID(ctx, user) if err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } } @@ -231,11 +258,11 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred // registered device. devices, err := f.identity.GetMFADevices(ctx, user, false /* withSecrets */) if err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } dev, ok := findDeviceByID(devices, parsedResp.RawID) if !ok { - return nil, "", trace.BadParameter( + return nil, trace.BadParameter( "unknown device credential: %q", base64.RawURLEncoding.EncodeToString(parsedResp.RawID)) } @@ -246,7 +273,7 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred rpID := f.Webauthn.RPID switch { case dev.GetU2F() != nil && f.U2F == nil: - return nil, "", trace.BadParameter("U2F device attempted login, but U2F configuration not present") + return nil, trace.BadParameter("U2F device attempted login, but U2F configuration not present") case dev.GetU2F() != nil: rpID = f.U2F.AppID } @@ -257,8 +284,23 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred challenge := parsedResp.Response.CollectedClientData.Challenge sd, err := f.sessionData.Get(ctx, user, challenge) if err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) + } + + // Check if the given scope is satisfied by the challenge scope. + if requiredExtensions.Scope != sd.ChallengeExtensions.Scope && requiredExtensions.Scope != mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED { + // old clients do not yet provide a scope, so we only enforce scope opportunistically. + // TODO(Joerger): DELETE IN v16.0.0 + if sd.ChallengeExtensions.Scope != mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED { + return nil, trace.AccessDenied("required scope %q is not satisfied by the given webauthn session with scope %q", requiredExtensions.Scope, sd.ChallengeExtensions.Scope) + } + } + + // If this session is reusable, but this login forbids reusable sessions, return an error. + if requiredExtensions.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO && sd.ChallengeExtensions.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES { + return nil, trace.AccessDenied("the given webauthn session allows reuse, but reuse is not permitted in this context") } + sessionData := wantypes.SessionDataToProtocol(sd) // Make sure _all_ credentials in the session are accounted for by the user. @@ -281,7 +323,7 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred requireUserVerification: passwordless, }) if err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } var credential *wan.Credential @@ -292,7 +334,7 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred credential, err = web.ValidateLogin(u, *sessionData, parsedResp) } if err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } if credential.Authenticator.CloneWarning { log.Warnf( @@ -301,7 +343,7 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred // Update last used timestamp and device counter. if err := setCounterAndTimestamps(dev, credential); err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } // Retroactively write the credential RPID, now that it cleared authn. if webDev := dev.GetWebauthn(); webDev != nil && webDev.CredentialRpId == "" { @@ -310,16 +352,24 @@ func (f *loginFlow) finish(ctx context.Context, user string, resp *wantypes.Cred } if err := f.identity.UpsertMFADevice(ctx, user, dev); err != nil { - return nil, "", trace.Wrap(err) + return nil, trace.Wrap(err) } // The user just solved the challenge, so let's make sure it won't be used - // again. - if err := f.sessionData.Delete(ctx, user, challenge); err != nil { - log.Warnf("WebAuthn: failed to delete login SessionData for user %v (passwordless = %v)", user, passwordless) + // again, unless reuse is explicitly allowed. + // Note that even reusable sessions are deleted when their expiration time + // passes. + if sd.ChallengeExtensions.AllowReuse != mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES { + if err := f.sessionData.Delete(ctx, user, challenge); err != nil { + log.Warnf("WebAuthn: failed to delete login SessionData for user %v (scope = %s)", user, sd.ChallengeExtensions.Scope) + } } - return dev, user, nil + return &LoginData{ + User: user, + Device: dev, + AllowReuse: sd.ChallengeExtensions.AllowReuse, + }, nil } func parseCredentialResponse(resp *wantypes.CredentialAssertionResponse) (*protocol.ParsedCredentialAssertionData, error) { diff --git a/lib/auth/webauthn/login_mfa.go b/lib/auth/webauthn/login_mfa.go index 8b083e4a0c706..fc5312f259e3a 100644 --- a/lib/auth/webauthn/login_mfa.go +++ b/lib/auth/webauthn/login_mfa.go @@ -22,8 +22,7 @@ import ( "context" "errors" - "github.com/gravitational/trace" - + mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/types" wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes" ) @@ -93,29 +92,34 @@ type LoginFlow struct { // assertion. // As a side effect Begin may assign (and record in storage) a WebAuthn ID for // the user. -func (f *LoginFlow) Begin(ctx context.Context, user string) (*wantypes.CredentialAssertion, error) { +// Requested challenge extensions will be stored on the stored webauthn challenge +// record. These extensions indicate additional rules/properties of the webauthn +// challenge that can be validated in the final login step. +func (f *LoginFlow) Begin(ctx context.Context, user string, challengeExtensions *mfav1.ChallengeExtensions) (*wantypes.CredentialAssertion, error) { lf := &loginFlow{ U2F: f.U2F, Webauthn: f.Webauthn, identity: mfaIdentity{f.Identity}, sessionData: (*userSessionStorage)(f), } - return lf.begin(ctx, user, false /* passwordless */) + return lf.begin(ctx, user, challengeExtensions) } // Finish is the second and last step of the LoginFlow. -// It returns the MFADevice used to solve the challenge. If login is successful, -// Finish has the side effect of updating the counter and last used timestamp of -// the returned device. -func (f *LoginFlow) Finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse) (*types.MFADevice, error) { +// Expected challenge extensions will be validated against the stored webauthn +// challenge record. +// It returns the MFADevice used to solve the challenge, the associated Teleport +// user name, and other login properties. If login is successful, Finish has the +// side effect of updating the counter and last used timestamp of the MFADevice +// used. +func (f *LoginFlow) Finish(ctx context.Context, user string, resp *wantypes.CredentialAssertionResponse, requiredExtensions *mfav1.ChallengeExtensions) (*LoginData, error) { lf := &loginFlow{ U2F: f.U2F, Webauthn: f.Webauthn, identity: mfaIdentity{f.Identity}, sessionData: (*userSessionStorage)(f), } - dev, _, err := lf.finish(ctx, user, resp, false /* passwordless */) - return dev, trace.Wrap(err) + return lf.finish(ctx, user, resp, requiredExtensions) } type mfaIdentity struct { diff --git a/lib/auth/webauthn/login_passwordless.go b/lib/auth/webauthn/login_passwordless.go index 0c28d0d5a463c..c79bbf638c8d0 100644 --- a/lib/auth/webauthn/login_passwordless.go +++ b/lib/auth/webauthn/login_passwordless.go @@ -23,6 +23,7 @@ import ( "encoding/base64" "errors" + mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/types" wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes" ) @@ -54,19 +55,27 @@ func (f *PasswordlessFlow) Begin(ctx context.Context) (*wantypes.CredentialAsser identity: passwordlessIdentity{f.Identity}, sessionData: (*globalSessionStorage)(f), } - return lf.begin(ctx, "" /* user */, true /* passwordless */) + chalExt := &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + } + return lf.begin(ctx, "" /* user */, chalExt) } // Finish is the last step of the passwordless login flow. // It works similarly to LoginFlow.Finish, but the user identity is established // via the response UserHandle, instead of an explicit Teleport username. -func (f *PasswordlessFlow) Finish(ctx context.Context, resp *wantypes.CredentialAssertionResponse) (*types.MFADevice, string, error) { +func (f *PasswordlessFlow) Finish(ctx context.Context, resp *wantypes.CredentialAssertionResponse) (*LoginData, error) { lf := &loginFlow{ Webauthn: f.Webauthn, identity: passwordlessIdentity{f.Identity}, sessionData: (*globalSessionStorage)(f), } - return lf.finish(ctx, "" /* user */, resp, true /* passwordless */) + requiredExt := &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + } + return lf.finish(ctx, "" /* user */, resp, requiredExt) } type passwordlessIdentity struct { diff --git a/lib/auth/webauthn/login_test.go b/lib/auth/webauthn/login_test.go index 7c3c0aefd827e..6f36b28cf8e05 100644 --- a/lib/auth/webauthn/login_test.go +++ b/lib/auth/webauthn/login_test.go @@ -31,6 +31,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/auth/mocku2f" wanlib "github.com/gravitational/teleport/lib/auth/webauthn" @@ -118,7 +119,9 @@ func TestLoginFlow_BeginFinish(t *testing.T) { } // 1st step of the login ceremony. - assertion, err := webLogin.Begin(ctx, user) + assertion, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) // We care about a few specific settings, for everything else defaults are // OK. @@ -146,15 +149,17 @@ func TestLoginFlow_BeginFinish(t *testing.T) { // 2nd and last step of the login ceremony. beforeLastUsed := time.Now().Add(-1 * time.Second) - loginDevice, err := webLogin.Finish(ctx, user, assertionResp) + loginData, err := webLogin.Finish(ctx, user, assertionResp, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) // Last used time and counter are updated. - require.True(t, beforeLastUsed.Before(loginDevice.LastUsed)) - require.Equal(t, wantCounter, getSignatureCounter(loginDevice)) + require.True(t, beforeLastUsed.Before(loginData.Device.LastUsed)) + require.Equal(t, wantCounter, getSignatureCounter(loginData.Device)) // Did we update the device in storage? require.NotEmpty(t, identity.UpdatedDevices) got := identity.UpdatedDevices[len(identity.UpdatedDevices)-1] - if diff := cmp.Diff(loginDevice, got); diff != "" { + if diff := cmp.Diff(loginData.Device, got); diff != "" { t.Errorf("Updated device mismatch (-want +got):\n%s", diff) } // Did we delete the challenge? @@ -220,7 +225,9 @@ func TestLoginFlow_Begin_errors(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - _, err := webLogin.Begin(ctx, test.user) + _, err := webLogin.Begin(ctx, test.user, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.True(t, test.assertErrType(err), "got err = %v, want BadParameter", err) require.Contains(t, err.Error(), test.wantErr) }) @@ -258,7 +265,9 @@ func TestLoginFlow_Finish_errors(t *testing.T) { Webauthn: webConfig, Identity: identity, } - assertion, err := webLogin.Begin(ctx, user) + assertion, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) okResp, err := key.SignAssertion(webOrigin, assertion) require.NoError(t, err) @@ -287,7 +296,9 @@ func TestLoginFlow_Finish_errors(t *testing.T) { name: "NOK assertion with bad origin", user: user, createResp: func() *wantypes.CredentialAssertionResponse { - assertion, err := webLogin.Begin(ctx, user) + assertion, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) resp, err := key.SignAssertion("https://badorigin.com", assertion) require.NoError(t, err) @@ -298,7 +309,9 @@ func TestLoginFlow_Finish_errors(t *testing.T) { name: "NOK assertion with bad RPID", user: user, createResp: func() *wantypes.CredentialAssertionResponse { - assertion, err := webLogin.Begin(ctx, user) + assertion, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) assertion.Response.RelyingPartyID = "badrpid.com" @@ -311,7 +324,9 @@ func TestLoginFlow_Finish_errors(t *testing.T) { name: "NOK assertion signed by unknown device", user: user, createResp: func() *wantypes.CredentialAssertionResponse { - assertion, err := webLogin.Begin(ctx, user) + assertion, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) unknownKey, err := mocku2f.Create() @@ -328,7 +343,9 @@ func TestLoginFlow_Finish_errors(t *testing.T) { name: "NOK assertion with invalid signature", user: user, createResp: func() *wantypes.CredentialAssertionResponse { - assertion, err := webLogin.Begin(ctx, user) + assertion, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) // Flip a challenge bit, this should be enough to consistently fail // signature checking. @@ -342,7 +359,9 @@ func TestLoginFlow_Finish_errors(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - _, err := webLogin.Finish(ctx, test.user, test.createResp()) + _, err := webLogin.Finish(ctx, test.user, test.createResp(), &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.Error(t, err) }) } @@ -421,6 +440,10 @@ func TestPasswordlessFlow_BeginAndFinish(t *testing.T) { AllowCredentials: [][]uint8{}, // aka unset ResidentKey: false, // irrelevant for login UserVerification: string(protocol.VerificationRequired), + ChallengeExtensions: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_PASSWORDLESS_LOGIN, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + }, } if diff := cmp.Diff(wantSD, sd); diff != "" { t.Fatalf("SessionData mismatch (-want +got):\n%s", diff) @@ -436,10 +459,10 @@ func TestPasswordlessFlow_BeginAndFinish(t *testing.T) { assertionResp.AssertionResponse.UserHandle = wla.UserID // 2nd and last step of the login ceremony. - mfaDevice, user, err := webLogin.Finish(ctx, assertionResp) + loginData, err := webLogin.Finish(ctx, assertionResp) require.NoError(t, err) - require.NotNil(t, mfaDevice) - require.Equal(t, test.user, user) + require.NotNil(t, loginData.Device) + require.Equal(t, test.user, loginData.User) }) } } @@ -498,7 +521,7 @@ func TestPasswordlessFlow_Finish_errors(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - _, _, err := webLogin.Finish(ctx, test.createResp()) + _, err := webLogin.Finish(ctx, test.createResp()) require.True(t, test.assertErrType(err), "assertErrType failed, err = %v", err) require.Contains(t, err.Error(), test.wantErrMsg) }) @@ -565,7 +588,9 @@ func TestCredentialRPID(t *testing.T) { Identity: identity, } - _, err := webLogin.Begin(ctx, user) + _, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) assert.NoError(t, err, "Begin failed, expected assertion for `dev1`") }) @@ -575,15 +600,19 @@ func TestCredentialRPID(t *testing.T) { Identity: identity, } - assertion, err := webLogin.Begin(ctx, user) + assertion, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err, "Begin failed") car, err := dev1Key.SignAssertion(origin, assertion) require.NoError(t, err, "SignAssertion failed") - mfaDev, err := webLogin.Finish(ctx, user, car) + loginData, err := webLogin.Finish(ctx, user, car, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err, "Finish failed") - assert.Equal(t, rpID, mfaDev.GetWebauthn().CredentialRpId, "CredentialRpId mismatch") + assert.Equal(t, rpID, loginData.Device.GetWebauthn().CredentialRpId, "CredentialRpId mismatch") }) t.Run("login doesn't issue challenges for the wrong RPIDs", func(t *testing.T) { @@ -592,7 +621,9 @@ func TestCredentialRPID(t *testing.T) { Identity: identity, } - _, err := webLogin.Begin(ctx, user) + _, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) assert.ErrorIs(t, err, wanlib.ErrInvalidCredentials, "Begin error mismatch") }) @@ -609,7 +640,9 @@ func TestCredentialRPID(t *testing.T) { Webauthn: webOtherRP, Identity: identity, } - assertion, err := webLogin.Begin(ctx, user) + assertion, err := webLogin.Begin(ctx, user, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err, "Begin failed, expected assertion for device `other1`") // Verify that we got the correct device. @@ -621,6 +654,213 @@ func TestCredentialRPID(t *testing.T) { }) } +func TestLogin_scopeAndReuse(t *testing.T) { + // webUser gets a newly registered device and a webID. + const webUser = "llama" + webIdentity := newFakeIdentity(webUser) + webConfig := &types.Webauthn{RPID: "example.com"} + + const webOrigin = "https://example.com" + ctx := context.Background() + + // Register a Webauthn device. + // Last registration step creates the user webID and adds the new device to + // identity. + webKey, err := mocku2f.Create() + require.NoError(t, err) + webKey.PreferRPID = true // Webauthn-registered device + webRegistration := &wanlib.RegistrationFlow{ + Webauthn: webConfig, + Identity: webIdentity, + } + cc, err := webRegistration.Begin(ctx, webUser, false /* passwordless */) + require.NoError(t, err) + ccr, err := webKey.SignCredentialCreation(webOrigin, cc) + require.NoError(t, err) + device, err := webRegistration.Finish(ctx, wanlib.RegisterResponse{ + User: webUser, + DeviceName: "webauthn1", + CreationResponse: ccr, + }) + require.NoError(t, err) + + t.Run("Begin", func(t *testing.T) { + tests := []struct { + name string + challengeExt *mfav1.ChallengeExtensions + assertErr require.ErrorAssertionFunc + }{ + { + name: "NOK challenge extensions not provided", + challengeExt: nil, + assertErr: func(tt require.TestingT, err error, i ...interface{}) { + require.True(t, trace.IsBadParameter(err), "expected bad parameter err but got %T", err) + require.ErrorContains(t, err, "extensions must be supplied") + }, + }, + { + name: "NOK reuse not allowed for scope", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, + assertErr: func(tt require.TestingT, err error, i ...interface{}) { + require.True(t, trace.IsBadParameter(err), "expected bad parameter err but got %T", err) + require.ErrorContains(t, err, "cannot allow reuse") + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + user := webUser + + webLogin := &wanlib.LoginFlow{ + Webauthn: webConfig, + Identity: webIdentity, + } + + _, err := webLogin.Begin(ctx, user, test.challengeExt) + if test.assertErr != nil { + test.assertErr(t, err) + return + } + require.NoError(t, err) + }) + } + }) + + t.Run("Finish", func(t *testing.T) { + tests := []struct { + name string + challengeExt *mfav1.ChallengeExtensions + requiredExt *mfav1.ChallengeExtensions + assertErr require.ErrorAssertionFunc + }{ + { + name: "NOK required challenge extensions not provided", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + requiredExt: nil, + assertErr: func(tt require.TestingT, err error, i ...interface{}) { + require.True(t, trace.IsBadParameter(err), "expected bad parameter err but got %T", err) + require.ErrorContains(t, err, "extensions must be supplied") + }, + }, { + name: "NOK scope not satisfied", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + requiredExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }, + assertErr: func(tt require.TestingT, err error, i ...interface{}) { + require.True(t, trace.IsAccessDenied(err), "expected access denied err but got %T", err) + require.ErrorContains(t, err, "is not satisfied") + }, + }, { + // Old clients do not yet provide a scope, so we only enforce scope + // opportunistically during login finish. + // TODO(Joerger): DELETE IN v16.0.0 - change to NOK + name: "OK scope not specified", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, + }, + requiredExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + }, { + name: "OK scope not required", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + requiredExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_UNSPECIFIED, + }, + }, { + name: "OK required scope satisfied", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + requiredExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + }, + }, { + name: "NOK reuse requested but not allowed", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, + requiredExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + }, + assertErr: func(tt require.TestingT, err error, i ...interface{}) { + require.True(t, trace.IsAccessDenied(err), "expected access denied err but got %T", err) + require.ErrorContains(t, err, "reuse is not permitted") + }, + }, { + name: "OK reuse not requested but allowed", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_NO, + }, + requiredExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, + }, { + name: "OK reuse requested and allowed", + challengeExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, + requiredExt: &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_ADMIN_ACTION, + AllowReuse: mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES, + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + identity := webIdentity + user := webUser + + webLogin := &wanlib.LoginFlow{ + Webauthn: webConfig, + Identity: webIdentity, + } + + assertion, err := webLogin.Begin(ctx, user, test.challengeExt) + require.NoError(t, err) + + assertionResp, err := webKey.SignAssertion(webOrigin, assertion) + require.NoError(t, err) + + loginData, err := webLogin.Finish(ctx, user, assertionResp, test.requiredExt) + if test.assertErr != nil { + test.assertErr(t, err) + return + } + + require.NoError(t, err) + require.Equal(t, loginData, &wanlib.LoginData{ + Device: device, + User: user, + AllowReuse: loginData.AllowReuse, + }) + + // Session data should only be deleted if reuse was not requested on begin. + if test.challengeExt.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES { + require.NotEmpty(t, identity.SessionData) + } else { + require.Empty(t, identity.SessionData) + } + }) + } + }) +} + type fakeIdentity struct { User *types.UserV2 // MappedUser is used as the reply to GetTeleportUserByWebauthnID. diff --git a/lib/auth/webauthncli/u2f_login_test.go b/lib/auth/webauthncli/u2f_login_test.go index cabd5d8895782..082979d0da0f9 100644 --- a/lib/auth/webauthncli/u2f_login_test.go +++ b/lib/auth/webauthncli/u2f_login_test.go @@ -36,6 +36,7 @@ import ( "github.com/gravitational/trace" "github.com/stretchr/testify/require" + mfav1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/mfa/v1" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/auth/mocku2f" wanlib "github.com/gravitational/teleport/lib/auth/webauthn" @@ -140,7 +141,9 @@ func TestLogin(t *testing.T) { } test.setUserPresence.SetUserPresence(true) - assertion, err := loginFlow.Begin(ctx, username) + assertion, err := loginFlow.Begin(ctx, username, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) if test.removeAppID { assertion.Response.Extensions = nil @@ -159,7 +162,9 @@ func TestLogin(t *testing.T) { require.NotNil(t, mfaResp.GetWebauthn()) require.Equal(t, test.wantRawID, mfaResp.GetWebauthn().RawId) - _, err = loginFlow.Finish(ctx, username, wantypes.CredentialAssertionResponseFromProto(mfaResp.GetWebauthn())) + _, err = loginFlow.Finish(ctx, username, wantypes.CredentialAssertionResponseFromProto(mfaResp.GetWebauthn()), &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) }) } @@ -182,7 +187,9 @@ func TestLogin_errors(t *testing.T) { const user = "llama" const origin = "https://localhost" ctx := context.Background() - okAssertion, err := loginFlow.Begin(ctx, user) + okAssertion, err := loginFlow.Begin(ctx, user, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) tests := []struct { @@ -215,7 +222,9 @@ func TestLogin_errors(t *testing.T) { name: "NOK assertion missing challenge", origin: origin, getAssertion: func() *wantypes.CredentialAssertion { - assertion, err := loginFlow.Begin(ctx, user) + assertion, err := loginFlow.Begin(ctx, user, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) assertion.Response.Challenge = nil return assertion @@ -225,7 +234,9 @@ func TestLogin_errors(t *testing.T) { name: "NOK assertion missing RPID", origin: origin, getAssertion: func() *wantypes.CredentialAssertion { - assertion, err := loginFlow.Begin(ctx, user) + assertion, err := loginFlow.Begin(ctx, user, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) assertion.Response.RelyingPartyID = "" return assertion @@ -235,7 +246,9 @@ func TestLogin_errors(t *testing.T) { name: "NOK assertion missing credentials", origin: origin, getAssertion: func() *wantypes.CredentialAssertion { - assertion, err := loginFlow.Begin(ctx, user) + assertion, err := loginFlow.Begin(ctx, user, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) assertion.Response.AllowedCredentials = nil return assertion @@ -245,7 +258,9 @@ func TestLogin_errors(t *testing.T) { name: "NOK assertion invalid user verification requirement", origin: origin, getAssertion: func() *wantypes.CredentialAssertion { - assertion, err := loginFlow.Begin(ctx, user) + assertion, err := loginFlow.Begin(ctx, user, &mfav1.ChallengeExtensions{ + Scope: mfav1.ChallengeScope_CHALLENGE_SCOPE_LOGIN, + }) require.NoError(t, err) assertion.Response.UserVerification = protocol.VerificationRequired return assertion