Re: MLS dominance check behavior on el7

2018-10-04 Thread Stephen Smalley

On 09/30/2018 10:43 AM, Chris PeBenito wrote:

On 09/11/2018 04:20 PM, Stephen Smalley wrote:

On 09/11/2018 03:04 PM, Joe Nall wrote:




On Sep 11, 2018, at 1:29 PM, Stephen Smalley  wrote:

On 09/11/2018 10:41 AM, Stephen Smalley wrote:

On 09/10/2018 06:30 PM, Ted Toth wrote:
mcstrans mcscolor.c also uses the same logic I'd been using to 
check dominance so this too will no longer function as expected on 
el7. Do you any suggestions for doing a 'generic' (one not tied to 
a specific resource class) dominance check in lieu of context 
contains?
You should probably define your own permission with its own 
constraint to avoid depending on the base policy's particular 
constraint definitions.  Certainly for your own code.  For 
mcstrans, mcscolor probably ought to be switched to using at least 
a separate permission in the context class if not its own class to 
avoid overloading the meaning with pam_selinux's usage (or vice 
versa, but likely harder to change pam_selinux at this point).
It is possible to define an entirely new class, its permissions, 
and its mls constraints via a CIL module IIUC, without needing to 
change the base policy.
I don't think you can add a permission to an existing class via a 
CIL module currently, unfortunately, so you can't just extend the 
context class without modifying the base policy.  So it may be 
easier to define an entirely new class.
The class and permission ought to be specific to the usage.  For 
example, mcstrans could have its own class (mcstrans) with its own 
permissions (e.g. color_match or color_use or ...) that abstract 
away the logical check being performed.  Dominance checks performed 
for different reasons ought to use different permissions so that 
one can distinguish what TE pairs are allowed them.

Your code could likewise define and use its own class and permission.
Does that make sense?


BTW, I noticed there is another permission ("translate") defined in 
the context class and its constraint is ((h1 dom h2) or (t1 == 
mlstranslate)).  I would have guessed that it was intended as a 
front-end service check over what processes could request context 
translations from mcstrans or what contexts they could translate, 
but I don't see it being used in mcstrans anywhere.  Is this a 
legacy thing from early setransd/mcstransd days?  There is a TODO 
comment in mcstrans process_request() that suggests there was an 
intent to perform a dominance check between the requester context 
and the specified context, but that's not implemented.  Appears to 
be allowed in current policy for all domains to the setrans_t domain 
itself.


I think 'translate' predates my mcstransd work and dates from the 
original TCS implementation. There is an argument to implement that 
constraint, but we've been operating without it for so long it does 
not seem worthwhile.


Well, I guess we ought to either implement it or delete the permission 
definition from refpolicy.


I'm fine removing it.  It's just the translate permission that is 
unused, not the whole class, correct?


Correct. Only caveat is that removing translate will change the 
permission index of contains, which could break a running mcstransd upon 
a policy reload (doesn't use selinux_check_access or even the avc; won't 
flush the class/perm string mapping on a reload automatically).



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

[PATCH] libsemanage: improve semanage_migrate_store import failure

2018-10-04 Thread Yuli Khodorkovskiy
The python module import error in semanage_migrate_store was misleading.
Before, it would print that the module is not installed, even though
it is in fact on the system.

Now the python module import failure is correctly reported if the module
is not installed or the exact reason for failure is reported to the user.

Signed-off-by: Yuli Khodorkovskiy 
---
 libsemanage/utils/semanage_migrate_store | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libsemanage/utils/semanage_migrate_store 
b/libsemanage/utils/semanage_migrate_store
index 2e6cb278..50eb59ef 100755
--- a/libsemanage/utils/semanage_migrate_store
+++ b/libsemanage/utils/semanage_migrate_store
@@ -15,10 +15,12 @@ sepol = ctypes.cdll.LoadLibrary('libsepol.so.1')
 try:
import selinux
import semanage
-except:
+except ImportError:
print("You must install libselinux-python and libsemanage-python before 
running this tool", file=sys.stderr)
exit(1)
-
+except Exception as e:
+   print("Failed to import libselinux-python/libsemanage-python: %s" % 
str(e))
+   exit(1)
 
 def copy_file(src, dst):
if DEBUG:
-- 
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: [PATCH] selinux-testsuite: update the dependencies in README.md

2018-10-04 Thread Stephen Smalley

On 10/03/2018 11:52 AM, Paul Moore wrote:

The overlayfs tests require setfattr and getfattr which are part of
the attr package in Fedora.

Signed-off-by: Paul Moore 


Acked-by: Stephen Smalley 


---
  README.md |4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 2c871d3..cf90ef6 100644
--- a/README.md
+++ b/README.md
@@ -50,6 +50,7 @@ similar dependencies):
  * netlabel\_tools _(to load NetLabel configuration during `inet_socket` 
tests)_
  * iptables _(to load the `iptables SECMARK` rules during `inet_socket` tests)_
  * lksctp-tools-devel _(to build the SCTP test programs)_
+* attr _(tools used by the overlayfs tests)_
  
  On a modern Fedora system you can install these dependencies with the

  following command:
@@ -63,7 +64,8 @@ following command:
net-tools \
netlabel_tools \
iptables \
-   lksctp-tools-devel
+   lksctp-tools-devel \
+   attr
  
  The testsuite requires a pre-existing base policy configuration of SELinux,

  using either the old example policy or the reference policy as the baseline.

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



___
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 00/34] VFS: Introduce filesystem context [ver #12]

2018-10-04 Thread Eric W. Biederman


David,

I have been going through these and it is a wonderful proof of concept
patchset.  There are a couple significant problems with it however.

- Many patches do more than one thing that could benefit from being
  broken up into more patches so that there is only one logical change
  per patch.  I have attempted a little of that and have found several
  significant bugs.

- There are many unnecessary changes in this patchset that just add
  noise and make it difficult to review.

- There are many typos and thinkos in this patchset that while not hard
  to correct keep this from being anywhere close to being ready for
  prime time.

- Some of the bugs I have encountered.
  * proc that isn't pid_ns_prepare_proc does not set fc->user_ns to
match the pid namespace.
  * mqueue does not set fc->user_ns to match the ipc namespace.
  * The cpuset filesystem always fails to mount
  * Non-converted filesystems don't have the old security hooks
and only have a bit blob so don't call into the new security
hooks either.
  * The changes to implement the new security hooks at least for
selinux are riddled with typos, and thinkos.

I was hoping to get into the semantic questions but I can't get
there until I get a good solid baseline patch to work with.

I have been able to hoist the permission check out of sget_fc for
converted filesystems.  So progress is being made.  That absolutely
requires fc->user_ns to be set properly before vfs_get_tree.  Something
that still needs to be fixed.

I have also observed that by not allowing unconverted filesystems
to mount using the new api.  The compatbitility code can be
significantly simplified, and the who data_size problem goes away.

I am going to be travelling for the next couple of days so I
don't expect I will be able to answer questions in a timely manner.
In the hopes that it might help below is my work in progress git
tree where I have cleaned up some of these issues.

https://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git 
new-mount-api-testing

Eric


___
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 v5 2/5] Smack: Prepare for PTRACE_MODE_SCHED

2018-10-04 Thread Jiri Kosina
On Thu, 27 Sep 2018, Jann Horn wrote:

> > Yes. Since the PTRACE_MODE_NOAUDIT was in PTRACE_MODE_IBPB in Jiri's 
> > previous patch set and not in PTRACE_MODE_SCHED in this one I assumed 
> > that there was a good reason for it.
> 
> Jiri, was there a good reason for it, and if so, what was it?

[ FWIW PTRACE_MODE_NOAUDIT being in PTRACE_MODE_IBPB goes back to original 
  Tim's pre-CRD patchset ]

Well, we can't really call out into audit from scheduler code, and the 
previous versions of the patchsets didn't have PTRACE_MODE_SCHED, so it 
had to be included in PTRACE_MODE_IBPB in order to make sure we're not 
calling into audit from context switch code.

Or did I misunderstand the question?

Thanks,

-- 
Jiri Kosina
SUSE Labs

___
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 v5 2/5] Smack: Prepare for PTRACE_MODE_SCHED

2018-10-04 Thread Jann Horn via Selinux
On Thu, Oct 4, 2018 at 9:47 AM Jiri Kosina  wrote:
> On Thu, 27 Sep 2018, Jann Horn wrote:
> > > Yes. Since the PTRACE_MODE_NOAUDIT was in PTRACE_MODE_IBPB in Jiri's
> > > previous patch set and not in PTRACE_MODE_SCHED in this one I assumed
> > > that there was a good reason for it.
> >
> > Jiri, was there a good reason for it, and if so, what was it?
>
> [ FWIW PTRACE_MODE_NOAUDIT being in PTRACE_MODE_IBPB goes back to original
>   Tim's pre-CRD patchset ]
>
> Well, we can't really call out into audit from scheduler code, and the
> previous versions of the patchsets didn't have PTRACE_MODE_SCHED, so it
> had to be included in PTRACE_MODE_IBPB in order to make sure we're not
> calling into audit from context switch code.
>
> Or did I misunderstand the question?

If I understand Casey correctly, he is saying that your patch
(https://lore.kernel.org/lkml/nycvar.yfh.7.76.1809251437340.15...@cbobk.fhfr.pm/)
doesn't include PTRACE_MODE_NOAUDIT for IBPB, but the previous v6 of
your patch 
(https://lore.kernel.org/lkml/nycvar.yfh.7.76.1809121105330.15...@cbobk.fhfr.pm/)
did include it, and therefore Casey thinks that there is a specific
reason why you removed PTRACE_MODE_NOAUDIT, and therefore Casey is
adding special-case logic for PTRACE_MODE_SCHED to Smack when simply
using PTRACE_MODE_NOAUDIT would also work.

I think that Casey should change ptrace_may_access_sched() to use
"mode | PTRACE_MODE_SCHED | PTRACE_MODE_NOAUDIT".
___
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 v4 14/19] LSM: Infrastructure management of the inode security

2018-10-04 Thread Casey Schaufler
On 10/3/2018 11:13 AM, James Morris wrote:
> On Fri, 21 Sep 2018, Kees Cook wrote:
>
>> On Fri, Sep 21, 2018 at 5:19 PM, Casey Schaufler  
>> wrote:
>>> + * lsm_early_inode - during initialization allocate a composite inode blob
>>> + * @inode: the inode that needs a blob
>>> + *
>>> + * Allocate the inode blob for all the modules if it's not already there
>>> + */
>>> +void lsm_early_inode(struct inode *inode)
>>> +{
>>> +   int rc;
>>> +
>>> +   if (inode == NULL)
>>> +   panic("%s: NULL inode.\n", __func__);
>>> +   if (inode->i_security != NULL)
>>> +   return;
>>> +   rc = lsm_inode_alloc(inode);
>>> +   if (rc)
>>> +   panic("%s: Early inode alloc failed.\n", __func__);
>>> +}
>> I'm still advising against using panic(), but I'll leave it up to James.
>>
> Calling panic() is not appropriate here. Perhaps if it was during 
> boot-time initialization of LSM infrastructure, but not on the fly.

Tetsuo's patch makes this an __init function. It's only for doing
init time stuff like root inode initialization during start-up.
If it fails the caller is going to have to panic. This came straight
out of the SELinux system initialization code. I could go back to
having each LSM do it's own panic, but that seems silly.

>
> Use a WARN_ONCE then propagate the error back and fail the operation.
>
>

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