Re: Some ostree observations

[Colin Walters <walters verbum org>]

[ Another mail client snafu, resending to the list ]

[ Replying mostly to security issues here, will follow up later with 
other ones ]

On Thu, Feb 20, 2014 at 6:31 PM, Colin Walters <walters verbum org> 
wrote:
> Colin asked me to have a look at OSTree. Here is what I found.
>
> In the images, the root account has no password. This is even true 
> for the images that run gnome-initial-setup, where it should be 
> unnecessary.

Yes.  We (Fedora) need an initial setup program for servers that works 
on a tty.

> /etc/machine-id is not generated at first boot.

https://bugzilla.gnome.org/show_bug.cgi?id=724833

> Some images have no networking by default.

That's intentional.  See:
https://apps.fedoraproject.org/packages/NetworkManager-config-server
> SELinux runs in permissive mode.

Fixed!
See 
https://git.gnome.org/browse/ostree/commit/?id=3337334be5c7abbb79796994523e445db53df838
and
https://github.com/cgwalters/rpm-ostree/commit/39a7c458ef24afc70801e4e73ee67483de855150

Will upload new images soon.

> "ostree admin upgrade" is slower than "yum upgrade" and transfers 
> about 50% more data. Most time is spent in downloading, not coming 
> even close to fully utilizing my end of the pipe. Apparently, HTTP 
> pipelining is not aggressive enough.

We're comparing apples to oranges a bit because "yum upgrade" isn't an 
atomic upgrade.  But yes - if you are updating multiple things, RPM + 
deltarpm is generally going to be more efficient on the network.

For very small updates, OSTree can be better.  Particularly without 
deltarpms (which from my POV have an excessive CPU penalty on both the 
client and server).

With static deltas, I'll cost more on the server in disk space, but 
I'll be equally or more efficient than RPM.   See:
https://bugzilla.gnome.org/show_bug.cgi?id=721799
Some recent discussion also in:
https://bugzilla.gnome.org/show_bug.cgi?id=709050

> The SELinux relabeling following an upgrade seems insecure because it 
> uses the full path across potentially user-writable directories, but 
> exploitation would need user-writable directories in RPMs (which are 
> fortunately rare).

By "user" here are we talking about humans that log in, or system 
daemons that run as non-root?  Both?

I can't think of a threat scenario here that OSTree would be vulnerable 
to that rpm/dpkg wouldn't.  Except maybe you're thinking I should be 
using setfscreatecon()?  Oh...I *definitely* need to do that for /etc, 
otherwise there is a window in which e.g. /etc/shadow is just etc_t and 
not shadow_t.   Filed:
https://bugzilla.gnome.org/show_bug.cgi?id=724872

But I'm assuming you meant some other bug...

> Non-fast-forward tree updates are not rejected in "ostree admin 
> upgrade".

Fixed!  
https://git.gnome.org/browse/ostree/commit/?id=f2e0162846cbb84abf6fd9a38fabdfd588f4eb2f
> The name of the repository head is not covered by the GnuPG 
> signature. As a result, a man-in-the-middle attack can cause an image 
> to upgrade to a totally unrelated head from the same repository 
> (because it is signed with the same trusted key).

Very good point.  Filed:
https://bugzilla.gnome.org/show_bug.cgi?id=724873
> The signature verification code enables downgrade attacks because it 
> does not reject stale/out-of-date signatures. Again, this is made 
> worse by not rejecting non-fast-forward commits.

I think this one is covered by the timestamp now.

> "ostree admin upgrade" stores .filez objects with malformed headers 
> in files in the /ostree/repo/tmp directory, causing subsequent 
> upgrades to fail even if a non-malformed object is now available for 
> download. I'm not sure what causes this, but it can't be good that 
> _ostree_repo_has_loose_object does not verify the checksum of the 
> loose object.

 _ostree_repo_has_loose_object() won't look in the tmp/ directory - 
that's only used as a staging area for partial objects.  Once something 
lands in /ostree/repo/objects, it should have had its SHA256 checked.  
(Although I am considering relaxing this for objects fetched over TLS, 
at least optionally)

One thing that may have led you astray is that the temporary files are 
named with a SHA256 hash in tmp/ - except this is a hash of their 
*URL*, not their content.  It's mainly an easy way to avoid having a 
full long URL (and quote metacharacters) in the filenames.

If an object is corrupted mid-transit (or on the server) *and* we only 
have a partial fetch, I think what will happen is next time we do a 
fetch, we'll keep trying until we get the original expected length, 
then we'll feed it to ostree_repo_write_content_async() which should 
verify the SHA256 of the actual content.

There's tests now that cover this in tests/test-pull-corruption.sh.