diff --git a/go.mod b/go.mod index c92dc708733ba..2cda5c0c73f73 100644 --- a/go.mod +++ b/go.mod @@ -302,7 +302,7 @@ replace ( github.com/gogo/protobuf => github.com/gravitational/protobuf v1.3.2-0.20201123192827-2b9fcfaffcbf github.com/gravitational/teleport/api => ./api github.com/julienschmidt/httprouter => github.com/gravitational/httprouter v1.3.1-0.20220408074523-c876c5e705a5 - github.com/keys-pub/go-libfido2 => github.com/gravitational/go-libfido2 v1.5.3-0.20220420140227-d3cb2f4b1e16 + github.com/keys-pub/go-libfido2 => github.com/gravitational/go-libfido2 v1.5.3-0.20220630200200-45a8c53e4500 github.com/russellhaering/gosaml2 => github.com/gravitational/gosaml2 v0.0.0-20220318224559-f06932032ae2 github.com/sirupsen/logrus => github.com/gravitational/logrus v1.4.4-0.20210817004754-047e20245621 github.com/vulcand/predicate => github.com/gravitational/predicate v1.2.1 diff --git a/go.sum b/go.sum index 82e4638c50949..38239bb27ce7a 100644 --- a/go.sum +++ b/go.sum @@ -545,8 +545,8 @@ github.com/gravitational/configure v0.0.0-20180808141939-c3428bd84c23 h1:havbccu github.com/gravitational/configure v0.0.0-20180808141939-c3428bd84c23/go.mod h1:XL9nebvlfNVvRzRPWdDcWootcyA0l7THiH/A+W1233g= github.com/gravitational/form v0.0.0-20151109031454-c4048f792f70 h1:To76nCJtM3DI0mdq3nGLzXqTV1wNOJByxv01+u9/BxM= github.com/gravitational/form v0.0.0-20151109031454-c4048f792f70/go.mod h1:88hFR45MpUd23d2vNWE/dYtesU50jKsbz0I9kH7UaBY= -github.com/gravitational/go-libfido2 v1.5.3-0.20220420140227-d3cb2f4b1e16 h1:eJtGFaUWU/TIQ7BC74lWCKxyDUcdHbr6nieyMRz21GY= -github.com/gravitational/go-libfido2 v1.5.3-0.20220420140227-d3cb2f4b1e16/go.mod h1:P0V19qHwJNY0htZwZDe9Ilvs/nokGhdFX7faKFyZ6+U= +github.com/gravitational/go-libfido2 v1.5.3-0.20220630200200-45a8c53e4500 h1:54z7/KbhT1dTmM1HnFQ5ggu5GZ4nUFARYaO6MNsxB1M= +github.com/gravitational/go-libfido2 v1.5.3-0.20220630200200-45a8c53e4500/go.mod h1:P0V19qHwJNY0htZwZDe9Ilvs/nokGhdFX7faKFyZ6+U= github.com/gravitational/go-mssqldb v0.11.1-0.20220509084309-3d41480ef74f h1:2x6F7hLm8XpDzV2cQL2yg3meJm7BtrWteEOnrq/pAwc= github.com/gravitational/go-mssqldb v0.11.1-0.20220509084309-3d41480ef74f/go.mod h1:iiK0YP1ZeepvmBQk/QpLEhhTNJgfzrpArPY/aFvc9yU= github.com/gravitational/go-mysql v1.5.0-teleport.1 h1:EyFryeiTYyP6KslLVp0Q5QTKwtUG5RioVrTIoP4pOuI= diff --git a/lib/auth/webauthncli/api.go b/lib/auth/webauthncli/api.go index 7b7dcf284c155..6ba46f58f633c 100644 --- a/lib/auth/webauthncli/api.go +++ b/lib/auth/webauthncli/api.go @@ -39,16 +39,9 @@ const ( // LoginOpts groups non-mandatory options for Login. type LoginOpts struct { // User is the desired credential username for login. - // If empty, Login may either choose a credential or error due to ambiguity. + // If empty, Login may either choose a credential or prompt the user for input + // (via LoginPrompt). User string - // OptimisticAssertion allows Login to skip credential listing and attempt - // to assert directly. The drawback of an optimistic assertion is that the - // authenticator chooses the login credential, so Login can't guarantee that - // the User field will be respected. The upside is that it saves a touch for - // some devices. - // Login may decide to forego optimistic assertions if it wouldn't save a - // touch. - OptimisticAssertion bool // AuthenticatorAttachment specifies the desired authenticator attachment. AuthenticatorAttachment AuthenticatorAttachment } diff --git a/lib/auth/webauthncli/fido2.go b/lib/auth/webauthncli/fido2.go index 5ee5d4c792ee7..384727658d904 100644 --- a/lib/auth/webauthncli/fido2.go +++ b/lib/auth/webauthncli/fido2.go @@ -48,9 +48,6 @@ type FIDODevice interface { // Cancel mirrors libfido2.Device.Cancel. Cancel() error - // Credentials mirrors libfido2.Device.Credentials. - Credentials(rpID string, pin string) ([]*libfido2.Credential, error) - // MakeCredential mirrors libfido2.Device.MakeCredential. MakeCredential( clientDataHash []byte, @@ -66,7 +63,7 @@ type FIDODevice interface { clientDataHash []byte, credentialIDs [][]byte, pin string, - opts *libfido2.AssertionOpts) (*libfido2.Assertion, error) + opts *libfido2.AssertionOpts) ([]*libfido2.Assertion, error) } // fidoDeviceLocations and fidoNewDevice are used to allow testing. @@ -105,7 +102,10 @@ func fido2Login( allowedCreds := assertion.Response.GetAllowedCredentialIDs() uv := assertion.Response.UserVerification == protocol.VerificationRequired - passwordless := len(allowedCreds) == 0 && uv + + // Presence of any allowed credential is interpreted as the user identity + // being partially established, aka non-passwordless. + passwordless := len(allowedCreds) == 0 // Prepare challenge data for the device. ccdJSON, err := json.Marshal(&CollectedClientData{ @@ -127,7 +127,6 @@ func fido2Login( // mu guards the variables below it. var mu sync.Mutex var assertionResp *libfido2.Assertion - var username string var usedAppID bool pathToRPID := &sync.Map{} // map[string]string @@ -155,25 +154,8 @@ func fido2Login( return true, nil } + user := opts.User deviceCallback := func(dev FIDODevice, info *deviceInfo, pin string) error { - creds := allowedCreds - var uName string - switch { - case passwordless && opts.OptimisticAssertion && info.bioEnroll: - log.Debugf("FIDO2: Using optimistic assertion for biometric device") - case passwordless: - cred, err := getPasswordlessCredentials(dev, pin, rpID, opts.User) - if err != nil { - return trace.Wrap(err) - } - creds = [][]byte{cred.ID} - uName = cred.User.Name - - // Ask for another touch before the assertion, we used the first touch - // in the Credentials() call. - prompt.PromptTouch() - } - actualRPID := rpID if val, ok := pathToRPID.Load(info.path); ok { actualRPID = val.(string) @@ -182,30 +164,39 @@ func fido2Login( opts := &libfido2.AssertionOpts{ UP: libfido2.True, } + // Note that "uv" fails for PIN-capable devices with an empty PIN. + // This is handled by runOnFIDO2Devices. if uv { opts.UV = libfido2.True } - resp, err := dev.Assertion(actualRPID, ccdHash[:], creds, pin, opts) + assertions, err := dev.Assertion(actualRPID, ccdHash[:], allowedCreds, pin, opts) + if err != nil { + return trace.Wrap(err) + } + log.Debugf("FIDO2: Got %v assertions", len(assertions)) + + // Find assertion for target user, or show the prompt. + assertion, err := pickAssertion(assertions, prompt, user, passwordless) if err != nil { return trace.Wrap(err) } + log.Debugf( - "FIDO2: Authenticated: credential ID (b64) = %v, user ID (hex) = %x, username = %q", - base64.RawURLEncoding.EncodeToString(resp.CredentialID), resp.User.ID, uName) + "FIDO2: Authenticated: credential ID (b64) = %v, user ID (hex) = %x, user name = %q", + base64.RawURLEncoding.EncodeToString(assertion.CredentialID), assertion.User.ID, assertion.User.Name) // Use the first successful assertion. // In practice it is very unlikely we'd hit this twice. mu.Lock() if assertionResp == nil { - assertionResp = resp - username = uName + assertionResp = assertion usedAppID = actualRPID != rpID } mu.Unlock() return nil } - if err := runOnFIDO2Devices(ctx, prompt, passwordless, filter, deviceCallback); err != nil { + if err := runOnFIDO2Devices(ctx, prompt, filter, deviceCallback); err != nil { return nil, "", trace.Wrap(err) } @@ -214,6 +205,12 @@ func fido2Login( return nil, "", trace.Wrap(err) } + // Trust the assertion user if present, otherwise go with the requested user. + actualUser := assertionResp.User.Name + if actualUser == "" { + actualUser = user + } + return &proto.MFAAuthenticateResponse{ Response: &proto.MFAAuthenticateResponse_Webauthn{ Webauthn: &wanpb.CredentialAssertionResponse{ @@ -230,56 +227,7 @@ func fido2Login( }, }, }, - }, username, nil -} - -func getPasswordlessCredentials(dev FIDODevice, pin, rpID, user string) (*libfido2.Credential, error) { - creds, err := dev.Credentials(rpID, pin) - if err != nil { - return nil, trace.Wrap(err) - } - - // TODO(codingllama): After this line we should cancel other devices, - // the user picked the current one. - - if user != "" { - log.Debugf("FIDO2: Searching credentials for user %q", user) - } - - switch { - case len(creds) == 0: - return nil, libfido2.ErrNoCredentials - case len(creds) == 1 && user == "": // no need to disambiguate - cred := creds[0] - log.Debugf("FIDO2: Found resident credential for user %q", cred.User.Name) - return cred, nil - case len(creds) > 1 && user == "": // can't disambiguate - return nil, trace.BadParameter("too many credentials found, explicit user required") - } - - duplicateWarning := false - var res *libfido2.Credential - for _, cred := range creds { - if cred.User.Name == user { - // Print information about matched credential, useful for debugging. - // ykman prints user IDs in hex, hence the unusual encoding choice below. - cID := base64.RawURLEncoding.EncodeToString(cred.ID) - uID := cred.User.ID - log.Debugf("FIDO2: Found resident credential for user %q, credential ID (b64) = %v, user ID (hex) = %x", user, cID, uID) - if res == nil { - res = cred - continue // Don't break, we want to warn about duplicates. - } - if !duplicateWarning { - duplicateWarning = true - log.Warnf("Found multiple credentials for %q, using first match", user) - } - } - } - if res == nil { - return nil, trace.BadParameter("no credentials for user %q", user) - } - return res, nil + }, actualUser, nil } func discoverRPID(dev FIDODevice, pin, rpID, appID string, allowedCreds [][]byte) (string, error) { @@ -300,6 +248,52 @@ func discoverRPID(dev FIDODevice, pin, rpID, appID string, allowedCreds [][]byte return "", libfido2.ErrNoCredentials } +func pickAssertion( + assertions []*libfido2.Assertion, prompt LoginPrompt, user string, passwordless bool) (*libfido2.Assertion, error) { + switch l := len(assertions); { + // Shouldn't happen, but let's be safe and handle it anyway. + case l == 0: + return nil, errors.New("authenticator returned empty assertions") + + // MFA or single credential (no explicit user). + case !passwordless, l == 1 && user == "": + return assertions[0], nil + + // Explicit user required. First occurrence wins. + case user != "": + for _, assertion := range assertions { + if assertion.User.Name == user { + return assertion, nil + } + } + return nil, fmt.Errorf("no credentials for user %q", user) + } + + // Prepare credentials and show picker. + creds := make([]*Credential, len(assertions)) + credToAssertion := make(map[*Credential]*libfido2.Assertion) + for i, assertion := range assertions { + cred := &Credential{ + ID: assertion.CredentialID, + User: User{ + ID: assertion.User.ID, + Name: assertion.User.Name, + }, + } + credToAssertion[cred] = assertion + creds[i] = cred + } + chosen, err := prompt.PromptCredential(creds) + if err != nil { + return nil, trace.Wrap(err) + } + assertion, ok := credToAssertion[chosen] + if !ok { + return nil, fmt.Errorf("prompt returned invalid credential: %#v", chosen) + } + return assertion, nil +} + // fido2Register implements FIDO2Register. func fido2Register( ctx context.Context, @@ -437,8 +431,7 @@ func fido2Register( return nil } - const passwordless = false - if err := runOnFIDO2Devices(ctx, prompt, passwordless, filter, deviceCallback); err != nil { + if err := runOnFIDO2Devices(ctx, prompt, filter, deviceCallback); err != nil { return nil, trace.Wrap(err) } @@ -522,11 +515,12 @@ type deviceFilterFunc func(dev FIDODevice, info *deviceInfo) (ok bool, err error type deviceCallbackFunc func(dev FIDODevice, info *deviceInfo, pin string) error // runPrompt defines the prompt operations necessary for runOnFIDO2Devices. -type runPrompt LoginPrompt +// (RegisterPrompt happens to match the minimal interface required.) +type runPrompt RegisterPrompt func runOnFIDO2Devices( ctx context.Context, - prompt runPrompt, passwordless bool, + prompt runPrompt, filter deviceFilterFunc, deviceCallback deviceCallbackFunc) error { // Do we have readily available devices? @@ -545,22 +539,15 @@ func runOnFIDO2Devices( return trace.Wrap(err) } - var dev deviceWithInfo - if !prompted && shouldDoEagerPINPrompt(passwordless, devices) { - dev = devices[0] // single device guaranteed in this case - } else { - if !prompted { - prompt.PromptTouch() // about to select - } - - d, requiresPIN, err := selectDevice(ctx, "" /* pin */, devices, deviceCallback) - switch { - case err != nil: - return trace.Wrap(err) - case !requiresPIN: - return nil - } - dev = d + if !prompted { + prompt.PromptTouch() // about to select + } + dev, requiresPIN, err := selectDevice(ctx, "" /* pin */, devices, deviceCallback) + switch { + case err != nil: + return trace.Wrap(err) + case !requiresPIN: + return nil } // Selected device requires PIN, let's use the prompt and run the callback @@ -573,11 +560,8 @@ func runOnFIDO2Devices( return libfido2.ErrPinRequired } - // Prompt again for a touch if MFA, but don't prompt for passwordless. - // The passwordless callback calls the prompt at a more appropriate time. - if !passwordless { - prompt.PromptTouch() - } + // Prompt a second touch after reading the PIN. + prompt.PromptTouch() // Run the callback again with the informed PIN. // selectDevice is used since it correctly deals with cancellation. @@ -585,20 +569,6 @@ func runOnFIDO2Devices( return trace.Wrap(err) } -func shouldDoEagerPINPrompt(passwordless bool, devices []deviceWithInfo) bool { - // Don't eagerly prompt for PIN if MFA, it usually doesn't require PINs. - // Also don't eagerly prompt if >1 device, the touch chooses the device and we - // can't know which device will be chosen. - if !passwordless || len(devices) > 1 { - return false - } - - // Only eagerly prompt for PINs if not bio, biometric devices unlock with - // touch instead (explicit PIN not required). - info := devices[0].info - return info.clientPinSet && !info.bioEnroll -} - func findSuitableDevicesOrTimeout( ctx context.Context, filter deviceFilterFunc, knownPaths map[string]struct{}) ([]deviceWithInfo, error) { ticker := time.NewTicker(FIDO2PollInterval) @@ -682,7 +652,16 @@ func selectDevice( callbackWrapper := func(dev FIDODevice, info *deviceInfo, pin string) (requiresPIN bool, err error) { // Attempt to select a device by running "deviceCallback" on it. // For most scenarios this works, saving a touch. - if err = deviceCallback(dev, info, pin); !errors.Is(err, libfido2.ErrPinRequired) { + err = deviceCallback(dev, info, pin) + switch { + case errors.Is(err, libfido2.ErrPinRequired): + // Continued below. + case errors.Is(err, libfido2.ErrUnsupportedOption) && pin == "" && !info.uv && info.clientPinSet: + // The failing option is likely to be "UV", so we handle this the same as + // ErrPinRequired: see if the user selects this device, ask for the PIN and + // try again. + // Continued below. + default: return } @@ -694,7 +673,7 @@ func selectDevice( // Another option is to put the authenticator into U2F mode. const rpID = "7f364cc0-958c-4177-b3ea-b2d8d7f15d4a" // arbitrary, unlikely to collide with a real RP const cdh = "00000000000000000000000000000000" // "random", size 32 - _, err = dev.Assertion(rpID, []byte(cdh), nil /* credentials */, pin, &libfido2.AssertionOpts{ + _, err = dev.Assertion(rpID, []byte(cdh), nil /* credentials */, "", &libfido2.AssertionOpts{ UP: libfido2.True, }) if errors.Is(err, libfido2.ErrNoCredentials) { @@ -752,7 +731,7 @@ func selectDevice( return resp.dev, resp.requiresPIN, trace.Wrap(resp.err) } -// deviceInfo contains an aggregate of a device's informations and capabilities. +// deviceInfo contains an aggregate of a device's information and capabilities. // Various fields match options under // https://fidoalliance.org/specs/fido-v2.1-ps-20210615/fido-client-to-authenticator-protocol-v2.1-ps-20210615.html#authenticatorGetInfo. type deviceInfo struct { diff --git a/lib/auth/webauthncli/fido2_common.go b/lib/auth/webauthncli/fido2_common.go index d49e56a295460..3fc64a8febfa7 100644 --- a/lib/auth/webauthncli/fido2_common.go +++ b/lib/auth/webauthncli/fido2_common.go @@ -30,6 +30,18 @@ import ( // FIDO2PollInterval is the poll interval used to check for new FIDO2 devices. var FIDO2PollInterval = 200 * time.Millisecond +// Credential represents a WebAuthn credential. +type Credential struct { + ID []byte + User User +} + +// User represents a credential user. +type User struct { + ID []byte + Name string +} + // LoginPrompt is the user interface for FIDO2Login. type LoginPrompt interface { // PromptPIN prompts the user for their PIN. @@ -38,6 +50,11 @@ type LoginPrompt interface { // In certain situations multiple touches may be required (PIN-protected // devices, passwordless flows, etc). PromptTouch() + // PromptCredential prompts the user to choose a credential, in case multiple + // credentials are available. + // Callers are free to modify the slice, such as by sorting the credentials, + // but must return one of the pointers contained within. + PromptCredential(creds []*Credential) (*Credential, error) } // FIDO2Login implements Login for CTAP1 and CTAP2 devices. diff --git a/lib/auth/webauthncli/fido2_prompt.go b/lib/auth/webauthncli/fido2_prompt.go index 5cabeeb323447..7c2c8b1ad278b 100644 --- a/lib/auth/webauthncli/fido2_prompt.go +++ b/lib/auth/webauthncli/fido2_prompt.go @@ -16,10 +16,14 @@ package webauthncli import ( "context" + "errors" "fmt" "io" + "sort" + "strconv" "github.com/gravitational/teleport/lib/utils/prompt" + "github.com/gravitational/trace" ) // DefaultPrompt is a default implementation for LoginPrompt and @@ -27,6 +31,7 @@ import ( type DefaultPrompt struct { PINMessage string FirstTouchMessage, SecondTouchMessage string + PromptCredentialMessage string ctx context.Context out io.Writer @@ -38,11 +43,12 @@ type DefaultPrompt struct { // customized by setting the appropriate fields. func NewDefaultPrompt(ctx context.Context, out io.Writer) *DefaultPrompt { return &DefaultPrompt{ - PINMessage: "Enter your security key PIN", - FirstTouchMessage: "Tap your security key", - SecondTouchMessage: "Tap your security key again to complete login", - ctx: ctx, - out: out, + PINMessage: "Enter your security key PIN", + FirstTouchMessage: "Tap your security key", + SecondTouchMessage: "Tap your security key again to complete login", + PromptCredentialMessage: "Choose the user for login", + ctx: ctx, + out: out, } } @@ -65,3 +71,41 @@ func (p *DefaultPrompt) PromptTouch() { fmt.Fprintln(p.out, p.SecondTouchMessage) } } + +// PromptCredential prompts the user to choose a credential, in case multiple +// credentials are available. +func (p *DefaultPrompt) PromptCredential(creds []*Credential) (*Credential, error) { + // Shouldn't happen, but let's check just in case. + if len(creds) == 0 { + return nil, errors.New("attempted to prompt credential with empty credentials") + } + + sort.Slice(creds, func(i, j int) bool { + c1 := creds[i] + c2 := creds[j] + return c1.User.Name < c2.User.Name + }) + for i, cred := range creds { + fmt.Fprintf(p.out, "[%v] %v\n", i+1, cred.User.Name) + } + + for { + numOrName, err := prompt.Input(p.ctx, p.out, prompt.Stdin(), p.PromptCredentialMessage) + if err != nil { + return nil, trace.Wrap(err) + } + + switch num, err := strconv.Atoi(numOrName); { + case err != nil: // See if a name was typed instead. + for _, cred := range creds { + if cred.User.Name == numOrName { + return cred, nil + } + } + case num >= 1 && num <= len(creds): // Valid number. + return creds[num-1], nil + } + + fmt.Fprintf(p.out, "Invalid user choice: %q", numOrName) + } +} diff --git a/lib/auth/webauthncli/fido2_prompt_test.go b/lib/auth/webauthncli/fido2_prompt_test.go new file mode 100644 index 0000000000000..7b9a9078fecf0 --- /dev/null +++ b/lib/auth/webauthncli/fido2_prompt_test.go @@ -0,0 +1,126 @@ +// Copyright 2022 Gravitational, Inc +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package webauthncli_test + +import ( + "context" + "strings" + "testing" + + "github.com/gravitational/teleport/lib/utils/prompt" + "github.com/stretchr/testify/assert" + + wancli "github.com/gravitational/teleport/lib/auth/webauthncli" +) + +func TestDefaultPrompt_PromptCredential(t *testing.T) { + oldStdin := prompt.Stdin() + t.Cleanup(func() { prompt.SetStdin(oldStdin) }) + + llamaCred := &wancli.Credential{ + User: wancli.User{ + Name: "llama", + }, + } + alpacaCred := &wancli.Credential{ + User: wancli.User{ + Name: "alpaca", + }, + } + camelCred := &wancli.Credential{ + User: wancli.User{ + Name: "camel", + }, + } + + ctx := context.Background() + + tests := []struct { + name string + fakeReader *prompt.FakeReader + ctx context.Context + creds []*wancli.Credential + wantCred *wancli.Credential + wantErr string + // Optional, verifies output text. + wantOut string + }{ + { + name: "credential by number (1)", + fakeReader: prompt.NewFakeReader().AddString("1"), // sorted by name + creds: []*wancli.Credential{llamaCred, alpacaCred, camelCred}, + wantCred: alpacaCred, + }, + { + name: "credential by number (2)", + fakeReader: prompt.NewFakeReader().AddString("3"), // sorted by name + creds: []*wancli.Credential{llamaCred, alpacaCred, camelCred}, + wantCred: llamaCred, + }, + { + name: "credential by name", + fakeReader: prompt.NewFakeReader().AddString("alpaca"), + creds: []*wancli.Credential{llamaCred, alpacaCred, camelCred}, + wantCred: alpacaCred, + }, + { + name: "loops until correct", + fakeReader: prompt.NewFakeReader(). + AddString("bad"). + AddString("5"). + AddString("llama"), + creds: []*wancli.Credential{llamaCred, alpacaCred, camelCred}, + wantCred: llamaCred, + }, + { + name: "NOK empty credentials errors", + fakeReader: prompt.NewFakeReader(), + creds: []*wancli.Credential{}, + wantErr: "empty credentials", + }, + { + name: "output text", + fakeReader: prompt.NewFakeReader().AddString("llama"), + creds: []*wancli.Credential{llamaCred, alpacaCred, camelCred}, + wantCred: llamaCred, + wantOut: `[1] alpaca +[2] camel +[3] llama +` + wancli.NewDefaultPrompt(ctx, nil).PromptCredentialMessage, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + prompt.SetStdin(test.fakeReader) + + out := &strings.Builder{} + p := wancli.NewDefaultPrompt(ctx, out) + got, err := p.PromptCredential(test.creds) + switch { + case err == nil && test.wantErr == "": // OK + case err == nil && test.wantErr != "": + fallthrough + case !strings.Contains(err.Error(), test.wantErr): + t.Fatalf("PromptCredential returned err = %v, want %q", err, test.wantErr) + } + assert.Equal(t, test.wantCred, got, "PromptCredential mismatch") + if test.wantOut != "" { + // Contains so we don't trip on punctuation from prompt.Input. + assert.Contains(t, out.String(), test.wantOut, "output mismatch") + } + }) + } +} diff --git a/lib/auth/webauthncli/fido2_test.go b/lib/auth/webauthncli/fido2_test.go index 80705e606cc7e..d72157f8c3ed7 100644 --- a/lib/auth/webauthncli/fido2_test.go +++ b/lib/auth/webauthncli/fido2_test.go @@ -18,7 +18,6 @@ package webauthncli_test import ( - "bytes" "context" "crypto/rand" "errors" @@ -106,7 +105,16 @@ var ( } ) -type noopPrompt struct{} +// simplePicker is a credential picker that always picks the first credential. +type simplePicker struct{} + +func (p simplePicker) PromptCredential(creds []*wancli.Credential) (*wancli.Credential, error) { + return creds[0], nil +} + +type noopPrompt struct { + simplePicker +} func (p noopPrompt) PromptPIN() (string, error) { return "", nil @@ -116,6 +124,8 @@ func (p noopPrompt) PromptTouch() {} // pinCancelPrompt exercises cancellation after device selection. type pinCancelPrompt struct { + simplePicker + pin string cancel context.CancelFunc } @@ -252,6 +262,7 @@ func TestFIDO2Login(t *testing.T) { // assertResponse and wantErr are mutually exclusive. assertResponse func(t *testing.T, resp *wanpb.CredentialAssertionResponse) wantErr string + wantUser string }{ { name: "single device", @@ -461,6 +472,7 @@ func TestFIDO2Login(t *testing.T) { assert.Equal(t, pin3.credentials[0].ID, resp.RawId, "RawId mismatch (want %q resident credential)", alpacaName) assert.Equal(t, alpacaID, resp.Response.UserHandle, "UserHandle mismatch (want %q)", alpacaName) }, + wantUser: alpacaName, }, { name: "passwordless biometric (llama)", @@ -480,6 +492,7 @@ func TestFIDO2Login(t *testing.T) { assert.Equal(t, bio2.credentials[0].ID, resp.RawId, "RawId mismatch (want %q resident credential)", llamaName) assert.Equal(t, llamaID, resp.Response.UserHandle, "UserHandle mismatch (want %q)", llamaName) }, + wantUser: llamaName, }, { name: "passwordless biometric (alpaca)", @@ -499,9 +512,27 @@ func TestFIDO2Login(t *testing.T) { assert.Equal(t, bio2.credentials[1].ID, resp.RawId, "RawId mismatch (want %q resident credential)", alpacaName) assert.Equal(t, alpacaID, resp.Response.UserHandle, "UserHandle mismatch (want %q)", alpacaName) }, + wantUser: alpacaName, }, { - name: "passwordless optimistic assertion", + name: "passwordless single-choice credential picker", + fido2: newFakeFIDO2(pin3), + setUP: pin3.setUP, + createAssertion: func() *wanlib.CredentialAssertion { + cp := *baseAssertion + cp.Response.AllowedCredentials = nil + cp.Response.UserVerification = protocol.VerificationRequired + return &cp + }, + prompt: pin3, + assertResponse: func(t *testing.T, resp *wanpb.CredentialAssertionResponse) { + assert.Equal(t, pin3.credentials[0].ID, resp.RawId, "RawId mismatch (want %q resident credential)", alpacaName) + assert.Equal(t, alpacaID, resp.Response.UserHandle, "UserHandle mismatch (want %q)", alpacaName) + }, + wantUser: alpacaName, + }, + { + name: "passwordless multi-choice credential picker", fido2: newFakeFIDO2(bio2), setUP: bio2.setUP, createAssertion: func() *wanlib.CredentialAssertion { @@ -510,17 +541,12 @@ func TestFIDO2Login(t *testing.T) { cp.Response.UserVerification = protocol.VerificationRequired return &cp }, - prompt: bio2, - opts: &wancli.LoginOpts{ - User: "", // ignored - OptimisticAssertion: true, - }, + prompt: bio2, // picks first credential from list. assertResponse: func(t *testing.T, resp *wanpb.CredentialAssertionResponse) { - // The fake authenticator always picks the first credential. - // Let's assert just to make sure the reply is consistent. assert.Equal(t, bio2.credentials[0].ID, resp.RawId, "RawId mismatch (want %q resident credential)", llamaName) assert.Equal(t, llamaID, resp.Response.UserHandle, "UserHandle mismatch (want %q)", llamaName) }, + wantUser: llamaName, }, { name: "NOK passwordless no credentials", @@ -535,22 +561,6 @@ func TestFIDO2Login(t *testing.T) { prompt: bio1, wantErr: libfido2.ErrNoCredentials.Error(), }, - { - name: "NOK passwordless ambiguous user", - fido2: newFakeFIDO2(bio2), - setUP: bio2.setUP, - createAssertion: func() *wanlib.CredentialAssertion { - cp := *baseAssertion - cp.Response.AllowedCredentials = nil - cp.Response.UserVerification = protocol.VerificationRequired - return &cp - }, - prompt: bio2, - opts: &wancli.LoginOpts{ - User: "", // >1 resident credential, can't pick unambiguous username. - }, - wantErr: "explicit user required", - }, { name: "NOK passwordless unknown user", fido2: newFakeFIDO2(bio2), @@ -591,10 +601,11 @@ func TestFIDO2Login(t *testing.T) { // Run FIDO2Login asynchronously, so we can fail the test if it hangs. // mfaResp and err checked below. var mfaResp *proto.MFAAuthenticateResponse + var actualUser string var err error done := make(chan struct{}) go func() { - mfaResp, _, err = wancli.FIDO2Login(ctx, origin, test.createAssertion(), prompt, test.opts) + mfaResp, actualUser, err = wancli.FIDO2Login(ctx, origin, test.createAssertion(), prompt, test.opts) close(done) }() select { @@ -637,6 +648,8 @@ func TestFIDO2Login(t *testing.T) { if test.assertResponse != nil { test.assertResponse(t, got) } + + assert.Equal(t, test.wantUser, actualUser, "actual user mismatch") } // Run tests against both "metered" and "non-metered" fake variants, so we @@ -743,11 +756,11 @@ func TestFIDO2Login_PromptTouch(t *testing.T) { wantTouches: 1, }, { - name: "Passwordless PIN plugged requires single touch", + name: "Passwordless PIN plugged requires two touches", fido2: newFakeFIDO2(pin1).withNonMeteredLocations(), assertion: pwdlessAssertion, prompt: pin1, - wantTouches: 1, + wantTouches: 2, }, { name: "Passwordless PIN not plugged requires two touches", @@ -757,24 +770,14 @@ func TestFIDO2Login_PromptTouch(t *testing.T) { wantTouches: 2, }, { - name: "Passwordless Bio with optimistic assertion requires single touch", - fido2: newFakeFIDO2(bio1), - assertion: pwdlessAssertion, - prompt: bio1, - opts: &wancli.LoginOpts{ - OptimisticAssertion: true, - }, - wantTouches: 1, - }, - { - name: "Passwordless Bio without optimistic assertion requires two touches", + name: "Passwordless Bio requires one touch", fido2: newFakeFIDO2(bio1), assertion: pwdlessAssertion, prompt: bio1, opts: &wancli.LoginOpts{ User: "llama", }, - wantTouches: 2, + wantTouches: 1, }, { name: "Passwordless with multiple devices requires two touches", @@ -1434,6 +1437,8 @@ func (f *fakeFIDO2) NewDevice(path string) (wancli.FIDODevice, error) { } type fakeFIDO2Device struct { + simplePicker + path string info *libfido2.DeviceInfo pin string @@ -1447,8 +1452,7 @@ type fakeFIDO2Device struct { key *mocku2f.Key pubKey []byte - // mu and cond guard up and cancel. - mu sync.Mutex + // cond guards up and cancel. cond *sync.Cond up, cancel bool } @@ -1480,7 +1484,7 @@ func newFIDO2Device(path, pin string, info *libfido2.DeviceInfo, creds ...*libfi cred.Type = libfido2.ES256 } - d := &fakeFIDO2Device{ + return &fakeFIDO2Device{ path: path, pin: pin, credentials: creds, @@ -1488,9 +1492,8 @@ func newFIDO2Device(path, pin string, info *libfido2.DeviceInfo, creds ...*libfi info: info, key: key, pubKey: pubKeyCBOR, - } - d.cond = sync.NewCond(&d.mu) - return d, nil + cond: sync.NewCond(&sync.Mutex{}), + }, nil } func (f *fakeFIDO2Device) PromptPIN() (string, error) { @@ -1514,34 +1517,20 @@ func (f *fakeFIDO2Device) Info() (*libfido2.DeviceInfo, error) { } func (f *fakeFIDO2Device) setUP() { - f.mu.Lock() + f.cond.L.Lock() f.up = true - f.mu.Unlock() + f.cond.L.Unlock() f.cond.Broadcast() } func (f *fakeFIDO2Device) Cancel() error { - f.mu.Lock() + f.cond.L.Lock() f.cancel = true - f.mu.Unlock() + f.cond.L.Unlock() f.cond.Broadcast() return nil } -func (f *fakeFIDO2Device) Credentials(rpID string, pin string) ([]*libfido2.Credential, error) { - if pin == "" && f.isBio() { - // Unlock with user interaction. - if err := f.maybeLockUntilInteraction(true); err != nil { - return nil, err - } - } else { - if err := f.validatePIN(pin); err != nil { - return nil, err - } - } - return f.credentials, nil -} - func (f *fakeFIDO2Device) MakeCredential( clientDataHash []byte, rp libfido2.RelyingParty, @@ -1612,7 +1601,7 @@ func (f *fakeFIDO2Device) Assertion( credentialIDs [][]byte, pin string, opts *libfido2.AssertionOpts, -) (*libfido2.Assertion, error) { +) ([]*libfido2.Assertion, error) { switch { case rpID == "": return nil, errors.New("rp.ID required") @@ -1620,12 +1609,20 @@ func (f *fakeFIDO2Device) Assertion( return nil, libfido2.ErrNoCredentials case len(clientDataHash) == 0: return nil, errors.New("clientDataHash required") - case opts.UV == libfido2.False: // can only be empty or true + } + + // Validate UV. + switch { + case opts.UV == "": // OK, actually works as false. + case opts.UV == libfido2.True && f.isBio(): // OK. + case opts.UV == libfido2.True && f.hasClientPin() && pin != "": // OK, doubles as UV. + default: // Anything else is invalid, including libfido2.False. return nil, libfido2.ErrUnsupportedOption } // Validate PIN only if present and UP is required. // This is in line with how current YubiKeys behave. + // TODO(codingllama): This should probably take UV into consideration. privilegedAccess := f.isBio() if pin != "" && opts.UP == libfido2.True { if err := f.validatePIN(pin); err != nil { @@ -1634,60 +1631,64 @@ func (f *fakeFIDO2Device) Assertion( privilegedAccess = true } - // Is our credential allowed? - foundCredential := false - var credID []byte - var userID []byte + // Block for user presence before accessing any credential data. + if err := f.maybeLockUntilInteraction(opts.UP == libfido2.True); err != nil { + return nil, err + } + + // Index credentialIDs for easier use. + credIDs := make(map[string]struct{}) for _, cred := range credentialIDs { - if bytes.Equal(cred, f.key.KeyHandle) { - foundCredential = true - credID = cred - break - } + credIDs[string(cred)] = struct{}{} + } - // Check resident credentials if we are properly authorized. - if !privilegedAccess { - continue - } + // Assemble one assertion for each allowed credential we hold. + var assertions []*libfido2.Assertion + + // "base" credential. Only add an assertion if explicitly requested. + if _, ok := credIDs[string(f.key.KeyHandle)]; ok { + assertions = append(assertions, &libfido2.Assertion{ + AuthDataCBOR: assertionAuthDataCBOR, + Sig: assertionSig, + CredentialID: f.key.KeyHandle, + User: libfido2.User{ + // We don't hold data about the user for the "base" credential / MFA + // scenario. + // A typical authenticator might choose to save some data within the + // key handle itself. + }, + }) + } + + // Resident credentials. + if privilegedAccess { for _, resident := range f.credentials { - if bytes.Equal(cred, resident.ID) { - foundCredential = true - credID = resident.ID - userID = resident.User.ID - break + allowed := len(credIDs) == 0 + if !allowed { + _, allowed = credIDs[string(resident.ID)] } - } - if foundCredential { - break + if !allowed { + continue + } + assertions = append(assertions, &libfido2.Assertion{ + AuthDataCBOR: assertionAuthDataCBOR, + Sig: assertionSig, + HMACSecret: []byte{}, + CredentialID: resident.ID, + User: libfido2.User{ + ID: resident.User.ID, + Name: resident.User.Name, + DisplayName: resident.User.DisplayName, + Icon: resident.User.Icon, + }, + }) } } - explicitCreds := len(credentialIDs) > 0 - if explicitCreds && !foundCredential { - return nil, libfido2.ErrNoCredentials - } - if err := f.maybeLockUntilInteraction(opts.UP == libfido2.True); err != nil { - return nil, err - } - - // Pick a credential for the user? - switch { - case !explicitCreds && privilegedAccess && len(f.credentials) > 0: - // OK, at this point an authenticator picks a credential for the user. - credID = f.credentials[0].ID - userID = f.credentials[0].User.ID - case !explicitCreds: + if len(assertions) == 0 { return nil, libfido2.ErrNoCredentials } - - return &libfido2.Assertion{ - AuthDataCBOR: assertionAuthDataCBOR, - Sig: assertionSig, - CredentialID: credID, - User: libfido2.User{ - ID: userID, - }, - }, nil + return assertions, nil } func (f *fakeFIDO2Device) validatePIN(pin string) error { @@ -1701,18 +1702,21 @@ func (f *fakeFIDO2Device) validatePIN(pin string) error { return nil } +func (f *fakeFIDO2Device) hasClientPin() bool { + return f.hasBoolOpt("clientPin") +} + func (f *fakeFIDO2Device) hasUV() bool { - for _, opt := range f.info.Options { - if opt.Name == "uv" { - return opt.Value == libfido2.True - } - } - return false + return f.hasBoolOpt("uv") } func (f *fakeFIDO2Device) isBio() bool { + return f.hasBoolOpt("bioEnroll") +} + +func (f *fakeFIDO2Device) hasBoolOpt(name string) bool { for _, opt := range f.info.Options { - if opt.Name == "bioEnroll" { + if opt.Name == name { return opt.Value == libfido2.True } } @@ -1725,10 +1729,11 @@ func (f *fakeFIDO2Device) maybeLockUntilInteraction(up bool) error { } // Lock until we get a touch or a cancel. - f.mu.Lock() + f.cond.L.Lock() for !f.up && !f.cancel { f.cond.Wait() } + defer f.cond.L.Unlock() // Record/reset state. isCancel := f.cancel @@ -1736,10 +1741,7 @@ func (f *fakeFIDO2Device) maybeLockUntilInteraction(up bool) error { f.cancel = false if isCancel { - f.mu.Unlock() return libfido2.ErrKeepaliveCancel } - f.mu.Unlock() - return nil } diff --git a/lib/client/api.go b/lib/client/api.go index 3b9ff4bd7b584..926b3c6540adf 100644 --- a/lib/client/api.go +++ b/lib/client/api.go @@ -3199,7 +3199,7 @@ func (tc *TeleportClient) Login(ctx context.Context) (*Key, error) { } var response *auth.SSHLoginResponse - + var username string switch authType := pr.Auth.Type; { case authType == constants.Local && pr.Auth.Local != nil && pr.Auth.Local.Name == constants.PasswordlessConnector: // Sanity check settings. @@ -3210,7 +3210,7 @@ func (tc *TeleportClient) Login(ctx context.Context) (*Key, error) { if err != nil { return nil, trace.Wrap(err) } - tc.Username = response.Username + username = response.Username case authType == constants.Local: response, err = tc.localLogin(ctx, pr.Auth.SecondFactor, key.Pub) if err != nil { @@ -3221,34 +3221,29 @@ func (tc *TeleportClient) Login(ctx context.Context) (*Key, error) { if err != nil { return nil, trace.Wrap(err) } - // in this case identity is returned by the proxy - tc.Username = response.Username - if tc.localAgent != nil { - tc.localAgent.username = response.Username - } + username = response.Username case authType == constants.SAML: response, err = tc.ssoLogin(ctx, pr.Auth.SAML.Name, key.Pub, constants.SAML) if err != nil { return nil, trace.Wrap(err) } - // in this case identity is returned by the proxy - tc.Username = response.Username - if tc.localAgent != nil { - tc.localAgent.username = response.Username - } + username = response.Username case authType == constants.Github: response, err = tc.ssoLogin(ctx, pr.Auth.Github.Name, key.Pub, constants.Github) if err != nil { return nil, trace.Wrap(err) } - // in this case identity is returned by the proxy - tc.Username = response.Username - if tc.localAgent != nil { - tc.localAgent.username = response.Username - } + username = response.Username default: return nil, trace.BadParameter("unsupported authentication type: %q", pr.Auth.Type) } + // Use proxy identity? + if username != "" { + tc.Username = username + if tc.localAgent != nil { + tc.localAgent.username = username + } + } // Check that a host certificate for at least one cluster was returned. if len(response.HostSigners) == 0 { @@ -3303,10 +3298,16 @@ func (tc *TeleportClient) pwdlessLogin(ctx context.Context, pubKey []byte) (*aut return nil, trace.BadParameter("passwordless: user verification requirement too lax (%v)", challenge.WebauthnChallenge.Response.UserVerification) } + // Only pass on the user if explicitly set, otherwise let the credential + // picker kick in. + user := "" + if tc.ExplicitUsername { + user = tc.Username + } + prompt := wancli.NewDefaultPrompt(ctx, tc.Stderr) mfaResp, _, err := promptWebauthn(ctx, webURL.String(), challenge.WebauthnChallenge, prompt, &wancli.LoginOpts{ - User: tc.Username, - OptimisticAssertion: !tc.ExplicitUsername, + User: user, AuthenticatorAttachment: tc.AuthenticatorAttachment, }) if err != nil { diff --git a/lib/client/mfa.go b/lib/client/mfa.go index f18eaac8b9181..8f154f075b9c9 100644 --- a/lib/client/mfa.go +++ b/lib/client/mfa.go @@ -219,9 +219,7 @@ func PromptMFAChallenge(ctx context.Context, c *proto.MFAAuthenticateChallenge, otpWait.Wait() }} - const user = "" resp, _, err := promptWebauthn(ctx, origin, wanlib.CredentialAssertionFromProto(c.WebauthnChallenge), mfaPrompt, &wancli.LoginOpts{ - User: user, AuthenticatorAttachment: opts.AuthenticatorAttachment, }) respC <- response{kind: "WEBAUTHN", resp: resp, err: err}