Skip to content

Commit

Permalink
(PE-36952) ensure all infrastructure serials are written
Browse files Browse the repository at this point in the history 
This adds a new behavior such that when a renewal or signing is done
and the host in question is in the infrastructure list, the serial number
for that signed cert is immediately written to the infra-serial file.

This has the benefit of ensuring that all certs will be correctly revoked in
the infra-crl when it is updated on revocation.
  • Loading branch information
@jonathannewman
jonathannewman committed Oct 5, 2023
1 parent bfbd43f commit 82305eb
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 19 deletions.
73 changes: 56 additions & 17 deletions src/clj/puppetlabs/puppetserver/certificate_authority.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,47 @@

(def buffer-copy-size (* 64 1024))

(schema/defn read-infra-nodes
"Returns a list of infra nodes or infra node serials from the specified file organized as one item per line."
[infra-file-reader :- Reader]
(line-seq infra-file-reader))

(schema/defn maybe-write-to-infra-serial!
"Determine if the host in question is an infra host, and if it is, add the provided serial number to the
infra-serials file"
[serial :- BigInteger
certname :- schema/Str
{:keys [infra-nodes-path infra-node-serials-path]} :- CaSettings]
(when (fs/exists? infra-nodes-path)
(with-open [infra-nodes-reader (io/reader infra-nodes-path)]
(let [infra-nodes (read-infra-nodes infra-nodes-reader)]
(when (seq (filter #(= certname %) infra-nodes))
(log/debug (i18n/trs "Appending serial number {0} for {1} " serial certname))
(let [stream-content-fn
(fn [^BufferedWriter writer]
(log/trace (i18n/trs "Begin append to serial file."))
(let [copy-buffer (CharBuffer/allocate buffer-copy-size)]
(try
(with-open [^BufferedReader reader (io/reader infra-node-serials-path)]
;; copy all the existing content
(loop [read-length (.read reader copy-buffer)]
;; theoretically read can return 0, which means try again
(when (<= 0 read-length)
(when (pos? read-length)
(.write writer (.array copy-buffer) 0 read-length))
(.clear copy-buffer)
(recur (.read reader copy-buffer)))))
(catch FileNotFoundException _e
(log/trace (i18n/trs "Inventory serial file not found. Assume empty.")))
(catch Throwable e
(log/error e (i18n/trs "Error while appending to serial file."))
(throw e))))
(.write writer (str serial))
(.newLine writer)
(.flush writer)
(log/trace (i18n/trs "Finish append to serial file. ")))]
(ks-file/atomic-write infra-node-serials-path stream-content-fn)))))))

(schema/defn ^:always-validate
write-cert-to-inventory!
"Writes an entry into Puppet's inventory file for a given certificate.
Expand All @@ -688,31 +729,33 @@
* $NA = The 'not after' field of the cert, as a date/timestamp in UTC.
* $S = The distinguished name of the cert's subject."
[cert :- Certificate
{:keys [cert-inventory inventory-lock inventory-lock-timeout-seconds]} :- CaSettings]
(let [serial-number (->> cert
(.getSerialNumber)
(format-serial-number)
(str "0x"))
{:keys [cert-inventory inventory-lock inventory-lock-timeout-seconds] :as settings} :- CaSettings]
(let [serial-number (.getSerialNumber cert)
formatted-serial-number (->> serial-number
(format-serial-number)
(str "0x"))
not-before (-> cert
(.getNotBefore)
(format-date-time))
not-after (-> cert
(.getNotAfter)
(format-date-time))
subject (utils/get-subject-from-x509-certificate cert)
entry (str serial-number " " not-before " " not-after " /" subject "\n")
cert-name (utils/x500-name->CN subject)
entry (str formatted-serial-number " " not-before " " not-after " /" subject "\n")
stream-content-fn (fn [^BufferedWriter writer]
(log/trace (i18n/trs "Begin append to inventory file."))
(let [copy-buffer (CharBuffer/allocate buffer-copy-size)]
(try
(with-open [^BufferedReader reader (io/reader cert-inventory)]
;; copy all the existing content
(loop [read-length (.read reader copy-buffer)]
(when (< 0 read-length)
(when (pos? read-length)
(.write writer (.array copy-buffer) 0 read-length))
(.clear copy-buffer)
(recur (.read reader copy-buffer)))))
;; theoretically read can return 0, which means try again
(when (<= 0 read-length)
(when (pos? read-length)
(.write writer (.array copy-buffer) 0 read-length))
(.clear copy-buffer)
(recur (.read reader copy-buffer)))))
(catch FileNotFoundException _e
(log/trace (i18n/trs "Inventory file not found. Assume empty.")))
(catch Throwable e
Expand All @@ -723,7 +766,8 @@
(log/trace (i18n/trs "Finish append to inventory file. ")))]
(common/with-safe-write-lock inventory-lock inventory-lock-descriptor inventory-lock-timeout-seconds
(log/debug (i18n/trs "Append \"{1}\" to inventory file {0}" cert-inventory entry))
(ks-file/atomic-write cert-inventory stream-content-fn))))
(ks-file/atomic-write cert-inventory stream-content-fn))
(maybe-write-to-infra-serial! serial-number cert-name settings)))

(schema/defn is-subject-in-inventory-row? :- schema/Bool
[cn-subject :- utils/ValidX500Name
Expand Down Expand Up @@ -841,11 +885,6 @@
netscape-comment-value)
(utils/create-ca-extensions issuer-public-key ca-public-key))))

(schema/defn read-infra-nodes
"Returns a list of infra nodes or infra node serials from the specified file organized as one item per line."
[infra-file-reader :- Reader]
(line-seq infra-file-reader))

(schema/defn extract-active-infra-serials :- [BigInteger]
"Read the infra nodes file to determine which nodes are infrastructure nodes.
For each node, check the inventory file for any serial numbers for that node,
Expand Down
69 changes: 68 additions & 1 deletion test/unit/puppetlabs/puppetserver/certificate_authority_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2033,7 +2033,74 @@
;; there are four fields, serial number, not before, not after, and subject
;; for ease of testing, just test the first and last
(is (= "0x0006" (first last-entry-fields)))
(is (= "/CN=foo" (last last-entry-fields)))))))))
(is (= "/CN=foo" (last last-entry-fields)))))
(testing "the new entry is not in the infra-serial file"
(is (= "" (slurp (:infra-node-serials-path settings))))))))
(testing "infra inventory correctly writes files"
(let [settings (testutils/ca-sandbox! cadir)
;; auto-renewal-cert-ttl is expected to be an int
;; unit tests skip some of the conversion flow so
;; transform the duration here
converted-auto-renewal-cert-ttl (ca/duration-str->sec (:auto-renewal-cert-ttl settings))
updated-settings (assoc settings :auto-renewal-cert-ttl converted-auto-renewal-cert-ttl)
;; simulate the node being in the infra inventory file
_ (spit (:infra-nodes-path settings) "bar\n")
ca-cert (create-ca-cert "ca1" 1)
keypair (utils/generate-key-pair)
subject (utils/cn "bar")
csr (utils/generate-certificate-request keypair subject)
validity (ca/cert-validity-dates 3600)
signed-cert (utils/sign-certificate
(utils/get-subject-from-x509-certificate (:cert ca-cert))
(:private-key ca-cert)
(ca/next-serial-number! settings)
(:not-before validity)
(:not-after validity)
subject
(utils/get-public-key csr)
(ca/create-agent-extensions csr (:cert ca-cert)))
expected-cert-path (ca/path-to-cert (:signeddir settings) "bar")]
(testing "simulate the cert being written"
(ca/write-cert signed-cert expected-cert-path)
(is (fs/exists? expected-cert-path)))
(Thread/sleep 1000) ;; ensure there is some time elapsed between the two
(let [renewed-cert (ca/renew-certificate! signed-cert updated-settings (constantly nil))]
(is (some? renewed-cert))
(testing "serial number has increased"
(is (< (.getSerialNumber signed-cert) (.getSerialNumber renewed-cert)))
(is (= 6 (.getSerialNumber renewed-cert))))
(testing "not before time stamps have changed"
(is (= -1 (.compareTo (.getNotBefore signed-cert) (.getNotBefore renewed-cert)))))
(testing "new not-after is later than before"
(is (= -1 (.compareTo (.getNotAfter signed-cert) (.getNotAfter renewed-cert)))))
(testing "new not-after should be 89 days (and some faction) away"
(let [diff (- (.getTime (.getNotAfter renewed-cert)) (.getTime (Date.)))
days (.convert TimeUnit/DAYS diff TimeUnit/MILLISECONDS)]
(is (= 89 days))))
(testing "certificate should have been replaced"
(is (fs/exists? expected-cert-path))
(testing "updated cert on disk matches renewed cert"
(let [updated-cert (utils/pem->cert expected-cert-path)]
(is (= 6 (.getSerialNumber updated-cert)))
(is (zero? (.compareTo (.getNotBefore updated-cert) (.getNotBefore renewed-cert))))
(is (zero? (.compareTo (.getNotAfter updated-cert) (.getNotAfter renewed-cert)))))))
(testing "extensions are preserved"
(let [extensions-before (utils/get-extensions signed-cert)
extensions-after (utils/get-extensions signed-cert)]
;; ordering may be different so use an unordered comparison
(is (= (set extensions-before)
(set extensions-after)))))
(testing "the new entry is written to the inventory file"
(let [entries (string/split (slurp (:cert-inventory settings)) #"\n")
last-entry-fields (string/split (last entries) #" ")]
;; since the content of the inventory is well established (because of the sandbox), we can
;; just assert that the last entry is there, and makes sense
;; there are four fields, serial number, not before, not after, and subject
;; for ease of testing, just test the first and last
(is (= "0x0006" (first last-entry-fields)))
(is (= "/CN=bar" (last last-entry-fields)))))
(testing "the new entry is in the infra-serial file"
(is (= "6\n" (slurp (:infra-node-serials-path settings)))))))))
(deftest supports-auto-renewal?-test
(let [keypair (utils/generate-key-pair)
subject (utils/cn "foo")]
Expand Down
2 changes: 1 addition & 1 deletion test/unit/puppetlabs/services/ca/ca_testutils.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
:serial (str cadir "/serial")
:ruby-load-path jruby-testutils/ruby-load-path
:gem-path jruby-testutils/gem-path
:infra-nodes-path (str cadir "/ca/infra_inventory.txt")
:infra-nodes-path (str cadir "/infra_inventory.txt")
:infra-node-serials-path (str cadir "/infra_serials")
:infra-crl-path (str cadir "/infra_crl.pem")
:enable-infra-crl false
Expand Down

0 comments on commit 82305eb

Please sign in to comment.