Skip to content

Commit

Permalink
(PE-34953) add sign/all endpoint
Browse files Browse the repository at this point in the history
This adds the CA "sign/all" endpoint that signs any CSRs that are
found in the directory, and comply with the rules for CSR signing.

It leverages the `sign-multiple-certificate-signing-requests!` which
has heavy testing in itself, so a basic smoke test is provided.
  • Loading branch information
jonathannewman committed Jan 2, 2024
1 parent 8475e17 commit ca295e0
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 14 deletions.
14 changes: 13 additions & 1 deletion src/clj/puppetlabs/puppetserver/certificate_authority.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
21 changes: 19 additions & 2 deletions src/clj/puppetlabs/puppetserver/common.clj
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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)))
17 changes: 10 additions & 7 deletions src/clj/puppetlabs/services/ca/certificate_authority_core.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand All @@ -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"
Expand Down
34 changes: 32 additions & 2 deletions test/unit/puppetlabs/puppetserver/certificate_authority_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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)
Expand Down Expand Up @@ -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))))))

0 comments on commit ca295e0

Please sign in to comment.