Skip to content

Commit

Permalink
Make x509-swoop more restrictive (#43)
Browse files Browse the repository at this point in the history
Bogus PEM-regions could case x509-dwim to send whole buffer to view functions. If the first thing in the file happened to be, for example, a certificate, then that would be output to the result buffer. Causing copies of the same cert several times.
  • Loading branch information
jobbflykt authored Nov 19, 2024
1 parent b30bc5e commit b4dc55e
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 27 deletions.
41 changes: 24 additions & 17 deletions x509-mode-tests.el
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ Repeat with `x509-dwim' which should produce the same result."
test-files
(list test-files))))
(dolist (test-file files)
(dolist (view-func (list view-command 'x509-dwim))
(dolist (view-func (list view-command #'x509-dwim))
(with-temp-buffer
(insert-file-contents-literally (find-testfile test-file))
(let ((view-buffer (funcall view-func)))
Expand All @@ -426,19 +426,18 @@ Repeat with `x509-dwim' which should produce the same result."

(ert-deftest x509-viewcert ()
"View cert."
(view-test-helper
'("CA/pki/crt/jobbflykt.crt" "CA/pki/crt/jobbflykt.cer")
'x509-viewcert
'x509-mode
"Certificate:"
"Warning"))
(view-test-helper '("CA/pki/crt/jobbflykt.crt" "CA/pki/crt/jobbflykt.cer")
'x509-viewcert
'x509-mode
"Certificate:"
"Warning"))

(ert-deftest x509-viewreq ()
"View cert request."
(view-test-helper
'("CA/ca/request/jobbflykt.pem"
"CA/ca/request/jobbflykt_req.der")
'x509-viewreq 'x509-mode "Certificate Request:" "Warning"))
(view-test-helper '("CA/ca/request/jobbflykt.pem"
"CA/ca/request/jobbflykt_req.der")
'x509-viewreq 'x509-mode "Certificate Request:"
"Warning"))

(ert-deftest x509-viewcrl ()
"View CRL."
Expand Down Expand Up @@ -774,12 +773,20 @@ SEQUENCE 30 0C
(let ((swoop-buffer (x509-swoop)))
(should swoop-buffer)
(with-current-buffer swoop-buffer
(check-content-helper swoop-buffer "Certificate:")
(check-content-helper swoop-buffer "Certificate Request:")
(check-content-helper swoop-buffer "DH Parameters:")
(check-content-helper swoop-buffer "Public-Key:")
(check-content-helper swoop-buffer "EC-Parameters: (512 bit)")
(check-content-helper swoop-buffer x509-swoop-separator)
(should (re-search-forward "Certificate:" nil t))
(should (re-search-forward x509-swoop-separator nil t))
;; Should _not_ find another certificate. This used to be a bug where
;; dwim would end up sending the whole buffer, producing the first
;; certificate again.
(should-not (re-search-forward "Certificate:" nil t))
(should (re-search-forward "Certificate Request:" nil t))
(should (re-search-forward x509-swoop-separator nil t))
(should (re-search-forward "DH Parameters:"))
(should (re-search-forward x509-swoop-separator nil t))
(should (re-search-forward "Public-Key:"))
(should (re-search-forward x509-swoop-separator nil t))
(should (re-search-forward "EC-Parameters: (512 bit)"))
(should-not (re-search-forward x509-swoop-separator nil t))
(x509-mode-kill-buffer)))
(with-current-buffer src-buffer
(should (= (point) 1322)))))
Expand Down
30 changes: 20 additions & 10 deletions x509-mode.el
Original file line number Diff line number Diff line change
Expand Up @@ -1009,8 +1009,9 @@ Intended to be called in a `x509-mode' or `x509-asn1-mode' buffer."
(or x509--x509-asn1-mode-shadow-arguments
x509-asn1parse-default-arg)
'x509--viewasn1-history 'x509-asn1-mode)
;; Else determine type and do x509-mode
(x509-dwim))))))))
;; Else determine type and do x509-mode. Only consider PEM
;; regions.
(x509--do-dwim t))))))))

;; ---------------------------------------------------------------------------
;;;###autoload
Expand Down Expand Up @@ -1048,16 +1049,15 @@ if the buffer contains data of certain type."
(apply #'call-process-region proc-args)))
(kill-buffer in-buf))))

;; ---------------------------------------------------------------------------
;;;###autoload
(defun x509-dwim ()
(defun x509--do-dwim (only-pem-region)
"Guess the type of object and call the corresponding view-function.
Look at -----BEGIN header for known object types. Then test
different openssl commands until one succeeds. Call
`x509-viewasn1' as a last resort.
Return the output buffer."
(interactive)
different openssl commands until one succeeds.
If ONLY-PEM-REGION is nil, call `x509-viewasn1' as a last resort.
If ONLY-PEM-REGION it t, only consider known PEM regions, i.e. don't send
buffer content to `x509--dwim-tester'.
Return the output buffer or nil"
(pcase (x509--pem-region-type)
((or "CERTIFICATE" "TRUSTED CERTIFICATE")
(call-interactively #'x509-viewcert))
Expand All @@ -1072,6 +1072,8 @@ Return the output buffer."
("X509 CRL" (call-interactively #'x509-viewcrl))
(_
(cond
(only-pem-region
nil)
((x509--dwim-tester x509-x509-default-arg)
(call-interactively #'x509-viewcert))
((x509--dwim-tester x509-crl-default-arg)
Expand All @@ -1091,6 +1093,13 @@ Return the output buffer."
(t
(call-interactively #'x509-viewasn1))))))

;; ---------------------------------------------------------------------------
;;;###autoload
(defun x509-dwim ()
"Guess the type of object and call the corresponding view-function."
(interactive)
(x509--do-dwim nil))

;; ---------------------------------------------------------------------------
;;;###autoload
(defun x509-swoop ()
Expand All @@ -1110,7 +1119,8 @@ Return view buffer on success."
(goto-char (point-min))
(while (re-search-forward "-----BEGIN" nil t)
(goto-char (match-beginning 0))
(when-let* ((tmp-output-buffer (x509-dwim)))
;; Call x509--do-dwim. Only consider valid PEM-regions.
(when-let* ((tmp-output-buffer (x509--do-dwim t)))
(with-current-buffer tmp-output-buffer
;; Only consider x509-mode output. We don't want asn1.
(if (eq major-mode 'x509-mode)
Expand Down

0 comments on commit b4dc55e

Please sign in to comment.