linux-kernel - Re: [PATCH 2/2] security/smack implement logging V2

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: <49D966DB.9070009@schaufler-ca.com>
Date:	Sun, 05 Apr 2009 19:20:11 -0700
From:	Casey Schaufler <casey@...aufler-ca.com>
To:	Etienne Basset <etienne.basset@...ericable.fr>
CC:	LSM <linux-security-module@...r.kernel.org>,
	Eric Paris <eparis@...hat.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-audit@...hat.com, Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [PATCH 2/2] security/smack implement logging V2

Etienne Basset wrote:
>> It looks like the only difference between these are their non-logging
>> versions is the logging. I say go ahead and add the auditdata parameter
>> to smk_access and smk_curacc and allow for the case where it is NULL.
>>
>>     
> i would prefer keeping a logging and a non-logging variant
> maybe rename smk_access & smk_curacc to smk_access_nolog & smk_curacc_nolog
> Or you really prefer that we pass a NULL when we want the non-logging?
> I guess its a matter of taste, so i let you decide :)
>   

I dislike functions that do nothing but set up another function.


>> It would be nice if audit data was only manipulated if audit is
>> installed, but I don't like the idea of ifdeffing everything
>> very much either. How about a static inline in smack.h that is
>> ifdeffed for audit? smack_audit_init? There would need to be one
>> for each field, too.
>>
>>     
> why not, but instead of a "initialiser" for each field i'll prefer a macro
>  To save some stack we could also conditionnaly define the "common_audit_data"
>
> something like :
>
> #ifdef CONFIG_AUDIT
> #define DEFINE_SMACK_AUDIT_DATA(ad) struct common_audit_data ad
> void smack_audit_init(..) {
>  COMMON_AUDIT_DATA_INIT(...)
> }
> #define SMACK_AUDIT_DATA_SET_FIELD(_ad, _field, _value) (_ad._field = _value)
> ...
> #else
> #define DEFINE_SMACK_AUDIT_DATA(ad)
> void smack_audit_init(..)
> {
> } 
> #define SMACK_AUDIT_DATA_SET_FIELD(_ad, _field, _value) 
> #endif
>   

I'm not convinced. #defines are good for assigning names to constants,
but I saw the original Bourne shell code, where defines were used to
make the code look like ALGOL68, so you have to overcome the fear that
was instilled in me when I was young.

 

>> Now this is tricky. You'll get an audit record for single-label
>> hosts, but not for those that use CIPSO. The former is good, the
>> latter is bad.
>>
>>     
> gargll you're right
>   

I love those french idioms.

Actually, I don't think there's much point in worrying about it.
The audit you do here is correct, and until you can get more subject
information over the wire there isn't much point doing it on the
receiving end.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/