On 5/30/19 12:22 PM, Richard Haines wrote:
On Tue, 2019-05-28 at 10:24 -0400, Stephen Smalley wrote:
On 5/24/19 2:06 PM, Richard Haines wrote:
On Fri, 2019-05-24 at 13:11 -0400, Stephen Smalley wrote:
On 5/22/19 12:42 PM, Richard Haines wrote:
These patches require [1] and [2] be installed first. They have
been implemented on Android and sent to the selinux list,
however
their
merge has been deferred. They will install the core hashing of
file_context entries and fix root stem processing.
Patch 1/3 updates selinux_restorecon() replacing the per-
mountpoint
security.restorecon_last attribute with a per-directory
security.sehash
attribute computed from only those file contexts entries that
partially
match the directory. This is to avoid the need to walk the
entire
tree
when any part of file_contexts changes, limiting relabels to
only
those
parts of the tree that could have changed.
One change is to add a new
selabel_get_digests_all_partial_matches(3)
function that is explained in the man page. This could replace
the
Android
version of selabel_hash_all_partial_matches(3), that could then
be
converted into a local function (The Android team would need to
approve).
Has Android sorted out all of the ramifications of this change?
As I'm not involved with Android I don't know. Note that as I do
not
change their core selabel code my new function could be added to
libselinux and they pick it up if they want it (or it gets rejected
by
the SELinux team). I've added Nick to the cc list in case any
comments.
The main reason I added the function was the call retrieved the
info in
one go plus you did't need to know the digest length.
What
about the triggering of CAP_SYS_ADMIN denials for setting the
security.sehash attribute?
They solved this by adding the *_RESTORECON_SKIP_SEHASH flag that
I've
copied (the selinux-testsuite restorecon has tests for this).
Per
https://android-review.googlesource.com/c/platform/external/selinux/+/940326,
they say:
"TODO: It would be better if the default for restorecon was to
suppress
the hash computation, since otherwise it encourages programs to be
overprivileged with CAP_SYS_ADMIN. I'll plan on doing that in a
followup
commit."
So I think we would want to disable the setting of sehash by default
upstream to match their planned future behavior. In fact, I think
that
is how your version of setting security.restoreconlast worked
upstream
originally - the caller had to selabel_open() with options containing
a
SELABEL_OPT_DIGEST option with a non-zero value in order to enable
setting of a digest in the first place? By the way, this patch set
seems to dispense with SELABEL_OPT_DIGEST and selabel_digest() usage
internally even though it leaves them defined, thereby changing
behavior
for existing API callers?
I'll fix this in the next set of patches to follow the original idea of
calling selabel_open to determine the behavior. However I see Android
have abandoned these changes (I could not see the reason why).
I'm happy to continue if you think worthwhile.
I think they abandoned changes to revert these changes, i.e. they are
actually keeping them. I assume this means that they were going to
revert them due to some bug (not accessible to us) but that they
resolved that bug in some other way.
[1]
https://android-review.googlesource.com/c/platform/external/selinux/+/932005
Patches 2/3 and 3/3 update restorecon, setfiles and
restorecond.
I will send a patch for the selinux-testsuite that will perform
tests on
the new code.
[1]
https://lore.kernel.org/selinux/20190311222442.49824-1-xunchang@xxxxxxxxxx/
[2]
https://lore.kernel.org/selinux/20190417180955.136942-1-xunchang@xxxxxxxxxx/
Richard Haines (3):
libselinux: Save digest of all partial matches for
directory
setfiles: Update utilities for the new digest scheme
restorecond: Update to handle new digest scheme
libselinux/include/selinux/label.h | 5 +
libselinux/include/selinux/restorecon.h | 17 +-
.../selabel_get_digests_all_partial_matches.3 | 70 +++++
libselinux/man/man3/selinux_restorecon.3 | 91 +++---
.../man3/selinux_restorecon_default_handle.3 | 9 +-
.../man/man3/selinux_restorecon_xattr.3 | 11 +-
libselinux/src/label.c | 15 +
libselinux/src/label_file.c | 51 ++++
libselinux/src/label_file.h | 4 +
libselinux/src/label_internal.h | 5 +
libselinux/src/selinux_restorecon.c | 267
+++++++++++
-------
libselinux/utils/.gitignore | 1 +
.../selabel_get_digests_all_partial_matches.c | 170
+++++++++++
policycoreutils/setfiles/restore.c | 7 +-
policycoreutils/setfiles/restore.h | 2 +-
policycoreutils/setfiles/restorecon.8 | 10 +-
policycoreutils/setfiles/restorecon_xattr.8 | 19 +-
policycoreutils/setfiles/restorecon_xattr.c | 66 +----
policycoreutils/setfiles/setfiles.8 | 10 +-
policycoreutils/setfiles/setfiles.c | 19 +-
restorecond/restore.c | 8 +-
restorecond/restore.h | 2 +-
restorecond/restorecond.c | 5 +-
23 files changed, 593 insertions(+), 271 deletions(-)
create mode 100644
libselinux/man/man3/selabel_get_digests_all_partial_matches.3
create mode 100644
libselinux/utils/selabel_get_digests_all_partial_matches.c
