Merge pull request #2805 from jonathannewman/PE-34953/main/sign-all-csr

(PE-34953) add sign/all endpoint

Author: steveax

src/clj/puppetlabs/puppetserver/certificate_authority.clj

@@ -18,7 +18,7 @@
             [slingshot.slingshot :as sling])
   (:import (java.io BufferedReader BufferedWriter ByteArrayInputStream ByteArrayOutputStream File FileNotFoundException IOException InputStream Reader StringReader)
            (java.nio CharBuffer)
-           (java.nio.file Files)
+           (java.nio.file Files Path Paths)
            (java.nio.file.attribute FileAttribute PosixFilePermissions)
            (java.security PrivateKey PublicKey)
            (java.security.cert CRLException CertPathValidatorException X509CRL X509Certificate)
@@ -1530,6 +1530,18 @@
     (when (fs/exists? cert-request-path)
       (slurp cert-request-path))))
 
+(schema/defn ^:always-validate
+  get-paths-to-all-certificate-requests :- [Path]
+  "Given a csr directory, return Path entries to all the files that could be CSRs"
+  [csrdir :- schema/Str]
+  (let [csr-dir-as-path (Paths/get csrdir (into-array String []))]
+    (if (Files/isDirectory csr-dir-as-path ks-file/nofollow-links)
+      (with-open [dir-stream (Files/newDirectoryStream csr-dir-as-path "*.pem")]
+        (doall (iterator-seq (.iterator dir-stream))))
+      (do
+        (log/error (i18n/trs "Attempting to use {0} as CSR directory, but it is not a directory." csrdir))
+        []))))
+
 (schema/defn ^:always-validate
   autosign-csr? :- schema/Bool
   "Return true if the CSR should be automatically signed given

src/clj/puppetlabs/puppetserver/common.clj

@@ -1,7 +1,8 @@
 (ns puppetlabs.puppetserver.common
-  (:require [clojure.tools.logging :as log]
-            [schema.core :as schema]
+  (:require [clojure.string :as str]
+            [clojure.tools.logging :as log]
             [puppetlabs.i18n.core :as i18n]
+            [schema.core :as schema]
             [slingshot.slingshot :as sling])
   (:import (java.util List Map Set)
            (java.util.concurrent TimeUnit)
@@ -124,3 +125,19 @@
   (let [yaml (new Yaml)
         data (.load yaml ^String yaml-string)]
     (java->clj data)))
+
+(defn extract-file-names-from-paths
+  "Given a sequence of java.nio.file.Path objects, return a lazy sequence of the file names of the file represented
+  by those paths. Example ['/foo/bar/baz.tmp'] will result in ['baz.tmp']"
+  [paths-to-files]
+  (map #(.toString (.getFileName %)) paths-to-files))
+
+(defn remove-suffix-from-file-names
+  "Given a suffix, and a sequence of file-names, remove the suffix from the filenames"
+  [files suffix]
+  (let [suffix-size (count suffix)]
+    (map (fn [s]
+           (if (str/ends-with? s suffix)
+             (subs s 0 (- (count s) suffix-size))
+             s))
+         files)))

src/clj/puppetlabs/services/ca/certificate_authority_core.clj

@@ -276,11 +276,14 @@
           (rr/content-type "application/json")))))
 
 (schema/defn handle-bulk-cert-signing-all
-  [_request
-   _ca-settings :- ca/CaSettings]
-  (-> (rr/response (cheshire/generate-string {}))
-      (rr/status 200)
-      (rr/content-type "application/json")))
+  [ca-settings :- ca/CaSettings report-activity]
+  (let [csr-files (-> (ca/get-paths-to-all-certificate-requests (:csrdir ca-settings))
+                      (common/extract-file-names-from-paths)
+                      (common/remove-suffix-from-file-names ".pem"))
+        results (ca/sign-multiple-certificate-signing-requests! csr-files ca-settings report-activity)]
+    (-> (rr/response (cheshire/generate-string results))
+        (rr/status 200)
+        (rr/content-type "application/json"))))
 
 (schema/defn ^:always-validate
   handle-cert-renewal
@@ -568,8 +571,8 @@
         (handle-cert-renewal request ca-settings report-activity))
       (POST ["/sign"] request
         (handle-bulk-cert-signing request ca-settings report-activity))
-      (POST ["/sign/all"] request
-        (handle-bulk-cert-signing-all request ca-settings)))
+      (POST ["/sign/all"] _request
+        (handle-bulk-cert-signing-all  ca-settings report-activity)))
     (comidi/not-found "Not Found")))
 
 (schema/defn ^:always-validate

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

@@ -8,6 +8,7 @@
     [me.raynes.fs :as fs]
     [puppetlabs.http.client.sync :as http-client]
     [puppetlabs.kitchensink.core :as ks]
+    [puppetlabs.kitchensink.file :as ks-file]
     [puppetlabs.puppetserver.bootstrap-testutils :as bootstrap]
     [puppetlabs.puppetserver.certificate-authority :as ca]
     [puppetlabs.puppetserver.testutils :as testutils :refer [http-get]]
@@ -84,6 +85,11 @@
     (ssl-utils/obj->pem! csr csr-path)
     key-pair))
 
+(defn delete-all-csrs
+  []
+  (let [csr-path (str bootstrap/server-conf-dir "/ca/requests/")]
+    (ks-file/delete-recursively csr-path)))
+
 (defn generate-and-sign-a-cert!
   [certname]
   (let [cert-path (str bootstrap/server-conf-dir "/ssl/certs/localhost.pem")
@@ -1207,6 +1213,8 @@
           (is (.contains body error-msg)))))))
 
 (deftest ca-bulk-signing-all-endpoint-test
+  ;; ensure the csr directory is empty as other tests leave cruft behind
+  (delete-all-csrs)
   (testing "returns 200 response"
     (bootstrap/with-puppetserver-running-with-mock-jrubies
       "JRuby mocking is safe here because all of the requests are to the CA
@@ -1219,14 +1227,25 @@
         :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")}}
-      (let [response (http-client/post
+      (testing "returns 200 with valid payload"
+        ;; note- more extensive testing of the behavior is done with the testing in sign-multiple-certificate-signing-requests!-test
+        (let [certname (ks/rand-str :alpha-lower 16)
+              certname-with-bad-extension (ks/rand-str :alpha-lower 16)
+              _ (generate-a-csr certname [] [])
+              _ (generate-a-csr certname-with-bad-extension [{:oid "1.9.9.9.9.9.0" :value "true" :critical false}] [])
+              response (http-client/post
                       "https://localhost:8140/puppet-ca/v1/sign/all"
                       {:ssl-cert (str bootstrap/server-conf-dir "/ca/ca_crt.pem")
                        :ssl-key (str bootstrap/server-conf-dir "/ca/ca_key.pem")
                        :ssl-ca-cert (str bootstrap/server-conf-dir "/ca/ca_crt.pem")
                        :as :text
                        :headers {"Accept" "application/json"}})]
-        (is (= 200 (:status response)))))))
+          (is (= 200 (:status response)))
+          (is (= {:signed [certname]
+                  ;; this would represent any files that are removed between when the set is collected, and when they are processed.
+                  :no-csr []
+                  :signing-errors [certname-with-bad-extension]}
+                 (json/parse-string (:body response) true))))))))
 
 (deftest ca-certificate-renew-endpoint-test
   (testing "with the feature enabled"

test/unit/puppetlabs/puppetserver/certificate_authority_test.clj

@@ -9,6 +9,7 @@
             [puppetlabs.kitchensink.core :as ks]
             [puppetlabs.kitchensink.file :as ks-file]
             [puppetlabs.puppetserver.certificate-authority :as ca]
+            [puppetlabs.puppetserver.common :as common]
             [puppetlabs.services.ca.ca-testutils :as testutils]
             [puppetlabs.services.jruby.jruby-puppet-testutils :as jruby-testutils]
             [puppetlabs.ssl-utils.core :as utils]
@@ -19,8 +20,10 @@
   (:import (com.puppetlabs.ssl_utils SSLUtils)
            (java.io ByteArrayInputStream
                     ByteArrayOutputStream
-                    StringReader
+                    File StringReader
                     StringWriter)
+           (java.nio.file Files Path)
+           (java.nio.file.attribute FileAttribute)
            (java.security MessageDigest PublicKey)
            (java.security.cert X509CRL X509Certificate)
            (java.time LocalDateTime ZoneOffset)
@@ -2384,4 +2387,31 @@
           (testing "all of the unauthorized names should be in the not-signed"
             (is (= unauthorized-set (clojure.set/intersection unsigned-set unauthorized-set))))
           (testing "all of the unapproved names should be in the not-signed"
-            (is (= unapproved-extensions-set (clojure.set/intersection unsigned-set unapproved-extensions-set)))))))))
+            (is (= unapproved-extensions-set (clojure.set/intersection unsigned-set unapproved-extensions-set)))))))))
+
+(def default-permissions
+  (into-array FileAttribute [(ks-file/perms->attribute "rw-------")]))
+
+(deftest get-paths-to-all-certificate-requests-test
+  (testing "finds all files in directory ending with the pem suffix"
+    (let [^File temp-directory (ks/temp-dir)
+          path-to-file (.toPath temp-directory)
+          a-pem-file-names (set (for [i (range 0 100)]
+                                  (format "a-%d.pem" i)))
+          b-pem-file-names (set (for [i (range 0 100)]
+                                  (format "b-%d.pem" i)))
+          a-foo-file-names  (set (for [i (range 0 100)]
+                                   (format "a-%d.foo" i)))
+          all-pem-file-names (clojure.set/union a-pem-file-names b-pem-file-names)]
+
+      ;; create a lot of files that match that end with pem
+      (doall
+        (for [^String i all-pem-file-names]
+          (Files/createFile (.resolve ^Path path-to-file i) default-permissions)))
+      ;; create a lot of files that don't end that start with pem
+      (doall
+        (for [^String i a-foo-file-names]
+          (Files/createFile (.resolve path-to-file i) default-permissions)))
+      (let [result (ca/get-paths-to-all-certificate-requests (.toString temp-directory))
+            file-names (set (common/extract-file-names-from-paths result))]
+        (is (= (set file-names) all-pem-file-names))))))