(PE-36269) ensure CRLs are regenerated when nearing expiration

src/clj/puppetlabs/puppetserver/certificate_authority.clj

@@ -6,6 +6,7 @@
            (java.security PrivateKey PublicKey)
            (java.security.cert X509Certificate CRLException CertPathValidatorException X509CRL)
            (java.text SimpleDateFormat)
+           (java.time LocalDateTime ZoneId)
            (java.util Date)
            (java.util.concurrent.locks ReentrantReadWriteLock)
            (org.apache.commons.io IOUtils)
@@ -246,6 +247,9 @@
 (def default-auto-ttl-renewal-seconds
   (duration-str->sec default-auto-ttl-renewal)) ; 90 days by default
 
+;; if the crl is going to expire in less than this number of days, it should be regenerated.
+(def crl-expiration-window-days 30)
+
 (schema/defn ^:always-validate initialize-ca-config
   "Adds in default ca config keys/values, which may be overwritten if a value for
   any of those keys already exists in the ca-data"
@@ -785,6 +789,15 @@
     ;; lack of an end date means we can't tell if it is expired or not, so assume it isn't.
     false))
 
+(schema/defn is-expired? :- schema/Bool
+  [now :- DateTime
+   [_serial _not-before not-after _row-subject] :- [schema/Str]]
+  (if-let [not-after-date (parse-date-time not-after)]
+    (time/after? now not-after-date)
+    ;; lack of an end date means we can't tell if it is expired or not, so assume it isn't.
+    false))
+
+
 (defn extract-inventory-row-contents
   [row]
   (str/split row #" "))
@@ -803,6 +816,30 @@
         (log/debug "Unable to find inventory file {0}" cert-inventory)
         false))))
 
+(schema/defn base-16-str->biginteger :- BigInteger
+  "Given a base-16 string with a leading 0x, return the result as a BigInteger"
+  [serial :- schema/Str]
+  (BigInteger. ^String (subs serial 2) 16))
+
+(schema/defn expired-inventory-serials :- [BigInteger]
+  [{:keys [cert-inventory inventory-lock inventory-lock-timeout-seconds]} :- CaSettings]
+  (common/with-safe-read-lock inventory-lock inventory-lock-descriptor inventory-lock-timeout-seconds
+    (log/trace (i18n/trs "Extracting expired serials from inventory file {1}" cert-inventory))
+    (if (fs/exists? cert-inventory)
+      (with-open [inventory-reader (io/reader cert-inventory)]
+        (let [now (time/now)]
+          (doall
+            (->>
+              (line-seq inventory-reader)
+              (map extract-inventory-row-contents )
+              (filter (partial is-expired? now))
+              (map first)
+              ;; assume serials are base 16 strings
+              (map base-16-str->biginteger)))))
+      (do
+        (log/debug "Unable to find inventory file {0}" cert-inventory)
+        []))))
+
 (schema/defn find-matching-valid-serial-numbers :- [BigInteger]
   [{:keys [cert-inventory inventory-lock inventory-lock-timeout-seconds]} :- CaSettings
    certname :- schema/Str]
@@ -820,7 +857,7 @@
                  ;; serial number is first entry in the array
                  (map first)
                  ;; assume serials are base 16 strings, drop the `0x` as BigInteger won't deal with them.
-                 (map #(BigInteger. ^String (subs % 2) 16))))))
+                 (map base-16-str->biginteger)))))
       (do
         (log/debug (i18n/trs "Unable to find inventory file {0}" cert-inventory))
         []))))
@@ -2298,3 +2335,68 @@
                                  (.getNotAfter signed-cert))))
     (report-activity [cert-subject] "renewed")
     signed-cert))
+
+(schema/defn crl-expires-in-n-days?
+  [crl-path {:keys [crl-lock crl-lock-timeout-seconds]} :- CaSettings
+   days :- schema/Int]
+  (common/with-safe-write-lock crl-lock crl-lock-descriptor crl-lock-timeout-seconds
+    (let [crl-object (first (utils/pem->crls crl-path))
+          next-update (-> crl-object
+                          .getNextUpdate
+                          .toInstant
+                          (.atZone (ZoneId/systemDefault))
+                          .toLocalDateTime)]
+      (.isBefore next-update (.plusDays (LocalDateTime/now) days)))))
+
+(schema/defn overwrite-existing-crl!
+  [crl :- X509CRL
+   rest-of-full-chain
+   capub :- schema/Str
+   cakey :- schema/Str
+   cacert :- X509Certificate
+   valid-serials :- [BigInteger]
+   crl-path :- schema/Str]
+  (let [^BigInteger crl-number (utils/get-crl-number crl)
+        public-key (utils/pem->public-key capub)
+        private-key (utils/pem->private-key cakey)
+        ;; because we are purging existing serials, we need a whole new CRL
+        ;; the "next update" and "crl-number" will get updated by the process that
+        ;; adds the serial numbers to the crl
+        new-full-crl (utils/generate-crl
+                       (.getIssuerX500Principal cacert)
+                       private-key
+                       public-key
+                       (.getThisUpdate crl)
+                       (.getNextUpdate crl)
+                       crl-number
+                       ;; original crl is generated with no extensions, continue the precedence
+                       nil)
+        ;; create a new CRL with incremented "next update" and crl-number, and all the serials
+        new-crl-with-revoked (utils/revoke-multiple new-full-crl
+                                                    (utils/pem->private-key cakey)
+                                                    (.getPublicKey cacert)
+                                                    valid-serials)
+        new-full-chain (cons new-crl-with-revoked (vec rest-of-full-chain))]
+    (write-crls new-full-chain crl-path)))
+(schema/defn update-and-sign-crl!
+  "Given a path to a CRL, and the ca-settings, update the CRl with all known valid serials that have been revoked"
+  [path-to-crl {:keys [crl-lock crl-lock-timeout-seconds cacert cakey capub] :as settings} :- CaSettings]
+  ;; read in the existing crl, and extract the serial numbers that have expired so we can remove them from the CRL set.
+  (let [expired-serials (set (expired-inventory-serials settings))]
+    (common/with-safe-write-lock crl-lock crl-lock-descriptor crl-lock-timeout-seconds
+      (let [[our-full-crl & rest-of-full-chain] (utils/pem->crls path-to-crl)
+            crl-serials  (set (map #(.getSerialNumber %) (.getRevokedCertificates our-full-crl)))
+            valid-crl-serials (set/difference crl-serials expired-serials)
+            cacert (utils/pem->ca-cert cacert cakey)]
+        (overwrite-existing-crl! our-full-crl rest-of-full-chain capub cakey cacert (vec valid-crl-serials) path-to-crl)))))
+
+(schema/defn maybe-update-crls-for-expiration
+   [{:keys [cacrl enable-infra-crl infra-crl-path] :as settings} :- CaSettings]
+  ;; check the age of the main crl
+  (when (crl-expires-in-n-days? cacrl settings crl-expiration-window-days)
+    (log/info (i18n/trs "CA CRL expiring within 30 days, updating."))
+    (update-and-sign-crl! cacrl settings))
+  (when (and enable-infra-crl (fs/exists? infra-crl-path))
+    (when (crl-expires-in-n-days? infra-crl-path settings crl-expiration-window-days)
+      (log/info (i18n/trs "infra crl expiring within 30 days, updating."))
+      (update-and-sign-crl! infra-crl-path settings))))

src/clj/puppetlabs/services/analytics/analytics_service.clj

@@ -8,10 +8,23 @@
 (defprotocol AnalyticsService
   "Protocol placeholder for the analytics service.")
 
+(def analytics-service-job-group-id
+  :analytics-service-job-group)
+
+(defn safe-run-dropsonde
+  "Prevent exceptions from escaping as this is run in a scheduled task"
+  [config]
+  (try
+    (log/debug (i18n/trs "Running dropsonde"))
+    (run-dropsonde config)
+    (log/debug (i18n/trs "dropsonde run complete"))
+    (catch Exception _
+      (log/info (i18n/trs "Failed while running dropsonde")))))
+
 (defservice analytics-service
   AnalyticsService
   [[:PuppetServerConfigService get-config]
-   [:SchedulerService interspaced]]
+   [:SchedulerService interspaced stop-jobs]]
 
   (start
    [this context]
@@ -30,7 +43,8 @@
                          (version-check/check-for-update
                           {:product-name product-name} update-server-url)
                          (catch Exception _
-                           (log/error (i18n/trs "Failed to check for product updates"))))))
+                           (log/error (i18n/trs "Failed to check for product updates")))))
+                      analytics-service-job-group-id)
          (log/info (i18n/trs "Not checking for updates - opt-out setting exists"))))
      (log/info (i18n/trs "Puppet Server Update Service has successfully started and will run in the background"))
 
@@ -40,7 +54,12 @@
            dropsonde-interval-millis (* 1000 (get-in config [:dropsonde :interval]
                                                (* 60 60 24 7)))]
        (if dropsonde-enabled
-         (interspaced dropsonde-interval-millis #(run-dropsonde config))
+         (interspaced dropsonde-interval-millis #(safe-run-dropsonde config) analytics-service-job-group-id)
          (log/info (i18n/trs (str "Not submitting module metrics via Dropsonde -- submission is disabled. "
                                   "Enable this feature by setting `dropsonde.enabled` to true in Puppet Server''s config."))))))
-   context))
+   context)
+
+  (stop [this context]
+    (log/info (i18n/trs "Puppet Server Update Service shutting down"))
+    (stop-jobs analytics-service-job-group-id)
+    context))

src/clj/puppetlabs/services/ca/certificate_authority_service.clj

@@ -12,14 +12,29 @@
             [puppetlabs.trapperkeeper.services.status.status-core :as status-core]
             [puppetlabs.rbac-client.protocols.activity :refer [ActivityReportingService] :as activity-proto]))
 
+(def one-day-ms
+  (* 24 60 60 1000))
+
+(def ca-scheduled-job-group-id
+  :ca-scheduled-job-group-id)
+
+(defn evaluate-crls-for-expiration
+  [ca-settings]
+  (try
+    ;; don't allow exceptions to escape
+    (ca/maybe-update-crls-for-expiration ca-settings)
+    (catch Exception e
+      (log/error e (i18n/trs "Failed to evaluate crls for expiration")))))
+
 (tk/defservice certificate-authority-service
   CaService
    {:required 
     [[:PuppetServerConfigService get-config get-in-config]
       [:WebroutingService add-ring-handler get-route]
       [:AuthorizationService wrap-with-authorization-check]
       [:FilesystemWatchService create-watcher]
-      [:StatusService register-status]]
+      [:StatusService register-status]
+      [:SchedulerService interspaced stop-jobs]]
    :optional [ActivityReportingService]}
   (init
     [this context]
@@ -86,7 +101,17 @@
         1
         core/v1-status)
       (assoc context :auth-handler auth-handler
-                     :watcher watcher)))
+                     :watcher watcher
+                     :ca-settings settings)))
+  (start [this context]
+    (log/info (i18n/trs "Starting CA service"))
+    (interspaced one-day-ms #(evaluate-crls-for-expiration (:ca-settings context)) ca-scheduled-job-group-id)
+    context)
+
+  (stop [this context]
+    (log/info (i18n/trs "Stopping CA service"))
+    (stop-jobs ca-scheduled-job-group-id)
+    (dissoc context :ca-settings))
 
   (initialize-master-ssl!
    [this master-settings certname]

src/clj/puppetlabs/services/master/master_service.clj

@@ -104,7 +104,6 @@
    [:CaService initialize-master-ssl! retrieve-ca-cert! retrieve-ca-crl! get-auth-handler]
    [:JRubyPuppetService]
    [:AuthorizationService wrap-with-authorization-check]
-   [:SchedulerService interspaced]
    [:StatusService register-status]
    [:VersionedCodeService get-code-content current-code-id]]
   (init

test/unit/puppetlabs/puppetserver/certificate_authority_test.clj

@@ -5,12 +5,14 @@
                     ByteArrayOutputStream)
            (com.puppetlabs.ssl_utils SSLUtils)
            (java.security PublicKey MessageDigest)
+           (java.time LocalDateTime ZoneOffset)
            (java.util Date)
            (java.util.concurrent TimeUnit)
            (java.util.concurrent.locks ReentrantReadWriteLock)
            (org.joda.time DateTime Period)
            (org.bouncycastle.asn1.x509 SubjectPublicKeyInfo))
-  (:require [puppetlabs.puppetserver.certificate-authority :as ca]
+  (:require [clojure.set :as set]
+            [puppetlabs.puppetserver.certificate-authority :as ca]
             [puppetlabs.trapperkeeper.testutils.logging :refer [logged?] :as logutils]
             [puppetlabs.ssl-utils.core :as utils]
             [puppetlabs.ssl-utils.simple :as simple]
@@ -2118,3 +2120,131 @@
           subject (utils/cn "foo")
           csr  (utils/generate-certificate-request keypair subject [] [{:oid "1.3.6.1.4.1.34380.1.3.2" :value true}])]
       (is (= [{:oid "1.3.6.1.4.1.34380.1.3.2", :values ["true"]}] (ca/get-csr-attributes csr))))))
+
+(deftest crl-expires-in-n-days?-test
+    (let [settings (testutils/ca-sandbox! cadir)]
+      ;; by default the crl expires in 5 years
+      (testing "CRL with long expiration don't expire soon"
+        (is (false? (ca/crl-expires-in-n-days? (:cacrl settings) settings 5))))
+      (testing "CRL with long expiration expires in ~6 years"
+        (is (ca/crl-expires-in-n-days? (:cacrl settings) settings (* 365 6))))
+      (testing "created crl expires in 5 days"
+        (let [[crl & rest-of-full-chain] (utils/pem->crls (:cacrl settings))
+              public-key (utils/pem->public-key (:capub settings))
+              private-key (utils/pem->private-key (:cakey settings))
+              new-crl (utils/generate-crl
+                        (.getIssuerX500Principal (utils/pem->ca-cert (:cacert settings) (:cakey settings)))
+                        private-key
+                        public-key
+                        (.getThisUpdate crl)
+                        ;; create a date 5 days from now using java.time
+                        (-> (LocalDateTime/now)
+                            (.plusDays 5)
+                            (.toInstant (ZoneOffset/UTC))
+                            (Date/from))
+                        (biginteger 1)
+                        nil)
+              new-full-chain (cons new-crl (vec rest-of-full-chain))
+              temp-path (fs/temp-file "crl" "pem")
+              path-as-string (.getCanonicalPath temp-path)]
+          (ca/write-crls new-full-chain (.getCanonicalPath temp-path))
+          (doseq [i (range 0 4)]
+            (is (false? (ca/crl-expires-in-n-days? path-as-string settings i))))
+          (is (ca/crl-expires-in-n-days? path-as-string settings 5))
+          (is (ca/crl-expires-in-n-days? path-as-string settings 6))))))
+
+(deftest overwrite-existing-crl!-test
+  (let [settings (testutils/ca-sandbox! cadir)
+        [crl & rest-of-full-chain] (utils/pem->crls (:cacrl settings))
+        crl-serials  (set (map #(.getSerialNumber %) (.getRevokedCertificates crl)))
+        all-serials (set/union crl-serials (set (map biginteger (range 1000 2000))))
+        ca-cert (utils/pem->ca-cert (:cacert settings) (:cakey settings))
+        before-next-update (.getNextUpdate crl)
+        crl-number (utils/get-crl-number crl)]
+    (ca/overwrite-existing-crl! crl rest-of-full-chain (:capub settings) (:cakey settings) ca-cert (vec all-serials) (:cacrl settings))
+    (testing "overwritten crl has updated properties"
+      (let [[updated-crl & _rest-of-full-chain] (utils/pem->crls (:cacrl settings))]
+        (is (.after (.getNextUpdate updated-crl) before-next-update))
+        (is (< crl-number (utils/get-crl-number updated-crl)))
+        (is (not= (set (map #(.getSerialNumber %) (.getRevokedCertificates updated-crl)))
+                  crl-serials))
+        (is (= (set (map #(.getSerialNumber %) (.getRevokedCertificates updated-crl)))
+               all-serials))))))
+(deftest expired-inventory-serials-test
+  (let [settings (testutils/ca-sandbox! cadir)
+        keypair (utils/generate-key-pair)
+        subject (utils/cn "foo")
+        csr  (utils/generate-certificate-request keypair subject)
+        serial-number  (ca/next-serial-number! settings)
+        ca-cert (utils/pem->ca-cert (:cacert settings) (:cakey settings))
+        private-key (utils/pem->private-key (:cakey settings))
+
+        signed-cert (utils/sign-certificate
+                      (utils/get-subject-from-x509-certificate ca-cert)
+                      private-key
+                      serial-number
+                      ;; valid from 100 days ago until yesterday
+                      (-> (LocalDateTime/now)
+                          (.minusDays 100)
+                          (.toInstant ZoneOffset/UTC)
+                          (Date/from))
+                      (-> (LocalDateTime/now)
+                          (.minusDays 1)
+                          (.toInstant ZoneOffset/UTC)
+                          (Date/from))
+                      subject
+                      (utils/get-public-key csr)
+                      (ca/create-agent-extensions csr ca-cert))]
+    (ca/write-cert-to-inventory! signed-cert settings)
+    ;; there are multiple entries in the inventory file that aren't expired by default
+    (is (= [(biginteger serial-number)] (ca/expired-inventory-serials settings)))))
+
+(deftest update-and-sign-crl!-test
+  (let [settings (testutils/ca-sandbox! cadir)
+        [crl & rest-of-full-chain] (utils/pem->crls (:cacrl settings))
+        crl-serials  (set (map #(.getSerialNumber %) (.getRevokedCertificates crl)))
+        crl-number (utils/get-crl-number crl)
+        serial-number  (ca/next-serial-number! settings)
+        keypair (utils/generate-key-pair)
+        subject (utils/cn "foo")
+        csr  (utils/generate-certificate-request keypair subject)
+        ca-cert (utils/pem->ca-cert (:cacert settings) (:cakey settings))
+        private-key (utils/pem->private-key (:cakey settings))
+        ;; create an add an expired cert to add to the inventory after we update the crl with
+        ;; the serial number of the cert
+        signed-cert (utils/sign-certificate
+                      (utils/get-subject-from-x509-certificate ca-cert)
+                      private-key
+                      serial-number
+                      ;; valid from 100 days ago until yesterday
+                      (-> (LocalDateTime/now)
+                          (.minusDays 100)
+                          (.toInstant ZoneOffset/UTC)
+                          (Date/from))
+                      (-> (LocalDateTime/now)
+                          (.minusDays 1)
+                          (.toInstant ZoneOffset/UTC)
+                          (Date/from))
+                      subject
+                      (utils/get-public-key csr)
+                      (ca/create-agent-extensions csr ca-cert))
+        extra-serials (set (map biginteger (range 1000 2000)))
+        all-serials (set/union crl-serials #{(biginteger serial-number)} extra-serials)]
+    (ca/overwrite-existing-crl! crl rest-of-full-chain (:capub settings) (:cakey settings) ca-cert (vec all-serials) (:cacrl settings))
+    (let [[updated-crl & _rest-of-full-chain] (utils/pem->crls (:cacrl settings))
+          updated-crl-number (utils/get-crl-number updated-crl)]
+      (is (< crl-number updated-crl-number))
+      (ca/write-cert-to-inventory! signed-cert settings)
+      (ca/update-and-sign-crl! (:cacrl settings) settings)
+      (let [[final-updated-crl & _rest-of-full-chain] (utils/pem->crls (:cacrl settings))
+            final-crl-number (utils/get-crl-number final-updated-crl)
+            final-crl-serials  (set (map #(.getSerialNumber %) (.getRevokedCertificates final-updated-crl)))]
+        (is (< updated-crl-number final-crl-number))
+        ;; should not contain the serial that was expired
+        (is (= (set/union crl-serials extra-serials) final-crl-serials))))))
+
+(deftest base-16-str->biginteger-test
+  (testing "returns expected results"
+    (doseq [i (range 1 10000)]
+      (is (= (biginteger i) (ca/base-16-str->biginteger (format "0x%x" i)))))))
+