From: Ted Zlatanov <tzz@lifelogs.com>
To: emacs-devel@gnu.org
Subject: Re: ELPA security
Date: Sun, 16 Jun 2013 07:18:56 -0400 [thread overview]
Message-ID: <m2li6as3q7.fsf@lifelogs.com> (raw)
In-Reply-To: jwvy5g34cvv.fsf-monnier+emacs@gnu.org
[-- Attachment #1: Type: text/plain, Size: 2022 bytes --]
On Tue, 08 Jan 2013 15:59:33 -0500 Stefan Monnier <monnier@IRO.UMontreal.CA> wrote:
>>> Actually, I see a problem with this scheme, now that we also keep around
>>> older versions of the packages. So maybe it's better to keep the
>>> signatures in a separate file, next to the signed file (e.g. have foo.tar
>>> and foo.tar.gpgsig).
>> Then maybe the file listed in the package vector should be the *.gpgsig
>> one, since otherwise it becomes easy to bypass the check by filtering
>> out any traces of the signature file.
SM> Right, we'd need to indicate somewhere that the sig should be
SM> present, indeed.
SM> A simple way to do that is to tell package.el directly, e.g. via
SM> `package-archives' or just by declaring that all ELPA archives should
SM> always have such signatures (they're pretty easy to add, so I'd expect
SM> marmalade and melpa to adjust pretty quickly).
Please see the attached patch. The code is not ready for testing, it's
just for review before I implement things further.
Changes:
* add `package-signed-archives', a list of logical archive names with
default '("gnu"). Add `package-archive-signed-p' to check it.
* change `package--with-work-buffer' to take an archive entry instead of
just the location. When an archive is `package-archive-signed-p',
create a signing buffer and load the archive filename with ".gpgsig"
appended. Then call `package--verify-signature' on the package buffer
and the signing buffer. If it fails, do `y-or-n-p', and if the user
rejects, error out.
* `package--verify-signature' is mocked to t right now, but will check
the maintainer signature.
* `package-download-single' and `package-download-tar' now pass the
archive entry, not just the location, to `package--with-work-buffer'
* rename `package-archive-base' to `package-archive-for'
* installable packages say "signed" or "unsigned" before the archive name
If you're OK with the code changes I'll get them working and start
implementing `package--verify-signature'.
Ted
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: package-archive-signed.patch --]
[-- Type: text/x-patch, Size: 7825 bytes --]
=== modified file 'lisp/emacs-lisp/package.el'
--- lisp/emacs-lisp/package.el 2013-06-15 15:36:11 +0000
+++ lisp/emacs-lisp/package.el 2013-06-16 11:05:16 +0000
@@ -229,6 +229,15 @@
:group 'package
:version "24.1")
+(defcustom package-signed-archives '("gnu")
+ "An list of archives whose contents are signed.
+
+Signed archives trigger verification of each package's contents."
+ :type '(list string :tag "Archive name")
+ :risky t
+ :group 'package
+ :version "24.4")
+
(defcustom package-pinned-packages nil
"An alist of packages that are pinned to a specific archive
@@ -699,20 +708,39 @@
nil nil nil 'excl))
(package--make-autoloads-and-compile name pkg-dir))))
-(defmacro package--with-work-buffer (location file &rest body)
- "Run BODY in a buffer containing the contents of FILE at LOCATION.
-LOCATION is the base location of a package archive, and should be
-one of the URLs (or file names) specified in `package-archives'.
+(defun package-archive-signed-p (archive)
+ "Returns whether ARCHIVE is signed.
+ARCHIVE is a package archive in the form (NAME . LOCATION) and should
+be specified in `package-archives'."
+ (member (car archive) package-signed-archives))
+
+(defmacro package--with-work-buffer (archive file &rest body)
+ "Run BODY in a buffer containing the contents of FILE at ARCHIVE.
+ARCHIVE is a package archive in the form (NAME . LOCATION) and should
+be specified in `package-archives'.
FILE is the name of a file relative to that base location.
-This macro retrieves FILE from LOCATION into a temporary buffer,
-and evaluates BODY while that buffer is current. This work
-buffer is killed afterwards. Return the last value in BODY."
- `(let* ((http (string-match "\\`https?:" ,location))
+This macro retrieves FILE from ARCHIVE into a temporary buffer,
+checks its signature if the ARCHIVE is defined to be signed by
+`package-signed-archives', and evaluates BODY while that buffer
+is current. This work buffer is killed afterwards. Return the
+last value in BODY."
+ `(let* ((archive-name (car ,archive))
+ (location (cdr ,archive))
+ (sign-file (concat ,file ".gpgsig"))
+ (http (string-match "\\`https?:" location))
+ (sign (when (package-archive-signed-p archive)
+ (concat location sign-file)))
(buffer
(if http
- (url-retrieve-synchronously (concat ,location ,file))
- (generate-new-buffer "*package work buffer*"))))
+ (url-retrieve-synchronously url)
+ (generate-new-buffer "*package work buffer*")))
+ (sign-buffer (when sign
+ (if http
+ ;; Retrieve the signature file too.
+ (url-retrieve-synchronously
+ (concat location sign-file))
+ (generate-new-buffer "*package sign buffer*")))))
(prog1
(with-current-buffer buffer
(if http
@@ -720,12 +748,32 @@
(re-search-forward "^$" nil 'move)
(forward-char)
(delete-region (point-min) (point)))
- (unless (file-name-absolute-p ,location)
+ (unless (file-name-absolute-p location)
(error "Archive location %s is not an absolute file name"
- ,location))
- (insert-file-contents (expand-file-name ,file ,location)))
+ location))
+ (insert-file-contents (expand-file-name ,file location)))
+ (when sign-buffer
+ (with-current-buffer sign-buffer
+ (if http
+ (progn (package-handle-response)
+ (re-search-forward "^$" nil 'move)
+ (forward-char)
+ (delete-region (point-min) (point)))
+ ;; No need to check the location again like we did above.
+ (insert-file-contents (expand-file-name sign-file location)))
+ (unless (package--verify-signature archive sign-buffer buffer)
+ (let ((q (format "Can't verify .gpgsig for %s"
+ (concat location ,file))))
+ (unless (y-or-n-p (concat q "; continue? (y/n)"))
+ (error q))))))
,@body)
- (kill-buffer buffer))))
+ (kill-buffer buffer)
+ (when sign-buffer
+ (kill-buffer sign-buffer)))))
+
+(defun package--verify-signature archive sign-buffer buffer
+ "Verify SIGN-BUFFER signs BUFFER correctly for ARCHIVE."
+ t)
(defun package-handle-response ()
"Handle the response from a `url-retrieve-synchronously' call.
@@ -742,16 +790,16 @@
(defun package-download-single (name version desc requires)
"Download and install a single-file package."
- (let ((location (package-archive-base name))
+ (let ((archive (package-archive-for name))
(file (concat (symbol-name name) "-" version ".el")))
- (package--with-work-buffer location file
+ (package--with-work-buffer archive file
(package-unpack-single name version desc requires))))
(defun package-download-tar (name version)
"Download and install a tar package."
- (let ((location (package-archive-base name))
+ (let ((archive (package-archive-for name))
(file (concat (symbol-name name) "-" version ".tar")))
- (package--with-work-buffer location file
+ (package--with-work-buffer archive file
(package-unpack name version))))
(defvar package--initialized nil)
@@ -875,6 +923,7 @@
(defun package--add-to-archive-contents (package archive)
"Add the PACKAGE from the given ARCHIVE if necessary.
PACKAGE should have the form (NAME . PACKAGE--AC-DESC).
+
Also, add the originating archive to the `package-desc' structure."
(let* ((name (car package))
(version (package--ac-desc-version (cdr package)))
@@ -1094,10 +1143,10 @@
(error "Package `%s' is a system package, not deleting"
(package-desc-full-name pkg-desc)))))
-(defun package-archive-base (name)
+(defun package-archive-for (name)
"Return the archive containing the package NAME."
(let ((desc (cdr (assq (intern-soft name) package-archive-contents))))
- (cdr (assoc (package-desc-archive desc) package-archives))))
+ (assoc (package-desc-archive desc) package-archives)))
(defun package--download-one-archive (archive file)
"Retrieve an archive file FILE from ARCHIVE, and cache it.
@@ -1106,7 +1155,7 @@
\"archives/NAME/archive-contents\" in `package-user-dir'."
(let* ((dir (expand-file-name (format "archives/%s" (car archive))
package-user-dir)))
- (package--with-work-buffer (cdr archive) file
+ (package--with-work-buffer archive file
;; Read the retrieved buffer to make sure it is valid (e.g. it
;; may fetch a URL redirect page).
(when (listp (read buffer))
@@ -1229,7 +1278,11 @@
'font-lock-face 'font-lock-builtin-face)
" Alternate version available")
(insert "Available"))
- (insert " from " archive)
+ (insert " from " (if (package-archive-signed-p
+ (assoc archive package-archives))
+ "signed"
+ "unsigned")
+ " " archive)
(insert " -- ")
(let ((button-text (if (display-graphic-p) "Install" "[Install]"))
(button-face (if (display-graphic-p)
@@ -1287,7 +1340,7 @@
;; For elpa packages, try downloading the commentary. If that
;; fails, try an existing readme file in `package-user-dir'.
(cond ((condition-case nil
- (package--with-work-buffer (package-archive-base package)
+ (package--with-work-buffer (package-archive-for package)
(concat package-name "-readme.txt")
(setq buffer-file-name
(expand-file-name readme package-user-dir))
next prev parent reply other threads:[~2013-06-16 11:18 UTC|newest]
Thread overview: 101+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-09 14:41 ELPA security George Kadianakis
2012-12-09 21:00 ` Nic Ferrier
2012-12-21 14:32 ` Ted Zlatanov
2012-12-21 22:12 ` Xue Fuqiao
2012-12-22 5:07 ` Bastien
2012-12-22 6:17 ` Xue Fuqiao
2012-12-22 12:34 ` Stephen J. Turnbull
2012-12-22 13:03 ` Bastien
2012-12-22 13:24 ` Bastien
2012-12-22 19:37 ` package.el + DVCS for security and convenience (was: ELPA security) Ted Zlatanov
2012-12-24 12:53 ` package.el + DVCS for security and convenience Nic Ferrier
2012-12-24 12:55 ` Bastien
2012-12-24 13:38 ` Ted Zlatanov
2012-12-24 13:39 ` Xue Fuqiao
2012-12-24 16:17 ` Stefan Monnier
2012-12-24 17:46 ` Ted Zlatanov
2012-12-25 1:03 ` Stephen J. Turnbull
2012-12-26 14:22 ` Ted Zlatanov
2012-12-27 3:06 ` Stephen J. Turnbull
2012-12-27 8:56 ` Xue Fuqiao
2012-12-31 11:18 ` Ted Zlatanov
2012-12-31 12:32 ` Stephen J. Turnbull
2012-12-31 13:50 ` Ted Zlatanov
2012-12-31 16:47 ` Stephen J. Turnbull
2012-12-31 21:41 ` Ted Zlatanov
2012-12-29 6:19 ` Stefan Monnier
2012-12-31 11:22 ` Ted Zlatanov
2013-01-03 16:41 ` Stefan Monnier
2013-01-04 16:05 ` Ted Zlatanov
2013-01-04 18:11 ` Stefan Monnier
2013-01-04 19:06 ` Ted Zlatanov
2013-01-05 3:25 ` Stephen J. Turnbull
2013-01-06 19:20 ` Ted Zlatanov
2013-01-07 2:03 ` Stephen J. Turnbull
2013-01-07 14:47 ` Ted Zlatanov
2013-01-08 1:44 ` Stephen J. Turnbull
2013-01-08 15:15 ` Ted Zlatanov
2013-01-08 17:53 ` Stephen J. Turnbull
2013-01-08 18:46 ` Ted Zlatanov
2013-01-08 21:20 ` Stefan Monnier
2013-01-09 2:37 ` Stephen J. Turnbull
2013-01-08 2:20 ` Stephen J. Turnbull
2013-01-08 14:05 ` Xue Fuqiao
2013-01-04 22:21 ` Xue Fuqiao
2012-12-31 20:06 ` Re:package.el + DVCS for security and convenience (was: ELPA security) Phil Hagelberg
2012-12-31 22:50 ` package.el + DVCS for security and convenience Ted Zlatanov
2012-12-22 16:20 ` ELPA security Stefan Monnier
2012-12-26 17:32 ` Paul Nathan
2012-12-31 11:50 ` Ted Zlatanov
2012-12-31 12:34 ` Stephen J. Turnbull
2012-12-31 13:39 ` Package signing infrastructure suggestion (was Re: ELPA security) Nic Ferrier
2012-12-31 22:32 ` Ted Zlatanov
2012-12-31 23:01 ` Xue Fuqiao
2012-12-31 19:48 ` ELPA security Tom Tromey
2012-12-31 19:57 ` Drew Adams
2012-12-31 22:19 ` Ted Zlatanov
2012-12-31 22:15 ` Ted Zlatanov
2013-01-05 16:46 ` Achim Gratz
2013-01-06 19:12 ` Ted Zlatanov
2013-01-07 5:32 ` Paul Nathan
2013-01-07 5:47 ` Jambunathan K
2013-01-07 5:53 ` Paul Nathan
2013-01-07 6:09 ` Jambunathan K
2013-01-07 6:20 ` Paul Nathan
2013-01-07 7:12 ` Stephen J. Turnbull
2013-01-07 7:18 ` chad
2013-01-07 14:34 ` Ted Zlatanov
2013-01-07 6:57 ` Stephen J. Turnbull
2013-01-07 14:35 ` Ted Zlatanov
2013-01-07 15:01 ` Ted Zlatanov
2013-01-08 3:07 ` Stefan Monnier
2013-01-08 14:47 ` Ted Zlatanov
2013-01-08 16:57 ` Stefan Monnier
2013-01-08 17:30 ` Ted Zlatanov
2013-01-08 20:50 ` Stefan Monnier
2013-01-08 21:30 ` Ted Zlatanov
2013-01-08 22:46 ` Stefan Monnier
2013-01-08 23:30 ` Ted Zlatanov
2013-03-12 18:29 ` Ted Zlatanov
2013-01-08 17:00 ` Stefan Monnier
2013-01-08 17:59 ` Achim Gratz
2013-01-08 18:37 ` Ted Zlatanov
2013-01-08 20:59 ` Stefan Monnier
2013-06-16 11:18 ` Ted Zlatanov [this message]
2013-06-16 23:12 ` Stefan Monnier
2013-06-17 1:56 ` Stephen J. Turnbull
2013-06-17 7:23 ` Ted Zlatanov
2013-06-17 15:54 ` Stephen J. Turnbull
2013-06-28 15:34 ` Ted Zlatanov
2013-06-17 14:34 ` Stefan Monnier
2013-06-17 7:20 ` Ted Zlatanov
2013-06-19 5:02 ` Ted Zlatanov
2013-06-19 12:38 ` Stefan Monnier
2013-06-23 11:58 ` Ted Zlatanov
2013-06-23 16:41 ` Stefan Monnier
2013-06-28 15:47 ` Ted Zlatanov
2013-06-28 16:28 ` Nic Ferrier
2013-06-28 22:49 ` Stefan Monnier
2013-06-24 3:44 ` Daiki Ueno
2013-06-28 15:32 ` Ted Zlatanov
2013-06-28 16:15 ` Daiki Ueno
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m2li6as3q7.fsf@lifelogs.com \
--to=tzz@lifelogs.com \
--cc=emacs-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).