From bc33ba9b1b9b6fae17cac78aa38ae826d3e649d5 Mon Sep 17 00:00:00 2001 From: Anton Filonenko Date: Tue, 2 Jun 2026 17:33:16 +0300 Subject: [PATCH 1/2] fix(session-key): allow wallet-only revocation of session keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MF3-M05: SubmitSessionKeyState required session_key_sig on every submit, including revocation (expires_at <= now). This let a lost, unavailable, or malicious delegate veto revocation of its own delegation — the user had to wait for the prior expires_at to pass while the key stayed usable. Revocation now validates user_sig alone; activation, extension, and rotation (expires_at > now) still require both user_sig and session_key_sig. A session_key_sig present on the revocation path is ignored. - pkg/app, pkg/core: add ValidateAppSessionKeyStateUserSigV1 / ValidateChannelSessionKeyStateUserSigV1; refactor the both-signature validators to reuse them. - nitronode handlers: branch on expires_at to pick the validator; capture now once and reuse it for the in-tx cap/version logic. - migration: drop the session_key_sig non-empty CHECK constraints added in 20260508000000. Active rows stay co-signed via app code, and owner/auth lookups filter expires_at > now, so revocation rows are never trusted. - sdk/go, sdk/ts: add wallet-only RevokeChannelSessionKey / RevokeAppSessionKey (revokeChannelSessionKey / revokeSessionKey) helpers. - docs/api.yaml: document session_key_sig as optional for revocation. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/api.yaml | 33 +++-- .../submit_session_key_state.go | 35 +++-- .../submit_session_key_state_test.go | 121 ++++++++++++++++++ .../channel_v1/submit_session_key_state.go | 35 +++-- .../submit_session_key_state_test.go | 121 ++++++++++++++++++ ...00000_session_key_user_only_revocation.sql | 29 +++++ pkg/app/session_key_v1.go | 48 +++++-- pkg/app/session_key_v1_test.go | 40 ++++++ pkg/core/session_key.go | 54 ++++++-- pkg/core/session_key_test.go | 44 +++++++ sdk/go/app_session.go | 50 +++++++- sdk/go/channel.go | 49 ++++++- sdk/ts/src/client.ts | 70 ++++++++-- .../public-api-drift.test.ts.snap | 2 + 14 files changed, 659 insertions(+), 72 deletions(-) create mode 100644 nitronode/config/migrations/postgres/20260602000000_session_key_user_only_revocation.sql diff --git a/docs/api.yaml b/docs/api.yaml index 0188f4f26..88e44557e 100644 --- a/docs/api.yaml +++ b/docs/api.yaml @@ -391,7 +391,10 @@ types: description: > Session-key holder's EIP-191 signature over the same payload as user_sig — abi.encode(SESSION_KEY_AUTH_TYPEHASH, session_key, metadata_hash) — proving possession of the key - being registered. Required on every submit to prevent registration of keys the submitter does not control. + being registered. Required when expires_at is in the future (registration, update, re-activation) + to prevent registration of keys the submitter does not control. Optional for revocation + (expires_at at or before now): a wallet may revoke unilaterally with user_sig alone so a lost or + uncooperative session key cannot block revocation. - app_session_key_state: description: Represents the state of an app session key @@ -423,7 +426,7 @@ types: description: User's signature over the session key metadata to authorize the registration/update of the session key - name: session_key_sig type: string - description: Session-key holder's signature over the same packed state (which already binds user_address) proving possession of the key being registered. Required on every submit to prevent registration of keys the submitter does not control. + description: Session-key holder's signature over the same packed state (which already binds user_address) proving possession of the key being registered. Required when expires_at is in the future (registration, update, re-activation) to prevent registration of keys the submitter does not control. Optional for revocation (expires_at at or before now); a wallet may revoke unilaterally with user_sig alone so a lost or uncooperative session key cannot block revocation. - app: description: Application definition @@ -661,15 +664,16 @@ api: The metadata hash binds `expires_at`, so each revoke or re-activation requires a fresh user signature over the new payload. Negative unix timestamps are rejected. - Both `user_sig` (wallet) and `session_key_sig` (session-key holder) are required - on every submit, including the revocation path — the session key must co-sign its - own deactivation. Wallet-only revocation (for a lost or compromised key) is not - supported by this method; that flow requires a separate code path and is tracked - as a follow-up. + Activation, update, and re-activation (`expires_at` in the future) require both + `user_sig` (wallet) and `session_key_sig` (session-key holder, proving possession). + Revocation (`expires_at` at or before now) requires only `user_sig`: a wallet may + revoke unilaterally so a lost, unavailable, or compromised session key cannot block + revocation of its own delegation. Any `session_key_sig` supplied on the revocation + path is ignored. request: - field_name: state type: channel_session_key_state - description: Session key metadata and delegation information. Set `expires_at` to a past value to revoke; future to (re-)activate. + description: Session key metadata and delegation information. Set `expires_at` to a past value to revoke (user_sig only); future to (re-)activate (both signatures). response: [] errors: - message: invalid_session_key_state @@ -876,15 +880,16 @@ api: The metadata hash binds `expires_at`, so each revoke or re-activation requires a fresh user signature over the new payload. Negative unix timestamps are rejected. - Both `user_sig` (wallet) and `session_key_sig` (session-key holder) are required - on every submit, including the revocation path — the session key must co-sign its - own deactivation. Wallet-only revocation (for a lost or compromised key) is not - supported by this method; that flow requires a separate code path and is tracked - as a follow-up. + Activation, update, and re-activation (`expires_at` in the future) require both + `user_sig` (wallet) and `session_key_sig` (session-key holder, proving possession). + Revocation (`expires_at` at or before now) requires only `user_sig`: a wallet may + revoke unilaterally so a lost, unavailable, or compromised session key cannot block + revocation of its own delegation. Any `session_key_sig` supplied on the revocation + path is ignored. request: - field_name: state type: app_session_key_state - description: Session key metadata and delegation information. Set `expires_at` to a past value to revoke; future to (re-)activate. + description: Session key metadata and delegation information. Set `expires_at` to a past value to revoke (user_sig only); future to (re-)activate (both signatures). response: [] errors: - message: invalid_session_key_state diff --git a/nitronode/api/app_session_v1/submit_session_key_state.go b/nitronode/api/app_session_v1/submit_session_key_state.go index 62f5cf9c8..f9b85e233 100644 --- a/nitronode/api/app_session_v1/submit_session_key_state.go +++ b/nitronode/api/app_session_v1/submit_session_key_state.go @@ -92,19 +92,34 @@ func (h *Handler) SubmitSessionKeyState(c *rpc.Context) { c.Fail(rpc.Errorf("invalid_session_key_state: user_sig is required"), "") return } - if coreState.SessionKeySig == "" { - c.Fail(rpc.Errorf("invalid_session_key_state: session_key_sig is required"), "") - return - } - // Validate both signatures: wallet's UserSig and session-key holder's SessionKeySig. - if err := app.ValidateAppSessionKeyStateV1(coreState); err != nil { - c.Fail(rpc.Errorf("invalid_session_key_state: %v", err), "") - return + // A submit with expires_at after now activates, extends, or rotates the key and requires + // both signatures. A submit with expires_at <= now is a revocation: it only deactivates an + // existing delegation, so the wallet's user_sig alone authorizes it. Requiring session_key_sig + // on the revocation path would let a lost, unavailable, or malicious session key veto its own + // revocation, stranding the user until the prior expires_at naturally passes. now is captured + // here and reused for the cap/version logic inside the transaction so the active/inactive + // decision is consistent across both. + now := time.Now() + if coreState.ExpiresAt.After(now) { + if coreState.SessionKeySig == "" { + c.Fail(rpc.Errorf("invalid_session_key_state: session_key_sig is required"), "") + return + } + // Validate both signatures: wallet's UserSig and session-key holder's SessionKeySig. + if err := app.ValidateAppSessionKeyStateV1(coreState); err != nil { + c.Fail(rpc.Errorf("invalid_session_key_state: %v", err), "") + return + } + } else { + // Revocation: validate only the wallet's UserSig. Any session_key_sig present is ignored. + if err := app.ValidateAppSessionKeyStateUserSigV1(coreState); err != nil { + c.Fail(rpc.Errorf("invalid_session_key_state: %v", err), "") + return + } } - // Validate version and store the session key state - now := time.Now() + // Validate version and store the session key state. err = h.useStoreInTx(func(tx Store) error { // Lock the (user, session_key, app_session) pointer row for the duration of the tx so // that concurrent submits for the same (user, session_key) serialize cleanly and report diff --git a/nitronode/api/app_session_v1/submit_session_key_state_test.go b/nitronode/api/app_session_v1/submit_session_key_state_test.go index d12253961..f898bf328 100644 --- a/nitronode/api/app_session_v1/submit_session_key_state_test.go +++ b/nitronode/api/app_session_v1/submit_session_key_state_test.go @@ -883,6 +883,127 @@ func TestSubmitSessionKeyState_RejectsMismatchedSessionKeySig(t *testing.T) { mockStore.AssertNotCalled(t, "LockSessionKeyState", mock.Anything, mock.Anything, mock.Anything) } +// Wallet-only revocation: a submit with a past expires_at and NO session_key_sig is accepted +// on the strength of user_sig alone. This is the core remediation — a lost, unavailable, or +// uncooperative session key can no longer veto revocation of its own delegation. +func TestSubmitSessionKeyState_RevokeUserSigOnly_Succeeds(t *testing.T) { + mockStore := new(MockStore) + userSigner := NewMockSigner() + userAddress := strings.ToLower(userSigner.PublicKey().Address().String()) + sessionKeySigner := NewMockSigner() + sessionKeyAddress := strings.ToLower(sessionKeySigner.PublicKey().Address().String()) + + handler := &Handler{ + useStoreInTx: func(handler StoreTxHandler) error { + return handler(mockStore) + }, + metrics: metrics.NewNoopRuntimeMetricExporter(), + maxSessionKeyIDs: 10, + } + + expiresAt := time.Now().Add(-time.Hour).Truncate(time.Second) + // keySigner=nil → SessionKeySig stays empty; the revocation path must not require it. + reqPayload := buildSignedSessionKeyStateReq(t, userAddress, sessionKeyAddress, 2, nil, nil, expiresAt, userSigner, nil) + + prevActiveExpiresAt := time.Now().Add(24 * time.Hour) + mockStore.On("LockSessionKeyState", userAddress, sessionKeyAddress, database.SessionKeyKindAppSession).Return(1, prevActiveExpiresAt, nil) + mockStore.On("StoreAppSessionKeyState", mock.AnythingOfType("app.AppSessionKeyStateV1")).Return(nil) + + payload, err := rpc.NewPayload(reqPayload) + require.NoError(t, err) + + ctx := &rpc.Context{ + Context: context.Background(), + Request: rpc.NewRequest(1, rpc.AppSessionsV1SubmitSessionKeyStateMethod.String(), payload), + } + + handler.SubmitSessionKeyState(ctx) + + require.NotNil(t, ctx.Response) + assert.Nil(t, ctx.Response.Error()) + mockStore.AssertExpectations(t) +} + +// On the revocation path a present-but-mismatched session_key_sig is ignored, not validated. +// The same signature would be rejected on the active path (see RejectsMismatchedSessionKeySig). +func TestSubmitSessionKeyState_RevokeIgnoresMismatchedSessionKeySig(t *testing.T) { + mockStore := new(MockStore) + userSigner := NewMockSigner() + userAddress := strings.ToLower(userSigner.PublicKey().Address().String()) + sessionKeySigner := NewMockSigner() + sessionKeyAddress := strings.ToLower(sessionKeySigner.PublicKey().Address().String()) + otherSigner := NewMockSigner() + + handler := &Handler{ + useStoreInTx: func(handler StoreTxHandler) error { + return handler(mockStore) + }, + metrics: metrics.NewNoopRuntimeMetricExporter(), + maxSessionKeyIDs: 10, + } + + expiresAt := time.Now().Add(-time.Hour).Truncate(time.Second) + // SessionKeySig signed by an unrelated key — would fail the active path, ignored on revoke. + reqPayload := buildSignedSessionKeyStateReq(t, userAddress, sessionKeyAddress, 2, nil, nil, expiresAt, userSigner, otherSigner) + + prevActiveExpiresAt := time.Now().Add(24 * time.Hour) + mockStore.On("LockSessionKeyState", userAddress, sessionKeyAddress, database.SessionKeyKindAppSession).Return(1, prevActiveExpiresAt, nil) + mockStore.On("StoreAppSessionKeyState", mock.AnythingOfType("app.AppSessionKeyStateV1")).Return(nil) + + payload, err := rpc.NewPayload(reqPayload) + require.NoError(t, err) + + ctx := &rpc.Context{ + Context: context.Background(), + Request: rpc.NewRequest(1, rpc.AppSessionsV1SubmitSessionKeyStateMethod.String(), payload), + } + + handler.SubmitSessionKeyState(ctx) + + require.NotNil(t, ctx.Response) + assert.Nil(t, ctx.Response.Error()) + mockStore.AssertExpectations(t) +} + +// Even on the revocation path the wallet's user_sig must be valid: a revoke signed by a key +// other than the user_address is rejected, so revocation stays a wallet-only right (not anyone's). +func TestSubmitSessionKeyState_RevokeInvalidUserSig_Rejected(t *testing.T) { + mockStore := new(MockStore) + userSigner := NewMockSigner() + differentSigner := NewMockSigner() + userAddress := strings.ToLower(userSigner.PublicKey().Address().String()) + sessionKeySigner := NewMockSigner() + sessionKeyAddress := strings.ToLower(sessionKeySigner.PublicKey().Address().String()) + + handler := &Handler{ + useStoreInTx: func(handler StoreTxHandler) error { + return handler(mockStore) + }, + metrics: metrics.NewNoopRuntimeMetricExporter(), + maxSessionKeyIDs: 10, + } + + expiresAt := time.Now().Add(-time.Hour).Truncate(time.Second) + // user_sig produced by a key other than userAddress; no session_key_sig. + reqPayload := buildSignedSessionKeyStateReq(t, userAddress, sessionKeyAddress, 2, nil, nil, expiresAt, differentSigner, nil) + + payload, err := rpc.NewPayload(reqPayload) + require.NoError(t, err) + + ctx := &rpc.Context{ + Context: context.Background(), + Request: rpc.NewRequest(1, rpc.AppSessionsV1SubmitSessionKeyStateMethod.String(), payload), + } + + handler.SubmitSessionKeyState(ctx) + + require.NotNil(t, ctx.Response) + respErr := ctx.Response.Error() + require.NotNil(t, respErr) + assert.Contains(t, respErr.Error(), "user_sig does not match user_address") + mockStore.AssertNotCalled(t, "LockSessionKeyState", mock.Anything, mock.Anything, mock.Anything) +} + func TestSubmitSessionKeyState_RejectsForeignOwner(t *testing.T) { mockStore := new(MockStore) userSigner := NewMockSigner() diff --git a/nitronode/api/channel_v1/submit_session_key_state.go b/nitronode/api/channel_v1/submit_session_key_state.go index 5bd7db3f3..6b028c03c 100644 --- a/nitronode/api/channel_v1/submit_session_key_state.go +++ b/nitronode/api/channel_v1/submit_session_key_state.go @@ -75,19 +75,34 @@ func (h *Handler) SubmitSessionKeyState(c *rpc.Context) { c.Fail(rpc.Errorf("invalid_session_key_state: user_sig is required"), "") return } - if coreState.SessionKeySig == "" { - c.Fail(rpc.Errorf("invalid_session_key_state: session_key_sig is required"), "") - return - } - // Validate both signatures: wallet's user_sig and session-key holder's session_key_sig. - if err := core.ValidateChannelSessionKeyStateV1(coreState); err != nil { - c.Fail(rpc.Errorf("invalid_session_key_state: %v", err), "") - return + // A submit with expires_at after now activates, extends, or rotates the key and requires + // both signatures. A submit with expires_at <= now is a revocation: it only deactivates an + // existing delegation, so the wallet's user_sig alone authorizes it. Requiring session_key_sig + // on the revocation path would let a lost, unavailable, or malicious session key veto its own + // revocation, stranding the user until the prior expires_at naturally passes. now is captured + // here and reused for the cap/version logic inside the transaction so the active/inactive + // decision is consistent across both. + now := time.Now() + if coreState.ExpiresAt.After(now) { + if coreState.SessionKeySig == "" { + c.Fail(rpc.Errorf("invalid_session_key_state: session_key_sig is required"), "") + return + } + // Validate both signatures: wallet's user_sig and session-key holder's session_key_sig. + if err := core.ValidateChannelSessionKeyStateV1(coreState); err != nil { + c.Fail(rpc.Errorf("invalid_session_key_state: %v", err), "") + return + } + } else { + // Revocation: validate only the wallet's user_sig. Any session_key_sig present is ignored. + if err := core.ValidateChannelSessionKeyStateUserSigV1(coreState); err != nil { + c.Fail(rpc.Errorf("invalid_session_key_state: %v", err), "") + return + } } - // Validate version and store the session key state - now := time.Now() + // Validate version and store the session key state. err = h.useStoreInTx(func(tx Store) error { // Lock the (user, session_key, channel) pointer row for the duration of the tx so that // concurrent submits for the same (user, session_key) serialize cleanly and report a diff --git a/nitronode/api/channel_v1/submit_session_key_state_test.go b/nitronode/api/channel_v1/submit_session_key_state_test.go index 80ebcf0ca..c036a3e56 100644 --- a/nitronode/api/channel_v1/submit_session_key_state_test.go +++ b/nitronode/api/channel_v1/submit_session_key_state_test.go @@ -723,6 +723,127 @@ func TestChannelSubmitSessionKeyState_RejectsMismatchedSessionKeySig(t *testing. mockStore.AssertNotCalled(t, "LockSessionKeyState", mock.Anything, mock.Anything, mock.Anything) } +// Wallet-only revocation: a submit with a past expires_at and NO session_key_sig is accepted +// on the strength of user_sig alone. This is the core remediation — a lost, unavailable, or +// uncooperative session key can no longer veto revocation of its own delegation. +func TestChannelSubmitSessionKeyState_RevokeUserSigOnly_Succeeds(t *testing.T) { + mockStore := new(MockStore) + userSigner := NewMockSigner() + userAddress := strings.ToLower(userSigner.PublicKey().Address().String()) + sessionKeySigner := NewMockSigner() + sessionKeyAddress := strings.ToLower(sessionKeySigner.PublicKey().Address().String()) + + handler := &Handler{ + useStoreInTx: func(handler StoreTxHandler) error { + return handler(mockStore) + }, + metrics: metrics.NewNoopRuntimeMetricExporter(), + maxSessionKeyIDs: 10, + } + + expiresAt := time.Now().Add(-time.Hour).Truncate(time.Second) + // keySigner=nil → SessionKeySig stays empty; the revocation path must not require it. + reqPayload := buildSignedChannelSessionKeyStateReq(t, userAddress, sessionKeyAddress, 2, []string{}, expiresAt, userSigner, nil) + + prevActiveExpiresAt := time.Now().Add(24 * time.Hour) + mockStore.On("LockSessionKeyState", userAddress, sessionKeyAddress, database.SessionKeyKindChannel).Return(1, prevActiveExpiresAt, nil) + mockStore.On("StoreChannelSessionKeyState", mock.AnythingOfType("core.ChannelSessionKeyStateV1")).Return(nil) + + payload, err := rpc.NewPayload(reqPayload) + require.NoError(t, err) + + ctx := &rpc.Context{ + Context: context.Background(), + Request: rpc.NewRequest(1, rpc.ChannelsV1SubmitSessionKeyStateMethod.String(), payload), + } + + handler.SubmitSessionKeyState(ctx) + + require.NotNil(t, ctx.Response) + assert.Nil(t, ctx.Response.Error()) + mockStore.AssertExpectations(t) +} + +// On the revocation path a present-but-mismatched session_key_sig is ignored, not validated. +// The same signature would be rejected on the active path (see RejectsMismatchedSessionKeySig). +func TestChannelSubmitSessionKeyState_RevokeIgnoresMismatchedSessionKeySig(t *testing.T) { + mockStore := new(MockStore) + userSigner := NewMockSigner() + userAddress := strings.ToLower(userSigner.PublicKey().Address().String()) + sessionKeySigner := NewMockSigner() + sessionKeyAddress := strings.ToLower(sessionKeySigner.PublicKey().Address().String()) + otherSigner := NewMockSigner() + + handler := &Handler{ + useStoreInTx: func(handler StoreTxHandler) error { + return handler(mockStore) + }, + metrics: metrics.NewNoopRuntimeMetricExporter(), + maxSessionKeyIDs: 10, + } + + expiresAt := time.Now().Add(-time.Hour).Truncate(time.Second) + // SessionKeySig signed by an unrelated key — would fail the active path, ignored on revoke. + reqPayload := buildSignedChannelSessionKeyStateReq(t, userAddress, sessionKeyAddress, 2, []string{}, expiresAt, userSigner, otherSigner) + + prevActiveExpiresAt := time.Now().Add(24 * time.Hour) + mockStore.On("LockSessionKeyState", userAddress, sessionKeyAddress, database.SessionKeyKindChannel).Return(1, prevActiveExpiresAt, nil) + mockStore.On("StoreChannelSessionKeyState", mock.AnythingOfType("core.ChannelSessionKeyStateV1")).Return(nil) + + payload, err := rpc.NewPayload(reqPayload) + require.NoError(t, err) + + ctx := &rpc.Context{ + Context: context.Background(), + Request: rpc.NewRequest(1, rpc.ChannelsV1SubmitSessionKeyStateMethod.String(), payload), + } + + handler.SubmitSessionKeyState(ctx) + + require.NotNil(t, ctx.Response) + assert.Nil(t, ctx.Response.Error()) + mockStore.AssertExpectations(t) +} + +// Even on the revocation path the wallet's user_sig must be valid: a revoke signed by a key +// other than the user_address is rejected, so revocation stays a wallet-only right (not anyone's). +func TestChannelSubmitSessionKeyState_RevokeInvalidUserSig_Rejected(t *testing.T) { + mockStore := new(MockStore) + userSigner := NewMockSigner() + differentSigner := NewMockSigner() + userAddress := strings.ToLower(userSigner.PublicKey().Address().String()) + sessionKeySigner := NewMockSigner() + sessionKeyAddress := strings.ToLower(sessionKeySigner.PublicKey().Address().String()) + + handler := &Handler{ + useStoreInTx: func(handler StoreTxHandler) error { + return handler(mockStore) + }, + metrics: metrics.NewNoopRuntimeMetricExporter(), + maxSessionKeyIDs: 10, + } + + expiresAt := time.Now().Add(-time.Hour).Truncate(time.Second) + // user_sig produced by a key other than userAddress; no session_key_sig. + reqPayload := buildSignedChannelSessionKeyStateReq(t, userAddress, sessionKeyAddress, 2, []string{}, expiresAt, differentSigner, nil) + + payload, err := rpc.NewPayload(reqPayload) + require.NoError(t, err) + + ctx := &rpc.Context{ + Context: context.Background(), + Request: rpc.NewRequest(1, rpc.ChannelsV1SubmitSessionKeyStateMethod.String(), payload), + } + + handler.SubmitSessionKeyState(ctx) + + require.NotNil(t, ctx.Response) + respErr := ctx.Response.Error() + require.NotNil(t, respErr) + assert.Contains(t, respErr.Error(), "does not match wallet") + mockStore.AssertNotCalled(t, "LockSessionKeyState", mock.Anything, mock.Anything, mock.Anything) +} + func TestChannelSubmitSessionKeyState_RejectsForeignOwner(t *testing.T) { mockStore := new(MockStore) userSigner := NewMockSigner() diff --git a/nitronode/config/migrations/postgres/20260602000000_session_key_user_only_revocation.sql b/nitronode/config/migrations/postgres/20260602000000_session_key_user_only_revocation.sql new file mode 100644 index 000000000..ae3983d1c --- /dev/null +++ b/nitronode/config/migrations/postgres/20260602000000_session_key_user_only_revocation.sql @@ -0,0 +1,29 @@ +-- +goose Up +-- User-only session-key revocation: a submit with expires_at <= now deactivates a delegation +-- and is authorized by the wallet's user_sig alone, so the session-key holder cannot veto +-- revocation of a lost, unavailable, or compromised key. Such revocation rows carry an empty +-- session_key_sig, which the *_session_key_sig_present_chk CHECK constraints added in +-- 20260508000000_session_key_ownership_constraints reject. Drop them. +-- +-- The ownership guarantee those constraints protected is preserved in application code: submits +-- that activate, extend, or rotate a key (expires_at > now) still require a valid session_key_sig, +-- so every *active* history row remains co-signed by the session-key holder. Owner and auth +-- lookups filter expires_at > now, so revocation rows with an empty session_key_sig are never +-- trusted as a session key's owner. +ALTER TABLE app_session_key_states_v1 + DROP CONSTRAINT IF EXISTS app_session_key_states_v1_session_key_sig_present_chk; + +ALTER TABLE channel_session_key_states_v1 + DROP CONSTRAINT IF EXISTS channel_session_key_states_v1_session_key_sig_present_chk; + +-- +goose Down +-- Re-add as NOT VALID: skip the legacy backfill scan (revocation rows with an empty +-- session_key_sig may now exist) so the down migration cannot fail on pre-existing data; only +-- future inserts are checked. Matches how 20260508000000 originally added them. +ALTER TABLE app_session_key_states_v1 + ADD CONSTRAINT app_session_key_states_v1_session_key_sig_present_chk + CHECK (session_key_sig IS NOT NULL AND session_key_sig <> '') NOT VALID; + +ALTER TABLE channel_session_key_states_v1 + ADD CONSTRAINT channel_session_key_states_v1_session_key_sig_present_chk + CHECK (session_key_sig IS NOT NULL AND session_key_sig <> '') NOT VALID; diff --git a/pkg/app/session_key_v1.go b/pkg/app/session_key_v1.go index f377ed71c..f3268fdf1 100644 --- a/pkg/app/session_key_v1.go +++ b/pkg/app/session_key_v1.go @@ -54,17 +54,14 @@ func GenerateSessionKeyStateIDV1(userAddress, sessionKey string, version uint64) return crypto.Keccak256Hash(packed).Hex(), nil } -// ValidateAppSessionKeyStateV1 verifies both signatures over the registration payload: -// UserSig must recover to state.UserAddress (wallet authorizes the delegation) and -// SessionKeySig must recover to state.SessionKey (session-key holder proves possession). -// Both signatures sign the same PackAppSessionKeyStateV1(state) payload, which already binds -// user_address and session_key — so a signature minted for one (wallet, session_key) pair -// cannot be replayed for another. -func ValidateAppSessionKeyStateV1(state AppSessionKeyStateV1) error { - if state.SessionKeySig == "" { - return fmt.Errorf("session_key_sig is required") - } - +// ValidateAppSessionKeyStateUserSigV1 verifies only UserSig over the registration payload: +// UserSig must recover to state.UserAddress (wallet authorizes the change). This is the +// revocation path (submitted expires_at <= now): the session-key holder's SessionKeySig is +// intentionally not required so a lost, unavailable, or malicious delegate cannot veto the +// wallet's revocation of its own delegation. The packed payload binds user_address, +// session_key, version and expires_at, so the signature authorizes exactly this revocation and +// cannot be replayed for another key, wallet, or version. +func ValidateAppSessionKeyStateUserSigV1(state AppSessionKeyStateV1) error { packed, err := PackAppSessionKeyStateV1(state) if err != nil { return fmt.Errorf("failed to pack session key state: %w", err) @@ -87,6 +84,35 @@ func ValidateAppSessionKeyStateV1(state AppSessionKeyStateV1) error { return fmt.Errorf("user_sig does not match user_address") } + return nil +} + +// ValidateAppSessionKeyStateV1 verifies both signatures over the registration payload: +// UserSig must recover to state.UserAddress (wallet authorizes the delegation) and +// SessionKeySig must recover to state.SessionKey (session-key holder proves possession). +// Both signatures sign the same PackAppSessionKeyStateV1(state) payload, which already binds +// user_address and session_key — so a signature minted for one (wallet, session_key) pair +// cannot be replayed for another. Used for activation, extension, and rotation (submitted +// expires_at > now); revocation uses ValidateAppSessionKeyStateUserSigV1. +func ValidateAppSessionKeyStateV1(state AppSessionKeyStateV1) error { + if state.SessionKeySig == "" { + return fmt.Errorf("session_key_sig is required") + } + + if err := ValidateAppSessionKeyStateUserSigV1(state); err != nil { + return err + } + + packed, err := PackAppSessionKeyStateV1(state) + if err != nil { + return fmt.Errorf("failed to pack session key state: %w", err) + } + + recoverer, err := sign.NewAddressRecoverer(sign.TypeEthereumMsg) + if err != nil { + return fmt.Errorf("failed to create address recoverer: %w", err) + } + sessionKeySigBytes, err := hexutil.Decode(state.SessionKeySig) if err != nil { return fmt.Errorf("failed to decode session_key_sig: %w", err) diff --git a/pkg/app/session_key_v1_test.go b/pkg/app/session_key_v1_test.go index ef3602890..deead3456 100644 --- a/pkg/app/session_key_v1_test.go +++ b/pkg/app/session_key_v1_test.go @@ -130,6 +130,46 @@ func TestValidateAppSessionKeyStateV1(t *testing.T) { assert.Error(t, ValidateAppSessionKeyStateV1(stateCrossKey)) } +func TestValidateAppSessionKeyStateUserSigV1(t *testing.T) { + t.Parallel() + userSigner, userAddress := createTestSigner(t) + _, sessionKeyAddr := createTestSigner(t) + + baseState := AppSessionKeyStateV1{ + UserAddress: userAddress, + SessionKey: sessionKeyAddr, + Version: 1, + ExpiresAt: time.Now().Add(-time.Hour), // revocation + } + + packed, err := PackAppSessionKeyStateV1(baseState) + require.NoError(t, err) + userSig, err := userSigner.Sign(packed) + require.NoError(t, err) + + state := baseState + state.UserSig = hexutil.Encode(userSig) + + // Valid user_sig alone passes — no session_key_sig required on the revocation path. + require.NoError(t, ValidateAppSessionKeyStateUserSigV1(state)) + + // A missing session_key_sig is irrelevant here (the field stays empty above). + // user_sig signed by the wrong wallet is rejected. + wrongSigner, _ := createTestSigner(t) + wrongUserSig, err := wrongSigner.Sign(packed) + require.NoError(t, err) + stateWrongUser := state + stateWrongUser.UserSig = hexutil.Encode(wrongUserSig) + err = ValidateAppSessionKeyStateUserSigV1(stateWrongUser) + require.Error(t, err) + assert.Contains(t, err.Error(), "user_sig does not match user_address") + + // Tampered version diverges the packed bytes, so recovery no longer matches. + stateTampered := state + stateTampered.Version = 2 + assert.Error(t, ValidateAppSessionKeyStateUserSigV1(stateTampered)) +} + func TestPackAppSessionKeyStateV1(t *testing.T) { t.Parallel() expiresAt := time.Unix(1739812234, 0) diff --git a/pkg/core/session_key.go b/pkg/core/session_key.go index 4b4eb1625..debedf2b3 100644 --- a/pkg/core/session_key.go +++ b/pkg/core/session_key.go @@ -132,17 +132,14 @@ func GetChannelSessionKeyAuthMetadataHashV1(userAddress string, version uint64, return hashedMetadata, nil } -// ValidateChannelSessionKeyStateV1 verifies both signatures over the registration payload: -// user_sig must recover to state.UserAddress (wallet authorizes the delegation) and -// session_key_sig must recover to state.SessionKey (session-key holder proves possession). -// Both signatures sign the same PackChannelKeyStateV1(session_key, metadataHash) payload; -// session_key binds the packed bytes and user_address binds the metadata hash, so a -// signature minted for one (wallet, session_key) pair cannot be replayed for another. -func ValidateChannelSessionKeyStateV1(state ChannelSessionKeyStateV1) error { - if state.SessionKeySig == "" { - return fmt.Errorf("session_key_sig is required") - } - +// ValidateChannelSessionKeyStateUserSigV1 verifies only user_sig over the registration payload: +// user_sig must recover to state.UserAddress (wallet authorizes the change). This is the +// revocation path (submitted expires_at <= now): the session-key holder's session_key_sig is +// intentionally not required so a lost, unavailable, or malicious delegate cannot veto the +// wallet's revocation of its own delegation. session_key binds the packed bytes and +// user_address binds the metadata hash, so the signature authorizes exactly this revocation and +// cannot be replayed for another key, wallet, or version. +func ValidateChannelSessionKeyStateUserSigV1(state ChannelSessionKeyStateV1) error { metadataHash, err := GetChannelSessionKeyAuthMetadataHashV1(state.UserAddress, state.Version, state.Assets, state.ExpiresAt.Unix()) if err != nil { return fmt.Errorf("failed to get metadata hash: %w", err) @@ -170,6 +167,41 @@ func ValidateChannelSessionKeyStateV1(state ChannelSessionKeyStateV1) error { return fmt.Errorf("invalid signature: recovered address %s does not match wallet %s", recoveredUser.String(), state.UserAddress) } + return nil +} + +// ValidateChannelSessionKeyStateV1 verifies both signatures over the registration payload: +// user_sig must recover to state.UserAddress (wallet authorizes the delegation) and +// session_key_sig must recover to state.SessionKey (session-key holder proves possession). +// Both signatures sign the same PackChannelKeyStateV1(session_key, metadataHash) payload; +// session_key binds the packed bytes and user_address binds the metadata hash, so a +// signature minted for one (wallet, session_key) pair cannot be replayed for another. Used for +// activation, extension, and rotation (submitted expires_at > now); revocation uses +// ValidateChannelSessionKeyStateUserSigV1. +func ValidateChannelSessionKeyStateV1(state ChannelSessionKeyStateV1) error { + if state.SessionKeySig == "" { + return fmt.Errorf("session_key_sig is required") + } + + if err := ValidateChannelSessionKeyStateUserSigV1(state); err != nil { + return err + } + + metadataHash, err := GetChannelSessionKeyAuthMetadataHashV1(state.UserAddress, state.Version, state.Assets, state.ExpiresAt.Unix()) + if err != nil { + return fmt.Errorf("failed to get metadata hash: %w", err) + } + + packed, err := PackChannelKeyStateV1(state.SessionKey, metadataHash) + if err != nil { + return fmt.Errorf("failed to pack session key state: %w", err) + } + + recoverer, err := sign.NewAddressRecoverer(sign.TypeEthereumMsg) + if err != nil { + return fmt.Errorf("failed to create address recoverer: %w", err) + } + sessionKeySigBytes, err := hexutil.Decode(state.SessionKeySig) if err != nil { return fmt.Errorf("failed to decode session_key_sig: %w", err) diff --git a/pkg/core/session_key_test.go b/pkg/core/session_key_test.go index 72d47d2a8..c6ee5e810 100644 --- a/pkg/core/session_key_test.go +++ b/pkg/core/session_key_test.go @@ -135,6 +135,50 @@ func TestValidateChannelSessionKeyStateV1(t *testing.T) { assert.Error(t, ValidateChannelSessionKeyStateV1(stateTampered)) } +func TestValidateChannelSessionKeyStateUserSigV1(t *testing.T) { + t.Parallel() + userSigner, userAddress := createSigner(t) + _, sessionKeyAddr := createSigner(t) + + version := uint64(1) + assets := []string{} + expiresAt := time.Now().Add(-time.Hour) // revocation + + metadataHash, err := GetChannelSessionKeyAuthMetadataHashV1(userAddress, version, assets, expiresAt.Unix()) + require.NoError(t, err) + packed, err := PackChannelKeyStateV1(sessionKeyAddr, metadataHash) + require.NoError(t, err) + userSig, err := userSigner.Sign(packed) + require.NoError(t, err) + + state := ChannelSessionKeyStateV1{ + UserAddress: userAddress, + SessionKey: sessionKeyAddr, + Version: version, + Assets: assets, + ExpiresAt: expiresAt, + UserSig: hexutil.Encode(userSig), + } + + // Valid user_sig alone passes — no session_key_sig required on the revocation path. + require.NoError(t, ValidateChannelSessionKeyStateUserSigV1(state)) + + // user_sig signed by the wrong wallet is rejected. + wrongSigner, _ := createSigner(t) + wrongUserSig, err := wrongSigner.Sign(packed) + require.NoError(t, err) + stateWrongUser := state + stateWrongUser.UserSig = hexutil.Encode(wrongUserSig) + err = ValidateChannelSessionKeyStateUserSigV1(stateWrongUser) + require.Error(t, err) + assert.Contains(t, err.Error(), "does not match wallet") + + // Tampered version diverges the packed bytes, so recovery no longer matches. + stateTampered := state + stateTampered.Version = 2 + assert.Error(t, ValidateChannelSessionKeyStateUserSigV1(stateTampered)) +} + // TestValidateChannelSessionKeyStateV1_NoReplay verifies that signatures cannot be replayed // across (wallet, session_key) pairs. session_key binds the packed payload and user_address // binds the metadata hash, so substituting either dimension causes signature recovery to diff --git a/sdk/go/app_session.go b/sdk/go/app_session.go index b88d3e82a..552fa8fc4 100644 --- a/sdk/go/app_session.go +++ b/sdk/go/app_session.go @@ -3,6 +3,7 @@ package sdk import ( "context" "fmt" + "time" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/layer-3/nitrolite/pkg/app" @@ -293,10 +294,10 @@ func (c *Client) RebalanceAppSessions(ctx context.Context, signedUpdates []app.S // SubmitAppSessionKeyState submits a session key state for registration, update, // revocation, or re-activation. The state must carry both the wallet's UserSig // (authorizing the delegation) and the session-key holder's SessionKeySig (proving -// possession of the key); submits without a valid SessionKeySig are rejected on every -// path, including revocation — the session key must co-sign its own deactivation. -// Wallet-only revocation (for a lost or compromised key) is not supported by this -// method. +// possession of the key) when state.ExpiresAt is in the future (registration, update, +// or re-activation). For revocation (state.ExpiresAt at or before now) only UserSig is +// required — use RevokeAppSessionKey for the wallet-only revocation of a lost, +// unavailable, or compromised key. // // Set state.ExpiresAt to a future time to register or update the key. Set it to a // value at or before time.Now() to revoke the key — the auth path filters by @@ -446,3 +447,44 @@ func SignAppSessionKeyOwnership(state app.AppSessionKeyStateV1, sessionKeySigner return hexutil.Encode(sig), nil } + +// RevokeAppSessionKey revokes an app session key using only the wallet's signature. +// Use it when the session-key holder cannot or will not co-sign — a lost, unavailable, or +// compromised delegate. The supplied state must carry the next monotonic Version (latest + 1) +// and an ExpiresAt at or before now; the method signs it with the wallet (UserSig) and submits +// with an empty SessionKeySig. The server accepts user-only signatures only on the revocation +// path (ExpiresAt <= now). For registration, rotation, or extension use +// SubmitAppSessionKeyState with both signatures. +// +// Parameters: +// - ctx: Context for the operation +// - state: The app session key state to revoke (Version = latest + 1, ExpiresAt <= now) +// +// Returns: +// - Error if ExpiresAt is in the future, signing fails, or the request fails +// +// Example: +// +// state := app.AppSessionKeyStateV1{ +// UserAddress: client.GetUserAddress(), +// SessionKey: lostSessionKey, +// Version: latestVersion + 1, +// ApplicationIDs: []string{}, +// AppSessionIDs: []string{}, +// ExpiresAt: time.Now(), +// } +// err := client.RevokeAppSessionKey(ctx, state) +func (c *Client) RevokeAppSessionKey(ctx context.Context, state app.AppSessionKeyStateV1) error { + if state.ExpiresAt.After(time.Now()) { + return fmt.Errorf("revocation requires expires_at at or before now, got %s", state.ExpiresAt) + } + + userSig, err := c.SignSessionKeyState(state) + if err != nil { + return fmt.Errorf("failed to sign session key revocation: %w", err) + } + state.UserSig = userSig + state.SessionKeySig = "" + + return c.SubmitAppSessionKeyState(ctx, state) +} diff --git a/sdk/go/channel.go b/sdk/go/channel.go index 9f6050dff..3bb78b7cb 100644 --- a/sdk/go/channel.go +++ b/sdk/go/channel.go @@ -3,6 +3,7 @@ package sdk import ( "context" "fmt" + "time" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/layer-3/nitrolite/pkg/core" @@ -915,10 +916,10 @@ type GetLastChannelKeyStatesOptions struct { // SubmitChannelSessionKeyState submits a channel session key state for registration, // update, revocation, or re-activation. The state must carry both the wallet's UserSig // (authorizing the delegation) and the session-key holder's SessionKeySig (proving -// possession of the key); submits without a valid SessionKeySig are rejected on every -// path, including revocation — the session key must co-sign its own deactivation. -// Wallet-only revocation (for a lost or compromised key) is not supported by this -// method. +// possession of the key) when state.ExpiresAt is in the future (registration, update, +// or re-activation). For revocation (state.ExpiresAt at or before now) only UserSig is +// required — use RevokeChannelSessionKey for the wallet-only revocation of a lost, +// unavailable, or compromised key. // // Set state.ExpiresAt to a future time to register or update the key. Set it to a // value at or before time.Now() to revoke the key — the auth path filters by @@ -1071,6 +1072,46 @@ func SignChannelSessionKeyOwnership(state core.ChannelSessionKeyStateV1, session return sig.String(), nil } +// RevokeChannelSessionKey revokes a channel session key using only the wallet's signature. +// Use it when the session-key holder cannot or will not co-sign — a lost, unavailable, or +// compromised delegate. The supplied state must carry the next monotonic Version (latest + 1) +// and an ExpiresAt at or before now; the method signs it with the wallet (UserSig) and submits +// with an empty SessionKeySig. The server accepts user-only signatures only on the revocation +// path (ExpiresAt <= now). For registration, rotation, or extension use +// SubmitChannelSessionKeyState with both signatures. +// +// Parameters: +// - ctx: Context for the operation +// - state: The channel session key state to revoke (Version = latest + 1, ExpiresAt <= now) +// +// Returns: +// - Error if ExpiresAt is in the future, signing fails, or the request fails +// +// Example: +// +// state := core.ChannelSessionKeyStateV1{ +// UserAddress: client.GetUserAddress(), +// SessionKey: lostSessionKey, +// Version: latestVersion + 1, +// Assets: []string{}, +// ExpiresAt: time.Now(), +// } +// err := client.RevokeChannelSessionKey(ctx, state) +func (c *Client) RevokeChannelSessionKey(ctx context.Context, state core.ChannelSessionKeyStateV1) error { + if state.ExpiresAt.After(time.Now()) { + return fmt.Errorf("revocation requires expires_at at or before now, got %s", state.ExpiresAt) + } + + userSig, err := c.SignChannelSessionKeyState(state) + if err != nil { + return fmt.Errorf("failed to sign channel session key revocation: %w", err) + } + state.UserSig = userSig + state.SessionKeySig = "" + + return c.SubmitChannelSessionKeyState(ctx, state) +} + // ApproveToken approves the ChannelHub contract to spend tokens on behalf of the user. // This is required before depositing ERC-20 tokens. Native tokens (e.g., ETH) do not // require approval and will return an error if attempted. diff --git a/sdk/ts/src/client.ts b/sdk/ts/src/client.ts index fb1757449..0abff82b5 100644 --- a/sdk/ts/src/client.ts +++ b/sdk/ts/src/client.ts @@ -1762,10 +1762,11 @@ export class Client { * - revocation: bump version with expires_at <= now to retire the key; the slot is freed * for the per-user cap and the auth path stops accepting state signed by it * - * The state must carry both the wallet's user_sig (proving the user authorized the - * delegation) and the session-key holder's session_key_sig (proving possession of the - * key being registered, rotated, or revoked). Submits without a valid session_key_sig - * are rejected. + * Activation, rotation, and re-activation (expires_at > now) require both the wallet's + * user_sig (proving the user authorized the delegation) and the session-key holder's + * session_key_sig (proving possession). Revocation (expires_at <= now) requires only + * user_sig — use revokeChannelSessionKey for the wallet-only revocation of a lost, + * unavailable, or compromised key. * * @param state - The channel session key state containing delegation information */ @@ -1776,6 +1777,32 @@ export class Client { await this.rpcClient.channelsV1SubmitSessionKeyState(req); } + /** + * Revoke a channel session key using only the wallet's signature. Use it when the + * session-key holder cannot or will not co-sign — a lost, unavailable, or compromised + * delegate. The supplied state must carry the next monotonic version (latest + 1) and an + * expires_at at or before now; this method signs it with the wallet (user_sig) and submits + * with an empty session_key_sig. The server accepts user-only signatures only on the + * revocation path (expires_at <= now). For registration, rotation, or extension use + * submitChannelSessionKeyState with both signatures. + * + * @param state - The channel session key state to revoke (version = latest + 1, expires_at <= now) + */ + async revokeChannelSessionKey(state: ChannelSessionKeyStateV1): Promise { + const nowSec = BigInt(Math.floor(Date.now() / 1000)); + if (BigInt(state.expires_at) > nowSec) { + throw new Error( + `revocation requires expires_at at or before now, got ${state.expires_at}` + ); + } + const userSig = await this.signChannelSessionKeyState(state); + await this.submitChannelSessionKeyState({ + ...state, + user_sig: userSig, + session_key_sig: '', + }); + } + /** * Retrieve the latest channel session key states for a user. Defaults to * active-only (server filters expired states); pass `includeInactive: true` @@ -1856,10 +1883,11 @@ export class Client { * - revocation: bump version with expires_at <= now to retire the key; the slot is freed * for the per-user cap and the auth path stops accepting state signed by it * - * The state must carry both the wallet's user_sig (proving the user authorized the - * delegation) and the session-key holder's session_key_sig (proving possession of the - * key being registered, rotated, or revoked). Submits without a valid session_key_sig - * are rejected. + * Activation, rotation, and re-activation (expires_at > now) require both the wallet's + * user_sig (proving the user authorized the delegation) and the session-key holder's + * session_key_sig (proving possession). Revocation (expires_at <= now) requires only + * user_sig — use revokeSessionKey for the wallet-only revocation of a lost, unavailable, + * or compromised key. * * @param state - The session key state containing delegation information */ @@ -1870,6 +1898,32 @@ export class Client { await this.rpcClient.appSessionsV1SubmitSessionKeyState(req); } + /** + * Revoke an app session key using only the wallet's signature. Use it when the + * session-key holder cannot or will not co-sign — a lost, unavailable, or compromised + * delegate. The supplied state must carry the next monotonic version (latest + 1) and an + * expires_at at or before now; this method signs it with the wallet (user_sig) and submits + * with an empty session_key_sig. The server accepts user-only signatures only on the + * revocation path (expires_at <= now). For registration, rotation, or extension use + * submitSessionKeyState with both signatures. + * + * @param state - The app session key state to revoke (version = latest + 1, expires_at <= now) + */ + async revokeSessionKey(state: app.AppSessionKeyStateV1): Promise { + const nowSec = BigInt(Math.floor(Date.now() / 1000)); + if (BigInt(state.expires_at) > nowSec) { + throw new Error( + `revocation requires expires_at at or before now, got ${state.expires_at}` + ); + } + const userSig = await this.signSessionKeyState(state); + await this.submitSessionKeyState({ + ...state, + user_sig: userSig, + session_key_sig: '', + }); + } + /** * Retrieve the latest app session key states for a user. Defaults to * active-only (server filters expired states); pass `includeInactive: true` diff --git a/sdk/ts/test/unit/__snapshots__/public-api-drift.test.ts.snap b/sdk/ts/test/unit/__snapshots__/public-api-drift.test.ts.snap index 7692360b1..fb26e71a8 100644 --- a/sdk/ts/test/unit/__snapshots__/public-api-drift.test.ts.snap +++ b/sdk/ts/test/unit/__snapshots__/public-api-drift.test.ts.snap @@ -1072,6 +1072,8 @@ exports[`SDK public runtime API drift guard keeps root TypeScript public API sig "ping: (): Promise", "rebalanceAppSessions: (signedUpdates: app.SignedAppStateUpdateV1[]): Promise", "registerApp: (appID: string, metadata: string, creationApprovalNotRequired: boolean): Promise", + "revokeChannelSessionKey: (state: ChannelSessionKeyStateV1): Promise", + "revokeSessionKey: (state: app.AppSessionKeyStateV1): Promise", "setHomeBlockchain: (asset: string, blockchainId: bigint): Promise", "signAppSessionKeyOwnership: (state: app.AppSessionKeyStateV1, sessionKeySigner: EthereumMsgSigner): Promise", "signChannelSessionKeyOwnership: (state: ChannelSessionKeyStateV1, sessionKeySigner: EthereumMsgSigner): Promise", From bf0852691587f4d570fc5ad07bb92dc1d66e0a22 Mon Sep 17 00:00:00 2001 From: Anton Filonenko Date: Thu, 4 Jun 2026 11:49:45 +0300 Subject: [PATCH 2/2] fix(session-key): block first-submit revoke and canonicalize revocation rows Address PR review on wallet-only session-key revocation: - Reject a revoke at version 1 in both submit handlers before LockSessionKeyState runs. A version-1 revoke has no prior delegation, so it would otherwise let a wallet seed a permanent (session_key, kind) ownership claim for a key it never proved possession of. A legitimate revoke is always version >= 2. - Clear SessionKeySig on the revocation path before persisting so stored revoke rows never retain unverified client input. - Compare Unix seconds in RevokeChannelSessionKey / RevokeAppSessionKey guards to match the precision the metadata hash is signed at, so a value inside the current Unix second is not rejected locally. - Use Omit<..., 'user_sig' | 'session_key_sig'> for the TS revoke method inputs so callers no longer pass dummy signature fields. - Fix docs/api.yaml field descriptions ("past value" -> "at or before now") and correct the stale seed-row permanence comment, which claimed the seed survives aborted transactions. Adds regression tests for the version-1 rejection and the cleared SessionKeySig. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/api.yaml | 4 ++-- .../submit_session_key_state.go | 13 +++++++++- .../submit_session_key_state_test.go | 23 +++++++++++------- .../channel_v1/submit_session_key_state.go | 13 +++++++++- .../submit_session_key_state_test.go | 23 +++++++++++------- .../database/current_session_key_state.go | 14 ++++++----- sdk/go/app_session.go | 5 +++- sdk/go/channel.go | 5 +++- sdk/ts/src/client.ts | 24 +++++++++++-------- .../public-api-drift.test.ts.snap | 4 ++-- 10 files changed, 86 insertions(+), 42 deletions(-) diff --git a/docs/api.yaml b/docs/api.yaml index 88e44557e..f152ac7b1 100644 --- a/docs/api.yaml +++ b/docs/api.yaml @@ -673,7 +673,7 @@ api: request: - field_name: state type: channel_session_key_state - description: Session key metadata and delegation information. Set `expires_at` to a past value to revoke (user_sig only); future to (re-)activate (both signatures). + description: Session key metadata and delegation information. Set `expires_at` to a value at or before now to revoke (user_sig only); future to (re-)activate (both signatures). response: [] errors: - message: invalid_session_key_state @@ -889,7 +889,7 @@ api: request: - field_name: state type: app_session_key_state - description: Session key metadata and delegation information. Set `expires_at` to a past value to revoke (user_sig only); future to (re-)activate (both signatures). + description: Session key metadata and delegation information. Set `expires_at` to a value at or before now to revoke (user_sig only); future to (re-)activate (both signatures). response: [] errors: - message: invalid_session_key_state diff --git a/nitronode/api/app_session_v1/submit_session_key_state.go b/nitronode/api/app_session_v1/submit_session_key_state.go index f9b85e233..15e80f00c 100644 --- a/nitronode/api/app_session_v1/submit_session_key_state.go +++ b/nitronode/api/app_session_v1/submit_session_key_state.go @@ -112,11 +112,22 @@ func (h *Handler) SubmitSessionKeyState(c *rpc.Context) { return } } else { - // Revocation: validate only the wallet's UserSig. Any session_key_sig present is ignored. + // Revocation only deactivates an existing delegation. Version 1 means there is no prior + // delegation, so reject it before LockSessionKeyState can seed a permanent ownership + // claim for a session_key the caller never proved possession of: a legitimate revoke is + // always version >= 2, since registration at version 1 is future-dated and requires both + // signatures. + if coreState.Version == 1 { + c.Fail(rpc.Errorf("invalid_session_key_state: cannot revoke a session key with no prior delegation"), "") + return + } + // Validate only the wallet's UserSig. Any session_key_sig present is ignored, so clear + // it before persisting to keep stored revocation rows canonical. if err := app.ValidateAppSessionKeyStateUserSigV1(coreState); err != nil { c.Fail(rpc.Errorf("invalid_session_key_state: %v", err), "") return } + coreState.SessionKeySig = "" } // Validate version and store the session key state. diff --git a/nitronode/api/app_session_v1/submit_session_key_state_test.go b/nitronode/api/app_session_v1/submit_session_key_state_test.go index f898bf328..814b7012c 100644 --- a/nitronode/api/app_session_v1/submit_session_key_state_test.go +++ b/nitronode/api/app_session_v1/submit_session_key_state_test.go @@ -292,7 +292,10 @@ func TestSubmitSessionKeyState_InvalidUserAddress(t *testing.T) { assert.Contains(t, respErr.Error(), "invalid user_address") } -func TestSubmitSessionKeyState_RevokeWithPastExpiresAt(t *testing.T) { +// A version-1 revoke has no prior delegation to deactivate; allowing it would let a wallet +// seed a permanent (session_key, kind) ownership claim for a key it never proved possession +// of. It must be rejected before LockSessionKeyState runs, so no seed row is ever written. +func TestSubmitSessionKeyState_RevokeFirstSubmit_Rejected(t *testing.T) { mockStore := new(MockStore) userSigner := NewMockSigner() userAddress := strings.ToLower(userSigner.PublicKey().Address().String()) @@ -307,14 +310,9 @@ func TestSubmitSessionKeyState_RevokeWithPastExpiresAt(t *testing.T) { maxSessionKeyIDs: 10, } - // expires_at in the past expresses a revoke: the same monotonic version sequence - // is preserved, the auth path filters expires_at > now so the key is deactivated. expiresAt := time.Now().Add(-time.Hour).Truncate(time.Second) reqPayload := buildSignedSessionKeyStateReq(t, userAddress, sessionKeyAddress, 1, nil, nil, expiresAt, userSigner, sessionKeySigner) - mockStore.On("LockSessionKeyState", userAddress, sessionKeyAddress, database.SessionKeyKindAppSession).Return(0, time.Time{}, nil) - mockStore.On("StoreAppSessionKeyState", mock.AnythingOfType("app.AppSessionKeyStateV1")).Return(nil) - payload, err := rpc.NewPayload(reqPayload) require.NoError(t, err) @@ -326,8 +324,11 @@ func TestSubmitSessionKeyState_RevokeWithPastExpiresAt(t *testing.T) { handler.SubmitSessionKeyState(ctx) require.NotNil(t, ctx.Response) - assert.Nil(t, ctx.Response.Error()) - mockStore.AssertExpectations(t) + respErr := ctx.Response.Error() + require.NotNil(t, respErr) + assert.Contains(t, respErr.Error(), "no prior delegation") + mockStore.AssertNotCalled(t, "LockSessionKeyState", mock.Anything, mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "StoreAppSessionKeyState", mock.Anything) } // Covers the typical revocation path: an active key (latestVersion > 0, prev expires_at in @@ -948,7 +949,11 @@ func TestSubmitSessionKeyState_RevokeIgnoresMismatchedSessionKeySig(t *testing.T prevActiveExpiresAt := time.Now().Add(24 * time.Hour) mockStore.On("LockSessionKeyState", userAddress, sessionKeyAddress, database.SessionKeyKindAppSession).Return(1, prevActiveExpiresAt, nil) - mockStore.On("StoreAppSessionKeyState", mock.AnythingOfType("app.AppSessionKeyStateV1")).Return(nil) + // The ignored session_key_sig must be cleared before persisting so stored revocation + // rows never retain unverified client input. + mockStore.On("StoreAppSessionKeyState", mock.MatchedBy(func(s app.AppSessionKeyStateV1) bool { + return s.SessionKeySig == "" + })).Return(nil) payload, err := rpc.NewPayload(reqPayload) require.NoError(t, err) diff --git a/nitronode/api/channel_v1/submit_session_key_state.go b/nitronode/api/channel_v1/submit_session_key_state.go index 6b028c03c..6dd2ceb1d 100644 --- a/nitronode/api/channel_v1/submit_session_key_state.go +++ b/nitronode/api/channel_v1/submit_session_key_state.go @@ -95,11 +95,22 @@ func (h *Handler) SubmitSessionKeyState(c *rpc.Context) { return } } else { - // Revocation: validate only the wallet's user_sig. Any session_key_sig present is ignored. + // Revocation only deactivates an existing delegation. Version 1 means there is no prior + // delegation, so reject it before LockSessionKeyState can seed a permanent ownership + // claim for a session_key the caller never proved possession of: a legitimate revoke is + // always version >= 2, since registration at version 1 is future-dated and requires both + // signatures. + if coreState.Version == 1 { + c.Fail(rpc.Errorf("invalid_session_key_state: cannot revoke a session key with no prior delegation"), "") + return + } + // Validate only the wallet's user_sig. Any session_key_sig present is ignored, so clear + // it before persisting to keep stored revocation rows canonical. if err := core.ValidateChannelSessionKeyStateUserSigV1(coreState); err != nil { c.Fail(rpc.Errorf("invalid_session_key_state: %v", err), "") return } + coreState.SessionKeySig = "" } // Validate version and store the session key state. diff --git a/nitronode/api/channel_v1/submit_session_key_state_test.go b/nitronode/api/channel_v1/submit_session_key_state_test.go index c036a3e56..975972214 100644 --- a/nitronode/api/channel_v1/submit_session_key_state_test.go +++ b/nitronode/api/channel_v1/submit_session_key_state_test.go @@ -212,7 +212,10 @@ func TestChannelSubmitSessionKeyState_InvalidUserAddress(t *testing.T) { assert.Contains(t, respErr.Error(), "invalid user_address") } -func TestChannelSubmitSessionKeyState_RevokeWithPastExpiresAt(t *testing.T) { +// A version-1 revoke has no prior delegation to deactivate; allowing it would let a wallet +// seed a permanent (session_key, kind) ownership claim for a key it never proved possession +// of. It must be rejected before LockSessionKeyState runs, so no seed row is ever written. +func TestChannelSubmitSessionKeyState_RevokeFirstSubmit_Rejected(t *testing.T) { mockStore := new(MockStore) userSigner := NewMockSigner() userAddress := strings.ToLower(userSigner.PublicKey().Address().String()) @@ -227,16 +230,11 @@ func TestChannelSubmitSessionKeyState_RevokeWithPastExpiresAt(t *testing.T) { maxSessionKeyIDs: 10, } - // expires_at in the past expresses a revoke: the same monotonic version sequence - // is preserved, the auth path filters expires_at > now so the key is deactivated. expiresAt := time.Now().Add(-time.Hour).Truncate(time.Second) assets := []string{"USDC"} reqPayload := buildSignedChannelSessionKeyStateReq(t, userAddress, sessionKeyAddress, 1, assets, expiresAt, userSigner, sessionKeySigner) - mockStore.On("LockSessionKeyState", userAddress, sessionKeyAddress, database.SessionKeyKindChannel).Return(0, time.Time{}, nil) - mockStore.On("StoreChannelSessionKeyState", mock.AnythingOfType("core.ChannelSessionKeyStateV1")).Return(nil) - payload, err := rpc.NewPayload(reqPayload) require.NoError(t, err) @@ -248,8 +246,11 @@ func TestChannelSubmitSessionKeyState_RevokeWithPastExpiresAt(t *testing.T) { handler.SubmitSessionKeyState(ctx) require.NotNil(t, ctx.Response) - assert.Nil(t, ctx.Response.Error()) - mockStore.AssertExpectations(t) + respErr := ctx.Response.Error() + require.NotNil(t, respErr) + assert.Contains(t, respErr.Error(), "no prior delegation") + mockStore.AssertNotCalled(t, "LockSessionKeyState", mock.Anything, mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "StoreChannelSessionKeyState", mock.Anything) } // Covers the typical revocation path: an active key (latestVersion > 0, prev expires_at in @@ -788,7 +789,11 @@ func TestChannelSubmitSessionKeyState_RevokeIgnoresMismatchedSessionKeySig(t *te prevActiveExpiresAt := time.Now().Add(24 * time.Hour) mockStore.On("LockSessionKeyState", userAddress, sessionKeyAddress, database.SessionKeyKindChannel).Return(1, prevActiveExpiresAt, nil) - mockStore.On("StoreChannelSessionKeyState", mock.AnythingOfType("core.ChannelSessionKeyStateV1")).Return(nil) + // The ignored session_key_sig must be cleared before persisting so stored revocation + // rows never retain unverified client input. + mockStore.On("StoreChannelSessionKeyState", mock.MatchedBy(func(s core.ChannelSessionKeyStateV1) bool { + return s.SessionKeySig == "" + })).Return(nil) payload, err := rpc.NewPayload(reqPayload) require.NoError(t, err) diff --git a/nitronode/store/database/current_session_key_state.go b/nitronode/store/database/current_session_key_state.go index 4e4dcb166..bbae87921 100644 --- a/nitronode/store/database/current_session_key_state.go +++ b/nitronode/store/database/current_session_key_state.go @@ -92,12 +92,14 @@ func upsertCurrentSessionKeyState(tx *gorm.DB, userAddress, sessionKey string, k // SELECT ... FOR UPDATE is postgres-only; on sqlite the locking clause is skipped and the // surrounding transaction provides the necessary ordering for the in-process test setup. // -// Seed-row permanence: the version=0 row written below is intentionally never deleted on -// failure paths (sig validation, version mismatch, cap exceeded, mid-tx errors). Once a wallet -// has staked a claim on (session_key, kind), no other wallet can take it for that kind — the -// seed is the ownership reservation, not a transient placeholder. CountSessionKeysForUser -// excludes version=0 rows so the per-user cap is unaffected, but the (session_key, kind) -// ownership bind is permanent by design. +// Seed-row permanence: the version=0 row written below is part of the caller's transaction, +// so it persists only when that transaction commits. Failure paths that abort the tx (version +// mismatch, cap exceeded, mid-tx errors) roll the seed back with everything else, and callers +// must guard against committing a seed for an unauthorized claim — e.g. the submit handlers +// reject a revoke at version 1, so a wallet cannot stake a claim on (session_key, kind) without +// a prior delegation it proved possession of. Once a submit does commit, the ownership bind is +// permanent: no other wallet can take that (session_key, kind). CountSessionKeysForUser excludes +// version=0 rows so the per-user cap is unaffected. // // When locked.Version > 0, the matching history row's expires_at is also returned so callers // can distinguish a reactivation (prev inactive → submitted active) from a rotation/update diff --git a/sdk/go/app_session.go b/sdk/go/app_session.go index 552fa8fc4..5297fe2a3 100644 --- a/sdk/go/app_session.go +++ b/sdk/go/app_session.go @@ -475,7 +475,10 @@ func SignAppSessionKeyOwnership(state app.AppSessionKeyStateV1, sessionKeySigner // } // err := client.RevokeAppSessionKey(ctx, state) func (c *Client) RevokeAppSessionKey(ctx context.Context, state app.AppSessionKeyStateV1) error { - if state.ExpiresAt.After(time.Now()) { + // Compare Unix seconds, matching the precision PackAppSessionKeyStateV1 signs at + // (ExpiresAt.Unix()), so a value inside the current Unix second is not rejected locally + // even though the server treats it as expires_at <= now. + if state.ExpiresAt.Unix() > time.Now().Unix() { return fmt.Errorf("revocation requires expires_at at or before now, got %s", state.ExpiresAt) } diff --git a/sdk/go/channel.go b/sdk/go/channel.go index 3bb78b7cb..fe0ee06db 100644 --- a/sdk/go/channel.go +++ b/sdk/go/channel.go @@ -1098,7 +1098,10 @@ func SignChannelSessionKeyOwnership(state core.ChannelSessionKeyStateV1, session // } // err := client.RevokeChannelSessionKey(ctx, state) func (c *Client) RevokeChannelSessionKey(ctx context.Context, state core.ChannelSessionKeyStateV1) error { - if state.ExpiresAt.After(time.Now()) { + // Compare Unix seconds, matching the precision SignChannelSessionKeyState signs at + // (ExpiresAt.Unix()), so a value inside the current Unix second is not rejected locally + // even though the server treats it as expires_at <= now. + if state.ExpiresAt.Unix() > time.Now().Unix() { return fmt.Errorf("revocation requires expires_at at or before now, got %s", state.ExpiresAt) } diff --git a/sdk/ts/src/client.ts b/sdk/ts/src/client.ts index 0abff82b5..21a53c224 100644 --- a/sdk/ts/src/client.ts +++ b/sdk/ts/src/client.ts @@ -1786,20 +1786,22 @@ export class Client { * revocation path (expires_at <= now). For registration, rotation, or extension use * submitChannelSessionKeyState with both signatures. * - * @param state - The channel session key state to revoke (version = latest + 1, expires_at <= now) + * @param state - The channel session key state to revoke (version = latest + 1, expires_at <= now). Signature fields are supplied by this method, so callers omit them. */ - async revokeChannelSessionKey(state: ChannelSessionKeyStateV1): Promise { + async revokeChannelSessionKey( + state: Omit + ): Promise { const nowSec = BigInt(Math.floor(Date.now() / 1000)); if (BigInt(state.expires_at) > nowSec) { throw new Error( `revocation requires expires_at at or before now, got ${state.expires_at}` ); } - const userSig = await this.signChannelSessionKeyState(state); + const fullState: ChannelSessionKeyStateV1 = { ...state, user_sig: '', session_key_sig: '' }; + const userSig = await this.signChannelSessionKeyState(fullState); await this.submitChannelSessionKeyState({ - ...state, + ...fullState, user_sig: userSig, - session_key_sig: '', }); } @@ -1907,20 +1909,22 @@ export class Client { * revocation path (expires_at <= now). For registration, rotation, or extension use * submitSessionKeyState with both signatures. * - * @param state - The app session key state to revoke (version = latest + 1, expires_at <= now) + * @param state - The app session key state to revoke (version = latest + 1, expires_at <= now). Signature fields are supplied by this method, so callers omit them. */ - async revokeSessionKey(state: app.AppSessionKeyStateV1): Promise { + async revokeSessionKey( + state: Omit + ): Promise { const nowSec = BigInt(Math.floor(Date.now() / 1000)); if (BigInt(state.expires_at) > nowSec) { throw new Error( `revocation requires expires_at at or before now, got ${state.expires_at}` ); } - const userSig = await this.signSessionKeyState(state); + const fullState: app.AppSessionKeyStateV1 = { ...state, user_sig: '', session_key_sig: '' }; + const userSig = await this.signSessionKeyState(fullState); await this.submitSessionKeyState({ - ...state, + ...fullState, user_sig: userSig, - session_key_sig: '', }); } diff --git a/sdk/ts/test/unit/__snapshots__/public-api-drift.test.ts.snap b/sdk/ts/test/unit/__snapshots__/public-api-drift.test.ts.snap index fb26e71a8..8fb2a4c6b 100644 --- a/sdk/ts/test/unit/__snapshots__/public-api-drift.test.ts.snap +++ b/sdk/ts/test/unit/__snapshots__/public-api-drift.test.ts.snap @@ -1072,8 +1072,8 @@ exports[`SDK public runtime API drift guard keeps root TypeScript public API sig "ping: (): Promise", "rebalanceAppSessions: (signedUpdates: app.SignedAppStateUpdateV1[]): Promise", "registerApp: (appID: string, metadata: string, creationApprovalNotRequired: boolean): Promise", - "revokeChannelSessionKey: (state: ChannelSessionKeyStateV1): Promise", - "revokeSessionKey: (state: app.AppSessionKeyStateV1): Promise", + "revokeChannelSessionKey: (state: Omit): Promise", + "revokeSessionKey: (state: Omit): Promise", "setHomeBlockchain: (asset: string, blockchainId: bigint): Promise", "signAppSessionKeyOwnership: (state: app.AppSessionKeyStateV1, sessionKeySigner: EthereumMsgSigner): Promise", "signChannelSessionKeyOwnership: (state: ChannelSessionKeyStateV1, sessionKeySigner: EthereumMsgSigner): Promise",