From 29f169294a782461a06d370ee5b0466078fc8c48 Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Tue, 2 Oct 2018 19:47:45 -0300 Subject: [PATCH 1/2] fix: consider user present in UV (User Verified) flag present --- lib/webauthn/authenticator_data.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/webauthn/authenticator_data.rb b/lib/webauthn/authenticator_data.rb index 08e0c3a6..c246779e 100644 --- a/lib/webauthn/authenticator_data.rb +++ b/lib/webauthn/authenticator_data.rb @@ -11,6 +11,7 @@ class AuthenticatorData SIGN_COUNT_LENGTH = 4 USER_PRESENT_FLAG_POSITION = 0 + USER_VERIFIED_FLAG_POSITION = 2 ATTESTED_CREDENTIAL_DATA_INCLUDED_FLAG_POSITION = 6 def initialize(data) @@ -26,7 +27,11 @@ def valid? end def user_present? - flags[USER_PRESENT_FLAG_POSITION] == "1" + flags[USER_PRESENT_FLAG_POSITION] == "1" || user_verified? + end + + def user_verified? + flags[USER_VERIFIED_FLAG_POSITION] == "1" end def rp_id_hash From d6c27114c5c6ad838aa567bdc6fec020086ad94d Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Thu, 4 Oct 2018 20:56:19 -0300 Subject: [PATCH 2/2] fix: make response valid in case UV is present despite UP missing --- lib/webauthn/authenticator_data.rb | 6 +- lib/webauthn/authenticator_response.rb | 2 +- spec/support/fake_authenticator.rb | 14 +---- .../authenticator_assertion_response_spec.rb | 4 +- spec/webauthn/authenticator_data_spec.rb | 62 +++++++++++++++++-- 5 files changed, 69 insertions(+), 19 deletions(-) diff --git a/lib/webauthn/authenticator_data.rb b/lib/webauthn/authenticator_data.rb index a65be8a1..f89d0fc4 100644 --- a/lib/webauthn/authenticator_data.rb +++ b/lib/webauthn/authenticator_data.rb @@ -30,8 +30,12 @@ def valid? end end + def user_flagged? + user_present? || user_verified? + end + def user_present? - flags[USER_PRESENT_FLAG_POSITION] == "1" || user_verified? + flags[USER_PRESENT_FLAG_POSITION] == "1" end def user_verified? diff --git a/lib/webauthn/authenticator_response.rb b/lib/webauthn/authenticator_response.rb index 01057a74..a2da4777 100644 --- a/lib/webauthn/authenticator_response.rb +++ b/lib/webauthn/authenticator_response.rb @@ -12,7 +12,7 @@ def valid?(original_challenge, original_origin) valid_origin?(original_origin) && valid_rp_id?(original_origin) && authenticator_data.valid? && - authenticator_data.user_present? + authenticator_data.user_flagged? end def client_data diff --git a/spec/support/fake_authenticator.rb b/spec/support/fake_authenticator.rb index 862cd50e..20914e97 100644 --- a/spec/support/fake_authenticator.rb +++ b/spec/support/fake_authenticator.rb @@ -34,7 +34,7 @@ def rp_id_hash attr_reader :challenge, :context, :rp_id def raw_flags - ["#{user_present_bit}00000#{attested_credential_data_present_bit}0"].pack("b*") + ["#{bit(:user_present)}0#{bit(:user_verified)}000#{attested_credential_data_present_bit}0"].pack("b*") end def attested_credential_data_present_bit @@ -53,22 +53,14 @@ def raw_sign_count [@sign_count].pack('L>') end - def user_present_bit - if user_present? + def bit(flag) + if context[flag].nil? || context[flag] "1" else "0" end end - def user_present? - if context[:user_present].nil? - true - else - context[:user_present] - end - end - def origin @origin ||= context[:origin] || fake_origin end diff --git a/spec/webauthn/authenticator_assertion_response_spec.rb b/spec/webauthn/authenticator_assertion_response_spec.rb index 54c3c2e9..0b89ed28 100644 --- a/spec/webauthn/authenticator_assertion_response_spec.rb +++ b/spec/webauthn/authenticator_assertion_response_spec.rb @@ -111,11 +111,11 @@ let(:authenticator) do FakeAuthenticator::Get.new( challenge: original_challenge, - context: { origin: original_origin, user_present: false } + context: { origin: original_origin, user_present: false, user_verified: false } ) end - it "is invalid if user-present flag is off" do + it "is invalid if user flags are off" do expect( assertion_response.valid?( original_challenge, diff --git a/spec/webauthn/authenticator_data_spec.rb b/spec/webauthn/authenticator_data_spec.rb index 36a3bade..6cecd7fa 100644 --- a/spec/webauthn/authenticator_data_spec.rb +++ b/spec/webauthn/authenticator_data_spec.rb @@ -2,12 +2,17 @@ RSpec.describe WebAuthn::AuthenticatorData do let(:authenticator) do - FakeAuthenticator::Base.new(rp_id: rp_id, sign_count: sign_count, context: { user_present: user_presence }) + FakeAuthenticator::Base.new( + rp_id: rp_id, + sign_count: sign_count, + context: { user_present: user_present, user_verified: user_verified } + ) end let(:rp_id) { "localhost" } let(:sign_count) { 42 } - let(:user_presence) { true } + let(:user_present) { true } + let(:user_verified) { false } let(:authenticator_data) { described_class.new(authenticator.authenticator_data) } @@ -23,13 +28,62 @@ describe "#user_present?" do subject { authenticator_data.user_present? } + context "when UP flag is set" do - let(:user_presence) { true } + let(:user_present) { true } it { is_expected.to be_truthy } end context "when UP flag is not set" do - let(:user_presence) { false } + let(:user_present) { false } + it { is_expected.to be_falsy } + end + end + + describe "#user_verified?" do + subject { authenticator_data.user_verified? } + + context "when UV flag is set" do + let(:user_verified) { true } + + it { is_expected.to be_truthy } + end + + context "when UV flag is not set" do + let(:user_verified) { false } + + it { is_expected.to be_falsy } + end + end + + describe "#user_flagged?" do + subject { authenticator_data.user_flagged? } + + context "when both UP and UV flag are set" do + let(:user_present) { true } + let(:user_verified) { true } + + it { is_expected.to be_truthy } + end + + context "when only UP is set" do + let(:user_present) { true } + let(:user_verified) { false } + + it { is_expected.to be_truthy } + end + + context "when only UV flag is set" do + let(:user_present) { false } + let(:user_verified) { true } + + it { is_expected.to be_truthy } + end + + context "when both UP and UV flag are not set" do + let(:user_present) { false } + let(:user_verified) { false } + it { is_expected.to be_falsy } end end