| Message ID | 1478170479-22841-1-git-send-email-jobol@nonadev.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Nov 3, 2016 at 6:54 AM, <jobol@nonadev.net> wrote: > From: jooseong lee <jooseong.lee@samsung.com> > > Assign smack_known_web label for kernel thread's socket in the sk_alloc_security hook > > Creating struct sock by sk_alloc function in various kernel subsystems > like bluetooth dosen't call smack_socket_post_create(). In such case, > received sock label is the floor('_') label and makes access deny. > > Signed-off-by: jooseong lee <jooseong.lee@samsung.com> > --- > security/smack/smack_lsm.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) I may be wrong, but given that you are mentioning bluetooth I suspect you running into problems with the child sockets created by the bluetooth code (you are probably seeing similar issues with AF_ALG, AF_IUCV, AF_SCTP, AF_TIPC) as they don't go through the normal socket creation process that you see. Assuming that is the problem, we solve this in SELinux by leveraging the security_sk_clone() hook to clone the parent's security attributes to the newly created child socket; I realize this diverges from the Smack socket labeling method of inheriting from the task, but it seems like a reasonable approach. > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > index 1cb0602..e73460d 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -2337,8 +2337,16 @@ static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags) > if (ssp == NULL) > return -ENOMEM; > > - ssp->smk_in = skp; > - ssp->smk_out = skp; > + /* > + * Sockets created by kernel threads receive web label. > + */ > + if (unlikely(current->flags & PF_KTHREAD)) { > + ssp->smk_in = &smack_known_web; > + ssp->smk_out = &smack_known_web; > + } else { > + ssp->smk_in = skp; > + ssp->smk_out = skp; > + } > ssp->smk_packet = NULL; > > sk->sk_security = ssp; > -- > 2.7.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Le jeudi 03 novembre 2016 à 18:10 -0400, Paul Moore a écrit : > On Thu, Nov 3, 2016 at 6:54 AM, <jobol@nonadev.net> wrote: > > > > From: jooseong lee <jooseong.lee@samsung.com> > > > > Assign smack_known_web label for kernel thread's socket in the > > sk_alloc_security hook > > > > Creating struct sock by sk_alloc function in various kernel > > subsystems > > like bluetooth dosen't call smack_socket_post_create(). In such > > case, > > received sock label is the floor('_') label and makes access deny. > > > > Signed-off-by: jooseong lee <jooseong.lee@samsung.com> > > --- > > security/smack/smack_lsm.c | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > I may be wrong, but given that you are mentioning bluetooth I suspect > you running into problems with the child sockets created by the > bluetooth code (you are probably seeing similar issues with AF_ALG, > AF_IUCV, AF_SCTP, AF_TIPC) as they don't go through the normal socket > creation process that you see. Hello Paul, You are right. > Assuming that is the problem, we solve > this in SELinux by leveraging the security_sk_clone() hook to clone > the parent's security attributes to the newly created child socket; I > realize this diverges from the Smack socket labeling method of > inheriting from the task, but it seems like a reasonable approach. During some investigation, I sew the solution used by SELinux. Casey, what do you think about using security_sk_clone()? Wouldn't it be interesting for long term evolution to solve the same issue with the same solution? Best regards José > > > > diff --git a/security/smack/smack_lsm.c > > b/security/smack/smack_lsm.c > > index 1cb0602..e73460d 100644 > > --- a/security/smack/smack_lsm.c > > +++ b/security/smack/smack_lsm.c > > @@ -2337,8 +2337,16 @@ static int smack_sk_alloc_security(struct > > sock *sk, int family, gfp_t gfp_flags) > > if (ssp == NULL) > > return -ENOMEM; > > > > - ssp->smk_in = skp; > > - ssp->smk_out = skp; > > + /* > > + * Sockets created by kernel threads receive web label. > > + */ > > + if (unlikely(current->flags & PF_KTHREAD)) { > > + ssp->smk_in = &smack_known_web; > > + ssp->smk_out = &smack_known_web; > > + } else { > > + ssp->smk_in = skp; > > + ssp->smk_out = skp; > > + } > > ssp->smk_packet = NULL; > > > > sk->sk_security = ssp; > > -- > > 2.7.4 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux- > > security-module" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/4/2016 2:24 AM, José Bollo wrote: > Le jeudi 03 novembre 2016 à 18:10 -0400, Paul Moore a écrit : >> On Thu, Nov 3, 2016 at 6:54 AM, <jobol@nonadev.net> wrote: >>> From: jooseong lee <jooseong.lee@samsung.com> >>> >>> Assign smack_known_web label for kernel thread's socket in the >>> sk_alloc_security hook >>> >>> Creating struct sock by sk_alloc function in various kernel >>> subsystems >>> like bluetooth dosen't call smack_socket_post_create(). In such >>> case, >>> received sock label is the floor('_') label and makes access deny. >>> >>> Signed-off-by: jooseong lee <jooseong.lee@samsung.com> >>> --- >>> security/smack/smack_lsm.c | 12 ++++++++++-- >>> 1 file changed, 10 insertions(+), 2 deletions(-) >> I may be wrong, but given that you are mentioning bluetooth I suspect >> you running into problems with the child sockets created by the >> bluetooth code (you are probably seeing similar issues with AF_ALG, >> AF_IUCV, AF_SCTP, AF_TIPC) as they don't go through the normal socket >> creation process that you see. > Hello Paul, > > You are right. > >> Assuming that is the problem, we solve >> this in SELinux by leveraging the security_sk_clone() hook to clone >> the parent's security attributes to the newly created child socket; I >> realize this diverges from the Smack socket labeling method of >> inheriting from the task, but it seems like a reasonable approach. > During some investigation, I sew the solution used by SELinux. > Casey, what do you think about using security_sk_clone()? > Wouldn't it be interesting for long term evolution to solve the same > issue with the same solution? I don't see an advantage to doing it that way. Jobol's patch addresses the issue very simply and has been tested in Tizen. > > Best regards > José > > >>> diff --git a/security/smack/smack_lsm.c >>> b/security/smack/smack_lsm.c >>> index 1cb0602..e73460d 100644 >>> --- a/security/smack/smack_lsm.c >>> +++ b/security/smack/smack_lsm.c >>> @@ -2337,8 +2337,16 @@ static int smack_sk_alloc_security(struct >>> sock *sk, int family, gfp_t gfp_flags) >>> if (ssp == NULL) >>> return -ENOMEM; >>> >>> - ssp->smk_in = skp; >>> - ssp->smk_out = skp; >>> + /* >>> + * Sockets created by kernel threads receive web label. >>> + */ >>> + if (unlikely(current->flags & PF_KTHREAD)) { >>> + ssp->smk_in = &smack_known_web; >>> + ssp->smk_out = &smack_known_web; >>> + } else { >>> + ssp->smk_in = skp; >>> + ssp->smk_out = skp; >>> + } >>> ssp->smk_packet = NULL; >>> >>> sk->sk_security = ssp; >>> -- >>> 2.7.4 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux- >>> security-module" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 1cb0602..e73460d 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -2337,8 +2337,16 @@ static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags) if (ssp == NULL) return -ENOMEM; - ssp->smk_in = skp; - ssp->smk_out = skp; + /* + * Sockets created by kernel threads receive web label. + */ + if (unlikely(current->flags & PF_KTHREAD)) { + ssp->smk_in = &smack_known_web; + ssp->smk_out = &smack_known_web; + } else { + ssp->smk_in = skp; + ssp->smk_out = skp; + } ssp->smk_packet = NULL; sk->sk_security = ssp;