WIP: Upstream updates

[joe-herman]

Summary

• chore: merge upstream updates from https://github.com/castle/ruby-u2f/ into from_hash
• chore: migrate to GHA

Context

This repo was out of date from the upstream repo. This PR merges to the latest and switches from Travis to GHA. We still have some customizations in using from_hash.

After this PR is merged, we should consider merging into one of the release branches, or using GitHub Releases.

Net diffs

Generated with git diff upstream/master

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
new file mode 100644
index 0000000..6dda94c
--- /dev/null
+++ b/.github/workflows/ci.yml
@@ -0,0 +1,37 @@
+name: Tests
+
+on:
+  pull_request:
+    branches:
+
+  push:
+    branches:
+      - master
+
+jobs:
+  test:
+    name: Run tests on Ruby ${{ matrix.RUBY_VERSION }}
+    runs-on: ubuntu-20.04
+    strategy:
+      fail-fast: false
+      matrix:
+        RUBY_VERSION: [2.4, 2.5, 2.6]
+    steps:
+      - name: Check out code
+        uses: actions/checkout@v4
+
+      - name: Install Ruby
+        uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: ${{ matrix.RUBY_VERSION }}
+          bundler: 2.3.27
+          rubygems: 3.2.3
+
+      - name: Bundle install
+        run: bundle install
+
+      - name: Rubocop
+        run: bundle exec rake test
+
+      - name: Rspec
+        run: bundle exec rspec
diff --git a/.travis.yml b/.travis.yml
deleted file mode 100644
index 14d4699..0000000
--- a/.travis.yml
+++ /dev/null
@@ -1,47 +0,0 @@
-services:
-  - docker
-
-dist: trusty
-sudo: false
-cache: bundler
-
-git:
-  depth: false
-
-test: &test
-  stage: test
-  language: ruby
-  before_install:
-    - gem install bundler
-    - gem update --system
-
-jobs:
-  include:
-    - <<: *test
-      rvm: 2.6.5
-    - <<: *test
-      rvm: 2.5.7
-    - <<: *test
-      rvm: 2.4.8
-
-    - stage: coditsu
-      language: ruby
-      rvm: 2.6.3
-      before_install:
-        - gem update --system
-        - gem install bundler
-      before_script:
-        - docker create -v /sources --name sources alpine:3.4 /bin/true
-        - docker cp ./ sources:/sources
-      script: >
-        docker run
-        -e CODITSU_API_KEY
-        -e CODITSU_API_SECRET
-        -e CODITSU_REPOSITORY_ID
-        -e CODITSU_BUILD_BRANCH=$TRAVIS_BRANCH
-        --volumes-from sources
-        coditsu/build-runner:latest
-
-stages:
-  - test
-  - coditsu
diff --git a/lib/u2f/client_data.rb b/lib/u2f/client_data.rb
index 1626353..f2d0bcd 100644
--- a/lib/u2f/client_data.rb
+++ b/lib/u2f/client_data.rb
@@ -11,6 +11,18 @@ module U2F
     attr_accessor :typ, :challenge, :origin
     alias type typ
 
+    def initialize(typ, challenge, origin)
+      @typ = typ
+      @challenge = challenge
+      @origin = origin
+
+      %i(typ challenge origin).each do |sym|
+        val = send(sym)
+        next if val.is_a?(String)
+        fail AttestationDecodeError, "Invalid #{sym}"
+      end
+    end
+
     def registration?
       typ == REGISTRATION_TYP
     end
@@ -20,12 +32,14 @@ module U2F
     end
 
     def self.load_from_json(json)
-      client_data = ::JSON.parse(json)
-      new.tap do |instance|
-        instance.typ = client_data['typ']
-        instance.challenge = client_data['challenge']
-        instance.origin = client_data['origin']
-      end
+      from_hash(::JSON.parse(json))
+      rescue JSON::ParserError => e
+
+      raise AttestationDecodeError, "Invalid JSON: #{e.message}"
+    end
+
+    def self.from_hash(data)
+      new(data['typ'], data['challenge'], data['origin'])
     end
   end
 end
diff --git a/lib/u2f/register_response.rb b/lib/u2f/register_response.rb
index c29a960..ffcc533 100644
--- a/lib/u2f/register_response.rb
+++ b/lib/u2f/register_response.rb
@@ -6,7 +6,7 @@ module U2F
   # See chapter 4.3:
   # http://fidoalliance.org/specs/fido-u2f-raw-message-formats-v1.0-rd-20141008.pdf
   class RegisterResponse
-    attr_accessor :client_data, :client_data_json, :registration_data_raw
+    attr_reader :client_data, :client_data_json, :registration_data_raw
 
     PUBLIC_KEY_OFFSET = 1
     PUBLIC_KEY_LENGTH = 65
@@ -14,24 +14,39 @@ module U2F
     KEY_HANDLE_LENGTH_OFFSET = PUBLIC_KEY_OFFSET + PUBLIC_KEY_LENGTH
     KEY_HANDLE_OFFSET = KEY_HANDLE_LENGTH_OFFSET + KEY_HANDLE_LENGTH_LENGTH
 
+    def initialize(client_data_encoded, registration_data_encoded)
+      begin
+        unless client_data_encoded.is_a?(String)
+          fail AttestationDecodeError, 'Must be a string'
+        end
+        @client_data_json = ::U2F.urlsafe_decode64(client_data_encoded)
+        @client_data = ClientData.load_from_json(client_data_json)
+      rescue AttestationDecodeError => e
+        raise AttestationDecodeError, "Invalid clientData: #{e.message}"
+      end
+
+      unless registration_data_encoded.is_a?(String)
+        fail AttestationDecodeError, 'Invalid registrationData: Not a string'
+      end
+      @registration_data_raw = ::U2F.urlsafe_decode64(registration_data_encoded)
+    end
+
     def self.load_from_json(json)
       # TODO: validate
-      data = JSON.parse(json)
 
+      from_hash(::JSON.parse(json))
+    rescue JSON::ParserError => e
+      raise AttestationDecodeError, "Invalid JSON: #{e.message}"
+    end
+
+    def self.from_hash(data)
       raise RegistrationError, code: data['errorCode'] if data['errorCode']&.positive?
 
       if !data.key?('clientData') || !data.key?('registrationData')
         raise RegistrationError, message: 'Invalid JSON'
       end
 
-      new.tap do |instance|
-        instance.client_data_json =
-          ::U2F.urlsafe_decode64(data['clientData'])
-        instance.client_data =
-          ClientData.load_from_json(instance.client_data_json)
-        instance.registration_data_raw =
-          ::U2F.urlsafe_decode64(data['registrationData'])
-      end
+      new(data['clientData'], data['registrationData'])
     end
 
     ##
diff --git a/lib/u2f/sign_response.rb b/lib/u2f/sign_response.rb
index 0b4c490..3a80972 100644
--- a/lib/u2f/sign_response.rb
+++ b/lib/u2f/sign_response.rb
@@ -2,38 +2,53 @@
 
 module U2F
   class SignResponse
-    attr_accessor :client_data, :client_data_json, :key_handle, :signature_data
+    attr_reader :key_handle, :client_data_json, :client_data
+    attr_reader :signature_data_raw
 
-    def self.load_from_json(json)
-      data = ::JSON.parse(json)
-      if !data.key?('clientData') || !data.key?('keyHandle') ||
-         !data.key?('signatureData')
-        raise Error, 'Missing required data'
+    def initialize(key_handle, client_data_encoded, signature_data_encoded)
+      unless key_handle.is_a?(String)
+        fail AttestationDecodeError, 'Invalid keyHandle: Not a string'
+      end
+      @key_handle = key_handle
+
+      begin
+        unless client_data_encoded.is_a?(String)
+          fail AttestationDecodeError, 'Not a string'
+        end
+        @client_data_json = ::U2F.urlsafe_decode64(client_data_encoded)
+        @client_data = ClientData.load_from_json(client_data_json)
+      rescue AttestationDecodeError => e
+        raise AttestationDecodeError, "Invalid clientData: #{e.message}"
+      end
+
+      unless signature_data_encoded.is_a?(String)
+        fail AttestationDecodeError, 'Invalid signatureData: Not a string'
       end
+      @signature_data_raw = ::U2F.urlsafe_decode64(signature_data_encoded)
+    end
+
+    def self.load_from_json(json)
+      from_hash(::JSON.parse(json))
+    rescue JSON::ParserError => e
+      raise AttestationDecodeError, "Invalid JSON: #{e.message}"
+    end
 
-      instance = new
-      instance.client_data_json =
-        ::U2F.urlsafe_decode64(data['clientData'])
-      instance.client_data =
-        ClientData.load_from_json(instance.client_data_json)
-      instance.key_handle = data['keyHandle']
-      instance.signature_data =
-        ::U2F.urlsafe_decode64(data['signatureData'])
-      instance
+    def self.from_hash(data)
+      new(data['keyHandle'], data['clientData'], data['signatureData']) 
     end
 
     ##
     # Counter value that the U2F token increments every time it performs an
     # authentication operation
     def counter
-      signature_data.byteslice(1, 4).unpack('N').first
+      signature_data_raw.byteslice(1, 4).unpack('N').first
     end
 
     ##
     # signature is to be verified using the public key obtained during
     # registration.
     def signature
-      signature_data.byteslice(5..-1)
+      signature_data_raw.byteslice(5..-1) || ''
     end
 
     # Bit 0 being set to 1 indicates that the user is present. A different value
@@ -43,7 +58,7 @@ module U2F
     ##
     # If user presence was verified
     def user_present?
-      byte = signature_data.byteslice(0).unpack('C').first
+      byte = signature_data_raw.byteslice(0).unpack('C').first
       byte & USER_PRESENCE_MASK == 1
     end
 
@@ -53,7 +68,7 @@ module U2F
     def verify(app_id, public_key_pem)
       data = [
         ::U2F::DIGEST.digest(app_id),
-        signature_data.byteslice(0, 5),
+        signature_data_raw.byteslice(0, 5),
         ::U2F::DIGEST.digest(client_data_json)
       ].join
 
diff --git a/lib/u2f/u2f.rb b/lib/u2f/u2f.rb
index 60001a0..781ad08 100644
--- a/lib/u2f/u2f.rb
+++ b/lib/u2f/u2f.rb
@@ -46,15 +46,15 @@ module U2F
     #   - +CounterTooLowError+:: if there is a counter mismatch between the registered one and
     #     the one in the response
     #
-    def authenticate!(challenge, response, registration_public_key, registration_counter)
-      # TODO: check that it's the correct key_handle as well
-      raise NoMatchingRequestError unless challenge == response.client_data.challenge
+    def authenticate!(response, registration_public_key, registration_counter)
+      pem = U2F.public_key_pem(registration_public_key)
 
-      raise ClientDataTypeError unless response.client_data.authentication?
+      fail AuthenticationFailedError unless response.verify(app_id, pem)
+      challenge = yield response.client_data.challenge if block_given?
 
-      pem = U2F.public_key_pem(registration_public_key)
+      raise NoMatchingRequestError unless challenge == response.client_data.challenge
 
-      raise AuthenticationFailedError unless response.verify(app_id, pem)
+      raise ClientDataTypeError unless response.client_data.authentication?
 
       raise UserNotPresentError unless response.user_present?
 
@@ -100,21 +100,20 @@ module U2F
     #   - +ClientDataTypeError+:: if the response is of the wrong type
     #   - +AttestationSignatureError+:: if the registration failed
     #
-    def register!(challenges, response)
-      challenges = [challenges] unless challenges.is_a? Array
-      challenge = challenges.detect do |chg|
-        chg == response.client_data.challenge
-      end
-
-      raise UnmatchedChallengeError unless challenge
-
-      raise ClientDataTypeError unless response.client_data.registration?
-
+    def register!(response)
       # Validate public key
       U2F.public_key_pem(response.public_key_raw)
 
       raise AttestationSignatureError unless response.verify(app_id)
 
+      challenge = yield response.client_data.challenge if block_given?
+
+      unless challenge == response.client_data.challenge
+        fail UnmatchedChallengeError
+      end
+
+      fail ClientDataTypeError unless response.client_data.registration?
+
       Registration.new(
         response.key_handle,
         response.public_key,
diff --git a/spec/lib/client_data_spec.rb b/spec/lib/client_data_spec.rb
index 1631b6e..c72cc12 100644
--- a/spec/lib/client_data_spec.rb
+++ b/spec/lib/client_data_spec.rb
@@ -7,11 +7,7 @@ describe U2F::ClientData do
   let(:registration_type) { U2F::ClientData::REGISTRATION_TYP }
   let(:authentication_type) { U2F::ClientData::AUTHENTICATION_TYP }
 
-  let(:client_data) do
-    described_class.new.tap do |cd|
-      cd.typ = type
-    end
-  end
+  let(:client_data) { U2F::ClientData.new(type, '', '') }
 
   describe '#registration?' do
     subject { client_data.registration? }
diff --git a/spec/lib/register_response_spec.rb b/spec/lib/register_response_spec.rb
index c3bdd5c..4d01fa6 100644
--- a/spec/lib/register_response_spec.rb
+++ b/spec/lib/register_response_spec.rb
@@ -15,88 +15,92 @@ describe U2F::RegisterResponse do
   end
   let(:error_response) { device.register_response(challenge, true) }
   let(:registration_request) { U2F::RegisterRequest.new(challenge) }
-  let(:register_response) do
-    described_class.load_from_json(registration_data_json)
-  end
-
-  context 'with error response' do
-    let(:registration_data_json) { error_response }
 
-    it 'raises RegistrationError with code' do
-      expect { register_response }.to raise_error(U2F::RegistrationError) do |error|
-        expect(error.code).to eq(4)
+  shared_examples 'register response examples' do
+    context 'with error response' do
+      let(:registration_data_json) { error_response }
+      it 'raises RegistrationError with code' do
+        expect {
+          register_response
+        }.to raise_error(U2F::RegistrationError) do |error|
+          expect(error.code).to eq(4)
+        end
       end
     end
-  end
-
-  context 'with invalid response' do
-    let(:registration_data_json) { '{}' }
 
-    it 'raises RegistrationError with code' do
-      expect { register_response }.to raise_error(U2F::RegistrationError) do |error|
-        expect(error.message).to eq('Invalid JSON')
+    context 'with unpadded response' do
+      let(:registration_data_json) { registration_data_json_without_padding }
+      it 'does not raise "invalid base64" exception' do
+        expect {
+          register_response
+        }.not_to raise_error
       end
     end
-  end
-
-  context 'with unpadded response' do
-    let(:registration_data_json) { registration_data_json_without_padding }
 
-    it 'does not raise "invalid base64" exception' do
-      expect { register_response }.not_to raise_error
+    describe '#certificate' do
+      subject { register_response.certificate }
+      it { is_expected.to eq certificate }
     end
-  end
-
-  describe '#certificate' do
-    subject { register_response.certificate }
 
-    it { is_expected.to eq certificate }
-  end
-
-  describe '#client_data' do
-    context 'when challenge' do
-      subject { register_response.client_data.challenge }
-
-      it { is_expected.to eq challenge }
+    describe '#client_data' do
+      context 'challenge' do
+        subject { register_response.client_data.challenge }
+        it { is_expected.to eq challenge }
+      end
     end
-  end
 
-  describe '#key_handle' do
-    subject { register_response.key_handle }
+    describe '#key_handle' do
+      subject { register_response.key_handle }
+      it { is_expected.to eq key_handle }
+    end
 
-    it { is_expected.to eq key_handle }
-  end
+    describe '#key_handle_length' do
+      subject { register_response.key_handle_length }
+      it { is_expected.to eq U2F.urlsafe_decode64(key_handle).length }
+    end
 
-  describe '#key_handle_length' do
-    subject { register_response.key_handle_length }
+    describe '#public_key' do
+      subject { register_response.public_key }
+      it { is_expected.to eq public_key }
+    end
 
-    it { is_expected.to eq U2F.urlsafe_decode64(key_handle).length }
-  end
+    describe '#verify' do
+      subject { register_response.verify(app_id) }
+      it { is_expected.to be_truthy }
+    end
 
-  describe '#public_key' do
-    subject { register_response.public_key }
+    describe '#verify with wrong app_id' do
+      subject { register_response.verify("other app") }
+      it { is_expected.to be_falsey }
+    end
 
-    it { is_expected.to eq public_key }
+    describe '#verify with corrupted signature' do
+      subject { register_response }
+      it "returns falsey" do
+        allow(subject).to receive(:signature).and_return("bad signature")
+        expect(subject.verify(app_id)).to be_falsey
+      end
+    end
   end
 
-  describe '#verify' do
-    subject { register_response.verify(app_id) }
-
-    it { is_expected.to be_truthy }
+  context 'instantiated via #load_from_json' do
+    let(:register_response) do
+      U2F::RegisterResponse.load_from_json(registration_data_json)
+    end
+    include_examples 'register response examples'
   end
 
-  describe '#verify with wrong app_id' do
-    subject { register_response.verify('other app') }
-
-    it { is_expected.to be_falsey }
+  context 'instantiated via #from_hash' do
+    let(:register_response) do
+      U2F::RegisterResponse.from_hash(JSON.parse registration_data_json)
+    end
+    include_examples 'register response examples'
   end
 
-  describe '#verify with corrupted signature' do
-    subject { register_response }
-
-    it 'returns falsey' do
-      allow(subject).to receive(:signature).and_return('bad signature')
-      expect(subject.verify(app_id)).to be_falsey
-    end
+  context 'instantiated via #from_hash' do
+    let(:register_response) do
+      U2F::RegisterResponse.from_hash(JSON.parse registration_data_json)
   end
+  include_examples 'register response examples'
+end
 end
diff --git a/spec/lib/sign_response_spec.rb b/spec/lib/sign_response_spec.rb
index 3f91f5f..08c8e30 100644
--- a/spec/lib/sign_response_spec.rb
+++ b/spec/lib/sign_response_spec.rb
@@ -7,48 +7,45 @@ describe U2F::SignResponse do
   let(:challenge) { U2F.urlsafe_encode64(SecureRandom.random_bytes(32)) }
   let(:device) { U2F::FakeU2F.new(app_id) }
   let(:json_response) { device.sign_response(challenge) }
-  let(:sign_response) { described_class.load_from_json json_response }
   let(:public_key_pem) { U2F::U2F.public_key_pem(device.origin_public_key_raw) }
 
-  context 'with invalid response' do
-    let(:json_response) { '{}' }
-
-    it do
-      expect { sign_response }.to raise_error(U2F::Error) do |error|
-        expect(error.message).to eq('Missing required data')
-      end
+  shared_examples 'sign response examples' do
+    describe '#counter' do
+      subject { sign_response.counter }
+      it { is_expected.to be device.counter }
     end
-  end
-
-  describe '#counter' do
-    subject { sign_response.counter }
 
-    it { is_expected.to be device.counter }
-  end
-
-  describe '#user_present?' do
-    subject { sign_response.user_present? }
+    describe '#user_present?' do
+      subject { sign_response.user_present? }
+      it { is_expected.to be true }
+    end
 
-    it { is_expected.to be true }
-  end
+    describe '#verify with correct app id' do
+      subject { sign_response.verify(app_id, public_key_pem) }
+      it { is_expected.to be_truthy}
+    end
 
-  describe '#verify with correct app id' do
-    subject { sign_response.verify(app_id, public_key_pem) }
+    describe '#verify with wrong app id' do
+      subject { sign_response.verify("other app", public_key_pem) }
+      it { is_expected.to be_falsey }
+    end
 
-    it { is_expected.to be_truthy }
+    describe '#verify with corrupted signature' do
+      subject { sign_response }
+      it "returns falsey" do
+        allow(subject).to receive(:signature).and_return("bad signature")
+        expect(subject.verify(app_id, public_key_pem)).to be_falsey
+      end
+    end
   end
 
-  describe '#verify with wrong app id' do
-    subject { sign_response.verify('other app', public_key_pem) }
-
-    it { is_expected.to be_falsey }
+  context 'instantiated via #load_from_json' do
+    let(:sign_response) { described_class.load_from_json json_response }
+    include_examples 'sign response examples'
   end
 
-  describe '#verify with corrupted signature' do
-    subject { sign_response.verify(app_id, public_key_pem) }
-
-    before { allow(sign_response).to receive(:signature).and_return('bad signature') }
-
-    it { is_expected.to be_falsey }
+  context 'instantiated via #from_hash' do
+    let(:sign_response) { described_class.from_hash(JSON.parse json_response) }
+    include_examples 'sign response examples'
   end
 end
diff --git a/spec/lib/u2f_spec.rb b/spec/lib/u2f_spec.rb
index 408d8d0..a57e951 100644
--- a/spec/lib/u2f_spec.rb
+++ b/spec/lib/u2f_spec.rb
@@ -39,7 +39,11 @@ describe U2F do
     let(:counter) { registration.counter }
     let(:reg_public_key) { registration.public_key }
     let(:u2f_authenticate) do
-      u2f.authenticate!(auth_challenge, sign_response, reg_public_key, counter)
+
+      u2f.authenticate!(sign_response, reg_public_key, counter) do |c|
+        expect(c).to eq(sign_response.client_data.challenge)
+        auth_challenge
+      end
     end
 
     context 'with correct parameters' do
@@ -83,18 +87,16 @@ describe U2F do
   end
 
   describe '#register!' do
-    context 'with correct registration data' do
-      it 'returns a registration' do
-        reg = nil
-        expect do
-          reg = u2f.register!(auth_challenge, register_response)
-        end.not_to raise_error
-        expect(reg.key_handle).to eq key_handle
+    def register
+      u2f.register!(register_response) do |c|
+        expect(c).to eq(register_response.client_data.challenge)
+        auth_challenge
       end
+    end
 
-      it 'accepts an array of challenges' do
-        reg = u2f.register!(['another-challenge', auth_challenge], register_response)
-        expect(reg).to be_a described_class::Registration
+    context 'with correct registration data' do
+      it 'returns a registration' do
+        expect(register.key_handle).to eq key_handle
       end
     end
 
@@ -102,9 +104,7 @@ describe U2F do
       let(:auth_challenge) { 'non-matching' }
 
       it 'raises an UnmatchedChallengeError' do
-        expect do
-          u2f.register!(auth_challenge, register_response)
-        end.to raise_error(described_class::UnmatchedChallengeError)
+        expect { register }.to raise_error(U2F::UnmatchedChallengeError)
       end
     end
   end

References

• https://github.com/castle/ruby-u2f