Skip to content

Connect: Refresh leaf cluster certs before fetching certs for database#12293

Merged
ravicious merged 6 commits intomasterfrom
ravicious/fix-gateways-on-leaf-clusters
Apr 29, 2022
Merged

Connect: Refresh leaf cluster certs before fetching certs for database#12293
ravicious merged 6 commits intomasterfrom
ravicious/fix-gateways-on-leaf-clusters

Conversation

@ravicious
Copy link
Copy Markdown
Member

Fixes gravitational/webapps.e#215.

tl;dr It was impossible to use Connect with databases on leaf clusters. Any attempt to create a db proxy were failing with "SSH cert not available" when Connect was trying to store reissued db certs.

Roman pointed out that the SSH key is indeed missing when looking at the tsh directory tree created by Connect. Connect was not doing the equivalent of what tsh login <cluster> was doing.

The fix is to reissue certs for the cluster first before attempting to reissue certs for the db.


Another issue I encountered is that when working with a leaf cluster, we used to create a TeleportClient instance aimed at the root cluster and only after it was created we updated TeleportClient.SiteName to point at the leaf cluster. This didn't update the site name in TeleportClient.LocalKeyAgent.

While it wasn't causing any problems yet, I decided to fix it while I was in this part of the codebase. So the first commit in this PR actually makes sure to pass the correct SiteName before we create an instance of TeleportClient.

// fromProfile creates a new cluster from its profile
func (s *Storage) fromProfile(clusterName string) (*Cluster, error) {
if clusterName == "" {
func (s *Storage) fromProfile(profileName, leafClusterName string) (*Cluster, error) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There's an outstanding issue in Teleport Connect where all our URIs use the proxy hostname as the cluster name (which is equivalent to a profile name, I think). I'll create a separate ticket for it soon, but in general the problem is that the proxy hostname can be different than the root cluster name.

So when Storage.GetByURI calls fromProfile, this name is actually the profile name and not the real root cluster name from the key. I changed the argument name here from clusterName to profileName to signal that for the future.

Comment thread lib/teleterm/clusters/storage.go Outdated
Comment on lines +207 to +210
clusterURI := uri.NewClusterURI(profileName)
if leafClusterName != "" {
clusterURI = clusterURI.AppendLeafCluster(leafClusterName)
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Previously this was done in Storage.GetByURI, but I see no reason why we couldn't do it here.

Comment on lines +213 to +215
// TODO(ravicious): This should probably use leafClusterName if available, but at this point I'm
// worried that changing it might break something else in the app.
Name: profileName,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

At the moment it doesn't cause any issues because Teleport Connect uses another code path to get names of leaf clusters.

Methods on Storage are used mostly to get a Cluster instance that lets us perform operations on the root/leaf cluster through TeleportClient.

@ravicious ravicious marked this pull request as ready for review April 28, 2022 10:10
@github-actions github-actions bot requested review from Tener and nklaassen April 28, 2022 10:11
Comment thread lib/teleterm/clusters/storage.go Outdated
@ravicious ravicious enabled auto-merge (squash) April 29, 2022 08:44
@ravicious ravicious merged commit 8c24aff into master Apr 29, 2022
@ravicious ravicious deleted the ravicious/fix-gateways-on-leaf-clusters branch April 29, 2022 09:07
ravicious added a commit that referenced this pull request Apr 29, 2022
#12293)

* Storage.fromProfile: Set correct SiteName for leaf clusters

* Cluster.CreateGateway: Use SiteName for CertPath

* Cluster.ReissueDBCerts: Refresh leaf cluster certs
ravicious added a commit that referenced this pull request Apr 29, 2022
#12293) (#12315)

* Storage.fromProfile: Set correct SiteName for leaf clusters

* Cluster.CreateGateway: Use SiteName for CertPath

* Cluster.ReissueDBCerts: Refresh leaf cluster certs
@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.

3 participants