Skip to content

Commit

Permalink
Extend support for identity files in tsh (#12686)
Browse files Browse the repository at this point in the history 
* Extend support for identity files in tsh

This enhances support for identity files in tsh, which previously only
worked for regular SSH access. The largest blocker for support is that
tsh uses profiles for all non-SSH resource access, and profiles have a
direct mapping to some on-disk resources. This patch works around this
in a few ways:
 * Virtual profiles: When an identity file is specified with `-i`, we
   use it to create an in-memory virtual profile using the cert as the
   root identity _and_ for every `RouteToDatabase` (and in the future,
   app) field contained in the cert.
 * Virtual profile paths: Certain profile operations require paths to
   valid certificates and other resources on disk, which may not exist
   inside the identity file.

   As the driving use-case for this change is integration with Machine
   ID, we can "cheat" and pass the correct paths to tsh via
   environment variables. A cooperating wrapper in `tbot` will execute
   `tsh` with appropriate flags and environment variables, which
   override tsh's usual certifiate paths. This allows commands like
   `tsh db connect ...` to work as expected.
 * Key stores: previously we used a `noLocalKeyStore{}` with which all
   lookups fail. This patch replaces it with an in-memory keystore if
   a client key is available.
 * Profile status: lastly, we add a new `StatusCurrentWithIdentity()`
   function to load virtual profiles where supported. Some commands
   are not supported in this PR (like `tsh app ...`), but others
   don't make sense to support (like cert reissuing).

   We might consider merging everything into the traditional
   `StatusCurrent()` when adding app support.

App access is still broken and will be addressed in a later change.

Partially fixes #11770

* Fix failing lint

* Combine `StatusCurrentWithIdentity()` into `StatusCurrent()`

Additionally, log a warning when environment variable paths aren't
found.

* Fix virtual profile flag always being true

* Update lib/client/api.go

Co-authored-by: Krzysztof Skrzętnicki <[email protected]>

* Address review feedback

Co-authored-by: Krzysztof Skrzętnicki <[email protected]>
  • Loading branch information
@timothyb89 @Tener
timothyb89 and Tener committed May 25, 2022
1 parent 038fddd commit 148afeb
Show file tree
Hide file tree
Showing 9 changed files with 515 additions and 82 deletions.
316 changes: 279 additions & 37 deletions lib/client/api.go
View file

Large diffs are not rendered by default.

73 changes: 73 additions & 0 deletions lib/client/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"

"github.com/gravitational/teleport/api/client/webclient"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/utils"
"github.com/gravitational/trace"
Expand Down Expand Up @@ -623,3 +624,75 @@ func TestParseSearchKeywords_SpaceDelimiter(t *testing.T) {
})
}
}

func TestVirtualPathNames(t *testing.T) {
t.Parallel()

testCases := []struct {
name string
kind VirtualPathKind
params VirtualPathParams
expected []string
}{
{
name: "dummy",
kind: VirtualPathKind("foo"),
params: VirtualPathParams{"a", "b", "c"},
expected: []string{
"TSH_VIRTUAL_PATH_FOO_A_B_C",
"TSH_VIRTUAL_PATH_FOO_A_B",
"TSH_VIRTUAL_PATH_FOO_A",
"TSH_VIRTUAL_PATH_FOO",
},
},
{
name: "key",
kind: VirtualPathKey,
params: nil,
expected: []string{"TSH_VIRTUAL_PATH_KEY"},
},
{
name: "host ca",
kind: VirtualPathCA,
params: VirtualPathCAParams(types.HostCA),
expected: []string{
"TSH_VIRTUAL_PATH_CA_HOST",
"TSH_VIRTUAL_PATH_CA",
},
},
{
name: "database",
kind: VirtualPathDatabase,
params: VirtualPathDatabaseParams("foo"),
expected: []string{
"TSH_VIRTUAL_PATH_DB_FOO",
"TSH_VIRTUAL_PATH_DB",
},
},
{
name: "app",
kind: VirtualPathApp,
params: VirtualPathAppParams("foo"),
expected: []string{
"TSH_VIRTUAL_PATH_APP_FOO",
"TSH_VIRTUAL_PATH_APP",
},
},
{
name: "kube",
kind: VirtualPathKubernetes,
params: VirtualPathKubernetesParams("foo"),
expected: []string{
"TSH_VIRTUAL_PATH_KUBE_FOO",
"TSH_VIRTUAL_PATH_KUBE",
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
names := VirtualPathEnvNames(tc.kind, tc.params)
require.Equal(t, tc.expected, names)
})
}
}
40 changes: 35 additions & 5 deletions lib/client/interfaces.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,21 @@ func NewKey() (key *Key, err error) {
}, nil
}

// extractIdentityFromCert parses a tlsca.Identity from raw PEM cert bytes.
func extractIdentityFromCert(certBytes []byte) (*tlsca.Identity, error) {
cert, err := tlsca.ParseCertificatePEM(certBytes)
if err != nil {
return nil, trace.Wrap(err, "failed to parse TLS certificate")
}

parsed, err := tlsca.FromSubject(cert.Subject, cert.NotAfter)
if err != nil {
return nil, trace.Wrap(err)
}

return parsed, nil
}

// KeyFromIdentityFile loads the private key + certificate
// from an identity file into a Key.
func KeyFromIdentityFile(path string) (*Key, error) {
Expand All @@ -127,11 +142,25 @@ func KeyFromIdentityFile(path string) (*Key, error) {
return nil, trace.Wrap(err)
}

dbTLSCerts := make(map[string][]byte)

// validate TLS Cert (if present):
if len(ident.Certs.TLS) > 0 {
if _, err := tls.X509KeyPair(ident.Certs.TLS, ident.PrivateKey); err != nil {
return nil, trace.Wrap(err)
}

parsedIdent, err := extractIdentityFromCert(ident.Certs.TLS)
if err != nil {
return nil, trace.Wrap(err)
}

// If this identity file has any database certs, copy it into the DBTLSCerts map.
if parsedIdent.RouteToDatabase.ServiceName != "" {
dbTLSCerts[parsedIdent.RouteToDatabase.ServiceName] = ident.Certs.TLS
}

// TODO: add k8s, app, etc certs as well.
}

// Validate TLS CA certs (if present).
Expand All @@ -157,11 +186,12 @@ func KeyFromIdentityFile(path string) (*Key, error) {
}

return &Key{
Priv: ident.PrivateKey,
Pub: signer.PublicKey().Marshal(),
Cert: ident.Certs.SSH,
TLSCert: ident.Certs.TLS,
TrustedCA: trustedCA,
Priv: ident.PrivateKey,
Pub: ssh.MarshalAuthorizedKey(signer.PublicKey()),
Cert: ident.Certs.SSH,
TLSCert: ident.Certs.TLS,
TrustedCA: trustedCA,
DBTLSCerts: dbTLSCerts,
}, nil
}

Expand Down
8 changes: 4 additions & 4 deletions tool/tsh/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func onAppLogin(cf *CLIConf) error {
if err != nil {
return trace.Wrap(err)
}
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy)
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy, cf.IdentityFileIn)
if err != nil {
return trace.Wrap(err)
}
Expand Down Expand Up @@ -152,7 +152,7 @@ func onAppLogout(cf *CLIConf) error {
if err != nil {
return trace.Wrap(err)
}
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy)
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy, cf.IdentityFileIn)
if err != nil {
return trace.Wrap(err)
}
Expand Down Expand Up @@ -195,7 +195,7 @@ func onAppConfig(cf *CLIConf) error {
if err != nil {
return trace.Wrap(err)
}
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy)
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy, cf.IdentityFileIn)
if err != nil {
return trace.Wrap(err)
}
Expand Down Expand Up @@ -284,7 +284,7 @@ func serializeAppConfig(configInfo *appConfigInfo, format string) (string, error
// If logged into multiple apps, returns an error unless one was specified
// explicitly on CLI.
func pickActiveApp(cf *CLIConf) (*tlsca.RouteToApp, error) {
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy)
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy, cf.IdentityFileIn)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down
2 changes: 1 addition & 1 deletion tool/tsh/aws.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ func (t tempSelfSignedLocalCert) Clean() error {
}

func pickActiveAWSApp(cf *CLIConf) (string, error) {
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy)
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy, cf.IdentityFileIn)
if err != nil {
return "", trace.Wrap(err)
}
Expand Down
78 changes: 47 additions & 31 deletions tool/tsh/db.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func onListDatabases(cf *CLIConf) error {
defer cluster.Close()

// Retrieve profile to be able to show which databases user is logged into.
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy)
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy, cf.IdentityFileIn)
if err != nil {
return trace.Wrap(err)
}
Expand Down Expand Up @@ -145,33 +145,40 @@ func databaseLogin(cf *CLIConf, tc *client.TeleportClient, db tlsca.RouteToDatab
// ref: https://redis.io/commands/auth
db.Username = defaults.DefaultRedisUsername
}
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy)

profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy, cf.IdentityFileIn)
if err != nil {
return trace.Wrap(err)
}

var key *client.Key
if err = client.RetryWithRelogin(cf.Context, tc, func() error {
key, err = tc.IssueUserCertsWithMFA(cf.Context, client.ReissueParams{
RouteToCluster: tc.SiteName,
RouteToDatabase: proto.RouteToDatabase{
ServiceName: db.ServiceName,
Protocol: db.Protocol,
Username: db.Username,
Database: db.Database,
},
AccessRequests: profile.ActiveRequests.AccessRequests,
})
return trace.Wrap(err)
}); err != nil {
return trace.Wrap(err)
}
if err = tc.LocalAgent().AddDatabaseKey(key); err != nil {
return trace.Wrap(err)
// Identity files themselves act as the database credentials (if any), so
// don't bother fetching new certs.
if profile.IsVirtual {
log.Info("Note: already logged in due to an identity file (`-i ...`); will only update database config files.")
} else {
var key *client.Key
if err = client.RetryWithRelogin(cf.Context, tc, func() error {
key, err = tc.IssueUserCertsWithMFA(cf.Context, client.ReissueParams{
RouteToCluster: tc.SiteName,
RouteToDatabase: proto.RouteToDatabase{
ServiceName: db.ServiceName,
Protocol: db.Protocol,
Username: db.Username,
Database: db.Database,
},
AccessRequests: profile.ActiveRequests.AccessRequests,
})
return trace.Wrap(err)
}); err != nil {
return trace.Wrap(err)
}
if err = tc.LocalAgent().AddDatabaseKey(key); err != nil {
return trace.Wrap(err)
}
}

// Refresh the profile.
profile, err = client.StatusCurrent(cf.HomePath, cf.Proxy)
profile, err = client.StatusCurrent(cf.HomePath, cf.Proxy, cf.IdentityFileIn)
if err != nil {
return trace.Wrap(err)
}
Expand All @@ -194,7 +201,7 @@ func onDatabaseLogout(cf *CLIConf) error {
if err != nil {
return trace.Wrap(err)
}
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy)
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy, cf.IdentityFileIn)
if err != nil {
return trace.Wrap(err)
}
Expand All @@ -203,6 +210,10 @@ func onDatabaseLogout(cf *CLIConf) error {
return trace.Wrap(err)
}

if profile.IsVirtual {
log.Info("Note: an identity file is in use (`-i ...`); will only update database config files.")
}

var logout []tlsca.RouteToDatabase
// If database name wasn't given on the command line, log out of all.
if cf.DatabaseService == "" {
Expand All @@ -219,7 +230,7 @@ func onDatabaseLogout(cf *CLIConf) error {
}
}
for _, db := range logout {
if err := databaseLogout(tc, db); err != nil {
if err := databaseLogout(tc, db, profile.IsVirtual); err != nil {
return trace.Wrap(err)
}
}
Expand All @@ -231,16 +242,20 @@ func onDatabaseLogout(cf *CLIConf) error {
return nil
}

func databaseLogout(tc *client.TeleportClient, db tlsca.RouteToDatabase) error {
func databaseLogout(tc *client.TeleportClient, db tlsca.RouteToDatabase, virtual bool) error {
// First remove respective connection profile.
err := dbprofile.Delete(tc, db)
if err != nil {
return trace.Wrap(err)
}
// Then remove the certificate from the keystore.
err = tc.LogoutDatabase(db.ServiceName)
if err != nil {
return trace.Wrap(err)

// Then remove the certificate from the keystore, but only for real
// profiles.
if !virtual {
err = tc.LogoutDatabase(db.ServiceName)
if err != nil {
return trace.Wrap(err)
}
}
return nil
}
Expand Down Expand Up @@ -296,7 +311,7 @@ func onDatabaseConfig(cf *CLIConf) error {
if err != nil {
return trace.Wrap(err)
}
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy)
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy, cf.IdentityFileIn)
if err != nil {
return trace.Wrap(err)
}
Expand Down Expand Up @@ -516,7 +531,7 @@ func onDatabaseConnect(cf *CLIConf) error {
if err != nil {
return trace.Wrap(err)
}
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy)
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy, cf.IdentityFileIn)
if err != nil {
return trace.Wrap(err)
}
Expand Down Expand Up @@ -712,10 +727,11 @@ func isMFADatabaseAccessRequired(cf *CLIConf, tc *client.TeleportClient, databas
// If logged into multiple databases, returns an error unless one specified
// explicitly via --db flag.
func pickActiveDatabase(cf *CLIConf) (*tlsca.RouteToDatabase, error) {
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy)
profile, err := client.StatusCurrent(cf.HomePath, cf.Proxy, cf.IdentityFileIn)
if err != nil {
return nil, trace.Wrap(err)
}

activeDatabases, err := profile.DatabasesForCluster(cf.SiteName)
if err != nil {
return nil, trace.Wrap(err)
Expand Down
2 changes: 1 addition & 1 deletion tool/tsh/proxy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func onProxyCommandDB(cf *CLIConf) error {
if err != nil {
return trace.Wrap(err)
}
profile, err := libclient.StatusCurrent(cf.HomePath, cf.Proxy)
profile, err := libclient.StatusCurrent(cf.HomePath, cf.Proxy, cf.IdentityFileIn)
if err != nil {
return trace.Wrap(err)
}
Expand Down
Loading

0 comments on commit 148afeb

Please sign in to comment.