On Mon, 2021-11-08 at 18:54 +0100, Ondrej Mosnacek wrote:
> Hi Richard,
>
> On Sun, Nov 7, 2021 at 3:21 PM Richard Haines
> <richard_c_haines@xxxxxxxxxxxxxx> wrote:
> >
> > Add tests for sctp_socket type_transition rules and also using
> > setsockcreatecon(3)
> >
> > Signed-off-by: Richard Haines <richard_c_haines@xxxxxxxxxxxxxx>
> > ---
> > policy/test_sctp.te | 57
> > ++++++++++++++++++++++++++++++++
> > tests/sctp/sctp_client.c | 19 +++++++++--
> > tests/sctp/sctp_common.c | 51 ++++++++++++++++++++++++++++
> > tests/sctp/sctp_common.h | 2 ++
> > tests/sctp/sctp_peeloff_client.c | 21 +++++++++---
> > tests/sctp/sctp_peeloff_server.c | 18 ++++++++--
> > tests/sctp/sctp_server.c | 18 ++++++++--
> > tests/sctp/test | 57
> > +++++++++++++++++++++++++++++++-
> > 8 files changed, 231 insertions(+), 12 deletions(-)
> >
> [...]
> > diff --git a/tests/sctp/sctp_peeloff_client.c
> > b/tests/sctp/sctp_peeloff_client.c
> > index 2d42c72..5470494 100644
> > --- a/tests/sctp/sctp_peeloff_client.c
> > +++ b/tests/sctp/sctp_peeloff_client.c
> > @@ -3,13 +3,14 @@
> > static void usage(char *progname)
> > {
> > fprintf(stderr,
> > - "usage: %s [-e expected_msg] [-v] [-n] [-x] addr
> > port\n"
> > + "usage: %s [-e expected_msg] [-v] [-n] [-t type] [-
> > x] addr port\n"
> > "\nWhere:\n\t"
> >
> > "-e Optional expected message from server e.g.
> > \"nopeer\".\n\t"
> > " If not present the client context will be
> > used as a\n\t"
> > " comparison with the servers reply.\n\t"
> > "-n Do NOT call connect(3) or connectx(3).\n\t"
> > + "-t New type for setsockcreatecon(3) test.\n\t"
> > "-v Print context and ip options
> > information.\n\t"
> > "-x Use sctp_connectx(3) instead of
> > connect(3).\n\t"
> > "addr IPv4 or IPv6 address (e.g. 127.0.0.1 or
> > ::1).\n\t"
> > @@ -28,10 +29,10 @@ int main(int argc, char **argv)
> > char byte = 0x41, label[1024], *expected = NULL;
> > bool verbose = false, connectx = false, no_connects = false;
> > bool ipv4 = false, expect_ipopt = false;
> > - char *context;
> > + char *context, *sock_type = NULL;
> > struct timeval tm;
> >
> > - while ((opt = getopt(argc, argv, "e:vxmni")) != -1) {
> > + while ((opt = getopt(argc, argv, "e:t:vxmni")) != -1) {
> > switch (opt) {
> > case 'e':
> > expected = optarg;
> > @@ -45,6 +46,9 @@ int main(int argc, char **argv)
> > case 'n':
> > no_connects = true;
> > break;
> > + case 't':
> > + sock_type = optarg;
> > + break;
> > case 'x':
> > connectx = true;
> > break;
> > @@ -187,11 +191,20 @@ int main(int argc, char **argv)
> > exit(1);
> > }
> >
> > + /* If -t option set new context for peeled off socket */
> > + if (sock_type) {
> > + result = set_newsock_con(sock_type, "Peeloff
> > Client");
> > + if (result < 0) {
> > + fprintf(stderr, "Failed
> > setsockcreatecon(3)\n");
> > + exit(20);
> > + }
> > + }
> > +
> > peeloff_sk = sctp_peeloff(sock, assoc_id);
> > if (peeloff_sk < 0) {
> > perror("Client sctp_peeloff");
> > close(sock);
> > - exit(1);
> > + exit(21);
> > }
> > if (verbose) {
> > printf("Client sctp_peeloff(3) on sk: %d with
> > association ID: %d\n",
>
> So are you implying that you expect the peeloff socket to get the
> label via the usual transition-or-sockcreatecon mechanism, rather than
> inheriting it from the parent socket?
No just testing the way it currently works when calling
sctp_peeloff(3). It was only the discussions on your patch series
threads that made me think of testing these various scenarios (also saw
testsuite issue #12 [1]). Once your new changes are done the tests can
be reworked.
[1] https://github.com/SELinuxProject/selinux-testsuite/issues/12
> My current opinion is the
> opposite, motivated by the (somewhat) analogical case of sockets
> created via accept(2).
I really have no issues either way, so long as the maintainers are
happy. The only thing I would like to see is that whichever way is
chosen, it's documented (preferably in Documentation/security/SCTP.rst,
but to keep Paul full of happiness & light also in the patch
description). I think the chances of anyone having code like my peeloff
test is very remote - but you never know !!!.
> Can you provide some arguments/use cases
> showing why the SCTP peeloff socket should have this behavior? Or
> would you argue that accept(2)-ed sockets should ideally also follow
> the transition-or-sockcreatecon behavior? Should it be combined with
> inheriting the parent socket's context by default or should each
> socket get its label independently based only on the creating process?
>
> Currently, the patches that Xin Long and I posted (or plan to post)
> are heading in the direction of just inheriting the parent socket's
> context. If there are good reasons to do it another way, I'm very
> interested in hearing them.
>
> --
> Ondrej Mosnacek
> Software Engineer, Linux Security - SELinux kernel
> Red Hat, Inc.
>
