From 1983014be65aea8cd9d72d7afdf0a816068934dc Mon Sep 17 00:00:00 2001 From: "Stanko K.R." Date: Wed, 25 Mar 2026 11:15:18 +0100 Subject: [PATCH] Rely solely on the challange signature for verification The cookie approach seems like the more secure aproach because it ties the authentication or registration attempt to the user's browser session, but it doesn't work reliably on Chrome for Windows. Also, a simila problem pops up on Chrome for Linux if the session is used instead of a separate cookie. It looks like the browser doesn't propagate the state change through fast enough which results in some requests contaiining the new/updated cookie, and others don't, which results in sporadic failures. Since we use a signed and expiring challange we still get protection from replay attacks and tampering which enables us to omit the cookie entierly and rely on the challange's signature to prove expiration and authenticity. The only thing we lose is the ability to tie and attemp to a single browser session. See: https://github.com/w3c/webauthn/wiki/Explainer:-WebAuthn-challengeURL which proposes to add the same challange fetching logic as part of the standard See: https://github.com/w3c/webauthn/issues/1856 which discusses issues that arise from having expiring challanges (which the spec recommends) See: https://github.com/OneUptime/oneuptime/security/advisories/GHSA-gjjc-pcwp-c74m which is an explot that can happen if the server isn't able to verify the authenticity of challanges that are sent outside of a cookie --- lib/action_pack/passkey.rb | 19 +++++---- .../passkey/challenges_controller.rb | 17 +++----- lib/action_pack/passkey/request.rb | 6 +-- .../authenticator/assertion_response.rb | 1 - .../authenticator/attestation_response.rb | 1 - .../web_authn/authenticator/response.rb | 20 ++++------ lib/action_pack/web_authn/current.rb | 6 +-- .../web_authn/public_key_credential.rb | 7 +--- .../my/passkey_challenges_controller_test.rb | 9 ----- test/lib/action_pack/passkey_test.rb | 6 +-- .../authenticator/assertion_response_test.rb | 28 ++++++++----- .../attestation_response_test.rb | 25 ++++++------ .../web_authn/authenticator/response_test.rb | 39 +++++++++++++------ 13 files changed, 89 insertions(+), 95 deletions(-) diff --git a/lib/action_pack/passkey.rb b/lib/action_pack/passkey.rb index ace33176c..085960ec2 100644 --- a/lib/action_pack/passkey.rb +++ b/lib/action_pack/passkey.rb @@ -47,14 +47,13 @@ class ActionPack::Passkey < Rails.configuration.action_pack.passkey.parent_class # Verifies the attestation response from the browser and persists a new passkey record. # The +passkey+ hash should contain +client_data_json+, +attestation_object+, and +transports+ - # as submitted by the registration form. The +challenge+ defaults to - # +ActionPack::WebAuthn::Current.challenge+, which is automatically populated from the session - # by ActionPack::Passkey::Request. Any additional +attributes+ (e.g. +holder+) are passed - # through to +create!+. + # as submitted by the registration form. The challenge is extracted from the authenticator's + # +clientDataJSON+ response and verified server-side. Any additional +attributes+ (e.g. +holder+) + # are passed through to +create!+. # # Raises ActionPack::WebAuthn::InvalidResponseError if the attestation is invalid. - def register(passkey, challenge: ActionPack::WebAuthn::Current.challenge, **attributes) - credential = ActionPack::WebAuthn::PublicKeyCredential.register(passkey, challenge: challenge) + def register(passkey, **attributes) + credential = ActionPack::WebAuthn::PublicKeyCredential.register(passkey) create!(**credential.to_h, **attributes) end @@ -74,17 +73,17 @@ class ActionPack::Passkey < Rails.configuration.action_pack.passkey.parent_class # Looks up a passkey by credential ID and verifies the assertion response from the browser. # Returns the authenticated Passkey record, or +nil+ if the credential is not found or # verification fails. - def authenticate(passkey, challenge: ActionPack::WebAuthn::Current.challenge) - find_by(credential_id: passkey[:id])&.authenticate(passkey, challenge: challenge) + def authenticate(passkey) + find_by(credential_id: passkey[:id])&.authenticate(passkey) end end # Verifies the assertion response against this passkey's stored credential and updates the # +sign_count+ and +backed_up+ attributes. Returns +self+ on success, or +nil+ if the # response is invalid. - def authenticate(passkey, challenge: ActionPack::WebAuthn::Current.challenge) + def authenticate(passkey) credential = to_public_key_credential - credential.authenticate(passkey, challenge: challenge) + credential.authenticate(passkey) update!(sign_count: credential.sign_count, backed_up: credential.backed_up) self rescue ActionPack::WebAuthn::InvalidResponseError diff --git a/lib/action_pack/passkey/challenges_controller.rb b/lib/action_pack/passkey/challenges_controller.rb index 82886266a..3eca28236 100644 --- a/lib/action_pack/passkey/challenges_controller.rb +++ b/lib/action_pack/passkey/challenges_controller.rb @@ -5,9 +5,10 @@ # authentication ceremony so that the challenge is issued just-in-time rather # than embedded in the initial page load. # -# The generated challenge is stored in an encrypted, HTTP-only, same-site -# cookie and simultaneously returned in the JSON response body. The cookie is -# consumed by ActionPack::Passkey::Request on the subsequent form submission. +# The generated challenge is returned in the JSON response body. The challenge +# is a signed, expiring token that the server can verify on the subsequent +# form submission without needing server-side state — the challenge is +# extracted from the authenticator's +clientDataJSON+ response. # # == Route # @@ -15,17 +16,11 @@ # via +config.action_pack.passkey.routes_prefix+). # class ActionPack::Passkey::ChallengesController < ActionController::Base - COOKIE_NAME = :action_pack_passkey_challenge - include ActionPack::Passkey::Request - # Generates a fresh challenge, stores it in an encrypted cookie, and returns - # it as JSON. The cookie is consumed on the next passkey form submission. + # Generates a fresh challenge and returns it as JSON. def create - challenge = create_passkey_challenge - - cookies.encrypted[COOKIE_NAME] = { value: challenge, httponly: true, same_site: :lax, secure: !request.local? && request.ssl? } - render json: { challenge: challenge } + render json: { challenge: create_passkey_challenge } end private diff --git a/lib/action_pack/passkey/request.rb b/lib/action_pack/passkey/request.rb index 6e7832517..265b49bc6 100644 --- a/lib/action_pack/passkey/request.rb +++ b/lib/action_pack/passkey/request.rb @@ -43,9 +43,7 @@ # == Before Action # # Automatically populates +ActionPack::WebAuthn::Current+ with the request -# host, origin, and challenge (read from the encrypted cookie set by -# ChallengesController). The cookie is deleted after being read to prevent -# replay. +# host and origin. # module ActionPack::Passkey::Request extend ActiveSupport::Concern @@ -54,8 +52,6 @@ module ActionPack::Passkey::Request before_action do ActionPack::WebAuthn::Current.host = request.host ActionPack::WebAuthn::Current.origin = request.base_url - ActionPack::WebAuthn::Current.challenge = cookies.encrypted[ActionPack::Passkey::ChallengesController::COOKIE_NAME] - cookies.delete(ActionPack::Passkey::ChallengesController::COOKIE_NAME) end end diff --git a/lib/action_pack/web_authn/authenticator/assertion_response.rb b/lib/action_pack/web_authn/authenticator/assertion_response.rb index 6d4f07cd0..b8f1292ec 100644 --- a/lib/action_pack/web_authn/authenticator/assertion_response.rb +++ b/lib/action_pack/web_authn/authenticator/assertion_response.rb @@ -17,7 +17,6 @@ # authenticator_data: params[:response][:authenticatorData], # signature: params[:response][:signature], # credential: credential.to_public_key_credential, -# challenge: ActionPack::WebAuthn::Current.challenge, # origin: "https://example.com" # ) # diff --git a/lib/action_pack/web_authn/authenticator/attestation_response.rb b/lib/action_pack/web_authn/authenticator/attestation_response.rb index 58fdecacd..42ddda86f 100644 --- a/lib/action_pack/web_authn/authenticator/attestation_response.rb +++ b/lib/action_pack/web_authn/authenticator/attestation_response.rb @@ -9,7 +9,6 @@ # response = ActionPack::WebAuthn::Authenticator::AttestationResponse.new( # client_data_json: params[:response][:clientDataJSON], # attestation_object: params[:response][:attestationObject], -# challenge: ActionPack::WebAuthn::Current.challenge, # origin: "https://example.com" # ) # diff --git a/lib/action_pack/web_authn/authenticator/response.rb b/lib/action_pack/web_authn/authenticator/response.rb index af878405b..fabb07815 100644 --- a/lib/action_pack/web_authn/authenticator/response.rb +++ b/lib/action_pack/web_authn/authenticator/response.rb @@ -23,7 +23,6 @@ # authenticator_data: authenticator_data, # signature: signature, # credential: credential, -# challenge: ActionPack::WebAuthn::Current.challenge, # origin: "https://example.com", # user_verification: :required # ) @@ -34,9 +33,9 @@ class ActionPack::WebAuthn::Authenticator::Response include ActiveModel::Validations attr_reader :client_data_json - attr_accessor :challenge, :origin, :user_verification + attr_accessor :origin, :user_verification - validate :challenge_must_match + validate :challenge_must_be_present validate :challenge_must_not_be_expired validate :origin_must_match validate :must_not_be_cross_origin @@ -45,9 +44,8 @@ class ActionPack::WebAuthn::Authenticator::Response validate :user_must_be_present validate :user_must_be_verified_when_required - def initialize(client_data_json:, challenge: nil, origin: nil, user_verification: :preferred) + def initialize(client_data_json:, origin: nil, user_verification: :preferred) @client_data_json = client_data_json - @challenge = challenge @origin = origin @user_verification = user_verification.to_sym end @@ -76,20 +74,16 @@ class ActionPack::WebAuthn::Authenticator::Response end private - def challenge_must_match - if challenge.blank? + def challenge_must_be_present + if client_data["challenge"].blank? errors.add(:base, "Challenge missing") - elsif client_data["challenge"].blank? - errors.add(:base, "Challenge missing in client data") - elsif !ActiveSupport::SecurityUtils.secure_compare(challenge.to_s, client_data["challenge"].to_s) - errors.add(:base, "Challenge does not match") end end def challenge_must_not_be_expired - return if errors.any? || challenge.blank? + return if errors.any? - signed_message = Base64.urlsafe_decode64(challenge) + signed_message = Base64.urlsafe_decode64(client_data["challenge"]) unless ActionPack::WebAuthn.challenge_verifier.verified(signed_message) errors.add(:base, "Challenge has expired") diff --git a/lib/action_pack/web_authn/current.rb b/lib/action_pack/web_authn/current.rb index 7189a4dd5..77293e83d 100644 --- a/lib/action_pack/web_authn/current.rb +++ b/lib/action_pack/web_authn/current.rb @@ -14,10 +14,6 @@ # The expected origin (typically +request.base_url+). Validated against the # +origin+ field in the authenticator's client data. # -# [+challenge+] -# The Base64URL-encoded challenge read from the encrypted cookie. Validated -# against the +challenge+ field in the authenticator's client data. -# class ActionPack::WebAuthn::Current < ActiveSupport::CurrentAttributes - attribute :host, :origin, :challenge + attribute :host, :origin end diff --git a/lib/action_pack/web_authn/public_key_credential.rb b/lib/action_pack/web_authn/public_key_credential.rb index 647443ef5..5279f29c9 100644 --- a/lib/action_pack/web_authn/public_key_credential.rb +++ b/lib/action_pack/web_authn/public_key_credential.rb @@ -10,7 +10,6 @@ # # credential = ActionPack::WebAuthn::PublicKeyCredential.register( # params[:passkey], -# challenge: ActionPack::WebAuthn::Current.challenge, # origin: ActionPack::WebAuthn::Current.origin # ) # @@ -80,11 +79,10 @@ class ActionPack::WebAuthn::PublicKeyCredential # PublicKeyCredential with the registered credential data. # # Raises +InvalidResponseError+ if the attestation is invalid. - def register(params, challenge: ActionPack::WebAuthn::Current.challenge, origin: ActionPack::WebAuthn::Current.origin) + def register(params, origin: ActionPack::WebAuthn::Current.origin) response = ActionPack::WebAuthn::Authenticator::AttestationResponse.new( client_data_json: params[:client_data_json], attestation_object: params[:attestation_object], - challenge: challenge, origin: origin ) @@ -126,13 +124,12 @@ class ActionPack::WebAuthn::PublicKeyCredential # Updates +sign_count+ and +backed_up+ on success. # # Raises +InvalidResponseError+ if the assertion is invalid. - def authenticate(params, challenge: ActionPack::WebAuthn::Current.challenge, origin: ActionPack::WebAuthn::Current.origin) + def authenticate(params, origin: ActionPack::WebAuthn::Current.origin) response = ActionPack::WebAuthn::Authenticator::AssertionResponse.new( client_data_json: params[:client_data_json], authenticator_data: params[:authenticator_data], signature: params[:signature], credential: self, - challenge: challenge, origin: origin ) diff --git a/test/controllers/my/passkey_challenges_controller_test.rb b/test/controllers/my/passkey_challenges_controller_test.rb index 80f64298c..43e0fb5fb 100644 --- a/test/controllers/my/passkey_challenges_controller_test.rb +++ b/test/controllers/my/passkey_challenges_controller_test.rb @@ -10,15 +10,6 @@ class My::PasskeyChallengesControllerTest < ActionDispatch::IntegrationTest end end - test "stores challenge in cookie" do - untenanted do - post my_passkey_challenge_url - - jar = ActionDispatch::Cookies::CookieJar.build(request, cookies.to_hash) - assert_equal response.parsed_body["challenge"], jar.encrypted[ActionPack::Passkey::ChallengesController::COOKIE_NAME] - end - end - test "returns a different challenge each time" do untenanted do post my_passkey_challenge_url diff --git a/test/lib/action_pack/passkey_test.rb b/test/lib/action_pack/passkey_test.rb index 56dc49095..a2c6ad934 100644 --- a/test/lib/action_pack/passkey_test.rb +++ b/test/lib/action_pack/passkey_test.rb @@ -20,7 +20,7 @@ class ActionPack::PasskeyTest < ActiveSupport::TestCase challenge = ActionPack::Passkey.request_options(credentials: [ @passkey ]).challenge assertion = build_assertion(challenge: challenge) - result = @passkey.authenticate(assertion, challenge: challenge) + result = @passkey.authenticate(assertion) assert_equal @passkey, result end @@ -30,14 +30,14 @@ class ActionPack::PasskeyTest < ActiveSupport::TestCase assertion = build_assertion(challenge: challenge) assertion[:signature] = Base64.urlsafe_encode64("invalid", padding: false) - assert_nil @passkey.authenticate(assertion, challenge: challenge) + assert_nil @passkey.authenticate(assertion) end test "authenticate updates sign count and backed_up" do challenge = ActionPack::Passkey.request_options(credentials: [ @passkey ]).challenge assertion = build_assertion(challenge: challenge, sign_count: 5, backed_up: true) - @passkey.authenticate(assertion, challenge: challenge) + @passkey.authenticate(assertion) assert_equal 5, @passkey.reload.sign_count assert @passkey.backed_up? diff --git a/test/lib/action_pack/web_authn/authenticator/assertion_response_test.rb b/test/lib/action_pack/web_authn/authenticator/assertion_response_test.rb index c0b2a96c4..abdad8122 100644 --- a/test/lib/action_pack/web_authn/authenticator/assertion_response_test.rb +++ b/test/lib/action_pack/web_authn/authenticator/assertion_response_test.rb @@ -27,7 +27,6 @@ class ActionPack::WebAuthn::Authenticator::AssertionResponseTest < ActiveSupport authenticator_data: @authenticator_data, signature: @signature, credential: @credential, - challenge: @challenge, origin: @origin ) end @@ -56,7 +55,6 @@ class ActionPack::WebAuthn::Authenticator::AssertionResponseTest < ActiveSupport authenticator_data: @authenticator_data, signature: sign(@authenticator_data, client_data_json), credential: @credential, - challenge: @challenge, origin: @origin ) @@ -73,7 +71,6 @@ class ActionPack::WebAuthn::Authenticator::AssertionResponseTest < ActiveSupport authenticator_data: @authenticator_data, signature: Base64.urlsafe_encode64("invalid-signature", padding: false), credential: @credential, - challenge: @challenge, origin: @origin ) @@ -84,14 +81,28 @@ class ActionPack::WebAuthn::Authenticator::AssertionResponseTest < ActiveSupport assert_equal "Invalid signature", error.message end - test "validate! raises when challenge does not match" do - @response.challenge = "wrong-challenge" + test "validate! raises when challenge in client data is invalid" do + client_data_json = { + challenge: "not-a-valid-signed-challenge", + origin: @origin, + type: "webauthn.get" + }.to_json + + authenticator_data = build_authenticator_data(user_verified: true) + + response = ActionPack::WebAuthn::Authenticator::AssertionResponse.new( + client_data_json: client_data_json, + authenticator_data: authenticator_data, + signature: sign(authenticator_data, client_data_json), + credential: @credential, + origin: @origin + ) error = assert_raises(ActionPack::WebAuthn::InvalidResponseError) do - @response.validate! + response.validate! end - assert_equal "Challenge does not match", error.message + assert_match /Challenge (is invalid|has expired)/, error.message end test "validate! raises when origin does not match" do @@ -111,7 +122,6 @@ class ActionPack::WebAuthn::Authenticator::AssertionResponseTest < ActiveSupport authenticator_data: authenticator_data, signature: sign(authenticator_data, @client_data_json), credential: @credential, - challenge: @challenge, origin: @origin, user_verification: :preferred ) @@ -128,7 +138,6 @@ class ActionPack::WebAuthn::Authenticator::AssertionResponseTest < ActiveSupport authenticator_data: authenticator_data, signature: sign(authenticator_data, @client_data_json), credential: @credential, - challenge: @challenge, origin: @origin, user_verification: :required ) @@ -145,7 +154,6 @@ class ActionPack::WebAuthn::Authenticator::AssertionResponseTest < ActiveSupport authenticator_data: authenticator_data, signature: sign(authenticator_data, @client_data_json), credential: @credential, - challenge: @challenge, origin: @origin, user_verification: :required ) diff --git a/test/lib/action_pack/web_authn/authenticator/attestation_response_test.rb b/test/lib/action_pack/web_authn/authenticator/attestation_response_test.rb index 7aaca8ae4..727cd9c5e 100644 --- a/test/lib/action_pack/web_authn/authenticator/attestation_response_test.rb +++ b/test/lib/action_pack/web_authn/authenticator/attestation_response_test.rb @@ -48,7 +48,6 @@ class ActionPack::WebAuthn::Authenticator::AttestationResponseTest < ActiveSuppo @response = ActionPack::WebAuthn::Authenticator::AttestationResponse.new( client_data_json: @client_data_json, attestation_object: ATTESTATION_NONE_VERIFIED, - challenge: @challenge, origin: @origin ) end @@ -67,7 +66,6 @@ class ActionPack::WebAuthn::Authenticator::AttestationResponseTest < ActiveSuppo response = ActionPack::WebAuthn::Authenticator::AttestationResponse.new( client_data_json: @client_data_json, attestation_object: ATTESTATION_NONE_NOT_VERIFIED, - challenge: @challenge, origin: @origin, user_verification: :preferred ) @@ -81,7 +79,6 @@ class ActionPack::WebAuthn::Authenticator::AttestationResponseTest < ActiveSuppo response = ActionPack::WebAuthn::Authenticator::AttestationResponse.new( client_data_json: @client_data_json, attestation_object: ATTESTATION_NONE_VERIFIED, - challenge: @challenge, origin: @origin, user_verification: :required ) @@ -95,7 +92,6 @@ class ActionPack::WebAuthn::Authenticator::AttestationResponseTest < ActiveSuppo response = ActionPack::WebAuthn::Authenticator::AttestationResponse.new( client_data_json: @client_data_json, attestation_object: ATTESTATION_NONE_NOT_VERIFIED, - challenge: @challenge, origin: @origin, user_verification: :required ) @@ -117,7 +113,6 @@ class ActionPack::WebAuthn::Authenticator::AttestationResponseTest < ActiveSuppo response = ActionPack::WebAuthn::Authenticator::AttestationResponse.new( client_data_json: client_data_json, attestation_object: ATTESTATION_NONE_VERIFIED, - challenge: @challenge, origin: @origin ) @@ -128,14 +123,24 @@ class ActionPack::WebAuthn::Authenticator::AttestationResponseTest < ActiveSuppo assert_equal "Client data type is not webauthn.create", error.message end - test "validate! raises when challenge does not match" do - @response.challenge = "wrong-challenge" + test "validate! raises when challenge in client data is invalid" do + client_data_json = { + challenge: "not-a-valid-signed-challenge", + origin: @origin, + type: "webauthn.create" + }.to_json + + response = ActionPack::WebAuthn::Authenticator::AttestationResponse.new( + client_data_json: client_data_json, + attestation_object: ATTESTATION_NONE_VERIFIED, + origin: @origin + ) error = assert_raises(ActionPack::WebAuthn::InvalidResponseError) do - @response.validate! + response.validate! end - assert_equal "Challenge does not match", error.message + assert_match /Challenge (is invalid|has expired)/, error.message end test "validate! raises when origin does not match" do @@ -152,7 +157,6 @@ class ActionPack::WebAuthn::Authenticator::AttestationResponseTest < ActiveSuppo response = ActionPack::WebAuthn::Authenticator::AttestationResponse.new( client_data_json: @client_data_json, attestation_object: ATTESTATION_PACKED_VERIFIED, - challenge: @challenge, origin: @origin ) @@ -173,7 +177,6 @@ class ActionPack::WebAuthn::Authenticator::AttestationResponseTest < ActiveSuppo response = ActionPack::WebAuthn::Authenticator::AttestationResponse.new( client_data_json: @client_data_json, attestation_object: ATTESTATION_PACKED_VERIFIED, - challenge: @challenge, origin: @origin ) diff --git a/test/lib/action_pack/web_authn/authenticator/response_test.rb b/test/lib/action_pack/web_authn/authenticator/response_test.rb index ce0b5dc9f..bd1a9b180 100644 --- a/test/lib/action_pack/web_authn/authenticator/response_test.rb +++ b/test/lib/action_pack/web_authn/authenticator/response_test.rb @@ -18,7 +18,6 @@ class ActionPack::WebAuthn::Authenticator::ResponseTest < ActiveSupport::TestCas @response = TestableResponse.new( client_data_json: @client_data_json, authenticator_data: @authenticator_data, - challenge: @challenge, origin: @origin ) end @@ -41,9 +40,20 @@ class ActionPack::WebAuthn::Authenticator::ResponseTest < ActiveSupport::TestCas assert @response.valid? end - test "valid? returns false when challenge does not match" do - @response.challenge = "wrong-challenge" - assert_not @response.valid? + test "valid? returns false when challenge in client data is invalid" do + client_data_json = { + challenge: "not-a-valid-signed-challenge", + origin: @origin, + type: "webauthn.create" + }.to_json + + response = TestableResponse.new( + client_data_json: client_data_json, + authenticator_data: @authenticator_data, + origin: @origin + ) + + assert_not response.valid? end test "valid? returns false when origin does not match" do @@ -51,14 +61,24 @@ class ActionPack::WebAuthn::Authenticator::ResponseTest < ActiveSupport::TestCas assert_not @response.valid? end - test "validate! raises when challenge does not match" do - @response.challenge = "wrong-challenge" + test "validate! raises when challenge in client data is invalid" do + client_data_json = { + challenge: "not-a-valid-signed-challenge", + origin: @origin, + type: "webauthn.create" + }.to_json + + response = TestableResponse.new( + client_data_json: client_data_json, + authenticator_data: @authenticator_data, + origin: @origin + ) error = assert_raises(ActionPack::WebAuthn::InvalidResponseError) do - @response.validate! + response.validate! end - assert_equal "Challenge does not match", error.message + assert_match /Challenge (is invalid|has expired)/, error.message end test "validate! raises when origin does not match" do @@ -82,7 +102,6 @@ class ActionPack::WebAuthn::Authenticator::ResponseTest < ActiveSupport::TestCas response = TestableResponse.new( client_data_json: client_data_json, authenticator_data: @authenticator_data, - challenge: @challenge, origin: @origin ) @@ -108,7 +127,6 @@ class ActionPack::WebAuthn::Authenticator::ResponseTest < ActiveSupport::TestCas response = TestableResponse.new( client_data_json: @client_data_json, authenticator_data: wrong_rp_data, - challenge: @challenge, origin: @origin ) @@ -130,7 +148,6 @@ class ActionPack::WebAuthn::Authenticator::ResponseTest < ActiveSupport::TestCas response = TestableResponse.new( client_data_json: client_data_json, authenticator_data: @authenticator_data, - challenge: @challenge, origin: @origin )