Skip to content

Commit

Permalink
Client store generalization (#19420)
Browse files Browse the repository at this point in the history
- Add a generalized client store made up of a key, profile, and trusted certs store. Each sub store can support different backends (~/.tsh, identity_file, in-memory). 
- Replace custom identity file handling with in-memory client store.
- Fix issues with trusted certs handling.
  • Loading branch information
Joerger authored Jan 6, 2023
1 parent fb850a7 commit 488af75
Show file tree
Hide file tree
Showing 51 changed files with 3,372 additions and 2,643 deletions.
6 changes: 3 additions & 3 deletions api/identityfile/identityfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ type Certs struct {

// CACerts contains PEM encoded CA certificates.
type CACerts struct {
// SSH are CA certs used for SSH.
// SSH are CA certs used for SSH in known_hosts format.
SSH [][]byte
// TLS are CA certs used for TLS.
TLS [][]byte
Expand Down Expand Up @@ -262,9 +262,9 @@ func decodeIdentityFile(idFile io.Reader) (*IdentityFile, error) {
for scanln() {
switch {
case isSSHCert(line):
ident.Certs.SSH = cloneln()
ident.Certs.SSH = append(cloneln(), '\n')
case hasPrefix("@cert-authority"):
ident.CACerts.SSH = append(ident.CACerts.SSH, cloneln())
ident.CACerts.SSH = append(ident.CACerts.SSH, append(cloneln(), '\n'))
case hasPrefix("-----BEGIN"):
// Current line marks the beginning of a PEM block. Consume all
// lines until a corresponding END is found.
Expand Down
9 changes: 3 additions & 6 deletions api/identityfile/identityfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ func TestIdentityFileBasics(t *testing.T) {
writeIDFile := &IdentityFile{
PrivateKey: []byte("-----BEGIN RSA PRIVATE KEY-----\nkey\n-----END RSA PRIVATE KEY-----\n"),
Certs: Certs{
SSH: []byte(ssh.CertAlgoRSAv01),
SSH: append([]byte(ssh.CertAlgoRSAv01), '\n'),
TLS: []byte("-----BEGIN CERTIFICATE-----\ntls-cert\n-----END CERTIFICATE-----\n"),
},
CACerts: CACerts{
SSH: [][]byte{[]byte("@cert-authority ssh-cacerts")},
SSH: [][]byte{[]byte("@cert-authority ssh-cacerts\n")},
TLS: [][]byte{[]byte("-----BEGIN CERTIFICATE-----\ntls-cacerts\n-----END CERTIFICATE-----\n")},
},
}
Expand All @@ -50,15 +50,13 @@ func TestIdentityFileBasics(t *testing.T) {
// Read identity file from file
readIDFile, err := ReadFile(path)
require.NoError(t, err)
require.Equal(t, writeIDFile, readIDFile)

// Read identity file from string
s, err := os.ReadFile(path)
require.NoError(t, err)
fromStringIDFile, err := FromString(string(s))
require.NoError(t, err)

// Check that read and write values are equal
require.Equal(t, writeIDFile, readIDFile)
require.Equal(t, writeIDFile, fromStringIDFile)
}

Expand Down Expand Up @@ -87,5 +85,4 @@ func TestIsSSHCert(t *testing.T) {
require.Equal(t, tc.expectBool, isSSHCert)
})
}

}
20 changes: 13 additions & 7 deletions api/profile/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ import (
const (
// profileDir is the default root directory where tsh stores profiles.
profileDir = ".tsh"
// currentProfileFilename is a file which stores the name of the
// currently active profile.
currentProfileFilename = "current-profile"
)

// Profile is a collection of most frequently used CLI flags
Expand Down Expand Up @@ -101,6 +98,15 @@ type Profile struct {
MFAMode string `yaml:"mfa_mode,omitempty"`
}

// Copy returns a shallow copy of p, or nil if p is nil.
func (p *Profile) Copy() *Profile {
if p == nil {
return nil
}
copy := *p
return &copy
}

// Name returns the name of the profile.
func (p *Profile) Name() string {
addr, _, err := net.SplitHostPort(p.WebProxyAddr)
Expand Down Expand Up @@ -221,7 +227,7 @@ func SetCurrentProfileName(dir string, name string) error {
return trace.BadParameter("cannot set current profile: missing dir")
}

path := filepath.Join(dir, currentProfileFilename)
path := keypaths.CurrentProfileFilePath(dir)
if err := os.WriteFile(path, []byte(strings.TrimSpace(name)+"\n"), 0660); err != nil {
return trace.Wrap(err)
}
Expand All @@ -244,7 +250,7 @@ func GetCurrentProfileName(dir string) (name string, err error) {
return "", trace.BadParameter("cannot get current profile: missing dir")
}

data, err := os.ReadFile(filepath.Join(dir, currentProfileFilename))
data, err := os.ReadFile(keypaths.CurrentProfileFilePath(dir))
if err != nil {
if os.IsNotExist(err) {
return "", trace.NotFound("current-profile is not set")
Expand Down Expand Up @@ -316,7 +322,7 @@ func FromDir(dir string, name string) (*Profile, error) {
return nil, trace.Wrap(err)
}
}
p, err := profileFromFile(filepath.Join(dir, name+".yaml"))
p, err := profileFromFile(keypaths.ProfileFilePath(dir, name))
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down Expand Up @@ -350,7 +356,7 @@ func (p *Profile) SaveToDir(dir string, makeCurrent bool) error {
if dir == "" {
return trace.BadParameter("cannot save profile: missing dir")
}
if err := p.saveToFile(filepath.Join(dir, p.Name()+".yaml")); err != nil {
if err := p.saveToFile(keypaths.ProfileFilePath(dir, p.Name())); err != nil {
return trace.Wrap(err)
}
if makeCurrent {
Expand Down
21 changes: 21 additions & 0 deletions api/utils/keypaths/keypaths.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,17 @@ const (
casDir = "cas"
// fileExtPem is the extension of a file where a public certificate is stored.
fileExtPem = ".pem"
// currentProfileFileName is a file containing the name of the current profile
currentProfileFilename = "current-profile"
// profileFileExt is the suffix of a profile file.
profileFileExt = ".yaml"
)

// Here's the file layout of all these keypaths.
// ~/.tsh/ --> default base directory
// ├── current-profile --> file containing the name of the currently active profile
// ├── one.example.com.yaml --> file containing profile details for proxy "one.example.com"
// ├── two.example.com.yaml --> file containing profile details for proxy "two.example.com"
// ├── known_hosts --> trusted certificate authorities (their keys) in a format similar to known_hosts
// └── keys --> session keys directory
// ├── one.example.com --> Proxy hostname
Expand Down Expand Up @@ -107,6 +114,20 @@ func KeyDir(baseDir string) string {
return filepath.Join(baseDir, sessionKeyDir)
}

// CurrentProfile returns the path to the current profile file.
//
// <baseDir>/current-profile
func CurrentProfileFilePath(baseDir string) string {
return filepath.Join(baseDir, currentProfileFilename)
}

// ProfileFilePath returns the path to the profile file for the given profile.
//
// <baseDir>/<profileName>.yaml
func ProfileFilePath(baseDir, profileName string) string {
return filepath.Join(baseDir, profileName+profileFileExt)
}

// KnownHostsPath returns the path to the known hosts file.
//
// <baseDir>/known_hosts
Expand Down
31 changes: 26 additions & 5 deletions api/utils/slices.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,38 @@ func JoinStrings[T ~string](elems []T, sep string) T {
return T(b.String())
}

// Deduplicate deduplicates list of strings
func Deduplicate(in []string) []string {
// Deduplicate deduplicates list of comparable values.
func Deduplicate[T comparable](in []T) []T {
if len(in) == 0 {
return in
}
out := make([]string, 0, len(in))
seen := make(map[string]bool, len(in))
out := make([]T, 0, len(in))
seen := make(map[T]struct{}, len(in))
for _, val := range in {
if _, ok := seen[val]; !ok {
out = append(out, val)
seen[val] = true
seen[val] = struct{}{}
}
}
return out
}

// DeduplicateAny deduplicates list of any values with compare function.
func DeduplicateAny[T any](in []T, compare func(T, T) bool) []T {
if len(in) == 0 {
return in
}
out := make([]T, 0, len(in))
for _, val := range in {
var seen bool
for _, outVal := range out {
if compare(val, outVal) {
seen = true
break
}
}
if !seen {
out = append(out, val)
}
}
return out
Expand Down
17 changes: 17 additions & 0 deletions api/utils/slices_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package utils

import (
"bytes"
"testing"

"github.com/stretchr/testify/require"
Expand All @@ -37,3 +38,19 @@ func TestDeduplicate(t *testing.T) {
})
}
}

func TestDeduplicateAny(t *testing.T) {
tests := []struct {
name string
in, expected [][]byte
}{
{name: "empty slice", in: [][]byte{}, expected: [][]byte{}},
{name: "slice with unique elements", in: [][]byte{{0}, {1}}, expected: [][]byte{{0}, {1}}},
{name: "slice with duplicate elements", in: [][]byte{{0}, {1}, {1}, {0}, {2}}, expected: [][]byte{{0}, {1}, {2}}},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
require.Equal(t, tc.expected, DeduplicateAny(tc.in, bytes.Equal))
})
}
}
51 changes: 48 additions & 3 deletions api/utils/sshutils/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"io"
"net"
"regexp"
"strings"

"github.com/gravitational/trace"
"golang.org/x/crypto/ssh"
Expand Down Expand Up @@ -68,23 +69,67 @@ func ParseCertificate(buf []byte) (*ssh.Certificate, error) {
}

// ParseKnownHosts parses provided known_hosts entries into ssh.PublicKey list.
func ParseKnownHosts(knownHosts [][]byte) ([]ssh.PublicKey, error) {
// If one or more hostnames are provided, only keys that have at least one match
// will be returned.
func ParseKnownHosts(knownHosts [][]byte, matchHostnames ...string) ([]ssh.PublicKey, error) {
var keys []ssh.PublicKey
for _, line := range knownHosts {
for {
_, _, publicKey, _, bytes, err := ssh.ParseKnownHosts(line)
_, hosts, publicKey, _, bytes, err := ssh.ParseKnownHosts(line)
if err == io.EOF {
break
} else if err != nil {
return nil, trace.Wrap(err, "failed parsing known hosts: %v; raw line: %q", err, line)
}
keys = append(keys, publicKey)

if len(matchHostnames) == 0 || HostNameMatch(matchHostnames, hosts) {
keys = append(keys, publicKey)
}

line = bytes
}
}
return keys, nil
}

// HostNameMatch returns whether at least one of the given hosts matches one
// of the given matchHosts. If a host has a wildcard prefix "*.", it will be
// used to match. Ex: "*.example.com" will match "proxy.example.com".
func HostNameMatch(matchHosts []string, hosts []string) bool {
for _, matchHost := range matchHosts {
for _, host := range hosts {
if host == matchHost || matchesWildcard(matchHost, host) {
return true
}
}
}
return false
}

// matchesWildcard ensures the given `hostname` matches the given `pattern`.
// The `pattern` should be prefixed with `*.` which will match exactly one domain
// segment, meaning `*.example.com` will match `foo.example.com` but not
// `foo.bar.example.com`.
func matchesWildcard(hostname, pattern string) bool {
pattern = strings.TrimSpace(pattern)

// Don't allow non-wildcard or empty patterns.
if !strings.HasPrefix(pattern, "*.") || len(pattern) < 3 {
return false
}
matchHost := pattern[2:]

// Trim any trailing "." in case of an absolute domain.
hostname = strings.TrimSuffix(hostname, ".")

_, hostnameRoot, found := strings.Cut(hostname, ".")
if !found {
return false
}

return hostnameRoot == matchHost
}

// ParseAuthorizedKeys parses provided authorized_keys entries into ssh.PublicKey list.
func ParseAuthorizedKeys(authorizedKeys [][]byte) ([]ssh.PublicKey, error) {
var keys []ssh.PublicKey
Expand Down
23 changes: 23 additions & 0 deletions api/utils/sshutils/ssh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,26 @@ func TestSSHMarshalEd25519(t *testing.T) {
result := KeysEqual(ak, bk)
require.True(t, result)
}

func TestMatchesWildcard(t *testing.T) {
t.Parallel()
t.Run("Wildcard match", func(t *testing.T) {
require.True(t, matchesWildcard("foo.example.com", "*.example.com"))
require.True(t, matchesWildcard("bar.example.com", "*.example.com"))
require.True(t, matchesWildcard("bar.example.com.", "*.example.com"))
require.True(t, matchesWildcard("bar.foo", "*.foo"))
})

t.Run("Wildcard mismatch", func(t *testing.T) {
require.False(t, matchesWildcard("foo.example.com", "example.com"), "Not a wildcard pattern")
require.False(t, matchesWildcard("foo.example.org", "*.example.com"), "Wildcard pattern shouldn't match different suffix")
require.False(t, matchesWildcard("a.b.example.com", "*.example.com"), "Wildcard pattern shouldn't match multiple prefixes")

t.Run("Single part hostname", func(t *testing.T) {
require.False(t, matchesWildcard("example", "*.example.com"))
require.False(t, matchesWildcard("example", "*.example"))
require.False(t, matchesWildcard("example", "example"))
require.False(t, matchesWildcard("example", "*."))
})
})
}
2 changes: 1 addition & 1 deletion fixtures/certs/identities/key-cert-ca.pem
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ w+VQYlOF3Nz0IrAwEWxKg4GxAoGBANlBOHShukF/qSMXqRer59ExgBuTG0KZ8QT0
rBjbUpA16Fi8NSro/mXDLCh8mTzu0tPG+e1jqcEVc5JDLYIau12j6jw=
-----END RSA PRIVATE KEY-----
ssh-rsa-cert-v01@openssh.com AAAAHHNzaC1yc2EtY2VydC12MDFAb3BlbnNzaC5jb20AAAAg0NeCnpO5ZAzWmMX6XwjrFyDi+JRrPLNb0vrEYqJp+bEAAAADAQABAAABAQC96eyjDJkj80k2JJ2imXQTXb4VfjEXHxPClX4uw0Th7dJ6NxvKb+AfAbaFdYu3xJjyUhFkHg0hPtMhe/ubq0wrejkTtYwd87iWj8wu+aiSziRexphXClxNt8RWv+1mgAVZuBSPHg4jykrYzpaQOqmIiOcMBpumFpwA2cXNRgLbEdZ4uwpNjBYwxGigh1m50OiFvcXFvrwvGkkqDwExIaCqSoK+E3NmTLt6I5eTVvjdhxSzKqwF65vY9XWqh4w1JP2NHCQSkyh2rlC4WM0mpkyL4ZmJdIsRFj1DxN7Ovma6HS8AKJeiDyShuWounwCsoK33onjr+ib9cYUAvsKTdB9pAAAAAAAAAAAAAAABAAAACmVrb250c2V2b3kAAAAOAAAACmVrb250c2V2b3kAAAAAAAAAAAAAAABZPPDKAAAAAAAAAHYAAAAWcGVybWl0LXBvcnQtZm9yd2FyZGluZwAAAAAAAAAKcGVybWl0LXB0eQAAAAAAAAAOdGVsZXBvcnQtcm9sZXMAAAAwAAAALHsidmVyc2lvbiI6InYxIiwicm9sZXMiOlsidXNlcjpla29udHNldm95Il19AAAAAAAAARcAAAAHc3NoLXJzYQAAAAMBAAEAAAEBAMA/0pVkfFhDPUDosZpM9nP/r/t6tORkmhXCxMkLZiS7+kg0htYSDLmwFGfzgSbYAH6Bryu9BZOxv1W23WW9oW7IdJpwfCpuyzFoRN0/2mHhAAHETtxucksTgYNwN+dDXF/IzG/QGVYswP4ENte4ZuNsd3bquBu+opK7CXU4B+UtsY0JUcV7gU5TzCZBdFpzLgB2VUpiHlFg0PUuV74aZmwzHlwoONBSIn2FZpHmvN2ZUdqTHSjof1vgH20cScMWGk05dFuM5gWjHEYC1gwdPpmTGcgN93SAQwKiAUQ6ZnJ+lVhzSp+/vxVz/aecDnrza+xI26DnB/nEEiCMu92WYxEAAAEPAAAAB3NzaC1yc2EAAAEAQRUml83QKsEeWB0WswfR1rvEzzumYRn/CAMTSGsF99bNzHmZ0lJbwCzNdl0hlJ3tGVPhANL5WwWuiLN1q6O8qrUU4cGJK3L8eNFUXmJIVc1xH2bIaws+nHikqPHnxbtAzJBbHeCngBX7eVT69bW6AdgWSHSzlPRaAaApMoEwVIMKOLiedjy7D9s/Cd+GtOtxTMoG/LmFBnvUiuXWwiQ658MRrg65ATl0x24ErJqz2cnj52Sy5G6SNUrENRqkP8TxRtp6a+FT1oJQ/2LqLwnlPQpb41j6fDqLRa47NU2TRnYRv0rhCMHO5tIhA51qhRlU9R2m5BH5o1JswN/phMgbjg==
@cert-authority *.turing.local ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDEk4cVIiydp9xSPIb8UqXpShY8zPlk/lpR69UL+0+RnNXtQl7GcQUZsrXDB2gOCfj+doKZj8Pt8oQVSDJF/vKhr+KS2Z+LC2Gyt8D5IY/acyyhSN5VoIo0JzIOr5CPGJNpLChREFuveV30hLihSfY52cqSvu7N5u34BlZ29WTLeBD9WssAG5HZUES8Xo3neHBl4SOck+mdiUvOIPhcnPiYRmYltOI3GJRu5y1xGemoPU3MnMziQMqnKCc2+To6IC8CkeQqa8D//BxLjenjSgn1K/SLUHraMb5qCmf77fyshj6A9jamgo0UOaOqem+jyg8idnz6JbVfXwW0nEaSyPzX type=host
@cert-authority proxy.example.com,turing.local,*.turing.local ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDEk4cVIiydp9xSPIb8UqXpShY8zPlk/lpR69UL+0+RnNXtQl7GcQUZsrXDB2gOCfj+doKZj8Pt8oQVSDJF/vKhr+KS2Z+LC2Gyt8D5IY/acyyhSN5VoIo0JzIOr5CPGJNpLChREFuveV30hLihSfY52cqSvu7N5u34BlZ29WTLeBD9WssAG5HZUES8Xo3neHBl4SOck+mdiUvOIPhcnPiYRmYltOI3GJRu5y1xGemoPU3MnMziQMqnKCc2+To6IC8CkeQqa8D//BxLjenjSgn1K/SLUHraMb5qCmf77fyshj6A9jamgo0UOaOqem+jyg8idnz6JbVfXwW0nEaSyPzX type=host
2 changes: 1 addition & 1 deletion integration/helpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func MustCreateUserIdentityFile(t *testing.T, tc *TeleInstance, username string,

hostCAs, err := tc.Process.GetAuthServer().GetCertAuthorities(context.Background(), types.HostCA, false)
require.NoError(t, err)
key.TrustedCA = auth.AuthoritiesToTrustedCerts(hostCAs)
key.TrustedCerts = auth.AuthoritiesToTrustedCerts(hostCAs)

idPath := filepath.Join(t.TempDir(), "user_identity")
_, err = identityfile.Write(identityfile.WriteConfig{
Expand Down
3 changes: 2 additions & 1 deletion integration/helpers/kube.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ func genUserKey() (*client.Key, error) {
return &client.Key{
PrivateKey: priv,
TLSCert: tlsCert,
TrustedCA: []auth.TrustedCerts{{
TrustedCerts: []auth.TrustedCerts{{
ClusterName: "localhost",
TLSCertificates: [][]byte{caCert},
}},
}, nil
Expand Down
2 changes: 1 addition & 1 deletion integration/proxy/teleterm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func testGatewayCertRenewal(t *testing.T, pack *dbhelpers.DatabasePack, creds *h
require.NoError(t, err)
// The profile on disk created by NewClientWithCreds doesn't have WebProxyAddr set.
tc.WebProxyAddr = pack.Root.Cluster.Web
tc.SaveProfile(tc.KeysDir, false /* makeCurrent */)
tc.SaveProfile(false /* makeCurrent */)

fakeClock := clockwork.NewFakeClockAt(time.Now())

Expand Down
Loading

0 comments on commit 488af75

Please sign in to comment.