netdev - Re: [RFC 6/9] snet: introduce snet_hooks.c and snet

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: <m2skanphoi.fsf@ssh.synack.fr>
Date:	Sun, 03 Jan 2010 12:10:53 +0100
From:	Samir Bellabes <sam@...ack.fr>
To:	Evgeniy Polyakov <zbr@...emap.net>
Cc:	linux-security-module@...r.kernel.org,
	Patrick McHardy <kaber@...sh.net>, jamal <hadi@...erus.ca>,
	Neil Horman <nhorman@...driver.com>, netdev@...r.kernel.org,
	netfilter-devel@...r.kernel.org
Subject: Re: [RFC 6/9] snet: introduce snet_hooks.c and snet_hook.h

hello Evgeniy

Evgeniy Polyakov <zbr@...emap.net> writes:

> Hi.
>
> On Sat, Jan 02, 2010 at 02:04:13PM +0100, Samir Bellabes (sam@...ack.fr) wrote:
>> This patch adds the snet LSM's subsystem
>> 
>> snet_hooks provides the security hook's functions and the security_operations
>> structure. Currently hook functions are only related to network stack.
>> 
>> For each hook function, there is a generic mecanism:
>>  0. check if the event [syscall, protocol] is registered
>>  1. prepare informations for userspace
>>  2. send informations to userspace (snet_netlink)
>>  3. wait for verdict from userspace (snet_verdict)
>>  4. apply verdict for the syscall
>> 
>> steps 3 and 4 are only valid for LSM hooks which are returning a value (a way to
>> 'filter' the syscall). For hooks returning 'void', steps 3 and 4 don't exist,
>> but snet sends security informations to userspace (step 2) to update the global
>> security policy.
>> 
>> Signed-off-by: Samir Bellabes <sam@...ack.fr>
>> ---
>
>> +#define SNET_DBG_V4()							\
>> +	snet_dbg("%u.%u.%u.%u:%u->%u.%u.%u.%u:%u\n",			\
>> +		 NIPQUAD(info.src.u3.ip), info.src.u.port,		\
>> +		 NIPQUAD(info.dst.u3.ip), info.dst.u.port)
>> +
>> +#define SNET_DBG_V6()							\
>> +	snet_dbg("%pI6:%u->%pI6:%u\n",					\
>> +		 &info.src.u3.ip, info.src.u.port,			\
>> +		 &info.dst.u3.ip, info.dst.u.port)
>> +
>
> There are cool %pi4 and %pi6 format modifiers for IP addresses.

I moved %u.%u.%u.%u to %pI4
thanks

>> +#define SNET_CHECK_LISTENERS()						\
>> +do {									\
>> +	if (atomic_read(&snet_num_listeners) < 0) {			\
>> +		snet_dbg("number of listeners is negative\n");		\
>> +		verdict = SNET_VERDICT_GRANT;				\
>> +		goto out;						\
>> +	} else if (atomic_read(&snet_num_listeners) == 0) {		\
>> +		verdict = SNET_VERDICT_GRANT;				\
>> +		goto out;						\
>> +	}								\
>> +} while (0)
>> +
>> +#define SNET_DO_VERDICT(sys, family)					\
>> +do {									\
>> +	if (verdict_id == 0)						\
>> +		goto skip_send_wait;					\
>> +	/* sending networking informations to userspace */		\
>> +	snet_nl_send_event(verdict_id, sys, protocol,			\
>> +			   family, (void *)&info,			\
>> +			   sizeof(struct snet_sock_info));		\
>> +	/* waiting for userspace reply or timeout */			\
>> +	verdict = snet_verdict_wait(verdict_id);			\
>> +		/* removing verdict */					\
>> +	snet_verdict_remove(verdict_id);				\
>> +} while (0)
>> +
>> +#define SNET_CHECK_LISTENERS_NOVERDICT()				\
>> +do {									\
>> +	if (atomic_read(&snet_num_listeners) < 0) {			\
>> +		snet_dbg("number of listeners is negative\n");		\
>> +		goto out;						\
>> +	} else if (atomic_read(&snet_num_listeners) == 0) {		\
>> +		goto out;						\
>> +	}								\
>> +} while (0)
>> +
>> +#define SNET_DO_SEND_EVENT(sys, family)					\
>> +do {									\
>> +	/* sending networking informations to userspace */		\
>> +	snet_nl_send_event(0, sys, protocol,				\
>> +			   family, (void *)&info,			\
>> +			   sizeof(struct snet_sock_info));		\
>> +} while (0)
>> +
>
> Why do you need those ugly macroses which reference external objects?
> Can it be refactored into some inline function with common error value
> instead?

I know it's ugly, but code in security hooks are duplicated.
functions (inline or not), which replace this macros, will resulte of
having lots of parameters. macros with external object seems to be the
most simple at this point.

thanks for your time Evgeniy.
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