From 5fa5d929c6a9d57ba9375c62624d79ecccb3f9e1 Mon Sep 17 00:00:00 2001 From: Alexander Lais Date: Fri, 29 Sep 2023 16:29:03 +0200 Subject: [PATCH] feat: add check for client cert metadata rules at templating time --- Gemfile | 2 + Gemfile.lock | 2 + jobs/gorouter/spec | 4 ++ jobs/gorouter/templates/gorouter.yml.erb | 63 +++++++++++++++++- packages/gorouter/spec | 17 +++++ spec/gorouter_templates_spec.rb | 83 ++++++++++++++++++++++++ src/code.cloudfoundry.org/gorouter | 2 +- 7 files changed, 170 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index d78e34991..08a8a9eea 100644 --- a/Gemfile +++ b/Gemfile @@ -1,5 +1,7 @@ source 'https://rubygems.org' +gem 'openssl' + group :test do gem 'bosh-template' gem 'rspec' diff --git a/Gemfile.lock b/Gemfile.lock index 3f8d7ebde..228766c19 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -6,6 +6,7 @@ GEM semi_semantic (~> 1.2.0) diff-lcs (1.3) json (2.6.2) + openssl (3.2.0) parallel (1.22.1) parser (3.1.2.1) ast (~> 2.4.1) @@ -46,6 +47,7 @@ PLATFORMS DEPENDENCIES bosh-template + openssl rspec rubocop diff --git a/jobs/gorouter/spec b/jobs/gorouter/spec index dc4c3845a..4391a94f1 100644 --- a/jobs/gorouter/spec +++ b/jobs/gorouter/spec @@ -518,6 +518,10 @@ properties: description: "The number of file descriptors a router can have open at one time" default: 100000 + router.enable_verify_client_certificate_metadata: + description: | + Enable additional client certificate verification via verify_client_certificate_metadata (see below). + default: false router.verify_client_certificate_metadata: description: | Additional client certificate verification which limits the allowed client certificate for given to a signing CA (identified by its subject) to the certificates with subjects provided in the list of valid subjects. Each list entry contains a ca_subject with a coresponding list of valid subjects. diff --git a/jobs/gorouter/templates/gorouter.yml.erb b/jobs/gorouter/templates/gorouter.yml.erb index 5d6a40b71..5d546129d 100644 --- a/jobs/gorouter/templates/gorouter.yml.erb +++ b/jobs/gorouter/templates/gorouter.yml.erb @@ -1,4 +1,4 @@ ---- +<% require "openssl" %>--- <%= def property_or_link(description, property, link_path, link_name=nil, optional=false) link_name ||= link_path.split('.').first @@ -115,7 +115,6 @@ params = { 'send_http_start_stop_server_event' => p('router.send_http_start_stop_server_event'), 'send_http_start_stop_client_event' => p("router.send_http_start_stop_client_event"), 'empty_pool_timeout' => p('for_backwards_compatibility_only.empty_pool_timeout'), - 'verify_client_certificate_metadata'=> p('router.verify_client_certificate_metadata') } if_p('router.prometheus.port') do |port| @@ -423,7 +422,67 @@ if_p('router.html_error_template') do |t| params['html_error_template_file'] = t == '' ? nil : '/var/vcap/jobs/gorouter/config/error.html' end +# Verification check for client certificate metadata. Only enabled if client_ca_certs +if_p('router.enable_verify_client_certificate_metadata', 'router.verify_client_certificate_metadata', 'router.client_ca_certs') do |enable, rules, client_ca_certs| + if enable and rules.length > 0 then + # Check consistency between client_ca_certs and rules. + + # Find pems in `client_ca_certs`, raise an error if none are defined. + pems = client_ca_certs.scan(/(-----BEGIN CERTIFICATE-----.*?-----END CERTIFICATE-----)/m) + raise "client certificate rules defined, but no client CA defined in `client_ca_certs`" unless pems.length > 0 + + field_map = { + 'common_name' => 'CN', + 'serial_number' => 'SN', + 'organization' => 'O', + 'organization_name' => 'ON', + 'locality' => 'L', + 'country' => 'C', + 'province' => 'ST', + 'street_address' => 'STREET', + } + + # convert the ca_subject of each rule to a X.509 name with its fields sorted alphabetically. + rule_subjects = rules.map { |rule| + fields = [] + # convert properties to X.509 DN field names. Multi-value fields create a tuple for each entry. + rule['ca_subject'].each { |k, v| + mapping = field_map[k] + if v.kind_of?(Array) + v.each { |val| fields.push [ mapping, val] } + else + fields.push [ mapping, v ] + end + } + + # fields are sorted for the configuration and the subject name of the certificate. + sorted_fields = fields.sort{|a,b|a[0] <=> b[0]} + OpenSSL::X509::Name.new sorted_fields + } + + # Get the client CA certificates' subject names in the same alphabetical order as from the configuration. + cert_subjects = pems.map { |pem| + cert = OpenSSL::X509::Certificate.new pem[0] + sorted_fields = cert.subject.to_a.sort{|a,b|a[0] <=> b[0]} + OpenSSL::X509::Name.new sorted_fields + } + + # Check for each of the rules if there is _at least one_ client CA certificate with the same subject. + # Raise an error if there isn't and show which client CA subjects _are_ configured. + rule_subjects.each{ |rule| + unless [rule].intersect?(cert_subjects) then + raise <<~EOF + no CA certificate subjects in `client_ca_certs` matches the rule's subject: #{rule}. \ + `ca_client_certs` subjects: #{cert_subjects.map { |c| c.to_s }.join(", ")}" + EOF + end + } + # now that consistency is checked, assign the values. + params['enable_verify_client_certificate_metadata'] = enable + params['verify_client_certificate_metadata'] = rules + end +end params.to_yaml[3..-1] %> diff --git a/packages/gorouter/spec b/packages/gorouter/spec index 7feda5dbe..4cbb56aa1 100644 --- a/packages/gorouter/spec +++ b/packages/gorouter/spec @@ -75,6 +75,9 @@ files: - code.cloudfoundry.org/routing-api/trace/*.go # gosub - code.cloudfoundry.org/routing-api/uaaclient/*.go # gosub - code.cloudfoundry.org/vendor/code.cloudfoundry.org/tlsconfig/*.go # gosub + - code.cloudfoundry.org/vendor/filippo.io/edwards25519/*.go # gosub + - code.cloudfoundry.org/vendor/filippo.io/edwards25519/field/*.go # gosub + - code.cloudfoundry.org/vendor/filippo.io/edwards25519/field/*.s # gosub - code.cloudfoundry.org/vendor/github.com/armon/go-proxyproto/*.go # gosub - code.cloudfoundry.org/vendor/github.com/beorn7/perks/quantile/*.go # gosub - code.cloudfoundry.org/vendor/github.com/bmizerany/pat/*.go # gosub @@ -192,8 +195,19 @@ files: - code.cloudfoundry.org/vendor/github.com/vmihailenco/tagparser/v2/*.go # gosub - code.cloudfoundry.org/vendor/github.com/vmihailenco/tagparser/v2/internal/*.go # gosub - code.cloudfoundry.org/vendor/github.com/vmihailenco/tagparser/v2/internal/parser/*.go # gosub + - code.cloudfoundry.org/vendor/go.step.sm/crypto/fingerprint/*.go # gosub + - code.cloudfoundry.org/vendor/go.step.sm/crypto/internal/bcrypt_pbkdf/*.go # gosub + - code.cloudfoundry.org/vendor/go.step.sm/crypto/internal/emoji/*.go # gosub + - code.cloudfoundry.org/vendor/go.step.sm/crypto/internal/utils/*.go # gosub + - code.cloudfoundry.org/vendor/go.step.sm/crypto/keyutil/*.go # gosub + - code.cloudfoundry.org/vendor/go.step.sm/crypto/pemutil/*.go # gosub + - code.cloudfoundry.org/vendor/go.step.sm/crypto/randutil/*.go # gosub + - code.cloudfoundry.org/vendor/go.step.sm/crypto/x25519/*.go # gosub - code.cloudfoundry.org/vendor/golang.org/x/crypto/blake2b/*.go # gosub - code.cloudfoundry.org/vendor/golang.org/x/crypto/blake2b/*.s # gosub + - code.cloudfoundry.org/vendor/golang.org/x/crypto/blowfish/*.go # gosub + - code.cloudfoundry.org/vendor/golang.org/x/crypto/chacha20/*.go # gosub + - code.cloudfoundry.org/vendor/golang.org/x/crypto/chacha20/*.s # gosub - code.cloudfoundry.org/vendor/golang.org/x/crypto/curve25519/*.go # gosub - code.cloudfoundry.org/vendor/golang.org/x/crypto/ed25519/*.go # gosub - code.cloudfoundry.org/vendor/golang.org/x/crypto/internal/alias/*.go # gosub @@ -204,6 +218,9 @@ files: - code.cloudfoundry.org/vendor/golang.org/x/crypto/pbkdf2/*.go # gosub - code.cloudfoundry.org/vendor/golang.org/x/crypto/salsa20/salsa/*.go # gosub - code.cloudfoundry.org/vendor/golang.org/x/crypto/salsa20/salsa/*.s # gosub + - code.cloudfoundry.org/vendor/golang.org/x/crypto/scrypt/*.go # gosub + - code.cloudfoundry.org/vendor/golang.org/x/crypto/ssh/*.go # gosub + - code.cloudfoundry.org/vendor/golang.org/x/crypto/ssh/internal/bcrypt_pbkdf/*.go # gosub - code.cloudfoundry.org/vendor/golang.org/x/net/context/*.go # gosub - code.cloudfoundry.org/vendor/golang.org/x/net/html/*.go # gosub - code.cloudfoundry.org/vendor/golang.org/x/net/html/atom/*.go # gosub diff --git a/spec/gorouter_templates_spec.rb b/spec/gorouter_templates_spec.rb index 02df7d6e7..4754e802b 100644 --- a/spec/gorouter_templates_spec.rb +++ b/spec/gorouter_templates_spec.rb @@ -771,6 +771,89 @@ end end end + context 'verify_client_certificate_metadata' do + context 'not enabled but rules provided' do + before do + deployment_manifest_fragment['router']['verify_client_certificate_metadata'] = [ + { "ca_subject" => { "common_name" => "test.com" }} + ] + end + it 'does not populate the property' do + expect { parsed_yaml }.not_to raise_error + expect(parsed_yaml['enable_verify_client_certificate_metadata']).to eq(nil) + expect(parsed_yaml['verify_client_certificate_metadata']).to eq(nil) + end + end + + context 'enabled but no rules provided' do + before do + deployment_manifest_fragment['router']['enable_verify_client_certificate_metadata'] = false + deployment_manifest_fragment['router']['verify_client_certificate_metadata'] = [] + + end + it 'does not populate the property' do + expect { parsed_yaml }.not_to raise_error + expect(parsed_yaml['enable_verify_client_certificate_metadata']).to eq(nil) + expect(parsed_yaml['verify_client_certificate_metadata']).to eq(nil) + end + end + + context 'enabled without configured client_ca_certs' do + before do + deployment_manifest_fragment['router']['enable_verify_client_certificate_metadata'] = true + deployment_manifest_fragment['router']['verify_client_certificate_metadata'] = [ + { "ca_subject" => { "common_name" => "test-with-san.com" }, + "valid_subjects" => [ + {"ca_subject" => { "common_name" => "test.com client cert1" }}, + {"ca_subject" => { "common_name" => "test.com client cert2", "locality" => ["US"] }} + ] + } + ] + end + it 'fails generating the template as there are metadata verification rules but no client ca certs' do + expect { parsed_yaml }.to raise_error RuntimeError, "client certificate rules defined, but no client CA defined in `client_ca_certs`" + end + end + context 'enabled with configured client_ca_certs' do + before do + deployment_manifest_fragment['router']['client_ca_certs'] = TEST_CERT + end + context 'and matching rule' do + before do + deployment_manifest_fragment['router']['enable_verify_client_certificate_metadata'] = true + deployment_manifest_fragment['router']['verify_client_certificate_metadata'] = [ + { "ca_subject" => { "common_name" => "test-with-san.com" }, + "valid_subjects" => [ + {"ca_subject" => { "common_name" => "test.com client cert1" }}, + {"ca_subject" => { "common_name" => "test.com client cert2", "locality" => ["US"] }} + ] + } + ] + end + it 'populates the properties after a successful check' do + expect { parsed_yaml }.not_to raise_error + expect(parsed_yaml['enable_verify_client_certificate_metadata']).to eq(true) + expect(parsed_yaml['verify_client_certificate_metadata']).to eq(deployment_manifest_fragment['router']['verify_client_certificate_metadata']) + end + end + context 'and not matching rule' do + before do + deployment_manifest_fragment['router']['enable_verify_client_certificate_metadata'] = true + deployment_manifest_fragment['router']['verify_client_certificate_metadata'] = [ + { "ca_subject" => { "common_name" => "test-with-san.com", "country" => ["US"] }, + "valid_subjects" => [ + {"ca_subject" => { "common_name" => "test.com client cert1" }}, + {"ca_subject" => { "common_name" => "test.com client cert2", "locality" => ["US"] }} + ] + } + ] + end + it 'fails and explains the validpopulates the properties after a successful check' do + expect { parsed_yaml }.to raise_error RuntimeError, /no CA certificate subjects in `client_ca_certs` matches the rule's subject:/ + end + end + end + end end # ca_certs, private_key, cert_chain diff --git a/src/code.cloudfoundry.org/gorouter b/src/code.cloudfoundry.org/gorouter index ee4861e41..683ebaa7f 160000 --- a/src/code.cloudfoundry.org/gorouter +++ b/src/code.cloudfoundry.org/gorouter @@ -1 +1 @@ -Subproject commit ee4861e41bc94143433eb823bf83454c21facce8 +Subproject commit 683ebaa7f4466d4140ddd15b09fe1bfd1c217e67