Merge pull request #8507 from joshcooper/tls_ciphers

(PUP-10889) Explicitly set ciphersuite and allow it to be configurable

Author: joshcooper

lib/puppet/defaults.rb

@@ -1085,6 +1085,14 @@ def self.initialize_default_settings!(settings)
           certificate revocation checking and does not attempt to download the CRL.
         EOT
     },
+    :ciphers => {
+      :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',
+      :type => :string,
+      :desc => "The list of ciphersuites for TLS connections initiated by puppet. The
+                default value is chosen to support TLS 1.0 and up, but can be made
+                more restrictive if needed. The ciphersuites must be specified in OpenSSL
+                format, not IANA."
+    },
     :key_type => {
       :default => 'rsa',
       :type    => :enum,

lib/puppet/network/http/factory.rb

@@ -27,6 +27,10 @@ def create_connection(site)
 
     http = Puppet::Util::HttpProxy.proxy(URI(site.addr))
     http.use_ssl = site.use_ssl?
+    if site.use_ssl?
+      http.min_version = OpenSSL::SSL::TLS1_VERSION if http.respond_to?(:min_version)
+      http.ciphers = Puppet[:ciphers]
+    end
     http.read_timeout = Puppet[:http_read_timeout]
     http.open_timeout = Puppet[:http_connect_timeout]
     http.keep_alive_timeout = KEEP_ALIVE_TIMEOUT if http.respond_to?(:keep_alive_timeout=)

lib/puppet/util/monkey_patches.rb

@@ -32,6 +32,13 @@ def daemonize
 # (#19151) Reject all SSLv2 ciphers and handshakes
 require 'puppet/ssl/openssl_loader'
 unless Puppet::Util::Platform.jruby_fips?
+  unless defined?(OpenSSL::SSL::TLS1_VERSION)
+    module OpenSSL::SSL
+      # see https://github.com/ruby/ruby/commit/609103dbb5fb182eec12f052226c43e39b907682#diff-09f822c26289f5347111795ca22ed7ed1cfadd6ebd28f987991d1d414eef565aR2755-R2759
+      OpenSSL::SSL::TLS1_VERSION = 0x301
+    end
+  end
+
   class OpenSSL::SSL::SSLContext
     if DEFAULT_PARAMS[:options]
       DEFAULT_PARAMS[:options] |= OpenSSL::SSL::OP_NO_SSLv2 | OpenSSL::SSL::OP_NO_SSLv3

spec/integration/application/plugin_spec.rb

@@ -2,7 +2,7 @@
 require 'puppet/face'
 require 'puppet_spec/puppetserver'
 
-describe "puppet plugin" do
+describe "puppet plugin", unless: Puppet::Util::Platform.jruby? do
   include_context "https client"
 
   let(:server) { PuppetSpec::Puppetserver.new }

spec/integration/http/client_spec.rb

@@ -151,4 +151,16 @@
       end
     end
   end
+
+  context 'ciphersuites' do
+    it "does not connect when using an SSLv3 ciphersuite" do
+      Puppet[:ciphers] = "DES-CBC3-SHA"
+
+      https_server.start_server do |port|
+        expect {
+          client.get(URI("https://127.0.0.1:#{port}"), options: {ssl_context: root_context})
+        }.to raise_error(Puppet::HTTP::ConnectionError, /no cipher match|sslv3 alert handshake failure/)
+      end
+    end
+  end
 end

spec/unit/network/http/factory_spec.rb

@@ -144,4 +144,23 @@ def create_connection(site)
       expect(conn.local_host).to eq('127.0.0.1')
     end
   end
+
+  context 'tls' do
+    it "sets the minimum version to TLS 1.0", if: RUBY_VERSION.to_f >= 2.5 do
+      conn = create_connection(site)
+      expect(conn.min_version).to eq(OpenSSL::SSL::TLS1_VERSION)
+    end
+
+    it "defaults to ciphersuites providing 128 bits of security or greater" do
+      conn = create_connection(site)
+      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")
+    end
+
+    it "can be restricted to TLSv1.3 ciphers" do
+      tls13_ciphers = "TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256"
+      Puppet[:ciphers] = tls13_ciphers
+      conn = create_connection(site)
+      expect(conn.ciphers).to eq(tls13_ciphers)
+    end
+  end
 end