bug#19479: Package manager vulnerable

classic Classic list List threaded Threaded
45 messages Options
123
Reply | Threaded
Open this post in threaded view
|

bug#19479: Package manager vulnerable to replay attacks

Stefan Kangas
I have just pushed the branch scratch/package-security with proper
support for timestamps, as discussed below.  More details are in the
commit messages and the proposed documentation changes.  Once this is
merged, I hope to work on adding support for this to both GNU ELPA and
NonGNU ELPA.

I would like to merge this change to the master branch.  Is it
sufficient to ask for reviews and comments here first, or is there
anything else I should do in addition?

Any comments and feedback on all this is of course more than welcome.
Please also see my previous message about this change below.

Stefan Kangas <[hidden email]> writes:

> Kelly Dean <[hidden email]> 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.



Reply | Threaded
Open this post in threaded view
|

bug#19479: Package manager vulnerable to replay attacks

Stefan Kangas
Stefan Monnier <[hidden email]> writes:

> Do we need this hash-checksum, really?

AFAIK, a cryptographic hash is the generally accepted solution for
securely verifying the contents of a file.  That is, when you worry
about that file having been tampered with by a hostile actor.

> AFAICT, I think if we want to avoid replay attacks we need indeed
> a monotone "counter" (e.g. a timestamp) on the `archive-contents` and
> then a way to verify that the tarballs are what they claim to be.

We also need to sign `archive-contents', of course.  But otherwise
correct: we need to know that the metadata is not out-of-date, and we
need to have a (secure) mapping from the package metadata to individual
packages.

> We can already verify that they are what they claim to be since the
> tarball includes the version number inside the `<pkg>-pkg.el` file.
>
> So, I think all we need is to verify the contents of `<pkg>-pkg.el`
> after unpacking a tarball, to make sure it is indeed the package and
> version its name claimed to be.  This check would be welcome in any case
> to detect packaging errors.

I think the question here is: how do we securely map from the (signed)
package metadata to an individual package?

While not perfect, with a secure hash function, collisions are hard
enough to find that we for our purposes (like the rest of the world) can
happily not worry about it.  In comparison, it is immeasurably easier to
fake a version number than having to defeat a hash function.  I think
this is a significant drawback of what you propose.

We would need to add in a number of assumptions (e.g. packages are
individually signed, we never accidentally sign an old package with a
newer version number, etc.), to gain more confidence in a simple version
number check.

But even then the security it provides will not be as strong, and much
more brittle; there are just more moving parts where things could go
wrong.  And I'm not sure what we would gain.  Importantly, I don't think
it would simplify our implementation in Emacs (or GNU/NonGNU ELPA)
significantly.

But we could of course also check that the version number is correct.
That sounds like a useful sanity check to do.

Thanks for taking a look at this!

PS. Note that if we add a checksum, there will no longer be any need to
    sign individual packages for future versions of Emacs.  We would
    then only need to sign the metadata.



Reply | Threaded
Open this post in threaded view
|

bug#19479: Package manager vulnerable to replay attacks

Stefan Monnier
> While not perfect, with a secure hash function, collisions are hard
> enough to find that we for our purposes (like the rest of the world) can
> happily not worry about it.  In comparison, it is immeasurably easier to
> fake a version number than having to defeat a hash function.  I think
> this is a significant drawback of what you propose.

I'm not sure what you mean by it being easier: since the tarballs are
cryptographically signed, just like `archive-contents`, if
`archive-contents` points to `foo-42.1.tar` and that tarball has
a correct signature and its content says that it is indeed the package
`foo-42.1`, then I'm not sure which easy attack would be applicable.

AFAICT you'd have to find some old signed tarball which we accidentally
built with an incorrect content?  But presumably if we ever mess up
a tarball like that (which is indeed possible), we'd then want to be
careful not to "reuse" that version number, no?

> We would need to add in a number of assumptions (e.g. packages are
> individually signed,

Which they already are.

> we never accidentally sign an old package with a newer version number,
> etc.),

That's indeed the case as well.

> to gain more confidence in a simple version
> number check.

I think it's comparable: the main failings wold require some error on
our side in how we build and sign packages, and in most such cases
I think we'd end up with holes with either scheme.

> But we could of course also check that the version number is correct.
> That sounds like a useful sanity check to do.

Exactly.

> PS. Note that if we add a checksum, there will no longer be any need to
>     sign individual packages for future versions of Emacs.  We would
>     then only need to sign the metadata.

I think we'd want to keep the signatures anyway, e.g. they can still be
checked manually for old tarballs which aren't listed in
`archive-contents` any more.  And more generally they allow
authenticating the origin of a package without having to look it up in
`archive-contents`.


        Stefan




Reply | Threaded
Open this post in threaded view
|

bug#19479: Package manager vulnerable to replay attacks

Stefan Monnier
> How about adding this check in addition to the checksum check?

I think we should add this check in any case, yes.

> Having two separate checks together should surely bring more
> confidence than either of them would separately.  That sounds like
> good "defense in depth" thinking to me.

I'm not sure the added hash is needed, but it seems reasonably harmless.

>> I think we'd want to keep the signatures anyway, e.g. they can still be
>> checked manually for old tarballs which aren't listed in
>> `archive-contents` any more.  And more generally they allow
>> authenticating the origin of a package without having to look it up in
>> `archive-contents`.
> Valid points.  Let's keep them indefinitely.

Especially since some people seem interested to add commands to
`package.el` to programatically install old packages.


        Stefan




Reply | Threaded
Open this post in threaded view
|

bug#19479: Package manager vulnerable to replay attacks

Jean Louis
In reply to this post by Stefan Kangas
* Stefan Kangas <[hidden email]> [2020-11-26 05:07]:
> PS. Note that if we add a checksum, there will no longer be any need to
>     sign individual packages for future versions of Emacs.  We would
>     then only need to sign the metadata.

I do not know internals as I did not see yet signed package. But if
signed package fetched from GNU ELPA then such is verified against
official key on user's computer, right?

Now take in account that signed packages will be distributed through
mirrors and mirrors already exist.

If archive-contents or meta data is signed and can be technically used
by mirror, that would be fine. If archive-contents need to be changed
or mirror wants to mirror only specific packages then package need to
be signed.



123