Merge pull request #1074 from slindberg/ticket/3.x/7962_cert_expire_w…

…arning_redux

(#7962) Certificate expiration warnings

ext/rack/files/apache2.conf

@@ -24,7 +24,8 @@ Listen 8140
         SSLCARevocationFile     /etc/puppet/ssl/ca/ca_crl.pem
         SSLVerifyClient optional
         SSLVerifyDepth  1
-        SSLOptions +StdEnvVars
+        # The `ExportCertData` option is needed for agent certificate expiration warnings
+        SSLOptions +StdEnvVars +ExportCertData
 
         # This header needs to be set if using a loadbalancer or proxy
         RequestHeader unset X-Forwarded-For

lib/puppet/agent.rb

@@ -70,7 +70,7 @@ def splay
     return unless Puppet[:splay]
     return if splayed?
 
-    time = rand(Integer(Puppet[:splaylimit]) + 1)
+    time = rand(Puppet[:splaylimit] + 1)
     Puppet.info "Sleeping for #{time} seconds (splay is enabled)"
     sleep(time)
     @splayed = true

lib/puppet/configurer.rb

@@ -3,13 +3,11 @@
 require 'timeout'
 require 'puppet/network/http_pool'
 require 'puppet/util'
-require 'puppet/util/config_timeout'
 
 class Puppet::Configurer
   require 'puppet/configurer/fact_handler'
   require 'puppet/configurer/plugin_handler'
 
-  extend Puppet::Util::ConfigTimeout
   include Puppet::Configurer::FactHandler
   include Puppet::Configurer::PluginHandler
 

lib/puppet/configurer/downloader.rb

@@ -1,10 +1,7 @@
 require 'puppet/configurer'
 require 'puppet/resource/catalog'
-require 'puppet/util/config_timeout'
 
 class Puppet::Configurer::Downloader
-  extend Puppet::Util::ConfigTimeout
-
   attr_reader :name, :path, :source, :ignore
 
   # Evaluate our download, returning the list of changed values.
@@ -13,7 +10,7 @@ def evaluate
 
     files = []
     begin
-      ::Timeout.timeout(self.class.timeout_interval) do
+      ::Timeout.timeout(Puppet[:configtimeout]) do
         catalog.apply do |trans|
           trans.changed?.find_all do |resource|
             yield resource if block_given?

lib/puppet/daemon.rb

@@ -158,11 +158,11 @@ def run_event_loop
     # input is strange or badly formed by returning 0.  Integer will raise,
     # which we don't want, and we want to protect against -1 or below.
     next_agent_run = 0
-    agent_run_interval = [Puppet[:runinterval].to_i, 0].max
+    agent_run_interval = [Puppet[:runinterval], 0].max
 
     # We may not want to reparse; that can be disable.  Fun times.
     next_reparse = 0
-    reparse_interval = Puppet[:filetimeout].to_i
+    reparse_interval = Puppet[:filetimeout]
 
     loop do
       now = Time.now.to_i
@@ -182,7 +182,7 @@ def run_event_loop
 
         # The time to the next reparse might have changed, so recalculate
         # now.  That way we react dynamically to reconfiguration.
-        reparse_interval = Puppet[:filetimeout].to_i
+        reparse_interval = Puppet[:filetimeout]
 
         # Set up the next reparse check based on the new reparse_interval.
         if reparse_interval > 0
@@ -193,7 +193,7 @@ def run_event_loop
         # We should also recalculate the agent run interval, and adjust the
         # next time it is scheduled to run, just in case.  In the event that
         # we made no change the result will be a zero second adjustment.
-        new_run_interval    = [Puppet[:runinterval].to_i, 0].max
+        new_run_interval    = [Puppet[:runinterval], 0].max
         next_agent_run     += agent_run_interval - new_run_interval
         agent_run_interval  = new_run_interval
       end

lib/puppet/defaults.rb

@@ -291,10 +291,11 @@ module Puppet
       :desc       => "The HTTP proxy port to use for outgoing connections",
     },
     :filetimeout => {
-      :default    => 15,
-      :desc       => "The minimum time to wait (in seconds) between checking for updates in
+      :default    => "15s",
+      :type       => :duration,
+      :desc       => "The minimum time to wait between checking for updates in
       configuration files.  This timeout determines how quickly Puppet checks whether
-      a file (such as manifests or templates) has changed on disk.",
+      a file (such as manifests or templates) has changed on disk. Can be specified as a duration.",
     },
     :queue_type => {
       :default    => "stomp",
@@ -576,6 +577,12 @@ module Puppet
         :type     => :boolean,
         :desc     => "Whether certificate revocation should be supported by downloading a Certificate Revocation List (CRL)
             to all clients.  If enabled, CA chaining will almost definitely not work.",
+    },
+    :certificate_expire_warning => {
+      :default  => "60d",
+      :type     => :duration,
+      :desc     => "The window of time leading up to a certificate's expiration that a notification
+        will be logged. This applies to CA, master, and agent certificates. Can be specified as a duration."
     }
   )
 
@@ -680,11 +687,8 @@ module Puppet
     },
     :ca_ttl => {
       :default    => "5y",
-      :desc       => "The default TTL for new certificates; valid values
-      must be an integer, optionally followed by one of the units
-      'y' (years of 365 days), 'd' (days), 'h' (hours), or
-      's' (seconds). The unit defaults to seconds.  Examples are '3600'
-      (one hour) and '1825d', which is the same as '5y' (5 years) ",
+      :type       => :duration,
+      :desc       => "The default TTL for new certificates. Can be specified as a duration."
     },
     :ca_md => {
       :default    => "md5",
@@ -887,8 +891,9 @@ module Puppet
     },
     :rrdinterval => {
       :default  => "$runinterval",
+      :type     => :duration,
       :desc     => "How often RRD should expect data.
-            This should match how often the hosts report back to the server.",
+            This should match how often the hosts report back to the server. Can be specified as a duration.",
     }
   )
 
@@ -1007,11 +1012,12 @@ module Puppet
       :desc       => "Whether puppet agent should be run in noop mode.",
     },
     :runinterval => {
-      :default  => 1800, # 30 minutes
+      :default  => "30m",
+      :type     => :duration,
       :desc     => "How often puppet agent applies the client configuration; in seconds.
           Note that a runinterval of 0 means \"run continuously\" rather than
           \"never run.\" If you want puppet agent to never run, you should start
-          it with the `--no-client` option.",
+          it with the `--no-client` option. Can be specified as a duration.",
     },
     :listen => {
       :default    => false,
@@ -1093,8 +1099,9 @@ module Puppet
     },
     :splaylimit => {
       :default    => "$runinterval",
+      :type       => :duration,
       :desc       => "The maximum time to delay before runs.  Defaults to being the same as the
-      run interval.",
+      run interval. Can be specified as a duration.",
     },
     :splay => {
       :default    => false,
@@ -1109,10 +1116,11 @@ module Puppet
       :desc     => "Where FileBucket files are stored locally."
     },
     :configtimeout => {
-      :default  => 120,
+      :default  => "2m",
+      :type     => :duration,
       :desc     => "How long the client should wait for the configuration to be retrieved
       before considering it a failure.  This can help reduce flapping if too
-      many clients contact the server at one time.",
+      many clients contact the server at one time. Can be specified as a duration.",
     },
     :report_server => {
       :default  => "$server",
@@ -1170,10 +1178,12 @@ module Puppet
       compression, but if it supports it, this setting might reduce performance on high-speed LANs.",
     },
     :waitforcert => {
-        :default  => 120, # 2 minutes
-        :desc     => "The time interval, specified in seconds, 'puppet agent' should connect to the server
-            and ask it to sign a certificate request. This is useful for the initial setup of a
-            puppet client. You can turn off waiting for certificates by specifying a time of 0.",
+      :default  => "2m",
+      :type     => :duration,
+      :desc     => "The time interval 'puppet agent' should connect to the server
+      and ask it to sign a certificate request. This is useful for the initial setup of a
+      puppet client. You can turn off waiting for certificates by specifying a time of 0.
+      Can be specified as a duration.",
     }
   )
 

lib/puppet/indirector/facts/facter.rb

@@ -1,10 +1,7 @@
 require 'puppet/node/facts'
 require 'puppet/indirector/code'
-require 'puppet/util/config_timeout'
 
 class Puppet::Node::Facts::Facter < Puppet::Indirector::Code
-  extend Puppet::Util::ConfigTimeout
-
   desc "Retrieve facts from Facter.  This provides a somewhat abstract interface
     between Puppet and Facter.  It's only `somewhat` abstract because it always
     returns the local host's facts, regardless of what you attempt to find."
@@ -35,7 +32,7 @@ def self.load_facts_in_dir(dir)
         fqfile = ::File.join(dir, file)
         begin
           Puppet.info "Loading facts in #{fqfile}"
-          ::Timeout::timeout(self.timeout_interval) do
+          ::Timeout::timeout(Puppet[:configtimeout]) do
             load file
           end
         rescue SystemExit,NoMemoryError

lib/puppet/indirector/indirection.rb

@@ -72,7 +72,7 @@ def ttl=(value)
 
   # Default to the runinterval for the ttl.
   def ttl
-    @ttl ||= Puppet[:runinterval].to_i
+    @ttl ||= Puppet[:runinterval]
   end
 
   # Calculate the expiration date for a returned instance.

lib/puppet/network/authentication.rb

@@ -0,0 +1,30 @@
+require 'puppet/ssl/certificate_authority'
+require 'puppet/util/log/rate_limited_logger'
+
+# Place for any authentication related bits
+module Puppet::Network::Authentication
+  # Create a rate-limited logger for the expiration warning that uses the run interval
+  # as the minimum amount of time before a warning about the same cert can be logged again.
+  # This is a class variable so that all classes that include the module share the same logger.
+  @@logger = Puppet::Util::Log::RateLimitedLogger.new(Puppet[:runinterval])
+
+  # Check the expiration of known certificates and optionally any that are specified as part of a request
+  def warn_if_near_expiration(*certs)
+    # Check CA cert if we're functioning as a CA
+    certs << Puppet::SSL::CertificateAuthority.instance.host.certificate if Puppet::SSL::CertificateAuthority.ca?
+
+    # Always check the host cert if we have one, this will be the agent or master cert depending on the run mode
+    certs << Puppet::SSL::Host.localhost.certificate if FileTest.exist?(Puppet[:hostcert])
+
+    # Remove nil values for caller convenience
+    certs.compact.each do |cert|
+      # Allow raw OpenSSL certificate instances or Puppet certificate wrappers to be specified
+      cert = Puppet::SSL::Certificate.from_instance(cert) if cert.is_a?(OpenSSL::X509::Certificate)
+      raise ArgumentError, "Invalid certificate '#{cert.inspect}'" unless cert.is_a?(Puppet::SSL::Certificate)
+
+      if cert.near_expiration?
+        @@logger.warning("Certificate '#{cert.unmunged_name}' will expire on #{cert.expiration.strftime('%Y-%m-%dT%H:%M:%S%Z')}")
+      end
+    end
+  end
+end

lib/puppet/network/http/connection.rb

@@ -1,6 +1,7 @@
 require 'net/https'
 require 'puppet/ssl/host'
 require 'puppet/ssl/configuration'
+require 'puppet/network/authentication'
 
 module Puppet::Network::HTTP
 
@@ -14,6 +15,7 @@ module Puppet::Network::HTTP
   # * Provides some useful error handling for any SSL errors that occur
   #   during a request.
   class Connection
+    include Puppet::Network::Authentication
 
     def initialize(host, port, use_ssl = true)
       @host = host
@@ -50,15 +52,21 @@ def request(method, *args)
         # constructing the error message if the verification failed.
         # This is necessary since we don't have direct access to the
         # cert that we expected the connection to use otherwise.
-        peer_certs << Puppet::SSL::Certificate.from_s(ssl_context.current_cert.to_pem)
+        peer_certs << Puppet::SSL::Certificate.from_instance(ssl_context.current_cert)
         # And also keep the detailed verification error if such an error occurs
         if ssl_context.error_string and not preverify_ok
           verify_errors << "#{ssl_context.error_string} for #{ssl_context.current_cert.subject}"
         end
         preverify_ok
       end
 
-      connection.send(method, *args)
+      response = connection.send(method, *args)
+
+      # Now that the request completed successfully, lets check the involved
+      # certificates for approaching expiration dates
+      warn_if_near_expiration(*peer_certs)
+
+      response
     rescue OpenSSL::SSL::SSLError => error
       if error.message.include? "certificate verify failed"
         msg = error.message

lib/puppet/network/http/handler.rb

@@ -3,12 +3,14 @@ module Puppet::Network::HTTP
 
 require 'puppet/network/http/api/v1'
 require 'puppet/network/authorization'
+require 'puppet/network/authentication'
 require 'puppet/network/rights'
 require 'resolv'
 
 module Puppet::Network::HTTP::Handler
   include Puppet::Network::HTTP::API::V1
   include Puppet::Network::Authorization
+  include Puppet::Network::Authentication
 
   attr_reader :server, :handler
 
@@ -64,6 +66,7 @@ def process(request, response)
     indirection, method, key, params = uri2indirection(http_method(request), path(request), params(request))
 
     check_authorization(indirection, method, key, params)
+    warn_if_near_expiration(client_cert(request))
 
     send("do_#{method}", indirection, key, params, request, response)
   rescue SystemExit,NoMemoryError
@@ -216,6 +219,11 @@ def params(request)
     raise NotImplementedError
   end
 
+  # Retrieve the client certificate from the request if possible
+  def client_cert(request)
+    raise NotImplementedError
+  end
+
   def decode_params(params)
     params.inject({}) do |result, ary|
       param, value = ary