From cf5940074f3e431c369e66dce94bcc528c8476f2 Mon Sep 17 00:00:00 2001 From: Alex Coomans Date: Mon, 3 Aug 2020 22:48:07 -0500 Subject: [PATCH] Remove ca_file, require_cert, and truststore options to X509 middleware --- CHANGES.md | 7 ++++ lib/rails/auth/x509/middleware.rb | 36 ++++---------------- spec/rails/auth/x509/middleware_spec.rb | 44 +++++-------------------- spec/support/create_certs.rb | 17 ---------- 4 files changed, 23 insertions(+), 81 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 01ee622..0f4644b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,10 @@ +### Unreleased + +* [#67](https://github.com/square/rails-auth/pull/67) + Remove `ca_file`, `require_cert`, and `truststore` options to X509 middleware + as we no longer verify the certificate chain. + ([@drcapulet]) + ### 2.2.2 (2020-07-02) * [#65](https://github.com/square/rails-auth/pull/65) diff --git a/lib/rails/auth/x509/middleware.rb b/lib/rails/auth/x509/middleware.rb index c6630c0..ec1d0a1 100644 --- a/lib/rails/auth/x509/middleware.rb +++ b/lib/rails/auth/x509/middleware.rb @@ -3,30 +3,20 @@ module Rails module Auth module X509 - # Raised when certificate verification is mandatory - CertificateVerifyFailed = Class.new(NotAuthorizedError) - - # Validates X.509 client certificates and adds credential objects for valid - # clients to the rack environment as env["rails-auth.credentials"]["x509"] + # Extracts X.509 client certificates and adds credential objects to the + # rack environment as env["rails-auth.credentials"]["x509"] class Middleware # Create a new X.509 Middleware object # - # @param [Object] app next app in the Rack middleware chain - # @param [String] ca_file path to the CA bundle to verify client certs with - # @param [Hash] cert_filters maps Rack environment names to cert extractors - # @param [Logger] logger place to log verification successes & failures - # @param [Boolean] require_cert causes middleware to raise if certs are unverified - # @param [OpenSSL::X509::Store] truststore (optional) provide your own truststore (for e.g. CRLs) + # @param [Object] app next app in the Rack middleware chain + # @param [Hash] cert_filters maps Rack environment names to cert extractors + # @param [Logger] logger place to log certificate extraction issues # # @return [Rails::Auth::X509::Middleware] new X509 middleware instance - def initialize(app, ca_file: nil, cert_filters: {}, logger: nil, require_cert: false, truststore: nil) - raise ArgumentError, "no ca_file or truststore given" unless ca_file || truststore - + def initialize(app, cert_filters: {}, logger: nil) @app = app @cert_filters = cert_filters @logger = logger - @require_cert = require_cert - @truststore = truststore || OpenSSL::X509::Store.new.add_file(ca_file) @cert_filters.each do |key, filter| next unless filter.is_a?(Symbol) @@ -53,17 +43,9 @@ def extract_credential(env) cert = extract_certificate_with_filter(filter, env[key]) next unless cert - if @truststore.verify(cert) - log("Verified", cert) - return Rails::Auth::X509::Certificate.new(cert) - else - log("Verify FAILED", cert) - raise CertificateVerifyFailed, "verify failed: #{subject(cert)}" if @require_cert - end + return Rails::Auth::X509::Certificate.new(cert) end - raise CertificateVerifyFailed, "no client certificate in request" if @require_cert - nil end @@ -79,10 +61,6 @@ def extract_certificate_with_filter(filter, raw_cert) nil end - def log(message, cert) - @logger.debug("rails-auth: #{message} (#{subject(cert)})") if @logger - end - def subject(cert) cert.subject.to_a.map { |attr, data| "#{attr}=#{data}" }.join(",") end diff --git a/spec/rails/auth/x509/middleware_spec.rb b/spec/rails/auth/x509/middleware_spec.rb index 3b619e3..16614e4 100644 --- a/spec/rails/auth/x509/middleware_spec.rb +++ b/spec/rails/auth/x509/middleware_spec.rb @@ -3,41 +3,32 @@ require "logger" RSpec.describe Rails::Auth::X509::Middleware do - let(:request) { Rack::MockRequest.env_for("https://www.example.com") } let(:app) { ->(env) { [200, env, "Hello, world!"] } } + let(:request) { Rack::MockRequest.env_for("https://www.example.com") } - let(:valid_cert_pem) { cert_path("valid.crt").read } - let(:bad_cert_pem) { cert_path("invalid.crt").read } - let(:cert_required) { false } - let(:cert_filter) { :pem } - let(:example_key) { "X-SSL-Client-Cert" } + let(:cert_filter) { :pem } + let(:cert_pem) { cert_path("valid.crt").read } + let(:example_key) { "X-SSL-Client-Cert" } let(:middleware) do described_class.new( app, - logger: Logger.new(STDERR), - ca_file: cert_path("ca.crt").to_s, cert_filters: { example_key => cert_filter }, - require_cert: cert_required + logger: Logger.new(STDERR) ) end context "certificate types" do describe "PEM certificates" do it "extracts Rails::Auth::X509::Certificate from a PEM certificate in the Rack environment" do - _response, env = middleware.call(request.merge(example_key => valid_cert_pem)) + _response, env = middleware.call(request.merge(example_key => cert_pem)) credential = Rails::Auth.credentials(env).fetch("x509") expect(credential).to be_a Rails::Auth::X509::Certificate end - it "ignores unverified certificates" do - _response, env = middleware.call(request.merge(example_key => bad_cert_pem)) - expect(Rails::Auth.credentials(env)).to be_empty - end - it "normalizes abnormal whitespace" do - _response, env = middleware.call(request.merge(example_key => valid_cert_pem.tr("\n", "\t"))) + _response, env = middleware.call(request.merge(example_key => cert_pem.tr("\n", "\t"))) credential = Rails::Auth.credentials(env).fetch("x509") expect(credential).to be_a Rails::Auth::X509::Certificate @@ -46,11 +37,11 @@ # :nocov: describe "Java certificates" do - let(:example_key) { "javax.servlet.request.X509Certificate" } let(:cert_filter) { :java } + let(:example_key) { "javax.servlet.request.X509Certificate" } let(:java_cert) do - ruby_cert = OpenSSL::X509::Certificate.new(valid_cert_pem) + ruby_cert = OpenSSL::X509::Certificate.new(cert_pem) input_stream = Java::JavaIO::ByteArrayInputStream.new(ruby_cert.to_der.to_java_bytes) java_cert_klass = Java::JavaSecurityCert::CertificateFactory.getInstance("X.509") java_cert_klass.generateCertificate(input_stream) @@ -67,21 +58,4 @@ end # :nocov: end - - describe "require_cert: true" do - let(:cert_required) { true } - - it "functions normally for valid certificates" do - _response, env = middleware.call(request.merge(example_key => valid_cert_pem)) - - credential = Rails::Auth.credentials(env).fetch("x509") - expect(credential).to be_a Rails::Auth::X509::Certificate - end - - it "raises Rails::Auth::X509::CertificateVerifyFailed for unverified certificates" do - expect do - middleware.call(request.merge(example_key => bad_cert_pem)) - end.to raise_error Rails::Auth::X509::CertificateVerifyFailed - end - end end diff --git a/spec/support/create_certs.rb b/spec/support/create_certs.rb index 15e896e..26f9428 100644 --- a/spec/support/create_certs.rb +++ b/spec/support/create_certs.rb @@ -95,20 +95,3 @@ File.write valid_cert_with_ext_path, valid_cert_with_ext.to_pem File.write valid_key_with_ext_path, valid_cert_with_ext.key_material.private_key.to_pem - -# -# Create evil MitM self-signed certificate -# - -self_signed_cert = CertificateAuthority::Certificate.new -self_signed_cert.subject.common_name = "127.0.0.1" -self_signed_cert.subject.organizational_unit = "ponycopter" -self_signed_cert.serial_number.number = 2 -self_signed_cert.key_material.generate_key -self_signed_cert.sign! - -self_signed_cert_path = File.join(cert_path, "invalid.crt") -self_signed_key_path = File.join(cert_path, "invalid.key") - -File.write self_signed_cert_path, self_signed_cert.to_pem -File.write self_signed_key_path, self_signed_cert.key_material.private_key.to_pem