diff --git a/agent/agentssh/x11.go b/agent/agentssh/x11.go index 00c2819cc0155..462bc1042bba9 100644 --- a/agent/agentssh/x11.go +++ b/agent/agentssh/x11.go @@ -6,6 +6,7 @@ import ( "encoding/hex" "errors" "fmt" + "io" "net" "os" "path/filepath" @@ -141,7 +142,7 @@ func addXauthEntry(ctx context.Context, fs afero.Fs, host string, display string } // Open or create the Xauthority file - file, err := fs.OpenFile(xauthPath, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0o600) + file, err := fs.OpenFile(xauthPath, os.O_RDWR|os.O_CREATE, 0o600) if err != nil { return xerrors.Errorf("failed to open Xauthority file: %w", err) } @@ -153,7 +154,105 @@ func addXauthEntry(ctx context.Context, fs afero.Fs, host string, display string return xerrors.Errorf("failed to decode auth cookie: %w", err) } - // Write Xauthority entry + // Read the Xauthority file and look for an existing entry for the host, + // display, and auth protocol. If an entry is found, overwrite the auth + // cookie (if it fits). Otherwise, mark the entry for deletion. + type deleteEntry struct { + start, end int + } + var deleteEntries []deleteEntry + pos := 0 + updated := false + for { + entry, err := readXauthEntry(file) + if err != nil { + if errors.Is(err, io.EOF) { + break + } + return xerrors.Errorf("failed to read Xauthority entry: %w", err) + } + + nextPos := pos + entry.Len() + cookieStartPos := nextPos - len(entry.authCookie) + + if entry.family == 0x0100 && entry.address == host && entry.display == display && entry.authProtocol == authProtocol { + if !updated && len(entry.authCookie) == len(authCookieBytes) { + // Overwrite the auth cookie + _, err := file.WriteAt(authCookieBytes, int64(cookieStartPos)) + if err != nil { + return xerrors.Errorf("failed to write auth cookie: %w", err) + } + updated = true + } else { + // Mark entry for deletion. + if len(deleteEntries) > 0 && deleteEntries[len(deleteEntries)-1].end == pos { + deleteEntries[len(deleteEntries)-1].end = nextPos + } else { + deleteEntries = append(deleteEntries, deleteEntry{ + start: pos, + end: nextPos, + }) + } + } + } + + pos = nextPos + } + + // In case the magic cookie changed, or we've previously bloated the + // Xauthority file, we may have to delete entries. + if len(deleteEntries) > 0 { + // Read the entire file into memory. This is not ideal, but it's the + // simplest way to delete entries from the middle of the file. The + // Xauthority file is small, so this should be fine. + _, err = file.Seek(0, io.SeekStart) + if err != nil { + return xerrors.Errorf("failed to seek Xauthority file: %w", err) + } + data, err := io.ReadAll(file) + if err != nil { + return xerrors.Errorf("failed to read Xauthority file: %w", err) + } + + // Delete the entries in reverse order. + for i := len(deleteEntries) - 1; i >= 0; i-- { + entry := deleteEntries[i] + // Safety check: ensure the entry is still there. + if entry.start > len(data) || entry.end > len(data) { + continue + } + data = append(data[:entry.start], data[entry.end:]...) + } + + // Write the data back to the file. + _, err = file.Seek(0, io.SeekStart) + if err != nil { + return xerrors.Errorf("failed to seek Xauthority file: %w", err) + } + _, err = file.Write(data) + if err != nil { + return xerrors.Errorf("failed to write Xauthority file: %w", err) + } + + // Truncate the file. + err = file.Truncate(int64(len(data))) + if err != nil { + return xerrors.Errorf("failed to truncate Xauthority file: %w", err) + } + } + + // Return if we've already updated the entry. + if updated { + return nil + } + + // Ensure we're at the end (append). + _, err = file.Seek(0, io.SeekEnd) + if err != nil { + return xerrors.Errorf("failed to seek Xauthority file: %w", err) + } + + // Append Xauthority entry. family := uint16(0x0100) // FamilyLocal err = binary.Write(file, binary.BigEndian, family) if err != nil { @@ -198,3 +297,96 @@ func addXauthEntry(ctx context.Context, fs afero.Fs, host string, display string return nil } + +// xauthEntry is an representation of an Xauthority entry. +// +// The Xauthority file format is as follows: +// +// - 16-bit family +// - 16-bit address length +// - address +// - 16-bit display length +// - display +// - 16-bit auth protocol length +// - auth protocol +// - 16-bit auth cookie length +// - auth cookie +type xauthEntry struct { + family uint16 + address string + display string + authProtocol string + authCookie []byte +} + +func (e xauthEntry) Len() int { + // 5 * uint16 = 10 bytes for the family/length fields. + return 2*5 + len(e.address) + len(e.display) + len(e.authProtocol) + len(e.authCookie) +} + +func readXauthEntry(r io.Reader) (xauthEntry, error) { + var entry xauthEntry + + // Read family + err := binary.Read(r, binary.BigEndian, &entry.family) + if err != nil { + return xauthEntry{}, xerrors.Errorf("failed to read family: %w", err) + } + + // Read address + var addressLength uint16 + err = binary.Read(r, binary.BigEndian, &addressLength) + if err != nil { + return xauthEntry{}, xerrors.Errorf("failed to read address length: %w", err) + } + + addressBytes := make([]byte, addressLength) + _, err = r.Read(addressBytes) + if err != nil { + return xauthEntry{}, xerrors.Errorf("failed to read address: %w", err) + } + entry.address = string(addressBytes) + + // Read display + var displayLength uint16 + err = binary.Read(r, binary.BigEndian, &displayLength) + if err != nil { + return xauthEntry{}, xerrors.Errorf("failed to read display length: %w", err) + } + + displayBytes := make([]byte, displayLength) + _, err = r.Read(displayBytes) + if err != nil { + return xauthEntry{}, xerrors.Errorf("failed to read display: %w", err) + } + entry.display = string(displayBytes) + + // Read auth protocol + var authProtocolLength uint16 + err = binary.Read(r, binary.BigEndian, &authProtocolLength) + if err != nil { + return xauthEntry{}, xerrors.Errorf("failed to read auth protocol length: %w", err) + } + + authProtocolBytes := make([]byte, authProtocolLength) + _, err = r.Read(authProtocolBytes) + if err != nil { + return xauthEntry{}, xerrors.Errorf("failed to read auth protocol: %w", err) + } + entry.authProtocol = string(authProtocolBytes) + + // Read auth cookie + var authCookieLength uint16 + err = binary.Read(r, binary.BigEndian, &authCookieLength) + if err != nil { + return xauthEntry{}, xerrors.Errorf("failed to read auth cookie length: %w", err) + } + + entry.authCookie = make([]byte, authCookieLength) + _, err = r.Read(entry.authCookie) + if err != nil { + return xauthEntry{}, xerrors.Errorf("failed to read auth cookie: %w", err) + } + + return entry, nil +} diff --git a/agent/agentssh/x11_internal_test.go b/agent/agentssh/x11_internal_test.go new file mode 100644 index 0000000000000..fdc3c04668663 --- /dev/null +++ b/agent/agentssh/x11_internal_test.go @@ -0,0 +1,254 @@ +package agentssh + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_addXauthEntry(t *testing.T) { + t.Parallel() + + type testEntry struct { + address string + display string + authProtocol string + authCookie string + } + tests := []struct { + name string + authFile []byte + wantAuthFile []byte + entries []testEntry + }{ + { + name: "add entry", + authFile: nil, + wantAuthFile: []byte{ + // w/unix:0 MIT-MAGIC-COOKIE-1 00 + // + // 00000000: 0100 0001 7700 0130 0012 4d49 542d 4d41 ....w..0..MIT-MA + // 00000010: 4749 432d 434f 4f4b 4945 2d31 0001 00 GIC-COOKIE-1... + 0x01, 0x00, 0x00, 0x01, 0x77, 0x00, 0x01, 0x30, + 0x00, 0x12, 0x4d, 0x49, 0x54, 0x2d, 0x4d, 0x41, + 0x47, 0x49, 0x43, 0x2d, 0x43, 0x4f, 0x4f, 0x4b, + 0x49, 0x45, 0x2d, 0x31, 0x00, 0x01, 0x00, + }, + entries: []testEntry{ + { + address: "w", + display: "0", + authProtocol: "MIT-MAGIC-COOKIE-1", + authCookie: "00", + }, + }, + }, + { + name: "add two entries", + authFile: []byte{}, + wantAuthFile: []byte{ + // w/unix:0 MIT-MAGIC-COOKIE-1 00 + // w/unix:1 MIT-MAGIC-COOKIE-1 11 + // + // 00000000: 0100 0001 7700 0130 0012 4d49 542d 4d41 ....w..0..MIT-MA + // 00000010: 4749 432d 434f 4f4b 4945 2d31 0001 0001 GIC-COOKIE-1.... + // 00000020: 0000 0177 0001 3100 124d 4954 2d4d 4147 ...w..1..MIT-MAG + // 00000030: 4943 2d43 4f4f 4b49 452d 3100 0111 IC-COOKIE-1... + 0x01, 0x00, 0x00, 0x01, 0x77, 0x00, 0x01, 0x30, + 0x00, 0x12, 0x4d, 0x49, 0x54, 0x2d, 0x4d, 0x41, + 0x47, 0x49, 0x43, 0x2d, 0x43, 0x4f, 0x4f, 0x4b, + 0x49, 0x45, 0x2d, 0x31, 0x00, 0x01, 0x00, + 0x01, 0x00, 0x00, 0x01, 0x77, 0x00, 0x01, 0x31, + 0x00, 0x12, 0x4d, 0x49, 0x54, 0x2d, 0x4d, 0x41, + 0x47, 0x49, 0x43, 0x2d, 0x43, 0x4f, 0x4f, 0x4b, + 0x49, 0x45, 0x2d, 0x31, 0x00, 0x01, 0x11, + }, + entries: []testEntry{ + { + address: "w", + display: "0", + authProtocol: "MIT-MAGIC-COOKIE-1", + authCookie: "00", + }, + { + address: "w", + display: "1", + authProtocol: "MIT-MAGIC-COOKIE-1", + authCookie: "11", + }, + }, + }, + { + name: "update entry with new auth cookie length", + authFile: []byte{ + // w/unix:0 MIT-MAGIC-COOKIE-1 00 + // w/unix:1 MIT-MAGIC-COOKIE-1 11 + // + // 00000000: 0100 0001 7700 0130 0012 4d49 542d 4d41 ....w..0..MIT-MA + // 00000010: 4749 432d 434f 4f4b 4945 2d31 0001 0001 GIC-COOKIE-1.... + // 00000020: 0000 0177 0001 3100 124d 4954 2d4d 4147 ...w..1..MIT-MAG + // 00000030: 4943 2d43 4f4f 4b49 452d 3100 0111 IC-COOKIE-1... + 0x01, 0x00, 0x00, 0x01, 0x77, 0x00, 0x01, 0x30, + 0x00, 0x12, 0x4d, 0x49, 0x54, 0x2d, 0x4d, 0x41, + 0x47, 0x49, 0x43, 0x2d, 0x43, 0x4f, 0x4f, 0x4b, + 0x49, 0x45, 0x2d, 0x31, 0x00, 0x01, 0x00, + 0x01, 0x00, 0x00, 0x01, 0x77, 0x00, 0x01, 0x31, + 0x00, 0x12, 0x4d, 0x49, 0x54, 0x2d, 0x4d, 0x41, + 0x47, 0x49, 0x43, 0x2d, 0x43, 0x4f, 0x4f, 0x4b, + 0x49, 0x45, 0x2d, 0x31, 0x00, 0x01, 0x11, + }, + wantAuthFile: []byte{ + // The order changed, due to new length of auth cookie resulting + // in remove + append, we verify that the implementation is + // behaving as expected (changing the order is not a requirement, + // simply an implementation detail). + 0x01, 0x00, 0x00, 0x01, 0x77, 0x00, 0x01, 0x31, + 0x00, 0x12, 0x4d, 0x49, 0x54, 0x2d, 0x4d, 0x41, + 0x47, 0x49, 0x43, 0x2d, 0x43, 0x4f, 0x4f, 0x4b, + 0x49, 0x45, 0x2d, 0x31, 0x00, 0x01, 0x11, + 0x01, 0x00, 0x00, 0x01, 0x77, 0x00, 0x01, 0x30, + 0x00, 0x12, 0x4d, 0x49, 0x54, 0x2d, 0x4d, 0x41, + 0x47, 0x49, 0x43, 0x2d, 0x43, 0x4f, 0x4f, 0x4b, + 0x49, 0x45, 0x2d, 0x31, 0x00, 0x02, 0xff, 0xff, + }, + entries: []testEntry{ + { + address: "w", + display: "0", + authProtocol: "MIT-MAGIC-COOKIE-1", + authCookie: "ffff", + }, + }, + }, + { + name: "update entry", + authFile: []byte{ + // 00000000: 0100 0001 7700 0130 0012 4d49 542d 4d41 ....w..0..MIT-MA + // 00000010: 4749 432d 434f 4f4b 4945 2d31 0001 0001 GIC-COOKIE-1.... + // 00000020: 0000 0177 0001 3100 124d 4954 2d4d 4147 ...w..1..MIT-MAG + // 00000030: 4943 2d43 4f4f 4b49 452d 3100 0111 IC-COOKIE-1... + 0x01, 0x00, 0x00, 0x01, 0x77, 0x00, 0x01, 0x30, + 0x00, 0x12, 0x4d, 0x49, 0x54, 0x2d, 0x4d, 0x41, + 0x47, 0x49, 0x43, 0x2d, 0x43, 0x4f, 0x4f, 0x4b, + 0x49, 0x45, 0x2d, 0x31, 0x00, 0x01, 0x00, + 0x01, 0x00, 0x00, 0x01, 0x77, 0x00, 0x01, 0x31, + 0x00, 0x12, 0x4d, 0x49, 0x54, 0x2d, 0x4d, 0x41, + 0x47, 0x49, 0x43, 0x2d, 0x43, 0x4f, 0x4f, 0x4b, + 0x49, 0x45, 0x2d, 0x31, 0x00, 0x01, 0x11, + }, + wantAuthFile: []byte{ + // 00000000: 0100 0001 7700 0130 0012 4d49 542d 4d41 ....w..0..MIT-MA + // 00000010: 4749 432d 434f 4f4b 4945 2d31 0001 0001 GIC-COOKIE-1.... + // 00000020: 0000 0177 0001 3100 124d 4954 2d4d 4147 ...w..1..MIT-MAG + // 00000030: 4943 2d43 4f4f 4b49 452d 3100 0111 IC-COOKIE-1... + 0x01, 0x00, 0x00, 0x01, 0x77, 0x00, 0x01, 0x30, + 0x00, 0x12, 0x4d, 0x49, 0x54, 0x2d, 0x4d, 0x41, + 0x47, 0x49, 0x43, 0x2d, 0x43, 0x4f, 0x4f, 0x4b, + 0x49, 0x45, 0x2d, 0x31, 0x00, 0x01, 0xff, + 0x01, 0x00, 0x00, 0x01, 0x77, 0x00, 0x01, 0x31, + 0x00, 0x12, 0x4d, 0x49, 0x54, 0x2d, 0x4d, 0x41, + 0x47, 0x49, 0x43, 0x2d, 0x43, 0x4f, 0x4f, 0x4b, + 0x49, 0x45, 0x2d, 0x31, 0x00, 0x01, 0x11, + }, + entries: []testEntry{ + { + address: "w", + display: "0", + authProtocol: "MIT-MAGIC-COOKIE-1", + authCookie: "ff", + }, + }, + }, + { + name: "clean up old entries", + authFile: []byte{ + // w/unix:0 MIT-MAGIC-COOKIE-1 80507df050756cdefa504b65adb3bcfb + // w/unix:0 MIT-MAGIC-COOKIE-1 267b37f6cbc11b97beb826bb1aab8570 + // w/unix:0 MIT-MAGIC-COOKIE-1 516e22e2b11d1bd0115dff09c028ca5c + // + // 00000000: 0100 0001 7700 0130 0012 4d49 542d 4d41 ....w..0..MIT-MA + // 00000010: 4749 432d 434f 4f4b 4945 2d31 0010 8050 GIC-COOKIE-1...P + // 00000020: 7df0 5075 6cde fa50 4b65 adb3 bcfb 0100 }.Pul..PKe...... + // 00000030: 0001 7700 0130 0012 4d49 542d 4d41 4749 ..w..0..MIT-MAGI + // 00000040: 432d 434f 4f4b 4945 2d31 0010 267b 37f6 C-COOKIE-1..&{7. + // 00000050: cbc1 1b97 beb8 26bb 1aab 8570 0100 0001 ......&....p.... + // 00000060: 7700 0130 0012 4d49 542d 4d41 4749 432d w..0..MIT-MAGIC- + // 00000070: 434f 4f4b 4945 2d31 0010 516e 22e2 b11d COOKIE-1..Qn"... + // 00000080: 1bd0 115d ff09 c028 ca5c ...]...(.\ + 0x01, 0x00, 0x00, 0x01, 0x77, 0x00, 0x01, 0x30, + 0x00, 0x12, 0x4d, 0x49, 0x54, 0x2d, 0x4d, 0x41, + 0x47, 0x49, 0x43, 0x2d, 0x43, 0x4f, 0x4f, 0x4b, + 0x49, 0x45, 0x2d, 0x31, 0x00, 0x10, 0x80, 0x50, + 0x7d, 0xf0, 0x50, 0x75, 0x6c, 0xde, 0xfa, 0x50, + 0x4b, 0x65, 0xad, 0xb3, 0xbc, 0xfb, 0x01, 0x00, + 0x00, 0x01, 0x77, 0x00, 0x01, 0x30, 0x00, 0x12, + 0x4d, 0x49, 0x54, 0x2d, 0x4d, 0x41, 0x47, 0x49, + 0x43, 0x2d, 0x43, 0x4f, 0x4f, 0x4b, 0x49, 0x45, + 0x2d, 0x31, 0x00, 0x10, 0x26, 0x7b, 0x37, 0xf6, + 0xcb, 0xc1, 0x1b, 0x97, 0xbe, 0xb8, 0x26, 0xbb, + 0x1a, 0xab, 0x85, 0x70, 0x01, 0x00, 0x00, 0x01, + 0x77, 0x00, 0x01, 0x30, 0x00, 0x12, 0x4d, 0x49, + 0x54, 0x2d, 0x4d, 0x41, 0x47, 0x49, 0x43, 0x2d, + 0x43, 0x4f, 0x4f, 0x4b, 0x49, 0x45, 0x2d, 0x31, + 0x00, 0x10, 0x51, 0x6e, 0x22, 0xe2, 0xb1, 0x1d, + 0x1b, 0xd0, 0x11, 0x5d, 0xff, 0x09, 0xc0, 0x28, + 0xca, 0x5c, + }, + wantAuthFile: []byte{ + // w/unix:0 MIT-MAGIC-COOKIE-1 516e5bc892b7162b844abd1fc1a7c16e + // + // 00000000: 0100 0001 7700 0130 0012 4d49 542d 4d41 ....w..0..MIT-MA + // 00000010: 4749 432d 434f 4f4b 4945 2d31 0010 516e GIC-COOKIE-1..Qn + // 00000020: 5bc8 92b7 162b 844a bd1f c1a7 c16e [....+.J.....n + 0x01, 0x00, 0x00, 0x01, 0x77, 0x00, 0x01, 0x30, + 0x00, 0x12, 0x4d, 0x49, 0x54, 0x2d, 0x4d, 0x41, + 0x47, 0x49, 0x43, 0x2d, 0x43, 0x4f, 0x4f, 0x4b, + 0x49, 0x45, 0x2d, 0x31, 0x00, 0x10, 0x51, 0x6e, + 0x5b, 0xc8, 0x92, 0xb7, 0x16, 0x2b, 0x84, 0x4a, + 0xbd, 0x1f, 0xc1, 0xa7, 0xc1, 0x6e, + }, + entries: []testEntry{ + { + address: "w", + display: "0", + authProtocol: "MIT-MAGIC-COOKIE-1", + authCookie: "516e5bc892b7162b844abd1fc1a7c16e", + }, + }, + }, + } + + homedir, err := os.UserHomeDir() + require.NoError(t, err) + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + if tt.authFile != nil { + err := afero.WriteFile(fs, filepath.Join(homedir, ".Xauthority"), tt.authFile, 0o600) + require.NoError(t, err) + } + + for _, entry := range tt.entries { + err := addXauthEntry(context.Background(), fs, entry.address, entry.display, entry.authProtocol, entry.authCookie) + require.NoError(t, err) + } + + gotAuthFile, err := afero.ReadFile(fs, filepath.Join(homedir, ".Xauthority")) + require.NoError(t, err) + + if diff := cmp.Diff(tt.wantAuthFile, gotAuthFile); diff != "" { + assert.Failf(t, "addXauthEntry() mismatch", "(-want +got):\n%s", diff) + } + }) + } +}