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.

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

(PE-37347) add bulk signing function #2801

Merged

Conversation

jonathannewman
Copy link
Contributor

This adds a new function that given a set of csr names, the CA settings and a function to report activity will attempt to sign all the csrs that are valid. The function returns a map that contains the names of the csrs that were signed, and those that weren't signed.

The function validates alt-names, authorization-extensions, and signatures.

Locking is done across the group to prevent lock contention. In local testing, it took ~5 seconds to process ~500 records, of which 100 were valid.

@jonathannewman jonathannewman requested a review from a team as a code owner December 21, 2023 20:07
@jonathannewman jonathannewman force-pushed the PE-37347/add-bulk-signing-function branch from d440a2e to d8efdb4 Compare December 21, 2023 20:10
@@ -1,34 +1,34 @@
(ns puppetlabs.puppetserver.certificate-authority
(:import (java.io BufferedReader BufferedWriter FileNotFoundException InputStream ByteArrayOutputStream ByteArrayInputStream File Reader StringReader IOException)
(:require [clj-time.coerce :as time-coerce]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ran the cursive "organize imports" to clean this up.

@@ -718,6 +718,29 @@
(log/trace (i18n/trs "Finish append to serial file. ")))]
(ks-file/atomic-write infra-node-serials-path stream-content-fn)))))))

(defn stream-content-to-file
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this out of "write-cert-to-inventory!" for reuse.

(maybe-write-to-infra-serial! serial-number cert-name settings))))

(schema/defn ^:always-validate
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above function, but assumes no locking needed

@@ -1,34 +1,34 @@
(ns puppetlabs.puppetserver.certificate-authority-test
(:import (java.io StringReader
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used Cursive to clean up imports.

@@ -2109,18 +2109,18 @@
subject (utils/cn "foo")]
(testing "should not support auto-renewal"
(is (false? (ca/supports-auto-renewal? (utils/generate-certificate-request keypair subject [] []))))
(is (false? (ca/supports-auto-renewal? (utils/generate-certificate-request keypair subject [] [{:oid "1.3.6.1.4.1.34380.1.3.2" :value false}]))))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a find/replace on that magic string with the constant definition in ca. Thought it was easier to read.

@@ -2251,3 +2251,131 @@
(doseq [i (range 1 10000)]
(is (= (biginteger i) (ca/base-16-str->biginteger (format "0x%x" i)))))))

(defn generate-csr
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Helper function to generate a csr for the tests.

(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)
_ (println "begin generate good csrs")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This takes a little while (a few seconds), so I left the print statements in here to make it easier to understand why it is delayed.

@jonathannewman jonathannewman force-pushed the PE-37347/add-bulk-signing-function branch 2 times, most recently from 5e7cc35 to a5ec471 Compare December 21, 2023 20:30
This adds a new function that given a set of csr names, the CA settings
and a function to report activity will attempt to sign all the csrs
that are valid.  The function returns a map that contains the names of
the csrs that were signed, and those that weren't signed.

The function validates alt-names, authorization-extensions, and signatures.

Locking is done across the group to prevent lock contention.  In local
testing, it took ~5 seconds to process ~500 records, of which 100 were valid.
@jonathannewman jonathannewman force-pushed the PE-37347/add-bulk-signing-function branch from a5ec471 to dc6b63e Compare December 21, 2023 20:32
Copy link
Contributor

@steveax steveax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@steveax steveax merged commit c9a0e6f into puppetlabs:main Dec 21, 2023
11 checks passed
@jonathannewman jonathannewman deleted the PE-37347/add-bulk-signing-function branch December 21, 2023 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants