From 090a190c7cb1ea04ed4e8bb6aafcec9bd3eaade8 Mon Sep 17 00:00:00 2001 From: Tim Buckley Date: Wed, 30 Mar 2022 17:48:35 -0600 Subject: [PATCH 01/10] Add new `identityfile` config template to `tbot` This adds a new `identityfile` config template to tbot which generates an identity file from any of the formats supported by `tctl auth sign`. It can be used by specifying one or more formats in the configuration like so: ```yaml destinations: - directory: /foo kinds: [ssh, tls] configs: - identityfile: formats: [file] ``` It requires both SSH and TLS certificates to work properly. App, Kubernetes, and Database certs are unlikely to work as they have additional cert requirements that will be added in separate PRs. Multiple formats can be specified, and each will be written to its own subdirectory within the destination using the name of the format. The particular files written inside this directory depend on the particular format selected, but n the above example, this means a file named `/foo/file/identity` is written. The files all have an `identity` prefix at the moment. This could be made configurable if desired. The `file` format can be used in conjunction with `tsh -i` and `tctl -i` to use those tools with a tbot-generated identity. Fixes #10812 --- api/identityfile/identityfile.go | 10 ++ lib/client/identityfile/identity.go | 80 +++++++-- tool/tbot/config/config_destination.go | 24 +++ tool/tbot/config/configtemplate.go | 26 ++- .../config/configtemplate_identityfile.go | 162 ++++++++++++++++++ tool/tbot/config/destination_directory.go | 29 +++- tool/tbot/config/destination_memory.go | 2 +- tool/tbot/destination/destination.go | 2 +- tool/tbot/init.go | 7 +- tool/tbot/main.go | 10 +- 10 files changed, 322 insertions(+), 30 deletions(-) create mode 100644 tool/tbot/config/configtemplate_identityfile.go diff --git a/api/identityfile/identityfile.go b/api/identityfile/identityfile.go index 9f19ce811a91d..d69d045830684 100644 --- a/api/identityfile/identityfile.go +++ b/api/identityfile/identityfile.go @@ -107,6 +107,16 @@ func Write(idFile *IdentityFile, path string) error { return nil } +// Encode encodes the given identityFile to bytes. +func Encode(idFile *IdentityFile) ([]byte, error) { + buf := new(bytes.Buffer) + if err := encodeIdentityFile(buf, idFile); err != nil { + return nil, trace.Wrap(err) + } + + return buf.Bytes(), nil +} + // Read reads an identity file from generic io.Reader interface. func Read(r io.Reader) (*IdentityFile, error) { ident, err := decodeIdentityFile(r) diff --git a/lib/client/identityfile/identity.go b/lib/client/identityfile/identity.go index 9cdf7d0aa3bb1..83e40ab5a1706 100644 --- a/lib/client/identityfile/identity.go +++ b/lib/client/identityfile/identity.go @@ -20,6 +20,7 @@ package identityfile import ( "context" "fmt" + "io/fs" "os" "path/filepath" "strings" @@ -90,6 +91,40 @@ func (f FormatList) String() string { return strings.Join(elems, ", ") } +// ConfigWriter is a simple filesystem abstraction to allow alternative simple +// read/write for this package. +type ConfigWriter interface { + // WriteFile writes the given data to path `name`, using the specified + // permissions if the file is new. + WriteFile(name string, data []byte, perm os.FileMode) error + + // Remove removes a file + Remove(name string) error + + // Stat fetches information about a file. + Stat(name string) (fs.FileInfo, error) +} + +// StandardConfigWriter is a trivial ConfigWriter that wraps the relevant `os` functions. +type StandardConfigWriter struct{} + +// WriteFile writes data to the named file, creating it if necessary. +func (s *StandardConfigWriter) WriteFile(name string, data []byte, perm os.FileMode) error { + return os.WriteFile(name, data, perm) +} + +// Remove removes the named file or (empty) directory. +// If there is an error, it will be of type *PathError. +func (s *StandardConfigWriter) Remove(name string) error { + return os.Remove(name) +} + +// Stat returns a FileInfo describing the named file. +// If there is an error, it will be of type *PathError. +func (s *StandardConfigWriter) Stat(name string) (fs.FileInfo, error) { + return os.Stat(name) +} + // WriteConfig holds the necessary information to write an identity file. type WriteConfig struct { // OutputPath is the output path for the identity file. Note that some @@ -107,11 +142,19 @@ type WriteConfig struct { // overwritten. When false, user will be prompted for confirmation of // overwrite first. OverwriteDestination bool + // Writer is the filesystem implementation. + Writer ConfigWriter } // Write writes user credentials to disk in a specified format. // It returns the names of the files successfully written. func Write(cfg WriteConfig) (filesWritten []string, err error) { + // If no writer was set, use the standard implementation. + writer := cfg.Writer + if writer == nil { + writer = &StandardConfigWriter{} + } + if cfg.OutputPath == "" { return nil, trace.BadParameter("identity output path is not specified") } @@ -120,7 +163,7 @@ func Write(cfg WriteConfig) (filesWritten []string, err error) { // dump user identity into a single file: case FormatFile: filesWritten = append(filesWritten, cfg.OutputPath) - if err := checkOverwrite(cfg.OverwriteDestination, filesWritten...); err != nil { + if err := checkOverwrite(writer, cfg.OverwriteDestination, filesWritten...); err != nil { return nil, trace.Wrap(err) } @@ -145,7 +188,12 @@ func Write(cfg WriteConfig) (filesWritten []string, err error) { idFile.CACerts.TLS = append(idFile.CACerts.TLS, ca.TLSCertificates...) } - if err := identityfile.Write(idFile, cfg.OutputPath); err != nil { + idBytes, err := identityfile.Encode(idFile) + if err != nil { + return nil, trace.Wrap(err) + } + + if err := writer.WriteFile(cfg.OutputPath, idBytes, identityfile.FilePermissions); err != nil { return nil, trace.Wrap(err) } @@ -154,16 +202,16 @@ func Write(cfg WriteConfig) (filesWritten []string, err error) { keyPath := cfg.OutputPath certPath := keypaths.IdentitySSHCertPath(keyPath) filesWritten = append(filesWritten, keyPath, certPath) - if err := checkOverwrite(cfg.OverwriteDestination, filesWritten...); err != nil { + if err := checkOverwrite(writer, cfg.OverwriteDestination, filesWritten...); err != nil { return nil, trace.Wrap(err) } - err = os.WriteFile(certPath, cfg.Key.Cert, identityfile.FilePermissions) + err = writer.WriteFile(certPath, cfg.Key.Cert, identityfile.FilePermissions) if err != nil { return nil, trace.Wrap(err) } - err = os.WriteFile(keyPath, cfg.Key.Priv, identityfile.FilePermissions) + err = writer.WriteFile(keyPath, cfg.Key.Priv, identityfile.FilePermissions) if err != nil { return nil, trace.Wrap(err) } @@ -181,16 +229,16 @@ func Write(cfg WriteConfig) (filesWritten []string, err error) { } filesWritten = append(filesWritten, keyPath, certPath, casPath) - if err := checkOverwrite(cfg.OverwriteDestination, filesWritten...); err != nil { + if err := checkOverwrite(writer, cfg.OverwriteDestination, filesWritten...); err != nil { return nil, trace.Wrap(err) } - err = os.WriteFile(certPath, cfg.Key.TLSCert, identityfile.FilePermissions) + err = writer.WriteFile(certPath, cfg.Key.TLSCert, identityfile.FilePermissions) if err != nil { return nil, trace.Wrap(err) } - err = os.WriteFile(keyPath, cfg.Key.Priv, identityfile.FilePermissions) + err = writer.WriteFile(keyPath, cfg.Key.Priv, identityfile.FilePermissions) if err != nil { return nil, trace.Wrap(err) } @@ -200,7 +248,7 @@ func Write(cfg WriteConfig) (filesWritten []string, err error) { caCerts = append(caCerts, cert...) } } - err = os.WriteFile(casPath, caCerts, identityfile.FilePermissions) + err = writer.WriteFile(casPath, caCerts, identityfile.FilePermissions) if err != nil { return nil, trace.Wrap(err) } @@ -211,10 +259,10 @@ func Write(cfg WriteConfig) (filesWritten []string, err error) { certPath := cfg.OutputPath + ".crt" casPath := cfg.OutputPath + ".cas" filesWritten = append(filesWritten, certPath, casPath) - if err := checkOverwrite(cfg.OverwriteDestination, filesWritten...); err != nil { + if err := checkOverwrite(writer, cfg.OverwriteDestination, filesWritten...); err != nil { return nil, trace.Wrap(err) } - err = os.WriteFile(certPath, append(cfg.Key.TLSCert, cfg.Key.Priv...), identityfile.FilePermissions) + err = writer.WriteFile(certPath, append(cfg.Key.TLSCert, cfg.Key.Priv...), identityfile.FilePermissions) if err != nil { return nil, trace.Wrap(err) } @@ -224,21 +272,21 @@ func Write(cfg WriteConfig) (filesWritten []string, err error) { caCerts = append(caCerts, cert...) } } - err = os.WriteFile(casPath, caCerts, identityfile.FilePermissions) + err = writer.WriteFile(casPath, caCerts, identityfile.FilePermissions) if err != nil { return nil, trace.Wrap(err) } case FormatKubernetes: filesWritten = append(filesWritten, cfg.OutputPath) - if err := checkOverwrite(cfg.OverwriteDestination, filesWritten...); err != nil { + if err := checkOverwrite(writer, cfg.OverwriteDestination, filesWritten...); err != nil { return nil, trace.Wrap(err) } // Clean up the existing file, if it exists. // // kubeconfig.Update would try to parse it and merge in new // credentials, which is not what we want. - if err := os.Remove(cfg.OutputPath); err != nil && !os.IsNotExist(err) { + if err := writer.Remove(cfg.OutputPath); err != nil && !os.IsNotExist(err) { return nil, trace.Wrap(err) } @@ -256,11 +304,11 @@ func Write(cfg WriteConfig) (filesWritten []string, err error) { return filesWritten, nil } -func checkOverwrite(force bool, paths ...string) error { +func checkOverwrite(writer ConfigWriter, force bool, paths ...string) error { var existingFiles []string // Check if the destination file exists. for _, path := range paths { - _, err := os.Stat(path) + _, err := writer.Stat(path) if os.IsNotExist(err) { // File doesn't exist, proceed. continue diff --git a/tool/tbot/config/config_destination.go b/tool/tbot/config/config_destination.go index 30106522289ef..490f4ea948c28 100644 --- a/tool/tbot/config/config_destination.go +++ b/tool/tbot/config/config_destination.go @@ -103,3 +103,27 @@ func (dc *DestinationConfig) ContainsKind(kind identity.ArtifactKind) bool { return false } + +// ListSubdirectories lists all subdirectories that should be contained within +// this destination. Primarily used for on-the-fly directory creation. +func (dc *DestinationConfig) ListSubdirectories() ([]string, error) { + // Note: currently no standard identity.Artifacts create subdirs. If that + // ever changes, we'll need to adapt this to ensure we initialize them + // properly on the fly. + var subdirs []string + + for _, config := range dc.Configs { + template, err := config.GetConfigTemplate() + if err != nil { + return nil, trace.Wrap(err) + } + + for _, file := range template.Describe() { + if file.IsDir { + subdirs = append(subdirs, file.Name) + } + } + } + + return subdirs, nil +} diff --git a/tool/tbot/config/configtemplate.go b/tool/tbot/config/configtemplate.go index f54e86a6ee165..4c796b99b7b3d 100644 --- a/tool/tbot/config/configtemplate.go +++ b/tool/tbot/config/configtemplate.go @@ -28,9 +28,11 @@ import ( const TemplateSSHClientName = "ssh_client" +const TemplateIdentityFileName = "identityfile" + // AllConfigTemplates lists all valid config templates, intended for help // messages -var AllConfigTemplates = [...]string{TemplateSSHClientName} +var AllConfigTemplates = [...]string{TemplateSSHClientName, TemplateIdentityFileName} // FileDescription is a minimal spec needed to create an empty end-user-owned // file with bot-writable ACLs during `tbot init`. @@ -60,7 +62,8 @@ type Template interface { // TemplateConfig contains all possible config template variants. Exactly one // variant must be set to be considered valid. type TemplateConfig struct { - SSHClient *TemplateSSHClient `yaml:"ssh_client,omitempty"` + SSHClient *TemplateSSHClient `yaml:"ssh_client,omitempty"` + IdentityFile *TemplateIdentityFile `yaml:"identityfile,omitempty"` } func (c *TemplateConfig) UnmarshalYAML(node *yaml.Node) error { @@ -75,6 +78,8 @@ func (c *TemplateConfig) UnmarshalYAML(node *yaml.Node) error { switch simpleTemplate { case TemplateSSHClientName: c.SSHClient = &TemplateSSHClient{} + case TemplateIdentityFileName: + return trace.BadParameter("`identityfile` requires parameters, provide `identityfile: ...` instead") default: return trace.BadParameter( "invalid config template '%s' on line %d, expected one of: %s", @@ -94,7 +99,18 @@ func (c *TemplateConfig) CheckAndSetDefaults() error { notNilCount := 0 if c.SSHClient != nil { - c.SSHClient.CheckAndSetDefaults() + if err := c.SSHClient.CheckAndSetDefaults(); err != nil { + return trace.Wrap(err) + } + + notNilCount++ + } + + if c.IdentityFile != nil { + if err := c.IdentityFile.CheckAndSetDefaults(); err != nil { + return trace.Wrap(err) + } + notNilCount++ } @@ -112,5 +128,9 @@ func (c *TemplateConfig) GetConfigTemplate() (Template, error) { return c.SSHClient, nil } + if c.IdentityFile != nil { + return c.IdentityFile, nil + } + return nil, trace.BadParameter("no valid config template") } diff --git a/tool/tbot/config/configtemplate_identityfile.go b/tool/tbot/config/configtemplate_identityfile.go new file mode 100644 index 0000000000000..9eba219870835 --- /dev/null +++ b/tool/tbot/config/configtemplate_identityfile.go @@ -0,0 +1,162 @@ +/* +Copyright 2022 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config + +import ( + "context" + "io/fs" + "os" + "path" + + "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/auth" + "github.com/gravitational/teleport/lib/client" + "github.com/gravitational/teleport/lib/client/identityfile" + "github.com/gravitational/teleport/tool/tbot/destination" + "github.com/gravitational/teleport/tool/tbot/identity" + "github.com/gravitational/trace" +) + +// BotConfigWriter is a trivial adapter to use the identityfile package with +// bot destinations. +type BotConfigWriter struct { + // dest is the destination that will handle writing of files. + dest destination.Destination + + // subpath is the subdirectory within the destination to which the files + // should be written. + subpath string +} + +// WriteFile writes the file to the destination. Only the basename of the path +// is used. Specified permissions are ignored. +func (b *BotConfigWriter) WriteFile(name string, data []byte, perm os.FileMode) error { + p := path.Join(b.subpath, path.Base(name)) + log.Debugf("WriteFile(%q, ...) to %q", name, p) + return trace.Wrap(b.dest.Write(p, data)) +} + +// Remove removes files. This is a dummy implementation that always returns not found. +func (b *BotConfigWriter) Remove(name string) error { + return &os.PathError{Op: "stat", Path: name, Err: os.ErrNotExist} +} + +// Stat checks file status. This implementation always returns not found. +func (b *BotConfigWriter) Stat(name string) (fs.FileInfo, error) { + return nil, &os.PathError{Op: "stat", Path: name, Err: os.ErrNotExist} +} + +// isFormatValid checks if the given format is a supported file format.s +func isFormatValid(format string) bool { + for _, f := range identityfile.KnownFileFormats { + if string(f) == format { + return true + } + } + + return false +} + +type TemplateIdentityFile struct { + Formats []string `yaml:"formats,omitempty"` +} + +func (t *TemplateIdentityFile) CheckAndSetDefaults() error { + for _, format := range t.Formats { + if !isFormatValid(format) { + return trace.BadParameter("invalid format %q, expected one of: %s", format, identityfile.KnownFileFormats.String()) + } + } + + return nil +} + +func (t *TemplateIdentityFile) Describe() []FileDescription { + var descriptions []FileDescription + + // identityfile may generate multiple files + for _, format := range t.Formats { + descriptions = append(descriptions, FileDescription{ + Name: format, + IsDir: true, + }) + } + + return descriptions +} + +func (t *TemplateIdentityFile) Render(ctx context.Context, authClient auth.ClientI, currentIdentity *identity.Identity, destination *DestinationConfig) error { + if !destination.ContainsKind(identity.KindSSH) { + return trace.BadParameter("%s config template requires kind `ssh` to be enabled", TemplateIdentityFileName) + } + + if !destination.ContainsKind(identity.KindTLS) { + return trace.BadParameter("%s config template requires kind `tls` to be enabled", TemplateIdentityFileName) + } + + dest, err := destination.GetDestination() + if err != nil { + return trace.Wrap(err) + } + + hostCAs, err := authClient.GetCertAuthorities(ctx, types.HostCA, false) + if err != nil { + return trace.Wrap(err) + } + + for _, format := range t.Formats { + cfg := identityfile.WriteConfig{ + // Hard code all written files as "identity" for now. We could make + // this user configurable if desired. + OutputPath: "identity", + Writer: &BotConfigWriter{ + dest: dest, + subpath: format, + }, + Key: &client.Key{ + KeyIndex: client.KeyIndex{ + ClusterName: currentIdentity.ClusterName, + }, + Priv: currentIdentity.PrivateKeyBytes, + Pub: currentIdentity.PublicKeyBytes, + Cert: currentIdentity.CertBytes, + TLSCert: currentIdentity.TLSCertBytes, + TrustedCA: auth.AuthoritiesToTrustedCerts(hostCAs), + + // TODO: configure these? we have a 1:1 mapping of destination + // -> app/db/kube cert so this should be knowable. + KubeTLSCerts: make(map[string][]byte), + DBTLSCerts: make(map[string][]byte), + }, + Format: identityfile.Format(format), + + // Always overwrite to avoid hitting our no-op Stat() and Remove() functions. + OverwriteDestination: true, + + // TODO: KubeProxyAddr: ?, + } + + files, err := identityfile.Write(cfg) + if err != nil { + return trace.Wrap(err) + } + + log.Debugf("Wrote identityfile entries: %+v", files) + } + + return nil +} diff --git a/tool/tbot/config/destination_directory.go b/tool/tbot/config/destination_directory.go index b3b39cd7da38c..21d1cccd322bc 100644 --- a/tool/tbot/config/destination_directory.go +++ b/tool/tbot/config/destination_directory.go @@ -20,6 +20,7 @@ import ( "fmt" "os" "os/user" + "path" "path/filepath" "github.com/gravitational/teleport/tool/tbot/botfs" @@ -115,18 +116,34 @@ func (dd *DestinationDirectory) CheckAndSetDefaults() error { return nil } -func (dd *DestinationDirectory) Init() error { - // Create the directory if needed. - stat, err := os.Stat(dd.Path) +// mkdir attempts to make the given directory with extra logging. +func mkdir(p string) error { + stat, err := os.Stat(p) if trace.IsNotFound(err) { - if err := os.MkdirAll(dd.Path, botfs.DefaultDirMode); err != nil { + if err := os.MkdirAll(p, botfs.DefaultDirMode); err != nil { return trace.Wrap(err) } - log.Infof("Created directory %q", dd.Path) + + log.Infof("Created directory %q", p) } else if err != nil { return trace.Wrap(err) } else if !stat.IsDir() { - return trace.BadParameter("Path %q already exists and is not a directory", dd.Path) + return trace.BadParameter("Path %q already exists and is not a directory", p) + } + + return nil +} + +func (dd *DestinationDirectory) Init(subdirs []string) error { + // Create the directory if needed. + if err := mkdir(dd.Path); err != nil { + return trace.Wrap(err) + } + + for _, dir := range subdirs { + if err := mkdir(path.Join(dd.Path, dir)); err != nil { + return trace.Wrap(err) + } } return nil diff --git a/tool/tbot/config/destination_memory.go b/tool/tbot/config/destination_memory.go index 2778bc838a52e..63b20e7146130 100644 --- a/tool/tbot/config/destination_memory.go +++ b/tool/tbot/config/destination_memory.go @@ -50,7 +50,7 @@ func (dm *DestinationMemory) CheckAndSetDefaults() error { return nil } -func (dm *DestinationMemory) Init() error { +func (dm *DestinationMemory) Init(subdirs []string) error { // Nothing to do. return nil } diff --git a/tool/tbot/destination/destination.go b/tool/tbot/destination/destination.go index 3ad0ca5358b3d..9c7addcfe1b11 100644 --- a/tool/tbot/destination/destination.go +++ b/tool/tbot/destination/destination.go @@ -21,7 +21,7 @@ type Destination interface { // Init attempts to initialize this destination for writing. Init should be // idempotent and may write informational log messages if resources are // created. - Init() error + Init(subdirs []string) error // Verify is run before renewals to check for any potential problems with // the destination. These errors may be informational (logged warnings) or diff --git a/tool/tbot/init.go b/tool/tbot/init.go index 8a78090190ef8..88b9d852ff6d4 100644 --- a/tool/tbot/init.go +++ b/tool/tbot/init.go @@ -424,9 +424,14 @@ func onInit(botConfig *config.BotConfig, cf *config.CLIConf) error { log.Infof("Initializing destination: %s", destImpl) + subdirs, err := destination.ListSubdirectories() + if err != nil { + return trace.Wrap(err) + } + // Create the directory if needed. We haven't checked directory ownership, // but it will fail when the ACLs are created if anything is misconfigured. - if err := destDir.Init(); err != nil { + if err := destDir.Init(subdirs); err != nil { return trace.Wrap(err) } diff --git a/tool/tbot/main.go b/tool/tbot/main.go index 8928eba7c33b6..9538002c0144a 100644 --- a/tool/tbot/main.go +++ b/tool/tbot/main.go @@ -278,7 +278,8 @@ func checkDestinations(cfg *config.BotConfig) error { // TODO: consider warning if ownership of all destintions is not expected. - if err := storage.Init(); err != nil { + // Note: no subdirs to init for bot's internal storage. + if err := storage.Init([]string{}); err != nil { return trace.Wrap(err) } @@ -288,7 +289,12 @@ func checkDestinations(cfg *config.BotConfig) error { return trace.Wrap(err) } - if err := destImpl.Init(); err != nil { + subdirs, err := dest.ListSubdirectories() + if err != nil { + return trace.Wrap(err) + } + + if err := destImpl.Init(subdirs); err != nil { return trace.Wrap(err) } } From 24d84a79520d136a1b91a938b4c02735ebcfcfbf Mon Sep 17 00:00:00 2001 From: Tim Buckley Date: Fri, 22 Apr 2022 19:17:54 -0600 Subject: [PATCH 02/10] Make identityfile formats first-class config templates This promotes most of the important identityfile formats to proper config templates. User-facing `kinds` are removed to reduce confusion and several config templates are now required. * The `ssh_client` template is now required and will be added automatically in all cases if not specified. * A new required `tls_cas` template is added to always export the current Teleport server CAs in a usable format. * A new required `identity` template is added to always export an identity file usable with tsh/tctl. * New optional `cockroach`, `mongo`, and `tls` templates can export specifically-formatted TLS certs for various databases and apps. Additionally some other changes were caught during testing: * `botfs` now allows users to specify if files should be opened for reading or for writing; previously, written files were never truncated when opened for writing leading to garbage at the end of files if the length changed. Truncation isn't sane for reading so the two use-cases are now split. --- tool/tbot/botfs/botfs.go | 19 +- tool/tbot/botfs/fs_linux.go | 27 +-- tool/tbot/config/config_destination.go | 71 +++++--- tool/tbot/config/config_test.go | 8 +- tool/tbot/config/configtemplate.go | 171 ++++++++++++++++-- tool/tbot/config/configtemplate_cockroach.go | 91 ++++++++++ tool/tbot/config/configtemplate_identity.go | 88 +++++++++ .../config/configtemplate_identityfile.go | 162 ----------------- tool/tbot/config/configtemplate_mongo.go | 92 ++++++++++ ...te_ssh.go => configtemplate_ssh_client.go} | 32 ++-- tool/tbot/config/configtemplate_tls.go | 96 ++++++++++ tool/tbot/config/configtemplate_tls_cas.go | 119 ++++++++++++ tool/tbot/configtemplate_test.go | 116 ++++++++++++ tool/tbot/identity/artifact.go | 24 ++- tool/tbot/identity/identity.go | 41 +++-- tool/tbot/identity/kinds.go | 46 +---- tool/tbot/init.go | 2 +- tool/tbot/init_test.go | 4 +- tool/tbot/renew.go | 68 +------ tool/tbot/renew_test.go | 2 - tool/tbot/testhelpers/srv.go | 5 +- 21 files changed, 925 insertions(+), 359 deletions(-) create mode 100644 tool/tbot/config/configtemplate_cockroach.go create mode 100644 tool/tbot/config/configtemplate_identity.go delete mode 100644 tool/tbot/config/configtemplate_identityfile.go create mode 100644 tool/tbot/config/configtemplate_mongo.go rename tool/tbot/config/{configtemplate_ssh.go => configtemplate_ssh_client.go} (89%) create mode 100644 tool/tbot/config/configtemplate_tls.go create mode 100644 tool/tbot/config/configtemplate_tls_cas.go create mode 100644 tool/tbot/configtemplate_test.go diff --git a/tool/tbot/botfs/botfs.go b/tool/tbot/botfs/botfs.go index 34e3f4b279c39..ccd00ac34f3e2 100644 --- a/tool/tbot/botfs/botfs.go +++ b/tool/tbot/botfs/botfs.go @@ -77,9 +77,13 @@ const ( // contents to succeed. DefaultDirMode fs.FileMode = 0700 - // OpenMode is the mode with which files should be opened for reading and + // ReadMode is the mode with which files should be opened for reading and // writing. - OpenMode int = os.O_CREATE | os.O_RDWR + ReadMode int = os.O_CREATE | os.O_RDONLY + + // WriteMode is the mode with which files should be opened specifically + // for writing. + WriteMode int = os.O_CREATE | os.O_WRONLY | os.O_TRUNC ) // ACLOptions contains parameters needed to configure ACLs @@ -94,8 +98,13 @@ type ACLOptions struct { // openStandard attempts to open the given path for reading and writing with // O_CREATE set. -func openStandard(path string) (*os.File, error) { - file, err := os.OpenFile(path, OpenMode, DefaultMode) +func openStandard(path string, write bool) (*os.File, error) { + mode := ReadMode + if write { + mode = WriteMode + } + + file, err := os.OpenFile(path, mode, DefaultMode) if err != nil { return nil, trace.ConvertSystemError(err) } @@ -114,7 +123,7 @@ func createStandard(path string, isDir bool) error { return nil } - f, err := openStandard(path) + f, err := openStandard(path, true) if err != nil { return trace.Wrap(err) } diff --git a/tool/tbot/botfs/fs_linux.go b/tool/tbot/botfs/fs_linux.go index c0e010569c40a..61e41d52a3891 100644 --- a/tool/tbot/botfs/fs_linux.go +++ b/tool/tbot/botfs/fs_linux.go @@ -58,12 +58,17 @@ var missingSyscallWarning sync.Once // openSecure opens the given path for writing (with O_CREAT, mode 0600) // with the RESOLVE_NO_SYMLINKS flag set. -func openSecure(path string) (*os.File, error) { +func openSecure(path string, write bool) (*os.File, error) { + mode := ReadMode + if write { + mode = WriteMode + } + how := unix.OpenHow{ // Equivalent to 0600. Unfortunately it's not worth reusing our // default file mode constant here. Mode: unix.O_RDONLY | unix.S_IRUSR | unix.S_IWUSR, - Flags: uint64(OpenMode), + Flags: uint64(mode), Resolve: unix.RESOLVE_NO_SYMLINKS, } @@ -78,16 +83,16 @@ func openSecure(path string) (*os.File, error) { return os.NewFile(uintptr(fd), filepath.Base(path)), nil } -// openSymlinks mode opens the file for read/write using the given symlink +// openSymlinks mode opens the file for read or write using the given symlink // mode, potentially failing or logging a warning if symlinks can't be // secured. -func openSymlinksMode(path string, symlinksMode SymlinksMode) (*os.File, error) { +func openSymlinksMode(path string, write bool, symlinksMode SymlinksMode) (*os.File, error) { var file *os.File var err error switch symlinksMode { case SymlinksSecure: - file, err = openSecure(path) + file, err = openSecure(path, write) if err == unix.ENOSYS { return nil, trace.Errorf("openSecure(%q) failed due to missing "+ "syscall; `symlinks: insecure` may be required for this "+ @@ -96,13 +101,13 @@ func openSymlinksMode(path string, symlinksMode SymlinksMode) (*os.File, error) return nil, trace.Wrap(err) } case SymlinksTrySecure: - file, err = openSecure(path) + file, err = openSecure(path, write) if err == unix.ENOSYS { log.Warnf("Failed to write to %q securely due to missing "+ "syscall; falling back to regular file write. Set "+ "`symlinks: insecure` on this destination to disable this "+ "warning.", path) - file, err = openStandard(path) + file, err = openStandard(path, write) if err != nil { return nil, trace.Wrap(err) } @@ -110,7 +115,7 @@ func openSymlinksMode(path string, symlinksMode SymlinksMode) (*os.File, error) return nil, trace.Wrap(err) } case SymlinksInsecure: - file, err = openStandard(path) + file, err = openStandard(path, write) if err != nil { return nil, trace.Wrap(err) } @@ -136,7 +141,7 @@ func createSecure(path string, isDir bool) error { return nil } - f, err := openSecure(path) + f, err := openSecure(path, true) if err == unix.ENOSYS { // bubble up the original error for comparison return err @@ -204,7 +209,7 @@ func Create(path string, isDir bool, symlinksMode SymlinksMode) error { // Read reads the contents of the given file into memory. func Read(path string, symlinksMode SymlinksMode) ([]byte, error) { - file, err := openSymlinksMode(path, symlinksMode) + file, err := openSymlinksMode(path, false, symlinksMode) if err != nil { return nil, trace.Wrap(err) } @@ -221,7 +226,7 @@ func Read(path string, symlinksMode SymlinksMode) ([]byte, error) { // Write stores the given data to the file at the given path. func Write(path string, data []byte, symlinksMode SymlinksMode) error { - file, err := openSymlinksMode(path, symlinksMode) + file, err := openSymlinksMode(path, true, symlinksMode) if err != nil { return trace.Wrap(err) } diff --git a/tool/tbot/config/config_destination.go b/tool/tbot/config/config_destination.go index 490f4ea948c28..515d28dd95c7e 100644 --- a/tool/tbot/config/config_destination.go +++ b/tool/tbot/config/config_destination.go @@ -17,7 +17,6 @@ limitations under the License. package config import ( - "github.com/gravitational/teleport/tool/tbot/identity" "github.com/gravitational/trace" ) @@ -49,9 +48,8 @@ func (dc *DatabaseConfig) CheckAndSetDefaults() error { type DestinationConfig struct { DestinationMixin `yaml:",inline"` - Roles []string `yaml:"roles,omitempty"` - Kinds []identity.ArtifactKind `yaml:"kinds,omitempty"` - Configs []TemplateConfig `yaml:"configs,omitempty"` + Roles []string `yaml:"roles,omitempty"` + Configs []TemplateConfig `yaml:"configs,omitempty"` Database *DatabaseConfig `yaml:"database,omitempty"` } @@ -63,6 +61,30 @@ func destinationDefaults(dm *DestinationMixin) error { return trace.BadParameter("destinations require some valid output sink") } +// addRequiredConfigs adds all configs with default parameters that were not +// explicitly requested by users. Several configs, including `identity`, `tls`, +// and `ssh_client`, are always generated (with defaults set, if any) but will +// not be overridden if already included by the user. +func (dc *DestinationConfig) addRequiredConfigs() { + if dc.GetConfigByName(TemplateSSHClientName) == nil { + dc.Configs = append(dc.Configs, TemplateConfig{ + SSHClient: &TemplateSSHClient{}, + }) + } + + if dc.GetConfigByName(TemplateIdentityName) == nil { + dc.Configs = append(dc.Configs, TemplateConfig{ + Identity: &TemplateIdentity{}, + }) + } + + if dc.GetConfigByName(TemplateTLSCAsName) == nil { + dc.Configs = append(dc.Configs, TemplateConfig{ + TLSCAs: &TemplateTLSCAs{}, + }) + } +} + func (dc *DestinationConfig) CheckAndSetDefaults() error { if err := dc.DestinationMixin.CheckAndSetDefaults(destinationDefaults); err != nil { return trace.Wrap(err) @@ -77,12 +99,7 @@ func (dc *DestinationConfig) CheckAndSetDefaults() error { // Note: empty roles is allowed; interpreted to mean "all" at generation // time - if len(dc.Kinds) == 0 && len(dc.Configs) == 0 { - dc.Kinds = []identity.ArtifactKind{identity.KindSSH} - dc.Configs = []TemplateConfig{{ - SSHClient: &TemplateSSHClient{}, - }} - } + dc.addRequiredConfigs() for _, cfg := range dc.Configs { if err := cfg.CheckAndSetDefaults(); err != nil { @@ -93,17 +110,6 @@ func (dc *DestinationConfig) CheckAndSetDefaults() error { return nil } -// ContainsKind determines if this destination contains the given ConfigKind. -func (dc *DestinationConfig) ContainsKind(kind identity.ArtifactKind) bool { - for _, k := range dc.Kinds { - if k == kind { - return true - } - } - - return false -} - // ListSubdirectories lists all subdirectories that should be contained within // this destination. Primarily used for on-the-fly directory creation. func (dc *DestinationConfig) ListSubdirectories() ([]string, error) { @@ -127,3 +133,26 @@ func (dc *DestinationConfig) ListSubdirectories() ([]string, error) { return subdirs, nil } + +// GetConfigByName returns the first valid template with the given name +// contained within this destination. +func (dc *DestinationConfig) GetConfigByName(name string) Template { + for _, cfg := range dc.Configs { + tpl, err := cfg.GetConfigTemplate() + if err != nil { + continue + } + + if tpl.Name() == name { + return tpl + } + } + + return nil +} + +// GetRequiredConfig returns the static list of all default / required config +// templates. +func GetRequiredConfigs() []string { + return []string{TemplateTLSCAsName, TemplateSSHClientName, TemplateIdentityName} +} diff --git a/tool/tbot/config/config_test.go b/tool/tbot/config/config_test.go index 9d583a480ed1f..ef3fb92c5ac37 100644 --- a/tool/tbot/config/config_test.go +++ b/tool/tbot/config/config_test.go @@ -22,7 +22,6 @@ import ( "time" "github.com/coreos/go-semver/semver" - "github.com/gravitational/teleport/tool/tbot/identity" "github.com/stretchr/testify/require" ) @@ -75,9 +74,9 @@ func TestConfigCLIOnlySample(t *testing.T) { // A single default destination should exist require.Len(t, cfg.Destinations, 1) dest := cfg.Destinations[0] - require.ElementsMatch(t, []identity.ArtifactKind{identity.KindSSH}, dest.Kinds) - require.Len(t, dest.Configs, 1) + // We have 3 required/default templates. + require.Len(t, dest.Configs, 3) template := dest.Configs[0] require.NotNil(t, template.SSHClient) @@ -109,8 +108,6 @@ func TestConfigFile(t *testing.T) { require.Len(t, cfg.Destinations, 1) destination := cfg.Destinations[0] - require.ElementsMatch(t, []identity.ArtifactKind{identity.KindSSH, identity.KindTLS}, destination.Kinds) - require.Len(t, destination.Configs, 1) template := destination.Configs[0] templateImpl, err := template.GetConfigTemplate() @@ -182,7 +179,6 @@ storage: destinations: - directory: path: /tmp/foo - kinds: [ssh, tls] configs: - ssh_client: proxy_port: 1234 diff --git a/tool/tbot/config/configtemplate.go b/tool/tbot/config/configtemplate.go index 4c796b99b7b3d..f5b69582c49db 100644 --- a/tool/tbot/config/configtemplate.go +++ b/tool/tbot/config/configtemplate.go @@ -18,21 +18,52 @@ package config import ( "context" + "io/fs" + "os" + "path" "strings" + "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/auth" + "github.com/gravitational/teleport/lib/client" + "github.com/gravitational/teleport/tool/tbot/destination" "github.com/gravitational/teleport/tool/tbot/identity" "github.com/gravitational/trace" "gopkg.in/yaml.v3" ) -const TemplateSSHClientName = "ssh_client" +const ( + // TemplateSSHClientName is the config name for generating ssh client + // config files. + TemplateSSHClientName = "ssh_client" -const TemplateIdentityFileName = "identityfile" + // TemplateIdentityName is the config name for Teleport identity files. + TemplateIdentityName = "identity" + + // TemplateTLSName is the config name for TLS client certificates. + TemplateTLSName = "tls" + + // TemplateTLSCAsName is the config name for TLS CA certificates. + TemplateTLSCAsName = "tls_cas" + + // TemplateMongoName is the config name for MongoDB-formatted certificates. + TemplateMongoName = "mongo" + + // TemplateCockroachName is the config name for CockroachDB-formatted + // certificates. + TemplateCockroachName = "cockroach" +) // AllConfigTemplates lists all valid config templates, intended for help // messages -var AllConfigTemplates = [...]string{TemplateSSHClientName, TemplateIdentityFileName} +var AllConfigTemplates = [...]string{ + TemplateSSHClientName, + TemplateIdentityName, + TemplateTLSName, + TemplateTLSCAsName, + TemplateMongoName, + TemplateCockroachName, +} // FileDescription is a minimal spec needed to create an empty end-user-owned // file with bot-writable ACLs during `tbot init`. @@ -48,6 +79,9 @@ type FileDescription struct { // Template defines functions for dynamically writing additional files to // a Destination. type Template interface { + // Name returns the name of this config template. + Name() string + // Describe generates a list of all files this ConfigTemplate will generate // at runtime. Currently ConfigTemplates are required to know this // statically as this must be callable without any auth clients (or any @@ -62,8 +96,12 @@ type Template interface { // TemplateConfig contains all possible config template variants. Exactly one // variant must be set to be considered valid. type TemplateConfig struct { - SSHClient *TemplateSSHClient `yaml:"ssh_client,omitempty"` - IdentityFile *TemplateIdentityFile `yaml:"identityfile,omitempty"` + SSHClient *TemplateSSHClient `yaml:"ssh_client,omitempty"` + Identity *TemplateIdentity `yaml:"identity,omitempty"` + TLS *TemplateTLS `yaml:"tls,omitempty"` + TLSCAs *TemplateTLSCAs `yaml:"tls_cas,omitempty"` + Mongo *TemplateMongo `yaml:"mongo,omitempty"` + Cockroach *TemplateCockroach `yaml:"cockroach,omitempty"` } func (c *TemplateConfig) UnmarshalYAML(node *yaml.Node) error { @@ -78,8 +116,16 @@ func (c *TemplateConfig) UnmarshalYAML(node *yaml.Node) error { switch simpleTemplate { case TemplateSSHClientName: c.SSHClient = &TemplateSSHClient{} - case TemplateIdentityFileName: - return trace.BadParameter("`identityfile` requires parameters, provide `identityfile: ...` instead") + case TemplateIdentityName: + c.Identity = &TemplateIdentity{} + case TemplateTLSName: + c.TLS = &TemplateTLS{} + case TemplateTLSCAsName: + c.TLSCAs = &TemplateTLSCAs{} + case TemplateMongoName: + c.Mongo = &TemplateMongo{} + case TemplateCockroachName: + c.Cockroach = &TemplateCockroach{} default: return trace.BadParameter( "invalid config template '%s' on line %d, expected one of: %s", @@ -106,8 +152,40 @@ func (c *TemplateConfig) CheckAndSetDefaults() error { notNilCount++ } - if c.IdentityFile != nil { - if err := c.IdentityFile.CheckAndSetDefaults(); err != nil { + if c.Identity != nil { + if err := c.Identity.CheckAndSetDefaults(); err != nil { + return trace.Wrap(err) + } + + notNilCount++ + } + + if c.TLS != nil { + if err := c.TLS.CheckAndSetDefaults(); err != nil { + return trace.Wrap(err) + } + + notNilCount++ + } + + if c.TLSCAs != nil { + if err := c.TLSCAs.CheckAndSetDefaults(); err != nil { + return trace.Wrap(err) + } + + notNilCount++ + } + + if c.Mongo != nil { + if err := c.Mongo.CheckAndSetDefaults(); err != nil { + return trace.Wrap(err) + } + + notNilCount++ + } + + if c.Cockroach != nil { + if err := c.Cockroach.CheckAndSetDefaults(); err != nil { return trace.Wrap(err) } @@ -123,14 +201,85 @@ func (c *TemplateConfig) CheckAndSetDefaults() error { return nil } +// GetConfigTemplate returns the first not-nil config template implementation +// in the struct. func (c *TemplateConfig) GetConfigTemplate() (Template, error) { if c.SSHClient != nil { return c.SSHClient, nil } - if c.IdentityFile != nil { - return c.IdentityFile, nil + if c.Identity != nil { + return c.Identity, nil + } + + if c.TLS != nil { + return c.TLS, nil + } + + if c.TLSCAs != nil { + return c.TLSCAs, nil + } + + if c.Mongo != nil { + return c.Mongo, nil + } + + if c.Cockroach != nil { + return c.Cockroach, nil } return nil, trace.BadParameter("no valid config template") } + +// BotConfigWriter is a trivial adapter to use the identityfile package with +// bot destinations. +type BotConfigWriter struct { + // dest is the destination that will handle writing of files. + dest destination.Destination + + // subpath is the subdirectory within the destination to which the files + // should be written. + subpath string +} + +// WriteFile writes the file to the destination. Only the basename of the path +// is used. Specified permissions are ignored. +func (b *BotConfigWriter) WriteFile(name string, data []byte, _ os.FileMode) error { + p := path.Base(name) + if b.subpath != "" { + p = path.Join(b.subpath, p) + } + + log.Debugf("WriteFile(%q, ...) to %q", name, p) + return trace.Wrap(b.dest.Write(p, data)) +} + +// Remove removes files. This is a dummy implementation that always returns not found. +func (b *BotConfigWriter) Remove(name string) error { + return &os.PathError{Op: "stat", Path: name, Err: os.ErrNotExist} +} + +// Stat checks file status. This implementation always returns not found. +func (b *BotConfigWriter) Stat(name string) (fs.FileInfo, error) { + return nil, &os.PathError{Op: "stat", Path: name, Err: os.ErrNotExist} +} + +// newClientKey returns a sane client.Key for the given bot identity. +func newClientKey(ident *identity.Identity, hostCAs []types.CertAuthority) *client.Key { + return &client.Key{ + KeyIndex: client.KeyIndex{ + ClusterName: ident.ClusterName, + }, + Priv: ident.PrivateKeyBytes, + Pub: ident.PublicKeyBytes, + Cert: ident.CertBytes, + TLSCert: ident.TLSCertBytes, + TrustedCA: auth.AuthoritiesToTrustedCerts(hostCAs), + + // TODO: configure these? we have a 1:1 mapping of destination + // -> app/db/kube cert so this should be knowable, but are these ever + // used by identityfile? + KubeTLSCerts: make(map[string][]byte), + DBTLSCerts: make(map[string][]byte), + } +} diff --git a/tool/tbot/config/configtemplate_cockroach.go b/tool/tbot/config/configtemplate_cockroach.go new file mode 100644 index 0000000000000..018e8789da1d2 --- /dev/null +++ b/tool/tbot/config/configtemplate_cockroach.go @@ -0,0 +1,91 @@ +/* +Copyright 2022 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config + +import ( + "context" + + "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/auth" + "github.com/gravitational/teleport/lib/client/identityfile" + "github.com/gravitational/teleport/tool/tbot/identity" + "github.com/gravitational/trace" +) + +const defaultCockroachDirName = "cockroach" + +// TemplateCockroach generates certificates for CockroachDB. These are standard +// TLS certs but have specific naming requirements. We write them to a +// subdirectory to ensure naming is clear. +type TemplateCockroach struct { + DirName string `yaml:"dir_name,omitempty"` +} + +func (t *TemplateCockroach) CheckAndSetDefaults() error { + if t.DirName == "" { + t.DirName = defaultCockroachDirName + } + + return nil +} + +func (t *TemplateCockroach) Name() string { + return TemplateCockroachName +} + +func (t *TemplateCockroach) Describe() []FileDescription { + return []FileDescription{ + { + Name: t.DirName, + IsDir: true, + }, + } +} + +func (t *TemplateCockroach) Render(ctx context.Context, authClient auth.ClientI, currentIdentity *identity.Identity, destination *DestinationConfig) error { + dest, err := destination.GetDestination() + if err != nil { + return trace.Wrap(err) + } + + hostCAs, err := authClient.GetCertAuthorities(ctx, types.HostCA, false) + if err != nil { + return trace.Wrap(err) + } + + cfg := identityfile.WriteConfig{ + OutputPath: t.DirName, + Writer: &BotConfigWriter{ + dest: dest, + subpath: t.DirName, + }, + Key: newClientKey(currentIdentity, hostCAs), + Format: identityfile.FormatCockroach, + + // Always overwrite to avoid hitting our no-op Stat() and Remove() functions. + OverwriteDestination: true, + } + + files, err := identityfile.Write(cfg) + if err != nil { + return trace.Wrap(err) + } + + log.Debugf("Wrote CockroachDB files: %+v", files) + + return nil +} diff --git a/tool/tbot/config/configtemplate_identity.go b/tool/tbot/config/configtemplate_identity.go new file mode 100644 index 0000000000000..f76ff8d39c40e --- /dev/null +++ b/tool/tbot/config/configtemplate_identity.go @@ -0,0 +1,88 @@ +/* +Copyright 2022 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config + +import ( + "context" + + "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/auth" + "github.com/gravitational/teleport/lib/client/identityfile" + "github.com/gravitational/teleport/tool/tbot/identity" + "github.com/gravitational/trace" +) + +const defaultIdentityFileName = "identity" + +// TemplateIdentity is a config template that generates a Teleport identity +// file that can be used by tsh and tctl. +type TemplateIdentity struct { + FileName string `yaml:"file_name,omitempty"` +} + +func (t *TemplateIdentity) CheckAndSetDefaults() error { + if t.FileName == "" { + t.FileName = defaultIdentityFileName + } + + return nil +} + +func (t *TemplateIdentity) Name() string { + return TemplateIdentityName +} + +func (t *TemplateIdentity) Describe() []FileDescription { + return []FileDescription{ + { + Name: t.FileName, + }, + } +} + +func (t *TemplateIdentity) Render(ctx context.Context, authClient auth.ClientI, currentIdentity *identity.Identity, destination *DestinationConfig) error { + dest, err := destination.GetDestination() + if err != nil { + return trace.Wrap(err) + } + + hostCAs, err := authClient.GetCertAuthorities(ctx, types.HostCA, false) + if err != nil { + return trace.Wrap(err) + } + + cfg := identityfile.WriteConfig{ + OutputPath: t.FileName, + Writer: &BotConfigWriter{ + dest: dest, + }, + Key: newClientKey(currentIdentity, hostCAs), + Format: identityfile.FormatFile, + + // Always overwrite to avoid hitting our no-op Stat() and Remove() functions. + OverwriteDestination: true, + } + + files, err := identityfile.Write(cfg) + if err != nil { + return trace.Wrap(err) + } + + log.Debugf("Wrote identity file: %+v", files) + + return nil +} diff --git a/tool/tbot/config/configtemplate_identityfile.go b/tool/tbot/config/configtemplate_identityfile.go deleted file mode 100644 index 9eba219870835..0000000000000 --- a/tool/tbot/config/configtemplate_identityfile.go +++ /dev/null @@ -1,162 +0,0 @@ -/* -Copyright 2022 Gravitational, Inc. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package config - -import ( - "context" - "io/fs" - "os" - "path" - - "github.com/gravitational/teleport/api/types" - "github.com/gravitational/teleport/lib/auth" - "github.com/gravitational/teleport/lib/client" - "github.com/gravitational/teleport/lib/client/identityfile" - "github.com/gravitational/teleport/tool/tbot/destination" - "github.com/gravitational/teleport/tool/tbot/identity" - "github.com/gravitational/trace" -) - -// BotConfigWriter is a trivial adapter to use the identityfile package with -// bot destinations. -type BotConfigWriter struct { - // dest is the destination that will handle writing of files. - dest destination.Destination - - // subpath is the subdirectory within the destination to which the files - // should be written. - subpath string -} - -// WriteFile writes the file to the destination. Only the basename of the path -// is used. Specified permissions are ignored. -func (b *BotConfigWriter) WriteFile(name string, data []byte, perm os.FileMode) error { - p := path.Join(b.subpath, path.Base(name)) - log.Debugf("WriteFile(%q, ...) to %q", name, p) - return trace.Wrap(b.dest.Write(p, data)) -} - -// Remove removes files. This is a dummy implementation that always returns not found. -func (b *BotConfigWriter) Remove(name string) error { - return &os.PathError{Op: "stat", Path: name, Err: os.ErrNotExist} -} - -// Stat checks file status. This implementation always returns not found. -func (b *BotConfigWriter) Stat(name string) (fs.FileInfo, error) { - return nil, &os.PathError{Op: "stat", Path: name, Err: os.ErrNotExist} -} - -// isFormatValid checks if the given format is a supported file format.s -func isFormatValid(format string) bool { - for _, f := range identityfile.KnownFileFormats { - if string(f) == format { - return true - } - } - - return false -} - -type TemplateIdentityFile struct { - Formats []string `yaml:"formats,omitempty"` -} - -func (t *TemplateIdentityFile) CheckAndSetDefaults() error { - for _, format := range t.Formats { - if !isFormatValid(format) { - return trace.BadParameter("invalid format %q, expected one of: %s", format, identityfile.KnownFileFormats.String()) - } - } - - return nil -} - -func (t *TemplateIdentityFile) Describe() []FileDescription { - var descriptions []FileDescription - - // identityfile may generate multiple files - for _, format := range t.Formats { - descriptions = append(descriptions, FileDescription{ - Name: format, - IsDir: true, - }) - } - - return descriptions -} - -func (t *TemplateIdentityFile) Render(ctx context.Context, authClient auth.ClientI, currentIdentity *identity.Identity, destination *DestinationConfig) error { - if !destination.ContainsKind(identity.KindSSH) { - return trace.BadParameter("%s config template requires kind `ssh` to be enabled", TemplateIdentityFileName) - } - - if !destination.ContainsKind(identity.KindTLS) { - return trace.BadParameter("%s config template requires kind `tls` to be enabled", TemplateIdentityFileName) - } - - dest, err := destination.GetDestination() - if err != nil { - return trace.Wrap(err) - } - - hostCAs, err := authClient.GetCertAuthorities(ctx, types.HostCA, false) - if err != nil { - return trace.Wrap(err) - } - - for _, format := range t.Formats { - cfg := identityfile.WriteConfig{ - // Hard code all written files as "identity" for now. We could make - // this user configurable if desired. - OutputPath: "identity", - Writer: &BotConfigWriter{ - dest: dest, - subpath: format, - }, - Key: &client.Key{ - KeyIndex: client.KeyIndex{ - ClusterName: currentIdentity.ClusterName, - }, - Priv: currentIdentity.PrivateKeyBytes, - Pub: currentIdentity.PublicKeyBytes, - Cert: currentIdentity.CertBytes, - TLSCert: currentIdentity.TLSCertBytes, - TrustedCA: auth.AuthoritiesToTrustedCerts(hostCAs), - - // TODO: configure these? we have a 1:1 mapping of destination - // -> app/db/kube cert so this should be knowable. - KubeTLSCerts: make(map[string][]byte), - DBTLSCerts: make(map[string][]byte), - }, - Format: identityfile.Format(format), - - // Always overwrite to avoid hitting our no-op Stat() and Remove() functions. - OverwriteDestination: true, - - // TODO: KubeProxyAddr: ?, - } - - files, err := identityfile.Write(cfg) - if err != nil { - return trace.Wrap(err) - } - - log.Debugf("Wrote identityfile entries: %+v", files) - } - - return nil -} diff --git a/tool/tbot/config/configtemplate_mongo.go b/tool/tbot/config/configtemplate_mongo.go new file mode 100644 index 0000000000000..3ff88fdf93eab --- /dev/null +++ b/tool/tbot/config/configtemplate_mongo.go @@ -0,0 +1,92 @@ +/* +Copyright 2022 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config + +import ( + "context" + + "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/auth" + "github.com/gravitational/teleport/lib/client/identityfile" + "github.com/gravitational/teleport/tool/tbot/identity" + "github.com/gravitational/trace" +) + +// defaultMongoPrefix is the default prefix in generated MongoDB certs. +const defaultMongoPrefix = "mongo" + +// TemplateMongo is a config template that generates TLS certs formatted for +// use with MongoDB. +type TemplateMongo struct { + Prefix string `yaml:"prefix,omitempty"` +} + +func (t *TemplateMongo) CheckAndSetDefaults() error { + if t.Prefix == "" { + t.Prefix = defaultMongoPrefix + } + + return nil +} + +func (t *TemplateMongo) Name() string { + return TemplateMongoName +} + +func (t *TemplateMongo) Describe() []FileDescription { + return []FileDescription{ + { + Name: t.Prefix + ".crt", + }, + { + Name: t.Prefix + ".cas", + }, + } +} + +func (t *TemplateMongo) Render(ctx context.Context, authClient auth.ClientI, currentIdentity *identity.Identity, destination *DestinationConfig) error { + dest, err := destination.GetDestination() + if err != nil { + return trace.Wrap(err) + } + + hostCAs, err := authClient.GetCertAuthorities(ctx, types.HostCA, false) + if err != nil { + return trace.Wrap(err) + } + + cfg := identityfile.WriteConfig{ + OutputPath: t.Prefix, + Writer: &BotConfigWriter{ + dest: dest, + }, + Key: newClientKey(currentIdentity, hostCAs), + Format: identityfile.FormatMongo, + + // Always overwrite to avoid hitting our no-op Stat() and Remove() functions. + OverwriteDestination: true, + } + + files, err := identityfile.Write(cfg) + if err != nil { + return trace.Wrap(err) + } + + log.Debugf("Wrote MongoDB identity files: %+v", files) + + return nil +} diff --git a/tool/tbot/config/configtemplate_ssh.go b/tool/tbot/config/configtemplate_ssh_client.go similarity index 89% rename from tool/tbot/config/configtemplate_ssh.go rename to tool/tbot/config/configtemplate_ssh_client.go index 2c8f7da6d9550..7ae641d7c8944 100644 --- a/tool/tbot/config/configtemplate_ssh.go +++ b/tool/tbot/config/configtemplate_ssh_client.go @@ -20,7 +20,6 @@ import ( "bytes" "context" "fmt" - "os" "os/exec" "path/filepath" "regexp" @@ -51,7 +50,16 @@ var openSSHVersionRegex = regexp.MustCompile(`^OpenSSH_(?P\d+)\.(?P 0, "file %q in template %q must be non-empty", file.Name, tplI.Name()) + } + + // Next, for supported template types, make sure they're valid. + // TODO: consider adding further type-specific tests. + switch tpl := tplI.(type) { + case *config.TemplateIdentity: + // Make sure the identityfile package can read this identity file. + b, err := dest.Read(tpl.FileName) + require.NoError(t, err) + + buf := bytes.NewBuffer(b) + _, err = identityfile.Read(buf) + require.NoError(t, err) + case *config.TemplateTLSCAs: + b, err := dest.Read(tpl.HostCAPath) + require.NoError(t, err) + _, err = tlsca.ParseCertificatePEM(b) + require.NoError(t, err) + + b, err = dest.Read(tpl.UserCAPath) + require.NoError(t, err) + _, err = tlsca.ParseCertificatePEM(b) + require.NoError(t, err) + } +} + +// TestTemplateRendering performs a full renewal and ensures all expected +// default config templates are present. +func TestDefaultTemplateRendering(t *testing.T) { + utils.InitLogger(utils.LoggingForDaemon, logrus.DebugLevel) + + // Make a new auth server. + fc := testhelpers.DefaultConfig(t) + _ = testhelpers.MakeAndRunTestAuthServer(t, fc) + rootClient := testhelpers.MakeDefaultAuthClient(t, fc) + + // Make and join a new bot instance. + botParams := testhelpers.MakeBot(t, rootClient, "test") + botConfig := testhelpers.MakeMemoryBotConfig(t, fc, botParams) + storage, err := botConfig.Storage.GetDestination() + require.NoError(t, err) + + ident, err := getIdentityFromToken(botConfig) + require.NoError(t, err) + + botClient := testhelpers.MakeBotAuthClient(t, fc, ident) + + _, _, err = renew(context.Background(), botConfig, botClient, ident, storage) + require.NoError(t, err) + + dest := botConfig.Destinations[0] + destImpl, err := dest.GetDestination() + require.NoError(t, err) + + for _, templateName := range config.GetRequiredConfigs() { + cfg := dest.GetConfigByName(templateName) + require.NotNilf(t, cfg, "template %q must exist", templateName) + + validateTemplate(t, cfg, destImpl) + } +} diff --git a/tool/tbot/identity/artifact.go b/tool/tbot/identity/artifact.go index 7716a177603ad..3ace0ed0398cc 100644 --- a/tool/tbot/identity/artifact.go +++ b/tool/tbot/identity/artifact.go @@ -51,7 +51,7 @@ var artifacts = []Artifact{ // SSH artifacts { Key: SSHCertKey, - Kind: KindSSH, + Kind: KindAlways, ToBytes: func(i *Identity) []byte { return i.CertBytes }, @@ -60,8 +60,13 @@ var artifacts = []Artifact{ }, }, { - Key: SSHCACertsKey, - Kind: KindSSH, + Key: SSHCACertsKey, + + // SSH CAs in this format are only used for saving/loading of bot + // identities and are not particularly useful to end users. We encode + // the current SSH CAs inside the known_hosts file generated with the + // `ssh_config` template, which is actually readable by OpenSSH. + Kind: KindBotInternal, ToBytes: func(i *Identity) []byte { return bytes.Join(i.SSHCACertBytes, []byte("$")) }, @@ -73,7 +78,7 @@ var artifacts = []Artifact{ // TLS artifacts { Key: TLSCertKey, - Kind: KindTLS, + Kind: KindAlways, ToBytes: func(i *Identity) []byte { return i.TLSCertBytes }, @@ -82,8 +87,15 @@ var artifacts = []Artifact{ }, }, { - Key: TLSCACertsKey, - Kind: KindTLS, + Key: TLSCACertsKey, + + // TLS CA certs are useful to end users, but this artifact contains an + // arbitrary number of CAs, including both Teleport's user and host CAs + // and potentially multiple sets if they've been rotated. + // Instead of exposing this mess of CAs to end users, we'll keep these + // for internal use and just present single standard CAs in destination + // dirs. + Kind: KindBotInternal, ToBytes: func(i *Identity) []byte { return bytes.Join(i.TLSCACertsBytes, []byte("$")) }, diff --git a/tool/tbot/identity/identity.go b/tool/tbot/identity/identity.go index 5543931cf4479..06fce471db6af 100644 --- a/tool/tbot/identity/identity.go +++ b/tool/tbot/identity/identity.go @@ -248,30 +248,33 @@ func (i *Identity) SSHClientConfig() (*ssh.ClientConfig, error) { // ReadIdentityFromStore reads stored identity credentials func ReadIdentityFromStore(params *LoadIdentityParams, certs *proto.Certs, kinds ...ArtifactKind) (*Identity, error) { var identity Identity - if ContainsKind(KindSSH, kinds) { - if len(certs.SSH) == 0 { - return nil, trace.BadParameter("identity requires SSH certificates but they are unset") - } - err := ReadSSHIdentityFromKeyPair(&identity, params.PrivateKeyBytes, params.PrivateKeyBytes, certs.SSH) - if err != nil { - return nil, trace.Wrap(err) - } + // Note: in practice we should always expect certificates to have all + // fields set even though destinations do not contain sufficient data to + // load a stored identity. This works in practice because we never read + // destination identities from disk and only read them from the result of + // `generateUserCerts`, which is always fully-formed. - if len(certs.SSHCACerts) != 0 { - identity.SSHCACertBytes = certs.SSHCACerts - } + if len(certs.SSH) == 0 { + return nil, trace.BadParameter("identity requires SSH certificates but they are unset") } - if ContainsKind(KindTLS, kinds) { - if len(certs.TLSCACerts) == 0 || len(certs.TLS) == 0 { - return nil, trace.BadParameter("identity requires TLS certificates but they are empty") - } + if len(certs.TLSCACerts) == 0 || len(certs.TLS) == 0 { + return nil, trace.BadParameter("identity requires TLS certificates but they are empty") + } - // Parse the key pair to verify that identity parses properly for future use. - if err := ReadTLSIdentityFromKeyPair(&identity, params.PrivateKeyBytes, certs.TLS, certs.TLSCACerts); err != nil { - return nil, trace.Wrap(err) - } + err := ReadSSHIdentityFromKeyPair(&identity, params.PrivateKeyBytes, params.PrivateKeyBytes, certs.SSH) + if err != nil { + return nil, trace.Wrap(err) + } + + if len(certs.SSHCACerts) != 0 { + identity.SSHCACertBytes = certs.SSHCACerts + } + + // Parse the key pair to verify that identity parses properly for future use. + if err := ReadTLSIdentityFromKeyPair(&identity, params.PrivateKeyBytes, certs.TLS, certs.TLSCACerts); err != nil { + return nil, trace.Wrap(err) } identity.PublicKeyBytes = params.PublicKeyBytes diff --git a/tool/tbot/identity/kinds.go b/tool/tbot/identity/kinds.go index 5e98ff2da6905..6b9e3058c8d85 100644 --- a/tool/tbot/identity/kinds.go +++ b/tool/tbot/identity/kinds.go @@ -16,13 +16,6 @@ limitations under the License. package identity -import ( - "strings" - - "github.com/gravitational/trace" - "gopkg.in/yaml.v3" -) - // ArtifactKind is a type of identity artifact that can be stored and loaded. type ArtifactKind string @@ -31,42 +24,11 @@ const ( // generated. KindAlways ArtifactKind = "always" - // KindSSH identifies resources that should only be generated for SSH use. - KindSSH ArtifactKind = "ssh" - - // KindTLS identifies resources that should only be stored for TLS use. - KindTLS ArtifactKind = "tls" - // KindBotInternal identifies resources that should only be stored in the // bot's internal data directory. KindBotInternal ArtifactKind = "bot-internal" ) -// allConfigKinds is a list of all ArtifactKinds allowed in config files. -var allConfigKinds = []string{string(KindSSH), string(KindTLS)} - -func (ac *ArtifactKind) UnmarshalYAML(node *yaml.Node) error { - var kind string - if err := node.Decode(&kind); err != nil { - return err - } - - // Only TLS and SSH are configurable values. - switch kind { - case string(KindTLS): - *ac = KindTLS - case string(KindSSH): - *ac = KindSSH - default: - return trace.BadParameter( - "invalid kind %q, expected one of: %s", - kind, strings.Join(allConfigKinds, ", "), - ) - } - - return nil -} - // ContainsKind determines if a particular artifact kind is included in the // list of kinds. func ContainsKind(kind ArtifactKind, kinds []ArtifactKind) bool { @@ -82,5 +44,11 @@ func ContainsKind(kind ArtifactKind, kinds []ArtifactKind) bool { // BotKinds returns a list of all artifact kinds used internally by the bot. // End-user destinations may contain a different set of artifacts. func BotKinds() []ArtifactKind { - return []ArtifactKind{KindAlways, KindBotInternal, KindSSH, KindTLS} + return []ArtifactKind{KindAlways, KindBotInternal} +} + +// DestinationKinds returns a list of all artifact kinds that should be written +// to end-user destinations. +func DestinationKinds() []ArtifactKind { + return []ArtifactKind{KindAlways} } diff --git a/tool/tbot/init.go b/tool/tbot/init.go index 88b9d852ff6d4..3773670e258cb 100644 --- a/tool/tbot/init.go +++ b/tool/tbot/init.go @@ -47,7 +47,7 @@ func getInitArtifacts(destination *config.DestinationConfig) (map[string]bool, e // Collect all base artifacts and filter for the destination. for _, artifact := range identity.GetArtifacts() { - if artifact.Matches(destination.Kinds...) { + if artifact.Matches(identity.DestinationKinds()...) { toCreate[artifact.Key] = false } } diff --git a/tool/tbot/init_test.go b/tool/tbot/init_test.go index d154e0cb39480..d0056f3641210 100644 --- a/tool/tbot/init_test.go +++ b/tool/tbot/init_test.go @@ -136,7 +136,7 @@ func validateFileDestination(t *testing.T, dest *config.DestinationConfig) *conf require.True(t, ok) for _, art := range identity.GetArtifacts() { - if !art.Matches(dest.Kinds...) { + if !art.Matches(identity.DestinationKinds()...) { continue } @@ -214,7 +214,7 @@ func TestInitMaybeACLs(t *testing.T) { // If we expect ACLs, verify them. if expectACLs { - require.NoError(t, destDir.Verify(identity.ListKeys(cfg.Destinations[0].Kinds...))) + require.NoError(t, destDir.Verify(identity.ListKeys(identity.DestinationKinds()...))) } else { t.Logf("Skipping ACL check on %q as they should not be supported.", dir) } diff --git a/tool/tbot/renew.go b/tool/tbot/renew.go index 2933eb48dc251..1cc042241465a 100644 --- a/tool/tbot/renew.go +++ b/tool/tbot/renew.go @@ -24,7 +24,6 @@ import ( "strings" "time" - "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/api/defaults" @@ -133,51 +132,6 @@ func describeTLSIdentity(ident *identity.Identity) (string, error) { ), nil } -// describeSSHIdentity generates an informational message about the given -// SSH identity, appropriate for user-facing log messages. -func describeSSHIdentity(ident *identity.Identity) (string, error) { - cert := ident.SSHCert - if cert == nil { - return "", trace.BadParameter("attempted to describe SSH identity without SSH credentials") - } - - renewable := false - if _, ok := cert.Extensions[teleport.CertExtensionRenewable]; ok { - renewable = true - } - - disallowReissue := false - if _, ok := cert.Extensions[teleport.CertExtensionDisallowReissue]; ok { - disallowReissue = true - } - - var roles []string - if rolesStr, ok := cert.Extensions[teleport.CertExtensionTeleportRoles]; ok { - if actualRoles, err := services.UnmarshalCertRoles(rolesStr); err == nil { - roles = actualRoles - } - } - - var principals []string - for _, principal := range cert.ValidPrincipals { - if !strings.HasPrefix(principal, constants.NoLoginPrefix) { - principals = append(principals, principal) - } - } - - duration := time.Second * time.Duration(cert.ValidBefore-cert.ValidAfter) - return fmt.Sprintf( - "valid: after=%v, before=%v, duration=%s | kind=ssh, renewable=%v, disallow-reissue=%v, roles=%v, principals=%v", - time.Unix(int64(cert.ValidAfter), 0).Format(time.RFC3339), - time.Unix(int64(cert.ValidBefore), 0).Format(time.RFC3339), - duration, - renewable, - disallowReissue, - roles, - principals, - ), nil -} - // identityConfigurator is a function that alters a cert request type identityConfigurator = func(req *proto.UserCertsRequest) @@ -258,7 +212,7 @@ func generateIdentity( newIdentity, err := identity.ReadIdentityFromStore(&identity.LoadIdentityParams{ PrivateKeyBytes: privateKey, PublicKeyBytes: publicKey, - }, certs, destCfg.Kinds...) + }, certs, identity.DestinationKinds()...) if err != nil { return nil, trace.Wrap(err) } @@ -536,7 +490,7 @@ func renew( // Check the ACLs. We can't fix them, but we can warn if they're // misconfigured. We'll need to precompute a list of keys to check. // Note: This may only log a warning, depending on configuration. - if err := destImpl.Verify(identity.ListKeys(dest.Kinds...)); err != nil { + if err := destImpl.Verify(identity.ListKeys(identity.DestinationKinds()...)); err != nil { return nil, nil, trace.Wrap(err) } @@ -553,22 +507,14 @@ func renew( return nil, nil, trace.Wrap(err, "Failed to generate impersonated certs for %s: %+v", destImpl, err) } - var impersonatedIdentStr string - if dest.ContainsKind(identity.KindTLS) { - impersonatedIdentStr, err = describeTLSIdentity(impersonatedIdent) - if err != nil { - return nil, nil, trace.Wrap(err, "could not describe impersonated certs for destination %s", destImpl) - } - } else { - // Note: kinds must contain at least 1 of TLS or SSH - impersonatedIdentStr, err = describeSSHIdentity(impersonatedIdent) - if err != nil { - return nil, nil, trace.Wrap(err, "could not describe impersonated certs for destination %s", destImpl) - } + impersonatedIdentStr, err := describeTLSIdentity(impersonatedIdent) + if err != nil { + return nil, nil, trace.Wrap(err, "could not describe impersonated certs for destination %s", destImpl) } + log.Infof("Successfully renewed impersonated certificates for %s, %s", destImpl, impersonatedIdentStr) - if err := identity.SaveIdentity(impersonatedIdent, destImpl, dest.Kinds...); err != nil { + if err := identity.SaveIdentity(impersonatedIdent, destImpl, identity.DestinationKinds()...); err != nil { return nil, nil, trace.Wrap(err, "failed to save impersonated identity to destination %s", destImpl) } diff --git a/tool/tbot/renew_test.go b/tool/tbot/renew_test.go index 3965ce3853d50..95813f602ddc6 100644 --- a/tool/tbot/renew_test.go +++ b/tool/tbot/renew_test.go @@ -26,7 +26,6 @@ import ( libconfig "github.com/gravitational/teleport/lib/config" "github.com/gravitational/teleport/lib/tlsca" "github.com/gravitational/teleport/tool/tbot/config" - "github.com/gravitational/teleport/tool/tbot/identity" "github.com/gravitational/teleport/tool/tbot/testhelpers" "github.com/gravitational/trace" "github.com/stretchr/testify/require" @@ -120,7 +119,6 @@ func TestDatabaseRequest(t *testing.T) { botConfig := testhelpers.MakeMemoryBotConfig(t, fc, botParams) dest := botConfig.Destinations[0] - dest.Kinds = []identity.ArtifactKind{identity.KindSSH, identity.KindTLS} dest.Database = &config.DatabaseConfig{ Service: "foo", Database: "bar", diff --git a/tool/tbot/testhelpers/srv.go b/tool/tbot/testhelpers/srv.go index 4bddd31065571..642026ba3f8e4 100644 --- a/tool/tbot/testhelpers/srv.go +++ b/tool/tbot/testhelpers/srv.go @@ -50,8 +50,9 @@ func DefaultConfig(t *testing.T) *config.FileConfig { Service: config.Service{ EnabledFlag: "true", }, - WebAddr: mustGetFreeLocalListenerAddr(t), - TunAddr: mustGetFreeLocalListenerAddr(t), + WebAddr: mustGetFreeLocalListenerAddr(t), + TunAddr: mustGetFreeLocalListenerAddr(t), + PublicAddr: []string{"proxy.example.com"}, }, Auth: config.Auth{ Service: config.Service{ From 05ccf3a8955a8ac45edab9ba8dbc6315452949ea Mon Sep 17 00:00:00 2001 From: Tim Buckley Date: Mon, 25 Apr 2022 15:07:54 -0600 Subject: [PATCH 03/10] Update lib/client/identityfile/identity.go Co-authored-by: Jakub Nyckowski --- lib/client/identityfile/identity.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/client/identityfile/identity.go b/lib/client/identityfile/identity.go index 83e40ab5a1706..651e02256a97a 100644 --- a/lib/client/identityfile/identity.go +++ b/lib/client/identityfile/identity.go @@ -98,7 +98,7 @@ type ConfigWriter interface { // permissions if the file is new. WriteFile(name string, data []byte, perm os.FileMode) error - // Remove removes a file + // Remove removes a file. Remove(name string) error // Stat fetches information about a file. From 8b9b2bc57ae2a12f95238fcc9b89dff3c833a78c Mon Sep 17 00:00:00 2001 From: Tim Buckley Date: Mon, 25 Apr 2022 17:56:04 -0600 Subject: [PATCH 04/10] Address first batch of review comments Tweaked the `botfs.openStandard` and `botfs.openSecure` functions to accept a plain file mode, and removed a ton of boilerplate in `configtemplate.go`. --- tool/tbot/botfs/botfs.go | 18 +++--- tool/tbot/botfs/fs_linux.go | 23 +++----- tool/tbot/botfs/fs_other.go | 4 +- tool/tbot/config/configtemplate.go | 94 +++++++++--------------------- 4 files changed, 46 insertions(+), 93 deletions(-) diff --git a/tool/tbot/botfs/botfs.go b/tool/tbot/botfs/botfs.go index ccd00ac34f3e2..33b12c3982183 100644 --- a/tool/tbot/botfs/botfs.go +++ b/tool/tbot/botfs/botfs.go @@ -68,6 +68,9 @@ const ( ACLRequired ACLMode = "required" ) +// OpenMode is a mode for opening files. +type OpenMode int + const ( // DefaultMode is the preferred permissions mode for bot files. DefaultMode fs.FileMode = 0600 @@ -79,11 +82,11 @@ const ( // ReadMode is the mode with which files should be opened for reading and // writing. - ReadMode int = os.O_CREATE | os.O_RDONLY + ReadMode OpenMode = OpenMode(os.O_CREATE | os.O_RDONLY) // WriteMode is the mode with which files should be opened specifically // for writing. - WriteMode int = os.O_CREATE | os.O_WRONLY | os.O_TRUNC + WriteMode OpenMode = OpenMode(os.O_CREATE | os.O_WRONLY | os.O_TRUNC) ) // ACLOptions contains parameters needed to configure ACLs @@ -98,13 +101,8 @@ type ACLOptions struct { // openStandard attempts to open the given path for reading and writing with // O_CREATE set. -func openStandard(path string, write bool) (*os.File, error) { - mode := ReadMode - if write { - mode = WriteMode - } - - file, err := os.OpenFile(path, mode, DefaultMode) +func openStandard(path string, mode OpenMode) (*os.File, error) { + file, err := os.OpenFile(path, int(mode), DefaultMode) if err != nil { return nil, trace.ConvertSystemError(err) } @@ -123,7 +121,7 @@ func createStandard(path string, isDir bool) error { return nil } - f, err := openStandard(path, true) + f, err := openStandard(path, WriteMode) if err != nil { return trace.Wrap(err) } diff --git a/tool/tbot/botfs/fs_linux.go b/tool/tbot/botfs/fs_linux.go index 61e41d52a3891..948aea2b4e382 100644 --- a/tool/tbot/botfs/fs_linux.go +++ b/tool/tbot/botfs/fs_linux.go @@ -58,12 +58,7 @@ var missingSyscallWarning sync.Once // openSecure opens the given path for writing (with O_CREAT, mode 0600) // with the RESOLVE_NO_SYMLINKS flag set. -func openSecure(path string, write bool) (*os.File, error) { - mode := ReadMode - if write { - mode = WriteMode - } - +func openSecure(path string, mode OpenMode) (*os.File, error) { how := unix.OpenHow{ // Equivalent to 0600. Unfortunately it's not worth reusing our // default file mode constant here. @@ -86,13 +81,13 @@ func openSecure(path string, write bool) (*os.File, error) { // openSymlinks mode opens the file for read or write using the given symlink // mode, potentially failing or logging a warning if symlinks can't be // secured. -func openSymlinksMode(path string, write bool, symlinksMode SymlinksMode) (*os.File, error) { +func openSymlinksMode(path string, mode OpenMode, symlinksMode SymlinksMode) (*os.File, error) { var file *os.File var err error switch symlinksMode { case SymlinksSecure: - file, err = openSecure(path, write) + file, err = openSecure(path, mode) if err == unix.ENOSYS { return nil, trace.Errorf("openSecure(%q) failed due to missing "+ "syscall; `symlinks: insecure` may be required for this "+ @@ -101,13 +96,13 @@ func openSymlinksMode(path string, write bool, symlinksMode SymlinksMode) (*os.F return nil, trace.Wrap(err) } case SymlinksTrySecure: - file, err = openSecure(path, write) + file, err = openSecure(path, mode) if err == unix.ENOSYS { log.Warnf("Failed to write to %q securely due to missing "+ "syscall; falling back to regular file write. Set "+ "`symlinks: insecure` on this destination to disable this "+ "warning.", path) - file, err = openStandard(path, write) + file, err = openStandard(path, mode) if err != nil { return nil, trace.Wrap(err) } @@ -115,7 +110,7 @@ func openSymlinksMode(path string, write bool, symlinksMode SymlinksMode) (*os.F return nil, trace.Wrap(err) } case SymlinksInsecure: - file, err = openStandard(path, write) + file, err = openStandard(path, mode) if err != nil { return nil, trace.Wrap(err) } @@ -141,7 +136,7 @@ func createSecure(path string, isDir bool) error { return nil } - f, err := openSecure(path, true) + f, err := openSecure(path, WriteMode) if err == unix.ENOSYS { // bubble up the original error for comparison return err @@ -209,7 +204,7 @@ func Create(path string, isDir bool, symlinksMode SymlinksMode) error { // Read reads the contents of the given file into memory. func Read(path string, symlinksMode SymlinksMode) ([]byte, error) { - file, err := openSymlinksMode(path, false, symlinksMode) + file, err := openSymlinksMode(path, ReadMode, symlinksMode) if err != nil { return nil, trace.Wrap(err) } @@ -226,7 +221,7 @@ func Read(path string, symlinksMode SymlinksMode) ([]byte, error) { // Write stores the given data to the file at the given path. func Write(path string, data []byte, symlinksMode SymlinksMode) error { - file, err := openSymlinksMode(path, true, symlinksMode) + file, err := openSymlinksMode(path, WriteMode, symlinksMode) if err != nil { return trace.Wrap(err) } diff --git a/tool/tbot/botfs/fs_other.go b/tool/tbot/botfs/fs_other.go index 87b2b90e3721a..3260ef4dab777 100644 --- a/tool/tbot/botfs/fs_other.go +++ b/tool/tbot/botfs/fs_other.go @@ -47,7 +47,7 @@ func Read(path string, symlinksMode SymlinksMode) ([]byte, error) { log.Warn("Secure symlinks not supported on this platform, set `symlinks: insecure` to disable this message", path) } - file, err := openStandard(path) + file, err := openStandard(path, ReadMode) if err != nil { return nil, trace.Wrap(err) } @@ -71,7 +71,7 @@ func Write(path string, data []byte, symlinksMode SymlinksMode) error { log.Warn("Secure symlinks not supported on this platform, set `symlinks: insecure` to disable this message", path) } - file, err := openStandard(path) + file, err := openStandard(path, WriteMode) if err != nil { return trace.Wrap(err) } diff --git a/tool/tbot/config/configtemplate.go b/tool/tbot/config/configtemplate.go index f5b69582c49db..95af9754cd8bf 100644 --- a/tool/tbot/config/configtemplate.go +++ b/tool/tbot/config/configtemplate.go @@ -142,54 +142,24 @@ func (c *TemplateConfig) UnmarshalYAML(node *yaml.Node) error { } func (c *TemplateConfig) CheckAndSetDefaults() error { - notNilCount := 0 - - if c.SSHClient != nil { - if err := c.SSHClient.CheckAndSetDefaults(); err != nil { - return trace.Wrap(err) - } - - notNilCount++ - } - - if c.Identity != nil { - if err := c.Identity.CheckAndSetDefaults(); err != nil { - return trace.Wrap(err) - } - - notNilCount++ - } - - if c.TLS != nil { - if err := c.TLS.CheckAndSetDefaults(); err != nil { - return trace.Wrap(err) - } - - notNilCount++ - } - - if c.TLSCAs != nil { - if err := c.TLSCAs.CheckAndSetDefaults(); err != nil { - return trace.Wrap(err) - } - - notNilCount++ + templates := []interface{ CheckAndSetDefaults() error }{ + c.SSHClient, + c.Identity, + c.TLS, + c.TLSCAs, + c.Mongo, + c.Cockroach, } - if c.Mongo != nil { - if err := c.Mongo.CheckAndSetDefaults(); err != nil { - return trace.Wrap(err) - } - - notNilCount++ - } + notNilCount := 0 + for _, template := range templates { + if template != nil { + if err := template.CheckAndSetDefaults(); err != nil { + return trace.Wrap(err) + } - if c.Cockroach != nil { - if err := c.Cockroach.CheckAndSetDefaults(); err != nil { - return trace.Wrap(err) + notNilCount++ } - - notNilCount++ } if notNilCount == 0 { @@ -204,28 +174,19 @@ func (c *TemplateConfig) CheckAndSetDefaults() error { // GetConfigTemplate returns the first not-nil config template implementation // in the struct. func (c *TemplateConfig) GetConfigTemplate() (Template, error) { - if c.SSHClient != nil { - return c.SSHClient, nil - } - - if c.Identity != nil { - return c.Identity, nil - } - - if c.TLS != nil { - return c.TLS, nil - } - - if c.TLSCAs != nil { - return c.TLSCAs, nil - } - - if c.Mongo != nil { - return c.Mongo, nil - } - - if c.Cockroach != nil { - return c.Cockroach, nil + templates := []Template{ + c.SSHClient, + c.Identity, + c.TLS, + c.TLSCAs, + c.Mongo, + c.Cockroach, + } + + for _, template := range templates { + if template != nil { + return template, nil + } } return nil, trace.BadParameter("no valid config template") @@ -250,7 +211,6 @@ func (b *BotConfigWriter) WriteFile(name string, data []byte, _ os.FileMode) err p = path.Join(b.subpath, p) } - log.Debugf("WriteFile(%q, ...) to %q", name, p) return trace.Wrap(b.dest.Write(p, data)) } From 859f3e04c9f4eb04f539b7938d68a49b0d2d411e Mon Sep 17 00:00:00 2001 From: Tim Buckley Date: Thu, 28 Apr 2022 14:50:21 -0600 Subject: [PATCH 05/10] Fix problematic nil interface check in configtemplate --- tool/tbot/config/configtemplate.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tool/tbot/config/configtemplate.go b/tool/tbot/config/configtemplate.go index 95af9754cd8bf..aa306dd06908d 100644 --- a/tool/tbot/config/configtemplate.go +++ b/tool/tbot/config/configtemplate.go @@ -21,6 +21,7 @@ import ( "io/fs" "os" "path" + "reflect" "strings" "github.com/gravitational/teleport/api/types" @@ -153,6 +154,13 @@ func (c *TemplateConfig) CheckAndSetDefaults() error { notNilCount := 0 for _, template := range templates { + // Note: this check is fragile and will fail if the templates aren't + // all simple pointer types. They are, though, and "correct" solution + // is insane, so we'll stick with this. + if template == nil || reflect.ValueOf(template).IsNil() { + continue + } + if template != nil { if err := template.CheckAndSetDefaults(); err != nil { return trace.Wrap(err) @@ -184,9 +192,12 @@ func (c *TemplateConfig) GetConfigTemplate() (Template, error) { } for _, template := range templates { - if template != nil { - return template, nil + // Note: same caveats as above. + if template == nil || reflect.ValueOf(template).IsNil() { + continue } + + return template, nil } return nil, trace.BadParameter("no valid config template") From 7da161d4792de6b47c0aad04115764fa63257355 Mon Sep 17 00:00:00 2001 From: Tim Buckley Date: Mon, 2 May 2022 16:24:39 -0600 Subject: [PATCH 06/10] Clarify comment about `client.Key` DB certs --- tool/tbot/config/configtemplate.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tool/tbot/config/configtemplate.go b/tool/tbot/config/configtemplate.go index aa306dd06908d..ee51044a960f9 100644 --- a/tool/tbot/config/configtemplate.go +++ b/tool/tbot/config/configtemplate.go @@ -247,9 +247,9 @@ func newClientKey(ident *identity.Identity, hostCAs []types.CertAuthority) *clie TLSCert: ident.TLSCertBytes, TrustedCA: auth.AuthoritiesToTrustedCerts(hostCAs), - // TODO: configure these? we have a 1:1 mapping of destination - // -> app/db/kube cert so this should be knowable, but are these ever - // used by identityfile? + // Note: these fields are never used or persisted with identity files, + // so we won't bother to set them. (They may need to be reconstituted + // on tsh's end based on cert fields, though.) KubeTLSCerts: make(map[string][]byte), DBTLSCerts: make(map[string][]byte), } From fc2a237a9f6198fde74af1ab0a171b531d3a4584 Mon Sep 17 00:00:00 2001 From: Tim Buckley Date: Mon, 2 May 2022 18:15:33 -0600 Subject: [PATCH 07/10] Address review feedback - Use `DatabaseCA` for database specific templates; make the `tls` template's CA configurable; write the database CA alongside the others. - Simplify nil interface check --- tool/tbot/config/configtemplate.go | 8 ++-- tool/tbot/config/configtemplate_cockroach.go | 2 +- tool/tbot/config/configtemplate_mongo.go | 2 +- tool/tbot/config/configtemplate_tls.go | 50 +++++++++++++++++++- tool/tbot/config/configtemplate_tls_cas.go | 26 ++++++++++ 5 files changed, 80 insertions(+), 8 deletions(-) diff --git a/tool/tbot/config/configtemplate.go b/tool/tbot/config/configtemplate.go index ee51044a960f9..2f4d3ea4b2380 100644 --- a/tool/tbot/config/configtemplate.go +++ b/tool/tbot/config/configtemplate.go @@ -155,9 +155,9 @@ func (c *TemplateConfig) CheckAndSetDefaults() error { notNilCount := 0 for _, template := range templates { // Note: this check is fragile and will fail if the templates aren't - // all simple pointer types. They are, though, and "correct" solution - // is insane, so we'll stick with this. - if template == nil || reflect.ValueOf(template).IsNil() { + // all simple pointer types. They are, though, and the "correct" + // solution is insane, so we'll stick with this. + if reflect.ValueOf(template).IsNil() { continue } @@ -193,7 +193,7 @@ func (c *TemplateConfig) GetConfigTemplate() (Template, error) { for _, template := range templates { // Note: same caveats as above. - if template == nil || reflect.ValueOf(template).IsNil() { + if reflect.ValueOf(template).IsNil() { continue } diff --git a/tool/tbot/config/configtemplate_cockroach.go b/tool/tbot/config/configtemplate_cockroach.go index 018e8789da1d2..b5cd72c79b8c1 100644 --- a/tool/tbot/config/configtemplate_cockroach.go +++ b/tool/tbot/config/configtemplate_cockroach.go @@ -62,7 +62,7 @@ func (t *TemplateCockroach) Render(ctx context.Context, authClient auth.ClientI, return trace.Wrap(err) } - hostCAs, err := authClient.GetCertAuthorities(ctx, types.HostCA, false) + hostCAs, err := authClient.GetCertAuthorities(ctx, types.DatabaseCA, false) if err != nil { return trace.Wrap(err) } diff --git a/tool/tbot/config/configtemplate_mongo.go b/tool/tbot/config/configtemplate_mongo.go index 3ff88fdf93eab..2d48ee312ab99 100644 --- a/tool/tbot/config/configtemplate_mongo.go +++ b/tool/tbot/config/configtemplate_mongo.go @@ -64,7 +64,7 @@ func (t *TemplateMongo) Render(ctx context.Context, authClient auth.ClientI, cur return trace.Wrap(err) } - hostCAs, err := authClient.GetCertAuthorities(ctx, types.HostCA, false) + hostCAs, err := authClient.GetCertAuthorities(ctx, types.DatabaseCA, false) if err != nil { return trace.Wrap(err) } diff --git a/tool/tbot/config/configtemplate_tls.go b/tool/tbot/config/configtemplate_tls.go index e2b7ce9878c11..183432b1eb271 100644 --- a/tool/tbot/config/configtemplate_tls.go +++ b/tool/tbot/config/configtemplate_tls.go @@ -24,16 +24,58 @@ import ( "github.com/gravitational/teleport/lib/client/identityfile" "github.com/gravitational/teleport/tool/tbot/identity" "github.com/gravitational/trace" + "gopkg.in/yaml.v3" ) const defaultTLSPrefix = "tls" +// CertAuthType is a types.CertAuthType wrapper with unmarshalling support. +type CertAuthType types.CertAuthType + +const defaultCAType = types.HostCA + +func (c *CertAuthType) UnmarshalYAML(node *yaml.Node) error { + var certType string + err := node.Decode(&certType) + if err != nil { + return trace.Wrap(err) + } + + switch certType { + case "": + *c = CertAuthType(defaultCAType) + case string(types.HostCA), string(types.UserCA), string(types.DatabaseCA): + *c = CertAuthType(certType) + default: + return trace.BadParameter("invalid CA certificate type: %q", certType) + } + + return nil +} + +func (c *CertAuthType) CheckAndSetDefaults() error { + switch types.CertAuthType(*c) { + case "": + *c = CertAuthType(defaultCAType) + case types.HostCA, types.UserCA, types.DatabaseCA: + // valid, nothing to do + default: + return trace.BadParameter("unsupported CA certificate type: %q", string(*c)) + } + + return nil +} + // TemplateTLS is a config template that wraps identityfile's TLS writer. // It's not generally needed but can be used to write out TLS certificates with // alternative prefix and file extensions if needed for application // compatibility reasons. type TemplateTLS struct { + // Prefix is the filename prefix for the output files. Prefix string `yaml:"prefix,omitempty"` + + // CACertType is the type of CA cert to be written + CACertType CertAuthType `yaml:"ca_cert_type,omitempty"` } func (t *TemplateTLS) CheckAndSetDefaults() error { @@ -41,6 +83,10 @@ func (t *TemplateTLS) CheckAndSetDefaults() error { t.Prefix = defaultTLSPrefix } + if err := t.CACertType.CheckAndSetDefaults(); err != nil { + return trace.Wrap(err) + } + return nil } @@ -68,7 +114,7 @@ func (t *TemplateTLS) Render(ctx context.Context, authClient auth.ClientI, curre return trace.Wrap(err) } - hostCAs, err := authClient.GetCertAuthorities(ctx, types.HostCA, false) + cas, err := authClient.GetCertAuthorities(ctx, types.CertAuthType(t.CACertType), false) if err != nil { return trace.Wrap(err) } @@ -78,7 +124,7 @@ func (t *TemplateTLS) Render(ctx context.Context, authClient auth.ClientI, curre Writer: &BotConfigWriter{ dest: dest, }, - Key: newClientKey(currentIdentity, hostCAs), + Key: newClientKey(currentIdentity, cas), Format: identityfile.FormatTLS, // Always overwrite to avoid hitting our no-op Stat() and Remove() functions. diff --git a/tool/tbot/config/configtemplate_tls_cas.go b/tool/tbot/config/configtemplate_tls_cas.go index 4b72dcbacc4ad..9048935d5770d 100644 --- a/tool/tbot/config/configtemplate_tls_cas.go +++ b/tool/tbot/config/configtemplate_tls_cas.go @@ -26,9 +26,15 @@ import ( ) const ( + // defaultHostCAPath is the default filename for the host CA certificate defaultHostCAPath = "teleport-host-ca.crt" + // defaultUserCAPath is the default filename for the user CA certificate defaultUserCAPath = "teleport-user-ca.crt" + + // defaultDatabaseCAPath is the default filename for the database CA + // certificate + defaultDatabaseCAPath = "teleport-database-ca.crt" ) // TemplateTLSCAs outputs Teleport's host and user CAs for miscellaneous TLS @@ -39,6 +45,10 @@ type TemplateTLSCAs struct { // UserCAPath is the path to which Teleport's user CAs will be written. UserCAPath string `yaml:"user_ca_path,omitempty"` + + // DatabaseCAPath is the path to which Teleport's database CA will be + // written. + DatabaseCAPath string `yaml:"database_ca_path,omitempty"` } func (t *TemplateTLSCAs) CheckAndSetDefaults() error { @@ -54,6 +64,10 @@ func (t *TemplateTLSCAs) CheckAndSetDefaults() error { t.UserCAPath = defaultUserCAPath } + if t.DatabaseCAPath == "" { + t.DatabaseCAPath = defaultDatabaseCAPath + } + return nil } @@ -69,6 +83,9 @@ func (t *TemplateTLSCAs) Describe() []FileDescription { { Name: t.HostCAPath, }, + { + Name: t.DatabaseCAPath, + }, } } @@ -97,6 +114,11 @@ func (t *TemplateTLSCAs) Render(ctx context.Context, authClient auth.ClientI, cu return trace.Wrap(err) } + databaseCAs, err := authClient.GetCertAuthorities(ctx, types.DatabaseCA, false) + if err != nil { + return trace.Wrap(err) + } + dest, err := destination.GetDestination() if err != nil { return trace.Wrap(err) @@ -115,5 +137,9 @@ func (t *TemplateTLSCAs) Render(ctx context.Context, authClient auth.ClientI, cu return trace.Wrap(err) } + if err := dest.Write(t.DatabaseCAPath, concatCACerts(databaseCAs)); err != nil { + return trace.Wrap(err) + } + return nil } From 0e4846ef707463b752bd2d3d5ac2e9e1af1e67f8 Mon Sep 17 00:00:00 2001 From: Tim Buckley Date: Wed, 4 May 2022 16:35:12 -0600 Subject: [PATCH 08/10] Fix outdated var names --- tool/tbot/config/configtemplate_cockroach.go | 4 ++-- tool/tbot/config/configtemplate_mongo.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tool/tbot/config/configtemplate_cockroach.go b/tool/tbot/config/configtemplate_cockroach.go index b5cd72c79b8c1..1c008c7985e6f 100644 --- a/tool/tbot/config/configtemplate_cockroach.go +++ b/tool/tbot/config/configtemplate_cockroach.go @@ -62,7 +62,7 @@ func (t *TemplateCockroach) Render(ctx context.Context, authClient auth.ClientI, return trace.Wrap(err) } - hostCAs, err := authClient.GetCertAuthorities(ctx, types.DatabaseCA, false) + dbCAs, err := authClient.GetCertAuthorities(ctx, types.DatabaseCA, false) if err != nil { return trace.Wrap(err) } @@ -73,7 +73,7 @@ func (t *TemplateCockroach) Render(ctx context.Context, authClient auth.ClientI, dest: dest, subpath: t.DirName, }, - Key: newClientKey(currentIdentity, hostCAs), + Key: newClientKey(currentIdentity, dbCAs), Format: identityfile.FormatCockroach, // Always overwrite to avoid hitting our no-op Stat() and Remove() functions. diff --git a/tool/tbot/config/configtemplate_mongo.go b/tool/tbot/config/configtemplate_mongo.go index 2d48ee312ab99..d7cf7f0627703 100644 --- a/tool/tbot/config/configtemplate_mongo.go +++ b/tool/tbot/config/configtemplate_mongo.go @@ -64,7 +64,7 @@ func (t *TemplateMongo) Render(ctx context.Context, authClient auth.ClientI, cur return trace.Wrap(err) } - hostCAs, err := authClient.GetCertAuthorities(ctx, types.DatabaseCA, false) + dbCAs, err := authClient.GetCertAuthorities(ctx, types.DatabaseCA, false) if err != nil { return trace.Wrap(err) } @@ -74,7 +74,7 @@ func (t *TemplateMongo) Render(ctx context.Context, authClient auth.ClientI, cur Writer: &BotConfigWriter{ dest: dest, }, - Key: newClientKey(currentIdentity, hostCAs), + Key: newClientKey(currentIdentity, dbCAs), Format: identityfile.FormatMongo, // Always overwrite to avoid hitting our no-op Stat() and Remove() functions. From 2fdc22e838fe30f84f6844cdf3f63eb68c815aed Mon Sep 17 00:00:00 2001 From: Tim Buckley Date: Thu, 5 May 2022 17:10:54 -0600 Subject: [PATCH 09/10] Make sure to use a random free port for the proxy service in tbot tests --- tool/tbot/testhelpers/srv.go | 1 + 1 file changed, 1 insertion(+) diff --git a/tool/tbot/testhelpers/srv.go b/tool/tbot/testhelpers/srv.go index 642026ba3f8e4..5a360ff166c24 100644 --- a/tool/tbot/testhelpers/srv.go +++ b/tool/tbot/testhelpers/srv.go @@ -49,6 +49,7 @@ func DefaultConfig(t *testing.T) *config.FileConfig { Proxy: config.Proxy{ Service: config.Service{ EnabledFlag: "true", + ListenAddress: mustGetFreeLocalListenerAddr(t), }, WebAddr: mustGetFreeLocalListenerAddr(t), TunAddr: mustGetFreeLocalListenerAddr(t), From ac20e471066cf253a7293e37373eaf41bf402ef4 Mon Sep 17 00:00:00 2001 From: Tim Buckley Date: Thu, 5 May 2022 17:18:02 -0600 Subject: [PATCH 10/10] Resolve lint --- tool/tbot/testhelpers/srv.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tool/tbot/testhelpers/srv.go b/tool/tbot/testhelpers/srv.go index 5a360ff166c24..bd16ac1856331 100644 --- a/tool/tbot/testhelpers/srv.go +++ b/tool/tbot/testhelpers/srv.go @@ -48,7 +48,7 @@ func DefaultConfig(t *testing.T) *config.FileConfig { }, Proxy: config.Proxy{ Service: config.Service{ - EnabledFlag: "true", + EnabledFlag: "true", ListenAddress: mustGetFreeLocalListenerAddr(t), }, WebAddr: mustGetFreeLocalListenerAddr(t),