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

Add support for backward compatible API Client behavior #11567

Merged
55 changes: 50 additions & 5 deletions api/profile/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,41 @@ func (p *Profile) TLSConfig() (*tls.Config, error) {
return nil, trace.Wrap(err)
}

pool, err := certPoolFromProfile(p)
if err != nil {
return nil, trace.Wrap(err)
}

return &tls.Config{
Certificates: []tls.Certificate{cert},
RootCAs: pool,
}, nil
}

func certPoolFromProfile(p *Profile) (*x509.CertPool, error) {
// Check if CAS dir exist if not try to load certs from legacy certs.pem file.
if _, err := os.Stat(p.TLSClusterCASDir()); err != nil {
if !os.IsNotExist(err) {
return nil, trace.Wrap(err)
}
pool, err := certPoolFromLegacyCAFile(p)
if err != nil {
return nil, trace.Wrap(err)
}
return pool, nil
}

// Load CertPool from CAS directory.
pool, err := certPoolFromCASDir(p)
if err != nil {
return nil, trace.Wrap(err)
}
return pool, nil
}

func certPoolFromCASDir(p *Profile) (*x509.CertPool, error) {
pool := x509.NewCertPool()
err = filepath.Walk(p.TLSClusterCASDir(), func(path string, info fs.FileInfo, err error) error {
err := filepath.Walk(p.TLSClusterCASDir(), func(path string, info fs.FileInfo, err error) error {
if err != nil {
return trace.Wrap(err)
}
Expand All @@ -129,11 +162,19 @@ func (p *Profile) TLSConfig() (*tls.Config, error) {
if err != nil {
return nil, trace.Wrap(err)
}
return pool, nil
}

return &tls.Config{
Certificates: []tls.Certificate{cert},
RootCAs: pool,
}, nil
func certPoolFromLegacyCAFile(p *Profile) (*x509.CertPool, error) {
caCerts, err := os.ReadFile(p.TLSCAsPath())
if err != nil {
return nil, trace.Wrap(err)
smallinsky marked this conversation as resolved.
Show resolved Hide resolved
}
pool := x509.NewCertPool()
if !pool.AppendCertsFromPEM(caCerts) {
return nil, trace.BadParameter("invalid CA cert PEM")
}
return pool, nil
}

// SSHClientConfig returns the profile's associated SSHClientConfig.
Expand Down Expand Up @@ -336,6 +377,10 @@ func (p *Profile) TLSClusterCASDir() string {
return keypaths.CAsDir(p.Dir, p.Name())
}

func (p *Profile) TLSCAsPath() string {
smallinsky marked this conversation as resolved.
Show resolved Hide resolved
return keypaths.TLSCAsPath(p.Dir, p.Name())
}

// SSHDir returns the path to the profile's ssh directory.
func (p *Profile) SSHDir() string {
return keypaths.SSHDir(p.Dir, p.Name(), p.Username)
Expand Down
10 changes: 10 additions & 0 deletions api/utils/keypaths/keypaths.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ const (
fileNameKnownHosts = "known_hosts"
// fileExtTLSCert is the suffix/extension of a file where a TLS cert is stored.
fileExtTLSCert = "-x509.pem"
// fileNameTLSCerts is a file where TLS Cert Authorities are stored.
fileNameTLSCerts = "certs.pem"
// fileExtCert is the suffix/extension of a file where an SSH Cert is stored.
fileExtSSHCert = "-cert.pub"
// fileExtPub is the extension of a file where a public key is stored.
Expand Down Expand Up @@ -144,6 +146,14 @@ func CAsDir(baseDir, proxy string) string {
return filepath.Join(ProxyKeyDir(baseDir, proxy), casDir)
}

// TLSCAsPath returns the path to the users's TLS CA's certificates
// for the given proxy.
// <baseDir>/keys/<proxy>/certs.pem
// DELETE IN 10.0. Deprecated
Copy link
Contributor

Choose a reason for hiding this comment

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

If this needs to be removed in 10.0 shouldn't we just fix it in 9.0? Technically this code will be deprecated the moment when is merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue is not stricte related to any teleport tsh or tctl flow but occurs in external tools that depends on teleport/api/clientpackage like [terraform](https://github.com/gravitational/teleport-plugins/blob/master/go.mod#L20) teleport-plugin where client still uses oldtshthat generates certs.pem dir but the teleport-plugin uses a news version ofteleport/api/client` that doesn't read certs from certs.pem.

If we will be sure that all clients switched to teleport v9 where trusted certs are stored in CAS directory we will be able to drop support for certs.pem file.

func TLSCAsPath(baseDir, proxy string) string {
return filepath.Join(ProxyKeyDir(baseDir, proxy), fileNameTLSCerts)
}

// TLSCAsPathCluster returns the path to the specified cluster's CA directory.
//
// <baseDir>/keys/<proxy>/cas/<cluster>.pem
Expand Down
15 changes: 15 additions & 0 deletions tool/tsh/tsh_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ limitations under the License.
package main

import (
"context"
"fmt"
"os/user"
"testing"
"time"

"github.com/stretchr/testify/require"

apiclient "github.com/gravitational/teleport/api/client"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/config"
"github.com/gravitational/teleport/lib/service"
Expand Down Expand Up @@ -246,3 +248,16 @@ func waitForEvents(t *testing.T, svc service.Supervisor, events ...string) {
}
}
}

func mustCreateAuthClientFormUserProfile(t *testing.T, tshHomePath, addr string) {
ctx := context.Background()
credentials := apiclient.LoadProfile(tshHomePath, "")
c, err := apiclient.New(context.Background(), apiclient.Config{
Addrs: []string{addr},
Credentials: []apiclient.Credentials{credentials},
InsecureAddressDiscovery: true,
})
require.NoError(t, err)
_, err = c.Ping(ctx)
require.NoError(t, err)
}
43 changes: 43 additions & 0 deletions tool/tsh/tsh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (

"github.com/gravitational/teleport/api/constants"
apidefaults "github.com/gravitational/teleport/api/defaults"
"github.com/gravitational/teleport/api/profile"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib"
"github.com/gravitational/teleport/lib/auth"
Expand Down Expand Up @@ -1233,6 +1234,48 @@ func TestSetX11Config(t *testing.T) {
}
}

// TestAuthClientFromTSHProfile tests if API Client can be successfully created from tsh profile where clusters
// certs are stored separately in CAS directory and in case where legacy certs.pem file was used.
func TestAuthClientFromTSHProfile(t *testing.T) {
tmpHomePath := t.TempDir()

connector := mockConnector(t)
alice, err := types.NewUser("[email protected]")
require.NoError(t, err)
alice.SetRoles([]string{"access"})
authProcess, proxyProcess := makeTestServers(t, withBootstrap(connector, alice))
authServer := authProcess.GetAuthServer()
require.NotNil(t, authServer)
proxyAddr, err := proxyProcess.ProxyWebAddr()
require.NoError(t, err)

err = Run([]string{
"login",
"--insecure",
"--debug",
"--auth", connector.GetName(),
"--proxy", proxyAddr.String(),
}, setHomePath(tmpHomePath), func(cf *CLIConf) error {
cf.mockSSOLogin = mockSSOLogin(t, authServer, alice)
return nil
})
require.NoError(t, err)

cn, err := authServer.GetClusterName()
require.NoError(t, err)
profile, err := profile.FromDir(tmpHomePath, "")
require.NoError(t, err)

mustCreateAuthClientFormUserProfile(t, tmpHomePath, proxyAddr.String())

// Simulate legacy tsh client behavior where all clusters certs were stored in the certs.pem file.
require.NoError(t, os.Rename(profile.TLSCAPathCluster(cn.GetClusterName()), profile.TLSCAsPath()))
require.NoError(t, os.RemoveAll(profile.TLSClusterCASDir()))

// Verify that authClient created from profile will create a valid client in case where cas dir doesn't exit.
mustCreateAuthClientFormUserProfile(t, tmpHomePath, proxyAddr.String())
}

type testServersOpts struct {
bootstrap []types.Resource
authConfigFuncs []func(cfg *service.AuthConfig)
Expand Down