Open Stack

On Fri, May 15, 2015 at 12:41:20PM +0100, Matthew Booth wrote:
> I was looking at the migrations api, and I noticed that the api passes
> the request query unchecked to get_migrations, where it ultimately ends
> up in a db query. I was curious and spent a couple of hours checking
> this morning. There are a few instances of this.
> 
> I didn't find any security bugs, however I feel that this extremely bad
> practise, and is likely to result in a security bug eventually. For
> example, note that os-assisted-volume-snapshots:delete does not validate
> delete_info before passing it to volume_snapshot_delete. I looked at
> this quite carefully, and I think we are only protected from a host
> compromise because:
> 
> 1. The api requires admin context
> 2. libvirt's security policy
> 
> I could be wrong on that, though, so perhaps somebody else could check?

Item 1 is pretty much the "protection" here. In general this is a problem
with the design of os-assisted-volume-snapshots:delete API - the very
fact that it is intended to allow arbitrary file paths to be specified
by the user makes it effectively impossible to validate - any path has
to be considered valid :-( This means it should never be allowed for
anyone except trusted cloud admin.

The majority of our APIs though are better designed and do not allow the
API user to supply file paths and similarly sensitive parameters that
refer to host resources. Usually the user only provides unique identifiers
(UUIDs) and high level requirements (ie MAC addresses, disk sizes) and
not file paths or similar.

> Passing unvalidated input to a function isn't necessarily bad, for
> example if it is only used for filtering, but it should be clearly
> marked as such so it isn't used in an unsafe manner. This marking should
> follow the data as far as it goes through any number of function calls.
> libvirt's _volume_snapshot_delete function is a long way from the
> originating api call, and it is not at all obvious that the commit_base
> and commit_top arguments to virt_dom.blockCommit() are unvalidated.

I think the most important thing is really not to design more APIs like
os-assisted-volume-snapshots which are inherantly dangerous due to the
parameters they are design to allow :-( For those few we do have, we
should definitely vet it as carefully as possible.

> Does python have anything like perl's taint mode? If so, it might be
> worth investigating its use.

I don't believe so.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|