From af83f77eae055472ab45b9984ac1c73067c4fa65 Mon Sep 17 00:00:00 2001 From: Braulio Martinez Date: Wed, 23 May 2018 19:13:21 -0300 Subject: [PATCH 1/3] feat: add client data origin validation --- .../authenticator_attestation_response.rb | 7 ++- lib/webauthn/client_data.rb | 4 ++ spec/spec_helper.rb | 9 ++++ ...authenticator_attestation_response_spec.rb | 47 ++++++++++++++++++- 4 files changed, 64 insertions(+), 3 deletions(-) diff --git a/lib/webauthn/authenticator_attestation_response.rb b/lib/webauthn/authenticator_attestation_response.rb index 81f4be49..78d25828 100644 --- a/lib/webauthn/authenticator_attestation_response.rb +++ b/lib/webauthn/authenticator_attestation_response.rb @@ -14,9 +14,10 @@ def initialize(attestation_object:, client_data_json:) @client_data_json = client_data_json end - def valid?(original_challenge) + def valid?(original_challenge, original_origin) valid_type? && valid_challenge?(original_challenge) && + valid_origin?(original_origin) && authenticator_data.valid? && user_present? && valid_attestation_statement? @@ -34,6 +35,10 @@ def valid_challenge?(original_challenge) Base64.urlsafe_decode64(client_data.challenge) == Base64.urlsafe_decode64(original_challenge) end + def valid_origin?(original_origin) + client_data.origin == original_origin + end + def valid_attestation_statement? if attestation_format == ATTESTATION_FORMAT_NONE true diff --git a/lib/webauthn/client_data.rb b/lib/webauthn/client_data.rb index f92be91f..af25a76c 100644 --- a/lib/webauthn/client_data.rb +++ b/lib/webauthn/client_data.rb @@ -14,6 +14,10 @@ def challenge data["challenge"] end + def origin + data["origin"] + end + private attr_reader :client_data_json diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 425fbbdb..7b148422 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,6 +2,7 @@ require "bundler/setup" require "webauthn" +require "cbor" require "byebug" @@ -39,3 +40,11 @@ def seeds } } end + +def hash_to_encoded_json(hash) + Base64.urlsafe_encode64(hash.to_json) +end + +def hash_to_encoded_cbor(hash) + Base64.urlsafe_encode64(CBOR.encode(hash)) +end diff --git a/spec/webauthn/authenticator_attestation_response_spec.rb b/spec/webauthn/authenticator_attestation_response_spec.rb index 0948874c..053e6933 100644 --- a/spec/webauthn/authenticator_attestation_response_spec.rb +++ b/spec/webauthn/authenticator_attestation_response_spec.rb @@ -12,7 +12,7 @@ client_data_json: response[:client_data_json] ) - expect(response.valid?(original_challenge)).to eq(true) + expect(response.valid?(original_challenge, "http://localhost:3000")).to eq(true) end it "returns user-friendly error if no client data received" do @@ -22,7 +22,50 @@ ) expect { - response.valid?("") + response.valid?("", "") }.to raise_exception(RuntimeError, "Missing client_data_json") end + + describe "origin validation" do + let(:base_url) { "http://localhost" } + let(:challenge) { Base64.urlsafe_encode64(SecureRandom.random_bytes(16)) } + let(:client_data_json) { hash_to_encoded_json(challenge: challenge, + clientExtensions: {}, + hashAlgorithm: "SHA-256", + origin: origin, + type: "webauthn.create") } + let(:auth_data) { [73, 150, 13, 229, 136, 14, 140, 104, 116, 52, 23, 15, 100, + 118, 96, 91, 143, 228, 174, 185, 162, 134, 50, 199, 153, + 92, 243, 186, 131, 29, 151, 99, 65, 0, 0, 0, 0].pack('c*') } + let(:attestation_object) { hash_to_encoded_cbor(fmt: "none", + attStmt: {}, + authData: auth_data) } + + context "matches the default one" do + let(:origin) { "http://localhost" } + + it "is valid" do + response = WebAuthn::AuthenticatorAttestationResponse.new( + attestation_object: attestation_object, + client_data_json: client_data_json + ) + + expect(response.valid?(challenge, base_url)).to be_truthy + end + end + + context "doesn't match the default one" do + let(:origin) { "http://invalid" } + + it "isn't valid" do + response = WebAuthn::AuthenticatorAttestationResponse.new( + attestation_object: attestation_object, + client_data_json: client_data_json + ) + + expect(response.valid?(challenge, base_url)).to be_falsy + end + end + end + end From f0250e79fcf8c801994f3358c79cc0035e65ef97 Mon Sep 17 00:00:00 2001 From: Braulio Martinez Date: Thu, 24 May 2018 10:51:41 -0300 Subject: [PATCH 2/3] refactor: rename var in spec --- spec/webauthn/authenticator_attestation_response_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/webauthn/authenticator_attestation_response_spec.rb b/spec/webauthn/authenticator_attestation_response_spec.rb index 053e6933..c8584a57 100644 --- a/spec/webauthn/authenticator_attestation_response_spec.rb +++ b/spec/webauthn/authenticator_attestation_response_spec.rb @@ -27,7 +27,7 @@ end describe "origin validation" do - let(:base_url) { "http://localhost" } + let(:original_origin) { "http://localhost" } let(:challenge) { Base64.urlsafe_encode64(SecureRandom.random_bytes(16)) } let(:client_data_json) { hash_to_encoded_json(challenge: challenge, clientExtensions: {}, @@ -50,7 +50,7 @@ client_data_json: client_data_json ) - expect(response.valid?(challenge, base_url)).to be_truthy + expect(response.valid?(challenge, original_origin)).to be_truthy end end @@ -63,7 +63,7 @@ client_data_json: client_data_json ) - expect(response.valid?(challenge, base_url)).to be_falsy + expect(response.valid?(challenge, original_origin)).to be_falsy end end end From 0d5b205e7fdca201de53a695dec487f3e1dcc49e Mon Sep 17 00:00:00 2001 From: Braulio Martinez Date: Thu, 24 May 2018 10:56:55 -0300 Subject: [PATCH 3/3] refactor: styles --- ...authenticator_attestation_response_spec.rb | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/spec/webauthn/authenticator_attestation_response_spec.rb b/spec/webauthn/authenticator_attestation_response_spec.rb index c8584a57..6315e41a 100644 --- a/spec/webauthn/authenticator_attestation_response_spec.rb +++ b/spec/webauthn/authenticator_attestation_response_spec.rb @@ -28,18 +28,26 @@ describe "origin validation" do let(:original_origin) { "http://localhost" } - let(:challenge) { Base64.urlsafe_encode64(SecureRandom.random_bytes(16)) } - let(:client_data_json) { hash_to_encoded_json(challenge: challenge, - clientExtensions: {}, - hashAlgorithm: "SHA-256", - origin: origin, - type: "webauthn.create") } - let(:auth_data) { [73, 150, 13, 229, 136, 14, 140, 104, 116, 52, 23, 15, 100, - 118, 96, 91, 143, 228, 174, 185, 162, 134, 50, 199, 153, - 92, 243, 186, 131, 29, 151, 99, 65, 0, 0, 0, 0].pack('c*') } - let(:attestation_object) { hash_to_encoded_cbor(fmt: "none", - attStmt: {}, - authData: auth_data) } + let(:challenge) { + Base64.urlsafe_encode64(SecureRandom.random_bytes(16)) + } + let(:client_data_json) { + hash_to_encoded_json(challenge: challenge, + clientExtensions: {}, + hashAlgorithm: "SHA-256", + origin: origin, + type: "webauthn.create") + } + let(:auth_data) { + [73, 150, 13, 229, 136, 14, 140, 104, 116, 52, 23, 15, 100, + 118, 96, 91, 143, 228, 174, 185, 162, 134, 50, 199, 153, + 92, 243, 186, 131, 29, 151, 99, 65, 0, 0, 0, 0].pack('c*') + } + let(:attestation_object) { + hash_to_encoded_cbor(fmt: "none", + attStmt: {}, + authData: auth_data) + } context "matches the default one" do let(:origin) { "http://localhost" } @@ -67,5 +75,4 @@ end end end - end