[systemd-devel] [systemd-commits] man/systemd.socket.xml src/core src/shared

[Lennart Poettering]

On Tue, 19.08.14 09:57, Michal Sekletar (msekleta at kemper.freedesktop.org) wrote:

Hi!

Hmm, can we please have patches like this sent to the ML? 

I am not sure I like the SELinuxLabelViaNet= name. Can we name this
SELinuxContextFromNet= at least?

We currently have "SELinuxContext=", and afaik "label" and "conetxt" are
kinda the same thing, could we please stick to one nomenclature here?

>                          <varlistentry>
> +                          <term><varname>SELinuxLabelViaNet=</varname></term>
> +                          <listitem><para>Takes a boolean
> +                          value. Controls whether systemd attempts to figure out
> +                          SELinux label used for instantiated service from
> +                          information handed by peer over the
> +                          network. Configuration option has effect only
> +                          on sockets with <literal>Accept=</literal>
> +                          mode set to <literal>yes</literal>.</para></listitem>
> +                        </varlistentry>
> +
> +                        <varlistentry>

This is indented differently from the rest of the file.

> +
> +                        if (context->selinux_label_via_net && use_selinux()) {
> +                                _cleanup_free_ char *label = NULL;
> +
> +                                err = label_get_child_label(socket_fd, command->path, &label);
> +                                if (err < 0) {
> +                                        r = EXIT_SELINUX_CONTEXT;
> +                                        goto fail_child;
> +                                }
> +
> +                                err = setexeccon(label);
> +                                if (err < 0) {
> +                                        r = EXIT_SELINUX_CONTEXT;
> +                                        goto fail_child;
> +                                }
> +                        }

You forgot to check if "socket_fd" is actually valid here.

There's also the question what actually happens if both SELinuxContext=
and this new thing are set. We probably should clearly prefer one over
the other and only apply that in such a case, instead of applying first
one, and then the other.

Did you actually see my earlier review?

http://lists.freedesktop.org/archives/systemd-devel/2014-August/021922.html

I already raised all the issues I am pointing out now, then.

I'll revert this for now, please send an updated patch to the ML, and we
can review and merge it.

Thanks!

Lennart

-- 
Lennart Poettering, Red Hat