On Wed, Jan 23, 2019 at 10:58:15AM +0100, Yves-Alexis Perez wrote: > following the release of fix for CVE-2019-3462, there are few things I'm > wondering about apt architecture. I'm leaving aside the http/https debate > (which I think we need to have for Buster, though), but here are my though > (especially in light of Max Justicz blog post at > https://justi.cz/security/2019/01/22/apt-rce.html) I don't want to delve too much on this, but our https and tor transport are just as effected by the CVE as http is – as it is the very same code running and in all cases we interact with untrusted entities as even a shiny certificate doesn't make a mirror suddenly a trusted vendor (… but but, http is worse … yada yada yada! By that logic >= 1.6 doesn't need an upgrade as the exploit isn't working for these less effected). Anyhow, https is outside of deity@ teams scope by now as buster will have the https method installed on all systems (as it was folded into the apt package itself). You will need to talk to mirror, d-i and co about having https mirrors in sources.list by default – there is nothing we as deities can do about it. (And they are working on it for a while now, but it is as usual not as easy and as overrun by contributors as comment sections suggest) > I didn't really know about apt architecture and the fact that fetchers are > forked. I think it's a good idea to isolate exposed workers dealing with > untrusted data (especially HTTP), but apt main process seems to trust data > coming from the workers. I'm unsure where is the boundary trust here, but if > the fetchers data is trusted, I guess workers shouldn't just copy content from > outside to inside but do a real (/complex) sanitation job before handling it > to apt? This architecture is allowed to drink beer in basically every country in the world by now (aka: older than 21 years) and is in its idea rather simple: * apt wants the file foobar * apt wants foobar downloaded from http://example.org/foobar * apt asks the http method to save the content in partial/foobar * http does its business, sometimes reporting back to apt what it is currently doing so apt can tell the user it made some progress * while working on the file http will also calculate all hashes – they are sometimes also used internally e.g. to fix pipelining errors * if http encounters a redirect it will report this back to apt as this redirect could be the job of another method (https, located on another host, …), part of a redirection loop or whatever * http will be done with the file at some point which it will report to apt and giving it various details about the file including the hashes it calculated * apt will process the reports, compares the hashes it got reported with the hashes it was expecting and so on. * apt has the file in partial/foobar now; indexes will usually need an uncompress now – which is handled again by a method which is called 'store'. It does the uncompress and potentially recompressed (in a different format) on the fly. deb files on the other hand are a single step process ATM (see Julians reply for that potentially changing) * eventually all is done and assuming all parts of the group were acquired successfully they are all moved to their final place on disk. That is the BASIC gist. Over the years we have added various details in the process like having methods run as _apt, seccomp (although disabled by default ATM), all individual steps compare hashes in a central place rather than only sometimes deep down in the specific file handling, all hashes have to match, … and we continue to do so. Sometimes by accident which is why the exploit hangs on >= 1.6 and sometimes by dedicated effort. > As I understand it, the file hashes are calculated (or in this case, injected > from outside) by the worker, and not by the apt main process. Is that a good > idea? So yes, we fully trust the method to behave nicely. It is rather hard to change that (see e.g. store – the potential compressed files it produces can't be verified) and in many ways I don't think it would be a good idea. It actually helps that a method can do basically everything as that allowed us to introduce things like _apt in the methods, it wouldn't have been possible to do it in the main process if we came at it with a "don't trust the methods" attitude. That said, methods don't need blind trust and nothing is preventing us from splitting the methods itself further and/or doing other tricks like passing sockets/pipes around instead of filenames – but http is a rather hard contender here as it supports a metric ton of HTTP/1.1 features like pipelining (and fixing them), resuming, If-* so if someone really wants to help instead of just talking they are probably better of starting with a more trivial method before tackling the big-player. In the end I don't think this (or any previous) security bug is inherently due to this or that architecture and we would have no security bugs if only we would have this or that other architecture. If there is a pattern in the last few security bugs than that git blame identifies me, but after being around for so/too long I guess that is to be expected and hopefully not an architectural design flaw in me… ;) > Finally, we're again bitten by GPG drawbacks: RCE is really possible here > because gpg won't actually complain when the release file is actually also > something else. Validating the release file format might be a good idea by > itself, but it'd be nice (though out of scope for deity@) if the signature > scheme wouldn't allow such things to happen. As details are important, I have to mention that Max is using the Release.gpg file, not the Release file nor the InRelease file. InRelease nowadays actually produces warnings in these situations. Release.gpg does not as apt wasn't really looking at that file – it is just a companion for the Release file and was hence always just handed of to gpgv, never to be read by any apt tool. I have an MR on salsa (!45) now which changes the warnings to errors, stultifies the parser as a result and goes to the trouble of parsing the Release.gpg doing basically the same check as for InRelease (just that no message is expected of course) as in the last two years nobody complained about seeing the warnings, so I would hope there is indeed no usecase (or derivative) with such files in existence. I did mention the git blame thing further above through, so I would as always appreciate some review, but that should go without saying. Julian has various other ideas as he already mentioned and we were always entertaining a few more on IRC. In other words: Business as usual. So, yeah, I know, the mob is out there and demands blood, but I don't think we have to rush into any drastic changes here. We have a track record of slow but steady evolution and I am pretty sure that will be the best approach again. Far more than clever ideas, endless rehashing reddit/hackernews/whatever threads and the "https probably solves world hunger, too" mob we will eventually need new contributors in apt (and for that matter many other teams). It is a bit sad that even through I am looking at contributing to APT for an entire decade in May, I am still the newest addition to the team… (if an optimistically counted trio can still be called a team) May the supercow be with you all & Best regards David Kalnischkies
Attachment:
signature.asc
Description: PGP signature