Bind cached tunnel reuse to the original session identity

HandleGatewayProtocol caches a *Tunnel keyed on the client-supplied
Rdg-Connection-Id header so the two halves of a session (RDG_OUT_DATA
and RDG_IN_DATA) can rendezvous on a single record. The cache hit path
previously reused the tunnel without checking who was making the
follow-up request.

Add tunnelOwnerMatches(t, id) to compare the cached tunnel's
UserName() and AttrClientIp against the request identity. On
mismatch, refuse with 401 instead of attaching the new request to the
existing tunnel. The helper is conservative: nil tunnel/user/identity,
empty username, or missing client-IP attribute all fail closed.

The legitimate case (the same client returns to attach its second
half-channel to its own first half) is unchanged.

Adds TestTunnelOwnershipEnforced.
This commit is contained in:
Bolke de Bruin
2026-04-30 14:04:15 +02:00
parent 75ef8ce289
commit 980c6266c0
2 changed files with 85 additions and 0 deletions

View File

@@ -75,6 +75,11 @@ func (g *Gateway) HandleGatewayProtocol(w http.ResponseWriter, r *http.Request)
}
} else {
t = x.(*Tunnel)
if !tunnelOwnerMatches(t, id) {
log.Printf("rejecting reuse of Rdg-Connection-Id %q from a different identity", connId)
http.Error(w, "Tunnel is owned by a different session", http.StatusUnauthorized)
return
}
}
ctx = context.WithValue(ctx, CtxTunnel, t)
@@ -106,6 +111,25 @@ func (g *Gateway) HandleGatewayProtocol(w http.ResponseWriter, r *http.Request)
}
}
// tunnelOwnerMatches reports whether the cached tunnel was opened by the same
// identity making the current request. The Rdg-Connection-Id is a client-
// chosen header that pairs the two halves of a session; without this check
// any caller who learns or guesses one can attach to another user's tunnel.
func tunnelOwnerMatches(t *Tunnel, id identity.Identity) bool {
if t == nil || t.User == nil || id == nil {
return false
}
if t.User.UserName() == "" || t.User.UserName() != id.UserName() {
return false
}
cachedIp, _ := t.User.GetAttribute(identity.AttrClientIp).(string)
reqIp, _ := id.GetAttribute(identity.AttrClientIp).(string)
if cachedIp == "" {
return false
}
return cachedIp == reqIp
}
// headerHasToken reports whether the HTTP header named by name contains
// token, matched case-insensitively against comma-separated tokens.
// Fields like Connection carry a list (e.g. "keep-alive, Upgrade") so a

View File

@@ -10,6 +10,7 @@ import (
"time"
"github.com/bolkedebruin/rdpgw/cmd/rdpgw/identity"
"github.com/patrickmn/go-cache"
)
func TestHeaderHasToken(t *testing.T) {
@@ -144,3 +145,63 @@ func TestHandleGatewayProtocolRouting(t *testing.T) {
})
}
}
// TestTunnelOwnershipEnforced asserts that an Rdg-Connection-Id which already
// has a cached tunnel cannot be reused by a different identity. The connection
// id travels in plain HTTP headers, so a client that learns or guesses one
// must not be able to attach to the original tunnel — the cache is for
// matching the two halves of the same client's session, not for authorizing
// access.
func TestTunnelOwnershipEnforced(t *testing.T) {
connId := "shared-connid-ownership"
aliceID := identity.NewUser()
aliceID.SetUserName("alice")
aliceID.SetAttribute(identity.AttrRemoteAddr, "10.1.1.1:1234")
aliceID.SetAttribute(identity.AttrClientIp, "10.1.1.1")
c.Set(connId, &Tunnel{
RDGId: connId,
RemoteAddr: "10.1.1.1:1234",
User: aliceID,
}, cache.DefaultExpiration)
defer c.Delete(connId)
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
bob := identity.NewUser()
bob.SetUserName("bob")
bob.SetAttribute(identity.AttrRemoteAddr, "10.2.2.2:5678")
bob.SetAttribute(identity.AttrClientIp, "10.2.2.2")
r = identity.AddToRequestCtx(bob, r)
(&Gateway{}).HandleGatewayProtocol(w, r)
})
srv := httptest.NewServer(handler)
defer srv.Close()
addr := strings.TrimPrefix(srv.URL, "http://")
conn, err := net.DialTimeout("tcp", addr, 2*time.Second)
if err != nil {
t.Fatalf("dial: %v", err)
}
defer conn.Close()
if err := conn.SetDeadline(time.Now().Add(3 * time.Second)); err != nil {
t.Fatalf("set deadline: %v", err)
}
req := "RDG_OUT_DATA /remoteDesktopGateway/ HTTP/1.1\r\n" +
"Host: " + addr + "\r\n" +
"Rdg-Connection-Id: " + connId + "\r\n" +
"\r\n"
if _, err := conn.Write([]byte(req)); err != nil {
t.Fatalf("write: %v", err)
}
line, err := bufio.NewReader(conn).ReadString('\n')
if err != nil {
t.Fatalf("read status line: %v", err)
}
line = strings.TrimRight(line, "\r\n")
if strings.HasPrefix(line, "HTTP/1.1 2") {
t.Fatalf("a different identity attached to a cached tunnel via Rdg-Connection-Id (status %q); the header was treated as authorization", line)
}
}