Re: [GIT PULL] SELinux patches for v4.17
On Mon, Apr 9, 2018 at 6:44 AM, Richard Haines wrote: > On Sun, 2018-04-08 at 19:59 +0100, Richard Haines via Selinux wrote: >> On Mon, 2018-04-09 at 01:43 +0800, Xin Long wrote: >> > On Sun, Apr 8, 2018 at 10:09 PM, Richard Haines >> > wrote: >> > > On Sun, 2018-04-08 at 08:50 -0400, Paul Moore wrote: >> > > > On April 7, 2018 1:03:57 PM Linus Torvalds > > > > da >> > > > tion >> > > > .org> wrote: >> > > > On Sat, Apr 7, 2018 at 9:54 AM, Richard Haines >> > > > wrote: >> > > > >> > > > So please check my resolution, but also somebody should tell me >> > > > "Linus, you're a cretin, sctp_connect() doesn't want that >> > > > security_sctp_bind_connect() at all because it was already done >> > > > by >> > > > XYZ" >> > > > >> > > > sctp_connect() or __sctp_connect() do not need to call >> > > > security_sctp_bind_connect(). This is because the connect(2) >> > > > call >> > > > will >> > > > handle the checks required via security_socket_connect(): >> > > > >> > > > Ok, thanks, that's exactly what I wanted to get. >> > > > >> > > > Anyway, somebody should still verify that it all looks good in >> > > > my >> > > > tree, but I don't actually expect the merge to have had any >> > > > issues >> > > > even if the refactoring made it a bit more complex than most >> > > > merges >> > > > are. >> > > > >> > > > Thanks for the quick response Richard. >> > > > >> > > > Xin Long looked it over and gave it the thumbs up, I'll take a >> > > > look >> > > > too, but to be honest I trust his SCTP understanding much more >> > > > than >> > > > mine. I also do weekly tests of each rcX release at a minimum >> > > > so >> > > > if >> > > > something odd pops up I'll make sure you get a fix. >> > > > >> > > > Thanks again everyone. >> > > >> > > I built the kernel this morning and sorry to spoil the party, but >> > > I've >> > > run into a problem with lksctp-tools when running the func_tests: >> > > >> > > make v6test >> > > .. >> > > .. >> > > ./test_timetolive_v6 >> > > test_timetolive.c 0 INFO : Creating fillmsg of size 3087 >> > > test_timetolive.c 1 PASS : Send a message with timeout >> > > test_timetolive.c 2 PASS : Send a message with no timeout >> > > test_timetolive.c 3 PASS : Send a fragmented message with >> > > timeout >> > > test_timetolive.c 0 INFO : ** SLEEPING for 3 seconds ** >> > > test_timetolive.c 4 BROK : Got a datamsg of unexpected >> > > length:23, >> > > expected length:27 >> > > DUMP_CORE sctputil.c: 247 >> > > /bin/sh: line 1: 30981 Segmentation fault (core dumped) ./$a >> > > test_timetolive_v6 fails >> > > >> > > make v4 test fails the same way. I'm using lksctp-tools from [1]. >> > > I >> > > have not investigated the cause yet as just found this and >> > > thought >> > > I >> > > should flag first just in case someone has the answer !!! >> > >> > test_timetolive(_v6) works for me, In lksctp-tools/src/func_tests, >> > I >> > had >> > another case failed,./test_1_to_1_events, it's caused by: >> > commit 30f6ebf65bc46161c5aaff1db2e6e7c76aa4a06b >> > Author: Xin Long >> > Date: Wed Mar 14 19:05:34 2018 +0800 >> > >> > sctp: add SCTP_AUTH_NO_AUTH type for AUTHENTICATION_EVENT >> > >> > It's not kernel's issue, after that commit, ./test_1_to_1_events >> > should >> > have been improved. or avoid it by 'sysctl -w >> > net.sctp.auth_enable=1' >> > >> > I'm not sure why test_timetolive(_v6) is not working in your env. >> >> It appears to depend on the run sequence of the tests. I rebooted the >> system, ran test_timetolive_v6, it worked okay. >> Ran "sctp-tests run" on a terminal, then ran test_timetolive_v6 at >> various intervals on another terminal. Once sctp-tests started the >> "=== >> ndatasched ===" sequence, test_timetolive_v6 failed. > > 1) When SCTP is initialised /proc/sys/net/sctp/prsctp_enable = 1 > 2) When sctp-tests/testcase/regression/extoverflow/test.sh is executed, > on exit it sets prsctp_enable = 0. This seems to be causing the issue > I'm seeing. I can now simulate the problem: > > Running from fresh boot: > checksctp > cat /proc/sys/net/sctp/prsctp_enable > 1 > ./test_timetolive_v6 > passes > echo 0 > /proc/sys/net/sctp/prsctp_enable > ./test_timetolive_v6 > fails > echo 1 > /proc/sys/net/sctp/prsctp_enable > ./test_timetolive_v6 > passes I see ... commit 8ae808eb853e3789b81b8a502cdf22bb01b76880 Author: Xin Long Date: Sat Oct 8 11:40:16 2016 +0800 sctp: remove the old ttl expires policy ttl expire is considered as one of the prsctp policies after this commit, so prsctp_enable is required. I will think to update this test case in lksctp-tools. Thanks for the reproducer.
Re: [GIT PULL] SELinux patches for v4.17
On Sun, Apr 8, 2018 at 10:09 PM, Richard Haines wrote: > On Sun, 2018-04-08 at 08:50 -0400, Paul Moore wrote: >> On April 7, 2018 1:03:57 PM Linus Torvalds > .org> wrote: >> On Sat, Apr 7, 2018 at 9:54 AM, Richard Haines >> wrote: >> >> So please check my resolution, but also somebody should tell me >> "Linus, you're a cretin, sctp_connect() doesn't want that >> security_sctp_bind_connect() at all because it was already done by >> XYZ" >> >> sctp_connect() or __sctp_connect() do not need to call >> security_sctp_bind_connect(). This is because the connect(2) call >> will >> handle the checks required via security_socket_connect(): >> >> Ok, thanks, that's exactly what I wanted to get. >> >> Anyway, somebody should still verify that it all looks good in my >> tree, but I don't actually expect the merge to have had any issues >> even if the refactoring made it a bit more complex than most merges >> are. >> >> Thanks for the quick response Richard. >> >> Xin Long looked it over and gave it the thumbs up, I'll take a look >> too, but to be honest I trust his SCTP understanding much more than >> mine. I also do weekly tests of each rcX release at a minimum so if >> something odd pops up I'll make sure you get a fix. >> >> Thanks again everyone. > > I built the kernel this morning and sorry to spoil the party, but I've > run into a problem with lksctp-tools when running the func_tests: > > make v6test > .. > .. > ./test_timetolive_v6 > test_timetolive.c 0 INFO : Creating fillmsg of size 3087 > test_timetolive.c 1 PASS : Send a message with timeout > test_timetolive.c 2 PASS : Send a message with no timeout > test_timetolive.c 3 PASS : Send a fragmented message with timeout > test_timetolive.c 0 INFO : ** SLEEPING for 3 seconds ** > test_timetolive.c 4 BROK : Got a datamsg of unexpected length:23, > expected length:27 > DUMP_CORE sctputil.c: 247 > /bin/sh: line 1: 30981 Segmentation fault (core dumped) ./$a > test_timetolive_v6 fails > > make v4 test fails the same way. I'm using lksctp-tools from [1]. I > have not investigated the cause yet as just found this and thought I > should flag first just in case someone has the answer !!! test_timetolive(_v6) works for me, In lksctp-tools/src/func_tests, I had another case failed,./test_1_to_1_events, it's caused by: commit 30f6ebf65bc46161c5aaff1db2e6e7c76aa4a06b Author: Xin Long Date: Wed Mar 14 19:05:34 2018 +0800 sctp: add SCTP_AUTH_NO_AUTH type for AUTHENTICATION_EVENT It's not kernel's issue, after that commit, ./test_1_to_1_events should have been improved. or avoid it by 'sysctl -w net.sctp.auth_enable=1' I'm not sure why test_timetolive(_v6) is not working in your env. > > On the bright side, I've run the sctp-tests from [2] with no problems > and also the selinux-testsuite with my SCTP patch from [3] using an > updated Fedora policy from [4] (with sctp support added), all in > enforcing mode. > > Also the LTP test passed: > cd /opt/ltp/ > cat runtest/syscalls |grep connect01>runtest/connect-syscall > ./runltp -pq -f connect-syscall > > > [1] https://github.com/sctp/lksctp-tools > [2] https://github.com/sctp/sctp-tests > [3] https://marc.info/?l=selinux&m=152156947715709&w=2 > [4] https://github.com/fedora-selinux/selinux-policy > > >> >> -- >> paul moore >> www.paul-moore.com >> >> >>
Re: [GIT PULL] SELinux patches for v4.17
On Sat, Apr 7, 2018 at 7:07 AM, Linus Torvalds wrote: > On Tue, Apr 3, 2018 at 6:37 PM, Paul Moore wrote: >> >> Everything passes the selinux-testsuite, but there are a few known >> merge conflicts. The first is with the netdev tree and is in >> net/sctp/socket.c. Unfortunately it is a bit ugly, thankfully Stephen >> Rothwell has already done the heavy lifting in resolving the merge for >> you, and the SCTP folks have given his merge patch a thumbs-up. > > I ended up re-doing the merge, and it looks like some more sctp > changes happened after Stephen's merge anyway, so mine didn't end up > quite like his. > You're right, a sctp fix went into net-next after Stephen's that merge. https://patchwork.ozlabs.org/patch/884469/ (v1) https://patchwork.ozlabs.org/patch/884971/ (v2) Which caused the resolution to have changed again. I've checked the new resolution on your tree, the SCTP part is all good. Thanks.
Re: [RFC PATCH 3/5] sctp: Add LSM hooks
On Fri, Oct 20, 2017 at 8:04 PM, Richard Haines
wrote:
> On Fri, 2017-10-20 at 07:16 -0400, Neil Horman wrote:
>> On Wed, Oct 18, 2017 at 11:05:09PM +0800, Xin Long wrote:
>> > On Tue, Oct 17, 2017 at 9:58 PM, Richard Haines
>> > wrote:
>> > > Add security hooks to allow security modules to exercise access
>> > > control
>> > > over SCTP.
>> > >
>> > > Signed-off-by: Richard Haines
>> > > ---
>> > > include/net/sctp/structs.h | 10
>> > > include/uapi/linux/sctp.h | 1 +
>> > > net/sctp/sm_make_chunk.c | 12 +
>> > > net/sctp/sm_statefuns.c| 14 ++-
>> > > net/sctp/socket.c | 61
>> > > +-
>> > > 5 files changed, 96 insertions(+), 2 deletions(-)
>> > >
>> > > diff --git a/include/net/sctp/structs.h
>> > > b/include/net/sctp/structs.h
>> > > index 7767577..6e72e3e 100644
>> > > --- a/include/net/sctp/structs.h
>> > > +++ b/include/net/sctp/structs.h
>> > > @@ -1270,6 +1270,16 @@ struct sctp_endpoint {
>> > > reconf_enable:1;
>> > >
>> > > __u8 strreset_enable;
>> > > +
>> > > + /* Security identifiers from incoming (INIT). These are
>> > > set by
>> > > +* security_sctp_assoc_request(). These will only be used
>> > > by
>> > > +* SCTP TCP type sockets and peeled off connections as
>> > > they
>> > > +* cause a new socket to be generated.
>> > > security_sctp_sk_clone()
>> > > +* will then plug these into the new socket.
>> > > +*/
>> > > +
>> > > + u32 secid;
>> > > + u32 peer_secid;
>> > > };
>> > >
>> > > /* Recover the outter endpoint structure. */
>> > > diff --git a/include/uapi/linux/sctp.h
>> > > b/include/uapi/linux/sctp.h
>> > > index 6217ff8..c04812f 100644
>> > > --- a/include/uapi/linux/sctp.h
>> > > +++ b/include/uapi/linux/sctp.h
>> > > @@ -122,6 +122,7 @@ typedef __s32 sctp_assoc_t;
>> > > #define SCTP_RESET_ASSOC 120
>> > > #define SCTP_ADD_STREAMS 121
>> > > #define SCTP_SOCKOPT_PEELOFF_FLAGS 122
>> > > +#define SCTP_SENDMSG_CONNECT 123
>> > >
>> > > /* PR-SCTP policies */
>> > > #define SCTP_PR_SCTP_NONE 0x
>> > > diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
>> > > index 6110447..ca4705b 100644
>> > > --- a/net/sctp/sm_make_chunk.c
>> > > +++ b/net/sctp/sm_make_chunk.c
>> > > @@ -3059,6 +3059,12 @@ static __be16
>> > > sctp_process_asconf_param(struct sctp_association *asoc,
>> > > if (af->is_any(&addr))
>> > > memcpy(&addr, &asconf->source,
>> > > sizeof(addr));
>> > >
>> > > + if (security_sctp_bind_connect(asoc->ep->base.sk,
>> > > + SCTP_PARAM_ADD_IP,
>> > > + (struct sockaddr
>> > > *)&addr,
>> > > + af->sockaddr_len))
>> > > + return SCTP_ERROR_REQ_REFUSED;
>> > > +
>> > > /* ADDIP 4.3 D9) If an endpoint receives an ADD
>> > > IP address
>> > > * request and does not have the local resources
>> > > to add this
>> > > * new address to the association, it MUST return
>> > > an Error
>> > > @@ -3125,6 +3131,12 @@ static __be16
>> > > sctp_process_asconf_param(struct sctp_association *asoc,
>> > > if (af->is_any(&addr))
>> > > memcpy(&addr.v4, sctp_source(asconf),
>> > > sizeof(addr));
>> > >
>> > > + if (security_sctp_bind_connect(asoc->ep->base.sk,
>> > > + SCTP_PARAM_SET_PRI
>> > > MARY,
>> > > + (struct sockaddr
>> > > *)&addr,
>> > > + af->sockaddr_len))
>> > > +
Re: [RFC PATCH 3/5] sctp: Add LSM hooks
On Tue, Oct 17, 2017 at 9:58 PM, Richard Haines
wrote:
> Add security hooks to allow security modules to exercise access control
> over SCTP.
>
> Signed-off-by: Richard Haines
> ---
> include/net/sctp/structs.h | 10
> include/uapi/linux/sctp.h | 1 +
> net/sctp/sm_make_chunk.c | 12 +
> net/sctp/sm_statefuns.c| 14 ++-
> net/sctp/socket.c | 61
> +-
> 5 files changed, 96 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index 7767577..6e72e3e 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -1270,6 +1270,16 @@ struct sctp_endpoint {
> reconf_enable:1;
>
> __u8 strreset_enable;
> +
> + /* Security identifiers from incoming (INIT). These are set by
> +* security_sctp_assoc_request(). These will only be used by
> +* SCTP TCP type sockets and peeled off connections as they
> +* cause a new socket to be generated. security_sctp_sk_clone()
> +* will then plug these into the new socket.
> +*/
> +
> + u32 secid;
> + u32 peer_secid;
> };
>
> /* Recover the outter endpoint structure. */
> diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> index 6217ff8..c04812f 100644
> --- a/include/uapi/linux/sctp.h
> +++ b/include/uapi/linux/sctp.h
> @@ -122,6 +122,7 @@ typedef __s32 sctp_assoc_t;
> #define SCTP_RESET_ASSOC 120
> #define SCTP_ADD_STREAMS 121
> #define SCTP_SOCKOPT_PEELOFF_FLAGS 122
> +#define SCTP_SENDMSG_CONNECT 123
>
> /* PR-SCTP policies */
> #define SCTP_PR_SCTP_NONE 0x
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index 6110447..ca4705b 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -3059,6 +3059,12 @@ static __be16 sctp_process_asconf_param(struct
> sctp_association *asoc,
> if (af->is_any(&addr))
> memcpy(&addr, &asconf->source, sizeof(addr));
>
> + if (security_sctp_bind_connect(asoc->ep->base.sk,
> + SCTP_PARAM_ADD_IP,
> + (struct sockaddr *)&addr,
> + af->sockaddr_len))
> + return SCTP_ERROR_REQ_REFUSED;
> +
> /* ADDIP 4.3 D9) If an endpoint receives an ADD IP address
> * request and does not have the local resources to add this
> * new address to the association, it MUST return an Error
> @@ -3125,6 +3131,12 @@ static __be16 sctp_process_asconf_param(struct
> sctp_association *asoc,
> if (af->is_any(&addr))
> memcpy(&addr.v4, sctp_source(asconf), sizeof(addr));
>
> + if (security_sctp_bind_connect(asoc->ep->base.sk,
> + SCTP_PARAM_SET_PRIMARY,
> + (struct sockaddr *)&addr,
> + af->sockaddr_len))
> + return SCTP_ERROR_REQ_REFUSED;
> +
> peer = sctp_assoc_lookup_paddr(asoc, &addr);
> if (!peer)
> return SCTP_ERROR_DNS_FAILED;
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index b2a74c3..4ba5805 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -314,6 +314,11 @@ sctp_disposition_t sctp_sf_do_5_1B_init(struct net *net,
> sctp_unrecognized_param_t *unk_param;
> int len;
>
> + /* Update socket peer label if first association. */
> + if (security_sctp_assoc_request((struct sctp_endpoint *)ep,
> + chunk->skb, SCTP_CID_INIT))
> + return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
> +
> /* 6.10 Bundling
> * An endpoint MUST NOT bundle INIT, INIT ACK or
> * SHUTDOWN COMPLETE with any other chunks.
> @@ -446,7 +451,6 @@ sctp_disposition_t sctp_sf_do_5_1B_init(struct net *net,
> }
>
> sctp_add_cmd_sf(commands, SCTP_CMD_NEW_ASOC, SCTP_ASOC(new_asoc));
> -
> sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(repl));
>
> /*
> @@ -507,6 +511,11 @@ sctp_disposition_t sctp_sf_do_5_1C_ack(struct net *net,
> struct sctp_chunk *err_chunk;
> struct sctp_packet *packet;
>
> + /* Update socket peer label if first association. */
> + if (security_sctp_assoc_request((struct sctp_endpoint *)ep,
> + chunk->skb, SCTP_CID_INIT_ACK))
> + return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
> +
Just thinking security_sctp_assoc_request hook should also be in
sctp_sf_do_unexpected_init() and sctp_sf_do_5_2_4_dupcook() ?
