Skip to content

Commit df96dcf

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

File tree

2 files changed

+387
-2
lines changed

2 files changed

+387
-2
lines changed

agent/agentssh/x11.go

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

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

0 commit comments

Comments
 (0)