From b4dc55ee6f9e22e6558fe72a0c350b2c3a771f66 Mon Sep 17 00:00:00 2001 From: Fredrik Axelsson Date: Tue, 19 Nov 2024 20:07:39 +0100 Subject: [PATCH] Make x509-swoop more restrictive (#43) 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. --- x509-mode-tests.el | 41 ++++++++++++++++++++++++----------------- x509-mode.el | 30 ++++++++++++++++++++---------- 2 files changed, 44 insertions(+), 27 deletions(-) diff --git a/x509-mode-tests.el b/x509-mode-tests.el index 0f29797..f17ff64 100644 --- a/x509-mode-tests.el +++ b/x509-mode-tests.el @@ -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))) @@ -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." @@ -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))))) diff --git a/x509-mode.el b/x509-mode.el index e97a269..2ba1b66 100644 --- a/x509-mode.el +++ b/x509-mode.el @@ -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 @@ -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)) @@ -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) @@ -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 () @@ -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)