From c031434b8653dc1a67e8da3712c0866cfde62cb6 Mon Sep 17 00:00:00 2001
From: Martin Brennan <mjrbrennan@gmail.com>
Date: Thu, 2 Jan 2020 10:14:22 +1000
Subject: [PATCH] FIX: Catch error when unknown COSE algorithm is supplied for
 Security Key (#8649)

Added a fix to gracefully error with a Webauthn::SecurityKeyError if somehow a user provides an unkown COSE algorithm when logging in with a security key.

If `COSE::Algorithm.find` returns nil we now fail gracefully and log the algorithm used along with the user ID and the security key params for debugging, as this will help us find other common algorithms to implement for webauthn
---
 config/locales/server.en.yml                         |  1 +
 lib/webauthn.rb                                      |  1 +
 lib/webauthn/security_key_authentication_service.rb  |  9 ++++++++-
 .../security_key_authentication_service_spec.rb      | 12 ++++++++++++
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index a3122c73d97..1c5eeb7c467 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -943,6 +943,7 @@ en:
       public_key_error: "The public key verification for the credential failed."
       ownership_error: "The security key is not owned by the user."
       not_found_error: "A security key with the provided credential ID could not be found."
+      unknown_cose_algorithm_error: "The algorithm used for the security key is not recognized."
 
   topic_flag_types:
     spam:
diff --git a/lib/webauthn.rb b/lib/webauthn.rb
index c8656d8fa70..cd4ecb917fd 100644
--- a/lib/webauthn.rb
+++ b/lib/webauthn.rb
@@ -26,4 +26,5 @@ module Webauthn
   class NotFoundError < SecurityKeyError; end
   class OwnershipError < SecurityKeyError; end
   class PublicKeyError < SecurityKeyError; end
+  class UnknownCOSEAlgorithmError < SecurityKeyError; end
 end
diff --git a/lib/webauthn/security_key_authentication_service.rb b/lib/webauthn/security_key_authentication_service.rb
index 34a840e167d..3256c0683f9 100644
--- a/lib/webauthn/security_key_authentication_service.rb
+++ b/lib/webauthn/security_key_authentication_service.rb
@@ -64,7 +64,14 @@ module Webauthn
 
       # 17. Using credentialPublicKey, verify that sig is a valid signature over the binary concatenation of authData and hash.
       cose_key = COSE::Key.deserialize(Base64.decode64(security_key.public_key))
-      if !cose_key.to_pkey.verify(COSE::Algorithm.find(cose_key.alg).hash_function, signature, auth_data + client_data_hash)
+      cose_algorithm = COSE::Algorithm.find(cose_key.alg)
+
+      if cose_algorithm.blank?
+        Rails.logger.error("Unknown COSE algorithm encountered. alg: #{cose_key.alg}. user_id: #{@current_user.id}. params: #{@params.inspect}")
+        raise(UnknownCOSEAlgorithmError, I18n.t('webauthn.validation.unknown_cose_algorithm_error'))
+      end
+
+      if !cose_key.to_pkey.verify(cose_algorithm.hash_function, signature, auth_data + client_data_hash)
         raise(PublicKeyError, I18n.t('webauthn.validation.public_key_error'))
       end
 
diff --git a/spec/lib/webauthn/security_key_authentication_service_spec.rb b/spec/lib/webauthn/security_key_authentication_service_spec.rb
index 8f6ce0c529d..c8d0a882150 100644
--- a/spec/lib/webauthn/security_key_authentication_service_spec.rb
+++ b/spec/lib/webauthn/security_key_authentication_service_spec.rb
@@ -131,4 +131,16 @@ describe Webauthn::SecurityKeyAuthenticationService do
       )
     end
   end
+
+  context 'when the COSE algorithm used cannot be found' do
+    before do
+      COSE::Algorithm.expects(:find).returns(nil)
+    end
+
+    it 'raises a UnknownCOSEAlgorithmError' do
+      expect { subject.authenticate_security_key }.to raise_error(
+        Webauthn::UnknownCOSEAlgorithmError, I18n.t('webauthn.validation.unknown_cose_algorithm_error')
+      )
+    end
+  end
 end