Skip to content

Commit

Permalink
Delete user k8s, etc. certificates on re-issue (#6492) (#6757)
Browse files Browse the repository at this point in the history
Delete user k8s, etc. certificates on re-issue

Prior to this change, user certificates for services like kubernetes were
preserved across a certificate re-issue. This led to issues where elevated
privileges granted by an access request were not applied to the service
certificates as they were not updated during the reissue process.

This patch changes the certificate re-issue process such that:
 * certificates for services (like Kuberenetes) are not preserved
   over a certificate re-issue. It is expected that they will be recreated
   on the first access to the service in question, and
 * the local keystore files for these certificates services are explicitly
   deleted so that the now-invalid cached certificates are not returned
   on keystore queries.

See-Also: #5047
  • Loading branch information
tcsc authored May 7, 2021
1 parent 8e677f2 commit 60dcb42
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 21 deletions.
15 changes: 9 additions & 6 deletions lib/client/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -1070,7 +1070,7 @@ func (tc *TeleportClient) LoadKeyForClusterWithReissue(ctx context.Context, clus
return trace.Wrap(err)
}
// Reissuing also loads the new key.
err = tc.ReissueUserCerts(ctx, ReissueParams{RouteToCluster: clusterName})
err = tc.ReissueUserCerts(ctx, CertCacheKeep, ReissueParams{RouteToCluster: clusterName})
if err != nil {
return trace.Wrap(err)
}
Expand Down Expand Up @@ -1133,14 +1133,14 @@ func (tc *TeleportClient) getTargetNodes(ctx context.Context, proxy *ProxyClient

// ReissueUserCerts issues new user certs based on params and stores them in
// the local key agent (usually on disk in ~/.tsh).
func (tc *TeleportClient) ReissueUserCerts(ctx context.Context, params ReissueParams) error {
func (tc *TeleportClient) ReissueUserCerts(ctx context.Context, cachePolicy CertCachePolicy, params ReissueParams) error {
proxyClient, err := tc.ConnectToProxy(ctx)
if err != nil {
return trace.Wrap(err)
}
defer proxyClient.Close()

return proxyClient.ReissueUserCerts(ctx, params)
return proxyClient.ReissueUserCerts(ctx, cachePolicy, params)
}

// IssueUserCertsWithMFA issues a single-use SSH or TLS certificate for
Expand All @@ -1159,9 +1159,12 @@ func (tc *TeleportClient) IssueUserCertsWithMFA(ctx context.Context, params Reis
}
defer proxyClient.Close()

return proxyClient.IssueUserCertsWithMFA(ctx, params, func(ctx context.Context, proxyAddr string, c *proto.MFAAuthenticateChallenge) (*proto.MFAAuthenticateResponse, error) {
return PromptMFAChallenge(ctx, proxyAddr, c, "")
})
key, err := proxyClient.IssueUserCertsWithMFA(ctx, params,
func(ctx context.Context, proxyAddr string, c *proto.MFAAuthenticateChallenge) (*proto.MFAAuthenticateResponse, error) {
return PromptMFAChallenge(ctx, proxyAddr, c, "")
})

return key, err
}

// CreateAccessRequest registers a new access request with the auth server.
Expand Down
45 changes: 38 additions & 7 deletions lib/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,27 +178,58 @@ func (p ReissueParams) isMFARequiredRequest(sshLogin string) *proto.IsMFARequire
return req
}

// CertCachePolicy describes what should happen to the certificate cache when a
// user certificate is re-issued
type CertCachePolicy int

const (
// CertCacheDrop indicates that all user certificates should be dropped as
// part of the re-issue process. This can be necessary if the roles
// assigned to the user are expected to change as a part of the re-issue.
CertCacheDrop CertCachePolicy = 0

// CertCacheKeep indicates that all user certificates (except those
// explicitly updated by the re-issue) should be preserved across the
// re-issue process.
CertCacheKeep CertCachePolicy = 1
)

// ReissueUserCerts generates certificates for the user
// that have a metadata instructing server to route the requests to the cluster
func (proxy *ProxyClient) ReissueUserCerts(ctx context.Context, params ReissueParams) error {
key, err := proxy.reissueUserCerts(ctx, params)
func (proxy *ProxyClient) ReissueUserCerts(ctx context.Context, cachePolicy CertCachePolicy, params ReissueParams) error {
key, err := proxy.reissueUserCerts(ctx, cachePolicy, params)
if err != nil {
return trace.Wrap(err)
}

if cachePolicy == CertCacheDrop {
proxy.localAgent().DeleteUserCerts("", WithAllCerts...)
}

// save the cert to the local storage (~/.tsh usually):
_, err = proxy.localAgent().AddKey(key)
return trace.Wrap(err)
}

func (proxy *ProxyClient) reissueUserCerts(ctx context.Context, params ReissueParams) (*Key, error) {
func (proxy *ProxyClient) reissueUserCerts(ctx context.Context, cachePolicy CertCachePolicy, params ReissueParams) (*Key, error) {
if params.RouteToCluster == "" {
params.RouteToCluster = proxy.siteName
}
key := params.ExistingCreds
if key == nil {
var err error
key, err = proxy.localAgent().GetKey(params.RouteToCluster, WithAllCerts...)

// Don't load the certs if we're going to drop all of them all as part
// of the re-issue. If we load all of the old certs now we won't be able
// to differentiate between legacy certificates (that need to be
// deleted) and newly re-issued certs (that we definitely do *not* want
// to delete) when it comes time to drop them from the local agent.
certOptions := []CertOption{}
if cachePolicy == CertCacheKeep {
certOptions = WithAllCerts
}

key, err = proxy.localAgent().GetKey(params.RouteToCluster, certOptions...)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down Expand Up @@ -271,7 +302,7 @@ func (proxy *ProxyClient) IssueUserCertsWithMFA(ctx context.Context, params Reis
if params.usage() == proto.UserCertsRequest_SSH && key.Cert != nil {
return key, nil
}
return proxy.reissueUserCerts(ctx, params)
return proxy.reissueUserCerts(ctx, CertCacheKeep, params)
}
return nil, trace.Wrap(err)
}
Expand All @@ -284,7 +315,7 @@ func (proxy *ProxyClient) IssueUserCertsWithMFA(ctx context.Context, params Reis
}
// All other targets need their name embedded in the cert for routing,
// fall back to non-MFA reissue.
return proxy.reissueUserCerts(ctx, params)
return proxy.reissueUserCerts(ctx, CertCacheKeep, params)
}

// Always connect to root for getting new credentials, but attempt to reuse
Expand Down Expand Up @@ -312,7 +343,7 @@ func (proxy *ProxyClient) IssueUserCertsWithMFA(ctx context.Context, params Reis
if params.usage() == proto.UserCertsRequest_SSH && key.Cert != nil {
return key, nil
}
return proxy.reissueUserCerts(ctx, params)
return proxy.reissueUserCerts(ctx, CertCacheKeep, params)
}
return nil, trace.Wrap(err)
}
Expand Down
2 changes: 1 addition & 1 deletion lib/client/keyagent.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ func (a *LocalKeyAgent) GetCoreKey() (*Key, error) {
// AddHostSignersToCache takes a list of CAs whom we trust. This list is added to a database
// of "seen" CAs.
//
// Every time we connect to a new host, we'll request its certificaate to be signed by one
// Every time we connect to a new host, we'll request its certificate to be signed by one
// of these trusted CAs.
//
// Why do we trust these CAs? Because we received them from a trusted Teleport Proxy.
Expand Down
2 changes: 1 addition & 1 deletion tool/tsh/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func onAppLogin(cf *CLIConf) error {
if err != nil {
return trace.Wrap(err)
}
err = tc.ReissueUserCerts(cf.Context, client.ReissueParams{
err = tc.ReissueUserCerts(cf.Context, client.CertCacheKeep, client.ReissueParams{
RouteToCluster: tc.SiteName,
RouteToApp: proto.RouteToApp{
Name: app.Name,
Expand Down
2 changes: 1 addition & 1 deletion tool/tsh/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func databaseLogin(cf *CLIConf, tc *client.TeleportClient, db tlsca.RouteToDatab
if err != nil {
return trace.Wrap(err)
}
err = tc.ReissueUserCerts(cf.Context, client.ReissueParams{
err = tc.ReissueUserCerts(cf.Context, client.CertCacheKeep, client.ReissueParams{
RouteToCluster: tc.SiteName,
RouteToDatabase: proto.RouteToDatabase{
ServiceName: db.ServiceName,
Expand Down
10 changes: 5 additions & 5 deletions tool/tsh/tsh.go
Original file line number Diff line number Diff line change
Expand Up @@ -711,7 +711,7 @@ func onLogin(cf *CLIConf) error {
// for the same proxy
case (cf.Proxy == "" || host(cf.Proxy) == host(profile.ProxyURL.Host)) && cf.SiteName != "":
// trigger reissue, preserving any active requests.
err = tc.ReissueUserCerts(cf.Context, client.ReissueParams{
err = tc.ReissueUserCerts(cf.Context, client.CertCacheKeep, client.ReissueParams{
AccessRequests: profile.ActiveRequests.AccessRequests,
RouteToCluster: cf.SiteName,
})
Expand Down Expand Up @@ -805,7 +805,7 @@ func onLogin(cf *CLIConf) error {
}

if autoRequest && cf.DesiredRoles == "" {
var reason, auto bool
var requireReason, auto bool
var prompt string
roleNames, err := key.CertRoles()
if err != nil {
Expand All @@ -821,7 +821,7 @@ func onLogin(cf *CLIConf) error {
if err != nil {
return trace.Wrap(err)
}
reason = reason || role.GetOptions().RequestAccess.RequireReason()
requireReason = requireReason || role.GetOptions().RequestAccess.RequireReason()
auto = auto || role.GetOptions().RequestAccess.ShouldAutoRequest()
if prompt == "" {
prompt = role.GetOptions().RequestPrompt
Expand All @@ -833,7 +833,7 @@ func onLogin(cf *CLIConf) error {
logoutErr := tc.Logout()
return trace.NewAggregate(err, logoutErr)
}
if reason && cf.RequestReason == "" {
if requireReason && cf.RequestReason == "" {
msg := "--request-reason must be specified"
if prompt != "" {
msg = msg + ", prompt=" + prompt
Expand Down Expand Up @@ -2032,7 +2032,7 @@ func reissueWithRequests(cf *CLIConf, tc *client.TeleportClient, reqIDs ...strin
if params.RouteToCluster == "" {
params.RouteToCluster = profile.Cluster
}
if err := tc.ReissueUserCerts(cf.Context, params); err != nil {
if err := tc.ReissueUserCerts(cf.Context, client.CertCacheDrop, params); err != nil {
return trace.Wrap(err)
}
if err := tc.SaveProfile("", true); err != nil {
Expand Down

0 comments on commit 60dcb42

Please sign in to comment.