scratch/package-security bcde5f8 2/2: Support expiration of metadata by package archives

[Stefan Kangas]

branch: scratch/package-security
commit bcde5f86c5a7f3a84115807520631a4f12fb6f70
Author: Stefan Kangas <stefan@marxist.se>
Commit: Stefan Kangas <stefan@marxist.se>

    Support expiration of metadata by package archives
    
    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.
---
 doc/lispref/package.texi                           |  23 ++++-
 etc/NEWS                                           |   5 +
 lisp/calendar/iso8601.el                           |   1 +
 lisp/emacs-lisp/package.el                         | 105 +++++++++++++++++++++
 .../archives/newer/archive-contents                |   1 +
 .../archives/older/archive-contents                |   1 +
 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 af87479..725fecd 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.
 
diff --git a/etc/NEWS b/etc/NEWS
index da18848..ead2698 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -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 906c29b..7a6f14a 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 308f9eb..1e73a16 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 0000000..59a7997
--- /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 0000000..193a6b5
--- /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 a81506d..b0da54a 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.