From 0b34833f79336770d2c4454176369b75d407e014 Mon Sep 17 00:00:00 2001 From: Alan Parra Date: Wed, 11 May 2022 12:25:47 -0300 Subject: [PATCH] Add `tsh touchid ls` and `rm` commands (#12505) Implement touch ID credential management via tsh touchid ls and tsh touchid rm. Departs slightly from RFD command names in order to better match the tsh mfa. See https://github.com/gravitational/teleport/blob/master/rfd/0054-passwordless-macos.md. #9160 * Implement touch ID credential listing * Add the `tsh touchid ls` command * Implement touch ID credential deletion * Add the `tsh touchid rm` command * Delegate MFA prompts to WebAuthn * Undo changes to tsh.go command switch * Prompt newline. Trace errors. * Update e/ to 6abb96b * Var initialization. Guard against NULL. Return all credentials. * Address review comments: simplifications and style --- e | 2 +- lib/auth/touchid/api.go | 39 ++++++++++- lib/auth/touchid/api_darwin.go | 65 +++++++++++++++---- lib/auth/touchid/api_other.go | 8 +++ lib/auth/touchid/api_test.go | 9 +++ lib/auth/touchid/authenticate.m | 4 +- lib/auth/touchid/common.m | 11 ++-- lib/auth/touchid/credentials.h | 13 ++++ lib/auth/touchid/credentials.m | 98 ++++++++++++++++++++++++++-- lib/auth/touchid/register.m | 4 +- lib/client/mfa.go | 11 +++- tool/tsh/touchid.go | 111 ++++++++++++++++++++++++++++++++ tool/tsh/tsh.go | 19 +++++- 13 files changed, 364 insertions(+), 30 deletions(-) create mode 100644 tool/tsh/touchid.go diff --git a/e b/e index cf63aa7dfb15d..6abb96b3b4b34 160000 --- a/e +++ b/e @@ -1 +1 @@ -Subproject commit cf63aa7dfb15dfd5f69ff8311cf2493ae85fb907 +Subproject commit 6abb96b3b4b340608bb5a65f4cbb71e845bdb95b diff --git a/lib/auth/touchid/api.go b/lib/auth/touchid/api.go index 0d8d33d366618..ca4c3301dba0a 100644 --- a/lib/auth/touchid/api.go +++ b/lib/auth/touchid/api.go @@ -32,6 +32,7 @@ import ( "github.com/gravitational/trace" wanlib "github.com/gravitational/teleport/lib/auth/webauthn" + log "github.com/sirupsen/logrus" ) var ( @@ -50,6 +51,12 @@ type nativeTID interface { // FindCredentials finds credentials without user interaction. // An empty user means "all users". FindCredentials(rpID, user string) ([]CredentialInfo, error) + + // ListCredentials lists all registered credentials. + // Requires user interaction. + ListCredentials() ([]CredentialInfo, error) + + DeleteCredential(credentialID string) error } // CredentialInfo holds information about a Secure Enclave credential. @@ -322,7 +329,7 @@ func Login(origin, user string, assertion *wanlib.CredentialAssertion) (*wanlib. infos, err := native.FindCredentials(rpID, user) switch { case err != nil: - return nil, "", err + return nil, "", trace.Wrap(err) case len(infos) == 0: return nil, "", ErrCredentialNotFound } @@ -373,3 +380,33 @@ func Login(origin, user string, assertion *wanlib.CredentialAssertion) (*wanlib. }, }, cred.User, nil } + +// ListCredentials lists all registered Secure Enclave credentials. +// Requires user interaction. +func ListCredentials() ([]CredentialInfo, error) { + // Skipped IsAvailable check in favor of a direct call to native. + infos, err := native.ListCredentials() + if err != nil { + return nil, trace.Wrap(err) + } + + // Parse public keys. + for i := range infos { + info := &infos[i] + key, err := pubKeyFromRawAppleKey(info.publicKeyRaw) + if err != nil { + log.Warnf("Failed to convert public key: %v", err) + } + info.PublicKey = key // this is OK, even if it's nil + info.publicKeyRaw = nil + } + + return infos, nil +} + +// DeleteCredential deletes a Secure Enclave credential. +// Requires user interaction. +func DeleteCredential(credentialID string) error { + // Skipped IsAvailable check in favor of a direct call to native. + return native.DeleteCredential(credentialID) +} diff --git a/lib/auth/touchid/api_darwin.go b/lib/auth/touchid/api_darwin.go index fbcadbb586e0d..260ca4c7df95d 100644 --- a/lib/auth/touchid/api_darwin.go +++ b/lib/auth/touchid/api_darwin.go @@ -29,7 +29,6 @@ import "C" import ( "encoding/base64" "errors" - "fmt" "strings" "unsafe" @@ -130,27 +129,46 @@ func (touchIDImpl) Authenticate(credentialID string, digest []byte) ([]byte, err } func (touchIDImpl) FindCredentials(rpID, user string) ([]CredentialInfo, error) { - infos, res := findCredentialsImpl(rpID, user, func(filter C.LabelFilter, infosC **C.CredentialInfo) C.int { - return C.FindCredentials(filter, infosC) + var filterC C.LabelFilter + if user == "" { + filterC.kind = C.LABEL_PREFIX + } + filterC.value = C.CString(makeLabel(rpID, user)) + defer C.free(unsafe.Pointer(filterC.value)) + + infos, res := readCredentialInfos(func(infosC **C.CredentialInfo) C.int { + return C.FindCredentials(filterC, infosC) }) if res < 0 { - return nil, fmt.Errorf("failed to find credentials: status %d", res) + return nil, trace.BadParameter("failed to find credentials: status %d", res) } return infos, nil } -func findCredentialsImpl(rpID, user string, find func(C.LabelFilter, **C.CredentialInfo) C.int) ([]CredentialInfo, int) { - var filterC C.LabelFilter - if user == "" { - filterC.kind = C.LABEL_PREFIX +func (touchIDImpl) ListCredentials() ([]CredentialInfo, error) { + // User prompt becomes: ""$binary" is trying to list credentials". + reasonC := C.CString("list credentials") + defer C.free(unsafe.Pointer(reasonC)) + + var errMsgC *C.char + defer C.free(unsafe.Pointer(errMsgC)) + + infos, res := readCredentialInfos(func(infosOut **C.CredentialInfo) C.int { + return C.ListCredentials(reasonC, infosOut, &errMsgC) + }) + if res < 0 { + errMsg := C.GoString(errMsgC) + return nil, errors.New(errMsg) } - filterC.value = C.CString(makeLabel(rpID, user)) - defer C.free(unsafe.Pointer(filterC.value)) + return infos, nil +} + +func readCredentialInfos(find func(**C.CredentialInfo) C.int) ([]CredentialInfo, int) { var infosC *C.CredentialInfo defer C.free(unsafe.Pointer(infosC)) - res := find(filterC, &infosC) + res := find(&infosC) if res < 0 { return nil, int(res) } @@ -211,3 +229,28 @@ func findCredentialsImpl(rpID, user string, find func(C.LabelFilter, **C.Credent } return infos, int(res) } + +// https://osstatus.com/search/results?framework=Security&search=-25300 +const errSecItemNotFound = -25300 + +func (touchIDImpl) DeleteCredential(credentialID string) error { + // User prompt becomes: ""$binary" is trying to delete credential". + reasonC := C.CString("delete credential") + defer C.free(unsafe.Pointer(reasonC)) + + idC := C.CString(credentialID) + defer C.free(unsafe.Pointer(idC)) + + var errC *C.char + defer C.free(unsafe.Pointer(errC)) + + switch C.DeleteCredential(reasonC, idC, &errC) { + case 0: // aka success + return nil + case errSecItemNotFound: + return ErrCredentialNotFound + default: + errMsg := C.GoString(errC) + return errors.New(errMsg) + } +} diff --git a/lib/auth/touchid/api_other.go b/lib/auth/touchid/api_other.go index 17c95d961fe2f..91440196fb002 100644 --- a/lib/auth/touchid/api_other.go +++ b/lib/auth/touchid/api_other.go @@ -36,3 +36,11 @@ func (noopNative) Authenticate(credentialID string, digest []byte) ([]byte, erro func (noopNative) FindCredentials(rpID, user string) ([]CredentialInfo, error) { return nil, ErrNotAvailable } + +func (noopNative) ListCredentials() ([]CredentialInfo, error) { + return nil, ErrNotAvailable +} + +func (noopNative) DeleteCredential(credentialID string) error { + return ErrNotAvailable +} diff --git a/lib/auth/touchid/api_test.go b/lib/auth/touchid/api_test.go index 30d2e7b8d8066..d25b52424a328 100644 --- a/lib/auth/touchid/api_test.go +++ b/lib/auth/touchid/api_test.go @@ -21,6 +21,7 @@ import ( "crypto/elliptic" "crypto/rand" "encoding/json" + "errors" "testing" "github.com/duo-labs/webauthn/protocol" @@ -141,6 +142,10 @@ func (f *fakeNative) Authenticate(credentialID string, data []byte) ([]byte, err return key.Sign(rand.Reader, data, crypto.SHA256) } +func (f *fakeNative) DeleteCredential(credentialID string) error { + return errors.New("not implemented") +} + func (f *fakeNative) IsAvailable() bool { return true } @@ -161,6 +166,10 @@ func (f *fakeNative) FindCredentials(rpID, user string) ([]touchid.CredentialInf return resp, nil } +func (f *fakeNative) ListCredentials() ([]touchid.CredentialInfo, error) { + return nil, errors.New("not implemented") +} + func (f *fakeNative) Register(rpID, user string, userHandle []byte) (*touchid.CredentialInfo, error) { key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil { diff --git a/lib/auth/touchid/authenticate.m b/lib/auth/touchid/authenticate.m index e05974421c8c1..d1285378b4861 100644 --- a/lib/auth/touchid/authenticate.m +++ b/lib/auth/touchid/authenticate.m @@ -1,5 +1,5 @@ -// go:build touchid -// +build touchid +//go:build touchid +// +build touchid // Copyright 2022 Gravitational, Inc // diff --git a/lib/auth/touchid/common.m b/lib/auth/touchid/common.m index b54cb0ac607b0..1bb60b03dacfe 100644 --- a/lib/auth/touchid/common.m +++ b/lib/auth/touchid/common.m @@ -1,3 +1,6 @@ +//go:build touchid +// +build touchid + // Copyright 2022 Gravitational, Inc // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -12,9 +15,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build touchid -// +build touchid - #include "common.h" #import @@ -22,5 +22,8 @@ #include char *CopyNSString(NSString *val) { - return strdup([val UTF8String]); + if (val) { + return strdup([val UTF8String]); + } + return strdup(""); } diff --git a/lib/auth/touchid/credentials.h b/lib/auth/touchid/credentials.h index 9379b3b578a2e..332e225fe65ca 100644 --- a/lib/auth/touchid/credentials.h +++ b/lib/auth/touchid/credentials.h @@ -33,4 +33,17 @@ typedef struct LabelFilter { // User interaction is not required. int FindCredentials(LabelFilter filter, CredentialInfo **infosOut); +// ListCredentials finds all registered credentials. +// Returns the numbers of credentials assigned to the infos array, or negative +// on failure (typically an OSStatus code). The caller is expected to free infos +// (and their contents!). +// Requires user interaction. +int ListCredentials(const char *reason, CredentialInfo **infosOut, + char **errOut); + +// DeleteCredential deletes a credential by its app_label. +// Requires user interaction. +// Returns zero if successful, non-zero otherwise (typically an OSStatus). +int DeleteCredential(const char *reason, const char *appLabel, char **errOut); + #endif // CREDENTIALS_H_ diff --git a/lib/auth/touchid/credentials.m b/lib/auth/touchid/credentials.m index 6d6c2c38e3fdb..46c718fbbb696 100644 --- a/lib/auth/touchid/credentials.m +++ b/lib/auth/touchid/credentials.m @@ -1,5 +1,5 @@ -// go:build touchid -// +build touchid +//go:build touchid +// +build touchid // Copyright 2022 Gravitational, Inc // @@ -19,11 +19,14 @@ #import #import +#import #import #include #include +#include + #include "common.h" BOOL matchesLabelFilter(LabelFilterKind kind, NSString *filter, @@ -37,7 +40,8 @@ BOOL matchesLabelFilter(LabelFilterKind kind, NSString *filter, return NO; } -int FindCredentials(LabelFilter filter, CredentialInfo **infosOut) { +int findCredentials(BOOL applyFilter, LabelFilter filter, + CredentialInfo **infosOut) { NSDictionary *query = @{ (id)kSecClass : (id)kSecClassKey, (id)kSecAttrKeyType : (id)kSecAttrKeyTypeECSECPrimeRandom, @@ -75,7 +79,7 @@ int FindCredentials(LabelFilter filter, CredentialInfo **infosOut) { CFStringRef label = CFDictionaryGetValue(attrs, kSecAttrLabel); NSString *nsLabel = (__bridge NSString *)label; - if (!matchesLabelFilter(filter.kind, nsFilter, nsLabel)) { + if (applyFilter && !matchesLabelFilter(filter.kind, nsFilter, nsLabel)) { continue; } @@ -113,3 +117,89 @@ int FindCredentials(LabelFilter filter, CredentialInfo **infosOut) { CFRelease(items); return infosLen; } + +int FindCredentials(LabelFilter filter, CredentialInfo **infosOut) { + return findCredentials(YES /* applyFilter */, filter, infosOut); +} + +int ListCredentials(const char *reason, CredentialInfo **infosOut, + char **errOut) { + LAContext *ctx = [[LAContext alloc] init]; + + __block LabelFilter filter; + filter.kind = LABEL_PREFIX; + filter.value = ""; + + __block int res; + __block NSString *nsError = NULL; + + // A semaphore is needed, otherwise we return before the prompt has a chance + // to resolve. + dispatch_semaphore_t sema = dispatch_semaphore_create(0); + [ctx evaluatePolicy:LAPolicyDeviceOwnerAuthenticationWithBiometrics + localizedReason:[NSString stringWithUTF8String:reason] + reply:^void(BOOL success, NSError *_Nullable error) { + if (success) { + res = + findCredentials(NO /* applyFilter */, filter, infosOut); + } else { + res = -1; + nsError = [error localizedDescription]; + } + + dispatch_semaphore_signal(sema); + }]; + dispatch_semaphore_wait(sema, DISPATCH_TIME_FOREVER); + // sema released by ARC. + + if (nsError) { + *errOut = CopyNSString(nsError); + } + + return res; +} + +OSStatus deleteCredential(const char *appLabel) { + NSData *nsAppLabel = [NSData dataWithBytes:appLabel length:strlen(appLabel)]; + NSDictionary *query = @{ + (id)kSecClass : (id)kSecClassKey, + (id)kSecAttrKeyType : (id)kSecAttrKeyTypeECSECPrimeRandom, + (id)kSecMatchLimit : (id)kSecMatchLimitOne, + (id)kSecAttrApplicationLabel : nsAppLabel, + }; + return SecItemDelete((__bridge CFDictionaryRef)query); +} + +int DeleteCredential(const char *reason, const char *appLabel, char **errOut) { + LAContext *ctx = [[LAContext alloc] init]; + + __block int res; + __block NSString *nsError = NULL; + + // A semaphore is needed, otherwise we return before the prompt has a chance + // to resolve. + dispatch_semaphore_t sema = dispatch_semaphore_create(0); + [ctx evaluatePolicy:LAPolicyDeviceOwnerAuthenticationWithBiometrics + localizedReason:[NSString stringWithUTF8String:reason] + reply:^void(BOOL success, NSError *_Nullable error) { + if (success) { + res = deleteCredential(appLabel); + } else { + res = -1; + nsError = [error localizedDescription]; + } + dispatch_semaphore_signal(sema); + }]; + dispatch_semaphore_wait(sema, DISPATCH_TIME_FOREVER); + // sema released by ARC. + + if (nsError) { + *errOut = CopyNSString(nsError); + } else if (res != errSecSuccess) { + CFStringRef err = SecCopyErrorMessageString(res, NULL); + NSString *nsErr = (__bridge_transfer NSString *)err; + *errOut = CopyNSString(nsErr); + } + + return res; +} diff --git a/lib/auth/touchid/register.m b/lib/auth/touchid/register.m index 155f1832d96d8..d2b5e650707fd 100644 --- a/lib/auth/touchid/register.m +++ b/lib/auth/touchid/register.m @@ -1,5 +1,5 @@ -// go:build touchid -// +build touchid +//go:build touchid +// +build touchid // Copyright 2022 Gravitational, Inc // diff --git a/lib/client/mfa.go b/lib/client/mfa.go index e53c7b216d47e..343d2886bad38 100644 --- a/lib/client/mfa.go +++ b/lib/client/mfa.go @@ -176,8 +176,6 @@ func PromptMFAChallenge(ctx context.Context, c *proto.MFAAuthenticateChallenge, }, } }() - } else if !quiet { - fmt.Fprintf(os.Stderr, "Tap any %ssecurity key\n", promptDevicePrefix) } // Fire Webauthn goroutine. @@ -192,8 +190,15 @@ func PromptMFAChallenge(ctx context.Context, c *proto.MFAAuthenticateChallenge, log.Debugf("WebAuthn: prompting devices with origin %q", origin) prompt := wancli.NewDefaultPrompt(ctx, os.Stderr) - prompt.FirstTouchMessage = "" // First prompt printed above. + + // Let OTP take over the prompt if present, but otherwise delegate to + // WebAuthn. + prompt.FirstTouchMessage = "" + if !hasTOTP && !quiet { + prompt.FirstTouchMessage = fmt.Sprintf("Tap any %ssecurity key", promptDevicePrefix) + } prompt.SecondTouchMessage = fmt.Sprintf("Tap your %ssecurity key to complete login", promptDevicePrefix) + mfaPrompt := &mfaPrompt{LoginPrompt: prompt, otpCancelAndWait: func() { otpCancel() otpWait.Wait() diff --git a/tool/tsh/touchid.go b/tool/tsh/touchid.go new file mode 100644 index 0000000000000..bac7e04ea3da0 --- /dev/null +++ b/tool/tsh/touchid.go @@ -0,0 +1,111 @@ +// 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 main + +import ( + "fmt" + "sort" + "strings" + + "github.com/gravitational/teleport/lib/asciitable" + "github.com/gravitational/teleport/lib/auth/touchid" + + "github.com/gravitational/kingpin" + "github.com/gravitational/trace" +) + +type touchIDCommand struct { + ls *touchIDLsCommand + rm *touchIDRmCommand +} + +func newTouchIDCommand(app *kingpin.Application) *touchIDCommand { + tid := app.Command("touchid", "Manage Touch ID credentials").Hidden() + return &touchIDCommand{ + ls: newTouchIDLsCommand(tid), + rm: newTouchIDRmCommand(tid), + } +} + +type touchIDLsCommand struct { + *kingpin.CmdClause +} + +func newTouchIDLsCommand(app *kingpin.CmdClause) *touchIDLsCommand { + return &touchIDLsCommand{ + CmdClause: app.Command("ls", "Get a list of system Touch ID credentials").Hidden(), + } +} + +func (c *touchIDLsCommand) run(cf *CLIConf) error { + infos, err := touchid.ListCredentials() + if err != nil { + return trace.Wrap(err) + } + + sort.Slice(infos, func(i, j int) bool { + i1 := &infos[i] + i2 := &infos[j] + if cmp := strings.Compare(i1.RPID, i2.RPID); cmp != 0 { + return cmp < 0 + } + if cmp := strings.Compare(i1.User, i2.User); cmp != 0 { + return cmp < 0 + } + return i1.CredentialID < i2.CredentialID + }) + + t := asciitable.MakeTable([]string{"RPID", "User", "Key Handle"}) + for _, info := range infos { + t.AddRow([]string{ + info.RPID, + info.User, + info.CredentialID, + }) + } + fmt.Println(t.AsBuffer().String()) + + return nil +} + +type touchIDRmCommand struct { + *kingpin.CmdClause + + credentialID string +} + +func newTouchIDRmCommand(app *kingpin.CmdClause) *touchIDRmCommand { + c := &touchIDRmCommand{ + CmdClause: app.Command("rm", "Remove a Touch ID credential").Hidden(), + } + c.Arg("id", "ID of the Touch ID credential to remove").Required().StringVar(&c.credentialID) + return c +} + +func (c *touchIDRmCommand) FullCommand() string { + if c.CmdClause == nil { + return "touchid rm" + } + return c.CmdClause.FullCommand() +} + +func (c *touchIDRmCommand) run(cf *CLIConf) error { + if err := touchid.DeleteCredential(c.credentialID); err != nil { + return trace.Wrap(err) + } + + fmt.Printf("Touch ID credential %q removed.\n", c.credentialID) + return nil +} diff --git a/tool/tsh/tsh.go b/tool/tsh/tsh.go index cd5e502e97699..4f81cb759001f 100644 --- a/tool/tsh/tsh.go +++ b/tool/tsh/tsh.go @@ -46,6 +46,7 @@ import ( apisshutils "github.com/gravitational/teleport/api/utils/sshutils" "github.com/gravitational/teleport/lib/asciitable" "github.com/gravitational/teleport/lib/auth" + "github.com/gravitational/teleport/lib/auth/touchid" wancli "github.com/gravitational/teleport/lib/auth/webauthncli" "github.com/gravitational/teleport/lib/benchmark" "github.com/gravitational/teleport/lib/client" @@ -696,6 +697,12 @@ func Run(args []string, opts ...cliOption) error { f2 := app.Command("fido2", "FIDO2 commands").Hidden() f2Diag := f2.Command("diag", "Run FIDO2 diagnostics").Hidden() + // touchid subcommands. + var tid *touchIDCommand + if touchid.IsAvailable() { + tid = newTouchIDCommand(app) + } + // On Windows, hide the "ssh", "join", "play", "scp", and "bench" commands // because they all use a terminal. if runtime.GOOS == constants.WindowsOS { @@ -851,8 +858,16 @@ func Run(args []string, opts ...cliOption) error { case f2Diag.FullCommand(): err = onFIDO2Diag(&cf) default: - // This should only happen when there's a missing switch case above. - err = trace.BadParameter("command %q not configured", command) + // Handle commands that might not be available. + switch { + case tid != nil && command == tid.ls.FullCommand(): + err = tid.ls.run(&cf) + case tid != nil && command == tid.rm.FullCommand(): + err = tid.rm.run(&cf) + default: + // This should only happen when there's a missing switch case above. + err = trace.BadParameter("command %q not configured", command) + } } if trace.IsNotImplemented(err) {