Re: [PATCH] secilc: better error handling
On Fri, Sep 21, 2018 at 5:12 PM Nick Kralevich via Selinux <
selinux@tycho.nsa.gov> wrote:
> Fix a situation where the secilc command line tool could return success
> even though the compilation failed.
>
> $ secilc /dev/null -o /dev/null -f /dev/null
> Failure reading file: /dev/null
> $ echo $?
> 0
>
> Fix a few other minor oversights while I'm here.
>
I'd prefer this split into at least 2 patches on the off chance we
need to revert the actual code changes we don't lose the spelling
and whitespace fixes. Otherwise LGTM.
>
> Signed-off-by: Nick Kralevich
> ---
> libsepol/include/sepol/errcodes.h | 2 +-
> secilc/secilc.c | 8 ++--
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/libsepol/include/sepol/errcodes.h
> b/libsepol/include/sepol/errcodes.h
> index 0136564a..6e9ff316 100644
> --- a/libsepol/include/sepol/errcodes.h
> +++ b/libsepol/include/sepol/errcodes.h
> @@ -12,7 +12,7 @@ extern "C" {
> #define SEPOL_OK 0
>
> /* These first error codes are defined for compatibility with
> - * previous version of libsepol. In the future, custome error
> + * previous version of libsepol. In the future, custom error
> * codes that don't map to system error codes should be defined
> * outside of the range of system error codes.
> */
> diff --git a/secilc/secilc.c b/secilc/secilc.c
> index 0be6975b..8578cc26 100644
> --- a/secilc/secilc.c
> +++ b/secilc/secilc.c
> @@ -257,14 +257,16 @@ int main(int argc, char *argv[])
> rc = stat(argv[i], &filedata);
> if (rc == -1) {
> fprintf(stderr, "Could not stat file: %s\n",
> argv[i]);
> + rc = SEPOL_ERR;
> goto exit;
> }
> - file_size = filedata.st_size;
> + file_size = filedata.st_size;
>
> buffer = malloc(file_size);
> rc = fread(buffer, file_size, 1, file);
> if (rc != 1) {
> fprintf(stderr, "Failure reading file: %s\n",
> argv[i]);
> + rc = SEPOL_ERR;
> goto exit;
> }
> fclose(file);
> @@ -345,11 +347,13 @@ int main(int argc, char *argv[])
>
> if (file_contexts == NULL) {
> fprintf(stderr, "Failed to open file_contexts file\n");
> + rc = SEPOL_ERR;
> goto exit;
> }
> -
> +
> if (fwrite(fc_buf, sizeof(char), fc_size, file_contexts) !=
> fc_size) {
> fprintf(stderr, "Failed to write file_contexts file\n");
> + rc = SEPOL_ERR;
> goto exit;
> }
>
> --
> 2.19.0.444.g18242da7ef-goog
>
> ___
> 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.
[PATCH] secilc: better error handling
Fix a situation where the secilc command line tool could return success
even though the compilation failed.
$ secilc /dev/null -o /dev/null -f /dev/null
Failure reading file: /dev/null
$ echo $?
0
Fix a few other minor oversights while I'm here.
Signed-off-by: Nick Kralevich
---
libsepol/include/sepol/errcodes.h | 2 +-
secilc/secilc.c | 8 ++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/libsepol/include/sepol/errcodes.h
b/libsepol/include/sepol/errcodes.h
index 0136564a..6e9ff316 100644
--- a/libsepol/include/sepol/errcodes.h
+++ b/libsepol/include/sepol/errcodes.h
@@ -12,7 +12,7 @@ extern "C" {
#define SEPOL_OK 0
/* These first error codes are defined for compatibility with
- * previous version of libsepol. In the future, custome error
+ * previous version of libsepol. In the future, custom error
* codes that don't map to system error codes should be defined
* outside of the range of system error codes.
*/
diff --git a/secilc/secilc.c b/secilc/secilc.c
index 0be6975b..8578cc26 100644
--- a/secilc/secilc.c
+++ b/secilc/secilc.c
@@ -257,14 +257,16 @@ int main(int argc, char *argv[])
rc = stat(argv[i], &filedata);
if (rc == -1) {
fprintf(stderr, "Could not stat file: %s\n", argv[i]);
+ rc = SEPOL_ERR;
goto exit;
}
- file_size = filedata.st_size;
+ file_size = filedata.st_size;
buffer = malloc(file_size);
rc = fread(buffer, file_size, 1, file);
if (rc != 1) {
fprintf(stderr, "Failure reading file: %s\n", argv[i]);
+ rc = SEPOL_ERR;
goto exit;
}
fclose(file);
@@ -345,11 +347,13 @@ int main(int argc, char *argv[])
if (file_contexts == NULL) {
fprintf(stderr, "Failed to open file_contexts file\n");
+ rc = SEPOL_ERR;
goto exit;
}
-
+
if (fwrite(fc_buf, sizeof(char), fc_size, file_contexts) != fc_size) {
fprintf(stderr, "Failed to write file_contexts file\n");
+ rc = SEPOL_ERR;
goto exit;
}
--
2.19.0.444.g18242da7ef-goog
___
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 1/1] python/sepolicy: fix compatibility with setools 4.2.0
On Thu, Sep 20, 2018 at 9:48 AM Vit Mojzis wrote: > > > On 19/09/2018 22:51, Nicolas Iooss wrote: > > 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 > > > Thank you for the patch, I've just been testing my version (almost > identical except for this block). > Why don't you get the boolean state directly from the booleans inside > the conditional? Thanks for your review. I missed that "boolean.state" was available when looking for a way to replace rule.qpol_symbol.is_enabled(), as it does not appear in "repr(boolean)". If you send your patch, I will accept it. Otherwise I will send a v2 that will most likely be exactly like your version. How do you want to proceed? By the way, I have tested that boolean.state is available in both setools 4.1.1 and setools 4.2.0-beta. 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
merged: https://github.com/SELinuxProject/selinux/pull/99
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);
> }
> 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;
> - print
[PATCH 10/34] selinux: Implement the new mount API LSM hooks [ver #12]
Implement the new mount API LSM hooks for SELinux. At some point the old
hooks will need to be removed.
Question: Should the ->fs_context_parse_source() hook be implemented to
check the labels on any source devices specified?
Signed-off-by: David Howells
cc: Paul Moore
cc: Stephen Smalley
cc: selinux@tycho.nsa.gov
cc: linux-security-mod...@vger.kernel.org
---
security/selinux/hooks.c| 336 ---
security/selinux/include/security.h | 16 +-
2 files changed, 319 insertions(+), 33 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 9102a8fecb15..5f2af9dd44fa 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -48,6 +48,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -439,24 +441,23 @@ static inline int inode_doinit(struct inode *inode)
}
enum {
- Opt_error = -1,
- Opt_context = 1,
+ Opt_context = 0,
+ Opt_defcontext = 1,
Opt_fscontext = 2,
- Opt_defcontext = 3,
- Opt_rootcontext = 4,
- Opt_labelsupport = 5,
- Opt_nextmntopt = 6,
+ Opt_rootcontext = 3,
+ Opt_seclabel = 4,
+ nr__selinux_params
};
-#define NUM_SEL_MNT_OPTS (Opt_nextmntopt - 1)
+#define NUM_SEL_MNT_OPTS (nr__selinux_params - 1)
static const match_table_t tokens = {
- {Opt_context, CONTEXT_STR "%s"},
- {Opt_fscontext, FSCONTEXT_STR "%s"},
- {Opt_defcontext, DEFCONTEXT_STR "%s"},
- {Opt_rootcontext, ROOTCONTEXT_STR "%s"},
- {Opt_labelsupport, LABELSUPP_STR},
- {Opt_error, NULL},
+ {Opt_context, CONTEXT_STR "=%s"},
+ {Opt_fscontext, FSCONTEXT_STR "=%s"},
+ {Opt_defcontext, DEFCONTEXT_STR "=%s"},
+ {Opt_rootcontext, ROOTCONTEXT_STR "=%s"},
+ {Opt_seclabel, SECLABEL_STR},
+ {-1, NULL},
};
#define SEL_MOUNT_FAIL_MSG "SELinux: duplicate or incompatible mount
options\n"
@@ -615,15 +616,11 @@ static int selinux_get_mnt_opts(const struct super_block
*sb,
if (!selinux_state.initialized)
return -EINVAL;
- /* make sure we always check enough bits to cover the mask */
- BUILD_BUG_ON(SE_MNTMASK >= (1 << NUM_SEL_MNT_OPTS));
-
tmp = sbsec->flags & SE_MNTMASK;
/* count the number of mount options for this sb */
for (i = 0; i < NUM_SEL_MNT_OPTS; i++) {
- if (tmp & 0x01)
+ if (tmp & (1 << i))
opts->num_mnt_opts++;
- tmp >>= 1;
}
/* Check if the Label support flag is set */
if (sbsec->flags & SBLABEL_MNT)
@@ -1154,7 +1151,7 @@ static int selinux_parse_opts_str(char *options,
goto out_err;
}
break;
- case Opt_labelsupport:
+ case Opt_seclabel:
break;
default:
rc = -EINVAL;
@@ -1259,7 +1256,7 @@ static void selinux_write_opts(struct seq_file *m,
break;
case SBLABEL_MNT:
seq_putc(m, ',');
- seq_puts(m, LABELSUPP_STR);
+ seq_puts(m, SECLABEL_STR);
continue;
default:
BUG();
@@ -1268,6 +1265,7 @@ static void selinux_write_opts(struct seq_file *m,
/* we need a comma before each option */
seq_putc(m, ',');
seq_puts(m, prefix);
+ seq_putc(m, '=');
if (has_comma)
seq_putc(m, '\"');
seq_escape(m, opts->mnt_opts[i], "\"\n\\");
@@ -2753,11 +2751,11 @@ static inline int match_prefix(char *prefix, int plen,
char *option, int olen)
static inline int selinux_option(char *option, int len)
{
- return (match_prefix(CONTEXT_STR, sizeof(CONTEXT_STR)-1, option, len) ||
- match_prefix(FSCONTEXT_STR, sizeof(FSCONTEXT_STR)-1, option,
len) ||
- match_prefix(DEFCONTEXT_STR, sizeof(DEFCONTEXT_STR)-1, option,
len) ||
- match_prefix(ROOTCONTEXT_STR, sizeof(ROOTCONTEXT_STR)-1,
option, len) ||
- match_prefix(LABELSUPP_STR, sizeof(LABELSUPP_STR)-1, option,
len));
+ return (match_prefix(CONTEXT_STR"=", sizeof(CONTEXT_STR)-1, option,
len) ||
+ match_prefix(FSCONTEXT_STR"=", sizeof(FSCONTEXT_STR)-1, option,
len) ||
+ match_prefix(DEFCONTEXT_STR"=", sizeof(DEFCONTEXT_STR)-1,
option, len) ||
+ match_prefix(ROOTCONTEXT_STR"=", sizeof(ROOTCONTEXT_STR)-1,
option, len) ||
+ match_prefix(SECLABEL_STR"=", sizeof(SECLABEL_STR)-1, option,
len));
}
static inline void take_option(char **to, char *from, int *first, int len)
@@ -2972,6 +2970,284 @@ static int selinux_umount(struct vfsmount *mnt, int
flags)
FILESYSTEM_
[PATCH 00/34] VFS: Introduce filesystem context [ver #12]
Hi Al,
Here are a set of patches to create a filesystem context prior to setting
up a new mount, populating it with the parsed options/binary data, creating
the superblock and then effecting the mount. This is also used for remount
since much of the parsing stuff is common in many filesystems.
This allows namespaces and other information to be conveyed through the
mount procedure.
This also allows Miklós Szeredi's idea of doing:
fd = fsopen("nfs");
fsconfig(fd, FSCONFIG_SET_STRING, "option", "val", 0);
fsconfig(fd, FSCONFIG_CMD_CREATE, NULL, NULL, 0);
mfd = fsmount(fd, MS_NODEV);
move_mount(mfd, "", AT_FDCWD, "/mnt", MOVE_MOUNT_F_EMPTY_PATH);
that he presented at LSF-2017 to be implemented (see the relevant patches
in the series).
I didn't use netlink as that would make the core kernel depend on
CONFIG_NET and CONFIG_NETLINK and would introduce network namespacing
issues.
I've implemented filesystem context handling for procfs, nfs, mqueue,
cpuset, kernfs, sysfs, cgroup and afs filesystems.
Unconverted filesystems are handled by a legacy filesystem wrapper.
WHY DO WE WANT THIS?
Firstly, there's a bunch of problems with the mount(2) syscall:
(1) It's actually six or seven different interfaces rolled into one and
weird combinations of flags make it do different things beyond the
original specification of the syscall.
(2) It produces a particularly large and diverse set of errors, which have
to be mapped back to a small error code. Yes, there's dmesg - if you
have it configured - but you can't necessarily see that if you're
doing a mount inside of a container.
(3) It copies a PAGE_SIZE block of data for each of the type, device name
and options.
(4) The size of the buffers is PAGE_SIZE - and this is arch dependent.
(5) You can't mount into another mount namespace. I could, for example,
build a container without having to be in that container's namespace
if I can do it from outside.
(6) It's not really geared for the specification of multiple sources, but
some filesystems really want that - overlayfs, for example.
and some problems in the internal kernel api:
(1) There's no defined way to supply namespace configuration for the
superblock - so, for instance, I can't say that I want to create a
superblock in a particular network namespace (on automount, say).
NFS hacks around this by creating multiple shadow file_system_types
with different ->mount() ops.
(2) When calling mount internally, unless you have NFS-like hacks, you
have to generate or otherwise provide text config data which then gets
parsed, when some of the time you could bypass the parsing stage
entirely.
(3) The amount of data in the data buffer is not known, but the data
buffer might be on a kernel stack somewhere, leading to the
possibility of tripping the stack underrun guard.
and other issues too:
(1) Superblock remount in some filesystems applies options on an as-parsed
basis, so if there's a parse failure, a partial alteration with no
rollback is effected.
(2) Under some circumstances, the mount data may get copied multiple times
so that it can have multiple parsers applied to it or because it has
to be parsed multiple times - for instance, once to get the
preliminary info required to access the on-disk superblock and then
again to update the superblock record in the kernel.
I want to be able to add support for a bunch of things:
(1) UID, GID and Project ID mapping/translation. I want to be able to
install a translation table of some sort on the superblock to
translate source identifiers (which may be foreign numeric UIDs/GIDs,
text names, GUIDs) into system identifiers. This needs to be done
before the superblock is published[*].
Note that this may, for example, involve using the context and the
superblock held therein to issue an RPC to a server to look up
translations.
[*] By "published" I mean made available through mount so that other
userspace processes can access it by path.
Maybe specifying a translation range element with something like:
fsconfig(fd, fsconfig_translate_uid, " ", 0, 0);
The translation information also needs to propagate over an automount
in some circumstances.
(2) Namespace configuration. I want to be able to tell the superblock
creation process what namespaces should be applied when it created (in
particular the userns and netns) for containerisation purposes, e.g.:
fsconfig(fd, FSCONFIG_SET_NAMESPACE, "user", 0, userns_fd);
fsconfig(fd, FSCONFIG_SET_NAMESPACE, "net", 0, netns_fd);
(3) Namespace propagation. I want to have a properly defined mechanism
for propagating namespace configuration over automounts within the
kernel. This will be particularly use
Re: Bug in selinux on ubuntu 16.04 with kernel 4.15.0-34
On 09/21/2018 04:50 AM, Benjamin Schüle wrote:
Hello,
just found a bug in selinux. It appears on ubuntu 16.04 with kernel
4.15, but not with kernel 4.4.
What's going wrong:
Copy a link with "-a" option while selinux is on.
steps to reproduce:
~$ mkdir -p a/b
~$ ln -s b a/c
~$ cp -a a b
cp: failed to restore the default file creation context: Invalid argument
Results of my investigation:
The "cp" of coreutils is calling "setfscreatecon (NULL)" to restore
the default file creation context (coreutils-8.30/src/copy.c:1771) as
it is stated in the selinux api
(/libselinux/include/selinux/selinux.h:71).
As we see in the result of strace below, the kernel returns an -1 on
try to restore the default file creation context. So, in my opinion,
is the bug has to be in the selinux_setprocattr method in the
security/selinux/hooks.c file.
Part of "strace cp -a a b"
lgetxattr("a/c", "security.selinux",
"system_u:object_r:user_home_dir_t:s0", 255) = 37
readlink("a/c", "b", 2) = 1
symlink("b", "b/a/c") = 0
open("/proc/self/task/2136/attr/fscreate", O_RDWR|O_CLOEXEC) = 3
write(3, NULL, 0) = -1 EINVAL (Invalid argument)
close(3)= 0
open("/usr/share/locale/locale.alias", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=2995, ...}) = 0
read(3, "# Locale name alias data base.\n#"..., 4096) = 2995
read(3, "", 4096) = 0
close(3)
This appears to be Ubuntu-specific; I can't reproduce upstream. If you
are able to reproduce with an upstream kernel, let us know; otherwise,
file a bug with Ubuntu. A quick look at the Ubuntu kernel git tree
shows the following commit which would explain this regression.
commit 36788bfe15f16b2eba39d0e563ae8027c5072b98
Author: Colin Ian King
Date: Tue Oct 3 13:12:54 2017 +0100
UBUNTU: SAUCE: LSM stacking: check for invalid zero sized writes
BugLink: http://bugs.launchpad.net/bugs/1720779
BugLink: http://bugs.launchpad.net/bugs/1763062
Writing zero bytes to /proc/$pid/task/$pid/attr/context via
security_setprocattr cause an oops in memcpy_erms. Fix this by
checking for zero size and returning -EINVAL for this invalid
write size.
Detected by running stress-ng --procfs 0
Signed-off-by: Colin Ian King
Signed-off-by: Seth Forshee
___
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
On 09/20/2018 06:59 PM, Taras Kondratiuk wrote:
Quoting Stephen Smalley (2018-09-20 07:49:12)
On 09/19/2018 10:41 PM, Taras Kondratiuk wrote:
Quoting Stephen Smalley (2018-09-19 12:00:33)
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?
Client and server are two embedded devices. They are part of a bigger
modular system and normally run the same SW version, so they understand
each others NFSv4 SELinux labels. But during migration from one SW
version to another a client and a server may run different versions for
some time.
The immediate issue I'm trying to address is a migration between
releases with and without SELinux policy. A client (with policy) gets
initial SID labels from a server (without policy). Those labels are
considered invalid, so files remain unlabeled. This also causes lots of
errors from nfs_setsecurity() in dmesg.
Why are you enabling SELinux on the server without loading a policy?
Are you concerned about denials on the server? For that, you could
always run it permissive until you are confident you have a working policy.
Why are you enabling security_label in exports before you have a policy
loaded? It doesn't appear that will ever work, since nfsd asks the
security module for the labels, not the local filesystem directly.
I understand that this doesn't fully address your use case, but it seems
like you could avoid this particular situation altogether just by
loading a policy (at which point your server can return actual contexts)
or by removing security_label from your exports (at which point your
server won't try returning contexts and the client will just apply the
default for nfs) until you get to the point of loading a policy.
It wasn't intentional configuration. Server is running v4.4.x kernel
that had security labels enabled by default for NFS 4.2. This was a bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1406885
Commit that introduced security_label 32ddd944a056 ("nfsd: opt in to
labeled nfs per export") appeared in v4.11 only.
We hit this bug during migration to newer releases with SELinux, but the
older release is already in the field and we need to handle it.
Ok, I understand the kernel bug, but not why your servers are in a state
where SELinux is enabled but no policy is loaded. That is not a
well-tested code path aside from early system initialization prior to
first policy load by systemd/init. You would be better off either
disabling SELinux on the servers entirely (thereby automatically
disabling NFSv4.2 security labeling) or installing/loading a policy on
the servers (thereby enabling them to produce valid security labels for
the client at least to the extent that they agree on policy). If you
are concerned about denials on the server, then you could always leave
the servers permissive initially and collect audit logs until you are
confident your policy is adequate.
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.
inode_doinit_with_dentry() invokes security_context_to_sid_default()
that invokes security_context_to_sid_core() with 'force' flag. Won't
sidtab_context_to_sid() in this case allocate a new SID for the invalid
xattr context instead of leaving it unlabeled?
It will be treated as having the unlabeled context for access control
purposes and that is what getxattr will return to userspace processes
unless they have CAP_MAC_ADMIN and SELinux mac_admin permission, but
internally SELinux will keep track of the original xattr context value
and will later retry to map it upon a policy reload, switching over to
using it for access control
Re: file context not being set on el7
On Fri, Sep 21, 2018 at 7:21 AM Ted Toth wrote: > > On Fri, Sep 21, 2018 at 3:58 AM Petr Lautrbach > wrote: > >> >> Ted Toth writes: >> >> > I have something very much like the following in an fc file: >> > /usr/lib64/python2\.(6|7)/site-packages/xyz/paste -- >> > gen_context(system_u:object_r:jxyz_exec_t,s0) >> > >> > and I use the same file on el6 and el7. On el6 the file is >> > labeled as >> > specified in the python2.6 directory. However on el7 where the >> > file gets >> > installed into python2.7 the file is not labeled correctly. On >> > el7 >> > `semanage fcontext -l | grep xyz` shows the file context >> > expected but >> > `matchpathcon /usr/lib64/python2.7/site-packages/xyz/paste` does >> > not return >> > the expected context and `restorecon -RFv >> > /usr/lib64/python2.7/site-packages/xyz` has no affect. The type >> > xyz_exec_t >> > exists on both systems. It's probably something stupid I'm doing >> > but I'm >> > just not seeing it. Has anyone else experienced similar issues? >> > >> >> There's equivalency rule /usr/lib64 -> /usr/lib on el7: >> >> # semanage fcontext -a -t tmp_t >> '/usr/lib64/python2\.(6|7)/site-packages/xyz/paste' >> >> ValueError: File spec >> /usr/lib64/python2\.(6|7)/site-packages/xyz/paste conflicts with >> equivalency rule '/usr/lib64 /usr/lib'; Try adding >> '/usr/lib/python2\.(6|7)/site-packages/xyz/paste' instead >> >> >> # semanage fcontext -a -t tmp_t >> '/usr/lib/python2\.(6|7)/site-packages/xyz/paste' >> >> # matchpathcon /usr/lib64/python2.7/site-packages/xyz/paste >> /usr/lib64/python2.7/site-packages/xyz/paste >> system_u:object_r:tmp_t:s0 >> >> >> Petr >> > > Thanks, where is this equivalency rule defined/documented? > /usr/lib(64)?/python... doesn't work either how can I make it backward compatible? ___ 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: file context not being set on el7
On Fri, Sep 21, 2018 at 3:58 AM Petr Lautrbach wrote: > > Ted Toth writes: > > > I have something very much like the following in an fc file: > > /usr/lib64/python2\.(6|7)/site-packages/xyz/paste -- > > gen_context(system_u:object_r:jxyz_exec_t,s0) > > > > and I use the same file on el6 and el7. On el6 the file is > > labeled as > > specified in the python2.6 directory. However on el7 where the > > file gets > > installed into python2.7 the file is not labeled correctly. On > > el7 > > `semanage fcontext -l | grep xyz` shows the file context > > expected but > > `matchpathcon /usr/lib64/python2.7/site-packages/xyz/paste` does > > not return > > the expected context and `restorecon -RFv > > /usr/lib64/python2.7/site-packages/xyz` has no affect. The type > > xyz_exec_t > > exists on both systems. It's probably something stupid I'm doing > > but I'm > > just not seeing it. Has anyone else experienced similar issues? > > > > There's equivalency rule /usr/lib64 -> /usr/lib on el7: > > # semanage fcontext -a -t tmp_t > '/usr/lib64/python2\.(6|7)/site-packages/xyz/paste' > > ValueError: File spec > /usr/lib64/python2\.(6|7)/site-packages/xyz/paste conflicts with > equivalency rule '/usr/lib64 /usr/lib'; Try adding > '/usr/lib/python2\.(6|7)/site-packages/xyz/paste' instead > > > # semanage fcontext -a -t tmp_t > '/usr/lib/python2\.(6|7)/site-packages/xyz/paste' > > # matchpathcon /usr/lib64/python2.7/site-packages/xyz/paste > /usr/lib64/python2.7/site-packages/xyz/paste > system_u:object_r:tmp_t:s0 > > > Petr > Thanks, where is this equivalency rule defined/documented? ___ 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
Quoting Stephen Smalley (2018-09-20 07:49:12)
> On 09/19/2018 10:41 PM, Taras Kondratiuk wrote:
> > Quoting Stephen Smalley (2018-09-19 12:00:33)
> >> 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?
> >
> > Client and server are two embedded devices. They are part of a bigger
> > modular system and normally run the same SW version, so they understand
> > each others NFSv4 SELinux labels. But during migration from one SW
> > version to another a client and a server may run different versions for
> > some time.
> >
> > The immediate issue I'm trying to address is a migration between
> > releases with and without SELinux policy. A client (with policy) gets
> > initial SID labels from a server (without policy). Those labels are
> > considered invalid, so files remain unlabeled. This also causes lots of
> > errors from nfs_setsecurity() in dmesg.
>
> Why are you enabling SELinux on the server without loading a policy?
> Are you concerned about denials on the server? For that, you could
> always run it permissive until you are confident you have a working policy.
>
> Why are you enabling security_label in exports before you have a policy
> loaded? It doesn't appear that will ever work, since nfsd asks the
> security module for the labels, not the local filesystem directly.
>
> I understand that this doesn't fully address your use case, but it seems
> like you could avoid this particular situation altogether just by
> loading a policy (at which point your server can return actual contexts)
> or by removing security_label from your exports (at which point your
> server won't try returning contexts and the client will just apply the
> default for nfs) until you get to the point of loading a policy.
It wasn't intentional configuration. Server is running v4.4.x kernel
that had security labels enabled by default for NFS 4.2. This was a bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1406885
Commit that introduced security_label 32ddd944a056 ("nfsd: opt in to
labeled nfs per export") appeared in v4.11 only.
We hit this bug during migration to newer releases with SELinux, but the
older release is already in the field and we need to handle it.
>
> > Using 'context=' at mount point level is an option, but in a normal case
> > when both sides have a policy we need more granular labels. It is
> > possible to detect a case when a server doesn't have a policy and
> > remount with 'context=', but this special case handling looks a bit
> > ugly. Having something like 'defcontext' whould allow to avoid this
> > special case and unconditionally assign default context to the
> > mountpoint.
> >
> > 'defcontext' may also help during migration between SELinux-enabled
> > releases if a new release introduces labels that are not recognized by
> > older release. In this case they can also fallback to defcontext.
>
> This is the more interesting case. And what would these defcontext
> values be? A context that is generally accessible to all domains on the
> client? Or one that is restricted to only unconfined domains? Would it
> be fundamentally different from unlabeled? Will it really differ
> per-mount, and if so, why?
Defcontext will be a restricted label. For example a part of confined
domains can access an exported flash disk, but they can't access
unlabeled_t. In one release the flash may have a coarse labels:
u:r:flash_t.
u:r:flash_tsome.file
u:r:flash_tlicence.file
u:r:flash_secret_t secret_dir
In the next release licence.file may get a more granular label
(licence_t), but the older release still need to be able to read the
file via NFS. With defcontext=u:r:flash_t it will be able to do it.
In our case defcontext will usually match a label or export's root
directory, so it needs to be set per-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
Re: file context not being set on el7
Ted Toth writes: I have something very much like the following in an fc file: /usr/lib64/python2\.(6|7)/site-packages/xyz/paste -- gen_context(system_u:object_r:jxyz_exec_t,s0) and I use the same file on el6 and el7. On el6 the file is labeled as specified in the python2.6 directory. However on el7 where the file gets installed into python2.7 the file is not labeled correctly. On el7 `semanage fcontext -l | grep xyz` shows the file context expected but `matchpathcon /usr/lib64/python2.7/site-packages/xyz/paste` does not return the expected context and `restorecon -RFv /usr/lib64/python2.7/site-packages/xyz` has no affect. The type xyz_exec_t exists on both systems. It's probably something stupid I'm doing but I'm just not seeing it. Has anyone else experienced similar issues? There's equivalency rule /usr/lib64 -> /usr/lib on el7: # semanage fcontext -a -t tmp_t '/usr/lib64/python2\.(6|7)/site-packages/xyz/paste' ValueError: File spec /usr/lib64/python2\.(6|7)/site-packages/xyz/paste conflicts with equivalency rule '/usr/lib64 /usr/lib'; Try adding '/usr/lib/python2\.(6|7)/site-packages/xyz/paste' instead # semanage fcontext -a -t tmp_t '/usr/lib/python2\.(6|7)/site-packages/xyz/paste' # matchpathcon /usr/lib64/python2.7/site-packages/xyz/paste /usr/lib64/python2.7/site-packages/xyz/paste system_u:object_r:tmp_t:s0 Petr ___ 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.
Bug in selinux on ubuntu 16.04 with kernel 4.15.0-34
Hello,
just found a bug in selinux. It appears on ubuntu 16.04 with kernel
4.15, but not with kernel 4.4.
What's going wrong:
Copy a link with "-a" option while selinux is on.
steps to reproduce:
~$ mkdir -p a/b
~$ ln -s b a/c
~$ cp -a a b
cp: failed to restore the default file creation context: Invalid argument
Results of my investigation:
The "cp" of coreutils is calling "setfscreatecon (NULL)" to restore
the default file creation context (coreutils-8.30/src/copy.c:1771) as
it is stated in the selinux api
(/libselinux/include/selinux/selinux.h:71).
As we see in the result of strace below, the kernel returns an -1 on
try to restore the default file creation context. So, in my opinion,
is the bug has to be in the selinux_setprocattr method in the
security/selinux/hooks.c file.
Part of "strace cp -a a b"
lgetxattr("a/c", "security.selinux",
"system_u:object_r:user_home_dir_t:s0", 255) = 37
readlink("a/c", "b", 2) = 1
symlink("b", "b/a/c") = 0
open("/proc/self/task/2136/attr/fscreate", O_RDWR|O_CLOEXEC) = 3
write(3, NULL, 0) = -1 EINVAL (Invalid argument)
close(3)= 0
open("/usr/share/locale/locale.alias", O_RDONLY|O_CLOEXEC) = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=2995, ...}) = 0
read(3, "# Locale name alias data base.\n#"..., 4096) = 2995
read(3, "", 4096) = 0
close(3)
___
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.
