Skip to content

Commit

Permalink
feat: add check for client cert metadata rules at templating time
Browse files Browse the repository at this point in the history
  • Loading branch information
peanball authored and ameowlia committed Oct 11, 2023
1 parent 355b595 commit 5fa5d92
Show file tree
Hide file tree
Showing 7 changed files with 170 additions and 3 deletions.
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
source 'https://rubygems.org'

gem 'openssl'

group :test do
gem 'bosh-template'
gem 'rspec'
Expand Down
2 changes: 2 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -46,6 +47,7 @@ PLATFORMS

DEPENDENCIES
bosh-template
openssl
rspec
rubocop

Expand Down
4 changes: 4 additions & 0 deletions jobs/gorouter/spec
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
63 changes: 61 additions & 2 deletions jobs/gorouter/templates/gorouter.yml.erb
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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|
Expand Down Expand Up @@ -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]
%>
17 changes: 17 additions & 0 deletions packages/gorouter/spec
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
83 changes: 83 additions & 0 deletions spec/gorouter_templates_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/code.cloudfoundry.org/gorouter

0 comments on commit 5fa5d92

Please sign in to comment.