Skip to content

Commit

Permalink
Merge pull request #2832 from jonathannewman/PE-37585/main/add-event-…
Browse files Browse the repository at this point in the history 
…submission-cert-signing

(PE-37585) add action tracking to certificate signing behaviors
  • Loading branch information
@steveax
steveax authored Feb 29, 2024
2 parents 7d3b62e + fc8bbe2 commit 0f36e96
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 37 deletions.
2 changes: 2 additions & 0 deletions src/clj/puppetlabs/puppetserver/certificate_authority.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1687,6 +1687,7 @@
(create-agent-extensions
csr
cacert))]
(common/record-action {:type :add :targets [subject] :meta {:type :certificate}})
(write-cert-to-inventory! signed-cert ca-settings)
(write-cert signed-cert (path-to-cert signeddir subject))
(delete-certificate-request! ca-settings subject)
Expand Down Expand Up @@ -2514,5 +2515,6 @@
result))]
;; submit the signing activity as one entry for all the hosts.
(when-not (empty? (:signed results))
(common/record-action {:type :add :targets (:signed results) :meta {:type :certificate}})
(report-activity (:signed results) "signed"))
results)))))
84 changes: 51 additions & 33 deletions .../integration/puppetlabs/services/certificate_authority/certificate_authority_int_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1193,6 +1193,10 @@
(let [certname (ks/rand-str :alpha-lower 16)
certname-no-exist (ks/rand-str :alpha-lower 16)
certname-with-bad-extension (ks/rand-str :alpha-lower 16)
call-results (atom [])
old-fn @common/action-registration-function
new-fn (fn [value] (swap! call-results conj value))
_ (reset! common/action-registration-function new-fn)
_ (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
Expand All @@ -1207,7 +1211,12 @@
(is (= {:signed [certname]
:no-csr [certname-no-exist]
:signing-errors [certname-with-bad-extension]}
(json/parse-string (:body response) true)))))
(json/parse-string (:body response) true)))
(is (= [{:type :add
:targets [certname]
:meta {:type :certificate}}]
@call-results))
(reset! common/action-registration-function old-fn)))
(testing "throws schema violation for invalid certname"
(let [error-msg "{\"kind\":\"schema-violation\""
response (http-client/post
Expand Down Expand Up @@ -1237,38 +1246,47 @@
: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")}}
(testing "PE-37634 PE-sign-all with no pending certs returns 200 with expected payload"
(let [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 (= {:signed []
:no-csr []
:signing-errors []}
(json/parse-string (:body response) true)))))
(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 (= {: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))))))))
(let [old-fn @common/action-registration-function
call-results (atom [])
new-fn (fn [value] (swap! call-results conj value))]
(reset! common/action-registration-function new-fn)
(testing "PE-37634 PE-sign-all with no pending certs returns 200 with expected payload"
(let [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 (= {:signed []
:no-csr []
:signing-errors []}
(json/parse-string (:body response) true)))
(is (= [] @call-results))))
(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 (= {: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)))
(is (= [{:type :add
:targets [certname]
:meta {:type :certificate}}] @call-results))))
(reset! common/action-registration-function old-fn)))))

(deftest ca-certificate-renew-endpoint-test
(testing "with the feature enabled"
Expand Down
31 changes: 27 additions & 4 deletions test/unit/puppetlabs/puppetserver/certificate_authority_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,11 @@
csr (-> (:csrdir settings)
(ca/path-to-cert-request "test-agent")
(utils/pem->csr))
expected-cert-path (ca/path-to-cert (:signeddir settings) "test-agent")]
expected-cert-path (ca/path-to-cert (:signeddir settings) "test-agent")
old-fn @common/action-registration-function
call-results (atom [])
new-fn (fn [value] (swap! call-results conj value))]
(reset! common/action-registration-function new-fn)
;; Fix the value of "now" so we can reliably test the dates
(time/do-at now
(ca/autosign-certificate-request! "test-agent" csr settings (constantly nil)))
Expand All @@ -469,7 +473,14 @@
(testing "not-before is 1 day before now"
(is (= (time/minus now (time/days 1)) not-before)))
(testing "not-after is 2 years from now"
(is (= (time/plus now (time/years 2)) not-after))))))))
(is (= (time/plus now (time/years 2)) not-after)))))

(testing "correctly reports node activity"
(is (= [{:type :add,
:targets ["test-agent"],
:meta {:type :certificate}}]
@call-results)))
(reset! common/action-registration-function old-fn))))

(deftest autosign-without-capub
(testing "The CA public key file is not necessary to autosign"
Expand Down Expand Up @@ -1320,7 +1331,10 @@
(ca/process-csr-submission! subject-name csr settings (constantly nil))
(is (= [{:type :info
:targets [subject-name]
:meta {:what :csr :action :submit}}]
:meta {:what :csr :action :submit}}
{:type :add
:targets [subject-name]
:meta {:type :certificate}}]
@call-results))
(reset! common/action-registration-function old-fn)))))

Expand Down Expand Up @@ -2385,6 +2399,10 @@
all-csrs (concat good-csrs bad-names unauthorized unapproved-extensions)
_ (println "add in bad names and shuffle")
all-names (shuffle (concat (map :subject-name all-csrs) random-csr-names))
old-fn @common/action-registration-function
call-results (atom [])
new-fn (fn [value] (swap! call-results conj value))
_ (reset! common/action-registration-function new-fn)
result (ca/sign-multiple-certificate-signing-requests! all-names settings report-activity)
signed-set (set (:signed result))
not-found-set (set (:no-csr result))
Expand All @@ -2397,6 +2415,10 @@
(testing "all the signed entries should be present"
(is (= good-csrs-set
signed-set))
(is (= 1 (count @call-results)))
(is (= {:type :add :targets good-csrs-set :meta {:type :certificate}}
;; convert the targets to a set for comparison
(update-in (first @call-results) [:targets] set)))
(testing "none of the valid csrs should be in the not-signed set"
(is (empty? (clojure.set/intersection unsigned-set good-csrs-set))))
(testing "all of the random names should be in the not-found-set"
Expand All @@ -2406,7 +2428,8 @@
(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)))))
(reset! common/action-registration-function old-fn)))))

(def default-permissions
(into-array FileAttribute [(ks-file/perms->attribute "rw-------")]))
Expand Down

0 comments on commit 0f36e96

Please sign in to comment.