Skip to content

Commit

Permalink
Fix trusted x11 forwarding with client xauth data (#48937) (#50034)
Browse files Browse the repository at this point in the history 
* Fix trusted x11 forwarding with client xauth data; use slog.

* Fix slog lint.
  • Loading branch information
@Joerger
Joerger authored Dec 11, 2024
1 parent 377fb74 commit 8343e9d
Showing 1 changed file with 39 additions and 26 deletions.
65 changes: 39 additions & 26 deletions lib/client/x11_session.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ package client

import (
"context"
"fmt"
"log/slog"
"os"
"time"

Expand All @@ -41,7 +43,7 @@ func (ns *NodeSession) handleX11Forwarding(ctx context.Context, sess *tracessh.S

display, err := x11.GetXDisplay()
if err != nil {
log.WithError(err).Info("X11 forwarding requested but $DISPLAY is invalid")
slog.InfoContext(ctx, "X11 forwarding requested but $DISPLAY is invalid", "err", err)
return ns.rejectX11Channels(ctx)
}

Expand All @@ -60,8 +62,8 @@ func (ns *NodeSession) handleX11Forwarding(ctx context.Context, sess *tracessh.S

if err := x11.RequestForwarding(sess.Session, ns.spoofedXAuthEntry); err != nil {
// Notify the user that x11 forwarding request failed regardless of debug level
log.Print("X11 forwarding request failed")
log.WithError(err).Debug("X11 forwarding request error")
fmt.Fprintln(os.Stderr, "X11 forwarding request failed")
slog.DebugContext(ctx, "X11 forwarding request error", "err", err)
// If the X11 forwarding request fails, we must reject all X11 channel requests.
return ns.rejectX11Channels(ctx)
}
Expand All @@ -76,25 +78,35 @@ func (ns *NodeSession) handleX11Forwarding(ctx context.Context, sess *tracessh.S
// This will be used during X11 forwarding to forward and authorize
// XServer requests from the remote server to the client's XServer.
func (ns *NodeSession) setXAuthData(ctx context.Context, display x11.Display) error {
checkXauthErr := x11.CheckXAuthPath()
if checkXauthErr != nil && !ns.nodeClient.TC.X11ForwardingTrusted {
slog.WarnContext(ctx, "Untrusted X11 forwarding requires xauth to be installed in order generated xauth key data")
return trace.Wrap(checkXauthErr)
}

if ns.nodeClient.TC.X11ForwardingTrusted {
// For trusted X11 forwarding, we can create a random cookie without xauth
slog.WarnContext(ctx, "Trusted X11 forwarding provides unmitigated access to your local XServer, use with caution")

// Check for existing xauth data. If found, it must be used in order to connect to the client's XServer.
var err error
if checkXauthErr == nil {
ns.clientXAuthEntry, err = x11.NewXAuthCommand(ctx, "" /* xauthFile */).ReadEntry(display)
if !trace.IsNotFound(err) {
return trace.Wrap(err)
}
}

// If no xauth data was found, we can create a random cookie without xauth
// as it is only used to validate the server-client connection. Locally,
// the client's XServer will ignore the cookie and use whatever authentication
// mechanisms it would use as if the client made the request locally.
log.Info("Creating a fake xauth cookie for trusted X11 forwarding.")
log.Warn("Trusted X11 forwarding provides unmitigated access to your local XServer, use with caution")
slog.InfoContext(ctx, "No xauth data found, creating a fake xauth cookie for trusted X11 forwarding.")

var err error
ns.clientXAuthEntry, err = x11.NewFakeXAuthEntry(display)
return trace.Wrap(err)
}

if err := x11.CheckXAuthPath(); err != nil {
log.Info("trusted X11 forwarding requested but xauth is not installed")
return trace.Wrap(err)
}

// Generate the xauth entry in a temporary file so it only exists within the context of this request.
// Generate an untrusted xauth entry in a temporary file so it only exists within the context of this request.
// The XServer will recognize the xauth data regardless of it's existence within the file system.
xauthFile, err := os.CreateTemp("", "tsh-xauthfile-*")
if err != nil {
Expand All @@ -110,7 +122,7 @@ func (ns *NodeSession) setXAuthData(ctx context.Context, display x11.Display) er

defer func() {
if err := os.Remove(xauthFile.Name()); err != nil {
log.WithError(err).Debug("Failed to remove temporary xauth file")
slog.DebugContext(ctx, "Failed to remove temporary xauth file", "err", err)
}
}()

Expand All @@ -123,14 +135,15 @@ func (ns *NodeSession) setXAuthData(ctx context.Context, display x11.Display) er
ns.x11RefuseTime = time.Now().Add(ns.nodeClient.TC.X11ForwardingTimeout)
}

log.Info("creating an untrusted xauth cookie for untrusted X11 forwarding")
slog.InfoContext(ctx, "creating an untrusted xauth cookie for untrusted X11 forwarding", "xauth_file", xauthFile.Name())
cmd := x11.NewXAuthCommand(ctx, xauthFile.Name())
if err := cmd.GenerateUntrustedCookie(display, ns.nodeClient.TC.X11ForwardingTimeout); err != nil {
return trace.Wrap(err)
}

ns.clientXAuthEntry, err = x11.NewXAuthCommand(ctx, xauthFile.Name()).ReadEntry(display)
if err != nil {
slog.ErrorContext(ctx, "untrusted X11 forwarding setup failed: failed to generate xauth key data")
return trace.Wrap(err)
}

Expand All @@ -142,21 +155,21 @@ func (ns *NodeSession) serveX11Channels(ctx context.Context, sess *tracessh.Sess
err := x11.ServeChannelRequests(ctx, ns.nodeClient.Client.Client, func(ctx context.Context, nch ssh.NewChannel) {
if !ns.x11RefuseTime.IsZero() && time.Now().After(ns.x11RefuseTime) {
nch.Reject(ssh.Prohibited, "rejected X11 channel request after ForwardX11Timeout")
log.Warn("rejected X11 forwarding attempt after the ForwardX11Timeout")
slog.WarnContext(ctx, "rejected X11 forwarding attempt after the ForwardX11Timeout")
return
}

var req x11.ChannelRequestPayload
if err := ssh.Unmarshal(nch.ExtraData(), &req); err != nil {
nch.Reject(ssh.Prohibited, "invalid payload")
log.WithError(err).Debug("rejected X11 channel request with invalid payload")
slog.DebugContext(ctx, "rejected X11 channel request with invalid payload", "err", err)
return
}

log.Debugf("received X11 channel request from %s:%d", req.OriginatorAddress, req.OriginatorPort)
slog.DebugContext(ctx, "received X11 channel request from %s:%d", req.OriginatorAddress, req.OriginatorPort)
xchan, sin, err := nch.Accept()
if err != nil {
log.WithError(err).Debug("failed to accept X11 channel request")
slog.DebugContext(ctx, "failed to accept X11 channel request", "err", err)
return
}
defer xchan.Close()
Expand All @@ -166,24 +179,24 @@ func (ns *NodeSession) serveX11Channels(ctx context.Context, sess *tracessh.Sess
// client's xauth cookie. Otherwise, the request will be denied.
authPacket, err := x11.ReadAndRewriteXAuthPacket(xchan, ns.spoofedXAuthEntry, ns.clientXAuthEntry)
if trace.IsAccessDenied(err) {
log.Error("X11 connection rejected due to wrong authentication")
slog.ErrorContext(ctx, "X11 connection rejected due to wrong authentication", "err", err)
return
} else if err != nil {
log.WithError(err).Debug("Failed to read xauth packet from X11 channel request")
slog.DebugContext(ctx, "Failed to read xauth packet from X11 channel request", "err", err)
return
}

// Dial a connection to the client's XServer.
xconn, err := ns.clientXAuthEntry.Display.Dial()
if err != nil {
log.WithError(err).Debug("Failed to connect to client's display")
slog.DebugContext(ctx, "Failed to connect to client's display", "err", err)
return
}
defer xconn.Close()

// Send the processed X11 auth packet to the client's XServer connection.
if _, err := xconn.Write(authPacket); err != nil {
log.WithError(err).Debug("Failed to write xauth packet")
slog.DebugContext(ctx, "Failed to write xauth packet", "err", err)
return
}

Expand All @@ -194,12 +207,12 @@ func (ns *NodeSession) serveX11Channels(ctx context.Context, sess *tracessh.Sess
go func() {
err := sshutils.ForwardRequests(ctx, sin, sess)
if err != nil {
log.WithError(err).Debug("Failed to forward ssh request from server during X11 forwarding")
slog.DebugContext(ctx, "Failed to forward ssh request from server during X11 forwarding", "err", err)
}
}()

if err := x11.Forward(ctx, xconn, xchan); err != nil {
log.WithError(err).Debug("Encountered error during X11 forwarding")
slog.DebugContext(ctx, "Encountered error during X11 forwarding", "err", err)
}
})
return trace.Wrap(err)
Expand All @@ -211,7 +224,7 @@ func (ns *NodeSession) rejectX11Channels(ctx context.Context) error {
// According to RFC 4254, client "implementations MUST reject any X11 channel
// open requests if they have not requested X11 forwarding". Following openssh's
// example, we treat such a request as a break in attempt and warn the user.
log.Warn("server tried X11 forwarding without client requesting it, this is likely a break-in attempt by a malicious user")
slog.WarnContext(ctx, "server tried X11 forwarding without client requesting it, this is likely a break-in attempt by a malicious user")
nch.Reject(ssh.Prohibited, "")
})
return trace.Wrap(err)
Expand Down

0 comments on commit 8343e9d

Please sign in to comment.