Filtering an avtab in libsepol

Filtering an avtab in libsepol

[Gary Tierney]

[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]

Hi,

I've been working on optimizing out AV rules with no applicable types as 
well as unused attributes to trim down the size of a policy which uses 
CIL blocks and attributes extensively.  Looking into the avtab code (and 
how creating a new avtab is implemented in expand.c) I have a question:

Does the following suffice for taking an existing avtab and creating a 
new one with all of its elements?  Or do I need to consider 
avtab_insert_nonunique() like expand.c does?  If I'm following the 
expand_avtab() code correctly, I'd think I'd need to consider conditional 
avtabs in the following code:

static int copy_avtab_map_fn(avtab_key_t *key, avtab_datum_t *datum,
			     void *args)
{
	avtab_t *avtab = (avtab_t *) args;

	return avtab_insert(avtab, key, datum);
}

static int copy_avtab(avtab_t *avtab, avtab_t **out)
{
	avtab_t *tmp = NULL;
	if (avtab_init(tmp)) {
		return POLICYDB_ERROR;
	}

	if (avtab_alloc(tmp, MAX_AVTAB_SIZE)) {
		return POLICYDB_ERROR;
	}

	if (avtab_map(avtab, copy_avtab_map_fn, tmp)) {
		return POLICYDB_ERROR;
	}

	*out = tmp;
	return POLICYDB_SUCCESS;
}

Is that the right idea?

Thanks.

-- 
Gary Tierney
 
GPG fingerprint: 412C 0EF9 C305 68E6 B660BDAF 706E D765 85AA 79D8
https://sks-keyservers.net/pks/lookup?op=get&search=0x706ED76585AA79D8 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: Filtering an avtab in libsepol

[Stephen Smalley]

On 12/06/2016 12:00 PM, Gary Tierney wrote:
> Hi,
> 
> I've been working on optimizing out AV rules with no applicable
> types as well as unused attributes to trim down the size of a
> policy which uses CIL blocks and attributes extensively.  Looking
> into the avtab code (and how creating a new avtab is implemented in
> expand.c) I have a question:
> 
> Does the following suffice for taking an existing avtab and
> creating a new one with all of its elements?  Or do I need to
> consider avtab_insert_nonunique() like expand.c does?  If I'm
> following the expand_avtab() code correctly, I'd think I'd need to
> consider conditional avtabs in the following code:
> 
> static int copy_avtab_map_fn(avtab_key_t *key, avtab_datum_t
> *datum, void *args) { avtab_t *avtab = (avtab_t *) args;
> 
> return avtab_insert(avtab, key, datum); }
> 
> static int copy_avtab(avtab_t *avtab, avtab_t **out) { avtab_t *tmp
> = NULL; if (avtab_init(tmp)) { return POLICYDB_ERROR; }
> 
> if (avtab_alloc(tmp, MAX_AVTAB_SIZE)) { return POLICYDB_ERROR; }
> 
> if (avtab_map(avtab, copy_avtab_map_fn, tmp)) { return
> POLICYDB_ERROR; }
> 
> *out = tmp; return POLICYDB_SUCCESS; }
> 
> Is that the right idea?
> 
> Thanks.

Did you consider doing this at the CIL layer instead, given that CIL
already does similar optimizations and has more semantic information
available?  Note that CIL used to be more aggressive about removing
unused attributes but backed off because some attributes are used in
neverallows and we want to preserve those for neverallow checking in CTS.

Conditional rules can indeed have non-unique entries, and so can
xperms rules.

Re: Filtering an avtab in libsepol

[Gary Tierney]

[-- Attachment #1: Type: text/plain, Size: 2535 bytes --]

On Tue, Dec 06, 2016 at 12:53:00PM -0500, Stephen Smalley wrote:
> On 12/06/2016 12:00 PM, Gary Tierney wrote:
> > Hi,
> > 
> > I've been working on optimizing out AV rules with no applicable
> > types as well as unused attributes to trim down the size of a
> > policy which uses CIL blocks and attributes extensively.  Looking
> > into the avtab code (and how creating a new avtab is implemented in
> > expand.c) I have a question:
> > 
> > Does the following suffice for taking an existing avtab and
> > creating a new one with all of its elements?  Or do I need to
> > consider avtab_insert_nonunique() like expand.c does?  If I'm
> > following the expand_avtab() code correctly, I'd think I'd need to
> > consider conditional avtabs in the following code:
> > 
> > static int copy_avtab_map_fn(avtab_key_t *key, avtab_datum_t
> > *datum, void *args) { avtab_t *avtab = (avtab_t *) args;
> > 
> > return avtab_insert(avtab, key, datum); }
> > 
> > static int copy_avtab(avtab_t *avtab, avtab_t **out) { avtab_t *tmp
> > = NULL; if (avtab_init(tmp)) { return POLICYDB_ERROR; }
> > 
> > if (avtab_alloc(tmp, MAX_AVTAB_SIZE)) { return POLICYDB_ERROR; }
> > 
> > if (avtab_map(avtab, copy_avtab_map_fn, tmp)) { return
> > POLICYDB_ERROR; }
> > 
> > *out = tmp; return POLICYDB_SUCCESS; }
> > 
> > Is that the right idea?
> > 
> > Thanks.
> 
> Did you consider doing this at the CIL layer instead, given that CIL
> already does similar optimizations and has more semantic information
> available?  Note that CIL used to be more aggressive about removing
> unused attributes but backed off because some attributes are used in
> neverallows and we want to preserve those for neverallow checking in CTS.
> 

Yes, I think I'll go down that route eventually once I move on to removing
typeattributes with no good AV rules.  This was mostly an exercise in getting
my feet wet with the binary policy and figuring out what makes up the
majority of its size.  I noticed that the CIL compiler currently does
optimize away typeattributes that aren't used in any AV rules so the
foundation seems to be there (and Jim also shared some input on this
previously: http://marc.info/?l=selinux&m=147871772206496&w=2).

> Conditional rules can indeed have non-unique entries, and so can
> xperms rules.
> 
> 

Thanks, that makes sense.

-- 
Gary Tierney

GPG fingerprint: 412C 0EF9 C305 68E6 B660  BDAF 706E D765 85AA 79D8
https://sks-keyservers.net/pks/lookup?op=get&search=0x706ED76585AA79D8

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: Filtering an avtab in libsepol

[Stephen Smalley]

On 12/06/2016 12:53 PM, Stephen Smalley wrote:
> On 12/06/2016 12:00 PM, Gary Tierney wrote:
>> Hi,
>>
>> I've been working on optimizing out AV rules with no applicable
>> types as well as unused attributes to trim down the size of a
>> policy which uses CIL blocks and attributes extensively.  Looking
>> into the avtab code (and how creating a new avtab is implemented in
>> expand.c) I have a question:
>>
>> Does the following suffice for taking an existing avtab and
>> creating a new one with all of its elements?  Or do I need to
>> consider avtab_insert_nonunique() like expand.c does?  If I'm
>> following the expand_avtab() code correctly, I'd think I'd need to
>> consider conditional avtabs in the following code:
>>
>> static int copy_avtab_map_fn(avtab_key_t *key, avtab_datum_t
>> *datum, void *args) { avtab_t *avtab = (avtab_t *) args;
>>
>> return avtab_insert(avtab, key, datum); }
>>
>> static int copy_avtab(avtab_t *avtab, avtab_t **out) { avtab_t *tmp
>> = NULL; if (avtab_init(tmp)) { return POLICYDB_ERROR; }
>>
>> if (avtab_alloc(tmp, MAX_AVTAB_SIZE)) { return POLICYDB_ERROR; }
>>
>> if (avtab_map(avtab, copy_avtab_map_fn, tmp)) { return
>> POLICYDB_ERROR; }
>>
>> *out = tmp; return POLICYDB_SUCCESS; }
>>
>> Is that the right idea?
>>
>> Thanks.
> 
> Did you consider doing this at the CIL layer instead, given that CIL
> already does similar optimizations and has more semantic information
> available?  Note that CIL used to be more aggressive about removing
> unused attributes but backed off because some attributes are used in
> neverallows and we want to preserve those for neverallow checking in CTS.
> 
> Conditional rules can indeed have non-unique entries, and so can
> xperms rules.

The other thing to remember about the conditional rules is that the
te_cond_avtab is only used for lookups; the "real" list of conditional
rules is what is in cond_list, and it is cond_list that is written out
to the kernel policy file.  So filtering the contents of te_cond_avtab
won't alter what is written to the kernel policy.

Re: Filtering an avtab in libsepol

[Gary Tierney]

[-- Attachment #1: Type: text/plain, Size: 2707 bytes --]

On Tue, Dec 06, 2016 at 01:06:28PM -0500, Stephen Smalley wrote:
>On 12/06/2016 12:53 PM, Stephen Smalley wrote:
>> On 12/06/2016 12:00 PM, Gary Tierney wrote:
>>> Hi,
>>>
>>> I've been working on optimizing out AV rules with no applicable
>>> types as well as unused attributes to trim down the size of a
>>> policy which uses CIL blocks and attributes extensively.  Looking
>>> into the avtab code (and how creating a new avtab is implemented in
>>> expand.c) I have a question:
>>>
>>> Does the following suffice for taking an existing avtab and
>>> creating a new one with all of its elements?  Or do I need to
>>> consider avtab_insert_nonunique() like expand.c does?  If I'm
>>> following the expand_avtab() code correctly, I'd think I'd need to
>>> consider conditional avtabs in the following code:
>>>
>>> static int copy_avtab_map_fn(avtab_key_t *key, avtab_datum_t
>>> *datum, void *args) { avtab_t *avtab = (avtab_t *) args;
>>>
>>> return avtab_insert(avtab, key, datum); }
>>>
>>> static int copy_avtab(avtab_t *avtab, avtab_t **out) { avtab_t *tmp
>>> = NULL; if (avtab_init(tmp)) { return POLICYDB_ERROR; }
>>>
>>> if (avtab_alloc(tmp, MAX_AVTAB_SIZE)) { return POLICYDB_ERROR; }
>>>
>>> if (avtab_map(avtab, copy_avtab_map_fn, tmp)) { return
>>> POLICYDB_ERROR; }
>>>
>>> *out = tmp; return POLICYDB_SUCCESS; }
>>>
>>> Is that the right idea?
>>>
>>> Thanks.
>>
>> Did you consider doing this at the CIL layer instead, given that CIL
>> already does similar optimizations and has more semantic information
>> available?  Note that CIL used to be more aggressive about removing
>> unused attributes but backed off because some attributes are used in
>> neverallows and we want to preserve those for neverallow checking in CTS.
>>
>> Conditional rules can indeed have non-unique entries, and so can
>> xperms rules.
>
>The other thing to remember about the conditional rules is that the
>te_cond_avtab is only used for lookups; the "real" list of conditional
>rules is what is in cond_list, and it is cond_list that is written out
>to the kernel policy file.  So filtering the contents of te_cond_avtab
>won't alter what is written to the kernel policy.
>

I'd skimmed over the write_cond_av_list() code, but wasn't aware that's 
what was going on.  Thanks for the clarification.  I suppose with that 
in mind then it is best to just go ahead and make these changes in 
libsepol/cil where we're dealing with high-level constructs than in the 
kernel policy writing code.

-- 
Gary Tierney
 
GPG fingerprint: 412C 0EF9 C305 68E6 B660BDAF 706E D765 85AA 79D8
https://sks-keyservers.net/pks/lookup?op=get&search=0x706ED76585AA79D8 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]