Re: [PATCH v2] selinux: rate-limit unrecognized netlink message warnings in selinux_nlmsg_perm()

Re: [PATCH v2] selinux: rate-limit unrecognized netlink message warnings in selinux_nlmsg_perm()

[Paul Moore]

On Wednesday, November 04, 2015 04:02:36 PM Vladis Dronov wrote:
> Any process is able to send netlink messages with invalid types.
> Make the warning rate-limited to prevent too much log spam.
> 
> The warning is supposed to help to find misbehaving programs, so
> print the triggering command name and pid.
> 
> Signed-off-by: Vladis Dronov <vdronov@redhat.com>
> ---
>  security/selinux/hooks.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

Did you want the "Reported-by" tag included?  I'm also adding the SELinux list 
back to the CC line.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e4369d8..3d8087d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4787,11 +4787,12 @@ static int selinux_nlmsg_perm(struct sock *sk,
> struct sk_buff *skb) err = selinux_nlmsg_lookup(sksec->sclass,
> nlh->nlmsg_type, &perm); if (err) {
>  		if (err == -EINVAL) {
> -			printk(KERN_WARNING
> -			       "SELinux: unrecognized netlink message:"
> -			       " protocol=%hu nlmsg_type=%hu sclass=%s\n",
> +			pr_warn_ratelimited("SELinux: unrecognized netlink"
> +			       " message: protocol=%hu nlmsg_type=%hu sclass=%s"
> +			       " from %s[%d]\n",
>  			       sk->sk_protocol, nlh->nlmsg_type,
> -			       secclass_map[sksec->sclass - 1].name);
> +			       secclass_map[sksec->sclass - 1].name,
> +			       current->comm, current->pid);
>  			if (!selinux_enforcing || security_get_allow_unknown())
>  				err = 0;
>  		}

-- 
paul moore
www.paul-moore.com

Re: [PATCH v2] selinux: rate-limit unrecognized netlink message warnings in selinux_nlmsg_perm()

[Vladis Dronov]

Hello, Paul.

>> Did you want the "Reported-by" tag included?  I'm also adding the SELinux list 
>> back to the CC line.

Yes, could you please, indeed include lost line "Reported-by: Florian Weimer <fweimer@redhat.com>".
It was my fault of omitting it while re-composing commit message.

Vladis Dronov | Red Hat, Inc.
| Product Security Engineer |

----- Original Message -----
From: "Paul Moore" <paul@paul-moore.com>
To: "Vladis Dronov" <vdronov@redhat.com>
Cc: linux-security-module@vger.kernel.org, selinux@tycho.nsa.gov
Sent: Friday, November 6, 2015 6:33:39 PM
Subject: Re: [PATCH v2] selinux: rate-limit unrecognized netlink message warnings in selinux_nlmsg_perm()

On Wednesday, November 04, 2015 04:02:36 PM Vladis Dronov wrote:
> Any process is able to send netlink messages with invalid types.
> Make the warning rate-limited to prevent too much log spam.
> 
> The warning is supposed to help to find misbehaving programs, so
> print the triggering command name and pid.
> 
> Signed-off-by: Vladis Dronov <vdronov@redhat.com>
> ---
>  security/selinux/hooks.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

Did you want the "Reported-by" tag included?  I'm also adding the SELinux list 
back to the CC line.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e4369d8..3d8087d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4787,11 +4787,12 @@ static int selinux_nlmsg_perm(struct sock *sk,
> struct sk_buff *skb) err = selinux_nlmsg_lookup(sksec->sclass,
> nlh->nlmsg_type, &perm); if (err) {
>  		if (err == -EINVAL) {
> -			printk(KERN_WARNING
> -			       "SELinux: unrecognized netlink message:"
> -			       " protocol=%hu nlmsg_type=%hu sclass=%s\n",
> +			pr_warn_ratelimited("SELinux: unrecognized netlink"
> +			       " message: protocol=%hu nlmsg_type=%hu sclass=%s"
> +			       " from %s[%d]\n",
>  			       sk->sk_protocol, nlh->nlmsg_type,
> -			       secclass_map[sksec->sclass - 1].name);
> +			       secclass_map[sksec->sclass - 1].name,
> +			       current->comm, current->pid);
>  			if (!selinux_enforcing || security_get_allow_unknown())
>  				err = 0;
>  		}

-- 
paul moore
www.paul-moore.com

Re: [PATCH v2] selinux: rate-limit unrecognized netlink message warnings in selinux_nlmsg_perm()

[Vladis Dronov]

Hello, Paul,
Thank you for the corrections and your time. I've just sent a next
patch version to the list. I hope it would be more acceptable.

Best regards,
Vladis Dronov | Red Hat, Inc. | Product Security Engineer

----- Original Message -----
From: "Paul Moore" <paul@paul-moore.com>
To: "Vladis Dronov" <vdronov@redhat.com>
Cc: selinux@tycho.nsa.gov, linux-security-module@vger.kernel.org
Sent: Wednesday, November 11, 2015 12:07:23 AM
Subject: Re: [PATCH v2] selinux: rate-limit unrecognized netlink message warnings in selinux_nlmsg_perm()

On Wednesday, November 04, 2015 11:35:51 AM Vladis Dronov wrote:
> Any process is able to send netlink messages with invalid types.
> Make the warning rate-limited to prevent too much log spam.
> 
> The warning is supposed to help to find misbehaving programs, so
> print the triggering command name and pid.
> 
> Reported-by: Florian Weimer <fweimer@redhat.com>
> Signed-off-by: Vladis Dronov <vdronov@redhat.com>
> ---
>  security/selinux/hooks.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e4369d8..3d8087d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4787,11 +4787,12 @@ static int selinux_nlmsg_perm(struct sock *sk,
> struct sk_buff *skb) err = selinux_nlmsg_lookup(sksec->sclass,
> nlh->nlmsg_type, &perm); if (err) {
>  		if (err == -EINVAL) {
> -			printk(KERN_WARNING
> -			       "SELinux: unrecognized netlink message:"
> -			       " protocol=%hu nlmsg_type=%hu sclass=%s\n",
> +			pr_warn_ratelimited("SELinux: unrecognized netlink"
> +			       " message: protocol=%hu nlmsg_type=%hu sclass=%s"
> +			       " from %s[%d]\n",
>  			       sk->sk_protocol, nlh->nlmsg_type,
> -			       secclass_map[sksec->sclass - 1].name);
> +			       secclass_map[sksec->sclass - 1].name,
> +			       current->comm, current->pid);

I should have looked at more closely at the actual changes earlier, but the 
other issues with your posting distracted me ... while this isn't an audit 
message, it tends to follow the audit-ish name=value format so let's stick 
with that in this message, I would also suggest using task_pid_nr() so the 
line should look something like this:

  pr_warn_ratelimited("SELinux: unrecognized netlink"
                      " message: protocol=%hu nlmsg_type=%hu sclass=%s"
                      " pig=%d comm=%s\n",
                      sk->sk_protocol, nlh->nlmsg_type,
                      secclass_map[sksec->sclass - 1].name,
                      task_pid_nr(current), current->comm);

-- 
paul moore
www.paul-moore.com

Re: [PATCH v2] selinux: rate-limit unrecognized netlink message warnings in selinux_nlmsg_perm()

[Paul Moore]

On Wednesday, November 04, 2015 11:35:51 AM Vladis Dronov wrote:
> Any process is able to send netlink messages with invalid types.
> Make the warning rate-limited to prevent too much log spam.
> 
> The warning is supposed to help to find misbehaving programs, so
> print the triggering command name and pid.
> 
> Reported-by: Florian Weimer <fweimer@redhat.com>
> Signed-off-by: Vladis Dronov <vdronov@redhat.com>
> ---
>  security/selinux/hooks.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e4369d8..3d8087d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4787,11 +4787,12 @@ static int selinux_nlmsg_perm(struct sock *sk,
> struct sk_buff *skb) err = selinux_nlmsg_lookup(sksec->sclass,
> nlh->nlmsg_type, &perm); if (err) {
>  		if (err == -EINVAL) {
> -			printk(KERN_WARNING
> -			       "SELinux: unrecognized netlink message:"
> -			       " protocol=%hu nlmsg_type=%hu sclass=%s\n",
> +			pr_warn_ratelimited("SELinux: unrecognized netlink"
> +			       " message: protocol=%hu nlmsg_type=%hu sclass=%s"
> +			       " from %s[%d]\n",
>  			       sk->sk_protocol, nlh->nlmsg_type,
> -			       secclass_map[sksec->sclass - 1].name);
> +			       secclass_map[sksec->sclass - 1].name,
> +			       current->comm, current->pid);

I should have looked at more closely at the actual changes earlier, but the 
other issues with your posting distracted me ... while this isn't an audit 
message, it tends to follow the audit-ish name=value format so let's stick 
with that in this message, I would also suggest using task_pid_nr() so the 
line should look something like this:

  pr_warn_ratelimited("SELinux: unrecognized netlink"
                      " message: protocol=%hu nlmsg_type=%hu sclass=%s"
                      " pig=%d comm=%s\n",
                      sk->sk_protocol, nlh->nlmsg_type,
                      secclass_map[sksec->sclass - 1].name,
                      task_pid_nr(current), current->comm);

-- 
paul moore
www.paul-moore.com

[PATCH v2] selinux: rate-limit unrecognized netlink message warnings in selinux_nlmsg_perm()

[Vladis Dronov]

Any process is able to send netlink messages with invalid types.
Make the warning rate-limited to prevent too much log spam.

The warning is supposed to help to find misbehaving programs, so
print the triggering command name and pid.

Reported-by: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Vladis Dronov <vdronov@redhat.com>
---
 security/selinux/hooks.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index e4369d8..3d8087d 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4787,11 +4787,12 @@ static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb)
 	err = selinux_nlmsg_lookup(sksec->sclass, nlh->nlmsg_type, &perm);
 	if (err) {
 		if (err == -EINVAL) {
-			printk(KERN_WARNING
-			       "SELinux: unrecognized netlink message:"
-			       " protocol=%hu nlmsg_type=%hu sclass=%s\n",
+			pr_warn_ratelimited("SELinux: unrecognized netlink"
+			       " message: protocol=%hu nlmsg_type=%hu sclass=%s"
+			       " from %s[%d]\n",
 			       sk->sk_protocol, nlh->nlmsg_type,
-			       secclass_map[sksec->sclass - 1].name);
+			       secclass_map[sksec->sclass - 1].name,
+			       current->comm, current->pid);
 			if (!selinux_enforcing || security_get_allow_unknown())
 				err = 0;
 		}
--