diff --git a/tool/tbot/botfs/botfs.go b/tool/tbot/botfs/botfs.go index ceacbb49fd071..34e3f4b279c39 100644 --- a/tool/tbot/botfs/botfs.go +++ b/tool/tbot/botfs/botfs.go @@ -76,6 +76,10 @@ const ( // Directories need the execute bit set for most operations on their // contents to succeed. DefaultDirMode fs.FileMode = 0700 + + // OpenMode is the mode with which files should be opened for reading and + // writing. + OpenMode int = os.O_CREATE | os.O_RDWR ) // ACLOptions contains parameters needed to configure ACLs @@ -88,9 +92,10 @@ type ACLOptions struct { ReaderUser *user.User } -// openStandard attempts to open the given path for writing with O_CREATE set. +// openStandard attempts to open the given path for reading and writing with +// O_CREATE set. func openStandard(path string) (*os.File, error) { - file, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, DefaultMode) + file, err := os.OpenFile(path, OpenMode, DefaultMode) if err != nil { return nil, trace.ConvertSystemError(err) } diff --git a/tool/tbot/botfs/botfs_test.go b/tool/tbot/botfs/botfs_test.go new file mode 100644 index 0000000000000..8bbc4ab1fa05f --- /dev/null +++ b/tool/tbot/botfs/botfs_test.go @@ -0,0 +1,56 @@ +/* +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 ( + "bytes" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestReadWrite attempts to test read/write against all possible symlink +// modes. +func TestReadWrite(t *testing.T) { + dir := t.TempDir() + + secureWriteExpected, err := HasSecureWriteSupport() + require.NoError(t, err) + + expectedData := []byte{1, 2, 3, 4} + + for _, mode := range []SymlinksMode{SymlinksInsecure, SymlinksTrySecure, SymlinksSecure} { + if mode == SymlinksSecure && !secureWriteExpected { + t.Logf("skipping secure read/write test due to lack of platform support") + continue + } + + path := filepath.Join(dir, string(mode)) + + err := Create(path, false, mode) + require.NoError(t, err) + + err = Write(path, expectedData, mode) + require.NoError(t, err) + + data, err := Read(path, mode) + require.NoError(t, err) + + require.Equal(t, 0, bytes.Compare(data, expectedData), "read bytes must be equal to those written") + } +} diff --git a/tool/tbot/botfs/fs_linux.go b/tool/tbot/botfs/fs_linux.go index 835fc6ab0ca95..c0e010569c40a 100644 --- a/tool/tbot/botfs/fs_linux.go +++ b/tool/tbot/botfs/fs_linux.go @@ -63,7 +63,7 @@ func openSecure(path string) (*os.File, error) { // 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, + Flags: uint64(OpenMode), Resolve: unix.RESOLVE_NO_SYMLINKS, } diff --git a/tool/tbot/identity/artifact.go b/tool/tbot/identity/artifact.go index 840c491500a13..7716a177603ad 100644 --- a/tool/tbot/identity/artifact.go +++ b/tool/tbot/identity/artifact.go @@ -28,6 +28,7 @@ type Artifact struct { Kind ArtifactKind ToBytes func(*Identity) []byte FromBytes func(*proto.Certs, *LoadIdentityParams, []byte) + Optional bool } // Matches returns true if this artifact's Kind matches any one of the given @@ -136,6 +137,7 @@ var artifacts = []Artifact{ FromBytes: func(c *proto.Certs, p *LoadIdentityParams, b []byte) { // nothing to do }, + Optional: true, }, } diff --git a/tool/tbot/identity/identity.go b/tool/tbot/identity/identity.go index 33cd4a050d936..5543931cf4479 100644 --- a/tool/tbot/identity/identity.go +++ b/tool/tbot/identity/identity.go @@ -432,6 +432,14 @@ func LoadIdentity(d destination.Destination, kinds ...ArtifactKind) (*Identity, return nil, trace.WrapWithMessage(err, "could not read artifact %q from destination %s", artifact.Key, d) } + // We generally expect artifacts to exist beforehand regardless of + // whether or not they've actually been written to (due to `tbot init` + // or our lightweight init during `tbot start`). If required artifacts + // are empty, this identity can't be loaded. + if !artifact.Optional && len(data) == 0 { + return nil, trace.NotFound("artifact %q is unexpectedly empty in destination %s", artifact.Key, d) + } + artifact.FromBytes(&certs, ¶ms, data) } diff --git a/tool/tbot/identity_test.go b/tool/tbot/identity_test.go new file mode 100644 index 0000000000000..d7eb662ff50a0 --- /dev/null +++ b/tool/tbot/identity_test.go @@ -0,0 +1,42 @@ +/* +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. +*/ + +// Note: this lives in main to avoid import cycles since this depends on the +// config/identity/destinations packages. + +package main + +import ( + "testing" + + "github.com/gravitational/teleport/tool/tbot/config" + "github.com/gravitational/teleport/tool/tbot/identity" + "github.com/gravitational/trace" + "github.com/stretchr/testify/require" +) + +func TestLoadEmptyIdentity(t *testing.T) { + dir := t.TempDir() + dest := config.DestinationDirectory{ + Path: dir, + } + require.NoError(t, dest.CheckAndSetDefaults()) + + _, err := identity.LoadIdentity(&dest, identity.BotKinds()...) + require.Error(t, err) + + require.True(t, trace.IsNotFound(err)) +} diff --git a/tool/tbot/main.go b/tool/tbot/main.go index 484a73f97caeb..f428a1c6a42a9 100644 --- a/tool/tbot/main.go +++ b/tool/tbot/main.go @@ -208,11 +208,14 @@ func onStart(botConfig *config.BotConfig) error { 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.") + } else if trace.IsNotFound(err) { + // This is _probably_ a fresh start, so we'll log the true error + // and try to fetch a fresh identity. + log.Debugf("Identity %s is not found or empty and could not be loaded, will start from scratch: %+v", dest, err) + } else { + return trace.Wrap(err) } - // 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)