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:
>> On 25 July 2016 at 18:16, Flavio Leitner <f...@redhat.com> wrote:
>> > Updates SELinux to allow ovs-vsctl to get parent process
>> > information and log that to the database:
>> >
>> > record 241: 2016-07-26 00:59:47.418 "ovs-vsctl (invoked by /bin/bash
>> > (pid 1589)): ovs-vsctl -t 10 -- --if-exist ...
>> >
>> > Jul 25 12:57:35 localhost.localdomain audit[830]: AVC avc: denied {
>> > search } for pid=830 comm="ovs-vsctl" name="731" dev="proc" ino=14140
>> > scontext=system_u:system_r:openvswitch_t:s0
>> > tcontext=system_u:system_r:initrc_t:s0 tclass=dir permissive=0
>> >
>> > Signed-off-by: Flavio Leitner <f...@redhat.com>
>> > ---
>> > selinux/openvswitch-custom.te | 5 +++++
>> > 1 file changed, 5 insertions(+)
>> >
>> > diff --git a/selinux/openvswitch-custom.te b/selinux/openvswitch-custom.te
>> > index fc32b97..5739595 100644
>> > --- a/selinux/openvswitch-custom.te
>> > +++ b/selinux/openvswitch-custom.te
>> > @@ -2,8 +2,13 @@ module openvswitch-custom 1.0;
>> >
>> > require {
>> > type openvswitch_t;
>> > + attribute domain;
>> > class netlink_socket { setopt getopt create connect getattr write
>> > read };
>> > + class dir { search };
>> > + class file { open getattr read };
>> > }
>> >
>> > #============= openvswitch_t ==============
>> > allow openvswitch_t self:netlink_socket { setopt getopt create connect
>> > getattr write read };
>> > +allow openvswitch_t domain:dir { search };
>> > +allow openvswitch_t domain:file { open getattr read };
>>
>> Hi Flavio,
>>
>> Thanks for spending some time to get OVS in better shape with SELinux.
>> I figure that once this settles down a bit we should take the policy
>> file here and work towards upstreaming all of the policy changes.
>
> Yeah, we can try to do both in parallel. Once this gets in, I will
> open the bz requesting to fix Fedora which would fix upstream too.
Sounds good.
>> 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:
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.
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.
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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
