Re: [RFC v3 02/10] Revert "lsm: Remove the socket_post_accept() hook"

Tetsuo Handa [off-list ref] writes:

Paul Moore wrote:

quoted

On Tuesday, May 03, 2011 10:28:24 PM Tetsuo Handa wrote:

quoted

Paul Moore wrote:

quoted

On Tuesday, May 03, 2011 10:24:15 AM Samir Bellabes wrote:

quoted

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 [off-list ref]
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 [off-list ref]

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:

quoted

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

On Friday, May 06, 2011 5:25:45 AM Samir Bellabes wrote:

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).

Looking at the snet_socket_post_accept() hook, I believe all of the 
information you are looking for should be available to you in the sock_graft() 
hook.

--
paul moore
linux @ hp