diff --git a/go.mod b/go.mod index 16ebbd81fb1a2..ba50efaf192ce 100644 --- a/go.mod +++ b/go.mod @@ -170,6 +170,9 @@ require ( github.com/jcmturner/goidentity/v6 v6.0.1 // indirect github.com/jcmturner/rpc/v2 v2.0.3 // indirect github.com/jmespath/go-jmespath v0.4.0 // indirect + github.com/joshlf/go-acl v0.0.0-20200411065538-eae00ae38531 // indirect + github.com/jstemmer/go-junit-report v0.9.1 // indirect + github.com/josharian/intern v1.0.0 // indirect github.com/klauspost/compress v1.9.5 // indirect github.com/kr/pretty v0.3.0 // indirect github.com/kr/text v0.2.0 // indirect diff --git a/go.sum b/go.sum index 468cf93640ef3..e421dc6348780 100644 --- a/go.sum +++ b/go.sum @@ -578,6 +578,8 @@ github.com/joho/godotenv v1.3.0/go.mod h1:7hK45KPybAkOC6peb+G5yklZfMxEjkZhHbwpqx github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo= github.com/jonboulle/clockwork v0.2.2 h1:UOGuzwb1PwsrDAObMuhUnj0p5ULPj8V/xJ7Kx9qUBdQ= github.com/jonboulle/clockwork v0.2.2/go.mod h1:Pkfl5aHPm1nk2H9h0bjmnJD/BcgbGXUBGnn1kMkgxc8= +github.com/joshlf/go-acl v0.0.0-20200411065538-eae00ae38531 h1:hgVxRoDDPtQE68PT4LFvNlPz2nBKd3OMlGKIQ69OmR4= +github.com/joshlf/go-acl v0.0.0-20200411065538-eae00ae38531/go.mod h1:fqTUQpVYBvhCNIsMXGl2GE9q6z94DIP6NtFKXCSTVbg= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/jpillora/backoff v1.0.0/go.mod h1:J/6gKK9jxlEcS3zixgDgUAsiuZ7yrSoa/FX5e0EB2j4= diff --git a/lib/utils/cli.go b/lib/utils/cli.go index 79362ed1e62eb..ccc9c9070a21f 100644 --- a/lib/utils/cli.go +++ b/lib/utils/cli.go @@ -251,6 +251,8 @@ func formatCertError(err error) string { } const ( + // Bold is an escape code to format as bold or increased intensity + Bold = 1 // Red is an escape code for red terminal color Red = 31 // Yellow is an escape code for yellow terminal color diff --git a/tool/tbot/botfs/botfs.go b/tool/tbot/botfs/botfs.go new file mode 100644 index 0000000000000..ceacbb49fd071 --- /dev/null +++ b/tool/tbot/botfs/botfs.go @@ -0,0 +1,156 @@ +/* +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 botfs + +import ( + "io/fs" + "os" + "os/user" + "runtime" + "strconv" + "syscall" + + "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/constants" + "github.com/gravitational/trace" + "github.com/sirupsen/logrus" +) + +var log = logrus.WithFields(logrus.Fields{ + trace.Component: teleport.ComponentTBot, +}) + +// SymlinksMode is an enum type listing various symlink behavior modes. +type SymlinksMode string + +const ( + // SymlinksInsecure does allow resolving symlink paths and does not issue + // any symlink-related warnings. + SymlinksInsecure SymlinksMode = "insecure" + + // SymlinksTrySecure attempts to write files securely and avoid symlink + // attacks, but falls back with a warning if the necessary OS / kernel + // support is missing. + SymlinksTrySecure SymlinksMode = "try-secure" + + // SymlinksSecure attempts to write files securely and fails with an error + // if the operation fails. This should be the default on systems where we + // expect it to be supported. + SymlinksSecure SymlinksMode = "secure" +) + +// ACLMode is an enum type listing various ACL behavior modes. +type ACLMode string + +const ( + // ACLOff disables ACLs + ACLOff ACLMode = "off" + + // ACLTry attempts to use ACLs but falls back to no ACLs with a warning if + // unavailable. + ACLTry ACLMode = "try" + + // ACLRequired enables ACL support and fails if ACLs are unavailable. + ACLRequired ACLMode = "required" +) + +const ( + // DefaultMode is the preferred permissions mode for bot files. + DefaultMode fs.FileMode = 0600 + + // DefaultDirMode is the preferred permissions mode for bot directories. + // Directories need the execute bit set for most operations on their + // contents to succeed. + DefaultDirMode fs.FileMode = 0700 +) + +// ACLOptions contains parameters needed to configure ACLs +type ACLOptions struct { + // BotUser is the bot user that should have write access to this entry + BotUser *user.User + + // ReaderUser is the user that should have read access to the file. This + // may be nil if the reader user is not known. + ReaderUser *user.User +} + +// openStandard attempts to open the given path for writing with O_CREATE set. +func openStandard(path string) (*os.File, error) { + file, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, DefaultMode) + if err != nil { + return nil, trace.ConvertSystemError(err) + } + + return file, nil +} + +// createStandard creates an empty file or directory at the given path without +// attempting to prevent symlink attacks. +func createStandard(path string, isDir bool) error { + if isDir { + if err := os.Mkdir(path, DefaultDirMode); err != nil { + return trace.ConvertSystemError(err) + } + + return nil + } + + f, err := openStandard(path) + if err != nil { + return trace.Wrap(err) + } + + if err := f.Close(); err != nil { + log.Warnf("Failed to close file at %q: %+v", path, err) + } + + return nil +} + +// GetOwner attempts to retrieve the owner of the given file. This is not +// supported on all platforms and will return a trace.NotImplemented in that +// case. +func GetOwner(fileInfo fs.FileInfo) (*user.User, error) { + info, ok := fileInfo.Sys().(*syscall.Stat_t) + if !ok { + return nil, trace.NotImplemented("Cannot verify file ownership on this platform.") + } + + user, err := user.LookupId(strconv.Itoa(int(info.Uid))) + if err != nil { + return nil, trace.Wrap(err) + } + + return user, nil +} + +// IsOwnedBy checks that the file at the given path is owned by the given user. +// Returns a trace.NotImplemented() on unsupported platforms. +func IsOwnedBy(fileInfo fs.FileInfo, user *user.User) (bool, error) { + if runtime.GOOS == constants.WindowsOS { + // no-op on windows + return false, trace.NotImplemented("Cannot verify file ownership on this platform.") + } + + info, ok := fileInfo.Sys().(*syscall.Stat_t) + if !ok { + return false, trace.NotImplemented("Cannot verify file ownership on this platform.") + } + + // Our files are 0600, so don't bother checking gid. + return strconv.Itoa(int(info.Uid)) == user.Uid, nil +} diff --git a/tool/tbot/botfs/fs_linux.go b/tool/tbot/botfs/fs_linux.go new file mode 100644 index 0000000000000..835fc6ab0ca95 --- /dev/null +++ b/tool/tbot/botfs/fs_linux.go @@ -0,0 +1,450 @@ +//go:build linux +// +build linux + +/* +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 botfs + +import ( + "io" + "io/fs" + "os" + "os/user" + "path/filepath" + "sync" + + "github.com/coreos/go-semver/semver" + "github.com/gravitational/teleport/lib/utils" + "github.com/gravitational/trace" + "github.com/joshlf/go-acl" + "golang.org/x/sys/unix" +) + +// Openat2MinKernel is the kernel release that adds support for the openat2() +// syscall. +const Openat2MinKernel = "5.6.0" + +// modeACLReadExecute is the permissions mode needed for read on directories. +const modeACLReadExecute fs.FileMode = 05 + +// modeACLReadWrite is the lower 3 bytes of a UNIX file mode for permission +// bits, i.e. just one r/w/x. +const modeACLReadWrite fs.FileMode = 06 + +// modeACLReadWriteExecute is the permissions mode needed for full rwx on +// directories. +const modeACLReadWriteExecute fs.FileMode = 07 + +// modeACLNone is the UNIX file mode for no permissions, used for group and +// world read/write. +const modeACLNone fs.FileMode = 0 + +// missingSyscallWarning is used to reduce log spam when a syscall is missing. +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) { + 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: unix.O_RDWR | unix.O_CREAT, + Resolve: unix.RESOLVE_NO_SYMLINKS, + } + + fd, err := unix.Openat2(unix.AT_FDCWD, path, &how) + if err != nil { + // note: returning the original error here for comparison purposes + return nil, err + } + + // os.File.Close() appears to close wrapped files sanely, so rely on that + // rather than relying on callers to use unix.Close(fd) + return os.NewFile(uintptr(fd), filepath.Base(path)), nil +} + +// openSymlinks mode opens the file for read/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) { + var file *os.File + var err error + + switch symlinksMode { + case SymlinksSecure: + file, err = openSecure(path) + if err == unix.ENOSYS { + return nil, trace.Errorf("openSecure(%q) failed due to missing "+ + "syscall; `symlinks: insecure` may be required for this "+ + "system", path) + } else if err != nil { + return nil, trace.Wrap(err) + } + case SymlinksTrySecure: + file, err = openSecure(path) + 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) + if err != nil { + return nil, trace.Wrap(err) + } + } else if err != nil { + return nil, trace.Wrap(err) + } + case SymlinksInsecure: + file, err = openStandard(path) + if err != nil { + return nil, trace.Wrap(err) + } + default: + return nil, trace.BadParameter("invalid symlinks mode %q", symlinksMode) + } + + return file, nil +} + +// createStandard creates an empty file or directory at the given path while +// attempting to prevent symlink attacks. +func createSecure(path string, isDir bool) error { + if isDir { + // We can't specify RESOLVE_NO_SYMLINKS for mkdir. This isn't the end + // of the world, though: if an attacker attempts a symlink attack we'll + // just open the correct file for read/write later (and error when it + // doesn't exist). + if err := os.Mkdir(path, DefaultDirMode); err != nil { + return trace.Wrap(err) + } + + return nil + } + + f, err := openSecure(path) + if err == unix.ENOSYS { + // bubble up the original error for comparison + return err + } else if err != nil { + return trace.Wrap(err) + } + + // No writing to do, just close it. + if err := f.Close(); err != nil { + log.Warnf("Failed to close file at %q: %+v", path, err) + } + + return nil +} + +// Create attempts to create the given file or directory with the given +// symlinks mode. +func Create(path string, isDir bool, symlinksMode SymlinksMode) error { + // Implementation note: paranoid file _creation_ is only really useful for + // providing an early warning if openat2() / ACLs are unsupported on the + // host system, as it will catch compatibility issues during `tbot init`. + // Read() and Write() with Symlinks(Try)Secure are the codepaths that + // actually prevents symlink attacks. + + switch symlinksMode { + case SymlinksSecure: + if err := createSecure(path, isDir); err != nil { + if err == unix.ENOSYS { + return trace.Errorf("createSecure(%q) failed due to missing "+ + "syscall; `symlinks: insecure` may be required for this "+ + "system", path) + } + + return trace.Wrap(err) + } + case SymlinksTrySecure: + err := createSecure(path, isDir) + if err == nil { + // All good, move on. + return nil + } + + if err != unix.ENOSYS { + // Something else went wrong, fail. + return trace.Wrap(err) + } + + // It's a bit gross to stuff this sync.Once into a global, but + // hopefully that's forgivable since it just manages a log message. + missingSyscallWarning.Do(func() { + log.Warnf("Failed to create %q securely due to missing syscall; "+ + "falling back to regular file creation. Set `symlinks: "+ + "insecure` on this destination to disable this warning.", path) + }) + + return trace.Wrap(createStandard(path, isDir)) + case SymlinksInsecure: + return trace.Wrap(createStandard(path, isDir)) + default: + return trace.BadParameter("invalid symlinks mode %q", symlinksMode) + } + + return nil +} + +// Read reads the contents of the given file into memory. +func Read(path string, symlinksMode SymlinksMode) ([]byte, error) { + file, err := openSymlinksMode(path, symlinksMode) + if err != nil { + return nil, trace.Wrap(err) + } + + defer file.Close() + + data, err := io.ReadAll(file) + if err != nil { + return nil, trace.Wrap(err) + } + + return data, nil +} + +// 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) + if err != nil { + return trace.Wrap(err) + } + + defer file.Close() + + if _, err := file.Write(data); err != nil { + return trace.Wrap(err) + } + + return nil +} + +// desiredPerms determines the desired bot permissions for an artifact at +// the given path. Directories require read/exec, files require read/write. +func desiredPerms(path string) (ownerMode fs.FileMode, botAndReaderMode fs.FileMode, err error) { + stat, err := os.Stat(path) + if err != nil { + return 0, 0, trace.Wrap(err) + } + + botAndReaderMode = modeACLReadWrite + ownerMode = modeACLReadWrite + if stat.IsDir() { + botAndReaderMode = modeACLReadExecute + ownerMode = modeACLReadWriteExecute + } + + return +} + +// VerifyACL verifies whether the ACL of the given file allows writes from the +// bot user. Errors may optionally be used as more informational warnings; +// ConfigureACL can be used to correct them, assuming the user has permissions. +func VerifyACL(path string, opts *ACLOptions) error { + current, err := acl.Get(path) + if err != nil { + return trace.Wrap(err) + } + + stat, err := os.Stat(path) + if err != nil { + return trace.Wrap(err) + } + + owner, err := GetOwner(stat) + if err != nil { + return trace.Wrap(err) + } + + ownerMode, botAndReaderMode, err := desiredPerms(path) + if err != nil { + return trace.Wrap(err) + } + + // Attempt to determine the max # of expected user tags. We can't know the + // reader user in all cases because we only know it during `tbot init`, so + // instead we'll try to determine the upper bound of expected entries here. + maxExpectedUserTags := 2 + if owner.Uid == opts.BotUser.Uid { + // This path is owned by the bot user, so at least one user tag will + // be missing. + maxExpectedUserTags-- + } + + // Also determine the minimum number of expected user tags. There should + // generally be at least 1 unless all users are the same. + minExpectedUserTags := 0 + if owner.Uid != opts.BotUser.Uid { + minExpectedUserTags++ + } + if opts.ReaderUser != nil && owner.Uid != opts.ReaderUser.Uid { + minExpectedUserTags++ + } + + userTagCount := 0 + errors := []error{} + + for _, entry := range current { + switch entry.Tag { + case acl.TagUserObj: + if entry.Perms != ownerMode { + errors = append(errors, trace.BadParameter("user entry has improper permissions %d", entry.Perms)) + } + case acl.TagGroupObj: + if entry.Perms != modeACLNone { + errors = append(errors, trace.BadParameter("group entry has improper permissions %d", entry.Perms)) + } + case acl.TagOther: + if entry.Perms != modeACLNone { + errors = append(errors, trace.BadParameter("other entry has improper permissions %d", entry.Perms)) + } + case acl.TagMask: + if entry.Perms != botAndReaderMode { + errors = append(errors, trace.BadParameter("mask entry has improper permissions %d", entry.Perms)) + } + case acl.TagGroup: + // Group tags currently not allowed. + errors = append(errors, trace.BadParameter("unexpected group entry found")) + case acl.TagUser: + userTagCount++ + + // It's only worth checking the qualifiers if we know all expected + // values in advance. We can't know them at bot runtime in any way + // that isn't also subject to e.g. an attacker with root / owner + // access, so we might as well not spam warnings every time we + // verify the ACLs. We'll have to rely on the tag counter instead. + if opts.BotUser != nil && + opts.ReaderUser != nil && + entry.Qualifier != opts.BotUser.Uid && + entry.Qualifier != opts.ReaderUser.Uid { + errors = append(errors, trace.BadParameter("invalid qualifier %q for user entry", entry.Qualifier)) + } + + if entry.Perms != botAndReaderMode { + errors = append(errors, trace.BadParameter("invalid permissions %q for bot user entry", entry.Perms)) + } + } + } + + if userTagCount > maxExpectedUserTags { + errors = append(errors, trace.BadParameter("too many user tags found")) + } else if userTagCount < minExpectedUserTags { + errors = append(errors, trace.BadParameter("too few user tags found")) + } + + return trace.NewAggregate(errors...) +} + +// ConfigureACL configures ACLs of the given file to allow writes from the bot +// user. +func ConfigureACL(path string, owner *user.User, opts *ACLOptions) error { + if owner.Uid == opts.BotUser.Uid && owner.Uid == opts.ReaderUser.Uid { + // We'll end up with an empty ACL. This isn't technically a problem + log.Warnf("The owner, bot, and reader all appear to be the same "+ + "user (%+v). This is an unusual configuration: consider setting "+ + "`acls: off` in the destination config to remove this warning.", + owner.Username) + } + + // We fully specify the ACL here to ensure the correct permissions are + // always set (rather than just appending an "allow" for the bot user). + // Note: These need to be sorted by tag value. + ownerMode, botAndReaderMode, err := desiredPerms(path) + if err != nil { + return trace.Wrap(err) + } + + // Note: ACL entries need to be ordered per acl_linux entryPriority + desiredACL := acl.ACL{ + { + // Note: Mask does not apply to user object. + Tag: acl.TagUserObj, + Perms: ownerMode, + }, + } + + // Only add an entry for the bot user if it isn't the owner. + if owner.Uid != opts.BotUser.Uid { + desiredACL = append(desiredACL, acl.Entry{ + // Entry to allow the bot to read/write. + Tag: acl.TagUser, + Qualifier: opts.BotUser.Uid, + Perms: botAndReaderMode, + }) + } + + // Only add entry for the reader if it isn't the owner. + if owner.Uid != opts.ReaderUser.Uid { + desiredACL = append(desiredACL, acl.Entry{ + // Entry to allow the reader user to read/write. + Tag: acl.TagUser, + Qualifier: opts.ReaderUser.Uid, + Perms: botAndReaderMode, + }) + } + + desiredACL = append(desiredACL, + acl.Entry{ + Tag: acl.TagGroupObj, + Perms: modeACLNone, + }, + acl.Entry{ + // Mask is the maximum permissions the ACL can grant. This should + // match the desired bot permissions. + Tag: acl.TagMask, + Perms: botAndReaderMode, + }, + acl.Entry{ + Tag: acl.TagOther, + Perms: modeACLNone, + }, + ) + + // Note: we currently give both the bot and reader read/write to all the + // files. This is done for simplicity and shouldn't represent a security + // risk - the bot obviously knows the contents of the destination, and + // the files being writable to the reader is (maybe arguably) not a + // security issue. + + log.Debugf("Configuring ACL on path %q: %v", path, desiredACL) + return trace.ConvertSystemError(trace.Wrap(acl.Set(path, desiredACL))) +} + +// HasACLSupport determines if this binary / system supports ACLs. +func HasACLSupport() (bool, error) { + // We just assume Linux _can_ support ACLs here, and will test for support + // at runtime. + return true, nil +} + +// HasSecureWriteSupport determines if `CreateSecure()` should be supported +// on this OS / kernel version. Note that it just checks the kernel version, +// so this should be treated as a fallible hint. +func HasSecureWriteSupport() (bool, error) { + minKernel := semver.New(Openat2MinKernel) + version, err := utils.KernelVersion() + if err != nil { + return false, trace.Wrap(err) + } + if version.LessThan(*minKernel) { + return false, nil + } + + return true, nil +} diff --git a/tool/tbot/botfs/fs_other.go b/tool/tbot/botfs/fs_other.go new file mode 100644 index 0000000000000..46ce3a73a815d --- /dev/null +++ b/tool/tbot/botfs/fs_other.go @@ -0,0 +1,111 @@ +//go:build !linux +// +build !linux + +/* +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 botfs + +import ( + "io" + "os/user" + + "github.com/gravitational/trace" +) + +// Create attempts to create the given file or directory without +// evaluating symlinks. This is only supported on recent Linux kernel versions +// (5.6+). The resulting file permissions are unspecified; Chmod should be +// called afterward. +func Create(path string, isDir bool, symlinksMode SymlinksMode) error { + if symlinksMode == SymlinksSecure { + return trace.BadParameter("cannot write with `symlinks: secure` on unsupported platform") + } + + return trace.Wrap(createStandard(path, isDir)) +} + +// Read reads the contents of the given file into memory. +func Read(path string, symlinksMode SymlinksMode) ([]byte, error) { + switch symlinksMode { + case SymlinksSecure: + return nil, trace.BadParameter("cannot read with `symlinks: secure` on unsupported platform") + case SymlinksTrySecure: + log.Warn("Secure symlinks not supported on this platform, set `symlinks: insecure` to disable this message", path) + } + + file, err := openStandard(path) + if err != nil { + return nil, trace.Wrap(err) + } + + defer file.Close() + + data, err := io.ReadAll(file) + if err != nil { + return nil, trace.Wrap(err) + } + + return data, nil +} + +// Write stores the given data to the file at the given path. +func Write(path string, data []byte, symlinksMode SymlinksMode) error { + switch symlinksMode { + case SymlinksSecure: + return trace.BadParameter("cannot write with `symlinks: secure` on unsupported platform") + case SymlinksTrySecure: + log.Warn("Secure symlinks not supported on this platform, set `symlinks: insecure` to disable this message", path) + } + + file, err := openStandard(path) + if err != nil { + return trace.Wrap(err) + } + + defer file.Close() + + if _, err := file.Write(data); err != nil { + return trace.Wrap(err) + } + + return nil +} + +// VerifyACL verifies whether the ACL of the given file allows writes from the +// bot user. +func VerifyACL(path string, opts *ACLOptions) error { + return trace.NotImplemented("ACLs not supported on this platform") +} + +// ConfigureACL configures ACLs of the given file to allow writes from the bot +// user. +func ConfigureACL(path string, owner *user.User, opts *ACLOptions) error { + return trace.NotImplemented("ACLs not supported on this platform") +} + +// HasACLSupport determines if this binary / system supports ACLs. This +// catch-all implementation just returns false. +func HasACLSupport() (bool, error) { + return false, nil +} + +// HasSecureWriteSupport determines if `CreateSecure()` should be supported +// on this OS / kernel version. This is only supported on Linux, so this +// catch-all implementation just returns false. +func HasSecureWriteSupport() (bool, error) { + return false, nil +} diff --git a/tool/tbot/config/config.go b/tool/tbot/config/config.go index b52a5bc21a2d1..fea9f0166aa52 100644 --- a/tool/tbot/config/config.go +++ b/tool/tbot/config/config.go @@ -34,6 +34,7 @@ import ( const ( DefaultCertificateTTL = 60 * time.Minute DefaultRenewInterval = 20 * time.Minute + DefaultJoinMethod = "token" ) var log = logrus.WithFields(logrus.Fields{ @@ -71,6 +72,26 @@ type CLIConf struct { // JoinMethod is the method the bot should use to exchange a token for the // initial certificate JoinMethod string + + // InitDir specifies which destination to initialize if multiple are + // configured. + InitDir string + + // BotUser is a Unix username that should be given permission to write + BotUser string + + // ReaderUser is the Unix username that will be reading the files + ReaderUser string + + // Owner is the user:group that will own the destination files. Due to SSH + // restrictions on key permissions, it cannot be the same as the reader + // user. If ACL support is unused or unavailable, the reader user will own + // files directly. + Owner string + + // Clean is a flag that, if set, instructs `tbot init` to remove existing + // unexpected files. + Clean bool } // OnboardingConfig contains values only required on first connect. @@ -93,7 +114,7 @@ type OnboardingConfig struct { // BotConfig is the bot's root config object. type BotConfig struct { Onboarding *OnboardingConfig `yaml:"onboarding,omitempty"` - Storage StorageConfig `yaml:"storage,omitempty"` + Storage *StorageConfig `yaml:"storage,omitempty"` Destinations []*DestinationConfig `yaml:"destinations,omitempty"` Debug bool `yaml:"debug"` @@ -103,6 +124,14 @@ type BotConfig struct { } func (conf *BotConfig) CheckAndSetDefaults() error { + if conf.AuthServer == "" { + return trace.BadParameter("an auth server address must be configured") + } + + if conf.Storage == nil { + conf.Storage = &StorageConfig{} + } + if err := conf.Storage.CheckAndSetDefaults(); err != nil { return trace.Wrap(err) } @@ -113,10 +142,6 @@ func (conf *BotConfig) CheckAndSetDefaults() error { } } - if conf.AuthServer == "" { - return trace.BadParameter("an auth server address must be configured") - } - if conf.CertificateTTL == 0 { conf.CertificateTTL = DefaultCertificateTTL } @@ -128,6 +153,32 @@ func (conf *BotConfig) CheckAndSetDefaults() error { return nil } +// GetDestinationByPath attempts to fetch a destination by its filesystem path. +// Only valid for filesystem destinations; returns nil if no matching +// destination exists. +func (conf *BotConfig) GetDestinationByPath(path string) (*DestinationConfig, error) { + for _, dest := range conf.Destinations { + destImpl, err := dest.GetDestination() + if err != nil { + return nil, trace.Wrap(err) + } + + destDir, ok := destImpl.(*DestinationDirectory) + if !ok { + continue + } + + // Note: this compares only paths as written in the config file. We + // might want to compare .Abs() if that proves to be confusing (though + // this may have its own problems) + if destDir.Path == path { + return dest, nil + } + } + + return nil, nil +} + // NewDefaultConfig creates a new minimal bot configuration from defaults. // CheckAndSetDefaults() will be called. func NewDefaultConfig(authServer string) (*BotConfig, error) { @@ -186,11 +237,13 @@ func FromCLIConf(cf *CLIConf) (*BotConfig, error) { // DataDir overrides any previously-configured storage config if cf.DataDir != "" { - if _, err := config.Storage.GetDestination(); err != nil { - log.Warnf("CLI parameters are overriding storage location from %s", cf.ConfigPath) + if config.Storage != nil { + if _, err := config.Storage.GetDestination(); err != nil { + log.Warnf("CLI parameters are overriding storage location from %s", cf.ConfigPath) + } } - config.Storage = StorageConfig{ + config.Storage = &StorageConfig{ DestinationMixin: DestinationMixin{ Directory: &DestinationDirectory{ Path: cf.DataDir, @@ -222,7 +275,7 @@ func FromCLIConf(cf *CLIConf) (*BotConfig, error) { // merging) if cf.Token != "" || len(cf.CAPins) > 0 || cf.JoinMethod != "" { onboarding := config.Onboarding - if onboarding != nil && (onboarding.Token != "" || onboarding.CAPath != "" || len(onboarding.CAPins) > 0) || cf.JoinMethod != "" { + if onboarding != nil && (onboarding.Token != "" || onboarding.CAPath != "" || len(onboarding.CAPins) > 0) || cf.JoinMethod != DefaultJoinMethod { // To be safe, warn about possible confusion. log.Warnf("CLI parameters are overriding onboarding config from %s", cf.ConfigPath) } diff --git a/tool/tbot/config/config_destination.go b/tool/tbot/config/config_destination.go index fbe4e89fde87a..8e986a824a8a1 100644 --- a/tool/tbot/config/config_destination.go +++ b/tool/tbot/config/config_destination.go @@ -17,23 +17,17 @@ limitations under the License. package config import ( + "github.com/gravitational/teleport/tool/tbot/identity" "github.com/gravitational/trace" ) -type Kind string - -const ( - KindSSH Kind = "ssh" - KindTLS Kind = "tls" -) - // DestinationConfig configures a user certificate destination. type DestinationConfig struct { DestinationMixin `yaml:",inline"` - Roles []string `yaml:"roles,omitempty"` - Kinds []Kind `yaml:"kinds,omitempty"` - Configs []TemplateConfig `yaml:"configs,omitempty"` + Roles []string `yaml:"roles,omitempty"` + Kinds []identity.ArtifactKind `yaml:"kinds,omitempty"` + Configs []TemplateConfig `yaml:"configs,omitempty"` } // destinationDefaults applies defaults for an output sink's destination. Since @@ -52,7 +46,7 @@ func (dc *DestinationConfig) CheckAndSetDefaults() error { // time if len(dc.Kinds) == 0 && len(dc.Configs) == 0 { - dc.Kinds = []Kind{KindSSH} + dc.Kinds = []identity.ArtifactKind{identity.KindSSH} dc.Configs = []TemplateConfig{{ SSHClient: &TemplateSSHClient{}, }} @@ -68,7 +62,7 @@ func (dc *DestinationConfig) CheckAndSetDefaults() error { } // ContainsKind determines if this destination contains the given ConfigKind. -func (dc *DestinationConfig) ContainsKind(kind Kind) bool { +func (dc *DestinationConfig) ContainsKind(kind identity.ArtifactKind) bool { for _, k := range dc.Kinds { if k == kind { return true diff --git a/tool/tbot/config/config_test.go b/tool/tbot/config/config_test.go index c2147e5bba8e1..c7184dca96eca 100644 --- a/tool/tbot/config/config_test.go +++ b/tool/tbot/config/config_test.go @@ -21,6 +21,7 @@ import ( "testing" "time" + "github.com/gravitational/teleport/tool/tbot/identity" "github.com/stretchr/testify/require" ) @@ -73,7 +74,7 @@ func TestConfigCLIOnlySample(t *testing.T) { // A single default destination should exist require.Len(t, cfg.Destinations, 1) dest := cfg.Destinations[0] - require.ElementsMatch(t, []Kind{KindSSH}, dest.Kinds) + require.ElementsMatch(t, []identity.ArtifactKind{identity.KindSSH}, dest.Kinds) require.Len(t, dest.Configs, 1) template := dest.Configs[0] @@ -107,7 +108,7 @@ func TestConfigFile(t *testing.T) { require.Len(t, cfg.Destinations, 1) destination := cfg.Destinations[0] - require.ElementsMatch(t, []Kind{KindSSH, KindTLS}, destination.Kinds) + require.ElementsMatch(t, []identity.ArtifactKind{identity.KindSSH, identity.KindTLS}, destination.Kinds) require.Len(t, destination.Configs, 1) template := destination.Configs[0] diff --git a/tool/tbot/config/configtemplate.go b/tool/tbot/config/configtemplate.go index b281a6344eaef..f54e86a6ee165 100644 --- a/tool/tbot/config/configtemplate.go +++ b/tool/tbot/config/configtemplate.go @@ -21,7 +21,6 @@ import ( "strings" "github.com/gravitational/teleport/lib/auth" - "github.com/gravitational/teleport/tool/tbot/destination" "github.com/gravitational/teleport/tool/tbot/identity" "github.com/gravitational/trace" "gopkg.in/yaml.v3" @@ -42,10 +41,6 @@ type FileDescription struct { // IsDir designates whether this describes a subdirectory inside the // destination. IsDir bool - - // ModeHint describes the intended permissions for this data, for - // Destination backends where permissions are relevant. - ModeHint destination.ModeHint } // Template defines functions for dynamically writing additional files to diff --git a/tool/tbot/config/configtemplate_ssh.go b/tool/tbot/config/configtemplate_ssh.go index 2055505e3a217..c464c5656de90 100644 --- a/tool/tbot/config/configtemplate_ssh.go +++ b/tool/tbot/config/configtemplate_ssh.go @@ -29,7 +29,6 @@ import ( "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/utils" - "github.com/gravitational/teleport/tool/tbot/destination" "github.com/gravitational/teleport/tool/tbot/identity" "github.com/gravitational/trace" "golang.org/x/crypto/ssh" @@ -51,18 +50,16 @@ func (c *TemplateSSHClient) CheckAndSetDefaults() error { func (c *TemplateSSHClient) Describe() []FileDescription { return []FileDescription{ { - Name: "ssh_config", - ModeHint: destination.ModeHintSecret, + Name: "ssh_config", }, { - Name: "known_hosts", - ModeHint: destination.ModeHintSecret, + Name: "known_hosts", }, } } func (c *TemplateSSHClient) Render(ctx context.Context, authClient auth.ClientI, currentIdentity *identity.Identity, destination *DestinationConfig) error { - if !destination.ContainsKind(KindSSH) { + if !destination.ContainsKind(identity.KindSSH) { return trace.BadParameter("%s config template requires kind `ssh` to be enabled", TemplateSSHClientName) } diff --git a/tool/tbot/config/destination.go b/tool/tbot/config/destination.go index c72a8afb7aace..6383ba82cc7a9 100644 --- a/tool/tbot/config/destination.go +++ b/tool/tbot/config/destination.go @@ -31,12 +31,12 @@ type DestinationMixin struct { type DestinationDefaults = func(*DestinationMixin) error -func (dm *DestinationMixin) CheckAndSetDefaults(applyDefaults DestinationDefaults) error { +// checkAndSetDefaultsInner performs member initialization that won't recurse +func (dm *DestinationMixin) checkAndSetDefaultsInner() (int, error) { notNilCount := 0 - if dm.Directory != nil { if err := dm.Directory.CheckAndSetDefaults(); err != nil { - return trace.Wrap(err) + return 0, trace.Wrap(err) } notNilCount++ @@ -44,17 +44,35 @@ func (dm *DestinationMixin) CheckAndSetDefaults(applyDefaults DestinationDefault if dm.Memory != nil { if err := dm.Memory.CheckAndSetDefaults(); err != nil { - return trace.Wrap(err) + return 0, trace.Wrap(err) } notNilCount++ } + return notNilCount, nil +} + +func (dm *DestinationMixin) CheckAndSetDefaults(applyDefaults DestinationDefaults) error { + notNilCount, err := dm.checkAndSetDefaultsInner() + if err != nil { + return trace.Wrap(err) + } if notNilCount == 0 { // use defaults if err := applyDefaults(dm); err != nil { return trace.Wrap(err) } + + // CheckAndSetDefaults() again + notNilCount, err := dm.checkAndSetDefaultsInner() + if err != nil { + return trace.Wrap(err) + } + + if notNilCount == 0 { + return trace.BadParameter("a destination is required") + } } else if notNilCount > 1 { return trace.BadParameter("only one destination backend may be specified at a time") } diff --git a/tool/tbot/config/destination_directory.go b/tool/tbot/config/destination_directory.go index de09b82bcc8b0..b3b39cd7da38c 100644 --- a/tool/tbot/config/destination_directory.go +++ b/tool/tbot/config/destination_directory.go @@ -19,16 +19,19 @@ package config import ( "fmt" "os" + "os/user" "path/filepath" - "github.com/gravitational/teleport/tool/tbot/destination" + "github.com/gravitational/teleport/tool/tbot/botfs" "github.com/gravitational/trace" "gopkg.in/yaml.v3" ) // DestinationDirectory is a Destination that writes to the local filesystem type DestinationDirectory struct { - Path string `yaml:"path,omitempty"` + Path string `yaml:"path,omitempty"` + Symlinks botfs.SymlinksMode `yaml:"symlinks,omitempty"` + ACLs botfs.ACLMode `yaml:"acls,omitempty"` } func (dd *DestinationDirectory) UnmarshalYAML(node *yaml.Node) error { @@ -58,23 +61,142 @@ func (dd *DestinationDirectory) CheckAndSetDefaults() error { return trace.BadParameter("destination path must not be empty") } + secureSupported, err := botfs.HasSecureWriteSupport() + if err != nil { + return trace.Wrap(err) + } + + aclsSupported, err := botfs.HasACLSupport() + if err != nil { + return trace.Wrap(err) + } + + switch dd.Symlinks { + case "": + if secureSupported { + // We expect Openat2 to be available, so try to use it by default. + dd.Symlinks = botfs.SymlinksSecure + } else { + // TrySecure will print a warning on fallback. + dd.Symlinks = botfs.SymlinksTrySecure + } + case botfs.SymlinksInsecure, botfs.SymlinksTrySecure: + // valid + case botfs.SymlinksSecure: + if !secureSupported { + return trace.BadParameter("symlink mode %q not supported on this system", dd.Symlinks) + } + default: + return trace.BadParameter("invalid symlinks mode: %q", dd.Symlinks) + } + + switch dd.ACLs { + case "": + if aclsSupported { + // Unlike openat2(), we can't ever depend on ACLs being available. + // We'll only ever try to use them, end users can opt-in to a hard + // ACL check if they wish. + dd.ACLs = botfs.ACLTry + } else { + // if aclsSupported == false here, we know it will never work, so + // don't bother trying. + dd.ACLs = botfs.ACLOff + } + case botfs.ACLOff, botfs.ACLTry: + // valid + case botfs.ACLRequired: + if !aclsSupported { + return trace.BadParameter("acls mode %q not supported on this system", dd.ACLs) + } + default: + return trace.BadParameter("invalid acls mode: %q", dd.ACLs) + } + return nil } -func (dd *DestinationDirectory) Write(name string, data []byte, modeHint destination.ModeHint) error { - // TODO: honor modeHint? - if err := os.WriteFile(filepath.Join(dd.Path, name), data, 0600); err != nil { +func (dd *DestinationDirectory) Init() error { + // Create the directory if needed. + stat, err := os.Stat(dd.Path) + if trace.IsNotFound(err) { + if err := os.MkdirAll(dd.Path, botfs.DefaultDirMode); err != nil { + return trace.Wrap(err) + } + log.Infof("Created directory %q", dd.Path) + } 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 nil } +func (dd *DestinationDirectory) Verify(keys []string) error { + aclsSupported, err := botfs.HasACLSupport() + if err != nil { + return trace.Wrap(err) + } + + currentUser, err := user.Current() + if err != nil { + return trace.Wrap(err) + } + + stat, err := os.Stat(dd.Path) + if err != nil { + return trace.Wrap(err) + } + + ownedByBot, err := botfs.IsOwnedBy(stat, currentUser) + if trace.IsNotImplemented(err) { + // If file owners aren't supported, ACLs certainly aren't. Just bail. + // (Subject to change if we ever try to support Windows ACLs.) + return nil + } else if err != nil { + return trace.Wrap(err) + } + + // Make sure it's worth warning about ACLs for this destination. If ACLs + // are disabled, unsupported, or the destination is owned by the bot + // (implying the user is not trying to use ACLs), just bail. + if dd.ACLs == botfs.ACLOff || !aclsSupported || ownedByBot { + return nil + } + + errors := []error{} + for _, key := range keys { + path := filepath.Join(dd.Path, key) + + errors = append(errors, botfs.VerifyACL(path, &botfs.ACLOptions{ + BotUser: currentUser, + })) + } + + aggregate := trace.NewAggregate(errors...) + if dd.ACLs == botfs.ACLRequired { + // Hard fail if ACLs are specifically requested and there are errors. + return aggregate + } + + if aggregate != nil { + log.Warnf("Destination %q has unexpected ACLs: %v", dd.Path, aggregate) + } + + return nil +} + +func (dd *DestinationDirectory) Write(name string, data []byte) error { + return trace.Wrap(botfs.Write(filepath.Join(dd.Path, name), data, dd.Symlinks)) +} + func (dd *DestinationDirectory) Read(name string) ([]byte, error) { - b, err := os.ReadFile(filepath.Join(dd.Path, name)) + data, err := botfs.Read(filepath.Join(dd.Path, name), dd.Symlinks) if err != nil { return nil, trace.Wrap(err) } - return b, nil + + return data, nil } func (dd *DestinationDirectory) String() string { diff --git a/tool/tbot/config/destination_memory.go b/tool/tbot/config/destination_memory.go index 1cacbb61a9fe1..2778bc838a52e 100644 --- a/tool/tbot/config/destination_memory.go +++ b/tool/tbot/config/destination_memory.go @@ -17,7 +17,6 @@ limitations under the License. package config import ( - "github.com/gravitational/teleport/tool/tbot/destination" "github.com/gravitational/trace" "gopkg.in/yaml.v3" ) @@ -51,7 +50,17 @@ func (dm *DestinationMemory) CheckAndSetDefaults() error { return nil } -func (dm *DestinationMemory) Write(name string, data []byte, _ destination.ModeHint) error { +func (dm *DestinationMemory) Init() error { + // Nothing to do. + return nil +} + +func (dm *DestinationMemory) Verify(keys []string) error { + // Nothing to do. + return nil +} + +func (dm *DestinationMemory) Write(name string, data []byte) error { dm.store[name] = data return nil diff --git a/tool/tbot/destination/destination.go b/tool/tbot/destination/destination.go index 093ce60bf71b6..3ad0ca5358b3d 100644 --- a/tool/tbot/destination/destination.go +++ b/tool/tbot/destination/destination.go @@ -16,23 +16,20 @@ limitations under the License. package destination -// ModeHint is a backend-agnostic file mode hint. -type ModeHint int64 - -const ( - // ModeHintUnspecified hints that files should be created with default - // (possibly insecure) permissions. - ModeHintUnspecified ModeHint = iota - - // ModeHintSecret hints that files should be created with restricted - // permissions, appropriate for secret data. - ModeHintSecret -) - // Destination can persist renewable certificates. 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 + + // Verify is run before renewals to check for any potential problems with + // the destination. These errors may be informational (logged warnings) or + // return an error that may potentially terminate the process. + Verify(keys []string) error + // Write stores data to the destination with the given name. - Write(name string, data []byte, modeHint ModeHint) error + Write(name string, data []byte) error // Read fetches data from the destination with a given name. Read(name string) ([]byte, error) diff --git a/tool/tbot/identity/artifact.go b/tool/tbot/identity/artifact.go new file mode 100644 index 0000000000000..840c491500a13 --- /dev/null +++ b/tool/tbot/identity/artifact.go @@ -0,0 +1,144 @@ +/* +Copyright 2021-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 identity + +import ( + "bytes" + + "github.com/gravitational/teleport/api/client/proto" +) + +// Artifact is a component of a serialized identity. +type Artifact struct { + Key string + Kind ArtifactKind + ToBytes func(*Identity) []byte + FromBytes func(*proto.Certs, *LoadIdentityParams, []byte) +} + +// Matches returns true if this artifact's Kind matches any one of the given +// kinds or if it's kind is KindAlways +func (a *Artifact) Matches(kinds ...ArtifactKind) bool { + if a.Kind == KindAlways { + return true + } + + for _, kind := range kinds { + if a.Kind == kind { + return true + } + } + + return false +} + +var artifacts = []Artifact{ + // SSH artifacts + { + Key: SSHCertKey, + Kind: KindSSH, + ToBytes: func(i *Identity) []byte { + return i.CertBytes + }, + FromBytes: func(c *proto.Certs, p *LoadIdentityParams, b []byte) { + c.SSH = b + }, + }, + { + Key: SSHCACertsKey, + Kind: KindSSH, + ToBytes: func(i *Identity) []byte { + return bytes.Join(i.SSHCACertBytes, []byte("$")) + }, + FromBytes: func(c *proto.Certs, p *LoadIdentityParams, b []byte) { + c.SSHCACerts = bytes.Split(b, []byte("$")) + }, + }, + + // TLS artifacts + { + Key: TLSCertKey, + Kind: KindTLS, + ToBytes: func(i *Identity) []byte { + return i.TLSCertBytes + }, + FromBytes: func(c *proto.Certs, p *LoadIdentityParams, b []byte) { + c.TLS = b + }, + }, + { + Key: TLSCACertsKey, + Kind: KindTLS, + ToBytes: func(i *Identity) []byte { + return bytes.Join(i.TLSCACertsBytes, []byte("$")) + }, + FromBytes: func(c *proto.Certs, p *LoadIdentityParams, b []byte) { + c.TLSCACerts = bytes.Split(b, []byte("$")) + }, + }, + + // Common artifacts + { + Key: PrivateKeyKey, + Kind: KindAlways, + ToBytes: func(i *Identity) []byte { + return i.PrivateKeyBytes + }, + FromBytes: func(c *proto.Certs, p *LoadIdentityParams, b []byte) { + p.PrivateKeyBytes = b + }, + }, + { + Key: PublicKeyKey, + Kind: KindAlways, + ToBytes: func(i *Identity) []byte { + return i.PublicKeyBytes + }, + FromBytes: func(c *proto.Certs, p *LoadIdentityParams, b []byte) { + p.PublicKeyBytes = b + }, + }, + { + // The token hash is used to detect changes to the token and + // request a new identity when changes are detected. + Key: TokenHashKey, + Kind: KindBotInternal, + ToBytes: func(i *Identity) []byte { + return i.TokenHashBytes + }, + FromBytes: func(c *proto.Certs, p *LoadIdentityParams, b []byte) { + p.TokenHashBytes = b + }, + }, + { + // The write test is used to ensure the destination is writable before + // attempting a renewal. + Key: WriteTestKey, + Kind: KindAlways, + ToBytes: func(i *Identity) []byte { + // always empty + return []byte{} + }, + FromBytes: func(c *proto.Certs, p *LoadIdentityParams, b []byte) { + // nothing to do + }, + }, +} + +func GetArtifacts() []Artifact { + return artifacts +} diff --git a/tool/tbot/identity/identity.go b/tool/tbot/identity/identity.go index b71ad535266a8..33cd4a050d936 100644 --- a/tool/tbot/identity/identity.go +++ b/tool/tbot/identity/identity.go @@ -17,7 +17,6 @@ limitations under the License. package identity import ( - "bytes" "crypto/tls" "crypto/x509" "fmt" @@ -57,8 +56,12 @@ const ( // PublicKeyKey is the ssh public key, required for successful SSH connections. PublicKeyKey = "key.pub" - // MetadataKey is the name under which additional metadata exists in a destination. - MetadataKey = "meta" + // TokenHashKey is the key where a hash of the onboarding token will be stored. + TokenHashKey = "tokenhash" + + // WriteTestKey is the key for a file used to check that the destination is + // writable. + WriteTestKey = ".write-test" ) var log = logrus.WithFields(logrus.Fields{ @@ -69,10 +72,10 @@ var log = logrus.WithFields(logrus.Fields{ // identity. This is derived from Teleport's usual auth.Identity with small // modifications to work with user rather than host certificates. type Identity struct { - // KeyBytes is a PEM encoded private key - KeyBytes []byte - // SSHPublicKeyBytes contains bytes of the original SSH public key - SSHPublicKeyBytes []byte + // PrivateKeyBytes is a PEM encoded private key + PrivateKeyBytes []byte + // PublicKeyBytes contains bytes of the original SSH public key + PublicKeyBytes []byte // CertBytes is a PEM encoded SSH host cert CertBytes []byte // TLSCertBytes is a PEM encoded TLS x509 client certificate @@ -84,19 +87,39 @@ type Identity struct { SSHCACertBytes [][]byte // KeySigner is an SSH host certificate signer KeySigner ssh.Signer - // Cert is a parsed SSH certificate - Cert *ssh.Certificate - // XCert is X509 client certificate - XCert *x509.Certificate + // SSHCert is a parsed SSH certificate + SSHCert *ssh.Certificate + // X509Cert is an X509 client certificate + X509Cert *x509.Certificate // ClusterName is a name of host's cluster ClusterName string + // TokenHashBytes is the hash of the original join token + TokenHashBytes []byte +} + +// LoadIdentityParams contains parameters beyond proto.Certs needed to load a +// stored identity. +type LoadIdentityParams struct { + PrivateKeyBytes []byte + PublicKeyBytes []byte + TokenHashBytes []byte +} + +// Params returns the LoadIdentityParams for this Identity, which are the +// local-only parameters to be carried over to a renewed identity. +func (i *Identity) Params() *LoadIdentityParams { + return &LoadIdentityParams{ + PrivateKeyBytes: i.PrivateKeyBytes, + PublicKeyBytes: i.PublicKeyBytes, + TokenHashBytes: i.TokenHashBytes, + } } // String returns user-friendly representation of the identity. func (i *Identity) String() string { var out []string - if i.XCert != nil { - out = append(out, fmt.Sprintf("cert(%v issued by %v:%v)", i.XCert.Subject.CommonName, i.XCert.Issuer.CommonName, i.XCert.Issuer.SerialNumber)) + if i.X509Cert != nil { + out = append(out, fmt.Sprintf("cert(%v issued by %v:%v)", i.X509Cert.Subject.CommonName, i.X509Cert.Issuer.CommonName, i.X509Cert.Issuer.SerialNumber)) } for j := range i.TLSCACertsBytes { cert, err := tlsca.ParseCertificatePEM(i.TLSCACertsBytes[j]) @@ -144,7 +167,7 @@ func (i *Identity) HasTLSConfig() bool { // HasPrincipals returns whether identity has principals func (i *Identity) HasPrincipals(additionalPrincipals []string) bool { - set := utils.StringsSet(i.Cert.ValidPrincipals) + set := utils.StringsSet(i.SSHCert.ValidPrincipals) for _, principal := range additionalPrincipals { if _, ok := set[principal]; !ok { return false @@ -155,10 +178,10 @@ func (i *Identity) HasPrincipals(additionalPrincipals []string) bool { // HasDNSNames returns true if TLS certificate has required DNS names func (i *Identity) HasDNSNames(dnsNames []string) bool { - if i.XCert == nil { + if i.X509Cert == nil { return false } - set := utils.StringsSet(i.XCert.DNSNames) + set := utils.StringsSet(i.X509Cert.DNSNames) for _, dnsName := range dnsNames { if _, ok := set[dnsName]; !ok { return false @@ -174,7 +197,7 @@ func (i *Identity) TLSConfig(cipherSuites []uint16) (*tls.Config, error) { if !i.HasTLSConfig() { return nil, trace.NotFound("no TLS credentials setup for this identity") } - tlsCert, err := tls.X509KeyPair(i.TLSCertBytes, i.KeyBytes) + tlsCert, err := tls.X509KeyPair(i.TLSCertBytes, i.PrivateKeyBytes) if err != nil { return nil, trace.BadParameter("failed to parse private key: %v", err) } @@ -211,185 +234,208 @@ func (i *Identity) SSHClientConfig() (*ssh.ClientConfig, error) { if err != nil { return nil, trace.Wrap(err) } - if len(i.Cert.ValidPrincipals) < 1 { + if len(i.SSHCert.ValidPrincipals) < 1 { return nil, trace.BadParameter("user cert has no valid principals") } return &ssh.ClientConfig{ - User: i.Cert.ValidPrincipals[0], + User: i.SSHCert.ValidPrincipals[0], Auth: []ssh.AuthMethod{ssh.PublicKeys(i.KeySigner)}, HostKeyCallback: callback, Timeout: apidefaults.DefaultDialTimeout, }, nil } -// ReadIdentityFromKeyPair reads SSH and TLS identity from key pair. -func ReadIdentityFromKeyPair(privateKey []byte, publicKey []byte, certs *proto.Certs) (*Identity, error) { - identity, err := ReadSSHIdentityFromKeyPair(privateKey, publicKey, certs.SSH) - if err != nil { - return nil, trace.Wrap(err) - } +// 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") + } - if len(certs.SSHCACerts) != 0 { - identity.SSHCACertBytes = certs.SSHCACerts + 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 + } } - if len(certs.TLSCACerts) != 0 { + 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") + } + // Parse the key pair to verify that identity parses properly for future use. - i, err := ReadTLSIdentityFromKeyPair(privateKey, certs.TLS, certs.TLSCACerts) - if err != nil { + if err := ReadTLSIdentityFromKeyPair(&identity, params.PrivateKeyBytes, certs.TLS, certs.TLSCACerts); err != nil { return nil, trace.Wrap(err) } - identity.XCert = i.XCert - identity.TLSCertBytes = certs.TLS - identity.TLSCACertsBytes = certs.TLSCACerts } - return identity, nil + identity.PublicKeyBytes = params.PublicKeyBytes + identity.PrivateKeyBytes = params.PrivateKeyBytes + identity.TokenHashBytes = params.TokenHashBytes + + return &identity, nil } // ReadTLSIdentityFromKeyPair reads TLS identity from key pair -func ReadTLSIdentityFromKeyPair(keyBytes, certBytes []byte, caCertsBytes [][]byte) (*Identity, error) { +func ReadTLSIdentityFromKeyPair(identity *Identity, keyBytes, certBytes []byte, caCertsBytes [][]byte) error { if len(keyBytes) == 0 { - return nil, trace.BadParameter("missing private key") + return trace.BadParameter("missing private key") } if len(certBytes) == 0 { - return nil, trace.BadParameter("missing certificate") + return trace.BadParameter("missing certificate") } cert, err := tlsca.ParseCertificatePEM(certBytes) if err != nil { - return nil, trace.Wrap(err, "failed to parse TLS certificate") + return trace.Wrap(err, "failed to parse TLS certificate") } if len(cert.Issuer.Organization) == 0 { - return nil, trace.BadParameter("missing CA organization") + return trace.BadParameter("missing CA organization") } clusterName := cert.Issuer.Organization[0] if clusterName == "" { - return nil, trace.BadParameter("misssing cluster name") - } - identity := &Identity{ - ClusterName: clusterName, - KeyBytes: keyBytes, - TLSCertBytes: certBytes, - TLSCACertsBytes: caCertsBytes, - XCert: cert, + return trace.BadParameter("misssing cluster name") } + + identity.ClusterName = clusterName + identity.PrivateKeyBytes = keyBytes + identity.TLSCertBytes = certBytes + identity.TLSCACertsBytes = caCertsBytes + identity.X509Cert = cert + // The passed in ciphersuites don't appear to matter here since the returned // *tls.Config is never actually used? _, err = identity.TLSConfig(utils.DefaultCipherSuites()) if err != nil { - return nil, trace.Wrap(err) + return trace.Wrap(err) } - return identity, nil + return nil } // ReadSSHIdentityFromKeyPair reads identity from initialized keypair -func ReadSSHIdentityFromKeyPair(keyBytes, publicKeyBytes, certBytes []byte) (*Identity, error) { +func ReadSSHIdentityFromKeyPair(identity *Identity, keyBytes, publicKeyBytes, certBytes []byte) error { if len(keyBytes) == 0 { - return nil, trace.BadParameter("PrivateKey: missing private key") + return trace.BadParameter("PrivateKey: missing private key") } if len(publicKeyBytes) == 0 { - return nil, trace.BadParameter("PublicKey: missing public key") + return trace.BadParameter("PublicKey: missing public key") } if len(certBytes) == 0 { - return nil, trace.BadParameter("Cert: missing parameter") + return trace.BadParameter("Cert: missing parameter") } cert, err := apisshutils.ParseCertificate(certBytes) if err != nil { - return nil, trace.BadParameter("failed to parse server certificate: %v", err) + return trace.BadParameter("failed to parse server certificate: %v", err) } signer, err := ssh.ParsePrivateKey(keyBytes) if err != nil { - return nil, trace.BadParameter("failed to parse private key: %v", err) + return trace.BadParameter("failed to parse private key: %v", err) } // this signer authenticates using certificate signed by the cert authority // not only by the public key certSigner, err := ssh.NewCertSigner(cert, signer) if err != nil { - return nil, trace.BadParameter("unsupported private key: %v", err) + return trace.BadParameter("unsupported private key: %v", err) } // check principals on certificate if len(cert.ValidPrincipals) < 1 { - return nil, trace.BadParameter("valid principals: at least one valid principal is required") + return trace.BadParameter("valid principals: at least one valid principal is required") } for _, validPrincipal := range cert.ValidPrincipals { if validPrincipal == "" { - return nil, trace.BadParameter("valid principal can not be empty: %q", cert.ValidPrincipals) + return trace.BadParameter("valid principal can not be empty: %q", cert.ValidPrincipals) } } clusterName := cert.Permissions.Extensions[teleport.CertExtensionTeleportRouteToCluster] if clusterName == "" { - return nil, trace.BadParameter("missing cert extension %v", utils.CertExtensionAuthority) + return trace.BadParameter("missing cert extension %v", utils.CertExtensionAuthority) } - return &Identity{ - ClusterName: clusterName, - KeyBytes: keyBytes, - SSHPublicKeyBytes: publicKeyBytes, - CertBytes: certBytes, - KeySigner: certSigner, - Cert: cert, - }, nil + identity.ClusterName = clusterName + identity.PrivateKeyBytes = keyBytes + identity.PublicKeyBytes = publicKeyBytes + identity.CertBytes = certBytes + identity.KeySigner = certSigner + identity.SSHCert = cert + + return nil +} + +// VerifyWrite attempts to write to the .write-test artifact inside the given +// destination. It should be called before attempting a renewal to help ensure +// we won't then fail to save the identity. +func VerifyWrite(dest destination.Destination) error { + return trace.Wrap(dest.Write(WriteTestKey, []byte{})) +} + +// ListKeys returns a list of artifact keys that will be written given a list +// of artifacts. +func ListKeys(kinds ...ArtifactKind) []string { + keys := []string{} + for _, artifact := range GetArtifacts() { + if !artifact.Matches(kinds...) { + continue + } + + keys = append(keys, artifact.Key) + } + + return keys } // SaveIdentity saves a bot identity to a destination. -func SaveIdentity(id *Identity, d destination.Destination) error { - for _, data := range []struct { - name string - data []byte - modeHint destination.ModeHint - }{ - {TLSCertKey, id.TLSCertBytes, destination.ModeHintSecret}, - {SSHCertKey, id.CertBytes, destination.ModeHintSecret}, - {TLSCACertsKey, bytes.Join(id.TLSCACertsBytes, []byte("$")), destination.ModeHintSecret}, - {SSHCACertsKey, bytes.Join(id.SSHCACertBytes, []byte("$")), destination.ModeHintSecret}, - {PrivateKeyKey, id.KeyBytes, destination.ModeHintSecret}, - {PublicKeyKey, id.SSHPublicKeyBytes, destination.ModeHintUnspecified}, - } { - log.Debugf("Writing %s", data.name) - if err := d.Write(data.name, data.data, data.modeHint); err != nil { - return trace.Wrap(err, "could not write to %v", data.name) +func SaveIdentity(id *Identity, d destination.Destination, kinds ...ArtifactKind) error { + for _, artifact := range GetArtifacts() { + // Only store artifacts matching one of the set kinds. + if !artifact.Matches(kinds...) { + continue + } + + data := artifact.ToBytes(id) + + log.Debugf("Writing %s", artifact.Key) + if err := d.Write(artifact.Key, data); err != nil { + return trace.WrapWithMessage(err, "could not write to %v", artifact.Key) } } + return nil } // LoadIdentity loads a bot identity from a destination. -func LoadIdentity(d destination.Destination) (*Identity, error) { - var key, sshPublicKey, tlsCA, sshCA []byte +func LoadIdentity(d destination.Destination, kinds ...ArtifactKind) (*Identity, error) { var certs proto.Certs - var err error - - for _, item := range []struct { - name string - out *[]byte - }{ - {TLSCertKey, &certs.TLS}, - {SSHCertKey, &certs.SSH}, - {TLSCACertsKey, &tlsCA}, - {SSHCACertsKey, &sshCA}, - {PrivateKeyKey, &key}, - {PublicKeyKey, &sshPublicKey}, - } { - *item.out, err = d.Read(item.name) + var params LoadIdentityParams + + for _, artifact := range GetArtifacts() { + // Only attempt to load artifacts matching one of the specified kinds + if !artifact.Matches(kinds...) { + continue + } + + data, err := d.Read(artifact.Key) if err != nil { - return nil, trace.Wrap(err, "could not read %v", item.name) + return nil, trace.WrapWithMessage(err, "could not read artifact %q from destination %s", artifact.Key, d) } - } - certs.SSHCACerts = bytes.Split(sshCA, []byte("$")) - certs.TLSCACerts = bytes.Split(tlsCA, []byte("$")) + artifact.FromBytes(&certs, ¶ms, data) + } log.Debugf("Loaded %d SSH CA certs and %d TLS CA certs", len(certs.SSHCACerts), len(certs.TLSCACerts)) - return ReadIdentityFromKeyPair(key, sshPublicKey, &certs) + return ReadIdentityFromStore(¶ms, &certs, kinds...) } diff --git a/tool/tbot/identity/kinds.go b/tool/tbot/identity/kinds.go new file mode 100644 index 0000000000000..5e98ff2da6905 --- /dev/null +++ b/tool/tbot/identity/kinds.go @@ -0,0 +1,86 @@ +/* +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 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 + +const ( + // KindAlways identifies identity resources that should always be + // 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 { + for _, k := range kinds { + if kind == k { + return true + } + } + + return false +} + +// 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} +} diff --git a/tool/tbot/init.go b/tool/tbot/init.go new file mode 100644 index 0000000000000..c586254f6937e --- /dev/null +++ b/tool/tbot/init.go @@ -0,0 +1,551 @@ +/* +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 main + +import ( + "fmt" + "os" + "os/user" + "path/filepath" + "runtime" + "strconv" + "strings" + + "github.com/google/uuid" + "github.com/gravitational/teleport/api/constants" + "github.com/gravitational/teleport/tool/tbot/botfs" + "github.com/gravitational/teleport/tool/tbot/config" + "github.com/gravitational/teleport/tool/tbot/identity" + "github.com/gravitational/trace" +) + +// RootUID is the UID of the root user +const RootUID = "0" + +// aclTestFailedMessage is printed when an ACL test fails. +const aclTestFailedMessage = "ACLs are not usable for destination %s; " + + "Change the destination's ACL mode to `off` to silence this warning." + +// getInitArtifacts returns a map of all desired artifacts for the destination +func getInitArtifacts(destination *config.DestinationConfig) (map[string]bool, error) { + // true = directory, false = regular file + toCreate := map[string]bool{} + + // Collect all base artifacts and filter for the destination. + for _, artifact := range identity.GetArtifacts() { + if artifact.Matches(destination.Kinds...) { + toCreate[artifact.Key] = false + } + } + + // Collect all config template artifacts. + for _, templateConfig := range destination.Configs { + template, err := templateConfig.GetConfigTemplate() + if err != nil { + return nil, trace.Wrap(err) + } + + for _, file := range template.Describe() { + toCreate[file.Name] = file.IsDir + } + } + + return toCreate, nil +} + +// getExistingArtifacts fetches all entries in a destination directory +func getExistingArtifacts(dir string) (map[string]bool, error) { + existing := map[string]bool{} + + entries, err := os.ReadDir(dir) + if err != nil { + return nil, trace.Wrap(err) + } + + for _, entry := range entries { + existing[entry.Name()] = entry.IsDir() + } + + return existing, nil +} + +// diffArtifacts computes the difference of two artifact sets +func diffArtifacts(a, b map[string]bool) map[string]bool { + diff := map[string]bool{} + + for k, v := range a { + if _, ok := b[k]; !ok { + diff[k] = v + } + } + + return diff +} + +// testACL creates a temporary file and attempts to apply an ACL to it. If the +// ACL is successfully applied, returns nil; otherwise, returns the error. +func testACL(directory string, ownerUser *user.User, opts *botfs.ACLOptions) error { + // Note: we need to create the test file in the dest dir to ensure we + // actually test the target filesystem. + id, err := uuid.NewRandom() + if err != nil { + return trace.Wrap(err) + } + + testFile := filepath.Join(directory, id.String()) + if err := botfs.Create(testFile, false, botfs.SymlinksInsecure); err != nil { + return trace.Wrap(err) + } + + defer func() { + err := os.Remove(testFile) + if err != nil { + log.Debugf("Failed to delete ACL test file %q", testFile) + } + }() + + if err := botfs.ConfigureACL(testFile, ownerUser, opts); err != nil { + return trace.Wrap(err) + } + + return nil +} + +// ensurePermissionsParams sets permissions options +type ensurePermissionsParams struct { + // dirPath is the directory containing the file + dirPath string + + // symlinksMode configures symlink attack mitigation behavior + symlinksMode botfs.SymlinksMode + + // ownerUser is the user that should own the file + ownerUser *user.User + + // ownerGroup is the group that should own the file + ownerGroup *user.Group + + // aclOptions contains configuration for ACLs, if they are in use. nil if + // ACLs are disabled + aclOptions *botfs.ACLOptions + + // toCreate is a set of files that will be newly created, used to reduce + // log spam when configuring new files + toCreate map[string]bool +} + +// ensurePermissions verifies permissions on the given path and, when +// possible, attempts to fix permissions / ACLs on any misconfigured paths. +func ensurePermissions(params *ensurePermissionsParams, key string, isDir bool) error { + path := filepath.Join(params.dirPath, key) + + stat, err := os.Stat(path) + if err != nil { + return trace.Wrap(err) + } + + cleanPath := filepath.Clean(path) + resolved, err := filepath.EvalSymlinks(path) + if err != nil { + return trace.Wrap(err) + } + + // Precomputed flag to determine if certain log messages should be hidden. + // We expect ownership, ACLs, etc to be wrong on initial create so warnings + // are not desirable. + _, newlyCreated := params.toCreate[key] + verboseLogging := key != "" && !newlyCreated + + // This is unlikely as CreateSecure() refuses to follow symlinks, but users + // could move things around after the fact. + if cleanPath != resolved { + switch params.symlinksMode { + case botfs.SymlinksSecure: + return trace.BadParameter("Path %q contains symlinks which is not "+ + "allowed for security reasons.", path) + case botfs.SymlinksInsecure: + // do nothing + default: + log.Warnf("Path %q contains symlinks and may be subject to symlink "+ + "attacks. If this is intentional, consider setting `symlinks: "+ + "insecure` in destination config.", path) + } + } + + // A few conditions we won't try to handle... + if isDir && !stat.IsDir() { + return trace.BadParameter("File %s is expected to be a directory but is a file", path) + } else if !isDir && stat.IsDir() { + return trace.BadParameter("File %s is expected to be a file but is a directory", path) + } + + currentUser, err := user.Current() + if err != nil { + return trace.Wrap(err) + } + + // Correct ownership. + ownedByDesiredOwner, err := botfs.IsOwnedBy(stat, params.ownerUser) + if err != nil { + log.WithError(err).Debugf("Could not determine file ownership of %q", path) + + // Can't read file ownership on this platform (e.g. Windows), so always + // attempt to chown (which does work on Windows) + ownedByDesiredOwner = false + } + + if !ownedByDesiredOwner { + // If we're not running as root, this will probably fail. + if currentUser.Uid != RootUID && runtime.GOOS != constants.WindowsOS { + log.Warnf("Not running as root, ownership change is likely to fail.") + } + + uid, err := strconv.Atoi(params.ownerUser.Uid) + if err != nil { + return trace.Wrap(err) + } + + gid, err := strconv.Atoi(params.ownerGroup.Gid) + if err != nil { + return trace.Wrap(err) + } + + if verboseLogging { + log.Warnf("Ownership of %q is incorrect and will be corrected to %s", path, params.ownerUser.Username) + } + + err = os.Chown(path, uid, gid) + if err != nil { + return trace.Wrap(err) + } + } + + if params.aclOptions != nil { + // We can verify ACLs as any user with read, but can only correct ACLs + // as root or the owner. + // Note that we rely on VerifyACL to return some error if permissions + // are incorrect. + + err = botfs.VerifyACL(path, params.aclOptions) + if err != nil && (currentUser.Uid == RootUID || currentUser.Uid == params.ownerUser.Uid) { + if verboseLogging { + log.Warnf("ACL for %q is not correct and will be corrected: %v", path, err) + } + + return trace.Wrap(botfs.ConfigureACL(path, params.ownerUser, params.aclOptions)) + } else if err != nil { + log.Errorf("ACL for %q is incorrect but `tbot init` must be run "+ + "as root or the owner (%s) to correct it: %v", + path, params.ownerUser.Username, err) + return trace.AccessDenied("Elevated permissions required") + } + + // ACL is valid. + return nil + } + + desiredMode := botfs.DefaultMode + if stat.IsDir() { + desiredMode = botfs.DefaultDirMode + } + + // Using regular permissions, so attempt to correct the file mode. + if stat.Mode().Perm() != desiredMode { + if err := os.Chmod(path, desiredMode); err != nil { + return trace.Wrap(err, "Could not fix permissions on file %q, expected %#o", path, desiredMode) + } + + log.Infof("Corrected permissions on %q from %#o to %#o", path, stat.Mode().Perm(), botfs.DefaultMode) + } + + return nil +} + +// parseOwnerString parses and looks up an user and group from a user:group +// owner string. +func parseOwnerString(owner string) (*user.User, *user.Group, error) { + ownerParts := strings.Split(owner, ":") + if len(ownerParts) != 2 { + return nil, nil, trace.BadParameter("invalid owner string: %q", owner) + } + + ownerUser, err := user.Lookup(ownerParts[0]) + if err != nil { + return nil, nil, trace.Wrap(err) + } + + ownerGroup, err := user.LookupGroup(ownerParts[1]) + if err != nil { + return nil, nil, trace.Wrap(err) + } + + return ownerUser, ownerGroup, nil +} + +// getOwner determines the user/group owner given a CLI parameter (--owner) +// and desired default value. An empty defaultOwner defaults to the current +// user and their primary group. +func getOwner(cliOwner, defaultOwner string) (*user.User, *user.Group, error) { + if cliOwner != "" { + // If --owner is set, always use it. + log.Debugf("Attempting to use explicitly requested owner: %s", cliOwner) + return parseOwnerString(cliOwner) + } + + if defaultOwner != "" { + log.Debugf("Attempting to use default owner: %s", defaultOwner) + // If a default owner is specified, try it instead. + return parseOwnerString(defaultOwner) + } + + log.Debugf("Will use current user as owner.") + // Otherwise, return the current user and group + currentUser, err := user.Current() + if err != nil { + return nil, nil, trace.Wrap(err) + } + + currentGroup, err := user.LookupGroupId(currentUser.Gid) + if err != nil { + return nil, nil, trace.Wrap(err) + } + + return currentUser, currentGroup, nil +} + +// getAndTestACLOptions gets options needed to configure an ACL from CLI +// options and attempts to configure a test ACL to validate them. Ownership is +// not validated here. +func getAndTestACLOptions(cf *config.CLIConf, destDir string) (*user.User, *user.Group, *botfs.ACLOptions, error) { + if cf.BotUser == "" { + return nil, nil, nil, trace.BadParameter("--bot-user must be set") + } + + if cf.ReaderUser == "" { + return nil, nil, nil, trace.BadParameter("--reader-user must be set") + } + + botUser, err := user.Lookup(cf.BotUser) + if err != nil { + return nil, nil, nil, trace.Wrap(err) + } + + botGroup, err := user.LookupGroupId(botUser.Gid) + if err != nil { + return nil, nil, nil, trace.Wrap(err) + } + + readerUser, err := user.Lookup(cf.ReaderUser) + if err != nil { + return nil, nil, nil, trace.Wrap(err) + } + + opts := botfs.ACLOptions{ + BotUser: botUser, + ReaderUser: readerUser, + } + + // Default to letting the bot own the destination, since by this point we + // know the bot user definitely exists and is a reasonable owner choice. + defaultOwner := fmt.Sprintf("%s:%s", botUser.Username, botGroup.Name) + + ownerUser, ownerGroup, err := getOwner(cf.Owner, defaultOwner) + if err != nil { + return nil, nil, nil, trace.Wrap(err) + } + + err = testACL(destDir, ownerUser, &opts) + if err != nil && trace.IsAccessDenied(err) { + return nil, nil, nil, trace.Wrap(err, "The destination %q does not appear to be writable", destDir) + } else if err != nil { + return nil, nil, nil, trace.Wrap(err, "ACL support may need to be enabled for the filesystem.") + } + + return ownerUser, ownerGroup, &opts, nil +} + +func onInit(botConfig *config.BotConfig, cf *config.CLIConf) error { + var destination *config.DestinationConfig + var err error + + // First, resolve the correct destination. If using a config file with only + // 1 destination we can assume we want to init that one; otherwise, + // --init-dir is required. + if cf.InitDir == "" { + if len(botConfig.Destinations) == 1 { + destination = botConfig.Destinations[0] + } else { + return trace.BadParameter("A destination to initialize must be specified with --init-dir") + } + } else { + destination, err = botConfig.GetDestinationByPath(cf.InitDir) + if err != nil { + return trace.WrapWithMessage(err, "Could not find specified destination %q", cf.InitDir) + } + + if destination == nil { + // TODO: in the future if/when other backends are supported, + // destination might be nil because the user tried to enter a non + // filesystem path, so this error message could be misleading. + return trace.NotFound("Cannot initialize destination %q because "+ + "it has not been configured.", cf.InitDir) + } + } + + destImpl, err := destination.GetDestination() + if err != nil { + return trace.Wrap(err) + } + + destDir, ok := destImpl.(*config.DestinationDirectory) + if !ok { + return trace.BadParameter("`tbot init` only supports directory destinations") + } + + log.Infof("Initializing destination: %s", destImpl) + + // 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 { + return trace.Wrap(err) + } + + // Next, test if we want + have ACL support, and set aclOpts if we do. + // Desired owner is derived from the ACL mode. + var aclOpts *botfs.ACLOptions + var ownerUser *user.User + var ownerGroup *user.Group + + switch destDir.ACLs { + case botfs.ACLRequired, botfs.ACLTry: + log.Debug("Testing for ACL support...") + + // Awkward control flow here, but we want these to fail together. + ownerUser, ownerGroup, aclOpts, err = getAndTestACLOptions(cf, destDir.Path) + if err != nil { + if destDir.ACLs == botfs.ACLRequired { + // ACLs were specifically requested (vs "try" mode), so fail. + return trace.Wrap(err, aclTestFailedMessage, destImpl) + } + + // Otherwise, fall back to no ACL with a warning. + log.WithError(err).Warnf(aclTestFailedMessage, destImpl) + aclOpts = nil + + // We'll also need to re-fetch the owner as the defaults are + // different in the fallback case. + ownerUser, ownerGroup, err = getOwner(cf.Owner, "") + if err != nil { + return trace.Wrap(err) + } + } else if aclOpts.ReaderUser.Uid == ownerUser.Uid { + log.Warnf("The destination owner (%s) and reader (%s) are the "+ + "same. This will break OpenSSH.", aclOpts.ReaderUser.Username, + ownerUser.Username) + } + default: + log.Info("ACLs disabled for this destination.") + ownerUser, ownerGroup, err = getOwner(cf.Owner, "") + if err != nil { + return trace.Wrap(err) + } + } + + // Next, resolve what we want and what we already have. + desired, err := getInitArtifacts(destination) + if err != nil { + return trace.Wrap(err) + } + + existing, err := getExistingArtifacts(destDir.Path) + if err != nil { + return trace.Wrap(err) + } + + toCreate := diffArtifacts(desired, existing) + toRemove := diffArtifacts(existing, desired) + + // Based on this, create any new files. + if len(toCreate) > 0 { + log.Infof("Attempting to create: %v", toCreate) + + for key, isDir := range toCreate { + path := filepath.Join(destDir.Path, key) + if err := botfs.Create(path, isDir, destDir.Symlinks); err != nil { + return trace.Wrap(err) + } + + log.Infof("Created: %s", path) + } + } else { + log.Info("Nothing to create.") + } + + // ... and warn about / remove any unneeded files. + if len(toRemove) > 0 && cf.Clean { + log.Infof("Attempting to remove: %v", toRemove) + + var errors []error + + for key := range toRemove { + path := filepath.Join(destDir.Path, key) + + if err := os.RemoveAll(path); err != nil { + errors = append(errors, err) + } else { + log.Infof("Removed: %s", path) + } + } + + if err := trace.NewAggregate(errors...); err != nil { + return trace.Wrap(err) + } + } else if len(toRemove) > 0 { + log.Warnf("Unexpected files found in destination directory, consider " + + "removing it manually or rerunning `tbot init` with the `--clean` " + + "flag.") + } else { + log.Info("Nothing to remove.") + } + + params := ensurePermissionsParams{ + dirPath: destDir.Path, + aclOptions: aclOpts, + ownerUser: ownerUser, + ownerGroup: ownerGroup, + symlinksMode: destDir.Symlinks, + toCreate: toCreate, + } + + // Check and set permissions on the directory itself. + if err := ensurePermissions(¶ms, "", true); err != nil { + return trace.Wrap(err) + } + + // Lastly, set and check permissions on all the desired files. + for key, isDir := range desired { + if err := ensurePermissions(¶ms, key, isDir); err != nil { + return trace.Wrap(err) + } + } + + log.Infof("Destination %s has been initialized. Note that these files "+ + "will be empty and invalid until the bot issues certificates.", + destImpl) + + return nil +} diff --git a/tool/tbot/init_test.go b/tool/tbot/init_test.go new file mode 100644 index 0000000000000..d154e0cb39480 --- /dev/null +++ b/tool/tbot/init_test.go @@ -0,0 +1,271 @@ +/* +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 main + +import ( + "fmt" + "os" + "os/user" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/gravitational/teleport/api/constants" + "github.com/gravitational/teleport/tool/tbot/botfs" + "github.com/gravitational/teleport/tool/tbot/config" + "github.com/gravitational/teleport/tool/tbot/identity" + "github.com/gravitational/trace" + "github.com/stretchr/testify/require" +) + +// usernamesToTry contains a list of usernames we can use as ACL targets in +// testing. +var usernamesToTry = []string{"nobody", "ci", "root"} + +func contains(entries []string, entry string) bool { + for _, e := range entries { + if e == entry { + return true + } + } + + return false +} + +// filterUsers returns the input list of usernames except for those in the +// exclude list. +func filterUsers(usernames, exclude []string) []string { + ret := []string{} + + for _, username := range usernames { + if !contains(exclude, username) { + ret = append(ret, username) + } + } + + return ret +} + +// findUser attempts to find a usable user on the local system from the given +// list of usernames and returns the first match found. +func findUser(usernamesToTry, usernamesToExclude []string) (*user.User, error) { + filtered := filterUsers(usernamesToTry, usernamesToExclude) + for _, username := range filtered { + u, err := user.Lookup(username) + if err == nil { + return u, nil + } + } + + return nil, trace.NotFound("No users found matching %+v (excluding %+v)", usernamesToTry, usernamesToExclude) +} + +// getACLOptions returns sane ACLOptions for this platform. +func getACLOptions() (*botfs.ACLOptions, error) { + if runtime.GOOS != constants.LinuxOS { + return nil, trace.NotImplemented("Unsupported platform") + } + + user, err := user.Current() + if err != nil { + return nil, trace.Wrap(err) + } + + exclude := []string{user.Name} + + // Find a set of users we can test against. + readerUser, err := findUser(usernamesToTry, exclude) + if trace.IsNotFound(err) { + return nil, trace.NotFound("Not enough usable users for testing ACLs") + } else if err != nil { + return nil, trace.Wrap(err) + } + + exclude = append(exclude, readerUser.Name) + botUser, err := findUser(usernamesToTry, exclude) + if trace.IsNotFound(err) { + return nil, trace.NotFound("Not enough suitable users found for testing ACLs.") + } else if err != nil { + return nil, trace.Wrap(err) + } + + return &botfs.ACLOptions{ + ReaderUser: readerUser, + BotUser: botUser, + }, nil +} + +// testConfigFromCLI creates a BotConfig from the given CLI config. +func testConfigFromCLI(t *testing.T, cf *config.CLIConf) *config.BotConfig { + cfg, err := config.FromCLIConf(cf) + require.NoError(t, err) + + return cfg +} + +// testConfigFromString parses a YAML config file from a string. +func testConfigFromString(t *testing.T, yaml string) *config.BotConfig { + cfg, err := config.ReadConfig(strings.NewReader(yaml)) + require.NoError(t, err) + + return cfg +} + +// validateFileDestinations ensures all files in a destination exist on disk as +// expected, and returns the destination. +func validateFileDestination(t *testing.T, dest *config.DestinationConfig) *config.DestinationDirectory { + destImpl, err := dest.GetDestination() + require.NoError(t, err) + + destDir, ok := destImpl.(*config.DestinationDirectory) + require.True(t, ok) + + for _, art := range identity.GetArtifacts() { + if !art.Matches(dest.Kinds...) { + continue + } + + require.FileExists(t, filepath.Join(destDir.Path, art.Key)) + } + + return destDir +} + +// TestInit ensures defaults work regardless of host platform. With no bot user +// specified, this never tries to use ACLs. +func TestInit(t *testing.T) { + dir := t.TempDir() + cf := &config.CLIConf{ + AuthServer: "example.com", + DestinationDir: dir, + } + cfg := testConfigFromCLI(t, cf) + + // Run init. + require.NoError(t, onInit(cfg, cf)) + + // Make sure everything was created. + _ = validateFileDestination(t, cfg.Destinations[0]) +} + +// TestInitMaybeACLs tests defaults with ACLs possibly enabled, by supplying +// bot and reader users. +func TestInitMaybeACLs(t *testing.T) { + opts, err := getACLOptions() + if trace.IsNotImplemented(err) { + t.Skipf("%+v", err) + } else if trace.IsNotFound(err) { + t.Skipf("%+v", err) + } + require.NoError(t, err) + + hasACLSupport, err := botfs.HasACLSupport() + require.NoError(t, err) + + currentUser, err := user.Current() + require.NoError(t, err) + + currentGroup, err := user.LookupGroupId(currentUser.Gid) + require.NoError(t, err) + + // Determine if we expect init to use ACLs. + expectACLs := false + if hasACLSupport { + if err := testACL(t.TempDir(), currentUser, opts); err == nil { + expectACLs = true + } + } + + // Note: we'll use the current user as owner as that's the only way to + // guarantee ACL write access. + dir := t.TempDir() + cf := &config.CLIConf{ + AuthServer: "example.com", + DestinationDir: dir, + BotUser: opts.BotUser.Username, + ReaderUser: opts.ReaderUser.Username, + + // This isn't a default, but unfortunately we need to specify a + // non-nobody owner for CI purposes. + Owner: fmt.Sprintf("%s:%s", currentUser.Name, currentGroup.Name), + } + cfg := testConfigFromCLI(t, cf) + + // Run init. + require.NoError(t, onInit(cfg, cf)) + + // Make sure everything was created. + destDir := validateFileDestination(t, cfg.Destinations[0]) + + // If we expect ACLs, verify them. + if expectACLs { + require.NoError(t, destDir.Verify(identity.ListKeys(cfg.Destinations[0].Kinds...))) + } else { + t.Logf("Skipping ACL check on %q as they should not be supported.", dir) + } +} + +// testInitSymlinksTemplate is a config template with a configurable symlinks +// mode and ACLs disabled. +const testInitSymlinksTemplate = ` +auth_server: example.com +destinations: + - directory: + path: %s + acls: off + symlinks: %s +` + +// TestInitSymlink tests tbot init with a symlink in the path. +func TestInitSymlink(t *testing.T) { + secureWriteSupported, err := botfs.HasSecureWriteSupport() + require.NoError(t, err) + if !secureWriteSupported { + t.Skip("Secure write not supported on this system.") + } + + dir := t.TempDir() + + realPath := filepath.Join(dir, "data") + dataDir := filepath.Join(dir, "data-symlink") + require.NoError(t, os.Symlink(realPath, dataDir)) + + // Should fail due to symlink in path. + cfg := testConfigFromString(t, fmt.Sprintf(testInitSymlinksTemplate, dataDir, botfs.SymlinksSecure)) + require.Error(t, onInit(cfg, &config.CLIConf{})) + + // Should succeed when writing to the dir directly. + cfg = testConfigFromString(t, fmt.Sprintf(testInitSymlinksTemplate, realPath, botfs.SymlinksSecure)) + require.NoError(t, onInit(cfg, &config.CLIConf{})) + + // Make sure everything was created. + _ = validateFileDestination(t, cfg.Destinations[0]) +} + +// TestInitSymlinksInsecure should work on all platforms. +func TestInitSymlinkInsecure(t *testing.T) { + dir := t.TempDir() + + realPath := filepath.Join(dir, "data") + dataDir := filepath.Join(dir, "data-symlink") + require.NoError(t, os.Symlink(realPath, dataDir)) + + // Should succeed due to SymlinksInsecure + cfg := testConfigFromString(t, fmt.Sprintf(testInitSymlinksTemplate, dataDir, botfs.SymlinksInsecure)) + require.Error(t, onInit(cfg, &config.CLIConf{})) +} diff --git a/tool/tbot/main.go b/tool/tbot/main.go index 5c7c941c4e1c2..ab57cb849be8d 100644 --- a/tool/tbot/main.go +++ b/tool/tbot/main.go @@ -17,7 +17,10 @@ limitations under the License. package main import ( + "bytes" "context" + "crypto/sha256" + "encoding/hex" "fmt" "os" "strings" @@ -35,6 +38,7 @@ import ( "github.com/gravitational/teleport/lib/tlsca" "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/tool/tbot/config" + "github.com/gravitational/teleport/tool/tbot/destination" "github.com/gravitational/teleport/tool/tbot/identity" "github.com/gravitational/trace" "github.com/kr/pretty" @@ -74,13 +78,28 @@ func Run(args []string) error { startCmd.Flag("destination-dir", "Directory to write generated certificates").StringVar(&cf.DestinationDir) startCmd.Flag("certificate-ttl", "TTL of generated certificates").Default("60m").DurationVar(&cf.CertificateTTL) startCmd.Flag("renew-interval", "Interval at which certificates are renewed; must be less than the certificate TTL.").DurationVar(&cf.RenewInterval) - startCmd.Flag("join-method", "Method to use to join the cluster.").Default("token").EnumVar(&cf.JoinMethod, "token", "iam") + startCmd.Flag("join-method", "Method to use to join the cluster.").Default(config.DefaultJoinMethod).EnumVar(&cf.JoinMethod, "token", "iam") + + initCmd := app.Command("init", "Initialize a certificate destination directory for writes from a separate bot user.") + initCmd.Flag("auth-server", "Specify the Teleport auth server host").Short('a').Envar(authServerEnvVar).StringVar(&cf.AuthServer) + initCmd.Flag("destination-dir", "If NOT using a config file, specify the destination directory.").StringVar(&cf.DestinationDir) + initCmd.Flag("init-dir", "If using a config file and multiple destinations are configured, specify which to initialize.").StringVar(&cf.InitDir) + initCmd.Flag("clean", "If set, remove unexpected files and directories from the destination.").BoolVar(&cf.Clean) + initCmd.Flag("reader-user", "If ACLs are in use, name of the Unix user "+ + "that will read from the destination.", + ).StringVar(&cf.ReaderUser) + initCmd.Flag("bot-user", "If ACLs are in use, name of the bot Unix user "+ + "which should have write access to the destination.", + ).StringVar(&cf.BotUser) + initCmd.Flag("owner", "Name of the user:group that will own the "+ + "destination. If ACLs are in use, must be different from the reader "+ + "user and defaults to nobody:nobody. Otherwise, assumes the current "+ + "user.", + ).StringVar(&cf.Owner) configCmd := app.Command("config", "Parse and dump a config file").Hidden() - initCmd := app.Command("init", "Initialize a certificate destination directory.") - - watchCmd := app.Command("watch", "Watch a destination directory for changes.") + watchCmd := app.Command("watch", "Watch a destination directory for changes.").Hidden() command, err := app.Parse(args) if err != nil { @@ -103,7 +122,7 @@ func Run(args []string) error { case configCmd.FullCommand(): err = onConfig(botConfig) case initCmd.FullCommand(): - err = onInit(botConfig) + err = onInit(botConfig, &cf) case watchCmd.FullCommand(): err = onWatch(botConfig) default: @@ -120,15 +139,16 @@ func onConfig(botConfig *config.BotConfig) error { return nil } -func onInit(botConfig *config.BotConfig) error { - return trace.NotImplemented("init not yet implemented") -} - func onWatch(botConfig *config.BotConfig) error { return trace.NotImplemented("watch not yet implemented") } func onStart(botConfig *config.BotConfig) error { + // First, try to make sure all destinations are usable. + if err := checkDestinations(botConfig); err != nil { + return trace.Wrap(err) + } + // Start by loading the bot's primary destination. dest, err := botConfig.Storage.GetDestination() if err != nil { @@ -141,20 +161,26 @@ func onStart(botConfig *config.BotConfig) error { ctx, cancel := context.WithCancel(context.Background()) defer cancel() + configTokenHashBytes := []byte{} + if botConfig.Onboarding != nil && botConfig.Onboarding.Token != "" { + sha := sha256.Sum256([]byte(botConfig.Onboarding.Token)) + configTokenHashBytes = []byte(hex.EncodeToString(sha[:])) + } + // First, attempt to load an identity from storage. - ident, err := identity.LoadIdentity(dest) - if err == nil { - identStr, err := describeIdentity(ident) + ident, err := identity.LoadIdentity(dest, identity.BotKinds()...) + if err == nil && !hasTokenChanged(ident.TokenHashBytes, configTokenHashBytes) { + identStr, err := describeTLSIdentity(ident) if err != nil { return trace.Wrap(err) } log.Infof("Successfully loaded bot identity, %s", identStr) - // TODO: we should cache the token; if --token is provided but - // different, assume the user is attempting to start with a new - // identity. (May want to store a sha256 has to avoid storing the - // token directly.) + if err := checkIdentity(ident); err != nil { + return trace.Wrap(err) + } + if botConfig.Onboarding != nil { log.Warn("Note: onboarding config ignored as identity was loaded from persistent storage") } @@ -167,9 +193,19 @@ func onStart(botConfig *config.BotConfig) error { // If the identity can't be loaded, assume we're starting fresh and // need to generate our initial identity from a token + if ident != nil { + // If ident is set here, we detected a token change above. + log.Warnf("Detected a token change, will attempt to fetch a new identity.") + } + // TODO: validate that errors from LoadIdentity are sanely typed; we // actually only want to ignore NotFound errors + // Verify we can write to the destination. + if err := identity.VerifyWrite(dest); err != nil { + return trace.Wrap(err, "Could not write to destination %s, aborting.", dest) + } + // Get first identity ident, err = getIdentityFromToken(botConfig) if err != nil { @@ -187,24 +223,18 @@ func onStart(botConfig *config.BotConfig) error { return trace.Wrap(err, "unable to communicate with auth server") } - identStr, err := describeIdentity(ident) + identStr, err := describeTLSIdentity(ident) if err != nil { return trace.Wrap(err) } log.Infof("Successfully generated new bot identity, %s", identStr) log.Debugf("Storing new bot identity to %s", dest) - if err := identity.SaveIdentity(ident, dest); err != nil { + if err := identity.SaveIdentity(ident, dest, identity.BotKinds()...); err != nil { return trace.Wrap(err, "unable to save generated identity back to destination") } } - // TODO: handle cases where an identity exists on disk but we might _not_ - // want to use it: - // - identity has expired - // - user provided a new token - // - ??? - watcher, err := authClient.NewWatcher(ctx, types.Watch{ Kinds: []types.WatchKind{{ Kind: types.KindCertAuthority, @@ -221,6 +251,81 @@ func onStart(botConfig *config.BotConfig) error { return renewLoop(ctx, botConfig, authClient, ident) } +func hasTokenChanged(configTokenBytes, identityBytes []byte) bool { + if len(configTokenBytes) == 0 || len(identityBytes) == 0 { + return false + } + + return !bytes.Equal(identityBytes, configTokenBytes) +} + +// checkDestinations checks all destinations and tries to create any that +// don't already exist. +func checkDestinations(cfg *config.BotConfig) error { + // Note: This is vaguely problematic as we don't recommend that users + // store renewable certs under the same user as end-user certs. That said, + // - if the destination was properly created via tbot init this is a no-op + // - if users intend to follow that advice but miss a step, it should fail + // due to lack of permissions + storage, err := cfg.Storage.GetDestination() + if err != nil { + return trace.Wrap(err) + } + + // TODO: consider warning if ownership of all destintions is not expected. + + if err := storage.Init(); err != nil { + return trace.Wrap(err) + } + + for _, dest := range cfg.Destinations { + destImpl, err := dest.GetDestination() + if err != nil { + return trace.Wrap(err) + } + + if err := destImpl.Init(); err != nil { + return trace.Wrap(err) + } + } + + return nil +} + +// checkIdentity performs basic startup checks on an identity and loudly warns +// end users if it is unlikely to work. +func checkIdentity(ident *identity.Identity) error { + var validAfter time.Time + var validBefore time.Time + + if ident.X509Cert != nil { + validAfter = ident.X509Cert.NotBefore + validBefore = ident.X509Cert.NotAfter + } else if ident.SSHCert != nil { + validAfter = time.Unix(int64(ident.SSHCert.ValidAfter), 0) + validBefore = time.Unix(int64(ident.SSHCert.ValidBefore), 0) + } else { + return trace.BadParameter("identity is invalid and contains no certificates") + } + + now := time.Now().UTC() + if now.After(validBefore) { + log.Errorf( + "Identity has expired. The renewal is likely to fail. (expires: %s, current time: %s)", + validBefore.Format(time.RFC3339), + now.Format(time.RFC3339), + ) + } else if now.Before(validAfter) { + log.Warnf( + "Identity is not yet valid. Confirm that the system time is correct. (valid after: %s, current time: %s)", + validAfter.Format(time.RFC3339), + now.Format(time.RFC3339), + ) + } + + return nil +} + func watchCARotations(watcher types.Watcher) { for { select { @@ -253,7 +358,7 @@ func getIdentityFromToken(cfg *config.BotConfig) (*identity.Identity, error) { return nil, trace.WrapWithMessage(err, "unable to generate new keypairs") } - log.Info("attempting to generate new identity from token") + log.Info("Attempting to generate new identity from token") params := auth.RegisterParams{ Token: cfg.Onboarding.Token, ID: auth.IdentityID{ @@ -271,7 +376,13 @@ func getIdentityFromToken(cfg *config.BotConfig) (*identity.Identity, error) { if err != nil { return nil, trace.Wrap(err) } - ident, err := identity.ReadIdentityFromKeyPair(tlsPrivateKey, sshPublicKey, certs) + sha := sha256.Sum256([]byte(params.Token)) + tokenHash := hex.EncodeToString(sha[:]) + ident, err := identity.ReadIdentityFromStore(&identity.LoadIdentityParams{ + PrivateKeyBytes: tlsPrivateKey, + PublicKeyBytes: sshPublicKey, + TokenHashBytes: []byte(tokenHash), + }, certs, identity.BotKinds()...) return ident, trace.Wrap(err) } @@ -299,18 +410,18 @@ func renewIdentityViaAuth( // Ask the auth server to generate a new set of certs with a new // expiration date. certs, err := client.GenerateUserCerts(ctx, proto.UserCertsRequest{ - PublicKey: currentIdentity.SSHPublicKeyBytes, - Username: currentIdentity.XCert.Subject.CommonName, + PublicKey: currentIdentity.PublicKeyBytes, + Username: currentIdentity.X509Cert.Subject.CommonName, Expires: time.Now().Add(cfg.CertificateTTL), }) if err != nil { return nil, trace.Wrap(err) } - newIdentity, err := identity.ReadIdentityFromKeyPair( - currentIdentity.KeyBytes, - currentIdentity.SSHPublicKeyBytes, + newIdentity, err := identity.ReadIdentityFromStore( + currentIdentity.Params(), certs, + identity.BotKinds()..., ) if err != nil { return nil, trace.Wrap(err) @@ -331,26 +442,31 @@ func fetchDefaultRoles(ctx context.Context, roleGetter services.RoleGetter, botR return conditions.Roles, nil } -// describeIdentity writes an informational message about the given identity to +// describeTLSIdentity writes an informational message about the given identity to // the log. -func describeIdentity(ident *identity.Identity) (string, error) { - tlsIdent, err := tlsca.FromSubject(ident.XCert.Subject, ident.XCert.NotAfter) +func describeTLSIdentity(ident *identity.Identity) (string, error) { + cert := ident.X509Cert + if cert == nil { + return "", trace.BadParameter("attempted to describe TLS identity without TLS credentials") + } + + tlsIdent, err := tlsca.FromSubject(cert.Subject, cert.NotAfter) if err != nil { return "", trace.Wrap(err, "bot TLS certificate can not be parsed as an identity") } var principals []string - for _, principal := range ident.Cert.ValidPrincipals { + for _, principal := range tlsIdent.Principals { if !strings.HasPrefix(principal, constants.NoLoginPrefix) { principals = append(principals, principal) } } - duration := time.Second * time.Duration(ident.Cert.ValidBefore-ident.Cert.ValidAfter) + duration := cert.NotAfter.Sub(cert.NotBefore) return fmt.Sprintf( - "valid: after=%v, before=%v, duration=%s | properties: renewable=%v, disallow-reissue=%v, roles=%v, principals=%v, generation=%v", - time.Unix(int64(ident.Cert.ValidAfter), 0), - time.Unix(int64(ident.Cert.ValidBefore), 0), + "valid: after=%v, before=%v, duration=%s | kind=tls, renewable=%v, disallow-reissue=%v, roles=%v, principals=%v, generation=%v", + cert.NotBefore.Format(time.RFC3339), + cert.NotAfter.Format(time.RFC3339), duration, tlsIdent.Renewable, tlsIdent.DisallowReissue, @@ -360,124 +476,214 @@ func describeIdentity(ident *identity.Identity) (string, error) { ), nil } -func renewLoop(ctx context.Context, cfg *config.BotConfig, client auth.ClientI, ident *identity.Identity) error { - // TODO: failures here should probably not just end the renewal loop, there - // should be some retry / back-off logic. - - // TODO: what should this interval be? should it be user configurable? - // Also, must be < the validity period. - // TODO: validate that cert is actually renewable. +// describeSSHIdentity writes an informational message about the given SSH +// identity to the log. +func describeSSHIdentity(ident *identity.Identity) (string, error) { + cert := ident.SSHCert + if cert == nil { + return "", trace.BadParameter("attempted to describe SSH identity without SSH credentials") + } - log.Infof("Beginning renewal loop: ttl=%s interval=%s", cfg.CertificateTTL, cfg.RenewInterval) - if cfg.RenewInterval > cfg.CertificateTTL { - log.Errorf( - "Certificate TTL (%s) is shorter than the renewal interval (%s). The next renewal is likely to fail.", - cfg.CertificateTTL, - cfg.RenewInterval, - ) + renewable := false + if _, ok := cert.Extensions[teleport.CertExtensionRenewable]; ok { + renewable = true } - // Determine where the bot should write its internal data (renewable cert - // etc) - botDestination, err := cfg.Storage.GetDestination() - if err != nil { - return trace.Wrap(err) + disallowReissue := false + if _, ok := cert.Extensions[teleport.CertExtensionDisallowReissue]; ok { + disallowReissue = true } - ticker := time.NewTicker(cfg.RenewInterval) - defer ticker.Stop() - for { - log.Debug("Attempting to renew bot certificates...") - newIdentity, err := renewIdentityViaAuth(ctx, client, ident, cfg) - if err != nil { - return trace.Wrap(err) + var roles []string + if rolesStr, ok := cert.Extensions[teleport.CertExtensionTeleportRoles]; ok { + if actualRoles, err := services.UnmarshalCertRoles(rolesStr); err == nil { + roles = actualRoles } + } - identStr, err := describeIdentity(ident) - if err != nil { - return trace.Wrap(err, "Could not describe bot identity at %s", botDestination) + 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 +} + +// renew performs a single renewal +func renew( + ctx context.Context, cfg *config.BotConfig, client auth.ClientI, + ident *identity.Identity, botDestination destination.Destination, +) (auth.ClientI, *identity.Identity, error) { + // Make sure we can still write to the bot's destination. + if err := identity.VerifyWrite(botDestination); err != nil { + return nil, nil, trace.Wrap(err, "Cannot write to destination %s, aborting.", botDestination) + } + + log.Debug("Attempting to renew bot certificates...") + newIdentity, err := renewIdentityViaAuth(ctx, client, ident, cfg) + if err != nil { + return nil, nil, trace.Wrap(err) + } + + identStr, err := describeTLSIdentity(ident) + if err != nil { + return nil, nil, trace.Wrap(err, "Could not describe bot identity at %s", botDestination) + } + + log.Infof("Successfully renewed bot certificates, %s", identStr) + + // TODO: warn if duration < certTTL? would indicate TTL > server's max renewable cert TTL + // TODO: error if duration < renewalInterval? next renewal attempt will fail + + // Immediately attempt to reconnect using the new identity (still + // haven't persisted the known-good certs). + newClient, err := authenticatedUserClientFromIdentity(ctx, newIdentity, cfg.AuthServer) + if err != nil { + return nil, nil, trace.Wrap(err) + } + + // Attempt a request to make sure our client works. + // TODO: consider a retry/backoff loop. + if _, err := newClient.Ping(ctx); err != nil { + return nil, nil, trace.Wrap(err, "unable to communicate with auth server") + } + + log.Debug("Auth client now using renewed credentials.") + client = newClient + ident = newIdentity - log.Infof("Successfully renewed bot certificates, %s", identStr) + // Now that we're sure the new creds work, persist them. + if err := identity.SaveIdentity(newIdentity, botDestination, identity.BotKinds()...); err != nil { + return nil, nil, trace.Wrap(err) + } - // TODO: warn if duration < certTTL? would indicate TTL > server's max renewable cert TTL - // TODO: error if duration < renewalInterval? next renewal attempt will fail + // Determine the default role list based on the bot role. The role's + // name should match the certificate's Key ID (user and role names + // should all match bot-$name) + botResourceName := ident.X509Cert.Subject.CommonName + defaultRoles, err := fetchDefaultRoles(ctx, client, botResourceName) + if err != nil { + log.WithError(err).Warnf("Unable to determine default roles, no roles will be requested if unspecified") + defaultRoles = []string{} + } - // Immediately attempt to reconnect using the new identity (still - // haven't persisted the known-good certs). - newClient, err := authenticatedUserClientFromIdentity(ctx, newIdentity, cfg.AuthServer) + // Next, generate impersonated certs + expires := ident.X509Cert.NotAfter + for _, dest := range cfg.Destinations { + destImpl, err := dest.GetDestination() if err != nil { - return trace.Wrap(err) + return nil, nil, trace.Wrap(err) } - // Attempt a request to make sure our client works. - // TODO: consider a retry/backoff loop. - if _, err := newClient.Ping(ctx); err != nil { - return trace.Wrap(err, "unable to communicate with auth server") + // 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 { + return nil, nil, trace.Wrap(err) } - log.Debug("Auth client now using renewed credentials.") - client = newClient - ident = newIdentity + // Ensure this destination is also writable. This is a hard fail if + // ACLs are misconfigured, regardless of configuration. + // TODO: consider not making these a hard error? e.g. write other + // destinations even if this one is broken? + if err := identity.VerifyWrite(destImpl); err != nil { + return nil, nil, trace.Wrap(err, "Could not write to destination %s, aborting.", destImpl) + } - // Now that we're sure the new creds work, persist them. - if err := identity.SaveIdentity(newIdentity, botDestination); err != nil { - return trace.Wrap(err) + var desiredRoles []string + if len(dest.Roles) > 0 { + desiredRoles = dest.Roles + } else { + log.Debugf("Destination specified no roles, defaults will be requested: %v", defaultRoles) + desiredRoles = defaultRoles } - // Determine the default role list based on the bot role. The role's - // name should match the certificate's Key ID (user and role names - // should all match bot-$name) - defaultRoles, err := fetchDefaultRoles(ctx, client, ident.Cert.KeyId) + impersonatedIdent, err := generateImpersonatedIdentity(ctx, client, ident, expires, desiredRoles, dest.Kinds) if err != nil { - log.WithError(err).Warnf("Unable to determine default roles, no roles will be requested if unspecified") - defaultRoles = []string{} + return nil, nil, trace.Wrap(err, "Failed to generate impersonated certs for %s: %+v", destImpl, err) } - // Next, generate impersonated certs - expires := time.Unix(int64(ident.Cert.ValidBefore), 0) - for _, dest := range cfg.Destinations { - destImpl, err := dest.GetDestination() + var impersonatedIdentStr string + if dest.ContainsKind(identity.KindTLS) { + impersonatedIdentStr, err = describeTLSIdentity(impersonatedIdent) if err != nil { - return trace.Wrap(err) + return nil, nil, trace.Wrap(err, "could not describe impersonated certs for destination %s", destImpl) } - - var desiredRoles []string - if len(dest.Roles) > 0 { - desiredRoles = dest.Roles - } else { - log.Debugf("Destination specified no roles, defaults will be requested: %v", defaultRoles) - desiredRoles = defaultRoles - } - - impersonatedIdent, err := generateImpersonatedIdentity(ctx, client, ident, expires, desiredRoles) + } else { + // Note: kinds must contain at least 1 of TLS or SSH + impersonatedIdentStr, err = describeSSHIdentity(impersonatedIdent) if err != nil { - return trace.Wrap(err, "Failed to generate impersonated certs for %s: %+v", destImpl, err) + 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 { + return nil, nil, trace.Wrap(err, "failed to save impersonated identity to destination %s", destImpl) + } - impersonatedIdentStr, err := describeIdentity(impersonatedIdent) + for _, templateConfig := range dest.Configs { + template, err := templateConfig.GetConfigTemplate() if err != nil { - return trace.Wrap(err, "could not describe impersonated certs for destination %s", destImpl) + return nil, nil, trace.Wrap(err) } - log.Infof("Successfully renewed impersonated certificates for %s, %s", destImpl, impersonatedIdentStr) - if err := identity.SaveIdentity(impersonatedIdent, destImpl); err != nil { - return trace.Wrap(err, "failed to save impersonated identity to destination %s", destImpl) + if err := template.Render(ctx, client, impersonatedIdent, dest); err != nil { + log.WithError(err).Warnf("Failed to render config template %+v", templateConfig) } + } + } - for _, templateConfig := range dest.Configs { - template, err := templateConfig.GetConfigTemplate() - if err != nil { - return trace.Wrap(err) - } + log.Infof("Persisted new certificates to disk. Next renewal in approximately %s", cfg.RenewInterval) + return newClient, newIdentity, nil +} - if err := template.Render(ctx, client, impersonatedIdent, dest); err != nil { - log.WithError(err).Warnf("Failed to render config template %+v", templateConfig) - } - } +func renewLoop(ctx context.Context, cfg *config.BotConfig, client auth.ClientI, ident *identity.Identity) error { + // TODO: failures here should probably not just end the renewal loop, there + // should be some retry / back-off logic. + + // TODO: what should this interval be? should it be user configurable? + // Also, must be < the validity period. + // TODO: validate that cert is actually renewable. + + log.Infof("Beginning renewal loop: ttl=%s interval=%s", cfg.CertificateTTL, cfg.RenewInterval) + if cfg.RenewInterval > cfg.CertificateTTL { + log.Errorf( + "Certificate TTL (%s) is shorter than the renewal interval (%s). The next renewal is likely to fail.", + cfg.CertificateTTL, + cfg.RenewInterval, + ) + } + + // Determine where the bot should write its internal data (renewable cert + // etc) + botDestination, err := cfg.Storage.GetDestination() + if err != nil { + return trace.Wrap(err) + } + + ticker := time.NewTicker(cfg.RenewInterval) + defer ticker.Stop() + for { + newClient, newIdentity, err := renew(ctx, cfg, client, ident, botDestination) + if err != nil { + return trace.Wrap(err) } - log.Infof("Persisted new certificates to disk. Next renewal in approximately %s", cfg.RenewInterval) + client = newClient + ident = newIdentity select { case <-ctx.Done(): @@ -489,7 +695,15 @@ func renewLoop(ctx context.Context, cfg *config.BotConfig, client auth.ClientI, } } +// authenticatedUserClientFromIdentity creates a new auth client from the given +// identity. Note that depending on the connection address given, this may +// attempt to connect via the proxy and therefore requires both SSH and TLS +// credentials. func authenticatedUserClientFromIdentity(ctx context.Context, id *identity.Identity, authServer string) (auth.ClientI, error) { + if id.SSHCert == nil || id.X509Cert == nil { + return nil, trace.BadParameter("auth client requires a fully formed identity") + } + tlsConfig, err := id.TLSConfig(nil /* cipherSuites */) if err != nil { return nil, trace.Wrap(err) @@ -522,6 +736,7 @@ func generateImpersonatedIdentity( currentIdentity *identity.Identity, expires time.Time, roleRequests []string, + kinds []identity.ArtifactKind, ) (*identity.Identity, error) { // TODO: enforce expiration > renewal period (by what margin?) @@ -537,7 +752,7 @@ func generateImpersonatedIdentity( // expiration date. certs, err := client.GenerateUserCerts(ctx, proto.UserCertsRequest{ PublicKey: publicKey, - Username: currentIdentity.XCert.Subject.CommonName, + Username: currentIdentity.X509Cert.Subject.CommonName, Expires: expires, RoleRequests: roleRequests, }) @@ -566,11 +781,10 @@ func generateImpersonatedIdentity( certs.TLSCACerts = append(certs.TLSCACerts, pemBytes) } - newIdentity, err := identity.ReadIdentityFromKeyPair( - privateKey, - publicKey, - certs, - ) + newIdentity, err := identity.ReadIdentityFromStore(&identity.LoadIdentityParams{ + PrivateKeyBytes: privateKey, + PublicKeyBytes: publicKey, + }, certs, kinds...) if err != nil { return nil, trace.Wrap(err) } diff --git a/tool/tctl/common/bots_command.go b/tool/tctl/common/bots_command.go index 58e570cb2c882..59848f3bb8412 100644 --- a/tool/tctl/common/bots_command.go +++ b/tool/tctl/common/bots_command.go @@ -34,6 +34,7 @@ import ( "github.com/gravitational/teleport/lib/auth" "github.com/gravitational/teleport/lib/service" "github.com/gravitational/teleport/lib/tlsca" + "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/trace" ) @@ -135,10 +136,30 @@ func (c *BotsCommand) ListBots(client auth.ClientI) error { return nil } -var startMessageTemplate = template.Must(template.New("node").Parse(`The bot token: {{.token}} +// bold wraps the given text in an ANSI escape to bold it +func bold(text string) string { + return utils.Color(utils.Bold, text) +} + +var startMessageTemplate = template.Must(template.New("node").Funcs(template.FuncMap{ + "bold": bold, +}).Parse(`The bot token: {{.token}} This token will expire in {{.minutes}} minutes. -Run this on the new bot node to join the cluster: +Optionally, if running the bot under an isolated user account, first initialize +the data directory by running the following command {{ bold "as root" }}: + +> tbot init \ + --auth-server={{.auth_server}} \ + --destination-dir=./tbot-user \ + --bot-user=tbot \ + --reader-user=alice + +... where "tbot" is the username of the bot's UNIX user, and "alice" is the +UNIX user that will be making use of the certificates. + +Then, run this {{ bold "as the bot user" }} to begin continuously fetching +certificates: > tbot start \ --destination-dir=./tbot-user \ @@ -149,6 +170,9 @@ Run this on the new bot node to join the cluster: Please note: + - The ./tbot-user destination directory can be changed as desired. + - /var/lib/teleport/bot must be accessible to the bot user, or --data-dir + must point to another accessible directory to store internal bot data. - This invitation token will expire in {{.minutes}} minutes - {{.auth_server}} must be reachable from the new node `))