On Thu, 2020-05-14 at 11:50 +0200, Ondrej Mosnacek wrote:
> 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.
>
I'll rework this to be like the nfs_filesystem test that had 28 tests x
2 with a minor change in each cycle.
> > +
> > 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
> >
