On Wed, Oct 21, 2020 at 12:59 PM Richard Haines <richard_c_haines@xxxxxxxxxxxxxx> wrote: > On Tue, 2020-10-20 at 13:55 +0200, Ondrej Mosnacek wrote: > > On Fri, Oct 16, 2020 at 3:02 PM Richard Haines > > <richard_c_haines@xxxxxxxxxxxxxx> wrote: > > > On Thu, 2020-10-15 at 18:04 +0100, Richard Haines wrote: > > > > On Thu, 2020-10-15 at 16:12 +0200, Ondrej Mosnacek wrote: > > > > > On Thu, Oct 15, 2020 at 3:49 PM Richard Haines > > > > > <richard_c_haines@xxxxxxxxxxxxxx> wrote: > > > > > > On Thu, 2020-10-15 at 12:28 +0200, Ondrej Mosnacek wrote: > > > > > <snip> > > > > > > Just a thought - have you tried running the server in one > > > > > > terminal > > > > > > session and the client in another (I've plugged in your > > > > > > Fedora 32 > > > > > > addresses): > > > > > > > > > > > > cd ...tests/sctp > > > > > > echo 1 > /proc/sys/net/sctp/addip_enable > > > > > > echo 1 > /proc/sys/net/sctp/addip_noauth_enable > > > > > > runcon -t sctp_asconf_params_server_t > > > > > > ./sctp_asconf_params_server > > > > > > 10.0.138.59 10.123.123.123 1035 > > > > > > > > > > > > cd ...tests/sctp > > > > > > runcon -t sctp_asconf_deny_param_add_client_t > > > > > > ./sctp_asconf_params_client 10.0.138.59 1035 > > > > > > > > > > Interesting... I just tried it a couple times and it's not > > > > > behaving > > > > > consistently - the first time I got "SCTP_PRIMARY_ADDR: > > > > > Permission > > > > > denied", then 'Dynamic Address Reconfiguration' twice in a row, > > > > > then > > > > > 7 > > > > > times "SCTP_PRIMARY_ADDR: Permission denied", then 'Dynamic > > > > > Address > > > > > Reconfiguration' 5 times. and then again "SCTP_PRIMARY_ADDR: > > > > > Permission denied". > > > > > > > > > > I tried (manually) different delays between starting the server > > > > > and > > > > > starting the client, but there didn't seem to be a pattern. > > > > > > > > > > > > > I wonder if this test is flaky. A bit of history: > > > > When I first produced the SCTP patches I had different > > > > permissions > > > > for > > > > SCTP_PARAM_SET_PRIMARY, SCTP_PARAM_ADD_IP etc. so that I could > > > > detect > > > > these denials with allow 'self' rules. However the maintainers > > > > wanted > > > > to keep things simple with just connect or bind permissions. This > > > > made > > > > it a bit more difficult to test this case. As it so happened > > > > (until > > > > now > > > > of course), the two LSM calls for SCTP_PARAM_SET_PRIMARY and > > > > SCTP_PARAM_ADD_IP in sm_make_chunk.c triggered the following > > > > rule: > > > > > > > > allow sctp_asconf_params_server_t > > > > sctp_asconf_deny_param_add_client_t:sctp_socket connect; > > > > > > > > therefore by not allowing this rule I could detect (using the > > > > tshark > > > > trace output "Client returns ASCONF_ACK's with 'Request refused - > > > > no > > > > authorization'") to prove this test case. > > > > > > > > If this boils down to a timing problem, then the test needs to be > > > > removed as I can't test this scenario, because the client needs > > > > the > > > > connect permission to be able to connect to the server in the > > > > first > > > > place. > > > > > > > > > > This test does have timimg issues. I put the three asconf tests in > > > a > > > loop of 200 iterations. Sometimes all would pass, but other times > > > the > > > third test would fail with the error. > > > > > > I guess there are two options: Revert the patch, or remove the > > > offending test. Any views !!! > > > > And do you think that the timing issue can be fixed by better > > synchronizing the client and server programs or is it inherently > > unavoidable? If you think it could be fixed in some way (even if > > non-trivial), then I'd prefer to just comment out the test with a > > note > > to revisit it in the future (leaving the code in place for now). I'll > > try to look into it at some point, unless you're faster than me :) > > I do have a possible new test for the 3rd asconf test that I've run > through 5000 interations without error, whereas the current one would > fail between 100 to 500 interations (I changed the 'tm.tv_sec = 3;' > client timer to 'tm.tv_usec = 50000;' so it ran faster). I hope to send > updates late next week. Note that I've had no problems running the 1st > and 2nd asconf tests in these loops. I think you just hid the bug with this :) I.e. the test will almost always timeout on the recvmsg(), so it will pass even if SELinux doesn't deny access. I think you can verify it by uncommenting the corresponding neverallow rule, converting it to allow, and then re-testing. If I'm right, then the test will still pass after that. I played a bit with it myself, adding sleep in "educatedly guessed" places and I think I might have found the fix. I got the test to pass by adding a sleep(1) between the server's /* Send client the new primary address */ sctp_sendmsg() call and sctp_bindx() calls. My theory is that the client needs to call recvmsg() and start waiting for the message before the server switches the address, otherwise the expected timeout doesn't happen. If I add the missing permission, then the test fails, so this could be the right fix. What do you think? -- Ondrej Mosnacek Software Engineer, Platform Security - SELinux kernel Red Hat, Inc.
