From f9620eab433308aa0eaac4023388c4ef9a808af6 Mon Sep 17 00:00:00 2001 From: Arnaud Moncel Date: Fri, 22 May 2026 10:48:16 +0200 Subject: [PATCH 1/2] fix(rpc agent): remove useless verrification to handle properly main node agent start --- .../middleware/authentication.rb | 29 +--------- .../middleware/authentication_spec.rb | 54 +++---------------- 2 files changed, 7 insertions(+), 76 deletions(-) diff --git a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/middleware/authentication.rb b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/middleware/authentication.rb index a195f4680..438a6bb95 100644 --- a/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/middleware/authentication.rb +++ b/packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/middleware/authentication.rb @@ -4,9 +4,6 @@ module ForestAdminRpcAgent module Middleware class Authentication ALLOWED_TIME_DIFF = 300 - SIGNATURE_REUSE_WINDOW = 5 - @@used_signatures = {} - @@signatures_mutex = Mutex.new def initialize(app) @app = app @@ -41,25 +38,7 @@ def valid_signature?(signature, timestamp) expected_signature = OpenSSL::HMAC.hexdigest('SHA256', auth_secret, timestamp) - return false unless Rack::Utils.secure_compare(signature, expected_signature) - - # check if this signature has already been used (replay attack) - # Reject if signature was used recently (within SIGNATURE_REUSE_WINDOW seconds) - # Use mutex to prevent race conditions in multi-threaded environments - now = current_time_in_seconds - - @@signatures_mutex.synchronize do - if @@used_signatures.key?(signature) - last_used = @@used_signatures[signature] - time_since_last_use = now - last_used - return false if time_since_last_use <= SIGNATURE_REUSE_WINDOW - end - @@used_signatures[signature] = now - - cleanup_old_signatures - end - - true + Rack::Utils.secure_compare(signature, expected_signature) end def valid_timestamp?(timestamp) @@ -73,12 +52,6 @@ def valid_timestamp?(timestamp) (current_time_in_seconds - time.to_i).abs <= ALLOWED_TIME_DIFF end - def cleanup_old_signatures - # Should be called within mutex synchronize block - now = current_time_in_seconds - @@used_signatures.delete_if { |_signature, last_used| now - last_used > ALLOWED_TIME_DIFF } - end - def current_time_in_seconds defined?(Time.current) ? Time.current.to_i : Time.now.utc.to_i end diff --git a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/middleware/authentication_spec.rb b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/middleware/authentication_spec.rb index 33068792a..f23eeabb2 100644 --- a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/middleware/authentication_spec.rb +++ b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/middleware/authentication_spec.rb @@ -74,44 +74,6 @@ module Middleware end end - context 'when signature is reused within allowed window (replay attack)' do - before do - env['HTTP_X_SIGNATURE'] = signature - env['HTTP_X_TIMESTAMP'] = timestamp - # First request - should pass - middleware.call(env) - end - - it 'blocks the replay attack' do - # Second request with same signature immediately - should be blocked - status, _headers, body = middleware.call(env) - expect(status).to eq(401) - expect(JSON.parse(body.first)).to eq({ 'error' => 'Unauthorized' }) - end - end - - context 'when signature is reused after allowed window' do - before do - env['HTTP_X_SIGNATURE'] = signature - env['HTTP_X_TIMESTAMP'] = timestamp - # First request - middleware.call(env) - - # Simulate time passing (6 seconds later) - travel_to = Time.now.utc + described_class::SIGNATURE_REUSE_WINDOW + 1 - allow(Time).to receive(:now).and_return(travel_to) - if defined?(Time.current) - allow(Time).to receive(:current).and_return(travel_to) - end - end - - it 'allows the request (signature expired from cache)' do - # After 6 seconds, signature can be reused (not in replay window anymore) - status, = middleware.call(env) - expect(status).to eq(200) - end - end - context 'when timestamp is too old' do let(:old_timestamp) { (Time.now.utc - (described_class::ALLOWED_TIME_DIFF + 10)).iso8601 } let(:old_signature) { OpenSSL::HMAC.hexdigest('SHA256', secret, old_timestamp) } @@ -212,22 +174,18 @@ module Middleware end end - it 'blocks replay with same millisecond timestamp' do - # First request with millisecond precision + it 'allows replay with the same millisecond timestamp (no anti-replay)' do + # The Node TS rpc-agent has no anti-replay protection; Ruby matches that behaviour + # so parallel calls from a Node main agent don't get spuriously rejected when two + # signatures land in the same millisecond. timestamp_ms = Time.now.utc.iso8601(3) signature_ms = OpenSSL::HMAC.hexdigest('SHA256', secret, timestamp_ms) env['HTTP_X_SIGNATURE'] = signature_ms env['HTTP_X_TIMESTAMP'] = timestamp_ms - # First request - should pass - status, = middleware.call(env) - expect(status).to eq(200) - - # Second request with exact same timestamp and signature - should be blocked - status, _headers, body = middleware.call(env) - expect(status).to eq(401) - expect(JSON.parse(body.first)).to eq({ 'error' => 'Unauthorized' }) + expect(middleware.call(env).first).to eq(200) + expect(middleware.call(env).first).to eq(200) end end From ae79ea2a2052f5824becb1b838a7aba4f2afcd49 Mon Sep 17 00:00:00 2001 From: Arnaud Moncel Date: Fri, 22 May 2026 14:41:04 +0200 Subject: [PATCH 2/2] fix: review --- .../middleware/authentication_spec.rb | 37 +------------------ 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/middleware/authentication_spec.rb b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/middleware/authentication_spec.rb index f23eeabb2..b6a081645 100644 --- a/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/middleware/authentication_spec.rb +++ b/packages/forest_admin_rpc_agent/spec/lib/forest_admin_rpc_agent/middleware/authentication_spec.rb @@ -119,41 +119,6 @@ module Middleware end end - context 'when cleaning up old signatures' do - it 'removes signatures older than ALLOWED_TIME_DIFF' do - # Add multiple signatures at different times - 5.times do |i| - ts = (Time.now.utc - (i * 100)).iso8601 - sig = OpenSSL::HMAC.hexdigest('SHA256', secret, ts) - env['HTTP_X_SIGNATURE'] = sig - env['HTTP_X_TIMESTAMP'] = ts - middleware.call(env) - end - - # Verify cleanup happens (internal state check via new request) - status, = middleware.call(env) - expect(status).to satisfy('be either 200 or 401') { |s| [200, 401].include?(s) } - end - end - - context 'with thread safety' do - it 'mutex protects shared state from race conditions' do - # This test verifies the mutex works - not testing exact behavior - # Just ensure no crashes when multiple threads access the middleware - threads = Array.new(3) do |i| - Thread.new do - ts = (Time.now.utc + (i * 10)).iso8601 - sig = OpenSSL::HMAC.hexdigest('SHA256', secret, ts) - test_env = { 'HTTP_X_SIGNATURE' => sig, 'HTTP_X_TIMESTAMP' => ts } - middleware.call(test_env) - end - end - - # Just verify no exceptions raised - expect { threads.each(&:join) }.not_to raise_error - end - end - context 'when multiple requests with millisecond timestamps (rapid fire)' do it 'allows multiple requests in the same second with different milliseconds' do # Simulate 3 rapid requests within the same second but with different milliseconds @@ -174,7 +139,7 @@ module Middleware end end - it 'allows replay with the same millisecond timestamp (no anti-replay)' do + it 'accepts concurrent requests with identical signature' do # The Node TS rpc-agent has no anti-replay protection; Ruby matches that behaviour # so parallel calls from a Node main agent don't get spuriously rejected when two # signatures land in the same millisecond.