[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#19479: Package manager vulnerable
From: |
Noam Postavsky |
Subject: |
bug#19479: Package manager vulnerable |
Date: |
Tue, 05 May 2020 20:55:53 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.0.91 (gnu/linux) |
Stefan Kangas <stefan@marxist.se> writes:
> Subject: [PATCH] Support package checksum verification
>
> This is the first step towards protecting users of package.el against
> metadata replay attacks.
> +(define-error 'bad-checksum "Failed to verify checksum")
Would it be useful to have bad-signature and this one share a parent?
(by the way, I kind of wonder why it's not called
package-bad-signature).
> + (cl-flet*
> + ((supported-hashes
> + (lambda ()
Is this a function (rather than a variable) just so it can be in the
same cl-flet* as do-check?
> + (or (seq-filter (lambda (h) (memql (car h)
> (secure-hash-algorithms)))
The list returned by secure-hash-algorithms includes sha1 and md5. This
is a problem if we're going to rely on signing them. We should at least
plan to have some way of filtering out some functions.
> + (a (cdr hash))
> + (b (secure-hash algorithm (current-buffer))))
> + (when-let ((a (package-desc-size pkg-desc))
> + (b (string-bytes (buffer-string))))
I risk descending into trivial nitpicking, but I think 'a' and 'b' are
bit too generic. Something like 'expected' and 'actual' would make it
harder to mix them up.
> +(defmacro run-verify-checksums-test (verify-checksums checksums)
> + "Run a test for `package-verify-checksums'."
> +(ert-deftest package-test--verify-package-checksums-nil/ignore-invalid ()
I think run-verify-checksums-test should be prefixed with package-test
(whereas the individual test names could be prefixed with just package).
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- bug#19479: Package manager vulnerable,
Noam Postavsky <=