From 3572582dd2568cd473676563077ab3985b9803f7 Mon Sep 17 00:00:00 2001
From: Timothy Andrew <mail@timothyandrew.net>
Date: Mon, 11 Jul 2016 13:02:24 +0530
Subject: [PATCH] Use a single challenge for U2F authentication.
1. According to the spec, either we have a single challenge with
a number of `signRequests`, or a number of `signRequests`, each with
it's own challenge.
2. Previously, we had both these - per-request challenges, as well as a
single extra challenge.
3. This commit changes this so that the per-request challenges are
removed, leaving only a single challenge, as per the v1.1 U2F API.
4. The existing implementation didn't work in Firefox, because the
Firefox (extension) implementation is less flexible with regard to
the inputs.
5. Fix teaspoon specs.
6. References: https://fidoalliance.org/specs/fido-u2f-v1.0-nfc-bt-amendment-20150514/fido-u2f-javascript-api.html#h2_background
---
app/assets/javascripts/u2f/authenticate.js.coffee | 15 ++++++++++++---
.../concerns/authenticates_with_two_factor.rb | 7 +++----
spec/javascripts/u2f/authenticate_spec.coffee | 3 +--
spec/javascripts/u2f/register_spec.js.coffee | 1 -
spec/support/fake_u2f_device.rb | 4 ++--
5 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/app/assets/javascripts/u2f/authenticate.js.coffee b/app/assets/javascripts/u2f/authenticate.js.coffee
index 6deb902c8de..be10e911c83 100644
--- a/app/assets/javascripts/u2f/authenticate.js.coffee
+++ b/app/assets/javascripts/u2f/authenticate.js.coffee
@@ -6,8 +6,17 @@
class @U2FAuthenticate
constructor: (@container, u2fParams) ->
@appId = u2fParams.app_id
- @challenges = u2fParams.challenges
- @signRequests = u2fParams.sign_requests
+ @challenge = u2fParams.challenge
+
+ # The U2F Javascript API v1.1 requires a single challenge, with _no
+ # challenges per-request_.
+ #
+ # The U2F Javascript API v1.0 requires a challenge per-request, which
+ # is done by copying the single challenge into every request.
+ #
+ # In either case, we don't need the per-request challenges that the server
+ # has generated, so we can remove them.
+ @signRequests = u2fParams.sign_requests.map (request) -> _(request).omit('challenge')
start: () =>
if U2FUtil.isU2FSupported()
@@ -16,7 +25,7 @@ class @U2FAuthenticate
@renderNotSupported()
authenticate: () =>
- u2f.sign(@appId, @challenges, @signRequests, (response) =>
+ u2f.sign(@appId, @challenge, @signRequests, (response) =>
if response.errorCode
error = new U2FError(response.errorCode)
@renderError(error);
diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb
index 0c755894790..ba07cea569c 100644
--- a/app/controllers/concerns/authenticates_with_two_factor.rb
+++ b/app/controllers/concerns/authenticates_with_two_factor.rb
@@ -57,7 +57,7 @@ module AuthenticatesWithTwoFactor
# Authenticate using the response from a U2F (universal 2nd factor) device
def authenticate_with_two_factor_via_u2f(user)
- if U2fRegistration.authenticate(user, u2f_app_id, user_params[:device_response], session[:challenges])
+ if U2fRegistration.authenticate(user, u2f_app_id, user_params[:device_response], session[:challenge])
# Remove any lingering user data from login
session.delete(:otp_user_id)
session.delete(:challenges)
@@ -77,9 +77,8 @@ module AuthenticatesWithTwoFactor
if key_handles.present?
sign_requests = u2f.authentication_requests(key_handles)
- challenges = sign_requests.map(&:challenge)
- session[:challenges] = challenges
- gon.push(u2f: { challenges: challenges, app_id: u2f_app_id,
+ session[:challenge] ||= u2f.challenge
+ gon.push(u2f: { challenge: session[:challenge], app_id: u2f_app_id,
sign_requests: sign_requests })
end
end
diff --git a/spec/javascripts/u2f/authenticate_spec.coffee b/spec/javascripts/u2f/authenticate_spec.coffee
index e8a2892d678..8ffeda11704 100644
--- a/spec/javascripts/u2f/authenticate_spec.coffee
+++ b/spec/javascripts/u2f/authenticate_spec.coffee
@@ -5,13 +5,12 @@
#= require ./mock_u2f_device
describe 'U2FAuthenticate', ->
- U2FUtil.enableTestMode()
fixture.load('u2f/authenticate')
beforeEach ->
@u2fDevice = new MockU2FDevice
@container = $("#js-authenticate-u2f")
- @component = new U2FAuthenticate(@container, {}, "token")
+ @component = new U2FAuthenticate(@container, {sign_requests: []}, "token")
@component.start()
it 'allows authenticating via a U2F device', ->
diff --git a/spec/javascripts/u2f/register_spec.js.coffee b/spec/javascripts/u2f/register_spec.js.coffee
index 0858abeca1a..87dc769792b 100644
--- a/spec/javascripts/u2f/register_spec.js.coffee
+++ b/spec/javascripts/u2f/register_spec.js.coffee
@@ -5,7 +5,6 @@
#= require ./mock_u2f_device
describe 'U2FRegister', ->
- U2FUtil.enableTestMode()
fixture.load('u2f/register')
beforeEach ->
diff --git a/spec/support/fake_u2f_device.rb b/spec/support/fake_u2f_device.rb
index 553fe9f1fbc..f550e9a0160 100644
--- a/spec/support/fake_u2f_device.rb
+++ b/spec/support/fake_u2f_device.rb
@@ -18,8 +18,8 @@ class FakeU2fDevice
def respond_to_u2f_authentication
app_id = @page.evaluate_script('gon.u2f.app_id')
- challenges = @page.evaluate_script('gon.u2f.challenges')
- json_response = u2f_device(app_id).sign_response(challenges[0])
+ challenge = @page.evaluate_script('gon.u2f.challenge')
+ json_response = u2f_device(app_id).sign_response(challenge)
@page.execute_script("
u2f.sign = function(appId, challenges, signRequests, callback) {
--
GitLab