Skip to content

Commit 43d09a5

Browse files
committed
fix(agent/agentssh): fix X11 forwarding by improving Xauthority management
Fixes #11531
1 parent 6e5c2ef commit 43d09a5

File tree

2 files changed

+396
-2
lines changed

2 files changed

+396
-2
lines changed

agent/agentssh/x11.go

+189-2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"encoding/hex"
77
"errors"
88
"fmt"
9+
"io"
910
"net"
1011
"os"
1112
"path/filepath"
@@ -141,7 +142,7 @@ func addXauthEntry(ctx context.Context, fs afero.Fs, host string, display string
141142
}
142143

143144
// Open or create the Xauthority file
144-
file, err := fs.OpenFile(xauthPath, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0o600)
145+
file, err := fs.OpenFile(xauthPath, os.O_RDWR|os.O_CREATE, 0o600)
145146
if err != nil {
146147
return xerrors.Errorf("failed to open Xauthority file: %w", err)
147148
}
@@ -153,7 +154,100 @@ func addXauthEntry(ctx context.Context, fs afero.Fs, host string, display string
153154
return xerrors.Errorf("failed to decode auth cookie: %w", err)
154155
}
155156

156-
// Write Xauthority entry
157+
// Read the Xauthority file and look for an existing entry for the host,
158+
// display, and auth protocol. If an entry is found, overwrite the auth
159+
// cookie (if it fits). Otherwise, mark the entry for deletion.
160+
type deleteEntry struct {
161+
start, end int64
162+
}
163+
var deleteEntries []deleteEntry
164+
pos := int64(0)
165+
updated := false
166+
for {
167+
entry, err := readXauthEntry(file)
168+
if err != nil {
169+
if errors.Is(err, io.EOF) {
170+
break
171+
}
172+
return xerrors.Errorf("failed to read Xauthority entry: %w", err)
173+
}
174+
175+
nextPos := pos + int64(entry.Len())
176+
cookieStartPos := nextPos - int64(len(entry.authCookie))
177+
178+
if entry.family == 0x0100 && entry.address == host && entry.display == display && entry.authProtocol == authProtocol {
179+
if !updated && len(entry.authCookie) == len(authCookieBytes) {
180+
// Overwrite the auth cookie
181+
_, err := file.WriteAt(authCookieBytes, cookieStartPos)
182+
if err != nil {
183+
return xerrors.Errorf("failed to write auth cookie: %w", err)
184+
}
185+
updated = true
186+
} else {
187+
// Mark entry for deletion.
188+
if len(deleteEntries) > 0 && deleteEntries[len(deleteEntries)-1].end == pos {
189+
deleteEntries[len(deleteEntries)-1].end = nextPos
190+
} else {
191+
deleteEntries = append(deleteEntries, deleteEntry{
192+
start: pos,
193+
end: nextPos,
194+
})
195+
}
196+
}
197+
}
198+
199+
pos = nextPos
200+
}
201+
202+
// In case the magic cookie changed, or we've previously bloated the
203+
// Xauthority file, we may have to delete entries.
204+
if len(deleteEntries) > 0 {
205+
// Read the entire file into memory. This is not ideal, but it's the
206+
// simplest way to delete entries from the middle of the file. The
207+
// Xauthority file is small, so this should be fine.
208+
_, err = file.Seek(0, io.SeekStart)
209+
if err != nil {
210+
return xerrors.Errorf("failed to seek Xauthority file: %w", err)
211+
}
212+
data, err := io.ReadAll(file)
213+
if err != nil {
214+
return xerrors.Errorf("failed to read Xauthority file: %w", err)
215+
}
216+
217+
// Delete the entries.
218+
for _, entry := range deleteEntries {
219+
data = append(data[:entry.start], data[entry.end:]...)
220+
}
221+
222+
// Write the data back to the file.
223+
_, err = file.Seek(0, io.SeekStart)
224+
if err != nil {
225+
return xerrors.Errorf("failed to seek Xauthority file: %w", err)
226+
}
227+
_, err = file.Write(data)
228+
if err != nil {
229+
return xerrors.Errorf("failed to write Xauthority file: %w", err)
230+
}
231+
232+
// Truncate the file.
233+
err = file.Truncate(int64(len(data)))
234+
if err != nil {
235+
return xerrors.Errorf("failed to truncate Xauthority file: %w", err)
236+
}
237+
}
238+
239+
// Return if we've already updated the entry.
240+
if updated {
241+
return nil
242+
}
243+
244+
// Ensure we're at the end (append).
245+
_, err = file.Seek(0, io.SeekEnd)
246+
if err != nil {
247+
return xerrors.Errorf("failed to seek Xauthority file: %w", err)
248+
}
249+
250+
// Append Xauthority entry.
157251
family := uint16(0x0100) // FamilyLocal
158252
err = binary.Write(file, binary.BigEndian, family)
159253
if err != nil {
@@ -198,3 +292,96 @@ func addXauthEntry(ctx context.Context, fs afero.Fs, host string, display string
198292

199293
return nil
200294
}
295+
296+
// xauthEntry is an representation of an Xauthority entry.
297+
//
298+
// The Xauthority file format is as follows:
299+
//
300+
// - 16-bit family
301+
// - 16-bit address length
302+
// - address
303+
// - 16-bit display length
304+
// - display
305+
// - 16-bit auth protocol length
306+
// - auth protocol
307+
// - 16-bit auth cookie length
308+
// - auth cookie
309+
type xauthEntry struct {
310+
family uint16
311+
address string
312+
display string
313+
authProtocol string
314+
authCookie []byte
315+
}
316+
317+
func (e xauthEntry) Len() int {
318+
// 5 * uint16 = 10 bytes for the family/length fields.
319+
return 2*5 + len(e.address) + len(e.display) + len(e.authProtocol) + len(e.authCookie)
320+
}
321+
322+
func readXauthEntry(r io.Reader) (xauthEntry, error) {
323+
var entry xauthEntry
324+
325+
// Read family
326+
err := binary.Read(r, binary.BigEndian, &entry.family)
327+
if err != nil {
328+
return xauthEntry{}, xerrors.Errorf("failed to read family: %w", err)
329+
}
330+
331+
// Read address
332+
var addressLength uint16
333+
err = binary.Read(r, binary.BigEndian, &addressLength)
334+
if err != nil {
335+
return xauthEntry{}, xerrors.Errorf("failed to read address length: %w", err)
336+
}
337+
338+
addressBytes := make([]byte, addressLength)
339+
_, err = r.Read(addressBytes)
340+
if err != nil {
341+
return xauthEntry{}, xerrors.Errorf("failed to read address: %w", err)
342+
}
343+
entry.address = string(addressBytes)
344+
345+
// Read display
346+
var displayLength uint16
347+
err = binary.Read(r, binary.BigEndian, &displayLength)
348+
if err != nil {
349+
return xauthEntry{}, xerrors.Errorf("failed to read display length: %w", err)
350+
}
351+
352+
displayBytes := make([]byte, displayLength)
353+
_, err = r.Read(displayBytes)
354+
if err != nil {
355+
return xauthEntry{}, xerrors.Errorf("failed to read display: %w", err)
356+
}
357+
entry.display = string(displayBytes)
358+
359+
// Read auth protocol
360+
var authProtocolLength uint16
361+
err = binary.Read(r, binary.BigEndian, &authProtocolLength)
362+
if err != nil {
363+
return xauthEntry{}, xerrors.Errorf("failed to read auth protocol length: %w", err)
364+
}
365+
366+
authProtocolBytes := make([]byte, authProtocolLength)
367+
_, err = r.Read(authProtocolBytes)
368+
if err != nil {
369+
return xauthEntry{}, xerrors.Errorf("failed to read auth protocol: %w", err)
370+
}
371+
entry.authProtocol = string(authProtocolBytes)
372+
373+
// Read auth cookie
374+
var authCookieLength uint16
375+
err = binary.Read(r, binary.BigEndian, &authCookieLength)
376+
if err != nil {
377+
return xauthEntry{}, xerrors.Errorf("failed to read auth cookie length: %w", err)
378+
}
379+
380+
entry.authCookie = make([]byte, authCookieLength)
381+
_, err = r.Read(entry.authCookie)
382+
if err != nil {
383+
return xauthEntry{}, xerrors.Errorf("failed to read auth cookie: %w", err)
384+
}
385+
386+
return entry, nil
387+
}

0 commit comments

Comments
 (0)