Re: [PATCH] checkpolicy: Fix bug in handling type declaration in optional block.
On 01/21/2017 08:58 AM, Nicolas Iooss wrote:
On Wed, Jan 18, 2017 at 9:53 PM, James Carter mailto:jwca...@tycho.nsa.gov>> wrote:
Nicolas Iooss discovered that requiring a type in an optional block
after the type has already been declared in another optional block
results in a duplicate declaration error.
The following policy will trigger the error.
optional {
type T1;
}
optional {
require { type T1; }
}
In this case, although symtab_insert() in libsepol properly identifies
that the first T1 is a declaration while the second is a require, it
will return -2 which is the return value for a duplicate declaration
and expect the calling code to properly handle the situation. The
caller is require_symbol() in checkpolicy and it checks if the previous
declaration is in the current scope. If it is, then the require can be
ignored. It also checks to see if the declaration is a type and the
require an attribute or vice versa which is an error. The problem is
that -2 is returned if the declaration is not in scope which is
interpreted as a duplicate declaration error.
The function should return 1 instead which means that they symbol was not
added and needs to be freed later.
Hello,
I tested your patch with the following policy module written in a file named
testmodule.te:
module testmodule 1.0.0;
require { class process { fork }; }
optional {
require { attribute ATTR; }
type TYPE1, ATTR;
}
optional {
require { type TYPE1; }
allow TYPE1 self:process fork;
}
checkmodule failed to compile this module:
testmodule.te:10:ERROR 'This block has no require section.' at token '}' on
line 10:
}
allow TYPE1 self:process fork;
I was looking at what my patch did late Friday and I thought that this might
happen.
Hence I modified the require statement of the second optional block to "require
{ type TYPE1, TYPE2; }" and checkmodule reported:
testmodule.te:9:ERROR 'type TYPE1 is not within scope' at token ';' on line 9:
require { type TYPE1, TYPE2; }
allow TYPE1 self:process fork;
It seems there is a scope issue with TYPE1 when it is used in a block where it
is required. Is this a bug?
This is not the desired behavior. I am looking at refactoring this code.
Thanks for the report.
Jim
Thanks,
Nicolas
PS: while debugging this issue I found some other memory leaks in checkpolicy. I
will send some patches later.
--
James Carter
National Security Agency
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] checkpolicy: Fix bug in handling type declaration in optional block.
On 01/19/2017 06:21 PM, Stephen Smalley wrote:
> On Wed, 2017-01-18 at 21:58 +0100, Dominick Grift wrote:
>> On 01/18/2017 09:53 PM, James Carter wrote:
>>>
>>> Nicolas Iooss discovered that requiring a type in an optional block
>>> after the type has already been declared in another optional block
>>> results in a duplicate declaration error.
>>>
>>
>> from what i have been told and from experience, types cannot,
>> reliably,
>> be declared in optional blocks.
>
> If true, that is likely a linker issue rather than a checkpolicy issue.
> But that would make optionals rather useless if true.
>
I beg to differ. optionals would not be useless if they would not allow
declarations IMHO.
In my view it makes little sense to declare anything in an optional.
Optionals are a modularity feature.
So lets look at that. For example the apache_content_template() and how
it is currently used.
For example the git module calls the apache_content_template() in an
optional. So the httpd_git_content_t type is declared if the apache
module is available, and not if it is not available. Same goes for a
bunch of other modules.
But that does not make things all that "modular". Because I cannot have
the git module and the apache module without the httpd_git_content_t.
What would make more sense if the apache_content_template(git) would be
put in a separate module (example apache_git). Then i could have the git
module , the apache module and not the "apache_git" module. That is
modularity IMHO.
>>
>> if the above is true, then the compiler should not allow one to
>> declare
>> a type in an optional block in the first place
>>
>>>
>>> The following policy will trigger the error.
>>>
>>> optional {
>>> type T1;
>>> }
>>>
>>> optional {
>>> require { type T1; }
>>> }
>>>
>>> In this case, although symtab_insert() in libsepol properly
>>> identifies
>>> that the first T1 is a declaration while the second is a require,
>>> it
>>> will return -2 which is the return value for a duplicate
>>> declaration
>>> and expect the calling code to properly handle the situation. The
>>> caller is require_symbol() in checkpolicy and it checks if the
>>> previous
>>> declaration is in the current scope. If it is, then the require can
>>> be
>>> ignored. It also checks to see if the declaration is a type and the
>>> require an attribute or vice versa which is an error. The problem
>>> is
>>> that -2 is returned if the declaration is not in scope which is
>>> interpreted as a duplicate declaration error.
>>>
>>> The function should return 1 instead which means that they symbol
>>> was not
>>> added and needs to be freed later.
>>>
>>> Signed-off-by: James Carter
>>> ---
>>> checkpolicy/module_compiler.c | 16 +++-
>>> 1 file changed, 3 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/checkpolicy/module_compiler.c
>>> b/checkpolicy/module_compiler.c
>>> index 6e5483c..bb60f34 100644
>>> --- a/checkpolicy/module_compiler.c
>>> +++ b/checkpolicy/module_compiler.c
>>> @@ -647,7 +647,6 @@ int require_symbol(uint32_t symbol_type,
>>> } else if (retval == -2) {
>>> /* ignore require statements if that symbol was
>>> * previously declared and is in current scope */
>>> - int prev_declaration_ok = 0;
>>> if (is_id_in_scope(symbol_type, key)) {
>>> if (symbol_type == SYM_TYPES) {
>>> /* check that previous symbol has
>>> same
>>> @@ -661,23 +660,14 @@ int require_symbol(uint32_t symbol_type,
>>>
>>> table, key);
>>> assert(old_datum != NULL);
>>> unsigned char old_isattr =
>>> old_datum->flavor;
>>> - prev_declaration_ok =
>>> - (old_isattr == new_isattr ? 1
>>> : 0);
>>> - } else {
>>> - prev_declaration_ok = 1;
>>> + if (old_isattr != new_isattr)
>>> + return -2;
>>> }
>>> - }
>>> - if (prev_declaration_ok) {
>>> /* ignore this require statement because
>>> it
>>> * was already declared within my scope */
>>> stack_top->require_given = 1;
>>> - return 1;
>>> - } else {
>>> - /* previous declaration was not in scope
>>> or
>>> -* had a mismatched type/attribute, so
>>> -* generate an error */
>>> - return -2;
>>> }
>>> + return 1;
>>> } else if (retval < 0) {
>>> return -3;
>>> } else {/* fall through possible if retval
>>> is 0 or 1 */
>>>
>>
>>
>> ___
>> Selinux mailing list
>> Selinux@tycho.nsa.gov
>> To unsubscribe, send email to selinux-le...@tycho.nsa.g
Re: [PATCH] checkpolicy: Fix bug in handling type declaration in optional block.
On Wed, Jan 18, 2017 at 9:53 PM, James Carter wrote:
> Nicolas Iooss discovered that requiring a type in an optional block
> after the type has already been declared in another optional block
> results in a duplicate declaration error.
>
> The following policy will trigger the error.
>
> optional {
> type T1;
> }
>
> optional {
> require { type T1; }
> }
>
> In this case, although symtab_insert() in libsepol properly identifies
> that the first T1 is a declaration while the second is a require, it
> will return -2 which is the return value for a duplicate declaration
> and expect the calling code to properly handle the situation. The
> caller is require_symbol() in checkpolicy and it checks if the previous
> declaration is in the current scope. If it is, then the require can be
> ignored. It also checks to see if the declaration is a type and the
> require an attribute or vice versa which is an error. The problem is
> that -2 is returned if the declaration is not in scope which is
> interpreted as a duplicate declaration error.
>
> The function should return 1 instead which means that they symbol was not
> added and needs to be freed later.
Hello,
I tested your patch with the following policy module written in a file
named testmodule.te:
module testmodule 1.0.0;
require { class process { fork }; }
optional {
require { attribute ATTR; }
type TYPE1, ATTR;
}
optional {
require { type TYPE1; }
allow TYPE1 self:process fork;
}
checkmodule failed to compile this module:
testmodule.te:10:ERROR 'This block has no require section.' at token '}'
on line 10:
}
allow TYPE1 self:process fork;
Hence I modified the require statement of the second optional block to
"require { type TYPE1, TYPE2; }" and checkmodule reported:
testmodule.te:9:ERROR 'type TYPE1 is not within scope' at token ';' on
line 9:
require { type TYPE1, TYPE2; }
allow TYPE1 self:process fork;
It seems there is a scope issue with TYPE1 when it is used in a block where
it is required. Is this a bug?
Thanks,
Nicolas
PS: while debugging this issue I found some other memory leaks in
checkpolicy. I will send some patches later.
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] checkpolicy: Fix bug in handling type declaration in optional block.
On 01/20/2017 06:14 PM, Steve Lawrence wrote: > On 01/20/2017 10:29 AM, Dominick Grift wrote: >> On 01/20/2017 04:16 PM, James Carter wrote: >>> On 01/19/2017 04:22 PM, Dominick Grift wrote: On 01/19/2017 06:21 PM, Stephen Smalley wrote: > On Wed, 2017-01-18 at 21:58 +0100, Dominick Grift wrote: >> On 01/18/2017 09:53 PM, James Carter wrote: >>> >>> Nicolas Iooss discovered that requiring a type in an optional block >>> after the type has already been declared in another optional block >>> results in a duplicate declaration error. >>> >> >> from what i have been told and from experience, types cannot, >> reliably, >> be declared in optional blocks. > > If true, that is likely a linker issue rather than a checkpolicy issue. > But that would make optionals rather useless if true. Let me present it in a different way then: Should one be able to, reliably, "blockinherit" in optionals? This is the commit in dssp1 where i hit the dead-end with declaring types in optionals: https://github.com/DefenSec/dssp-contrib/commit/45236da4030f0a541c26753e13a91f69411d022e I asked slawrence to have a look at the policy, and he indicated to me that blocks cannot be inherited in optionals. The compiler accepts it but the policy becomes inconsistent >>> >>> By inconsistent, do you mean that an the binary policy produced is not >>> valid, or something else? >> >> I used the word inconsistent because i actually forgot the details (it >> has been more than a year since I made this change and I tried hard to >> put this behind me, thinking that this was a fundamental limitation >> rather than a "bug"). >> >> I suspect the following: >> >> Rules end up in the policy but they aren't actually recognized/enforced. >> >> Either the above or: >> >> Rules do *not* end up in the policy even though they are specified. >> >> Maybe slawrence remembers what the issue exactly was (added to recipients) >> > > After digging through some past emails and discussions, I think part of > the issue is that things just start to get tricky and difficult to deal > with if you allow blockinherits (and for similar reasons blocks/macros) > in optionals. > > For example, say you had something like this: > > (optional foo > (block b > (blockinherit c) >) >... > ) > > (block d > (blockinherit b)) > > > If something in optional foo fails, then the entire block b goes away. > So does that mean the blockinherit b in block d goes away too? Or does > that stay? Removing it could be potentially tricky since we need to > remove tree nodes that have been copied (we don't currently where things > are copied to), and that removal might have side effects. This is a > similar reason why we don't allow macros in optionals. Once they've been > called, it becomes difficult to remove them if the macros go away. Not > impossible, but difficult. > > And since blockinherits can pull in blocks, you're essentially allowing > blocks inside optionals if you allow blockinherits inside optionals. So > this is likely the reason for not allowing blockinherits in optionals. > They can pull in blocks and macros, which aren't allowed to be optional. > > I agree that supporting optional blocks/macros/blockinherits/etc would > probably be really nice, but things get difficult pretty fast. > Re-resolve when optionals fail is a messy bit of code. Figuring out the > right thing to do and when isn't obvious or easy. > > Now, we could add in some restrictions that make the changes less > difficult (maybe), like blocks cannot be inside optionals, or > blockinherits inside optionals cannot inherit blocks that contain other > blocks. But again, that's still kindof a pain to implement and those are > probably restrictions that you'd end up running into eventually anyways. > So it would be best to just really figure out how to make it all work. > I'm sure it can work, it's just not an easy problem to solve and takes > time to figure out. > > - Steve > Thank you. I guess i somehow thought that the two issues were one and the same (ie. "corruption" when declaring types in optionals vs. "corruption" when inheriting blocks in optionals) Seems I was wrong there. The "type issue" is probably not CIL related and has been there for a very long time. The two issues have things in common though. Both declaring types in optionals, as well as inheriting blocks in optionals are unreliable and should be avoided. -- Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02 Dominick Grift signature.asc Description: OpenPGP digital signature ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.go
Re: [PATCH] checkpolicy: Fix bug in handling type declaration in optional block.
On 01/20/2017 10:29 AM, Dominick Grift wrote: > On 01/20/2017 04:16 PM, James Carter wrote: >> On 01/19/2017 04:22 PM, Dominick Grift wrote: >>> On 01/19/2017 06:21 PM, Stephen Smalley wrote: On Wed, 2017-01-18 at 21:58 +0100, Dominick Grift wrote: > On 01/18/2017 09:53 PM, James Carter wrote: >> >> Nicolas Iooss discovered that requiring a type in an optional block >> after the type has already been declared in another optional block >> results in a duplicate declaration error. >> > > from what i have been told and from experience, types cannot, > reliably, > be declared in optional blocks. If true, that is likely a linker issue rather than a checkpolicy issue. But that would make optionals rather useless if true. >>> >>> Let me present it in a different way then: >>> >>> Should one be able to, reliably, "blockinherit" in optionals? >>> >>> This is the commit in dssp1 where i hit the dead-end with declaring >>> types in optionals: >>> >>> https://github.com/DefenSec/dssp-contrib/commit/45236da4030f0a541c26753e13a91f69411d022e >>> >>> >>> I asked slawrence to have a look at the policy, and he indicated to me >>> that blocks cannot be inherited in optionals. >>> >>> The compiler accepts it but the policy becomes inconsistent >>> >> >> By inconsistent, do you mean that an the binary policy produced is not >> valid, or something else? > > I used the word inconsistent because i actually forgot the details (it > has been more than a year since I made this change and I tried hard to > put this behind me, thinking that this was a fundamental limitation > rather than a "bug"). > > I suspect the following: > > Rules end up in the policy but they aren't actually recognized/enforced. > > Either the above or: > > Rules do *not* end up in the policy even though they are specified. > > Maybe slawrence remembers what the issue exactly was (added to recipients) > After digging through some past emails and discussions, I think part of the issue is that things just start to get tricky and difficult to deal with if you allow blockinherits (and for similar reasons blocks/macros) in optionals. For example, say you had something like this: (optional foo (block b (blockinherit c) ) ... ) (block d (blockinherit b)) If something in optional foo fails, then the entire block b goes away. So does that mean the blockinherit b in block d goes away too? Or does that stay? Removing it could be potentially tricky since we need to remove tree nodes that have been copied (we don't currently where things are copied to), and that removal might have side effects. This is a similar reason why we don't allow macros in optionals. Once they've been called, it becomes difficult to remove them if the macros go away. Not impossible, but difficult. And since blockinherits can pull in blocks, you're essentially allowing blocks inside optionals if you allow blockinherits inside optionals. So this is likely the reason for not allowing blockinherits in optionals. They can pull in blocks and macros, which aren't allowed to be optional. I agree that supporting optional blocks/macros/blockinherits/etc would probably be really nice, but things get difficult pretty fast. Re-resolve when optionals fail is a messy bit of code. Figuring out the right thing to do and when isn't obvious or easy. Now, we could add in some restrictions that make the changes less difficult (maybe), like blocks cannot be inside optionals, or blockinherits inside optionals cannot inherit blocks that contain other blocks. But again, that's still kindof a pain to implement and those are probably restrictions that you'd end up running into eventually anyways. So it would be best to just really figure out how to make it all work. I'm sure it can work, it's just not an easy problem to solve and takes time to figure out. - Steve ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] checkpolicy: Fix bug in handling type declaration in optional block.
On 01/20/2017 04:29 PM, Dominick Grift wrote:
> On 01/20/2017 04:16 PM, James Carter wrote:
>> On 01/19/2017 04:22 PM, Dominick Grift wrote:
>>> On 01/19/2017 06:21 PM, Stephen Smalley wrote:
On Wed, 2017-01-18 at 21:58 +0100, Dominick Grift wrote:
> On 01/18/2017 09:53 PM, James Carter wrote:
>>
>> Nicolas Iooss discovered that requiring a type in an optional block
>> after the type has already been declared in another optional block
>> results in a duplicate declaration error.
>>
>
> from what i have been told and from experience, types cannot,
> reliably,
> be declared in optional blocks.
If true, that is likely a linker issue rather than a checkpolicy issue.
But that would make optionals rather useless if true.
>>>
>>> Let me present it in a different way then:
>>>
>>> Should one be able to, reliably, "blockinherit" in optionals?
>>>
>>> This is the commit in dssp1 where i hit the dead-end with declaring
>>> types in optionals:
>>>
>>> https://github.com/DefenSec/dssp-contrib/commit/45236da4030f0a541c26753e13a91f69411d022e
>>>
>>>
>>> I asked slawrence to have a look at the policy, and he indicated to me
>>> that blocks cannot be inherited in optionals.
>>>
>>> The compiler accepts it but the policy becomes inconsistent
>>>
>>
>> By inconsistent, do you mean that an the binary policy produced is not
>> valid, or something else?
>
> I used the word inconsistent because i actually forgot the details (it
> has been more than a year since I made this change and I tried hard to
> put this behind me, thinking that this was a fundamental limitation
> rather than a "bug").
>
> I suspect the following:
>
> Rules end up in the policy but they aren't actually recognized/enforced.
>
> Either the above or:
>
> Rules do *not* end up in the policy even though they are specified.
>
> Maybe slawrence remembers what the issue exactly was (added to recipients)
>
One thing is for sure, by moving my blockinherits out of the optional
blocks my problems were solved.
secilc did not disallow me to put the blockinherits in optionals, it did
not complain, it just did not give the expected results, and things were
fixed by moving the blocks out of the optionals
>>
>> Macro definitions are not allowed in optionals, so if you have a macro
>> definition in a block, that should cause an error. Other than that, I
>> can't think of what would cause problems.
>
> That is mentioned in the secilc documentation. So why does secilc not
> disallow inheriting blocks in optionals?
>
> I might be wrong but I believe that i was not actually defining macros
> in my blocks back then, and still the behavior was "inconsistent".
>
>
>>
>> Jim
>>
>
> if the above is true, then the compiler should not allow one to
> declare
> a type in an optional block in the first place
>
>>
>> The following policy will trigger the error.
>>
>> optional {
>> type T1;
>> }
>>
>> optional {
>> require { type T1; }
>> }
>>
>> In this case, although symtab_insert() in libsepol properly
>> identifies
>> that the first T1 is a declaration while the second is a require,
>> it
>> will return -2 which is the return value for a duplicate
>> declaration
>> and expect the calling code to properly handle the situation. The
>> caller is require_symbol() in checkpolicy and it checks if the
>> previous
>> declaration is in the current scope. If it is, then the require can
>> be
>> ignored. It also checks to see if the declaration is a type and the
>> require an attribute or vice versa which is an error. The problem
>> is
>> that -2 is returned if the declaration is not in scope which is
>> interpreted as a duplicate declaration error.
>>
>> The function should return 1 instead which means that they symbol
>> was not
>> added and needs to be freed later.
>>
>> Signed-off-by: James Carter
>> ---
>> checkpolicy/module_compiler.c | 16 +++-
>> 1 file changed, 3 insertions(+), 13 deletions(-)
>>
>> diff --git a/checkpolicy/module_compiler.c
>> b/checkpolicy/module_compiler.c
>> index 6e5483c..bb60f34 100644
>> --- a/checkpolicy/module_compiler.c
>> +++ b/checkpolicy/module_compiler.c
>> @@ -647,7 +647,6 @@ int require_symbol(uint32_t symbol_type,
>> } else if (retval == -2) {
>> /* ignore require statements if that symbol was
>> * previously declared and is in current scope */
>> -int prev_declaration_ok = 0;
>> if (is_id_in_scope(symbol_type, key)) {
>> if (symbol_type == SYM_TYPES) {
>> /* check that previous symbol has
>> same
>> @@ -661,23 +660,14 @@ int require_symbol(uint32_t symbol_type,
>>
>> table, key);
>> assert(old_datum !=
Re: [PATCH] checkpolicy: Fix bug in handling type declaration in optional block.
On 01/20/2017 04:16 PM, James Carter wrote:
> On 01/19/2017 04:22 PM, Dominick Grift wrote:
>> On 01/19/2017 06:21 PM, Stephen Smalley wrote:
>>> On Wed, 2017-01-18 at 21:58 +0100, Dominick Grift wrote:
On 01/18/2017 09:53 PM, James Carter wrote:
>
> Nicolas Iooss discovered that requiring a type in an optional block
> after the type has already been declared in another optional block
> results in a duplicate declaration error.
>
from what i have been told and from experience, types cannot,
reliably,
be declared in optional blocks.
>>>
>>> If true, that is likely a linker issue rather than a checkpolicy issue.
>>> But that would make optionals rather useless if true.
>>
>> Let me present it in a different way then:
>>
>> Should one be able to, reliably, "blockinherit" in optionals?
>>
>> This is the commit in dssp1 where i hit the dead-end with declaring
>> types in optionals:
>>
>> https://github.com/DefenSec/dssp-contrib/commit/45236da4030f0a541c26753e13a91f69411d022e
>>
>>
>> I asked slawrence to have a look at the policy, and he indicated to me
>> that blocks cannot be inherited in optionals.
>>
>> The compiler accepts it but the policy becomes inconsistent
>>
>
> By inconsistent, do you mean that an the binary policy produced is not
> valid, or something else?
I used the word inconsistent because i actually forgot the details (it
has been more than a year since I made this change and I tried hard to
put this behind me, thinking that this was a fundamental limitation
rather than a "bug").
I suspect the following:
Rules end up in the policy but they aren't actually recognized/enforced.
Either the above or:
Rules do *not* end up in the policy even though they are specified.
Maybe slawrence remembers what the issue exactly was (added to recipients)
>
> Macro definitions are not allowed in optionals, so if you have a macro
> definition in a block, that should cause an error. Other than that, I
> can't think of what would cause problems.
That is mentioned in the secilc documentation. So why does secilc not
disallow inheriting blocks in optionals?
I might be wrong but I believe that i was not actually defining macros
in my blocks back then, and still the behavior was "inconsistent".
>
> Jim
>
>>>
if the above is true, then the compiler should not allow one to
declare
a type in an optional block in the first place
>
> The following policy will trigger the error.
>
> optional {
> type T1;
> }
>
> optional {
> require { type T1; }
> }
>
> In this case, although symtab_insert() in libsepol properly
> identifies
> that the first T1 is a declaration while the second is a require,
> it
> will return -2 which is the return value for a duplicate
> declaration
> and expect the calling code to properly handle the situation. The
> caller is require_symbol() in checkpolicy and it checks if the
> previous
> declaration is in the current scope. If it is, then the require can
> be
> ignored. It also checks to see if the declaration is a type and the
> require an attribute or vice versa which is an error. The problem
> is
> that -2 is returned if the declaration is not in scope which is
> interpreted as a duplicate declaration error.
>
> The function should return 1 instead which means that they symbol
> was not
> added and needs to be freed later.
>
> Signed-off-by: James Carter
> ---
> checkpolicy/module_compiler.c | 16 +++-
> 1 file changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/checkpolicy/module_compiler.c
> b/checkpolicy/module_compiler.c
> index 6e5483c..bb60f34 100644
> --- a/checkpolicy/module_compiler.c
> +++ b/checkpolicy/module_compiler.c
> @@ -647,7 +647,6 @@ int require_symbol(uint32_t symbol_type,
> } else if (retval == -2) {
> /* ignore require statements if that symbol was
> * previously declared and is in current scope */
> -int prev_declaration_ok = 0;
> if (is_id_in_scope(symbol_type, key)) {
> if (symbol_type == SYM_TYPES) {
> /* check that previous symbol has
> same
> @@ -661,23 +660,14 @@ int require_symbol(uint32_t symbol_type,
>
> table, key);
> assert(old_datum != NULL);
> unsigned char old_isattr =
> old_datum->flavor;
> -prev_declaration_ok =
> -(old_isattr == new_isattr ? 1
> : 0);
> -} else {
> -prev_declaration_ok = 1;
> +if (old_isattr != new_isattr)
> +return -2;
> }
> -}
> -if (prev_declaration_ok) {
> /* ignore this require statement
Re: [PATCH] checkpolicy: Fix bug in handling type declaration in optional block.
On 01/19/2017 04:22 PM, Dominick Grift wrote:
On 01/19/2017 06:21 PM, Stephen Smalley wrote:
On Wed, 2017-01-18 at 21:58 +0100, Dominick Grift wrote:
On 01/18/2017 09:53 PM, James Carter wrote:
Nicolas Iooss discovered that requiring a type in an optional block
after the type has already been declared in another optional block
results in a duplicate declaration error.
from what i have been told and from experience, types cannot,
reliably,
be declared in optional blocks.
If true, that is likely a linker issue rather than a checkpolicy issue.
But that would make optionals rather useless if true.
Let me present it in a different way then:
Should one be able to, reliably, "blockinherit" in optionals?
This is the commit in dssp1 where i hit the dead-end with declaring
types in optionals:
https://github.com/DefenSec/dssp-contrib/commit/45236da4030f0a541c26753e13a91f69411d022e
I asked slawrence to have a look at the policy, and he indicated to me
that blocks cannot be inherited in optionals.
The compiler accepts it but the policy becomes inconsistent
By inconsistent, do you mean that an the binary policy produced is not valid, or
something else?
Macro definitions are not allowed in optionals, so if you have a macro
definition in a block, that should cause an error. Other than that, I can't
think of what would cause problems.
Jim
if the above is true, then the compiler should not allow one to
declare
a type in an optional block in the first place
The following policy will trigger the error.
optional {
type T1;
}
optional {
require { type T1; }
}
In this case, although symtab_insert() in libsepol properly
identifies
that the first T1 is a declaration while the second is a require,
it
will return -2 which is the return value for a duplicate
declaration
and expect the calling code to properly handle the situation. The
caller is require_symbol() in checkpolicy and it checks if the
previous
declaration is in the current scope. If it is, then the require can
be
ignored. It also checks to see if the declaration is a type and the
require an attribute or vice versa which is an error. The problem
is
that -2 is returned if the declaration is not in scope which is
interpreted as a duplicate declaration error.
The function should return 1 instead which means that they symbol
was not
added and needs to be freed later.
Signed-off-by: James Carter
---
checkpolicy/module_compiler.c | 16 +++-
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/checkpolicy/module_compiler.c
b/checkpolicy/module_compiler.c
index 6e5483c..bb60f34 100644
--- a/checkpolicy/module_compiler.c
+++ b/checkpolicy/module_compiler.c
@@ -647,7 +647,6 @@ int require_symbol(uint32_t symbol_type,
} else if (retval == -2) {
/* ignore require statements if that symbol was
* previously declared and is in current scope */
- int prev_declaration_ok = 0;
if (is_id_in_scope(symbol_type, key)) {
if (symbol_type == SYM_TYPES) {
/* check that previous symbol has
same
@@ -661,23 +660,14 @@ int require_symbol(uint32_t symbol_type,
table, key);
assert(old_datum != NULL);
unsigned char old_isattr =
old_datum->flavor;
- prev_declaration_ok =
- (old_isattr == new_isattr ? 1
: 0);
- } else {
- prev_declaration_ok = 1;
+ if (old_isattr != new_isattr)
+ return -2;
}
- }
- if (prev_declaration_ok) {
/* ignore this require statement because
it
* was already declared within my scope */
stack_top->require_given = 1;
- return 1;
- } else {
- /* previous declaration was not in scope
or
-* had a mismatched type/attribute, so
-* generate an error */
- return -2;
}
+ return 1;
} else if (retval < 0) {
return -3;
} else {/* fall through possible if retval
is 0 or 1 */
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to Selinux-request@tycho
.nsa.gov.
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
--
James Carter
National Security
Re: [PATCH] checkpolicy: Fix bug in handling type declaration in optional block.
On 01/19/2017 06:21 PM, Stephen Smalley wrote:
> On Wed, 2017-01-18 at 21:58 +0100, Dominick Grift wrote:
>> On 01/18/2017 09:53 PM, James Carter wrote:
>>>
>>> Nicolas Iooss discovered that requiring a type in an optional block
>>> after the type has already been declared in another optional block
>>> results in a duplicate declaration error.
>>>
>>
>> from what i have been told and from experience, types cannot,
>> reliably,
>> be declared in optional blocks.
>
> If true, that is likely a linker issue rather than a checkpolicy issue.
> But that would make optionals rather useless if true.
Let me present it in a different way then:
Should one be able to, reliably, "blockinherit" in optionals?
This is the commit in dssp1 where i hit the dead-end with declaring
types in optionals:
https://github.com/DefenSec/dssp-contrib/commit/45236da4030f0a541c26753e13a91f69411d022e
I asked slawrence to have a look at the policy, and he indicated to me
that blocks cannot be inherited in optionals.
The compiler accepts it but the policy becomes inconsistent
>
>>
>> if the above is true, then the compiler should not allow one to
>> declare
>> a type in an optional block in the first place
>>
>>>
>>> The following policy will trigger the error.
>>>
>>> optional {
>>> type T1;
>>> }
>>>
>>> optional {
>>> require { type T1; }
>>> }
>>>
>>> In this case, although symtab_insert() in libsepol properly
>>> identifies
>>> that the first T1 is a declaration while the second is a require,
>>> it
>>> will return -2 which is the return value for a duplicate
>>> declaration
>>> and expect the calling code to properly handle the situation. The
>>> caller is require_symbol() in checkpolicy and it checks if the
>>> previous
>>> declaration is in the current scope. If it is, then the require can
>>> be
>>> ignored. It also checks to see if the declaration is a type and the
>>> require an attribute or vice versa which is an error. The problem
>>> is
>>> that -2 is returned if the declaration is not in scope which is
>>> interpreted as a duplicate declaration error.
>>>
>>> The function should return 1 instead which means that they symbol
>>> was not
>>> added and needs to be freed later.
>>>
>>> Signed-off-by: James Carter
>>> ---
>>> checkpolicy/module_compiler.c | 16 +++-
>>> 1 file changed, 3 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/checkpolicy/module_compiler.c
>>> b/checkpolicy/module_compiler.c
>>> index 6e5483c..bb60f34 100644
>>> --- a/checkpolicy/module_compiler.c
>>> +++ b/checkpolicy/module_compiler.c
>>> @@ -647,7 +647,6 @@ int require_symbol(uint32_t symbol_type,
>>> } else if (retval == -2) {
>>> /* ignore require statements if that symbol was
>>> * previously declared and is in current scope */
>>> - int prev_declaration_ok = 0;
>>> if (is_id_in_scope(symbol_type, key)) {
>>> if (symbol_type == SYM_TYPES) {
>>> /* check that previous symbol has
>>> same
>>> @@ -661,23 +660,14 @@ int require_symbol(uint32_t symbol_type,
>>>
>>> table, key);
>>> assert(old_datum != NULL);
>>> unsigned char old_isattr =
>>> old_datum->flavor;
>>> - prev_declaration_ok =
>>> - (old_isattr == new_isattr ? 1
>>> : 0);
>>> - } else {
>>> - prev_declaration_ok = 1;
>>> + if (old_isattr != new_isattr)
>>> + return -2;
>>> }
>>> - }
>>> - if (prev_declaration_ok) {
>>> /* ignore this require statement because
>>> it
>>> * was already declared within my scope */
>>> stack_top->require_given = 1;
>>> - return 1;
>>> - } else {
>>> - /* previous declaration was not in scope
>>> or
>>> -* had a mismatched type/attribute, so
>>> -* generate an error */
>>> - return -2;
>>> }
>>> + return 1;
>>> } else if (retval < 0) {
>>> return -3;
>>> } else {/* fall through possible if retval
>>> is 0 or 1 */
>>>
>>
>>
>> ___
>> Selinux mailing list
>> Selinux@tycho.nsa.gov
>> To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
>> To get help, send an email containing "help" to Selinux-request@tycho
>> .nsa.gov.
--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
signature.asc
Description: OpenPGP digital signature
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux
Re: [PATCH] checkpolicy: Fix bug in handling type declaration in optional block.
On Wed, 2017-01-18 at 21:58 +0100, Dominick Grift wrote:
> On 01/18/2017 09:53 PM, James Carter wrote:
> >
> > Nicolas Iooss discovered that requiring a type in an optional block
> > after the type has already been declared in another optional block
> > results in a duplicate declaration error.
> >
>
> from what i have been told and from experience, types cannot,
> reliably,
> be declared in optional blocks.
If true, that is likely a linker issue rather than a checkpolicy issue.
But that would make optionals rather useless if true.
>
> if the above is true, then the compiler should not allow one to
> declare
> a type in an optional block in the first place
>
> >
> > The following policy will trigger the error.
> >
> > optional {
> > type T1;
> > }
> >
> > optional {
> > require { type T1; }
> > }
> >
> > In this case, although symtab_insert() in libsepol properly
> > identifies
> > that the first T1 is a declaration while the second is a require,
> > it
> > will return -2 which is the return value for a duplicate
> > declaration
> > and expect the calling code to properly handle the situation. The
> > caller is require_symbol() in checkpolicy and it checks if the
> > previous
> > declaration is in the current scope. If it is, then the require can
> > be
> > ignored. It also checks to see if the declaration is a type and the
> > require an attribute or vice versa which is an error. The problem
> > is
> > that -2 is returned if the declaration is not in scope which is
> > interpreted as a duplicate declaration error.
> >
> > The function should return 1 instead which means that they symbol
> > was not
> > added and needs to be freed later.
> >
> > Signed-off-by: James Carter
> > ---
> > checkpolicy/module_compiler.c | 16 +++-
> > 1 file changed, 3 insertions(+), 13 deletions(-)
> >
> > diff --git a/checkpolicy/module_compiler.c
> > b/checkpolicy/module_compiler.c
> > index 6e5483c..bb60f34 100644
> > --- a/checkpolicy/module_compiler.c
> > +++ b/checkpolicy/module_compiler.c
> > @@ -647,7 +647,6 @@ int require_symbol(uint32_t symbol_type,
> > } else if (retval == -2) {
> > /* ignore require statements if that symbol was
> > * previously declared and is in current scope */
> > - int prev_declaration_ok = 0;
> > if (is_id_in_scope(symbol_type, key)) {
> > if (symbol_type == SYM_TYPES) {
> > /* check that previous symbol has
> > same
> > @@ -661,23 +660,14 @@ int require_symbol(uint32_t symbol_type,
> >
> > table, key);
> > assert(old_datum != NULL);
> > unsigned char old_isattr =
> > old_datum->flavor;
> > - prev_declaration_ok =
> > - (old_isattr == new_isattr ? 1
> > : 0);
> > - } else {
> > - prev_declaration_ok = 1;
> > + if (old_isattr != new_isattr)
> > + return -2;
> > }
> > - }
> > - if (prev_declaration_ok) {
> > /* ignore this require statement because
> > it
> > * was already declared within my scope */
> > stack_top->require_given = 1;
> > - return 1;
> > - } else {
> > - /* previous declaration was not in scope
> > or
> > - * had a mismatched type/attribute, so
> > - * generate an error */
> > - return -2;
> > }
> > + return 1;
> > } else if (retval < 0) {
> > return -3;
> > } else {/* fall through possible if retval
> > is 0 or 1 */
> >
>
>
> ___
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho
> .nsa.gov.
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] checkpolicy: Fix bug in handling type declaration in optional block.
On 01/18/2017 10:15 PM, Dominick Grift wrote:
> On 01/18/2017 10:09 PM, James Carter wrote:
>> On 01/18/2017 03:58 PM, Dominick Grift wrote:
>>> On 01/18/2017 09:53 PM, James Carter wrote:
Nicolas Iooss discovered that requiring a type in an optional block
after the type has already been declared in another optional block
results in a duplicate declaration error.
>>>
>>> from what i have been told and from experience, types cannot, reliably,
>>> be declared in optional blocks.
>>>
>>> if the above is true, then the compiler should not allow one to declare
>>> a type in an optional block in the first place
>>>
>>
>> Well, this ordering issue reported by Nicolas would definitely cause
>> their declaration to be unreliable. ;)
>>
>> I hope to make declaring types in optional blocks reliable.
>
> That would be nice, but just so you know. This was not the issue i was
> encountering with declaring types in optionals. So even though this
> might have made things unreliable. It was not what made it unreliable
> for me.
>
Actually, sorry. I do actually recognize the second scenario (duplicate
declaration). So this might in fact be related. I can't believe that
this ancient bug would finally be fixed. Ive been lead to believe that
this was just a fundamental limitation of module policy all this time.
My whole policy was structured on the assumption that types cannot
reliably be declared in optionals
>>
>> Jim
>>
The following policy will trigger the error.
optional {
type T1;
}
optional {
require { type T1; }
}
In this case, although symtab_insert() in libsepol properly identifies
that the first T1 is a declaration while the second is a require, it
will return -2 which is the return value for a duplicate declaration
and expect the calling code to properly handle the situation. The
caller is require_symbol() in checkpolicy and it checks if the previous
declaration is in the current scope. If it is, then the require can be
ignored. It also checks to see if the declaration is a type and the
require an attribute or vice versa which is an error. The problem is
that -2 is returned if the declaration is not in scope which is
interpreted as a duplicate declaration error.
The function should return 1 instead which means that they symbol was
not
added and needs to be freed later.
Signed-off-by: James Carter
---
checkpolicy/module_compiler.c | 16 +++-
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/checkpolicy/module_compiler.c
b/checkpolicy/module_compiler.c
index 6e5483c..bb60f34 100644
--- a/checkpolicy/module_compiler.c
+++ b/checkpolicy/module_compiler.c
@@ -647,7 +647,6 @@ int require_symbol(uint32_t symbol_type,
} else if (retval == -2) {
/* ignore require statements if that symbol was
* previously declared and is in current scope */
-int prev_declaration_ok = 0;
if (is_id_in_scope(symbol_type, key)) {
if (symbol_type == SYM_TYPES) {
/* check that previous symbol has same
@@ -661,23 +660,14 @@ int require_symbol(uint32_t symbol_type,
table, key);
assert(old_datum != NULL);
unsigned char old_isattr = old_datum->flavor;
-prev_declaration_ok =
-(old_isattr == new_isattr ? 1 : 0);
-} else {
-prev_declaration_ok = 1;
+if (old_isattr != new_isattr)
+return -2;
}
-}
-if (prev_declaration_ok) {
/* ignore this require statement because it
* was already declared within my scope */
stack_top->require_given = 1;
-return 1;
-} else {
-/* previous declaration was not in scope or
- * had a mismatched type/attribute, so
- * generate an error */
-return -2;
}
+return 1;
} else if (retval < 0) {
return -3;
} else {/* fall through possible if retval is 0 or 1 */
>>>
>>>
>>>
>>>
>>> ___
>>> Selinux mailing list
>>> Selinux@tycho.nsa.gov
>>> To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
>>> To get help, send an email containing "help" to
>>> selinux-requ...@tycho.nsa.gov.
>>>
>>
>>
>
>
--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
signature.asc
Description: OpenPGP digital signature
___
Selinux mailin
Re: [PATCH] checkpolicy: Fix bug in handling type declaration in optional block.
On 01/18/2017 10:09 PM, James Carter wrote:
> On 01/18/2017 03:58 PM, Dominick Grift wrote:
>> On 01/18/2017 09:53 PM, James Carter wrote:
>>> Nicolas Iooss discovered that requiring a type in an optional block
>>> after the type has already been declared in another optional block
>>> results in a duplicate declaration error.
>>>
>>
>> from what i have been told and from experience, types cannot, reliably,
>> be declared in optional blocks.
>>
>> if the above is true, then the compiler should not allow one to declare
>> a type in an optional block in the first place
>>
>
> Well, this ordering issue reported by Nicolas would definitely cause
> their declaration to be unreliable. ;)
>
> I hope to make declaring types in optional blocks reliable.
That would be nice, but just so you know. This was not the issue i was
encountering with declaring types in optionals. So even though this
might have made things unreliable. It was not what made it unreliable
for me.
>
> Jim
>
>>> The following policy will trigger the error.
>>>
>>> optional {
>>> type T1;
>>> }
>>>
>>> optional {
>>> require { type T1; }
>>> }
>>>
>>> In this case, although symtab_insert() in libsepol properly identifies
>>> that the first T1 is a declaration while the second is a require, it
>>> will return -2 which is the return value for a duplicate declaration
>>> and expect the calling code to properly handle the situation. The
>>> caller is require_symbol() in checkpolicy and it checks if the previous
>>> declaration is in the current scope. If it is, then the require can be
>>> ignored. It also checks to see if the declaration is a type and the
>>> require an attribute or vice versa which is an error. The problem is
>>> that -2 is returned if the declaration is not in scope which is
>>> interpreted as a duplicate declaration error.
>>>
>>> The function should return 1 instead which means that they symbol was
>>> not
>>> added and needs to be freed later.
>>>
>>> Signed-off-by: James Carter
>>> ---
>>> checkpolicy/module_compiler.c | 16 +++-
>>> 1 file changed, 3 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/checkpolicy/module_compiler.c
>>> b/checkpolicy/module_compiler.c
>>> index 6e5483c..bb60f34 100644
>>> --- a/checkpolicy/module_compiler.c
>>> +++ b/checkpolicy/module_compiler.c
>>> @@ -647,7 +647,6 @@ int require_symbol(uint32_t symbol_type,
>>> } else if (retval == -2) {
>>> /* ignore require statements if that symbol was
>>> * previously declared and is in current scope */
>>> -int prev_declaration_ok = 0;
>>> if (is_id_in_scope(symbol_type, key)) {
>>> if (symbol_type == SYM_TYPES) {
>>> /* check that previous symbol has same
>>> @@ -661,23 +660,14 @@ int require_symbol(uint32_t symbol_type,
>>> table, key);
>>> assert(old_datum != NULL);
>>> unsigned char old_isattr = old_datum->flavor;
>>> -prev_declaration_ok =
>>> -(old_isattr == new_isattr ? 1 : 0);
>>> -} else {
>>> -prev_declaration_ok = 1;
>>> +if (old_isattr != new_isattr)
>>> +return -2;
>>> }
>>> -}
>>> -if (prev_declaration_ok) {
>>> /* ignore this require statement because it
>>> * was already declared within my scope */
>>> stack_top->require_given = 1;
>>> -return 1;
>>> -} else {
>>> -/* previous declaration was not in scope or
>>> - * had a mismatched type/attribute, so
>>> - * generate an error */
>>> -return -2;
>>> }
>>> +return 1;
>>> } else if (retval < 0) {
>>> return -3;
>>> } else {/* fall through possible if retval is 0 or 1 */
>>>
>>
>>
>>
>>
>> ___
>> Selinux mailing list
>> Selinux@tycho.nsa.gov
>> To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
>> To get help, send an email containing "help" to
>> selinux-requ...@tycho.nsa.gov.
>>
>
>
--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
signature.asc
Description: OpenPGP digital signature
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] checkpolicy: Fix bug in handling type declaration in optional block.
On 01/18/2017 03:58 PM, Dominick Grift wrote:
On 01/18/2017 09:53 PM, James Carter wrote:
Nicolas Iooss discovered that requiring a type in an optional block
after the type has already been declared in another optional block
results in a duplicate declaration error.
from what i have been told and from experience, types cannot, reliably,
be declared in optional blocks.
if the above is true, then the compiler should not allow one to declare
a type in an optional block in the first place
Well, this ordering issue reported by Nicolas would definitely cause their
declaration to be unreliable. ;)
I hope to make declaring types in optional blocks reliable.
Jim
The following policy will trigger the error.
optional {
type T1;
}
optional {
require { type T1; }
}
In this case, although symtab_insert() in libsepol properly identifies
that the first T1 is a declaration while the second is a require, it
will return -2 which is the return value for a duplicate declaration
and expect the calling code to properly handle the situation. The
caller is require_symbol() in checkpolicy and it checks if the previous
declaration is in the current scope. If it is, then the require can be
ignored. It also checks to see if the declaration is a type and the
require an attribute or vice versa which is an error. The problem is
that -2 is returned if the declaration is not in scope which is
interpreted as a duplicate declaration error.
The function should return 1 instead which means that they symbol was not
added and needs to be freed later.
Signed-off-by: James Carter
---
checkpolicy/module_compiler.c | 16 +++-
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c
index 6e5483c..bb60f34 100644
--- a/checkpolicy/module_compiler.c
+++ b/checkpolicy/module_compiler.c
@@ -647,7 +647,6 @@ int require_symbol(uint32_t symbol_type,
} else if (retval == -2) {
/* ignore require statements if that symbol was
* previously declared and is in current scope */
- int prev_declaration_ok = 0;
if (is_id_in_scope(symbol_type, key)) {
if (symbol_type == SYM_TYPES) {
/* check that previous symbol has same
@@ -661,23 +660,14 @@ int require_symbol(uint32_t symbol_type,
table, key);
assert(old_datum != NULL);
unsigned char old_isattr = old_datum->flavor;
- prev_declaration_ok =
- (old_isattr == new_isattr ? 1 : 0);
- } else {
- prev_declaration_ok = 1;
+ if (old_isattr != new_isattr)
+ return -2;
}
- }
- if (prev_declaration_ok) {
/* ignore this require statement because it
* was already declared within my scope */
stack_top->require_given = 1;
- return 1;
- } else {
- /* previous declaration was not in scope or
-* had a mismatched type/attribute, so
-* generate an error */
- return -2;
}
+ return 1;
} else if (retval < 0) {
return -3;
} else {/* fall through possible if retval is 0 or 1 */
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
--
James Carter
National Security Agency
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] checkpolicy: Fix bug in handling type declaration in optional block.
On 01/18/2017 09:53 PM, James Carter wrote:
> Nicolas Iooss discovered that requiring a type in an optional block
> after the type has already been declared in another optional block
> results in a duplicate declaration error.
>
from what i have been told and from experience, types cannot, reliably,
be declared in optional blocks.
if the above is true, then the compiler should not allow one to declare
a type in an optional block in the first place
> The following policy will trigger the error.
>
> optional {
> type T1;
> }
>
> optional {
> require { type T1; }
> }
>
> In this case, although symtab_insert() in libsepol properly identifies
> that the first T1 is a declaration while the second is a require, it
> will return -2 which is the return value for a duplicate declaration
> and expect the calling code to properly handle the situation. The
> caller is require_symbol() in checkpolicy and it checks if the previous
> declaration is in the current scope. If it is, then the require can be
> ignored. It also checks to see if the declaration is a type and the
> require an attribute or vice versa which is an error. The problem is
> that -2 is returned if the declaration is not in scope which is
> interpreted as a duplicate declaration error.
>
> The function should return 1 instead which means that they symbol was not
> added and needs to be freed later.
>
> Signed-off-by: James Carter
> ---
> checkpolicy/module_compiler.c | 16 +++-
> 1 file changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/checkpolicy/module_compiler.c b/checkpolicy/module_compiler.c
> index 6e5483c..bb60f34 100644
> --- a/checkpolicy/module_compiler.c
> +++ b/checkpolicy/module_compiler.c
> @@ -647,7 +647,6 @@ int require_symbol(uint32_t symbol_type,
> } else if (retval == -2) {
> /* ignore require statements if that symbol was
>* previously declared and is in current scope */
> - int prev_declaration_ok = 0;
> if (is_id_in_scope(symbol_type, key)) {
> if (symbol_type == SYM_TYPES) {
> /* check that previous symbol has same
> @@ -661,23 +660,14 @@ int require_symbol(uint32_t symbol_type,
> table, key);
> assert(old_datum != NULL);
> unsigned char old_isattr = old_datum->flavor;
> - prev_declaration_ok =
> - (old_isattr == new_isattr ? 1 : 0);
> - } else {
> - prev_declaration_ok = 1;
> + if (old_isattr != new_isattr)
> + return -2;
> }
> - }
> - if (prev_declaration_ok) {
> /* ignore this require statement because it
>* was already declared within my scope */
> stack_top->require_given = 1;
> - return 1;
> - } else {
> - /* previous declaration was not in scope or
> - * had a mismatched type/attribute, so
> - * generate an error */
> - return -2;
> }
> + return 1;
> } else if (retval < 0) {
> return -3;
> } else {/* fall through possible if retval is 0 or 1 */
>
--
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02
https://sks-keyservers.net/pks/lookup?op=get&search=0x3B6C5F1D2C7B6B02
Dominick Grift
signature.asc
Description: OpenPGP digital signature
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
