Merge pull request #2768 from steveax/PE-36566-fix-auto-renew-cert-ttl

(PE-36566) fix auto-renew-cert-ttl

src/clj/puppetlabs/puppetserver/certificate_authority.clj

@@ -2128,9 +2128,9 @@
   "Given a certificate and CaSettings create a new signed certificate using the public key from the certificate.
   It recreates all the extensions in the original certificate."
   [certificate :- X509Certificate
-   {:keys [cacert cakey auto_renewal_cert_ttl] :as ca-settings} :- CaSettings
+   {:keys [cacert cakey auto-renewal-cert-ttl] :as ca-settings} :- CaSettings
    report-activity]
-  (let [validity (cert-validity-dates (or auto_renewal_cert_ttl default-auto-ttl-renewal-seconds))
+  (let [validity (cert-validity-dates (or auto-renewal-cert-ttl default-auto-ttl-renewal-seconds))
         cacert (utils/pem->ca-cert cacert cakey)
         cert-subject (utils/get-subject-from-x509-certificate certificate)
         cert-name (utils/x500-name->CN cert-subject)

test/integration/puppetlabs/services/certificate_authority/certificate_authority_int_test.clj

@@ -1190,6 +1190,48 @@
                       days (.convert TimeUnit/DAYS diff TimeUnit/MILLISECONDS)]
                   (is (= 89 days))))))))
 
+      (testing "Honors non-default auto-renewal-cert-ttl"
+        (bootstrap/with-puppetserver-running-with-mock-jrubies
+          "JRuby mocking is safe here because all of the requests are to the CA
+          endpoints, which are implemented in Clojure."
+          app
+          {:jruby-puppet
+           {:gem-path [(ks/absolute-path jruby-testutils/gem-path)]}
+           :webserver
+           {:ssl-cert     (str bootstrap/server-conf-dir "/ssl/certs/localhost.pem")
+            :ssl-key      (str bootstrap/server-conf-dir "/ssl/private_keys/localhost.pem")
+            :ssl-ca-cert  (str bootstrap/server-conf-dir "/ca/ca_crt.pem")
+            :ssl-crl-path (str bootstrap/server-conf-dir "/ssl/crl.pem")}
+           :certificate-authority
+           {:allow-auto-renewal true
+            :auto-renewal-cert-ttl "42d"}}
+          (let [generated-cert-info (generate-and-sign-a-cert! "foobar")
+                signed-cert-file (ks/temp-file)
+                _ (spit signed-cert-file (:signed-cert generated-cert-info))
+                _ (Thread/sleep 1000) ;; ensure some time has passed so the timestamps are different
+                response (http-client/post
+                           "https://localhost:8140/puppet-ca/v1/certificate_renewal"
+                           {:ssl-cert    (str signed-cert-file)
+                            :ssl-key     (str (:private-key generated-cert-info))
+                            :ssl-ca-cert (str bootstrap/server-conf-dir "/ca/ca_crt.pem")
+                            :as          :text})]
+            (is (= 200 (:status response)))
+            (let [renewed-cert-pem (:body response)
+                  renewed-cert-file (ks/temp-file)
+                  _ (spit renewed-cert-file renewed-cert-pem)
+                  renewed-cert (ssl-utils/pem->cert renewed-cert-file)
+                  signed-cert (ssl-utils/pem->cert signed-cert-file)]
+              (testing "serial number has been incremented"
+                (is (< (.getSerialNumber signed-cert) (.getSerialNumber renewed-cert))))
+              (testing "not before time stamps have changed"
+                (is (true? (.before (.getNotBefore signed-cert) (.getNotBefore renewed-cert)))))
+              (testing "new not-after is earlier than before"
+                (is (true? (.after (.getNotAfter signed-cert) (.getNotAfter renewed-cert)))))
+              (testing "new not-after should be 41 days (and some fraction) away"
+                (let [diff (- (.getTime (.getNotAfter renewed-cert)) (.getTime (Date.)))
+                      days (.convert TimeUnit/DAYS diff TimeUnit/MILLISECONDS)]
+                  (is (= 41 days))))))))
+
       (testing "returns a 400 bad request response when the ssl-client-cert is not present"
         (bootstrap/with-puppetserver-running-with-mock-jrubies
           "JRuby mocking is safe here because all of the requests are to the CA

test/unit/puppetlabs/puppetserver/certificate_authority_test.clj

@@ -1971,6 +1971,11 @@
 (deftest renew-certificate!-test
   (testing "creates a new signed cert"
     (let [settings (testutils/ca-sandbox! cadir)
+          ;; auto-renewal-cert-ttl is expected to be an int
+          ;; unit tests skip some of the conversion flow so
+          ;; transform the duration here
+          converted-auto-renewal-cert-ttl (ca/duration-str->sec (:auto-renewal-cert-ttl settings))
+          updated-settings (assoc settings :auto-renewal-cert-ttl converted-auto-renewal-cert-ttl)
           ca-cert (create-ca-cert "ca1" 1)
           keypair (utils/generate-key-pair)
           subject (utils/cn "foo")
@@ -1990,7 +1995,7 @@
         (ca/write-cert signed-cert expected-cert-path)
         (is (fs/exists? expected-cert-path)))
       (Thread/sleep 1000) ;; ensure there is some time elapsed between the two
-      (let [renewed-cert (ca/renew-certificate! signed-cert settings (constantly nil))]
+      (let [renewed-cert (ca/renew-certificate! signed-cert updated-settings (constantly nil))]
         (is (some? renewed-cert))
         (testing "serial number has increased"
           (is (< (.getSerialNumber signed-cert) (.getSerialNumber renewed-cert)))