From 0801b8841776e7a08d2a0826bbca535310b02f37 Mon Sep 17 00:00:00 2001 From: Sam Berning <113054166+sam-berning@users.noreply.github.com> Date: Mon, 17 Apr 2023 14:16:26 -0700 Subject: [PATCH] fix: explicitly set permissions for socket_vmnet dependencies (#363) Issue #, if available: https://github.com/runfinch/finch/issues/57 *Description of changes:* Sets permissions explicitly for socket_vmnet dependencies, which will override a user's `umask` to ensure that Finch can use the files as needed. *Testing done:* ``` $ umask 077 $ make $ sudo rm -rf /opt/finch/bin/socket_vmnet* $ sudo rm -rf /etc/sudoers.d/finch-lima $ ./_output/bin/finch vm init ``` unit testing - [x] I've reviewed the guidance in CONTRIBUTING.md #### License Acceptance By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. Signed-off-by: Sam Berning --- pkg/dependency/vmnet/binaries.go | 8 ++++++++ pkg/dependency/vmnet/binaries_test.go | 17 ++++++++++++++++- pkg/dependency/vmnet/sudoers_file.go | 6 ++++++ pkg/dependency/vmnet/sudoers_file_test.go | 21 +++++++++++++++++++++ 4 files changed, 51 insertions(+), 1 deletion(-) diff --git a/pkg/dependency/vmnet/binaries.go b/pkg/dependency/vmnet/binaries.go index 02de29929..8c65323de 100644 --- a/pkg/dependency/vmnet/binaries.go +++ b/pkg/dependency/vmnet/binaries.go @@ -122,6 +122,14 @@ func (bin *binaries) Install() error { return fmt.Errorf("error changing owner of files in directory %s, err: %w", installationPathBinDir, err) } + // in most cases this should not be needed, as the permissions should be copied from the build artifacts, + // but the permissions can end up being wrong when Finch is built on a shell that has a restrictive umask. + chmodBinCmd := bin.cmdCreator.Create("sudo", "chmod", "755", bin.installationPathSocketVmnetExe()) + _, err = chmodBinCmd.Output() + if err != nil { + return fmt.Errorf("error setting correct permissions for socket_vmnet binary, err: %w", err) + } + return nil } diff --git a/pkg/dependency/vmnet/binaries_test.go b/pkg/dependency/vmnet/binaries_test.go index 5d386f1f4..af5066455 100644 --- a/pkg/dependency/vmnet/binaries_test.go +++ b/pkg/dependency/vmnet/binaries_test.go @@ -176,12 +176,13 @@ func TestBinaries_Install(t *testing.T) { { name: "happy path", mockSvc: func(cmd *mocks.Command, creator *mocks.CommandCreator) { - cmd.EXPECT().Output().Times(4) + cmd.EXPECT().Output().Times(5) creator.EXPECT().Create("sudo", "mkdir", "-p", "/opt/finch").Return(cmd) creator.EXPECT().Create("sudo", "cp", "-rp", "mock_prefix/dependencies/lima-socket_vmnet/opt/finch", "/opt").Return(cmd) creator.EXPECT().Create("sudo", "chown", "root:wheel", "/opt/finch").Return(cmd) creator.EXPECT().Create("sudo", "chown", "-R", "root:wheel", "/opt/finch/bin").Return(cmd) + creator.EXPECT().Create("sudo", "chmod", "755", "/opt/finch/bin/socket_vmnet").Return(cmd) }, want: nil, }, @@ -230,6 +231,20 @@ func TestBinaries_Install(t *testing.T) { }, want: fmt.Errorf("error changing owner of files in directory %s, err: %w", "/opt/finch/bin", errors.New("chown -R error")), }, + { + name: "sudo chmod of the binary throws an error", + mockSvc: func(cmd *mocks.Command, creator *mocks.CommandCreator) { + cmd.EXPECT().Output().Times(4) + cmd.EXPECT().Output().Return([]byte{}, errors.New("sudo chmod error")) + + creator.EXPECT().Create("sudo", "mkdir", "-p", "/opt/finch").Return(cmd) + creator.EXPECT().Create("sudo", "cp", "-rp", "mock_prefix/dependencies/lima-socket_vmnet/opt/finch", "/opt").Return(cmd) + creator.EXPECT().Create("sudo", "chown", "root:wheel", "/opt/finch").Return(cmd) + creator.EXPECT().Create("sudo", "chown", "-R", "root:wheel", "/opt/finch/bin").Return(cmd) + creator.EXPECT().Create("sudo", "chmod", "755", "/opt/finch/bin/socket_vmnet").Return(cmd) + }, + want: fmt.Errorf("error setting correct permissions for socket_vmnet binary, err: %w", errors.New("sudo chmod error")), + }, } for _, tc := range testCases { diff --git a/pkg/dependency/vmnet/sudoers_file.go b/pkg/dependency/vmnet/sudoers_file.go index 61636b1a6..e5f73208e 100644 --- a/pkg/dependency/vmnet/sudoers_file.go +++ b/pkg/dependency/vmnet/sudoers_file.go @@ -75,6 +75,12 @@ func (s *sudoersFile) Install() error { return fmt.Errorf("failed to write to the sudoers file: %w", err) } + // explicitly set permissions to ensure that the file is readable by the user + _, err = s.execCmdCreator.Create("sudo", "chmod", "644", s.path()).Output() + if err != nil { + return fmt.Errorf("failed to set correct permissions for sudoers file: %w", err) + } + return nil } diff --git a/pkg/dependency/vmnet/sudoers_file_test.go b/pkg/dependency/vmnet/sudoers_file_test.go index 9f580a674..da3068822 100644 --- a/pkg/dependency/vmnet/sudoers_file_test.go +++ b/pkg/dependency/vmnet/sudoers_file_test.go @@ -133,6 +133,8 @@ func TestSudoers_Install(t *testing.T) { ec.EXPECT().Create("sudo", "tee", "/etc/sudoers.d/finch-lima").Return(cmd) cmd.EXPECT().SetStdin(bytes.NewReader(mockSudoersOut)) cmd.EXPECT().Output().Return(mockSudoersOut, nil) + ec.EXPECT().Create("sudo", "chmod", "644", "/etc/sudoers.d/finch-lima").Return(cmd) + cmd.EXPECT().Output().Return([]byte{}, nil) }, want: nil, }, @@ -166,6 +168,25 @@ func TestSudoers_Install(t *testing.T) { }, want: fmt.Errorf("failed to write to the sudoers file: %w", errors.New("sudo tee command error")), }, + { + name: "sudo chmod command throws err", + mockSvc: func(t *testing.T, cmd *mocks.Command, mFs afero.Fs, ec *mocks.CommandCreator, lc *mocks.LimaCmdCreator) { + sudoersData := []byte("test data") + mockSudoersOut := []byte("mock_sudoers_out") + + err := afero.WriteFile(mFs, "/etc/sudoers.d/finch-lima", sudoersData, 0o666) + require.NoError(t, err) + + lc.EXPECT().CreateWithoutStdio("sudoers").Return(cmd) + cmd.EXPECT().Output().Return(mockSudoersOut, nil) + ec.EXPECT().Create("sudo", "tee", "/etc/sudoers.d/finch-lima").Return(cmd) + cmd.EXPECT().SetStdin(bytes.NewReader(mockSudoersOut)) + cmd.EXPECT().Output().Return(mockSudoersOut, nil) + ec.EXPECT().Create("sudo", "chmod", "644", "/etc/sudoers.d/finch-lima").Return(cmd) + cmd.EXPECT().Output().Return([]byte{}, errors.New("sudo chmod command error")) + }, + want: fmt.Errorf("failed to set correct permissions for sudoers file: %w", errors.New("sudo chmod command error")), + }, } for _, tc := range testCases {