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

client: set TLS certificate usage for k8s/app/db certs #6824

Merged
merged 2 commits into from
May 13, 2021

Conversation

awly
Copy link
Contributor

@awly awly commented May 11, 2021

TLS usage field

The certificate usage field prevents a certificate from being used for
other purposes. For example, a k8s-specific certificate will not be
accepted by a database service endpoint.

Server-side enforcement logic was already in place for a long time, but
we stopped setting the correct Usage in UserCertRequest during keystore
refactoring in 5.0 (with introduction of k8s certs).

TLS certificate overwrite

As part of this, client.ReissueUserCerts will no longer write
usage-restricted certificates into the top-level TLS certificate used
for Teleport API authentication.

For example, when generating a k8s-specific certificate, we used to
overwrite both:

  • ~/.tsh/keys/$proxy/$user-x509.pem
  • ~/.tsh/keys/$proxy/$user-kube/$cluster/$kubeCluster-x509.pem
    This PR stops overwriting ~/.tsh/keys/$proxy/$user-x509.pem.
    This is not a breaking change.

Selected k8s cluster

Prior to this PR, tsh status printed the selected k8s cluster based on
the top-level TLS certificate. Since we no longer overwrite that
certificate, it will not contain a k8s cluster name.

Instead, we extract it from the kubeconfig, which is actually more
accurate since a user could switch to a different context out-of-band.

Updates #6161
Updates #5815

lib/client/client.go Show resolved Hide resolved
Copy link
Contributor

@russjones russjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bot.

@andrejtokarcik
Copy link
Contributor

andrejtokarcik commented May 12, 2021

FWIW,

This PR stops overwriting ~/.tsh/keys/$proxy/$user-x509.pem.

is not entirely correct. The file still gets overwritten, just with the old already-stored cert.

As part of this, client.ReissueUserCerts will no longer write
usage-restricted certificates into the top-level TLS certificate used

Has this ever been the case?

Andrew Lytvynov added 2 commits May 13, 2021 09:52
--- TLS usage field

The certificate usage field prevents a certificate from being used for
other purposes. For example, a k8s-specific certificate will not be
accepted by a database service endpoint.

Server-side enforcement logic was already in place for a long time, but
we stopped setting the correct Usage in UserCertRequest during keystore
refactoring in 5.0 (with introduction of k8s certs).

--- TLS certificate overwrite

As part of this, client.ReissueUserCerts will no longer write
usage-restricted certificates into the top-level TLS certificate used
for Teleport API authentication.

For example, when generating a k8s-specific certificate, we used to
overwrite both:
- `~/.tsh/keys/$proxy/$user-x509.pem`
- `~/.tsh/keys/$proxy/$user-kube/$cluster/$kubeCluster-x509.pem`
This PR stops overwriting `~/.tsh/keys/$proxy/$user-x509.pem`.
This is not a breaking change.

--- Selected k8s cluster

Prior to this PR, `tsh status` printed the selected k8s cluster based on
the top-level TLS certificate. Since we no longer overwrite that
certificate, it will not contain a k8s cluster name.

Instead, we extract it from the kubeconfig, which is actually more
accurate since a user could switch to a different context out-of-band.
@awly awly force-pushed the andrew/tls-cert-usage branch from 2796fe2 to 4e29eb9 Compare May 13, 2021 16:52
@awly
Copy link
Contributor Author

awly commented May 13, 2021

@andrejtokarcik

The file still gets overwritten, just with the old already-stored cert.

That's correct. It's overwritten but not changed.

As part of this, client.ReissueUserCerts will no longer write
usage-restricted certificates into the top-level TLS certificate used

Has this ever been the case?

I think so. Before 5.0, we only ever had 1 SSH and 1 TLS cert in the profile directory (per cluster).
IIRC, server-side enforcement is very relaced though, so you could use a cert with k8s usage restriction against the proxy/auth APIs. We may need to tighten it in the future.

@awly awly enabled auto-merge (squash) May 13, 2021 16:56
@awly awly merged commit e987caa into master May 13, 2021
@awly awly deleted the andrew/tls-cert-usage branch May 13, 2021 17:26
awly pushed a commit that referenced this pull request May 13, 2021
* client: set TLS certificate usage for k8s/app/db certs

--- TLS usage field

The certificate usage field prevents a certificate from being used for
other purposes. For example, a k8s-specific certificate will not be
accepted by a database service endpoint.

Server-side enforcement logic was already in place for a long time, but
we stopped setting the correct Usage in UserCertRequest during keystore
refactoring in 5.0 (with introduction of k8s certs).

--- TLS certificate overwrite

As part of this, client.ReissueUserCerts will no longer write
usage-restricted certificates into the top-level TLS certificate used
for Teleport API authentication.

For example, when generating a k8s-specific certificate, we used to
overwrite both:
- `~/.tsh/keys/$proxy/$user-x509.pem`
- `~/.tsh/keys/$proxy/$user-kube/$cluster/$kubeCluster-x509.pem`
This PR stops overwriting `~/.tsh/keys/$proxy/$user-x509.pem`.
This is not a breaking change.

--- Selected k8s cluster

Prior to this PR, `tsh status` printed the selected k8s cluster based on
the top-level TLS certificate. Since we no longer overwrite that
certificate, it will not contain a k8s cluster name.

Instead, we extract it from the kubeconfig, which is actually more
accurate since a user could switch to a different context out-of-band.

* Document UserCertRequest CertUsage enum values
awly pushed a commit that referenced this pull request May 13, 2021
* client: set TLS certificate usage for k8s/app/db certs

--- TLS usage field

The certificate usage field prevents a certificate from being used for
other purposes. For example, a k8s-specific certificate will not be
accepted by a database service endpoint.

Server-side enforcement logic was already in place for a long time, but
we stopped setting the correct Usage in UserCertRequest during keystore
refactoring in 5.0 (with introduction of k8s certs).

--- TLS certificate overwrite

As part of this, client.ReissueUserCerts will no longer write
usage-restricted certificates into the top-level TLS certificate used
for Teleport API authentication.

For example, when generating a k8s-specific certificate, we used to
overwrite both:
- `~/.tsh/keys/$proxy/$user-x509.pem`
- `~/.tsh/keys/$proxy/$user-kube/$cluster/$kubeCluster-x509.pem`
This PR stops overwriting `~/.tsh/keys/$proxy/$user-x509.pem`.
This is not a breaking change.

--- Selected k8s cluster

Prior to this PR, `tsh status` printed the selected k8s cluster based on
the top-level TLS certificate. Since we no longer overwrite that
certificate, it will not contain a k8s cluster name.

Instead, we extract it from the kubeconfig, which is actually more
accurate since a user could switch to a different context out-of-band.

* Document UserCertRequest CertUsage enum values
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants