Skip to content

Commit d428e6c

Browse files
committed
(PUP-10889) Explicitly set ciphersuite and allow it to be configurable
Previously puppet's ciphersuite depended on the ruby+openssl version in use. This commit makes it explicit by adding a `ciphers` setting and defaulting to Mozilla's "backward compatibility" option. All of the ciphers supported in puppet-agent 6.x (with ruby 2.5.8 and openssl 1.1.1i) are still supported: > old_ciphers = OpenSSL::SSL::SSLContext.new.ciphers > ctx = OpenSSL::SSL::SSLContext.new > ctx.ciphers = Puppet[:ciphers] > pp ctx.ciphers - old_ciphers [] > pp ctx.ciphers [["TLS_AES_256_GCM_SHA384", "TLSv1.3", 256, 256], ["TLS_CHACHA20_POLY1305_SHA256", "TLSv1.3", 256, 256], ["TLS_AES_128_GCM_SHA256", "TLSv1.3", 128, 128], ["ECDHE-ECDSA-AES128-GCM-SHA256", "TLSv1.2", 128, 128], ["ECDHE-RSA-AES128-GCM-SHA256", "TLSv1.2", 128, 128], ["ECDHE-ECDSA-AES256-GCM-SHA384", "TLSv1.2", 256, 256], ["ECDHE-RSA-AES256-GCM-SHA384", "TLSv1.2", 256, 256], ["ECDHE-ECDSA-CHACHA20-POLY1305", "TLSv1.2", 256, 256], ["ECDHE-RSA-CHACHA20-POLY1305", "TLSv1.2", 256, 256], ["DHE-RSA-AES128-GCM-SHA256", "TLSv1.2", 128, 128], ["DHE-RSA-AES256-GCM-SHA384", "TLSv1.2", 256, 256], ["DHE-RSA-CHACHA20-POLY1305", "TLSv1.2", 256, 256], ["ECDHE-ECDSA-AES128-SHA256", "TLSv1.2", 128, 128], ["ECDHE-RSA-AES128-SHA256", "TLSv1.2", 128, 128], ["ECDHE-ECDSA-AES128-SHA", "TLSv1.0", 128, 128], ["ECDHE-RSA-AES128-SHA", "TLSv1.0", 128, 128], ["ECDHE-ECDSA-AES256-SHA384", "TLSv1.2", 256, 256], ["ECDHE-RSA-AES256-SHA384", "TLSv1.2", 256, 256], ["ECDHE-ECDSA-AES256-SHA", "TLSv1.0", 256, 256], ["ECDHE-RSA-AES256-SHA", "TLSv1.0", 256, 256], ["DHE-RSA-AES128-SHA256", "TLSv1.2", 128, 128], ["DHE-RSA-AES256-SHA256", "TLSv1.2", 256, 256], ["AES128-GCM-SHA256", "TLSv1.2", 128, 128], ["AES256-GCM-SHA384", "TLSv1.2", 256, 256], ["AES128-SHA256", "TLSv1.2", 128, 128], ["AES256-SHA256", "TLSv1.2", 256, 256]] Although puppet never makes use of DSA keys, it's possible for a 3rd party HTTPS server to use them, so we include them in the list. The following ciphers are dropped, but that is ok because they only apply to SSLv3 or preshared key (PSK), neither of which we support or use: > pp old_ciphers - ctx.ciphers [["DHE-RSA-AES256-SHA", "SSLv3", 256, 256], ["DHE-RSA-AES128-SHA", "SSLv3", 128, 128], ["RSA-PSK-AES256-GCM-SHA384", "TLSv1.2", 256, 256], ["DHE-PSK-AES256-GCM-SHA384", "TLSv1.2", 256, 256], ["RSA-PSK-CHACHA20-POLY1305", "TLSv1.2", 256, 256], ["DHE-PSK-CHACHA20-POLY1305", "TLSv1.2", 256, 256], ["ECDHE-PSK-CHACHA20-POLY1305", "TLSv1.2", 256, 256], ["PSK-AES256-GCM-SHA384", "TLSv1.2", 256, 256], ["PSK-CHACHA20-POLY1305", "TLSv1.2", 256, 256], ["RSA-PSK-AES128-GCM-SHA256", "TLSv1.2", 128, 128], ["DHE-PSK-AES128-GCM-SHA256", "TLSv1.2", 128, 128], ["PSK-AES128-GCM-SHA256", "TLSv1.2", 128, 128], ["ECDHE-PSK-AES256-CBC-SHA384", "TLSv1.0", 256, 256], ["ECDHE-PSK-AES256-CBC-SHA", "TLSv1.0", 256, 256], ["SRP-RSA-AES-256-CBC-SHA", "SSLv3", 256, 256], ["SRP-AES-256-CBC-SHA", "SSLv3", 256, 256], ["RSA-PSK-AES256-CBC-SHA384", "TLSv1.0", 256, 256], ["DHE-PSK-AES256-CBC-SHA384", "TLSv1.0", 256, 256], ["RSA-PSK-AES256-CBC-SHA", "SSLv3", 256, 256], ["DHE-PSK-AES256-CBC-SHA", "SSLv3", 256, 256], ["AES256-SHA", "SSLv3", 256, 256], ["PSK-AES256-CBC-SHA384", "TLSv1.0", 256, 256], ["PSK-AES256-CBC-SHA", "SSLv3", 256, 256], ["ECDHE-PSK-AES128-CBC-SHA256", "TLSv1.0", 128, 128], ["ECDHE-PSK-AES128-CBC-SHA", "TLSv1.0", 128, 128], ["SRP-RSA-AES-128-CBC-SHA", "SSLv3", 128, 128], ["SRP-AES-128-CBC-SHA", "SSLv3", 128, 128], ["RSA-PSK-AES128-CBC-SHA256", "TLSv1.0", 128, 128], ["DHE-PSK-AES128-CBC-SHA256", "TLSv1.0", 128, 128], ["RSA-PSK-AES128-CBC-SHA", "SSLv3", 128, 128], ["DHE-PSK-AES128-CBC-SHA", "SSLv3", 128, 128], ["AES128-SHA", "SSLv3", 128, 128], ["PSK-AES128-CBC-SHA256", "TLSv1.0", 128, 128], ["PSK-AES128-CBC-SHA", "SSLv3", 128, 128]] This also sets the minimum version to TLS 1.0. Our openssl builds don't include SSLv3 support, but at least we're explicit about not downgrading.
1 parent 4be7016 commit d428e6c

File tree

4 files changed

+43
-0
lines changed

4 files changed

+43
-0
lines changed

lib/puppet/defaults.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,6 +1085,14 @@ def self.initialize_default_settings!(settings)
10851085
certificate revocation checking and does not attempt to download the CRL.
10861086
EOT
10871087
},
1088+
:ciphers => {
1089+
:default => 'ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:DHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES256-SHA256:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256',
1090+
:type => :string,
1091+
:desc => "The list of ciphersuites for TLS connections initiated by puppet. The
1092+
default value is chosen to support TLS 1.0 and up, but can be made
1093+
more restrictive if needed. The ciphersuites must be specified in OpenSSL
1094+
format, not IANA."
1095+
},
10881096
:key_type => {
10891097
:default => 'rsa',
10901098
:type => :enum,

lib/puppet/network/http/factory.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ def create_connection(site)
2727

2828
http = Puppet::Util::HttpProxy.proxy(URI(site.addr))
2929
http.use_ssl = site.use_ssl?
30+
if site.use_ssl?
31+
http.min_version = OpenSSL::SSL::TLS1_VERSION
32+
http.ciphers = Puppet[:ciphers]
33+
end
3034
http.read_timeout = Puppet[:http_read_timeout]
3135
http.open_timeout = Puppet[:http_connect_timeout]
3236
http.keep_alive_timeout = KEEP_ALIVE_TIMEOUT if http.respond_to?(:keep_alive_timeout=)

spec/integration/http/client_spec.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,4 +151,16 @@
151151
end
152152
end
153153
end
154+
155+
context 'ciphersuites' do
156+
it "does not connect when using an SSLv3 ciphersuite" do
157+
Puppet[:ciphers] = "DES-CBC3-SHA"
158+
159+
https_server.start_server do |port|
160+
expect {
161+
client.get(URI("https://127.0.0.1:#{port}"), options: {ssl_context: root_context})
162+
}.to raise_error(Puppet::HTTP::ConnectionError, /no cipher match/)
163+
end
164+
end
165+
end
154166
end

spec/unit/network/http/factory_spec.rb

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,4 +144,23 @@ def create_connection(site)
144144
expect(conn.local_host).to eq('127.0.0.1')
145145
end
146146
end
147+
148+
context 'tls' do
149+
it "sets the minimum version to TLS 1.0" do
150+
conn = create_connection(site)
151+
expect(conn.min_version).to eq(OpenSSL::SSL::TLS1_VERSION)
152+
end
153+
154+
it "defaults to ciphersuites providing 128 bits of security or greater" do
155+
conn = create_connection(site)
156+
expect(conn.ciphers).to eq("ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:DHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES256-SHA256:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256")
157+
end
158+
159+
it "can be restricted to TLSv1.3 ciphers" do
160+
tls13_ciphers = "TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256"
161+
Puppet[:ciphers] = tls13_ciphers
162+
conn = create_connection(site)
163+
expect(conn.ciphers).to eq(tls13_ciphers)
164+
end
165+
end
147166
end

0 commit comments

Comments
 (0)