diff --git a/internal/workshop/lxd/lxd_backend.go b/internal/workshop/lxd/lxd_backend.go index ae52b0452..731c4df16 100644 --- a/internal/workshop/lxd/lxd_backend.go +++ b/internal/workshop/lxd/lxd_backend.go @@ -849,7 +849,7 @@ write_files: Description=Required for x11 support [Path] - PathChanged=/var/lib/workshop/run/ + PathModified=/var/lib/workshop/run/Xauthority/.Xauthority Unit=xauth-copy.service [Install] @@ -878,7 +878,7 @@ write_files: path: /etc/apt/apt.conf.d/01norecommend runcmd: - systemctl daemon-reload - - systemctl enable xauth-copy.service + - systemctl enable --now xauth-copy.service - systemctl enable --now xauth-watch.path ` diff --git a/internal/x11/x11.go b/internal/x11/x11.go index 44f1e4b6f..912845582 100644 --- a/internal/x11/x11.go +++ b/internal/x11/x11.go @@ -13,8 +13,8 @@ import ( ) // Copies the user's $XAUTHORITY file to the Workshopd run directory. -func MigrateXauthority(user *user.User, xauth string) (err error) { - if xauth == "" { +func MigrateXauthority(user *user.User, xauthPath string) (err error) { + if xauthPath == "" { return fmt.Errorf("xauth cannot be empty") } @@ -36,7 +36,7 @@ func MigrateXauthority(user *user.User, xauth string) (err error) { // since the user can just steal it without having to use workshop. This code // is just to ensure that a user who doesn't have those privileges can't // steal the file via 'workshop connect' - f, err := os.Stat(xauth) + f, err := os.Stat(xauthPath) if err != nil { return err } @@ -51,8 +51,25 @@ func MigrateXauthority(user *user.User, xauth string) (err error) { return fmt.Errorf("Xauthority file isn't owned by the current user %s", user.Uid) } - destFile := filepath.Join(destDir, ".Xauthority") - err = osutil.CopyFile(xauth, filepath.Join(destDir, ".Xauthority"), osutil.CopyFlagOverwrite) + xauth, err := os.Open(xauthPath) + if err != nil { + return err + } + defer xauth.Close() + + xauthEntries, err := ProcessFile(xauth) + if err != nil { + return err + } + + for i := range xauthEntries { + xauthEntries[i].Family = FamilyWild + xauthEntries[i].Host = []byte("workshop") + } + + b := EncodeEntries(xauthEntries) + + err = os.WriteFile(filepath.Join(destDir, ".Xauthority"), b, 0644) if err != nil { return err } @@ -62,7 +79,7 @@ func MigrateXauthority(user *user.User, xauth string) (err error) { return err } - if err = sys.ChownPath(destFile, uid, gid); err != nil { + if err = sys.ChownPath(filepath.Join(destDir, ".Xauthority"), uid, gid); err != nil { return err } diff --git a/internal/x11/x11_test.go b/internal/x11/x11_test.go index 890ea78d3..d6a0eedd8 100644 --- a/internal/x11/x11_test.go +++ b/internal/x11/x11_test.go @@ -37,7 +37,7 @@ func (x *X11TestSuit) TearDownTest(c *check.C) { x.restore() } -func (x *X11TestSuit) TestMigrateXAuthoritySuccess(c *check.C) { +func (x *X11TestSuit) TestMigrateXAuthority(c *check.C) { user, err := user.Current() c.Assert(err, check.IsNil) @@ -45,12 +45,29 @@ func (x *X11TestSuit) TestMigrateXAuthoritySuccess(c *check.C) { c.Assert(err, check.IsNil) defer xf.Close() + _, err = xf.Write(constructTestCookie(displayNone)) + c.Assert(err, check.IsNil) + err = x11.MigrateXauthority(user, filepath.Join(dirs.WorkshopdRunDir, ".workshop-Xauthority")) c.Assert(err, check.IsNil) c.Assert(filepath.Join(dirs.WorkshopdRunDir, user.Uid, "Xauthority", ".Xauthority"), testutil.FilePresent) } +func (x *X11TestSuit) TestMigrateXAuthorityInvalidFile(c *check.C) { + user, err := user.Current() + c.Assert(err, check.IsNil) + + xf, err := os.Create(filepath.Join(dirs.WorkshopdRunDir, ".workshop-Xauthority")) + c.Assert(err, check.IsNil) + defer xf.Close() + + err = x11.MigrateXauthority(user, filepath.Join(dirs.WorkshopdRunDir, ".workshop-Xauthority")) + c.Assert(err, check.ErrorMatches, "invalid Xauthority file: EOF") + + c.Assert(filepath.Join(dirs.WorkshopdRunDir, user.Uid, "Xauthority", ".Xauthority"), testutil.FileAbsent) +} + func (x *X11TestSuit) TestMigrateXAuthorityOwnershipFail(c *check.C) { user, err := user.Lookup("root") c.Assert(err, check.IsNil) diff --git a/internal/x11/xauth.go b/internal/x11/xauth.go new file mode 100644 index 000000000..8ca449bda --- /dev/null +++ b/internal/x11/xauth.go @@ -0,0 +1,100 @@ +package x11 + +import ( + "encoding/binary" + "fmt" + "io" +) + +type Family uint16 + +const ( + FamilyIpv4 Family = 0 + FamilyDECnet Family = 1 + FamilyChaos Family = 2 + FamilyServerInterpreted Family = 5 + FamilyIpv6 Family = 6 + FamilyLocalhost Family = 252 + FamilyKerberos Family = 253 + FamilyNetName Family = 254 + FamilyLocal Family = 256 + FamilyWild Family = 65535 +) + +type Entry struct { + Family Family + Host []byte + Display []byte + AuthorisationMethod []byte + AuthorisationData []byte +} + +func ProcessFile(r io.Reader) ([]Entry, error) { + var entries []Entry + + family := make([]byte, 2) + for { + var entry Entry + _, err := r.Read(family) + if err != nil { + if len(entries) == 0 { + return nil, fmt.Errorf("invalid Xauthority file: %w", err) + } + return entries, nil + } + + entry.Family = Family(binary.BigEndian.Uint16(family)) + err = readNext(r, &entry.Host) + if err != nil { + return nil, fmt.Errorf("invalid Xauthority file: %w", err) + } + + err = readNext(r, &entry.Display) + if err != nil { + return nil, fmt.Errorf("invalid Xauthority file: %w", err) + } + + err = readNext(r, &entry.AuthorisationMethod) + if err != nil { + return nil, fmt.Errorf("invalid Xauthority file: %w", err) + } + + err = readNext(r, &entry.AuthorisationData) + if err != nil { + return nil, fmt.Errorf("invalid Xauthority file: %w", err) + } + + entries = append(entries, entry) + } +} + +func EncodeEntries(entries []Entry) []byte { + var b []byte + for _, e := range entries { + b = binary.BigEndian.AppendUint16(b, uint16(e.Family)) + b = binary.BigEndian.AppendUint16(b, uint16(len(e.Host))) + b = append(b, e.Host...) + b = binary.BigEndian.AppendUint16(b, uint16(len(e.Display))) + b = append(b, e.Display...) + b = binary.BigEndian.AppendUint16(b, uint16(len(e.AuthorisationMethod))) + b = append(b, e.AuthorisationMethod...) + b = binary.BigEndian.AppendUint16(b, uint16(len(e.AuthorisationData))) + b = append(b, e.AuthorisationData...) + } + return b +} + +func readNext(r io.Reader, dest *[]byte) error { + l := make([]byte, 2) + _, err := r.Read(l) + if err != nil { + return err + } + + *dest = make([]byte, binary.BigEndian.Uint16(l)) + _, err = r.Read(*dest) + if err != nil { + return err + } + return nil +} diff --git a/internal/x11/xauth_test.go b/internal/x11/xauth_test.go new file mode 100644 index 000000000..e33cdf5b8 --- /dev/null +++ b/internal/x11/xauth_test.go @@ -0,0 +1,77 @@ +package x11_test + +import ( + "bytes" + "encoding/binary" + + "gopkg.in/check.v1" + + "github.com/canonical/workshop/internal/x11" +) + +var ( + // len (16) + 'MIT_MAGIC_COOKIE-1' in hex + magicCookie = []byte{0x00, 0x12, 0x4d, 0x49, 0x54, 0x5f, 0x4d, 0x41, 0x47, 0x49, 0x43, 0x5f, 0x43, 0x4f, 0x4f, 0x4b, 0x49, 0x45, 0x2d, 0x31} + // len (14) + auth data in hex + magicCookieAuth = []byte{0x00, 0x10, 0xdc, 0x71, 0xd2, 0xe2, 0x0a, 0x8b, 0x64, 0xbd, 0x78, 0x4e, 0xcb, 0x1b, 0xcf, 0xdc, 0xe4, 0x12} + // len (7) + 'example' in hex + hostExample = []byte{0x00, 0x07, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65} + // len (1) + '0' in hex + displayZero = []byte{0x00, 0x01, 0x30} + // len (0), null display + displayNone = []byte{0x00, 0x00} +) + +func (x *X11TestSuit) TestProcessFile(c *check.C) { + cookie := constructTestCookie(displayZero) + + entries, err := x11.ProcessFile(bytes.NewReader(cookie)) + c.Assert(err, check.IsNil) + c.Check(entries[0].Family, check.Equals, x11.FamilyLocal) + c.Check(entries[0].Host, check.DeepEquals, hostExample[2:]) + c.Check(entries[0].Display, check.DeepEquals, []byte{0x30}) + c.Check(entries[0].AuthorisationMethod, check.DeepEquals, magicCookie[2:]) + c.Check(entries[0].AuthorisationData, check.DeepEquals, magicCookieAuth[2:]) +} + +func (x *X11TestSuit) TestProcessFileNoDisplay(c *check.C) { + cookie := constructTestCookie(displayNone) + + entries, err := x11.ProcessFile(bytes.NewReader(cookie)) + c.Assert(err, check.IsNil) + c.Check(entries[0].Family, check.Equals, x11.FamilyLocal) + c.Check(entries[0].Host, check.DeepEquals, hostExample[2:]) + c.Check(entries[0].Display, check.DeepEquals, []byte{}) + c.Check(entries[0].AuthorisationMethod, check.DeepEquals, magicCookie[2:]) + c.Check(entries[0].AuthorisationData, check.DeepEquals, magicCookieAuth[2:]) +} + +func (x *X11TestSuit) TestEncodeEntries(c *check.C) { + cookie := constructTestCookie(displayZero) + + entries, err := x11.ProcessFile(bytes.NewReader(cookie)) + c.Assert(err, check.IsNil) + + encoded := x11.EncodeEntries(entries) + c.Assert(cookie, check.DeepEquals, encoded) +} + +func (x *X11TestSuit) TestEncodeEntriesNoDisplay(c *check.C) { + cookie := constructTestCookie(displayNone) + + entries, err := x11.ProcessFile(bytes.NewReader(cookie)) + c.Assert(err, check.IsNil) + + encoded := x11.EncodeEntries(entries) + c.Assert(cookie, check.DeepEquals, encoded) +} + +func constructTestCookie(display []byte) []byte { + // Construct a fake, however 'correct' cookie + cookie := binary.BigEndian.AppendUint16([]byte{}, uint16(x11.FamilyLocal)) + cookie = append(cookie, hostExample...) + cookie = append(cookie, display...) + cookie = append(cookie, magicCookie...) + cookie = append(cookie, magicCookieAuth...) + return cookie +}