public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* selinux: support IPPROTO_SMC in socket_type_to_security_class()
@ 2024-08-15  8:32 Jeongjun Park
  2024-08-15 16:28 ` Stephen Smalley
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Jeongjun Park @ 2024-08-15  8:32 UTC (permalink / raw)
  To: paul, stephen.smalley.work, omosnace; +Cc: selinux, linux-kernel, Jeongjun Park

IPPROTO_SMC feature has been added to net/smc. It is now possible to 
create smc sockets in the following way:

  /* create v4 smc sock */
  v4 = socket(AF_INET, SOCK_STREAM, IPPROTO_SMC);

  /* create v6 smc sock */
  v6 = socket(AF_INET6, SOCK_STREAM, IPPROTO_SMC);

Therefore, we need to add code to support IPPROTO_SMC in 
socket_type_to_security_class().

Signed-off-by: Jeongjun Park <aha310510@gmail.com>
---
 security/selinux/hooks.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index bfa61e005aac..36f951f0c574 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1176,6 +1176,8 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc
 				return SECCLASS_TCP_SOCKET;
 			else if (extsockclass && protocol == IPPROTO_SCTP)
 				return SECCLASS_SCTP_SOCKET;
+			else if (extsockclass && protocol == IPPROTO_SMC)
+				return SECCLASS_SMC_SOCKET;
 			else
 				return SECCLASS_RAWIP_SOCKET;
 		case SOCK_DGRAM:
--

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: selinux: support IPPROTO_SMC in socket_type_to_security_class()
  2024-08-15  8:32 selinux: support IPPROTO_SMC in socket_type_to_security_class() Jeongjun Park
@ 2024-08-15 16:28 ` Stephen Smalley
  2024-08-16  1:46 ` Paul Moore
  2024-08-19  9:46 ` Ondrej Mosnacek
  2 siblings, 0 replies; 14+ messages in thread
From: Stephen Smalley @ 2024-08-15 16:28 UTC (permalink / raw)
  To: Jeongjun Park; +Cc: paul, omosnace, selinux, linux-kernel

On Thu, Aug 15, 2024 at 4:32 AM Jeongjun Park <aha310510@gmail.com> wrote:
>
> IPPROTO_SMC feature has been added to net/smc. It is now possible to
> create smc sockets in the following way:
>
>   /* create v4 smc sock */
>   v4 = socket(AF_INET, SOCK_STREAM, IPPROTO_SMC);
>
>   /* create v6 smc sock */
>   v6 = socket(AF_INET6, SOCK_STREAM, IPPROTO_SMC);
>
> Therefore, we need to add code to support IPPROTO_SMC in
> socket_type_to_security_class().
>
> Signed-off-by: Jeongjun Park <aha310510@gmail.com>

Fixes: d25a92ccae6bed02327b63d138e12e7806830f78 ("net/smc: Introduce
IPPROTO_SMC")
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

> ---
>  security/selinux/hooks.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index bfa61e005aac..36f951f0c574 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1176,6 +1176,8 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc
>                                 return SECCLASS_TCP_SOCKET;
>                         else if (extsockclass && protocol == IPPROTO_SCTP)
>                                 return SECCLASS_SCTP_SOCKET;
> +                       else if (extsockclass && protocol == IPPROTO_SMC)
> +                               return SECCLASS_SMC_SOCKET;
>                         else
>                                 return SECCLASS_RAWIP_SOCKET;
>                 case SOCK_DGRAM:
> --

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: selinux: support IPPROTO_SMC in socket_type_to_security_class()
  2024-08-15  8:32 selinux: support IPPROTO_SMC in socket_type_to_security_class() Jeongjun Park
  2024-08-15 16:28 ` Stephen Smalley
@ 2024-08-16  1:46 ` Paul Moore
  2024-08-16  3:57   ` Jeongjun Park
  2024-08-19  9:46 ` Ondrej Mosnacek
  2 siblings, 1 reply; 14+ messages in thread
From: Paul Moore @ 2024-08-16  1:46 UTC (permalink / raw)
  To: Jeongjun Park; +Cc: stephen.smalley.work, omosnace, selinux, linux-kernel

On Thu, Aug 15, 2024 at 4:32 AM Jeongjun Park <aha310510@gmail.com> wrote:
>
> IPPROTO_SMC feature has been added to net/smc. It is now possible to
> create smc sockets in the following way:
>
>   /* create v4 smc sock */
>   v4 = socket(AF_INET, SOCK_STREAM, IPPROTO_SMC);
>
>   /* create v6 smc sock */
>   v6 = socket(AF_INET6, SOCK_STREAM, IPPROTO_SMC);
>
> Therefore, we need to add code to support IPPROTO_SMC in
> socket_type_to_security_class().
>
> Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> ---
>  security/selinux/hooks.c | 2 ++
>  1 file changed, 2 insertions(+)

I'm a little concerned that the small patch below might not be all
that is needed to properly support SMC in SELinux.  Can you explain
what testing you've done with SMC on a SELinux system?

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index bfa61e005aac..36f951f0c574 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1176,6 +1176,8 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc
>                                 return SECCLASS_TCP_SOCKET;
>                         else if (extsockclass && protocol == IPPROTO_SCTP)
>                                 return SECCLASS_SCTP_SOCKET;
> +                       else if (extsockclass && protocol == IPPROTO_SMC)
> +                               return SECCLASS_SMC_SOCKET;
>                         else
>                                 return SECCLASS_RAWIP_SOCKET;
>                 case SOCK_DGRAM:

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: selinux: support IPPROTO_SMC in socket_type_to_security_class()
  2024-08-16  1:46 ` Paul Moore
@ 2024-08-16  3:57   ` Jeongjun Park
  2024-08-16 11:21     ` Stephen Smalley
  0 siblings, 1 reply; 14+ messages in thread
From: Jeongjun Park @ 2024-08-16  3:57 UTC (permalink / raw)
  To: paul; +Cc: aha310510, linux-kernel, omosnace, selinux, stephen.smalley.work

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 2094 bytes --]

Paul Moore wrote:
>
> On Thu, Aug 15, 2024 at 4:32 AM Jeongjun Park <aha310510@gmail.com> wrote:
> >
> > IPPROTO_SMC feature has been added to net/smc. It is now possible to
> > create smc sockets in the following way:
> >
> >   /* create v4 smc sock */
> >   v4 = socket(AF_INET, SOCK_STREAM, IPPROTO_SMC);
> >
> >   /* create v6 smc sock */
> >   v6 = socket(AF_INET6, SOCK_STREAM, IPPROTO_SMC);
> >
> > Therefore, we need to add code to support IPPROTO_SMC in
> > socket_type_to_security_class().
> >
> > Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> > ---
> >  security/selinux/hooks.c | 2 ++
> >  1 file changed, 2 insertions(+)
>
> I'm a little concerned that the small patch below might not be all
> that is needed to properly support SMC in SELinux.  Can you explain
> what testing you've done with SMC on a SELinux system?

I don't have much knowledge about smc, so I can't tested everything, but 
I created a socket, performed setsockopt, and tested two sockets 
communicating with each other. When I tested it, performing smc-related 
functions worked well without any major problems.

And after analyzing it myself, I didn't see any additional patches needed 
to support IPPROTO_SMC in selinux other than this patch. So you don't 
have to worry.

Regards,
Jeongjun Park

>
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index bfa61e005aac..36f951f0c574 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -1176,6 +1176,8 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc
> >                                 return SECCLASS_TCP_SOCKET;
> >                         else if (extsockclass && protocol == IPPROTO_SCTP)
> >                                 return SECCLASS_SCTP_SOCKET;
> > +                       else if (extsockclass && protocol == IPPROTO_SMC)
> > +                               return SECCLASS_SMC_SOCKET;
> >                         else
> >                                 return SECCLASS_RAWIP_SOCKET;
> >                 case SOCK_DGRAM:
>
> --
> paul-moore.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: selinux: support IPPROTO_SMC in socket_type_to_security_class()
  2024-08-16  3:57   ` Jeongjun Park
@ 2024-08-16 11:21     ` Stephen Smalley
  2024-08-16 11:42       ` Stephen Smalley
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Smalley @ 2024-08-16 11:21 UTC (permalink / raw)
  To: Jeongjun Park; +Cc: paul, linux-kernel, omosnace, selinux

On Thu, Aug 15, 2024 at 11:57 PM Jeongjun Park <aha310510@gmail.com> wrote:
>
> Paul Moore wrote:
> >
> > On Thu, Aug 15, 2024 at 4:32 AM Jeongjun Park <aha310510@gmail.com> wrote:
> > >
> > > IPPROTO_SMC feature has been added to net/smc. It is now possible to
> > > create smc sockets in the following way:
> > >
> > >   /* create v4 smc sock */
> > >   v4 = socket(AF_INET, SOCK_STREAM, IPPROTO_SMC);
> > >
> > >   /* create v6 smc sock */
> > >   v6 = socket(AF_INET6, SOCK_STREAM, IPPROTO_SMC);
> > >
> > > Therefore, we need to add code to support IPPROTO_SMC in
> > > socket_type_to_security_class().
> > >
> > > Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> > > ---
> > >  security/selinux/hooks.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> >
> > I'm a little concerned that the small patch below might not be all
> > that is needed to properly support SMC in SELinux.  Can you explain
> > what testing you've done with SMC on a SELinux system?
>
> I don't have much knowledge about smc, so I can't tested everything, but
> I created a socket, performed setsockopt, and tested two sockets
> communicating with each other. When I tested it, performing smc-related
> functions worked well without any major problems.
>
> And after analyzing it myself, I didn't see any additional patches needed
> to support IPPROTO_SMC in selinux other than this patch. So you don't
> have to worry.

Note that Jeongjun is not introducing SELinux support for SMC sockets
for the first time here; he is just updating the already existing
support to correctly map the new IPPROTO_SMC to the already existing
SECCLASS_SMC_SOCKET. We were already handling such sockets created via
socket(AF_SMC, ...); what changed was that they added support for
creating them via socket(AF_INET, SOCK_STREAM, IPPROTO_SMC) too.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: selinux: support IPPROTO_SMC in socket_type_to_security_class()
  2024-08-16 11:21     ` Stephen Smalley
@ 2024-08-16 11:42       ` Stephen Smalley
  2024-08-16 12:53         ` Stephen Smalley
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Smalley @ 2024-08-16 11:42 UTC (permalink / raw)
  To: Jeongjun Park; +Cc: paul, linux-kernel, omosnace, selinux

On Fri, Aug 16, 2024 at 7:21 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Thu, Aug 15, 2024 at 11:57 PM Jeongjun Park <aha310510@gmail.com> wrote:
> >
> > Paul Moore wrote:
> > >
> > > On Thu, Aug 15, 2024 at 4:32 AM Jeongjun Park <aha310510@gmail.com> wrote:
> > > >
> > > > IPPROTO_SMC feature has been added to net/smc. It is now possible to
> > > > create smc sockets in the following way:
> > > >
> > > >   /* create v4 smc sock */
> > > >   v4 = socket(AF_INET, SOCK_STREAM, IPPROTO_SMC);
> > > >
> > > >   /* create v6 smc sock */
> > > >   v6 = socket(AF_INET6, SOCK_STREAM, IPPROTO_SMC);
> > > >
> > > > Therefore, we need to add code to support IPPROTO_SMC in
> > > > socket_type_to_security_class().
> > > >
> > > > Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> > > > ---
> > > >  security/selinux/hooks.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > >
> > > I'm a little concerned that the small patch below might not be all
> > > that is needed to properly support SMC in SELinux.  Can you explain
> > > what testing you've done with SMC on a SELinux system?
> >
> > I don't have much knowledge about smc, so I can't tested everything, but
> > I created a socket, performed setsockopt, and tested two sockets
> > communicating with each other. When I tested it, performing smc-related
> > functions worked well without any major problems.
> >
> > And after analyzing it myself, I didn't see any additional patches needed
> > to support IPPROTO_SMC in selinux other than this patch. So you don't
> > have to worry.
>
> Note that Jeongjun is not introducing SELinux support for SMC sockets
> for the first time here; he is just updating the already existing
> support to correctly map the new IPPROTO_SMC to the already existing
> SECCLASS_SMC_SOCKET. We were already handling such sockets created via
> socket(AF_SMC, ...); what changed was that they added support for
> creating them via socket(AF_INET, SOCK_STREAM, IPPROTO_SMC) too.

Also, the extent of the support is limited to just the socket layer
checks, but this is not a change and is no different than many of the
other AF_* families besides the small number that have been more
specifically instrumented for SELinux.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: selinux: support IPPROTO_SMC in socket_type_to_security_class()
  2024-08-16 11:42       ` Stephen Smalley
@ 2024-08-16 12:53         ` Stephen Smalley
  0 siblings, 0 replies; 14+ messages in thread
From: Stephen Smalley @ 2024-08-16 12:53 UTC (permalink / raw)
  To: Jeongjun Park; +Cc: paul, linux-kernel, omosnace, selinux

On Fri, Aug 16, 2024 at 7:42 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Fri, Aug 16, 2024 at 7:21 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Thu, Aug 15, 2024 at 11:57 PM Jeongjun Park <aha310510@gmail.com> wrote:
> > >
> > > Paul Moore wrote:
> > > >
> > > > On Thu, Aug 15, 2024 at 4:32 AM Jeongjun Park <aha310510@gmail.com> wrote:
> > > > >
> > > > > IPPROTO_SMC feature has been added to net/smc. It is now possible to
> > > > > create smc sockets in the following way:
> > > > >
> > > > >   /* create v4 smc sock */
> > > > >   v4 = socket(AF_INET, SOCK_STREAM, IPPROTO_SMC);
> > > > >
> > > > >   /* create v6 smc sock */
> > > > >   v6 = socket(AF_INET6, SOCK_STREAM, IPPROTO_SMC);
> > > > >
> > > > > Therefore, we need to add code to support IPPROTO_SMC in
> > > > > socket_type_to_security_class().
> > > > >
> > > > > Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> > > > > ---
> > > > >  security/selinux/hooks.c | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > >
> > > > I'm a little concerned that the small patch below might not be all
> > > > that is needed to properly support SMC in SELinux.  Can you explain
> > > > what testing you've done with SMC on a SELinux system?
> > >
> > > I don't have much knowledge about smc, so I can't tested everything, but
> > > I created a socket, performed setsockopt, and tested two sockets
> > > communicating with each other. When I tested it, performing smc-related
> > > functions worked well without any major problems.
> > >
> > > And after analyzing it myself, I didn't see any additional patches needed
> > > to support IPPROTO_SMC in selinux other than this patch. So you don't
> > > have to worry.
> >
> > Note that Jeongjun is not introducing SELinux support for SMC sockets
> > for the first time here; he is just updating the already existing
> > support to correctly map the new IPPROTO_SMC to the already existing
> > SECCLASS_SMC_SOCKET. We were already handling such sockets created via
> > socket(AF_SMC, ...); what changed was that they added support for
> > creating them via socket(AF_INET, SOCK_STREAM, IPPROTO_SMC) too.
>
> Also, the extent of the support is limited to just the socket layer
> checks, but this is not a change and is no different than many of the
> other AF_* families besides the small number that have been more
> specifically instrumented for SELinux.

Normally, this would be exercised by
selinux-testsuite/tests/extended_socket_class but we didn't include
SMC testing there originally because SMC sockets depend on INFINIBAND.
However, looking at the kconfig options, it appears that perhaps we
could test it locally via CONFIG_SMC_LO=y if we enable that along with
CONFIG_SMC and CONFIG_INFINIBAND in the selinux-testsuite/defconfig.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: selinux: support IPPROTO_SMC in socket_type_to_security_class()
  2024-08-15  8:32 selinux: support IPPROTO_SMC in socket_type_to_security_class() Jeongjun Park
  2024-08-15 16:28 ` Stephen Smalley
  2024-08-16  1:46 ` Paul Moore
@ 2024-08-19  9:46 ` Ondrej Mosnacek
  2024-08-20 18:24   ` Stephen Smalley
  2 siblings, 1 reply; 14+ messages in thread
From: Ondrej Mosnacek @ 2024-08-19  9:46 UTC (permalink / raw)
  To: Jeongjun Park; +Cc: paul, stephen.smalley.work, selinux, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 3343 bytes --]

On Thu, Aug 15, 2024 at 10:32 AM Jeongjun Park <aha310510@gmail.com> wrote:
>
> IPPROTO_SMC feature has been added to net/smc. It is now possible to
> create smc sockets in the following way:
>
>   /* create v4 smc sock */
>   v4 = socket(AF_INET, SOCK_STREAM, IPPROTO_SMC);
>
>   /* create v6 smc sock */
>   v6 = socket(AF_INET6, SOCK_STREAM, IPPROTO_SMC);
>
> Therefore, we need to add code to support IPPROTO_SMC in
> socket_type_to_security_class().
>
> Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> ---
>  security/selinux/hooks.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index bfa61e005aac..36f951f0c574 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1176,6 +1176,8 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc
>                                 return SECCLASS_TCP_SOCKET;
>                         else if (extsockclass && protocol == IPPROTO_SCTP)
>                                 return SECCLASS_SCTP_SOCKET;
> +                       else if (extsockclass && protocol == IPPROTO_SMC)
> +                               return SECCLASS_SMC_SOCKET;
>                         else
>                                 return SECCLASS_RAWIP_SOCKET;
>                 case SOCK_DGRAM:
> --
>

I'm not sure if this is the solution we want to go with... Consider
the following from af_smc(7):

>   Usage modes
>      Two usage modes are possible:
>
>      AF_SMC native usage
>             uses the socket domain AF_SMC instead of AF_INET and AF_INET6.  Specify SMCPROTO_SMC for AF_INET compatible socket semantics, and SMC_PROTO_SMC6 for AF_INET6 respectively.
>
>      Usage of AF_INET socket applications with SMC preload library
>             converts AF_INET and AF_INET6 sockets to AF_SMC sockets.  The SMC preload library is part of the SMC tools package.
>
>      SMC socket capabilities are negotiated at connection setup. If one peer is not SMC capable, further socket processing falls back to TCP usage automatically.

This means that the SMC sockets are intended to be used (also) as a
drop-in compatible replacement for normal TCP sockets in applications
and they even fall back to TCP when the endpoints fail to negotiate
communication via SMC. That's a situation similar to MPTCP, where we
just mapped MPTCP sockets to the tcp_socket SELinux class, so that
MPTCP can be swapped in place of TCP transparently without having to
do extensive policy changes. We may want to consider the same/similar
approach here.

I briefly played with this idea a couple of months ago, when I was
asked by someone at Red Hat about SMC sockets and their integration
with SELinux. IIRC, when I tried to implement the MPTCP approach and
adjusted the selinux-testsuite to test SMC similarly as TCP and MPTCP,
I saw that the netlabel-related tests (may have been more, I don't
remember) weren't passing out of the box like with MPTCP. However, the
person then didn't follow up on my questions, so I didn't look into it
further...

I'm attaching the WIP patches I worked with, in case someone would
like to continue the experiments.

--
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.

[-- Attachment #2: kernel-smc-as-tcp.patch --]
[-- Type: text/x-patch, Size: 1890 bytes --]

commit e6dca1fd3c713eaf69ff16fb23c6dc683082a2f8
Author: Ondrej Mosnacek <omosnace@redhat.com>
Date:   Tue Feb 6 15:39:21 2024 +0100

    TEST

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 55c78c318ccd..a5db62130d41 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1292,7 +1292,7 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc
 		case PF_QIPCRTR:
 			return SECCLASS_QIPCRTR_SOCKET;
 		case PF_SMC:
-			return SECCLASS_SMC_SOCKET;
+			return SECCLASS_TCP_SOCKET;
 		case PF_XDP:
 			return SECCLASS_XDP_SOCKET;
 		case PF_MCTP:
@@ -4772,6 +4772,7 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
 			}
 		}
 
+		// FIXME: do the same here
 		switch (sksec->sclass) {
 		case SECCLASS_TCP_SOCKET:
 			node_perm = TCP_SOCKET__NODE_BIND;
@@ -4852,6 +4853,7 @@ static int selinux_socket_connect_helper(struct socket *sock,
 		struct sockaddr_in6 *addr6 = NULL;
 		unsigned short snum;
 		u32 sid, perm;
+		u8 protocol;
 
 		/* sctp_connectx(3) calls via selinux_sctp_bind_connect()
 		 * that validates multiple connect addresses. Because of this
@@ -4881,22 +4883,25 @@ static int selinux_socket_connect_helper(struct socket *sock,
 				return -EAFNOSUPPORT;
 		}
 
-		err = sel_netport_sid(sk->sk_protocol, snum, &sid);
-		if (err)
-			return err;
-
 		switch (sksec->sclass) {
 		case SECCLASS_TCP_SOCKET:
+			protocol = IPPROTO_TCP;
 			perm = TCP_SOCKET__NAME_CONNECT;
 			break;
 		case SECCLASS_DCCP_SOCKET:
+			protocol = IPPROTO_DCCP;
 			perm = DCCP_SOCKET__NAME_CONNECT;
 			break;
 		case SECCLASS_SCTP_SOCKET:
+			protocol = IPPROTO_SCTP;
 			perm = SCTP_SOCKET__NAME_CONNECT;
 			break;
 		}
 
+		err = sel_netport_sid(protocol, snum, &sid);
+		if (err)
+			return err;
+
 		ad.type = LSM_AUDIT_DATA_NET;
 		ad.u.net = &net;
 		ad.u.net->dport = htons(snum);

[-- Attachment #3: testsuite-smc-as-tcp.patch --]
[-- Type: text/x-patch, Size: 2807 bytes --]

commit 23fd91fff6b4004a3e0e5e7a0b6ca5a97be3d20b
Author: Ondrej Mosnacek <omosnace@redhat.com>
Date:   Tue Feb 6 15:12:39 2024 +0100

    WIP

diff --git a/tests/inet_socket/client.c b/tests/inet_socket/client.c
index d3fedf4..188d8ce 100644
--- a/tests/inet_socket/client.c
+++ b/tests/inet_socket/client.c
@@ -64,6 +64,9 @@ int main(int argc, char **argv)
 		hints.ai_socktype = SOCK_STREAM;
 		hints.ai_protocol = IPPROTO_TCP;
 		sockprotocol      = IPPROTO_MPTCP;
+	} else if (!strcmp(argv[optind], "smc")) {
+		hints.ai_socktype = SOCK_STREAM;
+		hints.ai_protocol = IPPROTO_TCP;
 	} else if (!strcmp(argv[optind], "udp")) {
 		hints.ai_socktype = SOCK_DGRAM;
 		hints.ai_protocol = IPPROTO_UDP;
@@ -79,6 +82,20 @@ int main(int argc, char **argv)
 		exit(2);
 	}
 
+	if (!strcmp(argv[optind], "smc")) {
+		switch (serverinfo->ai_family) {
+		case AF_INET:
+			sockprotocol = 0; /* SMC_PROTO_SMC */
+			break;
+		case AF_INET6:
+			sockprotocol = 1; /* SMC_PROTO_SMC6 */
+			break;
+		default:
+			usage(argv[0]);
+		}
+		serverinfo->ai_family = AF_SMC;
+	}
+
 	sock = socket(serverinfo->ai_family, serverinfo->ai_socktype,
 		      sockprotocol);
 	if (sock < 0) {
diff --git a/tests/inet_socket/server.c b/tests/inet_socket/server.c
index 63b6849..baa3c93 100644
--- a/tests/inet_socket/server.c
+++ b/tests/inet_socket/server.c
@@ -74,6 +74,10 @@ int main(int argc, char **argv)
 		hints.ai_socktype = SOCK_STREAM;
 		hints.ai_protocol = IPPROTO_TCP;
 		sockprotocol      = IPPROTO_MPTCP;
+	} else if (!strcmp(argv[optind], "smc")) {
+		hints.ai_socktype = SOCK_STREAM;
+		hints.ai_protocol = IPPROTO_TCP;
+		sockprotocol      = 1; /* SMC_PROTO_SMC6 */
 	} else if (!strcmp(argv[optind], "udp")) {
 		hints.ai_socktype = SOCK_DGRAM;
 		hints.ai_protocol = IPPROTO_UDP;
@@ -88,6 +92,9 @@ int main(int argc, char **argv)
 		exit(1);
 	}
 
+	if (!strcmp(argv[optind], "smc"))
+		res->ai_family = AF_SMC;
+
 	sock = socket(res->ai_family, res->ai_socktype, sockprotocol);
 	if (sock < 0) {
 		perror("socket");
diff --git a/tests/inet_socket/smc b/tests/inet_socket/smc
new file mode 120000
index 0000000..945c9b4
--- /dev/null
+++ b/tests/inet_socket/smc
@@ -0,0 +1 @@
+.
\ No newline at end of file
diff --git a/tests/inet_socket/test b/tests/inet_socket/test
index 08c7b1d..545988a 100755
--- a/tests/inet_socket/test
+++ b/tests/inet_socket/test
@@ -8,7 +8,12 @@ BEGIN {
     chomp($basedir);
     $proto = basename($basedir);
 
-    if ( $proto eq "tcp" or $proto eq "mptcp" ) {
+    # check if SMC is enabled
+    if ( $proto eq "smc" and system("modprobe smc 2>/dev/null") != 0 ) {
+        plan skip_all => "SMC protocol not supported";
+    }
+
+    if ( $proto eq "tcp" or $proto eq "mptcp" or $proto eq "smc" ) {
         $is_stream   = 1;
         $fail_value1 = 5;
         $fail_value2 = 5;

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: selinux: support IPPROTO_SMC in socket_type_to_security_class()
  2024-08-19  9:46 ` Ondrej Mosnacek
@ 2024-08-20 18:24   ` Stephen Smalley
  2024-08-20 19:51     ` Paul Moore
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Smalley @ 2024-08-20 18:24 UTC (permalink / raw)
  To: Ondrej Mosnacek; +Cc: Jeongjun Park, paul, selinux, linux-kernel

On Mon, Aug 19, 2024 at 5:46 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> On Thu, Aug 15, 2024 at 10:32 AM Jeongjun Park <aha310510@gmail.com> wrote:
> >
> > IPPROTO_SMC feature has been added to net/smc. It is now possible to
> > create smc sockets in the following way:
> >
> >   /* create v4 smc sock */
> >   v4 = socket(AF_INET, SOCK_STREAM, IPPROTO_SMC);
> >
> >   /* create v6 smc sock */
> >   v6 = socket(AF_INET6, SOCK_STREAM, IPPROTO_SMC);
> >
> > Therefore, we need to add code to support IPPROTO_SMC in
> > socket_type_to_security_class().
> >
> > Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> > ---
> >  security/selinux/hooks.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index bfa61e005aac..36f951f0c574 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -1176,6 +1176,8 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc
> >                                 return SECCLASS_TCP_SOCKET;
> >                         else if (extsockclass && protocol == IPPROTO_SCTP)
> >                                 return SECCLASS_SCTP_SOCKET;
> > +                       else if (extsockclass && protocol == IPPROTO_SMC)
> > +                               return SECCLASS_SMC_SOCKET;
> >                         else
> >                                 return SECCLASS_RAWIP_SOCKET;
> >                 case SOCK_DGRAM:
> > --
> >
>
> I'm not sure if this is the solution we want to go with... Consider
> the following from af_smc(7):
>
> >   Usage modes
> >      Two usage modes are possible:
> >
> >      AF_SMC native usage
> >             uses the socket domain AF_SMC instead of AF_INET and AF_INET6.  Specify SMCPROTO_SMC for AF_INET compatible socket semantics, and SMC_PROTO_SMC6 for AF_INET6 respectively.
> >
> >      Usage of AF_INET socket applications with SMC preload library
> >             converts AF_INET and AF_INET6 sockets to AF_SMC sockets.  The SMC preload library is part of the SMC tools package.
> >
> >      SMC socket capabilities are negotiated at connection setup. If one peer is not SMC capable, further socket processing falls back to TCP usage automatically.
>
> This means that the SMC sockets are intended to be used (also) as a
> drop-in compatible replacement for normal TCP sockets in applications
> and they even fall back to TCP when the endpoints fail to negotiate
> communication via SMC. That's a situation similar to MPTCP, where we
> just mapped MPTCP sockets to the tcp_socket SELinux class, so that
> MPTCP can be swapped in place of TCP transparently without having to
> do extensive policy changes. We may want to consider the same/similar
> approach here.
>
> I briefly played with this idea a couple of months ago, when I was
> asked by someone at Red Hat about SMC sockets and their integration
> with SELinux. IIRC, when I tried to implement the MPTCP approach and
> adjusted the selinux-testsuite to test SMC similarly as TCP and MPTCP,
> I saw that the netlabel-related tests (may have been more, I don't
> remember) weren't passing out of the box like with MPTCP. However, the
> person then didn't follow up on my questions, so I didn't look into it
> further...
>
> I'm attaching the WIP patches I worked with, in case someone would
> like to continue the experiments.

I am not in favor of your approach, for the following reasons:
1. It would be backward-incompatible with any current code using
AF_SMC (although this could be addressed by making it conditional on a
new policy capability, so this is not too difficult to overcome),
2. It would not allow any distinction to ever be made in policy
between SMC sockets and TCP sockets, so we could never allow one
without the other.

Hence, I am still in favor of Jeongjun's patch to consistently treat
AF_SMC and (AF_INET, SOCK_STREAM, IPPROTO_SMC) sockets, and then if
someone wants to extend that support to also provide more complete
access controls and/or networking labeling, defer that to a future
patch.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: selinux: support IPPROTO_SMC in socket_type_to_security_class()
  2024-08-20 18:24   ` Stephen Smalley
@ 2024-08-20 19:51     ` Paul Moore
  2024-08-21 13:37       ` Stephen Smalley
  0 siblings, 1 reply; 14+ messages in thread
From: Paul Moore @ 2024-08-20 19:51 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: Ondrej Mosnacek, Jeongjun Park, selinux, linux-kernel

On Tue, Aug 20, 2024 at 2:24 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> On Mon, Aug 19, 2024 at 5:46 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > On Thu, Aug 15, 2024 at 10:32 AM Jeongjun Park <aha310510@gmail.com> wrote:
> > >
> > > IPPROTO_SMC feature has been added to net/smc. It is now possible to
> > > create smc sockets in the following way:
> > >
> > >   /* create v4 smc sock */
> > >   v4 = socket(AF_INET, SOCK_STREAM, IPPROTO_SMC);
> > >
> > >   /* create v6 smc sock */
> > >   v6 = socket(AF_INET6, SOCK_STREAM, IPPROTO_SMC);
> > >
> > > Therefore, we need to add code to support IPPROTO_SMC in
> > > socket_type_to_security_class().
> > >
> > > Signed-off-by: Jeongjun Park <aha310510@gmail.com>
> > > ---
> > >  security/selinux/hooks.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > > index bfa61e005aac..36f951f0c574 100644
> > > --- a/security/selinux/hooks.c
> > > +++ b/security/selinux/hooks.c
> > > @@ -1176,6 +1176,8 @@ static inline u16 socket_type_to_security_class(int family, int type, int protoc
> > >                                 return SECCLASS_TCP_SOCKET;
> > >                         else if (extsockclass && protocol == IPPROTO_SCTP)
> > >                                 return SECCLASS_SCTP_SOCKET;
> > > +                       else if (extsockclass && protocol == IPPROTO_SMC)
> > > +                               return SECCLASS_SMC_SOCKET;
> > >                         else
> > >                                 return SECCLASS_RAWIP_SOCKET;
> > >                 case SOCK_DGRAM:
> > > --
> > >
> >
> > I'm not sure if this is the solution we want to go with... Consider
> > the following from af_smc(7):
> >
> > >   Usage modes
> > >      Two usage modes are possible:
> > >
> > >      AF_SMC native usage
> > >             uses the socket domain AF_SMC instead of AF_INET and AF_INET6.  Specify SMCPROTO_SMC for AF_INET compatible socket semantics, and SMC_PROTO_SMC6 for AF_INET6 respectively.
> > >
> > >      Usage of AF_INET socket applications with SMC preload library
> > >             converts AF_INET and AF_INET6 sockets to AF_SMC sockets.  The SMC preload library is part of the SMC tools package.
> > >
> > >      SMC socket capabilities are negotiated at connection setup. If one peer is not SMC capable, further socket processing falls back to TCP usage automatically.
> >
> > This means that the SMC sockets are intended to be used (also) as a
> > drop-in compatible replacement for normal TCP sockets in applications
> > and they even fall back to TCP when the endpoints fail to negotiate
> > communication via SMC. That's a situation similar to MPTCP, where we
> > just mapped MPTCP sockets to the tcp_socket SELinux class, so that
> > MPTCP can be swapped in place of TCP transparently without having to
> > do extensive policy changes. We may want to consider the same/similar
> > approach here.
> >
> > I briefly played with this idea a couple of months ago, when I was
> > asked by someone at Red Hat about SMC sockets and their integration
> > with SELinux. IIRC, when I tried to implement the MPTCP approach and
> > adjusted the selinux-testsuite to test SMC similarly as TCP and MPTCP,
> > I saw that the netlabel-related tests (may have been more, I don't
> > remember) weren't passing out of the box like with MPTCP. However, the
> > person then didn't follow up on my questions, so I didn't look into it
> > further...
> >
> > I'm attaching the WIP patches I worked with, in case someone would
> > like to continue the experiments.
>
> I am not in favor of your approach, for the following reasons:
> 1. It would be backward-incompatible with any current code using
> AF_SMC (although this could be addressed by making it conditional on a
> new policy capability, so this is not too difficult to overcome),
> 2. It would not allow any distinction to ever be made in policy
> between SMC sockets and TCP sockets, so we could never allow one
> without the other.
>
> Hence, I am still in favor of Jeongjun's patch to consistently treat
> AF_SMC and (AF_INET, SOCK_STREAM, IPPROTO_SMC) sockets, and then if
> someone wants to extend that support to also provide more complete
> access controls and/or networking labeling, defer that to a future
> patch.

Without passing any judgement on the patches Ondrej submitted (I tend
to ignore patches as attachments for various reasons), I do share
Ondrej's concerns that this may not be as simple as suggested in the
original patch in this thread.  I saw the same thing as Ondrej
regarding the TCP fallback and that immediately raised a number of
questions that I don't believe have been properly addressed yet.

Someone needs to dig into how the standard SMC protocol works first to
ensure we have the necessary access controls for the current code; my
guess is that we are probably okay since the socket-level controls are
fairly generic, but I'm not sure we've actually seen proper
confirmation that everything is good from a conceptual standpoint.
Once that is done, we need to examine how the TCP fallback works,
specifically how are connections managed and are the existing TCP
hooks sufficient for SMC (the early connection state stuff can be
tricky) and how to distinguish between normal-TCP and SMC-TCP.

Basically I'm looking for some basic design concepts and not simply a
passing test without any understanding of why/how it passed.

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: selinux: support IPPROTO_SMC in socket_type_to_security_class()
  2024-08-20 19:51     ` Paul Moore
@ 2024-08-21 13:37       ` Stephen Smalley
  2024-08-21 20:01         ` Paul Moore
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Smalley @ 2024-08-21 13:37 UTC (permalink / raw)
  To: Paul Moore; +Cc: Ondrej Mosnacek, Jeongjun Park, selinux, linux-kernel

On Tue, Aug 20, 2024 at 3:51 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Tue, Aug 20, 2024 at 2:24 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> > On Mon, Aug 19, 2024 at 5:46 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > I'm not sure if this is the solution we want to go with... Consider
> > > the following from af_smc(7):
> > >
> > > >   Usage modes
> > > >      Two usage modes are possible:
> > > >
> > > >      AF_SMC native usage
> > > >             uses the socket domain AF_SMC instead of AF_INET and AF_INET6.  Specify SMCPROTO_SMC for AF_INET compatible socket semantics, and SMC_PROTO_SMC6 for AF_INET6 respectively.
> > > >
> > > >      Usage of AF_INET socket applications with SMC preload library
> > > >             converts AF_INET and AF_INET6 sockets to AF_SMC sockets.  The SMC preload library is part of the SMC tools package.
> > > >
> > > >      SMC socket capabilities are negotiated at connection setup. If one peer is not SMC capable, further socket processing falls back to TCP usage automatically.
> > >
> > > This means that the SMC sockets are intended to be used (also) as a
> > > drop-in compatible replacement for normal TCP sockets in applications
> > > and they even fall back to TCP when the endpoints fail to negotiate
> > > communication via SMC. That's a situation similar to MPTCP, where we
> > > just mapped MPTCP sockets to the tcp_socket SELinux class, so that
> > > MPTCP can be swapped in place of TCP transparently without having to
> > > do extensive policy changes. We may want to consider the same/similar
> > > approach here.
> > >
> > > I briefly played with this idea a couple of months ago, when I was
> > > asked by someone at Red Hat about SMC sockets and their integration
> > > with SELinux. IIRC, when I tried to implement the MPTCP approach and
> > > adjusted the selinux-testsuite to test SMC similarly as TCP and MPTCP,
> > > I saw that the netlabel-related tests (may have been more, I don't
> > > remember) weren't passing out of the box like with MPTCP. However, the
> > > person then didn't follow up on my questions, so I didn't look into it
> > > further...
> > >
> > > I'm attaching the WIP patches I worked with, in case someone would
> > > like to continue the experiments.
> >
> > I am not in favor of your approach, for the following reasons:
> > 1. It would be backward-incompatible with any current code using
> > AF_SMC (although this could be addressed by making it conditional on a
> > new policy capability, so this is not too difficult to overcome),
> > 2. It would not allow any distinction to ever be made in policy
> > between SMC sockets and TCP sockets, so we could never allow one
> > without the other.
> >
> > Hence, I am still in favor of Jeongjun's patch to consistently treat
> > AF_SMC and (AF_INET, SOCK_STREAM, IPPROTO_SMC) sockets, and then if
> > someone wants to extend that support to also provide more complete
> > access controls and/or networking labeling, defer that to a future
> > patch.
>
> Without passing any judgement on the patches Ondrej submitted (I tend
> to ignore patches as attachments for various reasons), I do share
> Ondrej's concerns that this may not be as simple as suggested in the
> original patch in this thread.  I saw the same thing as Ondrej
> regarding the TCP fallback and that immediately raised a number of
> questions that I don't believe have been properly addressed yet.
>
> Someone needs to dig into how the standard SMC protocol works first to
> ensure we have the necessary access controls for the current code; my
> guess is that we are probably okay since the socket-level controls are
> fairly generic, but I'm not sure we've actually seen proper
> confirmation that everything is good from a conceptual standpoint.
> Once that is done, we need to examine how the TCP fallback works,
> specifically how are connections managed and are the existing TCP
> hooks sufficient for SMC (the early connection state stuff can be
> tricky) and how to distinguish between normal-TCP and SMC-TCP.
>
> Basically I'm looking for some basic design concepts and not simply a
> passing test without any understanding of why/how it passed.

At present, we are already applying the general socket layer access
controls to AF_SMC sockets; hence, existing policies can prevent or
allow use of AF_SMC sockets through that mechanism. This is useful for
reducing kernel attack surface, e.g. prevent all use of AF_SMC by
untrusted code, or to limit use of AF_SMC to specific
processes/programs.

Since kernel commit d25a92ccae6bed02327b63d138e12e7806830f78
("net/smc: Introduce IPPROTO_SMC"), there is a way to bypass such
controls by creating such sockets via (AF_INET, SOCK_STREAM,
IPPROTO_SMC) instead of AF_SMC. In that situation, any process that is
allowed the socket layer permissions to the generic socket class would
be allowed to create/use SMC sockets.

Jeongjun's patch closes this bypass and ensures consistent application
of the general socket layer access controls for SMC sockets. Given
that, I don't see why we would defer merging it until someone figures
out a more complete solution for SMC sockets. It's more of a bug fix
than an enhancement.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: selinux: support IPPROTO_SMC in socket_type_to_security_class()
  2024-08-21 13:37       ` Stephen Smalley
@ 2024-08-21 20:01         ` Paul Moore
  2024-08-29 13:50           ` Stephen Smalley
  0 siblings, 1 reply; 14+ messages in thread
From: Paul Moore @ 2024-08-21 20:01 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: Ondrej Mosnacek, Jeongjun Park, selinux, linux-kernel

On Wed, Aug 21, 2024 at 9:38 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> On Tue, Aug 20, 2024 at 3:51 PM Paul Moore <paul@paul-moore.com> wrote:

...

> > Without passing any judgement on the patches Ondrej submitted (I tend
> > to ignore patches as attachments for various reasons), I do share
> > Ondrej's concerns that this may not be as simple as suggested in the
> > original patch in this thread.  I saw the same thing as Ondrej
> > regarding the TCP fallback and that immediately raised a number of
> > questions that I don't believe have been properly addressed yet.
> >
> > Someone needs to dig into how the standard SMC protocol works first to
> > ensure we have the necessary access controls for the current code; my
> > guess is that we are probably okay since the socket-level controls are
> > fairly generic, but I'm not sure we've actually seen proper
> > confirmation that everything is good from a conceptual standpoint.
> > Once that is done, we need to examine how the TCP fallback works,
> > specifically how are connections managed and are the existing TCP
> > hooks sufficient for SMC (the early connection state stuff can be
> > tricky) and how to distinguish between normal-TCP and SMC-TCP.
> >
> > Basically I'm looking for some basic design concepts and not simply a
> > passing test without any understanding of why/how it passed.
>
> At present, we are already applying the general socket layer access
> controls to AF_SMC sockets; hence, existing policies can prevent or
> allow use of AF_SMC sockets through that mechanism. This is useful for
> reducing kernel attack surface, e.g. prevent all use of AF_SMC by
> untrusted code, or to limit use of AF_SMC to specific
> processes/programs.

That's true.  I'm not suggesting we revert what we currently have, I'm
only expressing some caution about moving forward with
AF_INET/IPPROTO_SMC without a better understanding.  Ideally we would
have done so before adding AF_SMC support, but we didn't, or at least
I don't recall much discussion at the time.

> Since kernel commit d25a92ccae6bed02327b63d138e12e7806830f78
> ("net/smc: Introduce IPPROTO_SMC"), there is a way to bypass such
> controls by creating such sockets via (AF_INET, SOCK_STREAM,
> IPPROTO_SMC) instead of AF_SMC. In that situation, any process that is
> allowed the socket layer permissions to the generic socket class would
> be allowed to create/use SMC sockets.
>
> Jeongjun's patch closes this bypass and ensures consistent application
> of the general socket layer access controls for SMC sockets. Given
> that, I don't see why we would defer merging it until someone figures
> out a more complete solution for SMC sockets. It's more of a bug fix
> than an enhancement.

SCTP, that's why.  Granted, SCTP is likely a far more complicated
protocol than SMC, but the TCP fallback raises all sorts of complexity
red flags in my mind.  Before we go further with SMC I want to see
some evidence that someone has looked through the SMC protocol and can
write a few coherent paragraphs about how the SELinux access controls
for the SMC protocol should work.

... and yes, labeled SCTP is still broken.  Perhaps someday soon I'll
have the time to finish the patchset to fix it.

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: selinux: support IPPROTO_SMC in socket_type_to_security_class()
  2024-08-21 20:01         ` Paul Moore
@ 2024-08-29 13:50           ` Stephen Smalley
  2024-08-30 20:04             ` Paul Moore
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Smalley @ 2024-08-29 13:50 UTC (permalink / raw)
  To: Paul Moore; +Cc: Ondrej Mosnacek, Jeongjun Park, selinux, linux-kernel

On Wed, Aug 21, 2024 at 4:02 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Wed, Aug 21, 2024 at 9:38 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> > On Tue, Aug 20, 2024 at 3:51 PM Paul Moore <paul@paul-moore.com> wrote:
>
> ...
>
> > > Without passing any judgement on the patches Ondrej submitted (I tend
> > > to ignore patches as attachments for various reasons), I do share
> > > Ondrej's concerns that this may not be as simple as suggested in the
> > > original patch in this thread.  I saw the same thing as Ondrej
> > > regarding the TCP fallback and that immediately raised a number of
> > > questions that I don't believe have been properly addressed yet.
> > >
> > > Someone needs to dig into how the standard SMC protocol works first to
> > > ensure we have the necessary access controls for the current code; my
> > > guess is that we are probably okay since the socket-level controls are
> > > fairly generic, but I'm not sure we've actually seen proper
> > > confirmation that everything is good from a conceptual standpoint.
> > > Once that is done, we need to examine how the TCP fallback works,
> > > specifically how are connections managed and are the existing TCP
> > > hooks sufficient for SMC (the early connection state stuff can be
> > > tricky) and how to distinguish between normal-TCP and SMC-TCP.
> > >
> > > Basically I'm looking for some basic design concepts and not simply a
> > > passing test without any understanding of why/how it passed.
> >
> > At present, we are already applying the general socket layer access
> > controls to AF_SMC sockets; hence, existing policies can prevent or
> > allow use of AF_SMC sockets through that mechanism. This is useful for
> > reducing kernel attack surface, e.g. prevent all use of AF_SMC by
> > untrusted code, or to limit use of AF_SMC to specific
> > processes/programs.
>
> That's true.  I'm not suggesting we revert what we currently have, I'm
> only expressing some caution about moving forward with
> AF_INET/IPPROTO_SMC without a better understanding.  Ideally we would
> have done so before adding AF_SMC support, but we didn't, or at least
> I don't recall much discussion at the time.
>
> > Since kernel commit d25a92ccae6bed02327b63d138e12e7806830f78
> > ("net/smc: Introduce IPPROTO_SMC"), there is a way to bypass such
> > controls by creating such sockets via (AF_INET, SOCK_STREAM,
> > IPPROTO_SMC) instead of AF_SMC. In that situation, any process that is
> > allowed the socket layer permissions to the generic socket class would
> > be allowed to create/use SMC sockets.
> >
> > Jeongjun's patch closes this bypass and ensures consistent application
> > of the general socket layer access controls for SMC sockets. Given
> > that, I don't see why we would defer merging it until someone figures
> > out a more complete solution for SMC sockets. It's more of a bug fix
> > than an enhancement.
>
> SCTP, that's why.  Granted, SCTP is likely a far more complicated
> protocol than SMC, but the TCP fallback raises all sorts of complexity
> red flags in my mind.  Before we go further with SMC I want to see
> some evidence that someone has looked through the SMC protocol and can
> write a few coherent paragraphs about how the SELinux access controls
> for the SMC protocol should work.
>
> ... and yes, labeled SCTP is still broken.  Perhaps someday soon I'll
> have the time to finish the patchset to fix it.

I see this as being different than SCTP. The AF_SMC support was
introduced with the extended_socket_class support which merely
introduced distinct socket security classes for each address family so
that SELinux can distinguish each address family in policy. Previously
a number of the socket address families were defaulting to the generic
socket security class and could not be distinguished in policy. The
only change was introducing a distinct security class specifically for
SMC sockets, not introducing any family/protocol-specific logic. Only
the socket layer access controls were being applied (both before and
after the introduction of the SMC socket class, just changing which
class was being used). Fixing the kernel to also map IPPROTO_SMC to
the SMC socket class likewise just ensure consistent application of
the socket layer access controls on SMC sockets regardless of how they
are created and doesn't introduce any SMC-specific logic.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: selinux: support IPPROTO_SMC in socket_type_to_security_class()
  2024-08-29 13:50           ` Stephen Smalley
@ 2024-08-30 20:04             ` Paul Moore
  0 siblings, 0 replies; 14+ messages in thread
From: Paul Moore @ 2024-08-30 20:04 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: Ondrej Mosnacek, Jeongjun Park, selinux, linux-kernel

On Thu, Aug 29, 2024 at 9:51 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Wed, Aug 21, 2024 at 4:02 PM Paul Moore <paul@paul-moore.com> wrote:
> >
> > On Wed, Aug 21, 2024 at 9:38 AM Stephen Smalley
> > <stephen.smalley.work@gmail.com> wrote:
> > > On Tue, Aug 20, 2024 at 3:51 PM Paul Moore <paul@paul-moore.com> wrote:
> >
> > ...
> >
> > > > Without passing any judgement on the patches Ondrej submitted (I tend
> > > > to ignore patches as attachments for various reasons), I do share
> > > > Ondrej's concerns that this may not be as simple as suggested in the
> > > > original patch in this thread.  I saw the same thing as Ondrej
> > > > regarding the TCP fallback and that immediately raised a number of
> > > > questions that I don't believe have been properly addressed yet.
> > > >
> > > > Someone needs to dig into how the standard SMC protocol works first to
> > > > ensure we have the necessary access controls for the current code; my
> > > > guess is that we are probably okay since the socket-level controls are
> > > > fairly generic, but I'm not sure we've actually seen proper
> > > > confirmation that everything is good from a conceptual standpoint.
> > > > Once that is done, we need to examine how the TCP fallback works,
> > > > specifically how are connections managed and are the existing TCP
> > > > hooks sufficient for SMC (the early connection state stuff can be
> > > > tricky) and how to distinguish between normal-TCP and SMC-TCP.
> > > >
> > > > Basically I'm looking for some basic design concepts and not simply a
> > > > passing test without any understanding of why/how it passed.
> > >
> > > At present, we are already applying the general socket layer access
> > > controls to AF_SMC sockets; hence, existing policies can prevent or
> > > allow use of AF_SMC sockets through that mechanism. This is useful for
> > > reducing kernel attack surface, e.g. prevent all use of AF_SMC by
> > > untrusted code, or to limit use of AF_SMC to specific
> > > processes/programs.
> >
> > That's true.  I'm not suggesting we revert what we currently have, I'm
> > only expressing some caution about moving forward with
> > AF_INET/IPPROTO_SMC without a better understanding.  Ideally we would
> > have done so before adding AF_SMC support, but we didn't, or at least
> > I don't recall much discussion at the time.
> >
> > > Since kernel commit d25a92ccae6bed02327b63d138e12e7806830f78
> > > ("net/smc: Introduce IPPROTO_SMC"), there is a way to bypass such
> > > controls by creating such sockets via (AF_INET, SOCK_STREAM,
> > > IPPROTO_SMC) instead of AF_SMC. In that situation, any process that is
> > > allowed the socket layer permissions to the generic socket class would
> > > be allowed to create/use SMC sockets.
> > >
> > > Jeongjun's patch closes this bypass and ensures consistent application
> > > of the general socket layer access controls for SMC sockets. Given
> > > that, I don't see why we would defer merging it until someone figures
> > > out a more complete solution for SMC sockets. It's more of a bug fix
> > > than an enhancement.
> >
> > SCTP, that's why.  Granted, SCTP is likely a far more complicated
> > protocol than SMC, but the TCP fallback raises all sorts of complexity
> > red flags in my mind.  Before we go further with SMC I want to see
> > some evidence that someone has looked through the SMC protocol and can
> > write a few coherent paragraphs about how the SELinux access controls
> > for the SMC protocol should work.
> >
> > ... and yes, labeled SCTP is still broken.  Perhaps someday soon I'll
> > have the time to finish the patchset to fix it.
>
> I see this as being different than SCTP. The AF_SMC support was
> introduced with the extended_socket_class support which merely
> introduced distinct socket security classes for each address family so
> that SELinux can distinguish each address family in policy. Previously
> a number of the socket address families were defaulting to the generic
> socket security class and could not be distinguished in policy. The
> only change was introducing a distinct security class specifically for
> SMC sockets, not introducing any family/protocol-specific logic. Only
> the socket layer access controls were being applied (both before and
> after the introduction of the SMC socket class, just changing which
> class was being used). Fixing the kernel to also map IPPROTO_SMC to
> the SMC socket class likewise just ensure consistent application of
> the socket layer access controls on SMC sockets regardless of how they
> are created and doesn't introduce any SMC-specific logic.

Possibly.  However, I don't feel like it is a very big ask to have
someone spend some time to provide a basic summary of the SMC protocol
and what might be needed from a SELinux perspective; that would help
increase my confidence in our SMC support significantly and perhaps
then I would feel comfortable with the simple mapping originally
proposed here.

-- 
paul-moore.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-08-30 20:05 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-15  8:32 selinux: support IPPROTO_SMC in socket_type_to_security_class() Jeongjun Park
2024-08-15 16:28 ` Stephen Smalley
2024-08-16  1:46 ` Paul Moore
2024-08-16  3:57   ` Jeongjun Park
2024-08-16 11:21     ` Stephen Smalley
2024-08-16 11:42       ` Stephen Smalley
2024-08-16 12:53         ` Stephen Smalley
2024-08-19  9:46 ` Ondrej Mosnacek
2024-08-20 18:24   ` Stephen Smalley
2024-08-20 19:51     ` Paul Moore
2024-08-21 13:37       ` Stephen Smalley
2024-08-21 20:01         ` Paul Moore
2024-08-29 13:50           ` Stephen Smalley
2024-08-30 20:04             ` Paul Moore

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox