Skip to content

Commit b1d53a6

Browse files
authored
fix(agent/agentssh): fix X11 forwarding by improving Xauthority management (#11550)
Fixes #11531
1 parent 89ab659 commit b1d53a6

File tree

2 files changed

+448
-2
lines changed

2 files changed

+448
-2
lines changed

agent/agentssh/x11.go

+194-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,105 @@ 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 int
162+
}
163+
var deleteEntries []deleteEntry
164+
pos := 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 + entry.Len()
176+
cookieStartPos := nextPos - 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, int64(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 in reverse order.
218+
for i := len(deleteEntries) - 1; i >= 0; i-- {
219+
entry := deleteEntries[i]
220+
// Safety check: ensure the entry is still there.
221+
if entry.start > len(data) || entry.end > len(data) {
222+
continue
223+
}
224+
data = append(data[:entry.start], data[entry.end:]...)
225+
}
226+
227+
// Write the data back to the file.
228+
_, err = file.Seek(0, io.SeekStart)
229+
if err != nil {
230+
return xerrors.Errorf("failed to seek Xauthority file: %w", err)
231+
}
232+
_, err = file.Write(data)
233+
if err != nil {
234+
return xerrors.Errorf("failed to write Xauthority file: %w", err)
235+
}
236+
237+
// Truncate the file.
238+
err = file.Truncate(int64(len(data)))
239+
if err != nil {
240+
return xerrors.Errorf("failed to truncate Xauthority file: %w", err)
241+
}
242+
}
243+
244+
// Return if we've already updated the entry.
245+
if updated {
246+
return nil
247+
}
248+
249+
// Ensure we're at the end (append).
250+
_, err = file.Seek(0, io.SeekEnd)
251+
if err != nil {
252+
return xerrors.Errorf("failed to seek Xauthority file: %w", err)
253+
}
254+
255+
// Append Xauthority entry.
157256
family := uint16(0x0100) // FamilyLocal
158257
err = binary.Write(file, binary.BigEndian, family)
159258
if err != nil {
@@ -198,3 +297,96 @@ func addXauthEntry(ctx context.Context, fs afero.Fs, host string, display string
198297

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

0 commit comments

Comments
 (0)