Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v9] Add tbot proxy and tbot db wrapper commands (#12687) #12990

Merged
merged 1 commit into from
May 27, 2022

Conversation

timothyb89
Copy link
Contributor

Backport of #12687 for branch/v9


  • 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

  • Add tbot proxy and tbot db wrapper commands

This adds new wrapper commands that leverage tsh for proxy and
database access.

It also adds a new tshwrap helper package which contains utilities
for locating the tsh executable, checking its version, and loading
all necessary data (certificates, destinations, etc) that will need
to be passed to tsh for wrapped commands to function.

  • Fix failing unit test due to incorrect default IsVirtual profile flag

  • 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 krzysztof.skrzetnicki@goteleport.com

  • Address review feedback

  • Use tbot proxy in generated ssh_config

  • Add tests for mockable parts of our tsh integration

  • Fix lints

  • Clarify docstrings in CLIConf

  • Tweak comment for clarity; fix typo in onProxyCommand

  • Add missing copyright header

  • Fix failing unit test and pass destination to Describe()

This fixes a failing unit test by making the description for
ssh_config match its behavior in practice. This necessitated
passing the destination to all templates, unfortunately.

  • Add a few extra tests

  • Apply suggestions from code review

Co-authored-by: Alan Parra alan.parra@goteleport.com

  • Address another batch of review comments

  • Comment tweaks

  • Refactor tshwrap to remove the Runner interface.

  • Apply suggestions from code review

Co-authored-by: Alan Parra alan.parra@goteleport.com

  • Address review comments

Co-authored-by: Krzysztof Skrzętnicki krzysztof.skrzetnicki@goteleport.com
Co-authored-by: Alan Parra alan.parra@goteleport.com

* 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

* Add `tbot proxy` and `tbot db` wrapper commands

This adds new wrapper commands that leverage tsh for proxy and
database access.

It also adds a new `tshwrap` helper package which contains utilities
for locating the tsh executable, checking its version, and loading
all necessary data (certificates, destinations, etc) that will need
to be passed to tsh for wrapped commands to function.

* Fix failing unit test due to incorrect default IsVirtual profile flag

* 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 <krzysztof.skrzetnicki@goteleport.com>

* Address review feedback

* Use `tbot proxy` in generated `ssh_config`

* Add tests for mockable parts of our tsh integration

* Fix lints

* Clarify docstrings in CLIConf

* Tweak comment for clarity; fix typo in `onProxyCommand`

* Add missing copyright header

* Fix failing unit test and pass destination to `Describe()`

This fixes a failing unit test by making the description for
`ssh_config` match its behavior in practice. This necessitated
passing the destination to all templates, unfortunately.

* Add a few extra tests

* Apply suggestions from code review

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* Address another batch of review comments

* Comment tweaks

* Refactor tshwrap to remove the Runner interface.

* Apply suggestions from code review

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* Address review comments

Co-authored-by: Krzysztof Skrzętnicki <krzysztof.skrzetnicki@goteleport.com>
Co-authored-by: Alan Parra <alan.parra@goteleport.com>
@timothyb89 timothyb89 enabled auto-merge (squash) May 27, 2022 19:16
@timothyb89 timothyb89 merged commit 67c72c4 into branch/v9 May 27, 2022
@webvictim webvictim mentioned this pull request Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants