| Message ID | 20240815083229.42778-1-aha310510@gmail.com (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | selinux: support IPPROTO_SMC in socket_type_to_security_class() | expand |
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: > --
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 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
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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:
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(+) --