Re: [PATCH] selinux-testsuite: Add nftables to inet_socket and sctp tests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Sun, May 10, 2020 at 3:26 PM Richard Haines
<richard_c_haines@xxxxxxxxxxxxxx> wrote:
> Support secmark tests that require nftables version 9.3 or greater and
> kernel 4.20 or greater.
>
> Signed-off-by: Richard Haines <richard_c_haines@xxxxxxxxxxxxxx>
> ---
>  README.md                        |  4 +-
>  tests/inet_socket/nftables-flush |  2 +
>  tests/inet_socket/nftables-load  | 74 ++++++++++++++++++++++++++++++++
>  tests/inet_socket/test           | 72 +++++++++++++++++++++++++++++++
>  tests/sctp/nftables-flush        |  2 +
>  tests/sctp/nftables-load         | 68 +++++++++++++++++++++++++++++
>  tests/sctp/test                  | 74 ++++++++++++++++++++++++++++++++
>  7 files changed, 295 insertions(+), 1 deletion(-)
>  create mode 100644 tests/inet_socket/nftables-flush
>  create mode 100644 tests/inet_socket/nftables-load
>  create mode 100644 tests/sctp/nftables-flush
>  create mode 100644 tests/sctp/nftables-load

Thanks, this seems to work fine on Fedora 32 and even on RHEL-8 (after
adjusting the kernel version check). However, I have some style
suggestions (see below).

>
[...]
> diff --git a/tests/inet_socket/test b/tests/inet_socket/test
> index 47ce106..b551961 100755
> --- a/tests/inet_socket/test
> +++ b/tests/inet_socket/test
[...]
> @@ -406,6 +419,65 @@ server_end($pid);
>  # Flush iptables configuration.
>  system "/bin/sh $basedir/iptables-flush";
>
> +# Load nftables (IPv4 & IPv6) configuration.
> +if ($test_nft) {
> +    system "nft -f $basedir/nftables-load";
> +
> +    # Start the stream server.
> +    $pid = server_start( "-t test_inet_server_t", "-n stream 65535" );
> +
> +    # Verify that authorized client can communicate with the server.
> +    $result = system
> +"runcon -t test_inet_client_t -- $basedir/client -e nopeer stream 127.0.0.1 65535";
> +    ok( $result eq 0 );
> +
> +    # Verify that unauthorized client cannot communicate with the server.
> +    $result = system
> +"runcon -t test_inet_bad_client_t -- $basedir/client -e nopeer stream 127.0.0.1 65535 2>&1";
> +    ok( $result >> 8 eq 5 );
> +
> +    # Verify that authorized client can communicate with the server.
> +    $result = system
> +"runcon -t test_inet_client_t -- $basedir/client -e nopeer stream ::1 65535";
> +    ok( $result eq 0 );
> +
> +    # Verify that unauthorized client cannot communicate with the server.
> +    $result = system
> +"runcon -t test_inet_bad_client_t -- $basedir/client -e nopeer stream ::1 65535 2>&1";
> +    ok( $result >> 8 eq 5 );
> +
> +    # Kill the server.
> +    server_end($pid);
> +
> +    # Start the dgram server.
> +    $pid = server_start( "-t test_inet_server_t", "-n dgram 65535" );
> +
> +    # Verify that authorized client can communicate with the server.
> +    $result = system
> +"runcon -t test_inet_client_t $basedir/client -e nopeer dgram 127.0.0.1 65535";
> +    ok( $result eq 0 );
> +
> +    # Verify that unauthorized client cannot communicate with the server.
> +    $result = system
> +"runcon -t test_inet_bad_client_t -- $basedir/client -e nopeer dgram 127.0.0.1 65535 2>&1";
> +    ok( $result >> 8 eq 8 );
> +
> +    # Verify that authorized client can communicate with the server.
> +    $result = system
> +      "runcon -t test_inet_client_t $basedir/client -e nopeer dgram ::1 65535";
> +    ok( $result eq 0 );
> +
> +    # Verify that unauthorized client cannot communicate with the server.
> +    $result = system
> +"runcon -t test_inet_bad_client_t -- $basedir/client -e nopeer dgram ::1 65535 2>&1";
> +    ok( $result >> 8 eq 8 );
> +
> +    # Kill the server.
> +    server_end($pid);
> +
> +    system "nft -f $basedir/nftables-flush";
> +}

It seems that this block of tests is the same for both iptables and
nftables... could you extract it into a single shared function? I
think you were reluctant to do that because then you can't tell from
the line number in the test output which test has failed (iptables or
nftables), but I don't think it's worth the duplication here. AFAIK,
in case 2+ "ok()" calls are done from the same line, the test failure
report includes the number of the call that failed, so it should be
pretty easy to tell which variant has failed. Anyway, I would expect
that in 90% of cases either both calls pass or both fail.

> +
>  if ($test_calipso_stream) {
>
>      # Load NetLabel configuration for CALIPSO/IPv6 labeling over loopback.
[...]
> diff --git a/tests/sctp/test b/tests/sctp/test
> index 6631da4..afd28a1 100755
> --- a/tests/sctp/test
> +++ b/tests/sctp/test
[...]
> @@ -809,4 +819,68 @@ server_end($pid);
>
>  system "/bin/sh $basedir/iptables-flush";
>
> +#
> +##################### Test nftables configuration ############################
> +#
> +if ($test_nft) {
> +    print "# Testing nftables (IPv4/IPv6).\n";
> +    system "nft -f $basedir/nftables-load";
> +
> +    # Start the stream server.
> +    $pid = server_start( "-t test_sctp_server_t",
> +        "sctp_server", "$v -n stream 1035" );
> +
> + # Verify that authorized client can communicate with the server STREAM->STREAM.
> +    $result = system
> +"runcon -t test_sctp_client_t $basedir/sctp_client $v -e nopeer stream 127.0.0.1 1035";
> +    ok( $result eq 0 );
> +
> +# Verify that a client without peer { recv } permission cannot communicate with the server STREAM->STREAM.
> +    $result = system
> +"runcon -t test_sctp_deny_peer_client_t -- $basedir/sctp_client $v -e nopeer stream 127.0.0.1 1035 2>&1";
> +    ok( $result >> 8 eq 6 );
> +
> + # Verify that authorized client can communicate with the server STREAM->STREAM.
> +    $result = system
> +"runcon -t test_sctp_client_t $basedir/sctp_client $v -e nopeer stream ::1 1035";
> +    ok( $result eq 0 );
> +
> +# Verify that a client without peer { recv } permission cannot communicate with the server STREAM->STREAM.
> +    $result = system
> +"runcon -t test_sctp_deny_peer_client_t -- $basedir/sctp_client $v -e nopeer stream ::1 1035 2>&1";
> +    ok( $result >> 8 eq 6 );
> +
> +    # Kill the stream server.
> +    server_end($pid);
> +
> +    # Start the seq server.
> +    $pid =
> +      server_start( "-t test_sctp_server_t", "sctp_server", "$v -n seq 1035" );
> +
> +    # Verify that authorized client can communicate with the server SEQ->SEQ.
> +    $result = system
> +"runcon -t test_sctp_client_t $basedir/sctp_client $v -e nopeer seq 127.0.0.1 1035";
> +    ok( $result eq 0 );
> +
> +# Verify that a client without peer { recv } permission cannot communicate with the server SEQ->SEQ.
> +    $result = system
> +"runcon -t test_sctp_deny_peer_client_t -- $basedir/sctp_client $v -e nopeer seq 127.0.0.1 1035 2>&1";
> +    ok( $result >> 8 eq 6 );
> +
> +    # Verify that authorized client can communicate with the server SEQ->SEQ.
> +    $result = system
> +"runcon -t test_sctp_client_t $basedir/sctp_client $v -e nopeer seq ::1 1035";
> +    ok( $result eq 0 );
> +
> +# Verify that a client without peer { recv } permission cannot communicate with the server SEQ->SEQ.
> +    $result = system
> +"runcon -t test_sctp_deny_peer_client_t -- $basedir/sctp_client $v -e nopeer seq ::1 1035 2>&1";
> +    ok( $result >> 8 eq 6 );
> +
> +    # Kill the seq server.
> +    server_end($pid);

Same comment as above.

> +
> +    system "nft -f $basedir/nftables-flush";
> +}
> +
>  exit;
> --
> 2.25.3
>

-- 
Ondrej Mosnacek <omosnace at redhat dot com>
Software Engineer, Security Technologies
Red Hat, Inc.




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux