diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index b8bc2ec..66fadc1 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -295,10 +295,14 @@ class SessionsController < ApplicationController sign_count: stored_credential.sign_count ) - # Check for suspicious sign count (possible clone) + # Clone detection: a non-advancing signature counter signals the credential + # may have been copied. Reject the sign-in (do NOT create a session or update + # the stored counter) and alert the user, per WebAuthn §6.1.1. if stored_credential.suspicious_sign_count?(webauthn_credential.sign_count) - Rails.logger.warn "Suspicious WebAuthn sign count for user #{user.id}, credential #{stored_credential.id}" - # You might want to notify admins or temporarily disable the credential + Rails.logger.warn "Suspicious WebAuthn sign count for user #{user.id}, credential #{stored_credential.id} (stored=#{stored_credential.sign_count}, presented=#{webauthn_credential.sign_count})" + SecurityMailer.suspicious_passkey_used(user, nickname: stored_credential.display_name, **security_event_context).deliver_later + render json: {error: "Passkey authentication could not be completed. Please contact support."}, status: :unprocessable_entity + return end # Update credential usage diff --git a/app/mailers/security_mailer.rb b/app/mailers/security_mailer.rb index 9630da5..215f80a 100644 --- a/app/mailers/security_mailer.rb +++ b/app/mailers/security_mailer.rb @@ -40,6 +40,12 @@ class SecurityMailer < ApplicationMailer mail subject: "#{SUBJECT_PREFIX}An API key was revoked on your account", to: user.email_address end + def suspicious_passkey_used(user, nickname:, ip:, user_agent:, occurred_at:) + assign_context(user, ip, user_agent, occurred_at) + @nickname = nickname + mail subject: "#{SUBJECT_PREFIX}A passkey sign-in was blocked", to: user.email_address + end + def email_address_changed(user, recipient:, old_email:, new_email:, ip:, user_agent:, occurred_at:) assign_context(user, ip, user_agent, occurred_at) @recipient = recipient diff --git a/app/models/webauthn_credential.rb b/app/models/webauthn_credential.rb index 571a620..f22078c 100644 --- a/app/models/webauthn_credential.rb +++ b/app/models/webauthn_credential.rb @@ -52,13 +52,17 @@ class WebauthnCredential < ApplicationRecord end end - # Check if sign count is suspicious (clone detection) + # Check if sign count is suspicious (clone detection). + # + # Per WebAuthn §6.1.1, a signature counter of 0 means the authenticator does + # not implement a counter (true of most synced passkeys — Apple/Google report + # 0 every time), so it cannot be used for clone detection. Only when BOTH the + # stored and presented counts are non-zero does a non-increasing value signal + # a possible clone. def suspicious_sign_count?(new_sign_count) - return false if sign_count.zero? && new_sign_count > 0 # First use - return false if new_sign_count > sign_count # Normal increment + return false if sign_count.zero? || new_sign_count.zero? - # Sign count didn't increase - possible clone - true + new_sign_count <= sign_count end # Format for display in UI diff --git a/app/views/security_mailer/suspicious_passkey_used.html.erb b/app/views/security_mailer/suspicious_passkey_used.html.erb new file mode 100644 index 0000000..4d620c2 --- /dev/null +++ b/app/views/security_mailer/suspicious_passkey_used.html.erb @@ -0,0 +1,16 @@ +

Hello,

+ +

+ A sign-in to your Clinch account (<%= @user.email_address %>) + using your passkey (<%= @nickname %>) was blocked + because its security counter did not advance as expected. This can indicate the + passkey has been copied (cloned). +

+ +

+ If this was you and you are unable to sign in, remove this passkey and register + a new one. If you do not recognise this activity, treat it as a compromise: + remove the passkey and review your account security. +

+ +<%= render "event_metadata" %> diff --git a/app/views/security_mailer/suspicious_passkey_used.text.erb b/app/views/security_mailer/suspicious_passkey_used.text.erb new file mode 100644 index 0000000..7063685 --- /dev/null +++ b/app/views/security_mailer/suspicious_passkey_used.text.erb @@ -0,0 +1,11 @@ +Hello, + +A sign-in to your Clinch account (<%= @user.email_address %>) using your passkey +("<%= @nickname %>") was BLOCKED because its security counter did not advance as +expected. This can indicate the passkey has been copied (cloned). + +If this was you and you are unable to sign in, remove this passkey and register a +new one. If you do not recognise this activity, treat it as a compromise: remove +the passkey and review your account security. + +<%= render "event_metadata" %> diff --git a/test/mailers/security_mailer_test.rb b/test/mailers/security_mailer_test.rb index f89e6d5..dc8354e 100644 --- a/test/mailers/security_mailer_test.rb +++ b/test/mailers/security_mailer_test.rb @@ -54,6 +54,15 @@ class SecurityMailerTest < ActionMailer::TestCase assert_bodies_contain email, "Old MacBook" end + test "suspicious_passkey_used warns about a blocked clone sign-in" do + email = SecurityMailer.suspicious_passkey_used(@user, nickname: "Yubikey-5", **CONTEXT) + + assert_equal [@user.email_address], email.to + assert_match(/blocked/i, email.subject) + assert_bodies_contain email, "Yubikey-5" + assert_bodies_match email, /clon/i + end + test "api_key_created includes the key name" do email = SecurityMailer.api_key_created(@user, name: "CI bot", **CONTEXT) diff --git a/test/models/webauthn_credential_test.rb b/test/models/webauthn_credential_test.rb new file mode 100644 index 0000000..fd66b14 --- /dev/null +++ b/test/models/webauthn_credential_test.rb @@ -0,0 +1,29 @@ +require "test_helper" + +class WebauthnCredentialTest < ActiveSupport::TestCase + # suspicious_sign_count?(new_sign_count) — clone detection per WebAuthn §6.1.1. + # Build an in-memory credential with a given stored sign_count; no persistence + # needed since the method only reads self.sign_count. + def credential(stored:) + WebauthnCredential.new(sign_count: stored) + end + + test "does not flag when the authenticator reports no counter (synced passkeys)" do + # Both 0 -> authenticator doesn't implement a counter; must NOT be suspicious. + refute credential(stored: 0).suspicious_sign_count?(0) + # Stored 0, first real use. + refute credential(stored: 0).suspicious_sign_count?(5) + # Stored non-zero but authenticator now reports 0 -> no counter, not a clone. + refute credential(stored: 5).suspicious_sign_count?(0) + end + + test "does not flag a normal increasing counter" do + refute credential(stored: 5).suspicious_sign_count?(6) + refute credential(stored: 1).suspicious_sign_count?(1000) + end + + test "flags a non-advancing counter as a possible clone" do + assert credential(stored: 5).suspicious_sign_count?(5), "equal count is suspicious" + assert credential(stored: 5).suspicious_sign_count?(3), "decreasing count is suspicious" + end +end