Skip to content
This repository was archived by the owner on Apr 16, 2026. It is now read-only.

Commit d06830b

Browse files
Harden session JWT revocation (#64)
* Harden session JWT revocation * Confirm sessionjwt skew report is false positive * test: fix session jwt claim parsing clock --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
1 parent 80461ee commit d06830b

12 files changed

Lines changed: 420 additions & 25 deletions

File tree

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TABLE sessions
2+
ADD COLUMN IF NOT EXISTS token_id TEXT NOT NULL DEFAULT '';

internal/app/service.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ func (s *Service) CreateSession(ctx context.Context, req *core.CreateSessionRequ
131131
State: core.SessionStateActive,
132132
CreatedAt: now,
133133
}
134+
session.TokenID = "tok_" + session.ID
134135
if err := s.repo.SaveSession(ctx, session); err != nil {
135136
return nil, fmt.Errorf("create session %q: save session: %w", session.ID, err)
136137
}
@@ -446,6 +447,11 @@ func (s *Service) RevokeSession(ctx context.Context, req *core.RevokeSessionRequ
446447
return fmt.Errorf("revoke session %q: save session: %w", session.ID, err)
447448
}
448449
s.metrics.recordSessionTransition(previousState, session.State, session.TenantID)
450+
if s.runtime != nil && session.TokenID != "" {
451+
if err := s.runtime.RevokeSessionToken(ctx, session.TokenID, session.ExpiresAt); err != nil {
452+
return fmt.Errorf("revoke session %q: revoke session token %q: %w", session.ID, session.TokenID, err)
453+
}
454+
}
449455

450456
grants, err := s.repo.ListGrantsBySession(ctx, req.SessionID)
451457
if err != nil {
@@ -836,6 +842,15 @@ func (s *Service) loadActiveSession(ctx context.Context, raw string) (*core.Sess
836842
if err != nil {
837843
return nil, nil, fmt.Errorf("load active session: verify session token: %w", err)
838844
}
845+
if s.runtime != nil && claims.TokenID != "" {
846+
revoked, err := s.runtime.IsSessionTokenRevoked(ctx, claims.TokenID)
847+
if err != nil {
848+
return nil, nil, fmt.Errorf("load active session: check session token revocation: %w", err)
849+
}
850+
if revoked {
851+
return nil, nil, fmt.Errorf("%w: session token revoked", core.ErrForbidden)
852+
}
853+
}
839854
session, err := s.repo.GetSession(ctx, claims.SessionID)
840855
if err != nil {
841856
return nil, nil, fmt.Errorf("load active session %q: load session: %w", claims.SessionID, err)

internal/app/service_test.go

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,93 @@ func TestService_RevokeSessionRevokesOutstandingGrants(t *testing.T) {
573573
}
574574
}
575575

576+
func TestService_RequestGrantRejectsRevokedSessionToken(t *testing.T) {
577+
t.Parallel()
578+
579+
ctx := context.Background()
580+
now := testNow()
581+
repo := memstore.NewRepository()
582+
runtime := memstore.NewRuntimeStore()
583+
tools := toolregistry.New()
584+
engine := policy.NewEngine()
585+
signer := mustNewSigner(t)
586+
587+
mustPutTool(t, ctx, tools, core.Tool{
588+
TenantID: "t_acme",
589+
Tool: "github",
590+
ManifestHash: "sha256:test",
591+
RuntimeClass: core.RuntimeClassHosted,
592+
AllowedDeliveryModes: []core.DeliveryMode{core.DeliveryModeProxy},
593+
AllowedCapabilities: []string{"repo.read"},
594+
TrustTags: []string{"trusted", "github"},
595+
})
596+
mustPutPolicy(t, engine, core.Policy{
597+
TenantID: "t_acme",
598+
Capability: "repo.read",
599+
ResourceKind: core.ResourceKindGitHubRepo,
600+
AllowedDeliveryModes: []core.DeliveryMode{core.DeliveryModeProxy},
601+
DefaultTTL: 10 * time.Minute,
602+
MaxTTL: 10 * time.Minute,
603+
ApprovalMode: core.ApprovalModeNone,
604+
RequiredToolTags: []string{"trusted", "github"},
605+
Condition: `request.tool == "github"`,
606+
})
607+
608+
svc, err := app.NewService(app.Config{
609+
Clock: fixedClock(now),
610+
IDs: fixedIDs("sess_revoked"),
611+
Repository: repo,
612+
Runtime: runtime,
613+
Verifier: fakeVerifier{identity: workloadIdentity()},
614+
SessionTokens: signer,
615+
Policy: engine,
616+
Tools: tools,
617+
Connectors: fakeConnectorResolver{connector: &fakeConnector{kind: "github"}},
618+
Deliveries: map[core.DeliveryMode]core.DeliveryAdapter{
619+
core.DeliveryModeProxy: &fakeDeliveryAdapter{
620+
mode: core.DeliveryModeProxy,
621+
delivery: &core.Delivery{
622+
Kind: core.DeliveryKindProxyHandle,
623+
Handle: "ph_revoked",
624+
},
625+
},
626+
},
627+
})
628+
if err != nil {
629+
t.Fatalf("NewService() error = %v", err)
630+
}
631+
632+
sessionResp, err := svc.CreateSession(ctx, &core.CreateSessionRequest{
633+
TenantID: "t_acme",
634+
AgentID: "agent_pr_reviewer",
635+
RunID: "run_revoked",
636+
ToolContext: []string{"github"},
637+
Attestation: &core.Attestation{Kind: core.AttestationKindK8SServiceAccountJWT, Token: "jwt"},
638+
})
639+
if err != nil {
640+
t.Fatalf("CreateSession() error = %v", err)
641+
}
642+
643+
claims, err := signer.Verify(sessionResp.SessionToken)
644+
if err != nil {
645+
t.Fatalf("Verify() error = %v", err)
646+
}
647+
if err := runtime.RevokeSessionToken(ctx, claims.TokenID, claims.ExpiresAt); err != nil {
648+
t.Fatalf("RevokeSessionToken() error = %v", err)
649+
}
650+
651+
_, err = svc.RequestGrant(ctx, &core.RequestGrantRequest{
652+
SessionToken: sessionResp.SessionToken,
653+
Tool: "github",
654+
Capability: "repo.read",
655+
ResourceRef: "github:repo:acme/widgets",
656+
DeliveryMode: core.DeliveryModeProxy,
657+
})
658+
if err == nil || !strings.Contains(err.Error(), "session token revoked") {
659+
t.Fatalf("RequestGrant() error = %v, want revoked token failure", err)
660+
}
661+
}
662+
576663
func mustNewSigner(t *testing.T) core.SessionTokenManager {
577664
t.Helper()
578665

internal/bootstrap/service.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -403,18 +403,22 @@ func (v multiVerifier) Verify(ctx context.Context, in *core.Attestation) (*core.
403403
}
404404

405405
func newSessionTokenManager() (core.SessionTokenManager, error) {
406+
options := []sessionjwt.Option{}
407+
if issuer := strings.TrimSpace(os.Getenv("ASB_SESSION_TOKEN_ISSUER")); issuer != "" {
408+
options = append(options, sessionjwt.WithIssuer(issuer))
409+
}
406410
if path := os.Getenv("ASB_SESSION_SIGNING_PRIVATE_KEY_FILE"); path != "" {
407411
privateKey, err := loadEd25519PrivateKey(path)
408412
if err != nil {
409413
return nil, err
410414
}
411-
return sessionjwt.NewManager(privateKey)
415+
return sessionjwt.NewManager(privateKey, options...)
412416
}
413417
_, privateKey, err := ed25519.GenerateKey(rand.Reader)
414418
if err != nil {
415419
return nil, err
416420
}
417-
return sessionjwt.NewManager(privateKey)
421+
return sessionjwt.NewManager(privateKey, options...)
418422
}
419423

420424
func newDelegationValidator() (core.DelegationValidator, error) {

internal/core/types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ type Session struct {
133133
TenantID string
134134
AgentID string
135135
RunID string
136+
TokenID string
136137
WorkloadIdentity WorkloadIdentity
137138
Delegation *Delegation
138139
ToolContext []string
@@ -395,6 +396,7 @@ type SessionClaims struct {
395396
TenantID string
396397
AgentID string
397398
RunID string
399+
TokenID string
398400
ToolContext []string
399401
WorkloadHash string
400402
ExpiresAt time.Time
@@ -504,6 +506,8 @@ type RuntimeStore interface {
504506
CompleteProxyRequest(ctx context.Context, handle string, responseBytes int64) error
505507
SaveRelaySession(ctx context.Context, relay *BrowserRelaySession) error
506508
GetRelaySession(ctx context.Context, sessionID string) (*BrowserRelaySession, error)
509+
RevokeSessionToken(ctx context.Context, tokenID string, expiresAt time.Time) error
510+
IsSessionTokenRevoked(ctx context.Context, tokenID string) (bool, error)
507511
}
508512

509513
type GitHubProxyExecutor interface {

internal/crypto/sessionjwt/manager.go

Lines changed: 65 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,18 @@ import (
1212
type Manager struct {
1313
privateKey ed25519.PrivateKey
1414
publicKey ed25519.PublicKey
15+
issuer string
16+
clockSkew time.Duration
17+
now func() time.Time
1518
}
1619

20+
type Option func(*Manager)
21+
22+
const (
23+
defaultIssuer = "asb"
24+
defaultClockSkew = 30 * time.Second
25+
)
26+
1727
type claims struct {
1828
SessionID string `json:"sid"`
1929
TenantID string `json:"tenant_id"`
@@ -24,21 +34,56 @@ type claims struct {
2434
jwt.RegisteredClaims
2535
}
2636

27-
func NewManager(privateKey ed25519.PrivateKey) (*Manager, error) {
37+
func WithIssuer(issuer string) Option {
38+
return func(manager *Manager) {
39+
if issuer != "" {
40+
manager.issuer = issuer
41+
}
42+
}
43+
}
44+
45+
func WithClockSkew(clockSkew time.Duration) Option {
46+
return func(manager *Manager) {
47+
if clockSkew >= 0 {
48+
manager.clockSkew = clockSkew
49+
}
50+
}
51+
}
52+
53+
func WithNowFunc(now func() time.Time) Option {
54+
return func(manager *Manager) {
55+
if now != nil {
56+
manager.now = now
57+
}
58+
}
59+
}
60+
61+
func NewManager(privateKey ed25519.PrivateKey, options ...Option) (*Manager, error) {
2862
if len(privateKey) == 0 {
2963
return nil, fmt.Errorf("%w: private key is required", core.ErrInvalidRequest)
3064
}
3165
publicKey, ok := privateKey.Public().(ed25519.PublicKey)
3266
if !ok {
3367
return nil, fmt.Errorf("%w: private key public component is %T, want ed25519.PublicKey", core.ErrInvalidRequest, privateKey.Public())
3468
}
35-
return &Manager{
69+
manager := &Manager{
3670
privateKey: privateKey,
3771
publicKey: publicKey,
38-
}, nil
72+
issuer: defaultIssuer,
73+
clockSkew: defaultClockSkew,
74+
now: time.Now,
75+
}
76+
for _, option := range options {
77+
option(manager)
78+
}
79+
return manager, nil
3980
}
4081

4182
func (m *Manager) Sign(session *core.Session) (string, error) {
83+
tokenID := session.TokenID
84+
if tokenID == "" {
85+
tokenID = session.ID
86+
}
4287
token := jwt.NewWithClaims(jwt.SigningMethodEdDSA, claims{
4388
SessionID: session.ID,
4489
TenantID: session.TenantID,
@@ -49,7 +94,10 @@ func (m *Manager) Sign(session *core.Session) (string, error) {
4994
RegisteredClaims: jwt.RegisteredClaims{
5095
ExpiresAt: jwt.NewNumericDate(session.ExpiresAt),
5196
IssuedAt: jwt.NewNumericDate(session.CreatedAt),
97+
NotBefore: jwt.NewNumericDate(session.CreatedAt.Add(-m.clockSkew)),
98+
Issuer: m.issuer,
5299
Subject: session.WorkloadIdentity.Subject,
100+
ID: tokenID,
53101
},
54102
})
55103
return token.SignedString(m.privateKey)
@@ -61,7 +109,7 @@ func (m *Manager) Verify(raw string) (*core.SessionClaims, error) {
61109
return nil, fmt.Errorf("%w: unexpected signing method %q", core.ErrUnauthorized, token.Method.Alg())
62110
}
63111
return m.publicKey, nil
64-
})
112+
}, jwt.WithIssuer(m.issuer), jwt.WithIssuedAt(), jwt.WithLeeway(m.clockSkew), jwt.WithTimeFunc(m.now))
65113
if err != nil {
66114
return nil, fmt.Errorf("%w: %v", core.ErrUnauthorized, err)
67115
}
@@ -70,21 +118,27 @@ func (m *Manager) Verify(raw string) (*core.SessionClaims, error) {
70118
if !ok || !parsed.Valid {
71119
return nil, fmt.Errorf("%w: invalid session token", core.ErrUnauthorized)
72120
}
121+
if tokenClaims.ExpiresAt == nil {
122+
return nil, fmt.Errorf("%w: missing exp", core.ErrUnauthorized)
123+
}
124+
if tokenClaims.NotBefore == nil {
125+
return nil, fmt.Errorf("%w: missing nbf", core.ErrUnauthorized)
126+
}
127+
if tokenClaims.ID == "" {
128+
return nil, fmt.Errorf("%w: missing jti", core.ErrUnauthorized)
129+
}
130+
if tokenClaims.SessionID == "" || tokenClaims.TenantID == "" {
131+
return nil, fmt.Errorf("%w: missing required session claims", core.ErrUnauthorized)
132+
}
73133

74134
return &core.SessionClaims{
75135
SessionID: tokenClaims.SessionID,
76136
TenantID: tokenClaims.TenantID,
77137
AgentID: tokenClaims.AgentID,
78138
RunID: tokenClaims.RunID,
139+
TokenID: tokenClaims.ID,
79140
ToolContext: append([]string(nil), tokenClaims.ToolContext...),
80141
WorkloadHash: tokenClaims.WorkloadHash,
81142
ExpiresAt: tokenClaims.ExpiresAt.Time,
82143
}, nil
83144
}
84-
85-
func (c *claims) Valid() error {
86-
if c.ExpiresAt == nil {
87-
return fmt.Errorf("%w: missing exp", core.ErrUnauthorized)
88-
}
89-
return jwt.NewValidator(jwt.WithTimeFunc(time.Now)).Validate(c.RegisteredClaims)
90-
}

0 commit comments

Comments
 (0)