Skip to content
This repository has been archived by the owner on Jan 4, 2021. It is now read-only.

Commit

Permalink
(#613) only check cert name matches identity when local signer is used
Browse files Browse the repository at this point in the history
  • Loading branch information
ripienaar committed Dec 3, 2019
1 parent 5aa8edd commit f491988
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 1 deletion.
11 changes: 10 additions & 1 deletion lib/mcollective/util/choria.rb
Original file line number Diff line number Diff line change
Expand Up @@ -411,13 +411,22 @@ def valid_certificate?(pubcert, name, log=true)

Log.debug("Verified certificate %s against CA %s" % [incoming.subject.to_s, incoming.issuer.to_s]) if log

unless OpenSSL::SSL.verify_certificate_identity(incoming, name)
if !remote_signer_configured? && !OpenSSL::SSL.verify_certificate_identity(incoming, name)
raise("Could not parse certificate with subject %s as it has no CN part, or name %s invalid" % [incoming.subject.to_s, name])
end

name
end

# Determines if a remote signer is configured
#
# @return [Boolean]
def remote_signer_configured?
url = get_option("choria.security.request_signer.url", nil)

![nil, ""].include?(url)
end

# Utility function to split a chained certificate String into an Array
#
# @param pemdata [String] PEM encoded certificate
Expand Down
34 changes: 34 additions & 0 deletions spec/unit/mcollective/util/choria_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,23 @@ module Util
choria.stubs(:ca_path).returns("spec/fixtures/ca_crt.pem")
expect(choria.valid_certificate?(File.read("spec/fixtures/rip.mcollective.pem"), "rip.mcollective")).to be_truthy
end

it "should check the identity when a remote signer is not used" do
choria.stubs(:ca_path).returns("spec/fixtures/ca_crt.pem")
expect(choria.remote_signer_configured?).to be(false)
expect {
choria.valid_certificate?(File.read("spec/fixtures/rip.mcollective.pem"), "other.mcollective")
}.to raise_error("Could not parse certificate with subject /CN=rip.mcollective as it has no CN part, or name other.mcollective invalid")
expect(choria.valid_certificate?(File.read("spec/fixtures/rip.mcollective.pem"), "rip.mcollective")).to be_truthy
end

it "should not check identity when a remote signer is used" do
OpenSSL::SSL.expects(:verify_certificate_identity).never
Config.instance.stubs(:pluginconf).returns("choria.security.request_signer.url" => "http://foo")
choria.stubs(:ca_path).returns("spec/fixtures/ca_crt.pem")
expect(choria.remote_signer_configured?).to be(true)
choria.valid_certificate?(File.read("spec/fixtures/rip.mcollective.pem"), "other.mcollective")
end
end

describe "#valid_intermediate_certificate?" do
Expand Down Expand Up @@ -460,6 +477,23 @@ module Util
end
end

describe "#remote_signer_configured?" do
it "should detect when not configured or empty string" do
expect(choria.remote_signer_configured?).to be(false)
Config.instance.stubs(:pluginconf).returns(
"choria.security.request_signer.url" => "",
)
expect(choria.remote_signer_configured?).to be(false)
end

it "should detect when configured" do
Config.instance.stubs(:pluginconf).returns(
"choria.security.request_signer.url" => "http://foo",
)
expect(choria.remote_signer_configured?).to be(true)
end
end

describe "#federation_middleware_servers" do
it "should resolve correctly" do
choria.expects(:server_resolver).with(
Expand Down

0 comments on commit f491988

Please sign in to comment.