Handling errors in EnsureDirState()

[Gustavo Niemeyer]

The recommendation wasn't to remove every single security profile when
something fails, but to remove every single profile being written on that
one change set.

Then, we shouldn't try to write change sets that modify the whole system at
once. That'd be recipe for serious breakage as side effects of minor
changes, which is obviously a terrible idea. That was your point, I think,
and I agree. Let's not do that.

Instead, change sets should be cooked for particular snaps, when we detect
a snap is out of sync with expectations.

When applying such a change set, if a security profile write fails, we
shouldn't follow through trying to stick "whatever works" into the system.
That's a very dangerous thing to do with security.. it's hard enough to get
it exactly right when we know the entirety of the content written. It's
simply impossible to understand the consequences of having arbitrarily
partial security profiles loaded into the system.

So, that's the background. My recommendation remains: either we write it
all, or we back out and attempt to remove what we can and fail hard.



On Thu, Mar 17, 2016 at 4:19 AM, Zygmunt Krynicki <
zygmunt.krynicki at canonical.com> wrote:

> Hey
>
> This is an extended reply to the thread that started here [1].
>
> As I understand it, both Gustavo and Jamie recommend that if we fail
> to write any of the files in a directory we manage we should remove
> all of the managed files there because that is safer from security
> point of view (no partial security).
>
> For some back story, the EnsureDirState() function is meant to write
> four sets of files:
>
> /etc/udev/rules.d/ [subset]
> /etc/dbus-1/system.d [subset]
> /var/lib/snappy/apparmor/ [everything]
> /var/lib/snappy/seccomp/ [everything]
>
> Now imagine what happens if a particular write fails. I cannot give
> you a good reason why such a write would fail (except for a hardware
> fluke or running out of disk space). If one of the writes fail we'd
> remove all of the files managed in that directory. This would
> effectively cripple all the dbus services, all the
> device-passthroughs, all the apparmor and seccomp profiles.
>
> What I don't like about this is that failure in an isolated spot
> (installing a snap, connecting two snaps) has the side effect of
> breaking the whole system (as none of the apps would work anymore).
>
> If you follow the discussion there you can see that my recommendation
> is that we don't do this. On any write failure we bail out and let the
> caller decide. The caller still can decide to remove all the managed
> files so we don't lose that functionality. The advantage is that in
> the cases where we install new snaps or create new connections we can
> chose to remove only files related to the new content, thus not
> affecting other snaps that should work as before.
>
> Best regards
> ZK
>
> [1] https://github.com/ubuntu-core/snappy/pull/658#discussion_r56203587
>
> --
> snappy-devel mailing list
> snappy-devel at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/snappy-devel
>



-- 
gustavo @ http://niemeyer.net
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/snappy-devel/attachments/20160317/c26d9916/attachment.html>