Skip to content

Commit

Permalink
Merge pull request #2803 from jonathannewman/PE-37348/main/update-end…
Browse files Browse the repository at this point in the history 
…point-use-signing-function

(PE-37348) connect bulk signing behavior to endpoint
  • Loading branch information
@steveax
steveax authored Dec 22, 2023
2 parents 3855e8a + 5819957 commit 6708114
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 29 deletions.
14 changes: 8 additions & 6 deletions src/clj/puppetlabs/puppetserver/certificate_authority.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2422,7 +2422,7 @@
(log/info (i18n/trs "infra crl expiring within 30 days, updating."))
(update-and-sign-crl! infra-crl-path settings))))

(schema/defn maybe-sign-one :- (schema/enum :signed :not-signed)
(schema/defn maybe-sign-one :- (schema/enum :signed :signing-errors)
[subject :- schema/Str
csr-path :- schema/Str
cacert :- Certificate
Expand Down Expand Up @@ -2458,11 +2458,12 @@
(catch Throwable e
(log/debug e (i18n/trs "Failed in bulk signing for entry {0}" subject))
;; failure case, add the host to the set of not signed results
:not-signed)))
:signing-errors)))

(schema/defn ^:always-validate
sign-multiple-certificate-signing-requests! :- {:signed [schema/Str]
:not-signed [schema/Str]}
:no-csr [schema/Str]
:signing-errors [schema/Str]}
[subjects :- [schema/Str]
{:keys [cacert cakey csrdir
inventory-lock inventory-lock-timeout-seconds
Expand All @@ -2481,20 +2482,21 @@
;; loop through the subjects, one at a time, and collect the results for success or failure.
(loop [s subjects
result {:signed []
:not-signed []}]
:no-csr []
:signing-errors[]}]
(if-not (empty? s)
(let [subject (first s)
csr-path (path-to-cert-request csrdir subject)]
(if (fs/exists? csr-path)
(let [_ (log/trace (i18n/trs "File exists at {0}" csr-path))
one-result (maybe-sign-one subject csr-path cacert casubject ca-private-key ca-settings)]
;; one-result is either :signed or :not-signed
;; one-result is either :signed or :signing-errors
(recur (rest s)
(update result one-result conj subject)))
(do
(log/trace (i18n/trs "File does not exist at {0}" csr-path))
(recur (rest s)
(update result :not-signed conj subject)))))
(update result :no-csr conj subject)))))
result))]
;; submit the signing activity as one entry for all the hosts.
(when-not (empty? (:signed results))
Expand Down
7 changes: 4 additions & 3 deletions src/clj/puppetlabs/services/ca/certificate_authority_core.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,8 @@

(schema/defn handle-bulk-cert-signing
[request
_ca-settings :- ca/CaSettings]
ca-settings :- ca/CaSettings
report-activity]
(let [json-body (try-to-parse (:body request))
certnames (:certnames json-body)]
(if-let [schema-error (schema/check Certnames certnames)]
Expand All @@ -270,7 +271,7 @@
:error (str schema-error)}))
(rr/status 422)
(rr/content-type "application/json"))
(-> (rr/response (cheshire/generate-string {}))
(-> (rr/response (cheshire/generate-string (ca/sign-multiple-certificate-signing-requests! certnames ca-settings report-activity)))
(rr/status 200)
(rr/content-type "application/json")))))

Expand Down Expand Up @@ -566,7 +567,7 @@
(POST ["/certificate_renewal"] request
(handle-cert-renewal request ca-settings report-activity))
(POST ["/sign"] request
(handle-bulk-cert-signing request ca-settings))
(handle-bulk-cert-signing request ca-settings report-activity))
(POST ["/sign/all"] request
(handle-bulk-cert-signing-all request ca-settings)))
(comidi/not-found "Not Found")))
Expand Down
39 changes: 27 additions & 12 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 @@ -72,23 +72,29 @@
public-key
ca-exts)}))

(defn generate-a-csr
[certname extensions attributes]
(let [key-pair (ssl-utils/generate-key-pair)
csr (ssl-utils/generate-certificate-request
key-pair
(ssl-utils/cn certname)
extensions
attributes)
csr-path (str bootstrap/server-conf-dir "/ca/requests/" certname ".pem")]
(ssl-utils/obj->pem! csr csr-path)
key-pair))

(defn generate-and-sign-a-cert!
[certname]
(let [cert-path (str bootstrap/server-conf-dir "/ssl/certs/localhost.pem")
key-path (str bootstrap/server-conf-dir "/ssl/private_keys/localhost.pem")
ca-cert-path (str bootstrap/server-conf-dir "/ca/ca_crt.pem")
key-pair (ssl-utils/generate-key-pair)
csr (ssl-utils/generate-certificate-request
key-pair
(ssl-utils/cn certname))
csr-path (str bootstrap/server-conf-dir "/ca/requests/" certname ".pem")
status-url (str "https://localhost:8140/puppet-ca/v1/certificate_status/" certname)
cert-endpoint (str "https://localhost:8140/puppet-ca/v1/certificate/" certname)
request-opts {:ssl-cert cert-path
:ssl-key key-path
:ssl-ca-cert ca-cert-path}]

(ssl-utils/obj->pem! csr csr-path)
:ssl-ca-cert ca-cert-path}
key-pair (generate-a-csr certname [] [])]
(http-client/put
status-url
(merge request-opts
Expand Down Expand Up @@ -1166,17 +1172,26 @@
:ssl-ca-cert (str bootstrap/server-conf-dir "/ca/ca_crt.pem")
:ssl-crl-path (str bootstrap/server-conf-dir "/ssl/crl.pem")}}

(testing "returns 200 with valid payload"
(let [random-certname (ks/rand-str :alpha-lower 16)
(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-no-exist (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"
{:body (json/encode {:certnames [random-certname]})
{:body (json/encode {:certnames [certname certname-no-exist certname-with-bad-extension]})
: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]
:no-csr [certname-no-exist]
:signing-errors [certname-with-bad-extension]}
(json/parse-string (:body response) true)))))
(testing "throws schema violation for invalid certname"
(let [error-msg "{\"kind\":\"schema-violation\""
response (http-client/post
Expand Down
22 changes: 14 additions & 8 deletions test/unit/puppetlabs/puppetserver/certificate_authority_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2280,7 +2280,8 @@
(is (fs/exists? (:csr-path csr-info))))
(testing "correctly signs"
(is (= {:signed [(:subject-name csr-info)]
:not-signed []}
:no-csr []
:signing-errors []}
(ca/sign-multiple-certificate-signing-requests! [(:subject-name csr-info)] settings report-activity))))
(testing "csr is removed after routine"
(is (not (fs/exists? (:csr-path csr-info)))))
Expand Down Expand Up @@ -2315,25 +2316,29 @@
(testing "correctly rejects a non-existent csr"
(let [random-csr-name (ks/rand-str :alpha-digits 8)]
(is (= {:signed []
:not-signed [random-csr-name]}
:no-csr [random-csr-name]
:signing-errors []}
(ca/sign-multiple-certificate-signing-requests! [random-csr-name] settings report-activity)))))
(testing "correctly rejects a cert with Subject alternative names"
(let [alt-name-ext {:oid utils/subject-alt-name-oid
:value {:dns-name ["bad-name"]}
:critical false}
csr-info (generate-csr settings [alt-name-ext] [{:oid ca/pp_auth_auto_renew-attribute :value true}])]
(is (= {:signed []
:not-signed [(:subject-name csr-info)]}
:no-csr []
:signing-errors [(:subject-name csr-info)]}
(ca/sign-multiple-certificate-signing-requests! [(:subject-name csr-info)] settings report-activity)))))
(testing "correctly rejects a cert with authorization extensions when disabled"
(let [csr-info (generate-csr settings [{:oid ca/ppAuthCertExt :value "true" :critical false}] [{:oid ca/pp_auth_auto_renew-attribute :value true}])]
(is (= {:signed []
:not-signed [(:subject-name csr-info)]}
:no-csr []
:signing-errors [(:subject-name csr-info)]}
(ca/sign-multiple-certificate-signing-requests! [(:subject-name csr-info)] settings report-activity)))))
(testing "correctly rejects a cert with unapproved extensions"
(let [csr-info (generate-csr settings [{:oid "1.9.9.9.9.9.0" :value "true" :critical false}] [{:oid ca/pp_auth_auto_renew-attribute :value true}])]
(is (= {:signed []
:not-signed [(:subject-name csr-info)]}
:no-csr []
:signing-errors [(:subject-name csr-info)]}
(ca/sign-multiple-certificate-signing-requests! [(:subject-name csr-info)] settings report-activity))))))
(testing "multiple entry with both bad and good csrs"
(let [count-range (range 0 100)
Expand All @@ -2360,7 +2365,8 @@
all-names (shuffle (concat (map :subject-name all-csrs) random-csr-names))
result (ca/sign-multiple-certificate-signing-requests! all-names settings report-activity)
signed-set (set (:signed result))
unsigned-set (set (:not-signed result))
not-found-set (set (:no-csr result))
unsigned-set (set (:signing-errors result))
good-csrs-set (set (map :subject-name good-csrs))
random-names-set (set random-csr-names)
bad-names-set (set (map :subject-name bad-names))
Expand All @@ -2371,8 +2377,8 @@
signed-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-signed"
(is (= random-names-set (clojure.set/intersection unsigned-set random-names-set))))
(testing "all of the random names should be in the not-found-set"
(is (= random-names-set (clojure.set/intersection not-found-set random-names-set))))
(testing "all of the alt names should be in the not-signed"
(is (= bad-names-set (clojure.set/intersection unsigned-set bad-names-set))))
(testing "all of the unauthorized names should be in the not-signed"
Expand Down

0 comments on commit 6708114

Please sign in to comment.