From 49fa17002300520b3dcbf987b6563e50772443bc Mon Sep 17 00:00:00 2001 From: bolkedebruin Date: Thu, 30 Apr 2026 18:22:22 +0200 Subject: [PATCH] Make the PAA cookie self-contained and shrink it (#187) CheckPAACookie used to call OIDCProvider.UserInfo with a copy of the IdP access token embedded in the cookie itself, just to recover the Subject. The gateway already signs Subject at issue time, so the roundtrip is redundant -- and copying the IdP access token into the .rdp file makes the cookie much larger and ties gateway availability to IdP availability. Drop the AccessToken field from customClaims and from GeneratePAAToken (no other consumer exists), set tunnel.User.SetUserName from the signed Subject claim, and remove the UserInfo call from CheckPAACookie. Add Audience: "rdpgw-paa" to standard claims at issue and AnyAudience to the validation expectation so a JWS minted with the same signing key for any other purpose can't be presented as a PAA. For a representative RS256 access token the cookie shrinks from ~961 bytes to ~259 bytes. Adds tests: - TestPAACookieDoesNotEmbedAccessToken - TestPAACookieHasAudienceClaim - TestCheckPAACookieIsSelfContained --- cmd/rdpgw/security/jwt.go | 36 +++++----- cmd/rdpgw/security/jwt_test.go | 120 ++++++++++++++++++++++++++++++++- 2 files changed, 136 insertions(+), 20 deletions(-) diff --git a/cmd/rdpgw/security/jwt.go b/cmd/rdpgw/security/jwt.go index 40ffade..7752bf8 100644 --- a/cmd/rdpgw/security/jwt.go +++ b/cmd/rdpgw/security/jwt.go @@ -4,16 +4,19 @@ import ( "context" "errors" "fmt" + "log" + "time" + "github.com/bolkedebruin/rdpgw/cmd/rdpgw/identity" "github.com/bolkedebruin/rdpgw/cmd/rdpgw/protocol" "github.com/coreos/go-oidc/v3/oidc" "github.com/go-jose/go-jose/v4" "github.com/go-jose/go-jose/v4/jwt" "golang.org/x/oauth2" - "log" - "time" ) +const paaAudience = "rdpgw-paa" + var ( SigningKey []byte EncryptionKey []byte @@ -30,7 +33,6 @@ var VerifyClientIP bool = true type customClaims struct { RemoteServer string `json:"remoteServer"` ClientIP string `json:"clientIp"` - AccessToken string `json:"accessToken"` } func CheckSession(next protocol.CheckHostFunc) protocol.CheckHostFunc { @@ -87,28 +89,24 @@ func CheckPAACookie(ctx context.Context, tokenString string) (bool, error) { // ...but doesn't check the expiry claim :/ err = standard.Validate(jwt.Expected{ - Issuer: "rdpgw", - Time: time.Now(), + Issuer: "rdpgw", + AnyAudience: jwt.Audience{paaAudience}, + Time: time.Now(), }) if err != nil { - log.Printf("token validation failed due to %tunnel", err) - return false, err - } - - // validate the access token - tokenSource := Oauth2Config.TokenSource(ctx, &oauth2.Token{AccessToken: custom.AccessToken}) - user, err := OIDCProvider.UserInfo(ctx, tokenSource) - if err != nil { - log.Printf("Cannot get user info for access token: %tunnel", err) + log.Printf("token validation failed due to %s", err) return false, err } tunnel := getTunnel(ctx) + if tunnel == nil { + return false, errors.New("no tunnel in context") + } tunnel.TargetServer = custom.RemoteServer tunnel.RemoteAddr = custom.ClientIP - tunnel.User.SetUserName(user.Subject) + tunnel.User.SetUserName(standard.Subject) return true, nil } @@ -124,16 +122,16 @@ func GeneratePAAToken(ctx context.Context, username string, server string) (stri } standard := jwt.Claims{ - Issuer: "rdpgw", - Expiry: jwt.NewNumericDate(time.Now().Add(time.Minute * 5)), - Subject: username, + Issuer: "rdpgw", + Audience: jwt.Audience{paaAudience}, + Expiry: jwt.NewNumericDate(time.Now().Add(time.Minute * 5)), + Subject: username, } id := identity.FromCtx(ctx) private := customClaims{ RemoteServer: server, ClientIP: id.GetAttribute(identity.AttrClientIp).(string), - AccessToken: id.GetAttribute(identity.AttrAccessToken).(string), } if token, err := jwt.Signed(sig).Claims(standard).Claims(private).Serialize(); err != nil { diff --git a/cmd/rdpgw/security/jwt_test.go b/cmd/rdpgw/security/jwt_test.go index 84dc876..8a764c4 100644 --- a/cmd/rdpgw/security/jwt_test.go +++ b/cmd/rdpgw/security/jwt_test.go @@ -2,8 +2,13 @@ package security import ( "context" - "github.com/bolkedebruin/rdpgw/cmd/rdpgw/identity" + "encoding/base64" + "strings" "testing" + + "github.com/bolkedebruin/rdpgw/cmd/rdpgw/identity" + "github.com/bolkedebruin/rdpgw/cmd/rdpgw/protocol" + "golang.org/x/oauth2" ) func TestGenerateUserToken(t *testing.T) { @@ -74,3 +79,116 @@ func TestPAACookie(t *testing.T) { t.Fatalf("CheckPAACookie failed") }*/ } + +// paaPayload returns the decoded JSON payload of a signed JWT (compact +// serialization: header.payload.signature). The PAA cookie is a JWS over +// HS256, so the payload is base64url-decodable plaintext. +func paaPayload(t *testing.T, token string) string { + t.Helper() + parts := strings.Split(token, ".") + if len(parts) != 3 { + t.Fatalf("expected JWS compact (3 parts), got %d in %q", len(parts), token) + } + raw, err := base64.RawURLEncoding.DecodeString(parts[1]) + if err != nil { + t.Fatalf("decode payload: %v", err) + } + return string(raw) +} + +// TestPAACookieDoesNotEmbedAccessToken asserts that the PAA cookie does not +// carry the IdP access token in its payload. The access token is a bearer +// credential for other OIDC-protected resources; embedding it in a cookie +// that travels in the .rdp file (or any access log) leaks it well beyond +// the gateway. +func TestPAACookieDoesNotEmbedAccessToken(t *testing.T) { + SigningKey = []byte("5aa3a1568fe8421cd7e127d5ace28d2d") + + const accessToken = "redacted-idp-access-token-1234567890abcdef" + + id := identity.NewUser() + id.SetUserName("alice") + id.SetAttribute(identity.AttrClientIp, "10.0.0.1") + id.SetAttribute(identity.AttrAccessToken, accessToken) + ctx := context.WithValue(context.Background(), identity.CTXKey, id) + + token, err := GeneratePAAToken(ctx, "alice", "rdp.example.com") + if err != nil { + t.Fatalf("GeneratePAAToken: %v", err) + } + payload := paaPayload(t, token) + if strings.Contains(payload, accessToken) { + t.Errorf("PAA cookie embeds the IdP access token in plaintext\npayload: %s", payload) + } +} + +// TestPAACookieHasAudienceClaim asserts that the PAA cookie declares its +// audience. Without `aud`, any JWS the rdpgw signing key produces (a future +// non-PAA token, a maintainer-tool token, ...) would be indistinguishable +// from a PAA cookie at the gateway endpoint. +func TestPAACookieHasAudienceClaim(t *testing.T) { + SigningKey = []byte("5aa3a1568fe8421cd7e127d5ace28d2d") + + id := identity.NewUser() + id.SetUserName("alice") + id.SetAttribute(identity.AttrClientIp, "10.0.0.1") + id.SetAttribute(identity.AttrAccessToken, "irrelevant") + ctx := context.WithValue(context.Background(), identity.CTXKey, id) + + token, err := GeneratePAAToken(ctx, "alice", "rdp.example.com") + if err != nil { + t.Fatalf("GeneratePAAToken: %v", err) + } + payload := paaPayload(t, token) + if !strings.Contains(payload, `"aud"`) { + t.Errorf("PAA cookie has no aud claim\npayload: %s", payload) + } +} + +// TestCheckPAACookieIsSelfContained asserts that validating a PAA cookie +// does not require a live IdP. Today CheckPAACookie calls +// OIDCProvider.UserInfo with the embedded access token to look up the +// subject; both the network roundtrip and the dependency on a still-live +// access token are unnecessary because the gateway already signed Subject +// itself at issue time. +func TestCheckPAACookieIsSelfContained(t *testing.T) { + SigningKey = []byte("5aa3a1568fe8421cd7e127d5ace28d2d") + OIDCProvider = nil + Oauth2Config = oauth2.Config{} + VerifyClientIP = false + + issueId := identity.NewUser() + issueId.SetUserName("alice") + issueId.SetAttribute(identity.AttrClientIp, "10.0.0.1") + issueId.SetAttribute(identity.AttrAccessToken, "irrelevant") + issueCtx := context.WithValue(context.Background(), identity.CTXKey, issueId) + + token, err := GeneratePAAToken(issueCtx, "alice", "rdp.example.com") + if err != nil { + t.Fatalf("GeneratePAAToken: %v", err) + } + + checkId := identity.NewUser() + checkId.SetUserName("alice") + checkId.SetAttribute(identity.AttrClientIp, "10.0.0.1") + checkCtx := context.WithValue(context.Background(), identity.CTXKey, checkId) + tun := &protocol.Tunnel{User: checkId} + checkCtx = context.WithValue(checkCtx, protocol.CtxTunnel, tun) + + defer func() { + if r := recover(); r != nil { + t.Errorf("CheckPAACookie panicked without a live IdP (expected: trust the signed Subject): %v", r) + } + }() + + ok, err := CheckPAACookie(checkCtx, token) + if err != nil { + t.Errorf("CheckPAACookie returned error: %v", err) + } + if !ok { + t.Errorf("CheckPAACookie returned ok=false for a valid cookie") + } + if tun.User.UserName() != "alice" { + t.Errorf("tunnel.User = %q, want %q", tun.User.UserName(), "alice") + } +}