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") + } +}