[PATCH 1/1] python/sepolicy: fix compatibility with setools 4.2.0

[Nicolas Iooss]

When testing sepolicy gui with setools 4.2.0-beta, the following error
happened:

  File "python/sepolicy/sepolicy/__init__.py", line 277, in 
_setools_rule_to_dict
if isinstance(rule, setools.policyrep.terule.AVRule):
AttributeError: module 'setools.policyrep' has no attribute 'terule'

This is due to a reorganization of files in setools 4.2. After reporting
the issue on https://github.com/SELinuxProject/setools/issues/8 , it
appears that sepolicy has not been using setools API properly. Fix this
by:
* replacing exception types internal to setools with AttributeError, as
  they all inherit from it ;
* using rule.conditional.evaluate(...) in order to find out whether a
  conditional rule is enabled, instead of relying on
  rule.qpol_symbol.is_enabled() (which disappeared).

This last point required knowing the states of the booleans in the
policy. As sepolicy already retrieves all boolean states in
get_all_bools(), put them in a dict which can be used by
rule.conditional.evaluate().

This code has been tested with setools 4.1.1 and setools 4.2.0-beta.

Signed-off-by: Nicolas Iooss 
---
 python/sepolicy/sepolicy/__init__.py | 30 +++-
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/python/sepolicy/sepolicy/__init__.py 
b/python/sepolicy/sepolicy/__init__.py
index 89346aba0b15..ed6dfea9718a 100644
--- a/python/sepolicy/sepolicy/__init__.py
+++ b/python/sepolicy/sepolicy/__init__.py
@@ -112,6 +112,7 @@ login_mappings = None
 file_types = None
 port_types = None
 bools = None
+bools_dict = None
 all_attributes = None
 booleans = None
 booleans_dict = None
@@ -134,6 +135,7 @@ def policy(policy_file):
 global all_domains
 global all_attributes
 global bools
+global bools_dict
 global all_types
 global role_allows
 global users
@@ -143,6 +145,7 @@ def policy(policy_file):
 all_domains = None
 all_attributes = None
 bools = None
+bools_dict = None
 all_types = None
 role_allows = None
 users = None
@@ -272,34 +275,35 @@ def _setools_rule_to_dict(rule):
 'class': str(rule.tclass),
 }
 
+# Evaluate the boolean condition if it is a conditional rule.
+# In order to do this, extract the booleans which are used in the 
condition first.
 try:
-enabled = bool(rule.qpol_symbol.is_enabled(rule.policy))
+all_bools = get_all_bools_as_dict()
+used_bools = dict((str(name), all_bools[name]) for name in 
rule.conditional.booleans)
+enabled = rule.conditional.evaluate(**used_bools) == 
rule.conditional_block
 except AttributeError:
 enabled = True
 
-if isinstance(rule, setools.policyrep.terule.AVRule):
-d['enabled'] = enabled
+d['enabled'] = enabled
 
 try:
 d['permlist'] = list(map(str, rule.perms))
-except setools.policyrep.exception.RuleUseError:
+except AttributeError:
 pass
 
 try:
 d['transtype'] = str(rule.default)
-except setools.policyrep.exception.RuleUseError:
+except AttributeError:
 pass
 
 try:
 d['boolean'] = [(str(rule.conditional), enabled)]
-except (AttributeError, setools.policyrep.exception.RuleNotConditional):
+except AttributeError:
 pass
 
 try:
 d['filename'] = rule.filename
-except (AttributeError,
-setools.policyrep.exception.RuleNotConditional,
-setools.policyrep.exception.TERuleNoFilename):
+except AttributeError:
 pass
 
 return d
@@ -930,6 +934,14 @@ def get_all_bools():
 return bools
 
 
+def get_all_bools_as_dict():
+"""Return a name->state dict of the booleans defined in the policy"""
+global bools_dict
+if not bools_dict:
+bools_dict = dict((b['name'], b['state']) for b in get_all_bools())
+return bools_dict
+
+
 def prettyprint(f, trim):
 return " ".join(f[:-len(trim)].split("_"))
 
-- 
2.19.0

___
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: Bug report

[Nicolas Iooss]

On Mon, Sep 10, 2018 at 2:46 PM 李 武刚  wrote:
> Hi, ALL
>
> There is one bug which has not checking the result value of  hashtab_search 
> in the function define_level of policydb_define.c. If the category is not 
> defined, a null-pointer dereference will be taken place.
>
> The patch is attached.
>
Hi,
Thanks for your bugfix. It looks good to me and as no one complained,
I have merged it.

By the way, if you want to submit more patches, please send them
inline and not as attachment (with "git send-email" for example). This
makes the review easier.

Nicolas


___
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: remove extraneous policy build noise

[Stephen Smalley]

On 09/19/2018 03:41 PM, William Roberts wrote:



On Wed, Sep 19, 2018 at 12:36 PM Stephen Smalley > wrote:


On 09/19/2018 03:21 PM, William Roberts wrote:
 > Some people might be checking this output since it's been there
so long,
 > -s would be a good way to go.
 >
 > Alternatively, a way to bring back this information via a verbose
option
 > -V could
 > be considered.
 >
 > Either way, a simple logging mechanism analogous to
 > LOGV/LOGW/LOGE could be useful, I wonder what subordinate routines
 > are logging. IIRC it was all fprintf(stderr) stuff in libselinux
proper
 > as you allude
 > to in the redirection of stdout comment. We don't need to address
this
 > in this
 > patch, but we may want to consider it at some point.
 >
 > I would lean towards a silent flag as it's backwards compatible,
 > but noting that it doesn't suppress subordinate callers.
 >
 > I would also yield that opinion, as removing it works for me.

I'm ok dropping the output unless someone knows of an existing user
that
relies upon it (which I can't really envision).


Why don't we extend the review period to 72 hours, and ill apply this
Friday unless we hear of this breaking someone. Essentially
consider this a soft-ack.


With regard to subordinate routines, libsepol has sepol_debug(0) or
sepol_msg_set_callback() to suppress or redirect its logging.

checkpolicy doesn't use libselinux but it likewise has
selinux_set_callback().


What about things like:
libselinux/src/load_policy.c:299:fprintf(stderr, "libselinux:  %s\n", 
errormsg);


Yes, there are a few lingering cases that ought to be converted over to 
selinux_log().




Also utils and others are using fprintf directly perhaps something 
we wish to make common

across utilities and subordinate libs.


No, it is completely appropriate for the utilities to do it directly. 
Only the library should be using the callbacks.





 >
 > On Wed, Sep 19, 2018 at 12:13 PM Nick Kralevich via Selinux
 > mailto:selinux@tycho.nsa.gov>
>> wrote:
 >
 >     Reduce noise when calling the checkpolicy command line. In
Android, this
 >     creates unnecessary build noise which we'd like to avoid.
 >
 > https://en.wikipedia.org/wiki/Unix_philosophy
 >
 >        Rule of Silence
 >        Developers should design programs so that they do not print
 >        unnecessary output. This rule aims to allow other programs
 >        and developers to pick out the information they need from a
 >        program's output without having to parse verbosity.
 >
 >     An alternative approach would be to add a -s (silent) option
to these
 >     tools, or to have the Android build system redirect stdout to
/dev/null.
 >
 >     Signed-off-by: Nick Kralevich mailto:n...@google.com> >>
 >     ---
 >       checkpolicy/checkmodule.c |  8 
 >       checkpolicy/checkpolicy.c | 11 ---
 >       2 files changed, 19 deletions(-)
 >
 >     diff --git a/checkpolicy/checkmodule.c
b/checkpolicy/checkmodule.c
 >     index 46ce258f..8edc1f8c 100644
 >     --- a/checkpolicy/checkmodule.c
 >     +++ b/checkpolicy/checkmodule.c
 >     @@ -228,7 +228,6 @@ int main(int argc, char **argv)
 >                      if (optind != argc)
 >                              usage(argv[0]);
 >              }
 >     -       printf("%s:  loading policy configuration from %s\n",
 >     argv[0], file);
 >
 >              /* Set policydb and sidtab used by libsepol service
functions
 >                 to my structures, so that I can directly populate and
 >     @@ -302,8 +301,6 @@ int main(int argc, char **argv)
 >
 >              sepol_sidtab_destroy(&sidtab);
 >
 >     -       printf("%s:  policy configuration loaded\n", argv[0]);
 >     -
 >              if (outfile) {
 >                      FILE *outfp = fopen(outfile, "w");
 >
 >     @@ -313,16 +310,11 @@ int main(int argc, char **argv)
 >                      }
 >
 >                      if (!cil) {
 >     -                       printf("%s:  writing binary
representation
 >     (version %d) to %s\n",
 >     -                                  argv[0], policyvers, outfile);
 >     -
 >                              if (write_binary_policy(&modpolicydb,
 >     outfp) != 0) {
 >                                      fprintf(stderr, "%s:  error
writing
 >     %s\n", argv[0], outfile);
 >                                      exit(1);
 >                              }
 >                      } else {
 >     -                       printf("%s:  writing CIL t

Re: [PATCH] checkpolicy: remove extraneous policy build noise

[William Roberts]

On Wed, Sep 19, 2018 at 12:36 PM Stephen Smalley  wrote:

> On 09/19/2018 03:21 PM, William Roberts wrote:
> > Some people might be checking this output since it's been there so long,
> > -s would be a good way to go.
> >
> > Alternatively, a way to bring back this information via a verbose option
> > -V could
> > be considered.
> >
> > Either way, a simple logging mechanism analogous to
> > LOGV/LOGW/LOGE could be useful, I wonder what subordinate routines
> > are logging. IIRC it was all fprintf(stderr) stuff in libselinux proper
> > as you allude
> > to in the redirection of stdout comment. We don't need to address this
> > in this
> > patch, but we may want to consider it at some point.
> >
> > I would lean towards a silent flag as it's backwards compatible,
> > but noting that it doesn't suppress subordinate callers.
> >
> > I would also yield that opinion, as removing it works for me.
>
> I'm ok dropping the output unless someone knows of an existing user that
> relies upon it (which I can't really envision).
>

Why don't we extend the review period to 72 hours, and ill apply this
Friday unless we hear of this breaking someone. Essentially
consider this a soft-ack.


>
> With regard to subordinate routines, libsepol has sepol_debug(0) or
> sepol_msg_set_callback() to suppress or redirect its logging.



> checkpolicy doesn't use libselinux but it likewise has
> selinux_set_callback().
>

What about things like:
libselinux/src/load_policy.c:299: fprintf(stderr, "libselinux:  %s\n",
errormsg);

Also utils and others are using fprintf directly perhaps something we
wish to make common
across utilities and subordinate libs.


> >
> > On Wed, Sep 19, 2018 at 12:13 PM Nick Kralevich via Selinux
> > mailto:selinux@tycho.nsa.gov>> wrote:
> >
> > Reduce noise when calling the checkpolicy command line. In Android,
> this
> > creates unnecessary build noise which we'd like to avoid.
> >
> > https://en.wikipedia.org/wiki/Unix_philosophy
> >
> >Rule of Silence
> >Developers should design programs so that they do not print
> >unnecessary output. This rule aims to allow other programs
> >and developers to pick out the information they need from a
> >program's output without having to parse verbosity.
> >
> > An alternative approach would be to add a -s (silent) option to these
> > tools, or to have the Android build system redirect stdout to
> /dev/null.
> >
> > Signed-off-by: Nick Kralevich mailto:n...@google.com
> >>
> > ---
> >   checkpolicy/checkmodule.c |  8 
> >   checkpolicy/checkpolicy.c | 11 ---
> >   2 files changed, 19 deletions(-)
> >
> > diff --git a/checkpolicy/checkmodule.c b/checkpolicy/checkmodule.c
> > index 46ce258f..8edc1f8c 100644
> > --- a/checkpolicy/checkmodule.c
> > +++ b/checkpolicy/checkmodule.c
> > @@ -228,7 +228,6 @@ int main(int argc, char **argv)
> >  if (optind != argc)
> >  usage(argv[0]);
> >  }
> > -   printf("%s:  loading policy configuration from %s\n",
> > argv[0], file);
> >
> >  /* Set policydb and sidtab used by libsepol service
> functions
> > to my structures, so that I can directly populate and
> > @@ -302,8 +301,6 @@ int main(int argc, char **argv)
> >
> >  sepol_sidtab_destroy(&sidtab);
> >
> > -   printf("%s:  policy configuration loaded\n", argv[0]);
> > -
> >  if (outfile) {
> >  FILE *outfp = fopen(outfile, "w");
> >
> > @@ -313,16 +310,11 @@ int main(int argc, char **argv)
> >  }
> >
> >  if (!cil) {
> > -   printf("%s:  writing binary representation
> > (version %d) to %s\n",
> > -  argv[0], policyvers, outfile);
> > -
> >  if (write_binary_policy(&modpolicydb,
> > outfp) != 0) {
> >  fprintf(stderr, "%s:  error writing
> > %s\n", argv[0], outfile);
> >  exit(1);
> >  }
> >  } else {
> > -   printf("%s:  writing CIL to %s\n",argv[0],
> > outfile);
> > -
> >  if (sepol_module_policydb_to_cil(outfp,
> > &modpolicydb, 0) != 0) {
> >  fprintf(stderr, "%s:  error writing
> > %s\n", argv[0], outfile);
> >  exit(1);
> > diff --git a/checkpolicy/checkpolicy.c b/checkpolicy/checkpolicy.c
> > index fbda4558..12c4c405 100644
> > --- a/checkpolicy/checkpolicy.c
> > +++ b/checkpolicy/checkpolicy.c
> > @@ -512,8 +512,6 @@ int main(int argc, char **argv)
> >  if (optind != argc)
> >  usage(argv[0]);
> >  }
> > -   prin

Re: [PATCH] checkpolicy: remove extraneous policy build noise

[Stephen Smalley]

On 09/19/2018 03:21 PM, William Roberts wrote:

Some people might be checking this output since it's been there so long,
-s would be a good way to go.

Alternatively, a way to bring back this information via a verbose option 
-V could

be considered.

Either way, a simple logging mechanism analogous to
LOGV/LOGW/LOGE could be useful, I wonder what subordinate routines
are logging. IIRC it was all fprintf(stderr) stuff in libselinux proper 
as you allude
to in the redirection of stdout comment. We don't need to address this 
in this

patch, but we may want to consider it at some point.

I would lean towards a silent flag as it's backwards compatible,
but noting that it doesn't suppress subordinate callers.

I would also yield that opinion, as removing it works for me.


I'm ok dropping the output unless someone knows of an existing user that 
relies upon it (which I can't really envision).


With regard to subordinate routines, libsepol has sepol_debug(0) or 
sepol_msg_set_callback() to suppress or redirect its logging.


checkpolicy doesn't use libselinux but it likewise has 
selinux_set_callback().




On Wed, Sep 19, 2018 at 12:13 PM Nick Kralevich via Selinux 
mailto:selinux@tycho.nsa.gov>> wrote:


Reduce noise when calling the checkpolicy command line. In Android, this
creates unnecessary build noise which we'd like to avoid.

https://en.wikipedia.org/wiki/Unix_philosophy

   Rule of Silence
   Developers should design programs so that they do not print
   unnecessary output. This rule aims to allow other programs
   and developers to pick out the information they need from a
   program's output without having to parse verbosity.

An alternative approach would be to add a -s (silent) option to these
tools, or to have the Android build system redirect stdout to /dev/null.

Signed-off-by: Nick Kralevich mailto:n...@google.com>>
---
  checkpolicy/checkmodule.c |  8 
  checkpolicy/checkpolicy.c | 11 ---
  2 files changed, 19 deletions(-)

diff --git a/checkpolicy/checkmodule.c b/checkpolicy/checkmodule.c
index 46ce258f..8edc1f8c 100644
--- a/checkpolicy/checkmodule.c
+++ b/checkpolicy/checkmodule.c
@@ -228,7 +228,6 @@ int main(int argc, char **argv)
                 if (optind != argc)
                         usage(argv[0]);
         }
-       printf("%s:  loading policy configuration from %s\n",
argv[0], file);

         /* Set policydb and sidtab used by libsepol service functions
            to my structures, so that I can directly populate and
@@ -302,8 +301,6 @@ int main(int argc, char **argv)

         sepol_sidtab_destroy(&sidtab);

-       printf("%s:  policy configuration loaded\n", argv[0]);
-
         if (outfile) {
                 FILE *outfp = fopen(outfile, "w");

@@ -313,16 +310,11 @@ int main(int argc, char **argv)
                 }

                 if (!cil) {
-                       printf("%s:  writing binary representation
(version %d) to %s\n",
-                                  argv[0], policyvers, outfile);
-
                         if (write_binary_policy(&modpolicydb,
outfp) != 0) {
                                 fprintf(stderr, "%s:  error writing
%s\n", argv[0], outfile);
                                 exit(1);
                         }
                 } else {
-                       printf("%s:  writing CIL to %s\n",argv[0],
outfile);
-
                         if (sepol_module_policydb_to_cil(outfp,
&modpolicydb, 0) != 0) {
                                 fprintf(stderr, "%s:  error writing
%s\n", argv[0], outfile);
                                 exit(1);
diff --git a/checkpolicy/checkpolicy.c b/checkpolicy/checkpolicy.c
index fbda4558..12c4c405 100644
--- a/checkpolicy/checkpolicy.c
+++ b/checkpolicy/checkpolicy.c
@@ -512,8 +512,6 @@ int main(int argc, char **argv)
                 if (optind != argc)
                         usage(argv[0]);
         }
-       printf("%s:  loading policy configuration from %s\n",
argv[0], file);
-
         /* Set policydb and sidtab used by libsepol service functions
            to my structures, so that I can directly populate and
            manipulate them. */
@@ -623,8 +621,6 @@ int main(int argc, char **argv)
         if (policydb_load_isids(&policydb, &sidtab))
                 exit(1);

-       printf("%s:  policy configuration loaded\n", argv[0]);
-
         if (outfile) {
                 outfp = fopen(outfile, "w");
                 if (!outfp) {
@@ -636,8 +632,6 @@ int main(int argc, char **argv)

                 if (!cil) {
                         if (!conf) {
-                               printf("%s:  writing binary
representation (version %d) to %s\n", argv[0], policyvers

Re: [PATCH] checkpolicy: remove extraneous policy build noise

[William Roberts]

Some people might be checking this output since it's been there so long,
-s would be a good way to go.

Alternatively, a way to bring back this information via a verbose option -V
could
be considered.

Either way, a simple logging mechanism analogous to
LOGV/LOGW/LOGE could be useful, I wonder what subordinate routines
are logging. IIRC it was all fprintf(stderr) stuff in libselinux proper as
you allude
to in the redirection of stdout comment. We don't need to address this in
this
patch, but we may want to consider it at some point.

I would lean towards a silent flag as it's backwards compatible,
but noting that it doesn't suppress subordinate callers.

I would also yield that opinion, as removing it works for me.

On Wed, Sep 19, 2018 at 12:13 PM Nick Kralevich via Selinux <
selinux@tycho.nsa.gov> wrote:

> Reduce noise when calling the checkpolicy command line. In Android, this
> creates unnecessary build noise which we'd like to avoid.
>
> https://en.wikipedia.org/wiki/Unix_philosophy
>
>   Rule of Silence
>   Developers should design programs so that they do not print
>   unnecessary output. This rule aims to allow other programs
>   and developers to pick out the information they need from a
>   program's output without having to parse verbosity.
>
> An alternative approach would be to add a -s (silent) option to these
> tools, or to have the Android build system redirect stdout to /dev/null.
>
> Signed-off-by: Nick Kralevich 
> ---
>  checkpolicy/checkmodule.c |  8 
>  checkpolicy/checkpolicy.c | 11 ---
>  2 files changed, 19 deletions(-)
>
> diff --git a/checkpolicy/checkmodule.c b/checkpolicy/checkmodule.c
> index 46ce258f..8edc1f8c 100644
> --- a/checkpolicy/checkmodule.c
> +++ b/checkpolicy/checkmodule.c
> @@ -228,7 +228,6 @@ int main(int argc, char **argv)
> if (optind != argc)
> usage(argv[0]);
> }
> -   printf("%s:  loading policy configuration from %s\n", argv[0],
> file);
>
> /* Set policydb and sidtab used by libsepol service functions
>to my structures, so that I can directly populate and
> @@ -302,8 +301,6 @@ int main(int argc, char **argv)
>
> sepol_sidtab_destroy(&sidtab);
>
> -   printf("%s:  policy configuration loaded\n", argv[0]);
> -
> if (outfile) {
> FILE *outfp = fopen(outfile, "w");
>
> @@ -313,16 +310,11 @@ int main(int argc, char **argv)
> }
>
> if (!cil) {
> -   printf("%s:  writing binary representation
> (version %d) to %s\n",
> -  argv[0], policyvers, outfile);
> -
> if (write_binary_policy(&modpolicydb, outfp) != 0)
> {
> fprintf(stderr, "%s:  error writing %s\n",
> argv[0], outfile);
> exit(1);
> }
> } else {
> -   printf("%s:  writing CIL to %s\n",argv[0],
> outfile);
> -
> if (sepol_module_policydb_to_cil(outfp,
> &modpolicydb, 0) != 0) {
> fprintf(stderr, "%s:  error writing %s\n",
> argv[0], outfile);
> exit(1);
> diff --git a/checkpolicy/checkpolicy.c b/checkpolicy/checkpolicy.c
> index fbda4558..12c4c405 100644
> --- a/checkpolicy/checkpolicy.c
> +++ b/checkpolicy/checkpolicy.c
> @@ -512,8 +512,6 @@ int main(int argc, char **argv)
> if (optind != argc)
> usage(argv[0]);
> }
> -   printf("%s:  loading policy configuration from %s\n", argv[0],
> file);
> -
> /* Set policydb and sidtab used by libsepol service functions
>to my structures, so that I can directly populate and
>manipulate them. */
> @@ -623,8 +621,6 @@ int main(int argc, char **argv)
> if (policydb_load_isids(&policydb, &sidtab))
> exit(1);
>
> -   printf("%s:  policy configuration loaded\n", argv[0]);
> -
> if (outfile) {
> outfp = fopen(outfile, "w");
> if (!outfp) {
> @@ -636,8 +632,6 @@ int main(int argc, char **argv)
>
> if (!cil) {
> if (!conf) {
> -   printf("%s:  writing binary representation
> (version %d) to %s\n", argv[0], policyvers, outfile);
> -
> policydb.policy_type = POLICY_KERN;
>
> policy_file_init(&pf);
> @@ -645,8 +639,6 @@ int main(int argc, char **argv)
> pf.fp = outfp;
> ret = policydb_write(&policydb, &pf);
> } else {
> -   printf("%s:  writing policy.conf to %s\n",
> -  argv[0], outfile);
> ret = sepol_kernel_policydb_to_conf(outfp,
> policydbp);
> }
>   

[PATCH] checkpolicy: remove extraneous policy build noise

[Nick Kralevich via Selinux]

Reduce noise when calling the checkpolicy command line. In Android, this
creates unnecessary build noise which we'd like to avoid.

https://en.wikipedia.org/wiki/Unix_philosophy

  Rule of Silence
  Developers should design programs so that they do not print
  unnecessary output. This rule aims to allow other programs
  and developers to pick out the information they need from a
  program's output without having to parse verbosity.

An alternative approach would be to add a -s (silent) option to these
tools, or to have the Android build system redirect stdout to /dev/null.

Signed-off-by: Nick Kralevich 
---
 checkpolicy/checkmodule.c |  8 
 checkpolicy/checkpolicy.c | 11 ---
 2 files changed, 19 deletions(-)

diff --git a/checkpolicy/checkmodule.c b/checkpolicy/checkmodule.c
index 46ce258f..8edc1f8c 100644
--- a/checkpolicy/checkmodule.c
+++ b/checkpolicy/checkmodule.c
@@ -228,7 +228,6 @@ int main(int argc, char **argv)
if (optind != argc)
usage(argv[0]);
}
-   printf("%s:  loading policy configuration from %s\n", argv[0], file);
 
/* Set policydb and sidtab used by libsepol service functions
   to my structures, so that I can directly populate and
@@ -302,8 +301,6 @@ int main(int argc, char **argv)
 
sepol_sidtab_destroy(&sidtab);
 
-   printf("%s:  policy configuration loaded\n", argv[0]);
-
if (outfile) {
FILE *outfp = fopen(outfile, "w");
 
@@ -313,16 +310,11 @@ int main(int argc, char **argv)
}
 
if (!cil) {
-   printf("%s:  writing binary representation (version %d) 
to %s\n",
-  argv[0], policyvers, outfile);
-
if (write_binary_policy(&modpolicydb, outfp) != 0) {
fprintf(stderr, "%s:  error writing %s\n", 
argv[0], outfile);
exit(1);
}
} else {
-   printf("%s:  writing CIL to %s\n",argv[0], outfile);
-
if (sepol_module_policydb_to_cil(outfp, &modpolicydb, 
0) != 0) {
fprintf(stderr, "%s:  error writing %s\n", 
argv[0], outfile);
exit(1);
diff --git a/checkpolicy/checkpolicy.c b/checkpolicy/checkpolicy.c
index fbda4558..12c4c405 100644
--- a/checkpolicy/checkpolicy.c
+++ b/checkpolicy/checkpolicy.c
@@ -512,8 +512,6 @@ int main(int argc, char **argv)
if (optind != argc)
usage(argv[0]);
}
-   printf("%s:  loading policy configuration from %s\n", argv[0], file);
-
/* Set policydb and sidtab used by libsepol service functions
   to my structures, so that I can directly populate and
   manipulate them. */
@@ -623,8 +621,6 @@ int main(int argc, char **argv)
if (policydb_load_isids(&policydb, &sidtab))
exit(1);
 
-   printf("%s:  policy configuration loaded\n", argv[0]);
-
if (outfile) {
outfp = fopen(outfile, "w");
if (!outfp) {
@@ -636,8 +632,6 @@ int main(int argc, char **argv)
 
if (!cil) {
if (!conf) {
-   printf("%s:  writing binary representation 
(version %d) to %s\n", argv[0], policyvers, outfile);
-
policydb.policy_type = POLICY_KERN;
 
policy_file_init(&pf);
@@ -645,8 +639,6 @@ int main(int argc, char **argv)
pf.fp = outfp;
ret = policydb_write(&policydb, &pf);
} else {
-   printf("%s:  writing policy.conf to %s\n",
-  argv[0], outfile);
ret = sepol_kernel_policydb_to_conf(outfp, 
policydbp);
}
if (ret) {
@@ -655,7 +647,6 @@ int main(int argc, char **argv)
exit(1);
}
} else {
-   printf("%s:  writing CIL to %s\n",argv[0], outfile);
if (binary) {
ret = sepol_kernel_policydb_to_cil(outfp, 
policydbp);
} else {
@@ -894,8 +885,6 @@ int main(int argc, char **argv)
FGETS(ans, sizeof(ans), stdin);
pathlen = strlen(ans);
ans[pathlen - 1] = 0;
-   printf("%s:  loading policy configuration from %s\n",
-  argv[0], ans);
fd = open(ans, O_RDONLY);
if (fd < 0) {
fprintf(stderr, "Can't open '%s':  %s\n",
-- 
2.19.0.397.gdd90340f6a-goog

___
Selinux mailing list
Selinux@tyc

Re: [RFC PATCH] selinux: add a fallback to defcontext for native labeling

[Stephen Smalley]

On 09/19/2018 12:52 PM, Taras Kondratiuk wrote:

When files on NFSv4 server are not properly labeled (label doesn't match
a policy on a client) they will end up with unlabeled_t type which is
too generic. We would like to be able to set a default context per
mount. 'defcontext' mount option looks like a nice solution, but it
doesn't seem to be fully implemented for native labeling. Default
context is stored, but is never used.

The patch adds a fallback to a default context if a received context is
invalid. If the inode context is already initialized, then it is left
untouched to preserve a context set locally on a client.


Can you explain the use case further?  Why are you exporting a 
filesystem with security labeling enabled to a client that doesn't 
understand all of the labels used within it?  Why wouldn't you just 
disable NFSv4 security labeling and/or use a regular context= mount to 
assign a single context to all files in the mount?


To be clear, defcontext= doesn't work that way for local/FS_USE_XATTR 
filesystems. The context specified by it is only used for:

1) files that don't implement the xattr inode operations at all,
2) files that lack a security.selinux xattr,
3) the MLS portion of the context if it was missing (strictly as a 
legacy compatibility mechanism for RHEL4 which predated the enabling of 
the MLS field/logic).


A file with a security.selinux xattr that is invalid under policy for 
any reason other than a missing MLS field will be handled as having the 
unlabeled context.


So this would be a divergence in semantics for defcontext= between 
local/FS_USE_XATTR and NFS/FS_USE_NATIVE filesystems.




Signed-off-by: Taras Kondratiuk 
---
  security/selinux/hooks.c | 25 -
  1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ad9a9b8e9979..f7debe798bf5 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6598,7 +6598,30 @@ static void selinux_inode_invalidate_secctx(struct inode 
*inode)
   */
  static int selinux_inode_notifysecctx(struct inode *inode, void *ctx, u32 
ctxlen)
  {
-   return selinux_inode_setsecurity(inode, XATTR_SELINUX_SUFFIX, ctx, 
ctxlen, 0);
+   struct superblock_security_struct *sbsec;
+   struct inode_security_struct *isec;
+   int rc;
+
+   rc = selinux_inode_setsecurity(inode, XATTR_SELINUX_SUFFIX, ctx, 
ctxlen, 0);


In this case, we likely don't gain much by reusing 
selinux_inode_setsecurity() here and could just inline the relevant 
portion of it if we were to make this change.  Logically they mean 
different things.



+
+   /*
+* In case of Native labeling with defcontext mount option fall back
+* to a default SID if received context is invalid.
+*/
+   if (rc == -EINVAL) {
+   sbsec = inode->i_sb->s_security;
+   if (sbsec->behavior == SECURITY_FS_USE_NATIVE &&
+   sbsec->flags & DEFCONTEXT_MNT) {
+   isec = inode->i_security;
+   if (!isec->initialized) {
+   isec->sclass = 
inode_mode_to_security_class(inode->i_mode);
+   isec->sid = sbsec->def_sid;
+   isec->initialized = 1;
+   }
+   rc = 0;
+   }
+   }
+   return rc;
  }
  
  /*




___
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: [RFC PATCH] selinux: add a fallback to defcontext for native labeling

[Paul Moore]

On Wed, Sep 19, 2018 at 12:52 PM Taras Kondratiuk  wrote:
> When files on NFSv4 server are not properly labeled (label doesn't match
> a policy on a client) they will end up with unlabeled_t type which is
> too generic. We would like to be able to set a default context per
> mount. 'defcontext' mount option looks like a nice solution, but it
> doesn't seem to be fully implemented for native labeling. Default
> context is stored, but is never used.
>
> The patch adds a fallback to a default context if a received context is
> invalid. If the inode context is already initialized, then it is left
> untouched to preserve a context set locally on a client.
>
> Signed-off-by: Taras Kondratiuk 
> ---
>  security/selinux/hooks.c | 25 -
>  1 file changed, 24 insertions(+), 1 deletion(-)

The idea seems reasonable to me; if we want to treat labeled NFS like
we treat local filesystems we should also support the defcontext mount
option.

However, I wonder if we are better off generalizing some of the
SECURITY_FS_USE_XATTR based code in inode_doinit_with_dentry() such
that it can be used both in selinux_inode_notifysecctx() and in
inode_doinit_with_dentry().  Or maybe we just need a sbsec->def_sid
variant of selinux_inode_setsecurity().  Regardless, the key is the
call to security_context_to_sid_default(), the
selinux_inode_setsecurity() function only calls
security_context_to_sid().

Just in case anyone is wondering, I'm not sure I want to update
selinux_inode_setsecurity() to call security_context_to_sid_default();
at least not without more investigation.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index ad9a9b8e9979..f7debe798bf5 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6598,7 +6598,30 @@ static void selinux_inode_invalidate_secctx(struct 
> inode *inode)
>   */
>  static int selinux_inode_notifysecctx(struct inode *inode, void *ctx, u32 
> ctxlen)
>  {
> -   return selinux_inode_setsecurity(inode, XATTR_SELINUX_SUFFIX, ctx, 
> ctxlen, 0);
> +   struct superblock_security_struct *sbsec;
> +   struct inode_security_struct *isec;
> +   int rc;
> +
> +   rc = selinux_inode_setsecurity(inode, XATTR_SELINUX_SUFFIX, ctx, 
> ctxlen, 0);
> +
> +   /*
> +* In case of Native labeling with defcontext mount option fall back
> +* to a default SID if received context is invalid.
> +*/
> +   if (rc == -EINVAL) {
> +   sbsec = inode->i_sb->s_security;
> +   if (sbsec->behavior == SECURITY_FS_USE_NATIVE &&
> +   sbsec->flags & DEFCONTEXT_MNT) {
> +   isec = inode->i_security;
> +   if (!isec->initialized) {
> +   isec->sclass = 
> inode_mode_to_security_class(inode->i_mode);
> +   isec->sid = sbsec->def_sid;
> +   isec->initialized = 1;
> +   }
> +   rc = 0;
> +   }
> +   }
> +   return rc;
>  }
>
>  /*
> --
> 2.10.3.dirty
>


-- 
paul moore
www.paul-moore.com
___
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.

[RFC PATCH] selinux: add a fallback to defcontext for native labeling

[Taras Kondratiuk via Selinux]

When files on NFSv4 server are not properly labeled (label doesn't match
a policy on a client) they will end up with unlabeled_t type which is
too generic. We would like to be able to set a default context per
mount. 'defcontext' mount option looks like a nice solution, but it
doesn't seem to be fully implemented for native labeling. Default
context is stored, but is never used.

The patch adds a fallback to a default context if a received context is
invalid. If the inode context is already initialized, then it is left
untouched to preserve a context set locally on a client.

Signed-off-by: Taras Kondratiuk 
---
 security/selinux/hooks.c | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ad9a9b8e9979..f7debe798bf5 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6598,7 +6598,30 @@ static void selinux_inode_invalidate_secctx(struct inode 
*inode)
  */
 static int selinux_inode_notifysecctx(struct inode *inode, void *ctx, u32 
ctxlen)
 {
-   return selinux_inode_setsecurity(inode, XATTR_SELINUX_SUFFIX, ctx, 
ctxlen, 0);
+   struct superblock_security_struct *sbsec;
+   struct inode_security_struct *isec;
+   int rc;
+
+   rc = selinux_inode_setsecurity(inode, XATTR_SELINUX_SUFFIX, ctx, 
ctxlen, 0);
+
+   /*
+* In case of Native labeling with defcontext mount option fall back
+* to a default SID if received context is invalid.
+*/
+   if (rc == -EINVAL) {
+   sbsec = inode->i_sb->s_security;
+   if (sbsec->behavior == SECURITY_FS_USE_NATIVE &&
+   sbsec->flags & DEFCONTEXT_MNT) {
+   isec = inode->i_security;
+   if (!isec->initialized) {
+   isec->sclass = 
inode_mode_to_security_class(inode->i_mode);
+   isec->sid = sbsec->def_sid;
+   isec->initialized = 1;
+   }
+   rc = 0;
+   }
+   }
+   return rc;
 }
 
 /*
-- 
2.10.3.dirty

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

FYI: email change

[Paul Moore]

A quick note that my @redhat.com email address is going to stop
working in the next day or two, so if you are using my Red Hat email
address to reach me please start using my @paul-moore.com address.

Everything else, e.g. my community involvement, will remain unaffected.

-- 
paul moore
www.paul-moore.com
___
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.