bug#19479: Package manager vulnerable

[Stefan Kangas <stefan@marxist.se>]

[-- Attachment #1: Type: text/plain, Size: 4472 bytes --]

Kelly Dean <kelly@prtime.org> writes:

> Ivan Shmakov requested that I send this message to the bug list.
>
> For details, see my message with subject ⌜Emacs package manager vulnerable to replay attacks⌝ to emacs-devel on 30 Dec 2014:
> https://lists.gnu.org/archive/html/emacs-devel/2014-12/msg02319.html
>
> Executive summary to fix the vulnerabilities:
>
> 0. Include a hash and length of each package's content in the package's record
> in archive-contents, rather than only including the package name and version
> number in that file as Emacs currently does. Barf if a package hash doesn't
> verify, regardless of whether any signatures verify.
> (Length technically not necessary, but still generally useful, e.g. if
> there's a length mismatch then you know there's a content mismatch and
> you don't have to bother checking the hash.)

I have implemented the first part of the protection against metadata
replay attacks in the attached patch: support for checksum (or hash)
verification.  This change is backwards-compatible; the new fields can
be added to "archive-contents" file without impacting old clients.
I've not yet updated documentation, NEWS, etc. but will get to that
next.

I introduce a new user option `package-verify-checksums' that controls
this new behaviour.  The default is 'allow-missing', which only
carries out this check if there are checksums in "archive-contents",
and does nothing otherwise.  In itself, this does nothing to protect
against metadata replay attacks (but might protect against data
corruption).  You need to set `package-verify-checksums' to t, and
implement timestamping (discussed below).

I still suggest to stick with this default for Emacs 27.1, or at least
until common package archives can catch up.  Once this is implemented
in GNU ELPA and MELPA, it makes more sense to move to a stricter
default.  Otherwise, the transition will be very bumpy.  I therefore
suggest to discuss stricter defaults later.

(BTW, I didn't bother fixing the package-x.el code for this patch,
since it seems like it's not that widely used.  It will work as
before, but lack support for adding the checksums automatically.)

> 1. Include a timestamp of archive-contents in that file itself (so that the
> signature in archive-contents.sig depends on the timestamp, so that the
> timestamp can't be forged), and have Emacs ignore any new archive-contents
> that's older than the latest valid one that Emacs has already seen or is older
> than some specified limit. One thing I forgot to mention in my original message:
> have Emacs signal a warning if it ever sees an archive-contents dated in the
> future, which indicates misconfiguration of the client or server (or of course,
> some kind of mischief).

To protect against metadata replay attacks, it is correct that we need
timestamps too.  I haven't done that in this first patch, but I hope
to do it in a following patch.  I wanted to get this first part done
before I started working on that.

My current best idea for how to do it is one which AFAICT haven't been
raised in this thread before: to add a comment with an RFC3339
timestamp to the top of the "archive-contents" file:

    ;; Last-Updated: 2019-10-01T15:32:55.000Z

This will be ignored by older versions of Emacs, since package.el uses
(read (current-buffer)) to read this file.  New versions will have
an easy time parsing this header, caching the value, and refusing to
update the package cache if the timestamp is older than one we have
already seen.  With that, we would have implemented protection
against metadata replay attacks.

I think it would be highly beneficial if this could go in before Emacs
27, not least so that package archives can start implementing support
for this.

Comments on all this are obviously more than welcome.

Best regards,
Stefan Kangas

PS. Note that the original thread ended up highly off-topic discussing
copyright issues, because one potential contributor refused to sign
the standard copyright assignment.  The eventual outcome was that we
could not use a patch written by that person.  I have therefore
deliberately not looked at that persons patch in order to avoid any
copyright issues.  I have implemented this from scratch based solely
on the below link, and the discussion in this thread:
https://www2.cs.arizona.edu/stork/packagemanagersecurity/attacks-on-package-managers.html

[-- Attachment #2: 0001-Support-package-checksum-verification.patch --]
[-- Type: text/x-patch, Size: 29457 bytes --]

From 543029f4d3dcc4e0401263c93e01fe9319395708 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Fri, 4 Oct 2019 10:36:14 +0200
Subject: [PATCH] Support package checksum verification

This is the first step towards protecting users of package.el against
metadata replay attacks.

* lisp/emacs-lisp/package.el (package-verify-checksums): New
defcustom.
(package-desc, package--ac-desc)
(package--add-to-archive-contents, package-install-from-archive): New
fields 'size' and 'checksums'.
(package-desc-filename): New function.
(package-process-define-package): Doc fix.

(bad-checksum): New error type.
(package--verify-package-checksum)
(package--verify-package-size): New function to verify that the
checksum and size of a package corresponds to the checksum and size
data in the "archive-contents" file on the package archive.
(package--show-verify-checksum-error): New function to show
details of an error on checksum verification.  (Bug#19479)

* lisp/emacs-lisp/package-x.el (package-upload-buffer-internal):
Update to use above new fields 'size' and 'checksums'.

* test/lisp/emacs-lisp/package-tests.el (package-test-refresh-contents)
(package-test-install-single-from-archive): Update tests.
(with-install-using-checksum): New macro.
(package-test-install-with-checksum/single-valid)
(package-test-install-with-checksum/single-invalid)
(package-test-install-with-checksum/tar-valid)
(package-test-install-with-checksum/tar-invalid): New tests for
installing packages with checksums.
(package-test-verification-text)
(package-tests-valid-md5-checksum)
(package-tests-valid-sha256-checksum)
(package-tests-valid-sha512-checksum): New variables.
(run-verify-checksums-test): New macro.
(package-test--verify-package-checksums-nil/ignore-invalid)
(package-test--verify-package-checksums-allow-missing)
(package-test--verify-package-checksums-allow-missing/missing)
(package-test--verify-package-checksums-allow-missing/ignore-unsupported)
(package-test--verify-package-checksums-t)
(package-test--verify-package-checksums-t/invalid-fails)
(package-test--verify-package-checksums-t/missing-fails)
(package-test--verify-package-checksums-all)
(package-test--verify-package-checksums-all/invalid-fails)
(package-test--verify-package-checksums-all/missing-fails)
(package-test--verify-package-checksums-all/no-supported-hash-fails)
(package-test--verify-package-checksums-all/ignore-unsupported)
(package-test--verify-package-size): New tests for the checksum
support.

* test/lisp/emacs-lisp/package-resources/archive-contents:
* test/lisp/emacs-lisp/package-resources/checksum-invalid-1.0.el:
* test/lisp/emacs-lisp/package-resources/checksum-valid-123.el:
* test/lisp/emacs-lisp/package-resources/checksum-valid-tar-0.99.tar:
* test/lisp/emacs-lisp/package-resources/checksum-valid-tar-0.99.tar:
New test data files.
---
 lisp/emacs-lisp/package-x.el                  |   4 +-
 lisp/emacs-lisp/package.el                    | 159 ++++++++++++++--
 .../package-resources/archive-contents        |  26 ++-
 .../package-resources/checksum-invalid-1.0.el |  17 ++
 .../checksum-invalid-tar-0.1.tar              | Bin 0 -> 10240 bytes
 .../package-resources/checksum-valid-123.el   |  17 ++
 .../checksum-valid-tar-0.99.tar               | Bin 0 -> 10240 bytes
 test/lisp/emacs-lisp/package-tests.el         | 177 +++++++++++++++++-
 8 files changed, 378 insertions(+), 22 deletions(-)
 create mode 100644 test/lisp/emacs-lisp/package-resources/checksum-invalid-1.0.el
 create mode 100644 test/lisp/emacs-lisp/package-resources/checksum-invalid-tar-0.1.tar
 create mode 100644 test/lisp/emacs-lisp/package-resources/checksum-valid-123.el
 create mode 100644 test/lisp/emacs-lisp/package-resources/checksum-valid-tar-0.99.tar

diff --git a/lisp/emacs-lisp/package-x.el b/lisp/emacs-lisp/package-x.el
index 2815be3fe6..56373c14c4 100644
--- a/lisp/emacs-lisp/package-x.el
+++ b/lisp/emacs-lisp/package-x.el
@@ -219,7 +219,9 @@ package-upload-buffer-internal
 	  (let ((contents (or (package--archive-contents-from-url archive-url)
 			      (package--archive-contents-from-file)))
 		(new-desc (package-make-ac-desc
-                           split-version requires desc file-type extras)))
+                           split-version requires desc file-type extras
+                           ;; FIXME: Use better values than nil nil.
+                           nil nil)))
 	    (if (> (car contents) package-archive-version)
 		(error "Unrecognized archive version %d" (car contents)))
 	    (let ((elt (assq pkg-name (cdr contents))))
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 188f398a56..6795e17ac3 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -335,6 +335,30 @@ package-gnupghome-dir
   :risky t
   :version "26.1")
 
+(defcustom package-verify-checksums 'allow-missing
+  "Non-nil means to verify the checksum of a package before installing it.
+
+This can be one of:
+- nil: Ignore checksums.
+- `allow-missing': Same as t if a checksum exists, but install a
+  package even if there is no checksum.
+- t: Require a valid checksum; refuse to install package if the
+  checksum is missing or invalid.  Verifies one checksum.
+- `all': Same as t, but verify all available (and supported)
+  checksums.
+
+The package checksums are automatically fetched from package
+archives with the package data on `package-refresh-contents'.
+
+Note that setting this to nil is intended for debugging, and
+should normally not be used since it will decrease security."
+  :type '(choice (const nil :tag "Never")
+                 (const allow-missing :tag "Allow missing")
+                 (const t :tag "Require valid checksum")
+                 (const t :tag "Require valid checksum, and check all"))
+  :risky t
+  :version "27.1")
+
 (defcustom package-check-signature 'allow-unsigned
   "Non-nil means to check package signatures when installing.
 More specifically the value can be:
@@ -429,6 +453,8 @@ package--default-summary
                                  requirements)))
                  (kind (plist-get rest-plist :kind))
                  (archive (plist-get rest-plist :archive))
+                 (checksums (plist-get rest-plist :checksums))
+                 (size (plist-get rest-plist :size))
                  (extras (let (alist)
                            (while rest-plist
                              (unless (memq (car rest-plist) '(:kind :archive))
@@ -466,6 +492,13 @@ package--default-summary
 
 `extras' Optional alist of additional keyword-value pairs.
 
+`size'  Size of the package in bytes.
+
+`checksums' Checksums for the package file.  Alist of ((ALGORITHM
+        . CHECKSUM)) where ALGORITHM is a symbol specifying a
+        `secure-hash' algorithm, and CHECKSUM is a string
+        containing the checksum.
+
 `signed' Flag to indicate that the package is signed by provider."
   name
   version
@@ -475,7 +508,9 @@ package--default-summary
   archive
   dir
   extras
-  signed)
+  signed
+  size
+  checksums)
 
 (defun package--from-builtin (bi-desc)
   "Create a `package-desc' object from BI-DESC.
@@ -538,6 +573,13 @@ package-desc-suffix
     ('dir "")
     (kind (error "Unknown package kind: %s" kind))))
 
+(defun package-desc-filename (pkg-desc)
+  "Return file-name of package-desc object PKG-DESC.
+This is the concatenation of `package-desc-full-name' and
+`package-desc-suffix'."
+  (concat (package-desc-full-name pkg-desc)
+          (package-desc-suffix pkg-desc)))
+
 (defun package-desc--keywords (pkg-desc)
   "Return keywords of package-desc object PKG-DESC.
 These keywords come from the foo-pkg.el file, and in general
@@ -603,11 +645,11 @@ package-activated-list
 (defun package-process-define-package (exp)
   "Process define-package expression EXP and push it to `package-alist'.
 EXP should be a form read from a foo-pkg.el file.
-Convert EXP into a `package-desc' object using the
-`package-desc-from-define' constructor before pushing it to
-`package-alist'.
-If there already exists a package by that name in
-`package-alist', replace that definition with the new one."
+
+Convert EXP into a `package-desc' object, then push it to
+`package-alist'.  If there already exists a package by the same
+name in `package-alist', add the object to the list of packages,
+and sort the entries by version."
   (when (eq (car-safe exp) 'define-package)
     (let* ((new-pkg-desc (apply #'package-desc-from-define (cdr exp)))
            (name (package-desc-name new-pkg-desc))
@@ -1310,6 +1352,81 @@ package--with-response-buffer-1
                    url))
           (insert-file-contents-literally url)))))
 
+(define-error 'bad-checksum "Failed to verify checksum")
+
+(defun package--show-verify-checksum-error (pkg-desc details)
+  "Show error on failed checksum verification of PKG-DESC with DETAILS.
+Error is displayed in a new buffer named \"*Error*\"."
+  (with-output-to-temp-buffer "*Error*"
+    (with-current-buffer standard-output
+      (insert (format "Failed to verify checksum of package `%s':\n\n"
+                      (package-desc-name pkg-desc)))
+      (insert details))))
+
+(defun package--verify-package-checksum (pkg-desc)
+  "Verify checksums of `package-desc' object PKG-DESC.
+This assumes that the we are in a buffer containing package.
+
+The value of `package-verify-checksums' decides what this
+function does:
+- nil: Do nothing.
+- 'allow-missing: Verify checksum if it exists, otherwise do
+  nothing.
+- t: Verify that there is at least one valid checksum.
+- 'all': Like t, but check all (supported) checksums in turn.
+
+Signal an error of type `bad-checksum' if the verification."
+  (cl-flet*
+      ((supported-hashes
+        (lambda ()
+          (or (seq-filter (lambda (h) (memql (car h) (secure-hash-algorithms)))
+                          (package-desc-checksums pkg-desc))
+              ;; Failed; signal error.
+              (package--show-verify-checksum-error
+               pkg-desc
+               (concat
+                (if (package-desc-checksums pkg-desc)
+                    (concat
+                     "No supported checksums found\n\n"
+                     (format-message "Package archive had: %s\n"
+                                     (package-desc-checksums pkg-desc))
+                     (format-message "Emacs supports: %s\n"
+                                     (secure-hash-algorithms)))
+                  "Package archive had no checksums for this package\n")))
+              (signal 'bad-checksum "no supported checksums found"))))
+       (do-check
+        (lambda (&optional all)
+          (dolist (hash (seq-take (supported-hashes)
+                                  (if all most-positive-fixnum 1)))
+            (let* ((algorithm (car hash))
+                   (a (cdr hash))
+                   (b (secure-hash algorithm (current-buffer))))
+              (if (equal a b) t
+                ;; Failed; signal error.
+                (package--show-verify-checksum-error
+                 pkg-desc
+                 (concat
+                  (format-message "\nChecksum mismatch (%s)\n\n" algorithm)
+                  (format-message "Expected: %s\n" a)
+                  (format-message "Result: %s\n" b)))
+                (signal 'bad-checksum (list "checksum mismatch" a b))))))))
+    (pcase package-verify-checksums
+      ('nil nil)
+      ('allow-missing (when (package-desc-checksums pkg-desc) (do-check)))
+      ('t (do-check))
+      ('all (do-check 'all))
+      (_ (user-error "Value of `package-verify-checksums' is invalid: `%s'"
+                     package-verify-checksums)))))
+
+(defun package--verify-package-size (pkg-desc)
+  "Verify package size of `package-desc' object PKG-DESC.
+This assumes that the we are in a buffer containing package."
+  (when-let ((a (package-desc-size pkg-desc))
+             (b (string-bytes (buffer-string))))
+    (unless (equal a b)
+      (error "Mismatch in size in package `%s'.  Size was %s, but expected %s."
+             (package-desc-name pkg-desc) b a))))
+
 (define-error 'bad-signature "Failed to verify signature")
 
 (defun package--check-signature-content (content string &optional sig-file)
@@ -1437,14 +1554,19 @@ package--add-to-compatibility-table
                 (version-list-< table-version version))
         (puthash name version package--compatibility-table)))))
 
-;; Package descriptor objects used inside the "archive-contents" file.
-;; Changing this defstruct implies changing the format of the
-;; "archive-contents" files.
 (cl-defstruct (package--ac-desc
-               (:constructor package-make-ac-desc (version reqs summary kind extras))
+               (:constructor
+                package-make-ac-desc (version reqs summary kind extras size checksums))
                (:copier nil)
                (:type vector))
-  version reqs summary kind extras)
+  "Package descriptor object used inside the \"archive-contents\" file.
+Changing this defstruct implies changing the format of the
+\"archive-contents\" files.
+
+This is mainly used in `package--add-to-archive-contents' to make
+the code that parses the \"archive-contents\" file more
+readable."
+  version reqs summary kind extras size checksums)
 
 (defun package--append-to-alist (pkg-desc alist)
   "Append an entry for PKG-DESC to the start of ALIST and return it.
@@ -1482,10 +1604,14 @@ package--add-to-archive-contents
            :summary (package--ac-desc-summary (cdr package))
            :kind (package--ac-desc-kind (cdr package))
            :archive archive
+           ;; Older "archive-contents" files might not have the
+           ;; below elements.
            :extras (and (> (length (cdr package)) 4)
-                        ;; Older archive-contents files have only 4
-                        ;; elements here.
-                        (package--ac-desc-extras (cdr package)))))
+                        (package--ac-desc-extras (cdr package)))
+           :size (and (> (length (cdr package)) 5)
+                      (package--ac-desc-size (cdr package)))
+           :checksums (and (> (length (cdr package)) 6)
+                           (package--ac-desc-checksums (cdr package)))))
          (pinned-to-archive (assoc name package-pinned-packages)))
     ;; Skip entirely if pinned to another archive.
     (when (not (and pinned-to-archive
@@ -1956,9 +2082,10 @@ package-install-from-archive
   (when (eq (package-desc-kind pkg-desc) 'dir)
     (error "Can't install directory package from archive"))
   (let* ((location (package-archive-base pkg-desc))
-         (file (concat (package-desc-full-name pkg-desc)
-                       (package-desc-suffix pkg-desc))))
+         (file (package-desc-filename pkg-desc)))
     (package--with-response-buffer location :file file
+      (package--verify-package-size pkg-desc)
+      (package--verify-package-checksum pkg-desc)
       (if (or (not (package-check-signature))
               (member (package-desc-archive pkg-desc)
                       package-unsigned-archives))
diff --git a/test/lisp/emacs-lisp/package-resources/archive-contents b/test/lisp/emacs-lisp/package-resources/archive-contents
index e2f92304f8..b41287ae89 100644
--- a/test/lisp/emacs-lisp/package-resources/archive-contents
+++ b/test/lisp/emacs-lisp/package-resources/archive-contents
@@ -14,4 +14,28 @@
  (multi-file .
              [(0 2 3)
               nil "Example of a multi-file tar package" tar
-              ((:url . "http://puddles.li"))]))
+              ((:url . "http://puddles.li"))])
+ (checksum-valid  .
+                  [(123)
+                    nil "A single-file package with a valid checksum." single
+                    nil
+                    343
+                    ((sha512 . "6270d64d63c90ef541c3386c40acb7716865499c506465f2ff9b408f05c6612fb0c58f5d83b60af9d7f8d972796ee270d0bc6ca8a17bd0412cc249dede6f7359"))])
+ (checksum-valid-tar .
+                     [(0 99)
+                     nil "A multi-file package with a valid checksum." tar
+                     nil
+                     10240
+                     ((sha512 . "2be7c37a16db32a2b08fc917ed5f4241814e2665bda1bd15328c2e5a842e45b81f6f31274697248ffaabf8010796685acb3342c5920af53ddd1e75d7fd764bd1"))])
+ (checksum-invalid .
+                   [(1 0)
+                    nil "A single-file package with an invalid checksum." single
+                    nil
+                    365
+                    ((sha512 . "not-a-valid-checksum"))])
+ (checksum-invalid-tar .
+                       [(0 1)
+                        nil "A multi-file package with an invalid checksum." tar
+                        nil
+                        10240
+                        ((sha512 . "not-a-valid-checksum"))]))
diff --git a/test/lisp/emacs-lisp/package-resources/checksum-invalid-1.0.el b/test/lisp/emacs-lisp/package-resources/checksum-invalid-1.0.el
new file mode 100644
index 0000000000..3b8b07a4b8
--- /dev/null
+++ b/test/lisp/emacs-lisp/package-resources/checksum-invalid-1.0.el
@@ -0,0 +1,17 @@
+;;; invalid-checksum.el --- A package with an invalid checksum in archive-contents
+
+;; Version: 1.0
+
+;;; Commentary:
+
+;; This package has an invalid checksum in archive-contents and is
+;; just used to verify that package.el refuses to install.
+
+;;; Code:
+
+(defun p-equal-to-np-p ()
+  (error "FIXME"))
+
+(provide 'invalid-checksum)
+
+;;; invalid-checksum.el ends here
diff --git a/test/lisp/emacs-lisp/package-resources/checksum-invalid-tar-0.1.tar b/test/lisp/emacs-lisp/package-resources/checksum-invalid-tar-0.1.tar
new file mode 100644
index 0000000000000000000000000000000000000000..f153eb366b1264c293327c08c507876943572533
GIT binary patch
literal 10240
zcmeIwv2KGf5C&k+d5YUQL2a<ZGi2`zoWOvEV=8PS<?U;v3>}Ipq70S#-{Rry!~TBS
z(Y8}uuZ0UY_O2@uFNG}CyLes6T#YdzFRC%}`?|HZ5~?=Zn5I@|Eu=D)R)WmyuCPC8
zjrqkyB2F9zj=LLw>+c@?+l_WF|DPJA_0PO!3;3*at~>dwx_RWUO|2>+D`grfNIvti
zqi6nk{vV^Ib`Hsg6lv}$jV{tBw-XPRy4l9?mgveU*`+*P62);|eMdbzPwW@V-JLkm
z%`UFLyD;PddEn!xDo;n#z<vlo00Izz00bZa0SG_<0uX=z1Rwwb2tWV=5P$##AOHaf
TKmY;|fB*y_009U<;DEpvqoiQ<

literal 0
HcmV?d00001

diff --git a/test/lisp/emacs-lisp/package-resources/checksum-valid-123.el b/test/lisp/emacs-lisp/package-resources/checksum-valid-123.el
new file mode 100644
index 0000000000..9611fd8c87
--- /dev/null
+++ b/test/lisp/emacs-lisp/package-resources/checksum-valid-123.el
@@ -0,0 +1,17 @@
+;;; valid-checksum.el --- A package with an valid checksum in archive-contents
+
+;; Version: 123
+
+;;; Commentary:
+
+;; This package has an valid checksum in archive-contents and is
+;; used to verify that package.el installs it.
+
+;;; Code:
+
+(defun p-equal-to-np-p ()
+  (error "FIXME"))
+
+(provide 'valid-checksum)
+
+;;; valid-checksum.el ends here
diff --git a/test/lisp/emacs-lisp/package-resources/checksum-valid-tar-0.99.tar b/test/lisp/emacs-lisp/package-resources/checksum-valid-tar-0.99.tar
new file mode 100644
index 0000000000000000000000000000000000000000..e468754bb94d96a047df7a5c5926eb1d065e363f
GIT binary patch
literal 10240
zcmeHLU60y25aoG)#V9WYX#)vMsLEDX;_ihOX+IFKZTHb616Z5bxwZr4-`{cg=w1O^
zwc3?h)e#~P+cV>HX3iMm1;&rM$owTsdEy(U{Gk5sU8C}XS3uX>D5}scd>aK%?{>&u
zmGB~JMNvC!h3!fZMnM=<AbP<VPMRt-?HQn=ADNgleRAIS#!oK%wFlx8{EvFwe{Om&
z6T2n2-D*AMeU}Gzh`5gS{8JuS{@d+l1%j7x|Gbqyod3y!YoHf{DO2cAr9ce|S&|{l
zcuH?lfmt9NCJN*%eq?j3pFNMT8~ue5IHYx>|A)`(nEvEGY>hut|IOf{wXgq+uvrP3
zAvFwF|33-$4=uaAygX9c#5MT7552D}%Si;}j0EB^CBU}MtqPo-k)<n{uN9v!3{<ab
z&<w&R^c5nE<;<)|U!X4wkubxhIER6V>*cE9Mn^hFJXoWjP-$Aw0edhh7nGt^suL+!
z&XlnMez?7dUdCd*F}nY1)oo^j(Ayw7u$BeHOpwkcTpPuwg+bs3m`EBVcbQE1Y;9fB
zGl~~C3TRMe+Iq5bXw82>fr_OtET$=s;hM)NGy<cT>=V7f1g51OyW%tu$Z1@`a<fmk
z*!kERJO`<4FjZ!3<O*bN0jyEDYe*S|wOe{*=ifG%02gKG3z;K*AZxU}6;D%`A`}{D
zMS5=i5E}4#F!^|QKY{M;1AOj|M%~-V!zE2N3rVB6#EmIV*}-X-#I0h&tSNG9;ifmb
z`bbN<e-Ew)lLv|)`h&@BZ#;(n-3Yoc?2aabtNZKj2!{8g;oZ1b2N)Aa1cWv447#=-
zWs*`ULBn{uW&42`)f8Krz=AE2n2nJyDKbi1%E=v~r|nv=ER|wjZt_Vo0Ssm!q&Wvo
z@x5OdlXn`8)oN9ri=r;oyg}Ss-gV=z5`S9-S%!jYW8L|du9Sr~3o=G5l&QN-B=4}S
zyj@N0?IlQ-stHjfAoua#f~CHZzR3L?s<ktKDk??5c>Tebo1Ryz(d=BwB~K9E96F%k
z({+y`(Lni#uC4!&lK!5P50m3)|IZ)iuh##AX3*aEe>x0o`ak{tbLaop&nYV^T%f=0
z;&4=sJllCa@cwrk$cof(zm&2k$AFD|mVYMf+qnmVrzBFHLZ#qs6*MoBBt>0MxmR;a
y^ZG<P4-Nr`fJ49`;1F;KI0PI54grUNL%<>65O4@M1RMem0f&G?z#;HQA@Cm&a2WFd

literal 0
HcmV?d00001

diff --git a/test/lisp/emacs-lisp/package-tests.el b/test/lisp/emacs-lisp/package-tests.el
index f450fd27c2..823b68b234 100644
--- a/test/lisp/emacs-lisp/package-tests.el
+++ b/test/lisp/emacs-lisp/package-tests.el
@@ -43,6 +43,9 @@
 
 (setq package-menu-async nil)
 
+;; Silence byte-compiler.
+(defvar epg-config--program-alist)
+
 (defvar package-test-user-dir nil
   "Directory to use for installing packages during testing.")
 
@@ -296,14 +299,15 @@ package-test-refresh-contents
   (with-package-test ()
     (package-initialize)
     (package-refresh-contents)
-    (should (eq 4 (length package-archive-contents)))))
+    (should (eq 8 (length package-archive-contents)))))
 
 (ert-deftest package-test-install-single-from-archive ()
   "Install a single package from a package archive."
   (with-package-test ()
     (package-initialize)
     (package-refresh-contents)
-    (package-install 'simple-single)))
+    (package-install 'simple-single)
+    (should (package-installed-p 'simple-single))))
 
 (ert-deftest package-test-install-prioritized ()
   "Install a lower version from a higher-prioritized archive."
@@ -557,6 +561,167 @@ package-test-signed
 		"Status: Installed in ['`‘]signed-good-1.0/['’]."
 		nil t))))))
 
+\f
+;;; Tests for package checksum verification.
+
+(defmacro with-install-using-checksum (ok fail package &rest body)
+  "Test installing PACKAGE while setting `package-verify-checksums'."
+  (declare (indent 2))
+  `(progn
+     (dolist (opt ,ok)
+       (let ((package-verify-checksums opt))
+         (with-package-test ()
+           (package-initialize)
+           (package-refresh-contents)
+           (package-install ,package)
+           (package-installed-p ,package))))
+     (dolist (opt ,fail)
+       (let ((package-verify-checksums opt))
+         (should-error
+          (with-package-test ()
+            (package-initialize)
+            (package-refresh-contents)
+            (package-install ,package))
+          :type 'bad-checksum)))))
+
+(ert-deftest package-test-install-with-checksum/single-valid ()
+  "Install a single package with valid checksum."
+  (with-install-using-checksum '(nil allow-missing t all) '() 'checksum-valid))
+
+(ert-deftest package-test-install-with-checksum/single-invalid ()
+  "Install a tar package with invalid checksum."
+  (with-install-using-checksum '(nil) '(allow-missing t all) 'checksum-invalid))
+
+(ert-deftest package-test-install-with-checksum/tar-valid ()
+  "Install a tar package with valid checksum."
+  (with-install-using-checksum '(nil allow-missing t all) '() 'checksum-valid-tar))
+
+(ert-deftest package-test-install-with-checksum/tar-invalid ()
+  "Install a tar package with invalid checksum."
+  (with-install-using-checksum '(nil) '(allow-missing t all) 'checksum-invalid-tar))
+
+(defconst package-test-verification-text
+  "Example text for testing checksum verification.")
+(defconst package-tests-valid-md5-checksum
+  ;; (secure-hash 'md5 package-test-verification-text)
+  "abe6375809e532f081b808b3aa052dfb")
+(defconst package-tests-valid-sha256-checksum
+  ;; (secure-hash 'sha256 package-test-verification-text)
+  "6875aa4523e45ddef627b4edf1296f1d7dd0c22ddd6a6584f0228215d25eefcd")
+(defconst package-tests-valid-sha512-checksum
+  ;; (secure-hash 'sha512 package-test-verification-text)
+  (concat "bdc631f9e675b1ea34570f0a4bb44568dc5cecac905eea737f5f451bc52fd0c6"
+          "81b0d8b3dc2a942b9950fbe9096ebdf517668245c9b5a7bbdea8487a8f9cdce6"))
+
+(defmacro run-verify-checksums-test (verify-checksums checksums)
+  "Run a test for `package-verify-checksums'."
+  (declare (indent 1))
+  `(with-temp-buffer
+     (insert package-test-verification-text)
+     (let ((package-verify-checksums ,verify-checksums)
+           (pkg (package-desc-create :name 'foobar
+                              :version '(1 0)
+                              :summary "Just a package with checksum."
+                              :kind 'single
+                              :checksums ,checksums)))
+       (package--verify-package-checksum pkg))))
+
+(ert-deftest package-test--verify-package-checksums-nil/ignore-invalid ()
+  "Ignore all checksums even when invalid."
+  (run-verify-checksums-test nil
+    '((sha512 . "invalid")
+      (invalid . "invalid"))))
+
+(ert-deftest package-test--verify-package-checksums-nil/ignore-empty ()
+  "Ignore all checksums even when empty."
+  (run-verify-checksums-test nil
+    nil))
+
+(ert-deftest package-test--verify-package-checksums-allow-missing ()
+  "Verify checksums (allow-missing) -- verify if available."
+  (run-verify-checksums-test 'allow-missing
+    `((sha512 . ,package-tests-valid-sha512-checksum))))
+
+(ert-deftest package-test--verify-package-checksums-allow-missing/missing ()
+  "Verify checksums (allow-missing) -- allow missing."
+  (run-verify-checksums-test 'allow-missing
+    nil))
+
+(ert-deftest package-test--verify-package-checksums-allow-missing/ignore-unsupported ()
+  "Verify checksums (t) -- ignore unsupported algorithm."
+  (run-verify-checksums-test 'allow-missing
+    `((ignore . "not supported")
+      (sha512 . ,package-tests-valid-sha512-checksum))))
+
+(ert-deftest package-test--verify-package-checksums-t ()
+  "Verify checksums (t) -- succeed when valid."
+  (run-verify-checksums-test t
+    `((sha512 . ,package-tests-valid-sha512-checksum))))
+
+(ert-deftest package-test--verify-package-checksums-t/invalid-fails ()
+  "Verify checksums (t) -- fail on invalid."
+  (should-error
+   (run-verify-checksums-test t
+     '((sha512 . "invalid")))
+   :type 'bad-checksum))
+
+(ert-deftest package-test--verify-package-checksums-t/missing-fails ()
+  "Verify checksums (t) -- fail on missing."
+  (should-error
+   (run-verify-checksums-test t
+     nil)
+   :type 'bad-checksum))
+
+(ert-deftest package-test--verify-package-checksums-t/ignore-unsupported ()
+  "Verify checksums (t) -- ignore unsupported algorithm."
+  (run-verify-checksums-test t
+    `((ignore . "not supported")
+      (sha512 . ,package-tests-valid-sha512-checksum))))
+
+(ert-deftest package-test--verify-package-checksums-all ()
+  "Verify checksums (all) -- succeed on valid."
+  (run-verify-checksums-test 'all
+    `((md5    . ,package-tests-valid-md5-checksum)
+      (sha256 . ,package-tests-valid-sha256-checksum)
+      (sha512 . ,package-tests-valid-sha512-checksum))))
+
+(ert-deftest package-test--verify-package-checksums-all/invalid-fails ()
+  "Verify checksums (all) -- fail if one checksum is invalid."
+  (should-error
+   (run-verify-checksums-test 'all
+     `((md5    . ,package-tests-valid-md5-checksum)
+       (sha256 . "invalid")
+       (sha512 . ,package-tests-valid-sha512-checksum)))
+   :type 'bad-checksum))
+
+(ert-deftest package-test--verify-package-checksums-all/missing-fails ()
+  "Verify checksums (all) -- fail on missing checksums."
+  (should-error
+   (run-verify-checksums-test 'all
+     nil)
+   :type 'bad-checksum))
+
+(ert-deftest package-test--verify-package-checksums-all/no-supported-hash-fails ()
+  "Verify checksums (all) -- fail if we have no supported hash."
+  (should-error
+   (run-verify-checksums-test 'all
+     '((unsupported . "invalid")))
+   :type 'bad-checksum))
+
+(ert-deftest package-test--verify-package-checksums-all/ignore-unsupported ()
+  "Verify checksums (all) -- succed if one hash algorithm is unsupported.
+If the rest succeed, just ignore the unsupported one."
+  (run-verify-checksums-test 'all
+    `((md5    . ,package-tests-valid-md5-checksum)
+      (sha256 . ,package-tests-valid-sha256-checksum)
+      (sha512 . ,package-tests-valid-sha512-checksum)
+      (ignore . "not supported"))))
+
+(ert-deftest package-test--verify-package-size ()
+  (with-temp-buffer
+    (let ((len (1+ (abs (random 1000)))))
+      (insert (make-string len ?#))
+      (package--verify-package-size (package-desc-create :size len)))))
 
 \f
 ;;; Tests for package-x features.
@@ -570,7 +735,9 @@ package-x-test--single-archive-entry-1-3
                               'single
                               '((:authors ("J. R. Hacker" . "jrh@example.com"))
                                 (:maintainer "J. R. Hacker" . "jrh@example.com")
-                                (:url . "http://doodles.au"))))
+                                (:url . "http://doodles.au"))
+                              nil
+                              nil))
   "Expected contents of the archive entry from the \"simple-single\" package.")
 
 (defvar package-x-test--single-archive-entry-1-4
@@ -579,7 +746,9 @@ package-x-test--single-archive-entry-1-4
                               "A single-file package with no dependencies"
                               'single
                               '((:authors ("J. R. Hacker" . "jrh@example.com"))
-                                (:maintainer "J. R. Hacker" . "jrh@example.com"))))
+                                (:maintainer "J. R. Hacker" . "jrh@example.com"))
+                              nil
+                              nil))
   "Expected contents of the archive entry from the updated \"simple-single\" package.")
 
 (ert-deftest package-x-test-upload-buffer ()
-- 
2.20.1