diff --git a/lib/tbot/botfs/botfs_test.go b/lib/tbot/botfs/botfs_test.go index 8bbc4ab1fa05f..0b93bf300954a 100644 --- a/lib/tbot/botfs/botfs_test.go +++ b/lib/tbot/botfs/botfs_test.go @@ -29,8 +29,7 @@ import ( func TestReadWrite(t *testing.T) { dir := t.TempDir() - secureWriteExpected, err := HasSecureWriteSupport() - require.NoError(t, err) + secureWriteExpected := HasSecureWriteSupport() expectedData := []byte{1, 2, 3, 4} diff --git a/lib/tbot/botfs/fs_linux.go b/lib/tbot/botfs/fs_linux.go index 1d9502edfb934..7d967287ed53b 100644 --- a/lib/tbot/botfs/fs_linux.go +++ b/lib/tbot/botfs/fs_linux.go @@ -422,10 +422,10 @@ func ConfigureACL(path string, owner *user.User, opts *ACLOptions) error { } // HasACLSupport determines if this binary / system supports ACLs. -func HasACLSupport() (bool, error) { +func HasACLSupport() bool { // We just assume Linux _can_ support ACLs here, and will test for support // at runtime. - return true, nil + return true } // HasSecureWriteSupport determines if `CreateSecure()` should be supported @@ -434,15 +434,16 @@ func HasACLSupport() (bool, error) { // // We've encountered this being incorrect in environments where access to the // kernel is hampered e.g. seccomp/apparmor/container runtimes. -func HasSecureWriteSupport() (bool, error) { +func HasSecureWriteSupport() bool { minKernel := semver.New(Openat2MinKernel) version, err := utils.KernelVersion() if err != nil { - return false, trace.Wrap(err) + log.WithError(err).Info("Failed to determine kernel version. It will be assumed secure write support is not available.") + return false } if version.LessThan(*minKernel) { - return false, nil + return false } - return true, nil + return true } diff --git a/lib/tbot/botfs/fs_other.go b/lib/tbot/botfs/fs_other.go index eff8e517eab8f..7136452e1c308 100644 --- a/lib/tbot/botfs/fs_other.go +++ b/lib/tbot/botfs/fs_other.go @@ -112,13 +112,13 @@ func ConfigureACL(path string, owner *user.User, opts *ACLOptions) error { // HasACLSupport determines if this binary / system supports ACLs. This // catch-all implementation just returns false. -func HasACLSupport() (bool, error) { - return false, nil +func HasACLSupport() bool { + return false } // 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 +func HasSecureWriteSupport() bool { + return false } diff --git a/lib/tbot/config/destination_directory.go b/lib/tbot/config/destination_directory.go index 2936e62998eb4..8df11dcb68027 100644 --- a/lib/tbot/config/destination_directory.go +++ b/lib/tbot/config/destination_directory.go @@ -69,16 +69,6 @@ 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 "": // We default to SymlinksTrySecure. It's become apparent that the @@ -89,13 +79,14 @@ func (dd *DestinationDirectory) CheckAndSetDefaults() error { case botfs.SymlinksInsecure, botfs.SymlinksTrySecure: // valid case botfs.SymlinksSecure: - if !secureSupported { + if !botfs.HasSecureWriteSupport() { return trace.BadParameter("symlink mode %q not supported on this system", dd.Symlinks) } default: return trace.BadParameter("invalid symlinks mode: %q", dd.Symlinks) } + aclsSupported := botfs.HasACLSupport() switch dd.ACLs { case "": if aclsSupported { @@ -162,11 +153,6 @@ func (dd *DestinationDirectory) Init(_ context.Context, subdirs []string) error } 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) @@ -189,7 +175,7 @@ func (dd *DestinationDirectory) Verify(keys []string) error { // 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 { + if dd.ACLs == botfs.ACLOff || !botfs.HasACLSupport() || ownedByBot { return nil } diff --git a/tool/tbot/init_test.go b/tool/tbot/init_test.go index 95fd66b813cef..3af569d376adf 100644 --- a/tool/tbot/init_test.go +++ b/tool/tbot/init_test.go @@ -165,9 +165,6 @@ func TestInitMaybeACLs(t *testing.T) { } require.NoError(t, err) - hasACLSupport, err := botfs.HasACLSupport() - require.NoError(t, err) - currentUser, err := user.Current() require.NoError(t, err) @@ -176,7 +173,7 @@ func TestInitMaybeACLs(t *testing.T) { // Determine if we expect init to use ACLs. expectACLs := false - if hasACLSupport { + if botfs.HasACLSupport() { if err := testACL(t.TempDir(), currentUser, opts); err == nil { expectACLs = true } @@ -227,9 +224,7 @@ outputs: // 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 { + if !botfs.HasSecureWriteSupport() { t.Skip("Secure write not supported on this system.") }