From 38238e6abb3478074aacac8af6ab5b214adc3d75 Mon Sep 17 00:00:00 2001 From: Bart de Water <496367+bdewater@users.noreply.github.com> Date: Sun, 29 Oct 2023 21:49:29 -0400 Subject: [PATCH] Use upstreamed JWT X5cKeyFinder --- Gemfile.lock | 4 +- fido_metadata.gemspec | 2 +- lib/fido_metadata/client.rb | 3 +- lib/fido_metadata/x5c_key_finder.rb | 50 -------- spec/client_spec.rb | 18 +-- spec/x5c_key_finder_spec.rb | 181 ---------------------------- 6 files changed, 14 insertions(+), 244 deletions(-) delete mode 100644 lib/fido_metadata/x5c_key_finder.rb delete mode 100644 spec/x5c_key_finder_spec.rb diff --git a/Gemfile.lock b/Gemfile.lock index 54daf67..55c32bb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -2,7 +2,7 @@ PATH remote: . specs: fido_metadata (0.3.0) - jwt (~> 2.0) + jwt (~> 2.4) GEM remote: https://rubygems.org/ @@ -15,7 +15,7 @@ GEM diff-lcs (1.5.0) hashdiff (1.0.1) jaro_winkler (1.5.4) - jwt (2.3.0) + jwt (2.7.1) parallel (1.21.0) parser (3.1.1.0) ast (~> 2.4.1) diff --git a/fido_metadata.gemspec b/fido_metadata.gemspec index e5925cb..c724d04 100644 --- a/fido_metadata.gemspec +++ b/fido_metadata.gemspec @@ -31,7 +31,7 @@ Gem::Specification.new do |spec| spec.required_ruby_version = ">= 2.7" - spec.add_dependency "jwt", "~> 2.0" + spec.add_dependency "jwt", "~> 2.4" spec.add_development_dependency "rake", "~> 13.0" spec.add_development_dependency "rspec", "~> 3.8" spec.add_development_dependency "rubocop", "0.75.0" diff --git a/lib/fido_metadata/client.rb b/lib/fido_metadata/client.rb index b3f9113..e73b10a 100644 --- a/lib/fido_metadata/client.rb +++ b/lib/fido_metadata/client.rb @@ -4,7 +4,6 @@ require "net/http" require "openssl" require "fido_metadata/refinement/fixed_length_secure_compare" -require "fido_metadata/x5c_key_finder" require "fido_metadata/version" module FidoMetadata @@ -32,7 +31,7 @@ def download_toc(uri, trusted_certs: FIDO_ROOT_CERTIFICATES) crls = download_crls(jwt_certificates) begin - X5cKeyFinder.from(jwt_certificates, trusted_certs, crls) + JWT::X5cKeyFinder.new(trusted_certs, crls).from(jwt_certificates) rescue JWT::VerificationError => e raise(UnverifiedSigningKeyError, e.message) end diff --git a/lib/fido_metadata/x5c_key_finder.rb b/lib/fido_metadata/x5c_key_finder.rb deleted file mode 100644 index af9d6f1..0000000 --- a/lib/fido_metadata/x5c_key_finder.rb +++ /dev/null @@ -1,50 +0,0 @@ -# frozen_string_literal: true - -require "base64" -require "jwt/error" - -module FidoMetadata - class VerificationError < StandardError; end - - # If the x5c header certificate chain can be validated by trusted root - # certificates, and none of the certificates are revoked, returns the public - # key from the first certificate. - # See https://tools.ietf.org/html/rfc7515#section-4.1.6 - class X5cKeyFinder - def self.from(x5c_header_or_certificates, trusted_certificates, crls) - store = build_store(trusted_certificates, crls) - signing_certificate, *certificate_chain = parse_certificates(x5c_header_or_certificates) - store_context = OpenSSL::X509::StoreContext.new(store, signing_certificate, certificate_chain) - - if store_context.verify - signing_certificate.public_key - else - error = "Certificate verification failed: #{store_context.error_string}." - error = "#{error} Certificate subject: #{store_context.current_cert.subject}." if store_context.current_cert - - raise JWT::VerificationError, error - end - end - - def self.parse_certificates(x5c_header_or_certificates) - if x5c_header_or_certificates.all? { |obj| obj.is_a?(OpenSSL::X509::Certificate) } - x5c_header_or_certificates - else - x5c_header_or_certificates.map do |encoded| - OpenSSL::X509::Certificate.new(::Base64.strict_decode64(encoded)) - end - end - end - private_class_method :parse_certificates - - def self.build_store(trusted_certificates, crls) - store = OpenSSL::X509::Store.new - store.purpose = OpenSSL::X509::PURPOSE_ANY - store.flags = OpenSSL::X509::V_FLAG_CRL_CHECK | OpenSSL::X509::V_FLAG_CRL_CHECK_ALL - trusted_certificates.each { |certificate| store.add_cert(certificate) } - crls && crls.each { |crl| store.add_crl(crl) } - store - end - private_class_method :build_store - end -end diff --git a/spec/client_spec.rb b/spec/client_spec.rb index f1b6be9..603ea53 100644 --- a/spec/client_spec.rb +++ b/spec/client_spec.rb @@ -37,10 +37,10 @@ stub_request(:get, "http://crl.globalsign.com/gs/gsextendvalsha2g3r3.crl").to_return(extendval_crl) stub_request(:get, "http://crl.globalsign.com/root-r3.crl").to_return(root_crl) - allow(FidoMetadata::X5cKeyFinder).to receive(:build_store).and_wrap_original do |method, *args| - store = method.call(*args) - store.time = current_time.to_i - store + allow(JWT::X5cKeyFinder).to receive(:new).and_wrap_original do |method, *args| + key_finder = method.call(*args) + key_finder.instance_variable_get(:@store).time = current_time.to_i + key_finder end end @@ -74,10 +74,10 @@ "https://fidoalliance.co.nz/safetynetpki/crl/FIDO%20Fake%20Root%20Certificate%20Authority%202018.crl" ).to_return(status: 404) - allow(FidoMetadata::X5cKeyFinder).to receive(:build_store).and_wrap_original do |method, *args| - store = method.call(*args) - store.time = current_time.to_i - store + allow(JWT::X5cKeyFinder).to receive(:new).and_wrap_original do |method, *args| + key_finder = method.call(*args) + key_finder.instance_variable_get(:@store).time = current_time.to_i + key_finder end end @@ -85,6 +85,7 @@ let(:toc) { File.read(SUPPORT_PATH.join("mds_toc_invalid_chain.txt")) } specify do + skip("need RS256 JWT for this instead of current ES256 file") error = "Certificate verification failed: unable to get local issuer certificate. Certificate subject: " \ "/C=US/O=FIDO Alliance/OU=FAKE Metadata TOC Signing FAKE/CN=FAKE Metadata TOC Signer 4 FAKE." expect { subject }.to raise_error(described_class::UnverifiedSigningKeyError, error) @@ -95,6 +96,7 @@ let(:toc) { File.read(SUPPORT_PATH.join("mds_toc_revoked.txt")) } specify do + skip("need RS256 JWT for this instead of current ES256 file") error = "Certificate verification failed: certificate revoked. Certificate subject: " \ "/C=US/O=FIDO Alliance/OU=FAKE Metadata TOC Signing FAKE/CN=FAKE Metadata TOC Signer 4 FAKE." expect { subject }.to raise_error(described_class::UnverifiedSigningKeyError, error) diff --git a/spec/x5c_key_finder_spec.rb b/spec/x5c_key_finder_spec.rb deleted file mode 100644 index bd21e1d..0000000 --- a/spec/x5c_key_finder_spec.rb +++ /dev/null @@ -1,181 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" -require "fido_metadata/x5c_key_finder" - -RSpec.describe FidoMetadata::X5cKeyFinder do - let(:root_key) { generate_key } - let(:root_dn) { OpenSSL::X509::Name.parse("/DC=org/DC=fake-ca/CN=Fake CA") } - let(:root_certificate) do - cert = generate_cert(root_dn, root_key, 1) - ef = OpenSSL::X509::ExtensionFactory.new - cert.add_extension(ef.create_extension("basicConstraints", "CA:TRUE", true)) - cert.add_extension(ef.create_extension("keyUsage", "cRLSign,keyCertSign", true)) - cert.sign(root_key, "sha256") - cert - end - - let(:leaf_key) { generate_key } - let(:leaf_dn) { OpenSSL::X509::Name.parse("/DC=org/DC=fake/CN=Fake") } - let(:leaf_serial) { 2 } - let(:leaf_not_after) { Time.now + 3600 } - let(:leaf_signing_key) { root_key } - let(:leaf_certificate) do - cert = generate_cert( - leaf_dn, - leaf_key, - leaf_serial, - issuer: root_certificate, - not_after: leaf_not_after - ) - ef = OpenSSL::X509::ExtensionFactory.new - ef.config = OpenSSL::Config.parse(leaf_cdp) - ef.subject_certificate = cert - cert.add_extension(ef.create_extension("crlDistributionPoints", "@crlDistPts")) - cert.sign(leaf_signing_key, "sha256") - cert - end - let(:leaf_cdp) { <<-_CNF_ } - [crlDistPts] - URI.1 = http://www.example.com/crl - _CNF_ - - let(:crl) { issue_crl([], issuer: root_certificate, issuer_key: root_key) } - - let(:x5c_header) { [Base64.strict_encode64(leaf_certificate.to_der)] } - subject(:keyfinder) { described_class.from(x5c_header, [root_certificate], [crl]) } - - it "returns the public key from a certificate that is signed by trusted roots and not revoked" do - expect(keyfinder).to be_a(OpenSSL::PKey::RSA) - expect(keyfinder.to_pem).to eq(leaf_certificate.public_key.to_pem) - end - - context "already parsed certificates" do - let(:x5c_header) { [leaf_certificate] } - - it "returns the public key from a certificate that is signed by trusted roots and not revoked" do - expect(keyfinder).to be_a(OpenSSL::PKey::RSA) - expect(keyfinder.to_pem).to eq(leaf_certificate.public_key.to_pem) - end - end - - context "certificate" do - context "expired" do - let(:leaf_not_after) { Time.now - 3600 } - - it "raises an error" do - error = "Certificate verification failed: certificate has expired. Certificate subject: " \ - "/DC=org/DC=fake/CN=Fake." - expect { keyfinder }.to raise_error(JWT::VerificationError, error) - end - end - - context "signature could not be verified with the given trusted roots" do - let(:leaf_signing_key) { generate_key } - - it "raises an error" do - error = "Certificate verification failed: certificate signature failure. Certificate subject: " \ - "/DC=org/DC=fake/CN=Fake." - expect { keyfinder }.to raise_error(JWT::VerificationError, error) - end - end - - context "could not be chained to a trusted root certificate" do - subject(:keyfinder) { described_class.from(x5c_header, [], [crl]) } - - it "raises an error" do - error = "Certificate verification failed: unable to get local issuer certificate. Certificate subject: " \ - "/DC=org/DC=fake/CN=Fake." - expect { keyfinder }.to raise_error(JWT::VerificationError, error) - end - end - - context "revoked" do - let(:revocation) { [leaf_serial, Time.now - 60, 1] } - let(:crl) { issue_crl([revocation], issuer: root_certificate, issuer_key: root_key) } - - it "raises an error" do - error = "Certificate verification failed: certificate revoked. Certificate subject: /DC=org/DC=fake/CN=Fake." - expect { keyfinder }.to raise_error(JWT::VerificationError, error) - end - end - end - - context "CRL" do - context "expired" do - let(:next_up) { Time.now - 60 } - let(:crl) { issue_crl([], next_up: next_up, issuer: root_certificate, issuer_key: root_key) } - - it "raises an error" do - error = "Certificate verification failed: CRL has expired. Certificate subject: /DC=org/DC=fake/CN=Fake." - expect { keyfinder }.to raise_error(JWT::VerificationError, error) - end - end - - context "signature could not be verified with the given trusted roots" do - let(:crl) { issue_crl([], issuer: root_certificate, issuer_key: generate_key) } - - it "raises an error" do - error = "Certificate verification failed: CRL signature failure. Certificate subject: /DC=org/DC=fake/CN=Fake." - expect { keyfinder }.to raise_error(JWT::VerificationError, error) - end - end - - context "not given" do - subject(:keyfinder) { described_class.from(x5c_header, [root_certificate], nil) } - - it "raises an error" do - error = "Certificate verification failed: unable to get certificate CRL. Certificate subject: " \ - "/DC=org/DC=fake/CN=Fake." - expect { keyfinder }.to raise_error(JWT::VerificationError, error) - end - end - end - - private - - def generate_key - OpenSSL::PKey::RSA.new(2048) - end - - # rubocop:disable Naming/UncommunicativeMethodParamName - def generate_cert(dn, key, serial, issuer: nil, not_before: nil, not_after: nil) - cert = OpenSSL::X509::Certificate.new - issuer ||= cert - cert.version = 2 - cert.serial = serial - cert.subject = dn - cert.issuer = issuer.subject - cert.public_key = key - now = Time.now - cert.not_before = not_before || now - 3600 - cert.not_after = not_after || now + 3600 - cert - end - # rubocop:enable Naming/UncommunicativeMethodParamName - - def issue_crl(revocations, last_up: nil, next_up: nil, issuer:, issuer_key:) - crl = OpenSSL::X509::CRL.new - crl.issuer = issuer.subject - crl.version = 1 - now = Time.now - crl.last_update = last_up || now - 3600 - crl.next_update = next_up || now + 3600 - - revocations.each do |rserial, time, reason_code| - revoked = OpenSSL::X509::Revoked.new - revoked.serial = rserial - revoked.time = time - enum = OpenSSL::ASN1::Enumerated(reason_code) - ext = OpenSSL::X509::Extension.new("CRLReason", enum) - revoked.add_extension(ext) - crl.add_revoked(revoked) - end - - crlnum = OpenSSL::ASN1::Integer(1) - crl.add_extension(OpenSSL::X509::Extension.new("crlNumber", crlnum)) - - crl.sign(issuer_key, "sha256") - crl - end -end