Skip to content

use correct certificate extension when getting cluster of agentless node#24909

Merged
capnspacehook merged 1 commit intomasterfrom
capnspacehook/fix-agentless-cert-cluster
Apr 20, 2023
Merged

use correct certificate extension when getting cluster of agentless node#24909
capnspacehook merged 1 commit intomasterfrom
capnspacehook/fix-agentless-cert-cluster

Conversation

@capnspacehook
Copy link
Copy Markdown
Contributor

@capnspacehook capnspacehook commented Apr 20, 2023

lib/utils.CertTeleportClusterName is set by the SSH user key auth handlers, so it should always be set.

Updates #24778.

lib/utils.CertTeleportClusterName is set by the SSH user key auth
handlers, so it should always be set.
Copy link
Copy Markdown
Contributor

@nklaassen nklaassen left a comment

Choose a reason for hiding this comment

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

Tried this out and it works 👍 Do you think there's an appropriate test we could add to somehow check that the certs generated by tctl auth sign --format openssh can be used successfully? I'm alright with merging this fix in without it, so we can use it for the rest of the test plan, and adding a test later

@capnspacehook
Copy link
Copy Markdown
Contributor Author

I'm pretty sure the certificate used here is the user's Teleport issued SSH certificate, not the OpenSSH server's copy of the OpenSHH CA public key. I tried to create a regression test mimicking how tsh proxy ssh would connect to an agentless node, but it didn't fail before the change. So not sure how to test it tbh. I think I'll merge this for now and we can add a test later potentially as you said.

@capnspacehook capnspacehook added this pull request to the merge queue Apr 20, 2023
Merged via the queue into master with commit 4160f8e Apr 20, 2023
@capnspacehook capnspacehook deleted the capnspacehook/fix-agentless-cert-cluster branch April 20, 2023 19:39
@strideynet
Copy link
Copy Markdown
Contributor

Does this need backporting ??

@strideynet
Copy link
Copy Markdown
Contributor

This PR does not need backporting as it is superceded by #24935

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