Skip to content

Commit 4fcf3d9

Browse files
authored
Merge pull request #7850 from joshcooper/create_system_context_10144
(PUP-10144) Add method for loading system ssl context
2 parents e4a4812 + a4c3a30 commit 4fcf3d9

File tree

3 files changed

+212
-0
lines changed

3 files changed

+212
-0
lines changed

lib/puppet/ssl/ssl_provider.rb

+20
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,26 @@ def create_root_context(cacerts:, crls: [], revocation: Puppet[:certificate_revo
3636
Puppet::SSL::SSLContext.new(store: store, cacerts: cacerts, crls: crls, revocation: revocation).freeze
3737
end
3838

39+
# Create an `SSLContext` using the trusted `cacerts` and any certs in OpenSSL's
40+
# default verify path locations. When running puppet as a gem, the location is
41+
# system dependent. When running puppet from puppet-agent packages, the location
42+
# refers to the cacerts bundle in the puppet-agent package.
43+
#
44+
# Connections made from the returned context will authenticate the server,
45+
# i.e. `VERIFY_PEER`, but will not use a client certificate and will not
46+
# perform revocation checking.
47+
#
48+
# @param cacerts [Array<OpenSSL::X509::Certificate>] Array of trusted CA certs
49+
# @return [Puppet::SSL::SSLContext] A context to use to create connections
50+
# @raise (see #create_context)
51+
# @api private
52+
def create_system_context(cacerts:)
53+
store = create_x509_store(cacerts, [], false)
54+
store.set_default_paths
55+
56+
Puppet::SSL::SSLContext.new(store: store, cacerts: cacerts, crls: [], revocation: false).freeze
57+
end
58+
3959
# Create an `SSLContext` using the trusted `cacerts`, `crls`, `private_key`,
4060
# `client_cert`, and `revocation` mode. Connections made from the returned
4161
# context will be mutually authenticated.

spec/integration/http/client_spec.rb

+121
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
require 'spec_helper'
2+
require 'puppet_spec/https'
3+
require 'puppet_spec/files'
4+
5+
describe Puppet::HTTP::Client, unless: Puppet::Util::Platform.jruby? do
6+
include PuppetSpec::Files
7+
8+
before :all do
9+
WebMock.disable!
10+
end
11+
12+
after :all do
13+
WebMock.enable!
14+
end
15+
16+
before :each do
17+
# make sure we don't take too long
18+
Puppet[:http_connect_timeout] = '5s'
19+
end
20+
21+
let(:hostname) { '127.0.0.1' }
22+
let(:wrong_hostname) { 'localhost' }
23+
let(:server) { PuppetSpec::HTTPSServer.new }
24+
let(:client) { Puppet::HTTP::Client.new }
25+
let(:ssl_provider) { Puppet::SSL::SSLProvider.new }
26+
let(:root_context) { ssl_provider.create_root_context(cacerts: [server.ca_cert], crls: [server.ca_crl]) }
27+
28+
context "when verifying an HTTPS server" do
29+
it "connects over SSL" do
30+
server.start_server do |port|
31+
res = client.get(URI("https://127.0.0.1:#{port}"), ssl_context: root_context)
32+
expect(res).to be_success
33+
end
34+
end
35+
36+
it "raises if the server's cert doesn't match the hostname we connected to" do
37+
server.start_server do |port|
38+
expect {
39+
client.get(URI("https://#{wrong_hostname}:#{port}"), ssl_context: root_context)
40+
}.to raise_error { |err|
41+
expect(err).to be_instance_of(Puppet::HTTP::ConnectionError)
42+
expect(err.message).to match(/Server hostname '#{wrong_hostname}' did not match server certificate; expected one of (.+)/)
43+
44+
md = err.message.match(/expected one of (.+)/)
45+
expect(md[1].split(', ')).to contain_exactly('127.0.0.1', 'DNS:127.0.0.1', 'DNS:127.0.0.2')
46+
}
47+
end
48+
end
49+
50+
it "raises if the server's CA is unknown" do
51+
wrong_ca = cert_fixture('netlock-arany-utf8.pem')
52+
alt_context = ssl_provider.create_root_context(cacerts: [wrong_ca], revocation: false)
53+
54+
server.start_server do |port|
55+
expect {
56+
client.get(URI("https://127.0.0.1:#{port}"), ssl_context: alt_context)
57+
}.to raise_error(Puppet::HTTP::ConnectionError,
58+
%r{certificate verify failed.* .self signed certificate in certificate chain for CN=Test CA.})
59+
end
60+
end
61+
end
62+
63+
context "with client certs" do
64+
let(:ctx_proc) {
65+
-> ctx {
66+
# configures the server to require the client to present a client cert
67+
ctx.verify_mode = OpenSSL::SSL::VERIFY_PEER | OpenSSL::SSL::VERIFY_FAIL_IF_NO_PEER_CERT
68+
}
69+
}
70+
71+
it "mutually authenticates the connection" do
72+
client_context = ssl_provider.create_context(
73+
cacerts: [server.ca_cert], crls: [server.ca_crl],
74+
client_cert: server.server_cert, private_key: server.server_key
75+
)
76+
77+
server.start_server(ctx_proc: ctx_proc) do |port|
78+
res = client.get(URI("https://127.0.0.1:#{port}"), ssl_context: client_context)
79+
expect(res).to be_success
80+
end
81+
end
82+
end
83+
84+
context "with a system trust store" do
85+
it "connects when the client trusts the server's CA" do
86+
system_context = ssl_provider.create_system_context(cacerts: [server.ca_cert])
87+
88+
server.start_server do |port|
89+
res = client.get(URI("https://127.0.0.1:#{port}"), ssl_context: system_context)
90+
expect(res).to be_success
91+
end
92+
end
93+
94+
it "connects when the server's CA is in the system store" do
95+
# create a temp cacert bundle
96+
ssl_file = tmpfile('systemstore')
97+
File.write(ssl_file, server.ca_cert)
98+
99+
# override path to system cacert bundle, this must be done before
100+
# the SSLContext is created and the call to X509::Store.set_default_paths
101+
Puppet::Util.withenv("SSL_CERT_FILE" => ssl_file) do
102+
system_context = ssl_provider.create_system_context(cacerts: [])
103+
server.start_server do |port|
104+
res = client.get(URI("https://127.0.0.1:#{port}"), ssl_context: system_context)
105+
expect(res).to be_success
106+
end
107+
end
108+
end
109+
110+
it "raises if the server's CA is not in the context or system store" do
111+
system_context = ssl_provider.create_system_context(cacerts: [cert_fixture('netlock-arany-utf8.pem')])
112+
113+
server.start_server do |port|
114+
expect {
115+
client.get(URI("https://127.0.0.1:#{port}"), ssl_context: system_context)
116+
}.to raise_error(Puppet::HTTP::ConnectionError,
117+
%r{certificate verify failed.* .self signed certificate in certificate chain for CN=Test CA.})
118+
end
119+
end
120+
end
121+
end

spec/unit/ssl/ssl_provider_spec.rb

+71
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,67 @@
7373
sslctx.verify_peer = false
7474
}.to raise_error(/can't modify frozen/)
7575
end
76+
77+
it 'verifies peer' do
78+
sslctx = subject.create_root_context(config)
79+
expect(sslctx.verify_peer).to eq(true)
80+
end
81+
end
82+
83+
context 'when creating a system ssl context' do
84+
it 'accepts empty list of CA certs' do
85+
sslctx = subject.create_system_context(cacerts: [])
86+
expect(sslctx.cacerts).to eq([])
87+
end
88+
89+
it 'accepts valid root certs' do
90+
certs = [cert_fixture('ca.pem')]
91+
sslctx = subject.create_system_context(cacerts: certs)
92+
expect(sslctx.cacerts).to eq(certs)
93+
end
94+
95+
it 'accepts valid intermediate certs' do
96+
certs = [cert_fixture('ca.pem'), cert_fixture('intermediate.pem')]
97+
sslctx = subject.create_system_context(cacerts: certs)
98+
expect(sslctx.cacerts).to eq(certs)
99+
end
100+
101+
it 'accepts expired CA certs' do
102+
expired = [cert_fixture('ca.pem'), cert_fixture('intermediate.pem')]
103+
expired.each { |x509| x509.not_after = Time.at(0) }
104+
105+
sslctx = subject.create_system_context(cacerts: expired)
106+
expect(sslctx.cacerts).to eq(expired)
107+
end
108+
109+
it 'raises if the frozen context is modified' do
110+
sslctx = subject.create_system_context(cacerts: [])
111+
expect {
112+
sslctx.verify_peer = false
113+
}.to raise_error(/can't modify frozen/)
114+
end
115+
116+
it 'trusts system ca store' do
117+
expect_any_instance_of(OpenSSL::X509::Store).to receive(:set_default_paths)
118+
119+
subject.create_system_context(cacerts: [])
120+
end
121+
122+
it 'verifies peer' do
123+
sslctx = subject.create_system_context(cacerts: [])
124+
expect(sslctx.verify_peer).to eq(true)
125+
end
126+
127+
it 'disable revocation' do
128+
sslctx = subject.create_system_context(cacerts: [])
129+
expect(sslctx.revocation).to eq(false)
130+
end
131+
132+
it 'sets client cert and private key to nil' do
133+
sslctx = subject.create_system_context(cacerts: [])
134+
expect(sslctx.client_cert).to be_nil
135+
expect(sslctx.private_key).to be_nil
136+
end
76137
end
77138

78139
context 'when creating an ssl context with crls' do
@@ -99,6 +160,11 @@
99160
sslctx = subject.create_root_context(config.merge(crls: expired))
100161
expect(sslctx.crls).to eq(expired)
101162
end
163+
164+
it 'verifies peer' do
165+
sslctx = subject.create_root_context(config)
166+
expect(sslctx.verify_peer).to eq(true)
167+
end
102168
end
103169

104170
context 'when creating an ssl context with client certs' do
@@ -345,6 +411,11 @@
345411
sslctx.verify_peer = false
346412
}.to raise_error(/can't modify frozen/)
347413
end
414+
415+
it 'verifies peer' do
416+
sslctx = subject.create_context(config)
417+
expect(sslctx.verify_peer).to eq(true)
418+
end
348419
end
349420

350421
context 'when loading an ssl context' do

0 commit comments

Comments
 (0)