Adding William since he is the author of commit 484371776e
On Wed, Jul 27, 2016 at 01:54:45PM -0700, Joe Stringer wrote:
> On 26 July 2016 at 14:10, Flavio Leitner <f...@sysclose.org> wrote:
> > On Tue, Jul 26, 2016 at 12:57:07PM -0700, Joe Stringer wrote:
> >> As far as I can follow, this "domain" type is not just for accessing
> >> OVS directories and files (like openvswitch_t), but ifor a much wider
> >> range of paths:
> >> https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/4/html/SELinux_Guide/rhlcommon-section-0048.html
> >>
> >> "# The domain attribute identifies every type that can be
> >> # assigned to a process. This attribute is used in TE rules
> >> # that should be applied to all domains, e.g. permitting
> >> # init to kill all processes."
> >>
> >> Is my understanding (+documentation) correct here? Is there an similar
> >
> > Your understanding is correct. Turns out that we don't know which
> > process will be the parent, so it could bash unconfined or initrc_t
> > or in any other context (neutron?).
> >
> >> but more restrictive policy that allows ovs-vsctl to access, for
> >> example, /var/run/openvswitch/* (with var_run_openvswitch_t or
> >> similar)? Alternatively is there an example of another daemon that has
> >> a similar policy that set a precedence for writing the policy like
> >> this?
> >
> > I spent few hours on this and I couldn't find a way to restrict it
> > more that I proposed with selinux. Basically the above is an expansion
> > of the interface domain_read_all_domains_state()[1] which other
> > applications are using to read other processes states. However, that
> > seemed relatively new and probably not available on older distros, so
> > I have expanded to the relevant actions removing what we don't need.
> >
> > [1] http://danwalsh.livejournal.com/51435.html
> >
> >> Would you also be able to provide the full ovs-vsctl commandline? It
> >> was a little difficult to understand exactly what was going on during
> >> this event, or try to reproduce.
> >
> > utilities/ovs-vsctl.c:2473
> >
> > 2472 static char *
> > 2473 vsctl_parent_process_info(void)
> > 2474 {
> > 2475 #ifdef __linux__
> > 2476 pid_t parent_pid;
> > 2477 char *procfile;
> > 2478 struct ds s;
> > 2479 FILE *f;
> > 2480
> > 2481 parent_pid = getppid();
> > 2482 procfile = xasprintf("/proc/%d/cmdline", parent_pid);
> > 2483
> > 2484 f = fopen(procfile, "r");
> >
> > That is called from do_vsctl() to find the parent info. If you run as
> > root, then it's unconfined and it works, but it doesn't work during
> > boot time (initrc_t) for instance.
> >
> > To reproduce you just need to configure an OVS interface using ifcfg
> > with ONBOOT=yes and reboot.
>
> Hmm. So all we want to do in this case is to get the parent's name to
> log it. I'm of two minds:
Yes, funny how that can be a pain :-)
> 1) Be more restrictive. If this were the only place in OVS that we
> read /proc then maybe it should just print the pid on failure to read
> /proc, then we just accept that SELinux denies this access. It seems
> like it's just a niceity for prettyprinting which shouldn't affect
> actual setup/operation of OVS. While a pid is less than a name, it's
> still just as much information for cases like this where it's being
> invoked from initrc_t.
It generates AVC messages which triggers notifications, so we actually
need to consider three options:
1) Allow it (your option 2 below)
2) Fix the code (proposed in your option 1)
3) Use dontaudit to deny but not log.
The problem is that PIDs numbers don't mean anything useful for us when
you are looking at logs from another system. Even if you are on the
system, PIDs are ephemeral and probably gone or worse, recycled.
I think the feature has an interesting value when you consider a OSP
compute node where agents and scripts are running and configuring OVS.
I got reports like "we added an port but there is no traffic", the
next step is to make sense out of ovsdb logs and there you find
an application adding and deleting the same port. Without that, we
have the actions, but we can't tell who did it. How to find that out
then?
If we can reproduce the issue and start to record pids and names with
another script, then it's fine, but most probably it's one of those
"happened once", "it's in production", "can't enable this or that"
kind of situation.
> 2) Be more lenient. It seems there are a few other /proc reads,
> particularly in netdev-linux.c, which I'm guessing will just need the
> same policy that you're proposing above, so we can just accept this
> policy. If someone's concerned about tightening this policy, they can
> choose not to install this package.
IIRC, the other reads on /proc are system_u:object_r:proc_net_t:s0 so
it can be more restricted by using that and not 'domain'.
> I suppose that if SELinux isn't granular enough at this point then we
> could go with #2 and tighten the policy later when there are better
> attributes in place.
As I said, the feature is interesting but only for troubleshooting
most probably, so maybe exposing more everyone because of a possible
use-case might be a bad idea after all.
Having said that, I think the safest option is to go with your
proposal #1.
BTW, although I posted the SELinux update along with Bonding patch,
they don't depend on each other besides the context for reproducing
the selinux issue. So, I appreciate any feedback on the bonding
patch as well.
This is from a system without the patchset showing the same SElinux
issue:
[...]
Jul 28 10:07:53 localhost.localdomain ovs-vsctl[974]:
ovs|00001|vsctl|INFO|Called as ovs-vsctl -t 10 -- --may-exist add-br
ovsbr0
Jul 28 10:07:53 localhost.localdomain audit[974]: AVC avc: denied {
search } for pid=974 comm="ovs-vsctl" name="936" dev="proc" ino=15068
scontext=system_u:system_r:openvswitch_t:s0
Jul 28 10:07:53 localhost.localdomain ovs-vsctl[974]:
ovs|00002|vsctl|WARN|/proc/936/cmdline: open failed (Permission
denied)
Jul 28 10:07:53 localhost.localdomain network[575]:
2016-07-28T13:07:53Z|00002|vsctl|WARN|/proc/936/cmdline: open failed
(Permission denied)
[...]
Thanks,
--
fbl
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
