fix(landscape): Restrict landscape config file from 0604 to 0640 (#700)

The landscape group needs read-access to the /etc/landscape/client.conf
file. However, we do not want any user to be able to read it as it may
contain confidential details such as the registration key. The solution
is to restrict its visibility to its owner and group, and to add it to
landscape's group

Thanks @iosifache for spotting this vulnerability.

---

UDENG-2540

wsl-pro-service/internal/system/backend.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"os"
 	"os/exec"
+	"os/user"
 	"path/filepath"
 )
 
@@ -50,3 +51,7 @@ func (b realBackend) CmdExe(ctx context.Context, path string, args ...string) *e
 
 	return cmd
 }
+
+func (b realBackend) LookupGroup(name string) (*user.Group, error) {
+	return user.LookupGroup("landscape")
+}

wsl-pro-service/internal/system/landscape.go

@@ -51,18 +51,33 @@ func (s *System) LandscapeDisable(ctx context.Context) (err error) {
 func (s *System) writeConfig(landscapeConfig string) (err error) {
 	defer decorate.OnError(&err, "could not write Landscape configuration")
 
+	userID, err := s.currentUser()
+	if err != nil {
+		return err
+	}
+
+	groupID, err := s.groupToGUID("landscape")
+	if err != nil {
+		return err
+	}
+
 	tmp := s.backend.Path(landscapeConfigPath + ".new")
 	final := s.backend.Path(landscapeConfigPath)
 
 	if err := os.MkdirAll(filepath.Dir(tmp), 0750); err != nil {
 		return fmt.Errorf("could not create config directory: %v", err)
 	}
 
-	//nolint:gosec // Needs 0604 for the Landscape client to be able to read it
-	if err = os.WriteFile(tmp, []byte(landscapeConfig), 0604); err != nil {
+	//nolint:gosec // Needs 0640 for the landscape client to be able to read it.
+	if err := os.WriteFile(tmp, []byte(landscapeConfig), 0640); err != nil {
 		return fmt.Errorf("could not write to file: %v", err)
 	}
 
+	if err := os.Chown(tmp, userID, groupID); err != nil {
+		_ = os.RemoveAll(tmp)
+		return fmt.Errorf("could not change ownership to landscape group: %v", err)
+	}
+
 	if err := os.Rename(tmp, final); err != nil {
 		_ = os.RemoveAll(tmp)
 		return err

wsl-pro-service/internal/system/system.go

@@ -6,9 +6,12 @@ import (
 	"bufio"
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
+	"os/user"
+	"strconv"
 	"strings"
 
 	agentapi "github.com/canonical/ubuntu-pro-for-wsl/agentapi/go"
@@ -34,6 +37,7 @@ type Backend interface {
 	Path(p ...string) string
 	Hostname() (string, error)
 	GetenvWslDistroName() string
+	LookupGroup(string) (*user.Group, error)
 
 	ProExecutable(ctx context.Context, args ...string) *exec.Cmd
 	LandscapeConfigExecutable(ctx context.Context, args ...string) *exec.Cmd
@@ -274,3 +278,33 @@ func (s *System) findCmdExe() (cmdExe string, err error) {
 
 	return "", fmt.Errorf("none of the mounted drives contains subpath %s", subPath)
 }
+
+// groupToGUID searches the group with the specified name and returns its GID.
+func (s *System) groupToGUID(name string) (int, error) {
+	group, err := s.backend.LookupGroup(name)
+	if err != nil {
+		return 0, err
+	}
+
+	guid, err := strconv.ParseInt(group.Gid, 10, 32)
+	if err != nil {
+		return 0, errors.New("could not parse %s as an integer")
+	}
+
+	return int(guid), nil
+}
+
+// currentUser returns the UID of the current user.
+func (s *System) currentUser() (int, error) {
+	user, err := user.Current()
+	if err != nil {
+		return 0, err
+	}
+
+	userID, err := strconv.ParseInt(user.Uid, 10, 32)
+	if err != nil {
+		return 0, errors.New("could not parse %s as an integer")
+	}
+
+	return int(userID), nil
+}

wsl-pro-service/internal/system/system_test.go

@@ -393,6 +393,7 @@ func TestLandscapeEnable(t *testing.T) {
 		breakWriteConfig     bool
 		breakLandscapeConfig bool
 		breakWSLPath         bool
+		noLandscapeGroup     bool
 
 		wantErr bool
 	}{
@@ -406,6 +407,7 @@ func TestLandscapeEnable(t *testing.T) {
 		"Error when the config file cannot be written":           {breakWriteConfig: true, wantErr: true},
 		"Error when the landscape-config command fails":          {breakLandscapeConfig: true, wantErr: true},
 		"Error when failing to override the SSL certficate path": {breakWSLPath: true, wantErr: true},
+		"Error when the Landscape user does not exist":           {noLandscapeGroup: true, wantErr: true},
 	}
 
 	for name, tc := range testCases {
@@ -428,6 +430,10 @@ func TestLandscapeEnable(t *testing.T) {
 				mock.SetControlArg(testutils.WslpathErr)
 			}
 
+			if tc.noLandscapeGroup {
+				mock.LandscapeGroupGID = ""
+			}
+
 			config, err := os.ReadFile(filepath.Join(commontestutils.TestFixturePath(t), "landscape.conf"))
 			require.NoError(t, err, "Setup: could not load fixture")
 

wsl-pro-service/internal/system/testdata/TestLandscapeEnable/error_when_the_landscape_user_does_not_exist/landscape.conf

@@ -0,0 +1,5 @@
+[host]
+url = www.example.com
+
+[client]
+hello = world

wsl-pro-service/internal/testutils/mock_executables.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"os/user"
 	"path/filepath"
 	"strings"
 	"syscall"
@@ -36,6 +37,9 @@ type SystemMock struct {
 	// string when false
 	WslDistroNameEnvEnabled bool
 
+	// LookupGroupError makes the LookupGroup function fail.
+	LandscapeGroupGID string
+
 	// extraEnv are extra environment variables that will be passed to mocked executables
 	extraEnv []string
 }
@@ -114,12 +118,16 @@ const (
 func MockSystem(t *testing.T) (system.System, *SystemMock) {
 	t.Helper()
 
+	u, err := user.Current()
+	require.NoError(t, err, "Setup: could not get current user")
+
 	distroHostname := "TEST_DISTRO_HOSTNAME"
 	mock := &SystemMock{
 		FsRoot:                  mockFilesystemRoot(t),
 		WslDistroName:           "TEST_DISTRO",
 		DistroHostname:          &distroHostname,
 		WslDistroNameEnvEnabled: true,
+		LandscapeGroupGID:       u.Gid,
 	}
 
 	return system.New(system.WithTestBackend(mock)), mock
@@ -158,6 +166,22 @@ func (m *SystemMock) GetenvWslDistroName() string {
 	return ""
 }
 
+// LookupGroup mocks the user.LookupGroup function.
+func (m *SystemMock) LookupGroup(name string) (*user.Group, error) {
+	if name != "landscape" {
+		return nil, fmt.Errorf("mock does not support group %q", name)
+	}
+
+	if m.LandscapeGroupGID == "" {
+		return nil, user.UnknownGroupError(name)
+	}
+
+	return &user.Group{
+		Gid:  m.LandscapeGroupGID,
+		Name: name,
+	}, nil
+}
+
 // mockExec generates a command of the form `bash -ec <SCRIPT>` that will call an alternate binary
 // to the one we are mocking.
 //