diff options
author | Stefan Kangas <stefan@marxist.se> | 2020-09-07 07:31:56 +0200 |
---|---|---|
committer | Stefan Kangas <stefan@marxist.se> | 2020-11-22 00:38:35 +0100 |
commit | bcde5f86c5a7f3a84115807520631a4f12fb6f70 (patch) | |
tree | 6254f8d19ef474f965567ce7922b84ed93312a19 | |
parent | 733e674af4f66ba7e9f0614b931c44484acce2b9 (diff) | |
download | emacs-scratch/package-security.tar.gz |
Support expiration of metadata by package archivesscratch/package-security
Expiring package metadata is done by checking the timestamp in package
archive file. This is intended to limit the effectiveness of a replay
attack. The onus is on the package archives to implement a secure and
reasonable policy. (Debian uses 7 days before metadata expires.)
Together with package checksums, this adds sufficient protection
against metadata replay attacks. (Bug#19479)
* lisp/emacs-lisp/package.el (package-check-timestamp): New defcustom.
(bad-timestamp): New error.
(package--parse-header-from-buffer)
(package--parse-valid-until-from-buffer)
(package--parse-last-updated-from-buffer)
(package--archive-verify-timestamp)
(package--archive-verify-not-expired)
(package--compare-archive-timestamps)
(package--check-archive-timestamp): New defuns.
(package--download-one-archive): Check timestamp of the
'archive-contents' file using above functions. It is only checked if
it exists, which makes this change backwards compatible.
* lisp/calendar/iso8601.el (iso8601-parse): Add autoload cookie.
* test/lisp/emacs-lisp/package-tests.el
(package-test-parse-valid-until-from-buffer)
(package-test-parse-last-updated-from-buffer)
(package-test-archive-verify-timestamp)
(package-test-check-archive-timestamp)
(package-test-check-archive-timestamp/not-expired)
(package-test-check-archive-timestamp/expired): New tests.
* test/lisp/emacs-lisp/package-resources/archives/older/archive-contents:
* test/lisp/emacs-lisp/package-resources/archives/newer/archive-contents:
New files.
* doc/lispref/package.texi (Package Archives, Archive Web Server):
Document how to increase the security of a package archive using
checksums, signing and timestamps.
-rw-r--r-- | doc/lispref/package.texi | 23 | ||||
-rw-r--r-- | etc/NEWS | 5 | ||||
-rw-r--r-- | lisp/calendar/iso8601.el | 1 | ||||
-rw-r--r-- | lisp/emacs-lisp/package.el | 105 | ||||
-rw-r--r-- | test/lisp/emacs-lisp/package-resources/archives/newer/archive-contents | 1 | ||||
-rw-r--r-- | test/lisp/emacs-lisp/package-resources/archives/older/archive-contents | 1 | ||||
-rw-r--r-- | test/lisp/emacs-lisp/package-tests.el | 61 |
7 files changed, 193 insertions, 4 deletions
diff --git a/doc/lispref/package.texi b/doc/lispref/package.texi index af87479c7d2..725fecd8952 100644 --- a/doc/lispref/package.texi +++ b/doc/lispref/package.texi @@ -332,10 +332,22 @@ installing user. (This is true for Emacs code in general, not just for packages.) So you should ensure that your archive is well-maintained and keep the hosting system secure. - One way to increase the security of your packages is to @dfn{sign} -them using a cryptographic key. If you have generated a -private/public gpg key pair, you can use gpg to sign the package like -this: + To increase the security of your packages, you should distribute +package checksums in the package metadata file +@file{archive-contents}. You should also @dfn{sign} the package +metadata file using a cryptographic key. Finally, it is important to +include creation and expiration timestamps information in that file. + + Signing individual packages is also supported, but considered +obsolete. It provides less security than package checksums, signing +the @file{archive-contents} file, and creation and expiration +timestamps does when used together. More specifically, signing +individual packages does not protect against ``replay attacks''. Note +that distributing signatures for individual packages is still +recommended to support Emacs versions older than 28.1. + + If you have generated a private/public gpg key pair, you can use gpg +to sign a package or the @file{archive-contents} file like this: @c FIXME EasyPG / package-x way to do this. @example @@ -371,6 +383,9 @@ Return a lisp form describing the archive contents. The form is a list of 'package-desc' structures (see @file{package.el}), except the first element of the list is the archive version. +@item archive-contents.sig +Return the signature for @file{archive-contents}. + @item <package name>-readme.txt Return the long description of the package. @@ -882,6 +882,11 @@ For improved security, you might want to set this to 't' or before setting these values, or you will be unable to install packages. +*** Support expiration of package archive metadata. +When a package archive distributes a last-updated and expiration +timestamp, they will automatically be used to verify that distributed +packages are not out of date. + ** gdb-mi +++ diff --git a/lisp/calendar/iso8601.el b/lisp/calendar/iso8601.el index 906c29b15f4..7a6f14a858f 100644 --- a/lisp/calendar/iso8601.el +++ b/lisp/calendar/iso8601.el @@ -114,6 +114,7 @@ iso8601--duration-week-match iso8601--duration-combined-match))) +;;;###autoload (defun iso8601-parse (string &optional form) "Parse an ISO 8601 date/time string and return a `decode-time' structure. diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el index 308f9eb3a63..1e73a1690cc 100644 --- a/lisp/emacs-lisp/package.el +++ b/lisp/emacs-lisp/package.el @@ -360,6 +360,15 @@ should normally not be used since it will decrease security." :risky t :version "28.1") +(defcustom package-check-timestamp t + "Non-nil means to verify the package archive timestamp. + +Note that setting this to nil is intended for debugging, and +should normally not be used since it will decrease security." + :type 'boolean + :risky t + :version "28.1") + (defcustom package-check-signature 'allow-unsigned "Non-nil means to check package signatures when installing. More specifically the value can be: @@ -449,6 +458,7 @@ synchronously." (define-error 'bad-size "Package size mismatch" 'package-error) (define-error 'bad-signature "Failed to verify signature" 'package-error) (define-error 'bad-checksum "Failed to verify checksum" 'package-error) +(define-error 'bad-timestamp "Failed to verify timestamp" 'package-error) ;;; `package-desc' object definition @@ -1812,6 +1822,100 @@ Once it's empty, run `package--post-download-archives-hook'." (message "Package refresh done") (run-hooks 'package--post-download-archives-hook))) +(defun package--parse-header-from-buffer (header name) + "Find and return \"archive-contents\" HEADER for archive NAME. +This function assumes that the current buffer contains the +\"archive-contents\" file. + +A valid header looks like: \";; HEADER: <TIMESTAMP>\" + +Where <TIMESTAMP> is a valid ISO-8601 (RFC 3339) date. If there +is such a line but <TIMESTAMP> is invalid, show a warning and +return nil. If there is no valid header, return nil." + (save-excursion + (goto-char (point-min)) + (when (re-search-forward (concat "^;; " header ": *\\(.+?\\) *$") nil t) + (condition-case-unless-debug nil + (encode-time (iso8601-parse (match-string 1))) + (lwarn '(package timestamp) + (list (format "Malformed timestamp for archive `%s': `%s'" + name (match-string 1)))))))) + +(defun package--parse-valid-until-from-buffer (name) + "Find and return \"Valid-Until\" header for archive NAME." + (package--parse-header-from-buffer "Valid-Until" name)) + +(defun package--parse-last-updated-from-buffer (name) + "Find and return \"Last-Updated\" header for archive NAME." + (package--parse-header-from-buffer "Last-Updated" name)) + +(defun package--archive-verify-timestamp (new old name) + "Return t if timestamp NEW is more recent than OLD for archive NAME. +Signal error otherwise. +Warn if NEW is in the future." + ;; If timestamp is missing on cached (old) file, do nothing here. + ;; This package archive recently introduced support for timestamps. + ;; We will require a timestamp for that archive in future updates. + (if old + (cond + ((not new) + (signal 'bad-timestamp + (list (format-message + (concat + "New archive contents for `%s' missing " + "timestamp, refusing to proceed") + name)))) + ((time-less-p new old) + (signal 'bad-timestamp + (list (format-message + (concat + "New archive contents for `%s' older than " + "cached, refusing to proceed") + name)))) + ((time-less-p (current-time) new) + (signal 'bad-timestamp + (list (format-message + (concat + "New archive contents for `%s' is " + "in the future: %s") + name (format-time-string "%c" new))))) + ;; Check ok, return t. + (t)) + t)) + +(defun package--archive-verify-not-expired (timestamp name) + "Return t if TIMESTAMP has not yet expired for archive NAME. +Signal error otherwise." + (unless (time-less-p (current-time) timestamp) + (signal 'bad-timestamp + (list (format-message + (concat + "Package archive `%s' has sent " + "an expired `archive-contents' file") + name))))) + +(defun package--check-archive-timestamp (name) + "Verify timestamp of \"archive-contents\" file for archive NAME. +Compare the archive timestamp of the previously downloaded +\"archive-contents\" file to the timestamp in the current buffer. +Signal error if the old timestamp is more recent than the new one. + +Do nothing if there is no previously downloaded file, if such a +file exists but does not contain any timestamp, or if +`package-check-timestamp' is nil." + (let ((old-file (expand-file-name + (concat "archives/" name "/archive-contents") + package-user-dir))) + (when (and package-check-timestamp + (file-readable-p old-file)) + (let ((old (with-temp-buffer + (insert-file-contents old-file) + (package--parse-last-updated-from-buffer name))) + (new (package--parse-last-updated-from-buffer name)) + (new-expires (package--parse-valid-until-from-buffer name))) + (package--archive-verify-timestamp new old name) + (package--archive-verify-not-expired new-expires name))))) + (defun package--download-one-archive (archive file &optional async) "Retrieve an archive file FILE from ARCHIVE, and cache it. ARCHIVE should be a cons cell of the form (NAME . LOCATION), @@ -1825,6 +1929,7 @@ similar to an entry in `package-alist'. Save the cached copy to (content (buffer-string)) (dir (expand-file-name (concat "archives/" name) package-user-dir)) (local-file (expand-file-name file dir))) + (package--check-archive-timestamp name) (when (listp (read content)) (make-directory dir t) (if (or (not (package-check-signature)) diff --git a/test/lisp/emacs-lisp/package-resources/archives/newer/archive-contents b/test/lisp/emacs-lisp/package-resources/archives/newer/archive-contents new file mode 100644 index 00000000000..59a79970b6b --- /dev/null +++ b/test/lisp/emacs-lisp/package-resources/archives/newer/archive-contents @@ -0,0 +1 @@ +;; Last-Updated: 2020-06-01T00:00:00.000Z diff --git a/test/lisp/emacs-lisp/package-resources/archives/older/archive-contents b/test/lisp/emacs-lisp/package-resources/archives/older/archive-contents new file mode 100644 index 00000000000..193a6b5ab94 --- /dev/null +++ b/test/lisp/emacs-lisp/package-resources/archives/older/archive-contents @@ -0,0 +1 @@ +;; Last-Updated: 2019-01-01T00:00:00.000Z diff --git a/test/lisp/emacs-lisp/package-tests.el b/test/lisp/emacs-lisp/package-tests.el index a81506d626b..b0da54a3015 100644 --- a/test/lisp/emacs-lisp/package-tests.el +++ b/test/lisp/emacs-lisp/package-tests.el @@ -857,6 +857,67 @@ If the rest succeed, just ignore the unsupported one." (insert "7") (should-error (package--verify-package-size pkg-desc))))) +(ert-deftest package-test-parse-valid-until-from-buffer () + (with-temp-buffer + (insert ";; Valid-Until: 2020-05-01T15:43:35.000Z\n(foo bar baz)") + (should (equal (package--parse-valid-until-from-buffer "foo") + '(24236 17319))))) + +(ert-deftest package-test-parse-last-updated-from-buffer () + (with-temp-buffer + (insert ";; Last-Updated: 2020-05-01T15:43:35.000Z\n(foo bar baz)") + (should (equal (package--parse-last-updated-from-buffer "foo") + '(24236 17319))))) + +(defun package-tests--parse-last-updated (timestamp) + (with-temp-buffer + (insert timestamp) + (package--parse-last-updated-from-buffer "test"))) + +(ert-deftest package-test-archive-verify-timestamp () + (let ((a (package-tests--parse-last-updated + ";; Last-Updated: 2020-05-01T15:43:35.000Z\n")) + (b (package-tests--parse-last-updated + ";; Last-Updated: 2020-06-01T15:43:35.000Z\n")) + (c (package-tests--parse-last-updated + ";; Last-Updated: 2020-07-01T15:43:35.000Z\n"))) + (should (package--archive-verify-timestamp b nil "foo")) + (should (package--archive-verify-timestamp b a "foo")) + (should (package--archive-verify-timestamp c a "foo")) + (should (package--archive-verify-timestamp c b "foo")) + ;; Signal error. + (should-error (package--archive-verify-timestamp a b "foo") + :type 'bad-timestamp) + (should-error (package--archive-verify-timestamp a c "foo") + :type 'bad-timestamp) + (should-error (package--archive-verify-timestamp b c "foo") + :type 'bad-timestamp) + (should-error (package--archive-verify-timestamp nil a "foo") + :type 'bad-timestamp))) + +(ert-deftest package-test-check-archive-timestamp () + (let ((package-user-dir package-test-data-dir)) + (with-temp-buffer + (insert ";; Last-Updated: 2020-01-01T00:00:00.000Z\n") + (package--check-archive-timestamp "older") + (package--check-archive-timestamp "missing") + (should-error (package--check-archive-timestamp "newer") + :type 'bad-timestamp)))) + +(ert-deftest package-test-check-archive-timestamp/not-expired () + (let ((package-user-dir package-test-data-dir)) + (with-temp-buffer + (insert ";; Last-Updated: 2020-01-01T00:00:00.000Z\n" + ";; Valid-Until: 2999-01-02T00:00:00.000Z\n") + (should-not (package--check-archive-timestamp "older"))))) + +(ert-deftest package-test-check-archive-timestamp/expired () + (let ((package-user-dir package-test-data-dir)) + (with-temp-buffer + (insert ";; Last-Updated: 2020-01-01T00:00:00.000Z\n" + ";; Valid-Until: 2020-01-02T00:00:00.000Z\n") + (should-error (package--check-archive-timestamp "older"))))) + ;;; Tests for package-x features. |