(#7224) Fix error message ssl verification error message#907
(#7224) Fix error message ssl verification error message#907jeffmccune wants to merge 1 commit intopuppetlabs:2.7.xfrom
Conversation
|
specs don't pass for me: Finished in 0.39581 seconds |
|
@kelseyhightower What's your voltron? (read: what versions of the codependent codebases are you running?) They pass fine for me: $ rvm current ruby-1.9.3-p194@puppet $ rake voltron (in /workspace/codereview) * puppet jeffmccune-bug/2.7.x/7224_hostname_not_match_error 2.7.17-27-g372265a 372265a * facter 1.6.x 1.6.10-2-ga6519b5 a6519b5 * rspec-puppet master v0.1.3 eed11c7 * puppetlabs_spec_helper master 0.2.0-3-ge5cc86a e5cc86a * fog 0.7.2 v0.7.2 c7ec7c9 * rbvmomi master N/A 5dc0ca3 * stdlib master 2.3.3-25-gcc414a4 cc414a4 * mount_providers master 0.0.1 2d0dabe * pe_mcollective master 0.0.45-2-g64c6523 64c6523 * pe_accounts master 1.0.3 0725c16 * pe_compliance master 0.0.5 dddc001 * cloud_provisioner master 1.0.4-2-gf68dacd f68dacd * cloud_provisioner_vmware master v1.0.0-5-g6eaab64 6eaab64 * mocha gem 0.10.0 N/A * multi_json gem 1.3.6 N/A * rack gem 1.4.1 N/A * rspec gem 2.10.0 N/A * rspec-core gem 2.10.1 N/A * rspec-expectations gem 2.10.0 N/A * rspec-mocks gem 2.10.1 N/A |
|
@kelseyhightower Are you getting that error after you merge? It seems like someone added a bad error handler to the certificate name handling in 2.7.x. The certname I'm using in the fixture does not contain unprintable or non-ascii characters. =) |
|
kelseyhightower@dev:/workspace/puppet$ ruby --version *** LOCAL GEMS *** builder (3.0.0) |
|
This fails when merged on 2.7.x, things pass without this patch, fail with it. |
|
Yeah, I think someone else merged in a bad error handler. Note to self, I need to bisect this and fix the bad merge in the mainline. Kelsey, could you kick the to me in trello or Redmine? |
|
@jeffmccune sure! |
Without this patch the error message displayed when a CA chain is being
used is incorrect. The error looks like:
err: Could not retrieve catalog from remote server: Server hostname
'maynard' did not match server certificate; expected jeff mccune
root authority/c=us/st=oregon/l=portland/o=puppet labs/ou=jeff
mccune/emailaddress=jeff@puppetlabs.com
This is incorrect because the wrong certificate has been selected from
the chain of certificates. The SSL server certificate itself needs to
be selected, not a CA certificate.
This patch fixes the problem by only using the last certificate in the
chain to display the error message. The error now looks like:
err: Could not retrieve catalog from remote server: Server hostname
'maynard' did not match server certificate; expected one of
maynard2, DNS:maynard.puppetlabs.lan, DNS:maynard2, DNS:puppet,
DNS:puppet.puppetlabs.lan
The spec tests have been augmented to load certificate data from fixture
data. This should speed up the testing a bit and has the added benefit
of being able to test CA chains in addition to self signed CA
certificates.
To add additional test cases, simply drop new fixture data into
spec/fixtures/unit/indirector/rest/peer_cert_*.pem
The fixture file should be series of PEM encoded certificates separated
by "\n---\n" The CA's should be listed first and the master SSL
certificate listed last.
|
Sadly, this fails for me after manually merging the test code: ⚡ rspec spec/unit/indirector/rest_spec.rb
1) Puppet::Indirector::REST when making http requests when verification fails with a CA chain Certificate Chain: peer_cert_one_self_signed_ca.pem should provide a helpful error message when hostname was not match with server certificate
Failure/Error: expect { @searcher.http_request(:get, stub('request')) }.to(
expected Puppet::Error with message matching /Server hostname 'my_server' did not match server certificate; expected one of (.+)/, got #>
# ./spec/unit/indirector/rest_spec.rb:146:in `expect_helpful_error'
# ./spec/unit/indirector/rest_spec.rb:136
2) Puppet::Indirector::REST when making http requests when verification fails with a CA chain Certificate Chain: peer_cert_three_ca_chain.pem should provide a helpful error message when hostname was not match with server certificate
Failure/Error: expect { @searcher.http_request(:get, stub('request')) }.to(
expected Puppet::Error with message matching /Server hostname 'my_server' did not match server certificate; expected one of (.+)/, got #
# ./spec/unit/indirector/rest_spec.rb:146:in `expect_helpful_error'
# ./spec/unit/indirector/rest_spec.rb:136
101/101: 100% |==========================================| Time: 00:00:00
Finished in 0.75071 seconds
101 examples, 2 failures
|
|
I'm going to close this pull request. Next action: It needs to be rebased against master. -Jeff |
3 participants
Without this patch the error message displayed when a CA chain is being used
is incorrect. The error looks like:
This is incorrect because the wrong certificate has been selected from the
chain of certificates. The SSL server certificate itself needs to be
selected, not a CA certificate.
This patch fixes the problem by only using the last certificate in the chain
to display the error message. The error now looks like:
The spec tests have been augmented to load certificate data from fixture
data. This should speed up the testing a bit and has the added benefit of
being able to test CA chains in addition to self signed CA certificates.
To add additional test cases, simply drop new fixture data into
spec/fixtures/unit/indirector/rest/peer_cert_*.pem
The fixture file should be series of PEM encoded certificates separated by
"\n---\n" The CA's should be listed first and the master SSL certificate
listed last.