Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(PE-37348) connect bulk signing behavior to endpoint #2803

Merged
merged 2 commits into from
Dec 22, 2023
Merged

(PE-37348) connect bulk signing behavior to endpoint #2803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a1eaf35
(PE-37348) update signing endpoint results to match API expectations
jonathannewman Dec 21, 2023
5819957
(PE-37348) enable /sign endpoint
jonathannewman Dec 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions src/clj/puppetlabs/puppetserver/certificate_authority.clj
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading