netdev - Re: [RFC v3 02/10] Revert "lsm: Remove the socket_post

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87iptop4di.fsf@synack.fr>
Date:	Fri, 06 May 2011 11:25:45 +0200
From:	Samir Bellabes <sam@...ack.fr>
To:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Cc:	paul.moore@...com, linux-security-module@...r.kernel.org,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	netfilter-devel@...r.kernel.org, hadi@...erus.ca, kaber@...sh.net,
	zbr@...emap.net, root@...aldomain.pl
Subject: Re: [RFC v3 02/10] Revert "lsm: Remove the socket_post_accept() hook"

Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp> writes:

> Paul Moore wrote:
>> On Tuesday, May 03, 2011 10:28:24 PM Tetsuo Handa wrote:
>> > Paul Moore wrote:
>> > > On Tuesday, May 03, 2011 10:24:15 AM Samir Bellabes wrote:
>> > > > snet needs to reintroduce this hook, as it was designed to be: a hook
>> > > > for updating security informations on objects.
>> > > 
>> > > Looking at this and 5/10 again, it seems that you should be able to do
>> > > what you need with the sock_graft() hook.  Am I missing something?
>> > > 
>> > > My apologies if we've already discussed this approach previously ...
>> > 
>> > Second problem is that genlmsg_unicast() might return -EAGAIN because we
>> > can't sleep inside write_lock_bh()/write_unlock_bh().
>> 
>> Ah yes, the real problem.  I forgot that snet relied on a user space tool.  I 
>> tend to agree with others who have suggested this is not the right approach, 
>> but I understand why you want the post_accept() hook; thanks for reminding me.
>> 
> However, it sounds that Samir says genlmsg_unicast() failure is not fatal.

Actually, if the request to userspace is lost, no retransmission occurs.
there is a timeout to protect this case, and at the end of the tiemout,
a default verdict is apply. So no LSM decision is lost.

for the case of not checking return values, I fixed this in v4 with this
patch : 

commit 955d0a69c31684703dbeb1b15a462b06d4c79b52
Author: Samir Bellabes <sam@...ack.fr>
Date:   Thu May 5 12:36:58 2011 +0200

    snet: fix returned value of snet_do_verdict() and
    snet_do_send_event()
    
    Signed-off-by: Samir Bellabes <sam@...ack.fr>

diff --git a/security/snet/snet_hooks.c b/security/snet/snet_hooks.c
index 84ea5fc..5eb3848 100644
--- a/security/snet/snet_hooks.c
+++ b/security/snet/snet_hooks.c
@@ -67,23 +67,22 @@ static inline int snet_check_listeners(enum
snet_verdict *verdict)
        return 0;
 }
 
-static int snet_do_verdict(enum snet_verdict *verdict, struct snet_info
 *info)
+static void snet_do_verdict(enum snet_verdict *verdict, struct
snet_info *info)
 {
        if (info->verdict_id == 0)
-               return -1;
+               return;
        /* sending networking informations to userspace */
        if (snet_nl_send_event(info) == 0)
                /* waiting for userspace reply or timeout */
                *verdict = snet_verdict_wait(info->verdict_id);
        /* removing verdict */
        snet_verdict_remove(info->verdict_id);
-       return 0;
+       return;
 }
 
-static void snet_do_send_event(struct snet_info *info)
+static int snet_do_send_event(struct snet_info *info)
 {
-       snet_nl_send_event(info);
-       return;
+       return snet_nl_send_event(info);
 }
 
 /*

and introduce the statistics mecanism to count errors on all hooks
(statistics are available in /proc/snet/snet_stats)
for example:
                if (snet_do_send_event(&info) < 0)
                        SNET_STATS_INC(SNET_STATS_REG_ERROR,
                                       SNET_SOCKET_POST_ACCEPT);
                else
                        SNET_STATS_INC(SNET_STATS_REG_GRANT,
                                       SNET_SOCKET_POST_ACCEPT);

> Samir Bellabes wrote:
>> using snet_do_send_event() means that system is sending data to
>> userspace. the system is not waiting for a verdict from userspace.
>> 
>> If error occurs, we actually loose the information data.
>> I may be able to write a solution which try to send the data again, but
>> we need a exit solution for this loop (a number of try ?).
>
> If genlmsg_unicast() failure is not fatal, snet doesn't need the
> socket_post_accept hook. Samir, is genlmsg_unicast() failure fatal for snet?
> (Although, I'd like to ask for revival of the hook for TOMOYO anyway.)

the main argument for socket_post_accept is to known informations of the
remote inet.

from socket_accept(), we have no clue of who (inet->daddr and
inet->saddr) is connecting to the local service.
with socket_post_accept(), inet->daddr and inet->saddr are filled with
the true distant informations.

This informations is interesting for next security operations on the
socket. (we known with who we are talking to).


thanks,
sam
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html