* libsemanage: Perform access check using euid instead of uid v2
@ 2018-03-06 11:58 Vit Mojzis
2018-03-06 11:58 ` [PATCH 1/3] libsemanage: remove access() check to make setuid programs work Vit Mojzis
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Vit Mojzis @ 2018-03-06 11:58 UTC (permalink / raw)
To: selinux
Changes:
- replace semanage_copy_file by copy_file_if_exists to make sure "retval" is 0 if the file does not exist
- restructure if statements to be more clear ("fail" is last part of the statement)
- replace read test (attempt to open the file) by stat() call
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/3] libsemanage: remove access() check to make setuid programs work
2018-03-06 11:58 libsemanage: Perform access check using euid instead of uid v2 Vit Mojzis
@ 2018-03-06 11:58 ` Vit Mojzis
2018-03-06 11:58 ` [PATCH 2/3] " Vit Mojzis
2018-03-06 11:58 ` [PATCH 3/3] libsemanage: replace access() checks " Vit Mojzis
2 siblings, 0 replies; 19+ messages in thread
From: Vit Mojzis @ 2018-03-06 11:58 UTC (permalink / raw)
To: selinux
access() uses real UID instead of effective UID which causes false
negative checks in setuid programs. Remove redundant access() checks
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
---
libsemanage/src/direct_api.c | 7 -------
libsemanage/src/semanage_store.c | 17 ++++++++---------
2 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index 88873c43..b7899d68 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -148,9 +148,6 @@ int semanage_direct_connect(semanage_handle_t * sh)
if (semanage_create_store(sh, 1))
goto err;
- if (semanage_access_check(sh) < SEMANAGE_CAN_READ)
- goto err;
-
sh->u.direct.translock_file_fd = -1;
sh->u.direct.activelock_file_fd = -1;
@@ -398,10 +395,6 @@ static int semanage_direct_disconnect(semanage_handle_t *sh)
static int semanage_direct_begintrans(semanage_handle_t * sh)
{
-
- if (semanage_access_check(sh) != SEMANAGE_CAN_WRITE) {
- return -1;
- }
if (semanage_get_trans_lock(sh) < 0) {
return -1;
}
diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c
index 936e6495..4bd1d651 100644
--- a/libsemanage/src/semanage_store.c
+++ b/libsemanage/src/semanage_store.c
@@ -538,7 +538,6 @@ char *semanage_conf_path(void)
int semanage_create_store(semanage_handle_t * sh, int create)
{
struct stat sb;
- int mode_mask = R_OK | W_OK | X_OK;
const char *path = semanage_files[SEMANAGE_ROOT];
int fd;
@@ -557,9 +556,9 @@ int semanage_create_store(semanage_handle_t * sh, int create)
return -1;
}
} else {
- if (!S_ISDIR(sb.st_mode) || access(path, mode_mask) == -1) {
+ if (!S_ISDIR(sb.st_mode)) {
ERR(sh,
- "Could not access module store at %s, or it is not a directory.",
+ "Module store at %s is not a directory.",
path);
return -1;
}
@@ -580,9 +579,9 @@ int semanage_create_store(semanage_handle_t * sh, int create)
return -1;
}
} else {
- if (!S_ISDIR(sb.st_mode) || access(path, mode_mask) == -1) {
+ if (!S_ISDIR(sb.st_mode)) {
ERR(sh,
- "Could not access module store active subdirectory at %s, or it is not a directory.",
+ "Module store active subdirectory at %s is not a directory.",
path);
return -1;
}
@@ -603,9 +602,9 @@ int semanage_create_store(semanage_handle_t * sh, int create)
return -1;
}
} else {
- if (!S_ISDIR(sb.st_mode) || access(path, mode_mask) == -1) {
+ if (!S_ISDIR(sb.st_mode)) {
ERR(sh,
- "Could not access module store active modules subdirectory at %s, or it is not a directory.",
+ "Module store active modules subdirectory at %s is not a directory.",
path);
return -1;
}
@@ -624,8 +623,8 @@ int semanage_create_store(semanage_handle_t * sh, int create)
return -1;
}
} else {
- if (!S_ISREG(sb.st_mode) || access(path, R_OK | W_OK) == -1) {
- ERR(sh, "Could not access lock file at %s.", path);
+ if (!S_ISREG(sb.st_mode)) {
+ ERR(sh, "Object at %s is not a lock file.", path);
return -1;
}
}
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/3] libsemanage: remove access() check to make setuid programs work
2018-03-06 11:58 libsemanage: Perform access check using euid instead of uid v2 Vit Mojzis
2018-03-06 11:58 ` [PATCH 1/3] libsemanage: remove access() check to make setuid programs work Vit Mojzis
@ 2018-03-06 11:58 ` Vit Mojzis
2018-03-08 19:54 ` Stephen Smalley
2018-03-06 11:58 ` [PATCH 3/3] libsemanage: replace access() checks " Vit Mojzis
2 siblings, 1 reply; 19+ messages in thread
From: Vit Mojzis @ 2018-03-06 11:58 UTC (permalink / raw)
To: selinux
F_OK access checks only work properly as long as all directories along
the path are accessible to real user running the program.
Replace F_OK access checks by testing return value of open, write, etc.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
---
libsemanage/src/direct_api.c | 47 ++++++++++++++++++++++----------------------
1 file changed, 23 insertions(+), 24 deletions(-)
diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index b7899d68..49cac14f 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -1171,6 +1171,14 @@ cleanup:
return status;
}
+/* Copies a file from src to dst. If dst already exists then
+ * overwrite it. If source doesn't exist then return success.
+ * Returns 0 on success, -1 on error. */
+static int copy_file_if_exists(const char *src, const char *dst, mode_t mode){
+ int rc = semanage_copy_file(src, dst, mode);
+ return (rc < 0 && errno != ENOENT) ? rc : 0;
+}
+
/********************* direct API functions ********************/
/* Commits all changes in sandbox to the actual kernel policy.
@@ -1563,34 +1571,25 @@ rebuild:
goto cleanup;
}
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC_LOCAL);
- if (access(path, F_OK) == 0) {
- retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC_LOCAL),
- semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC_LOCAL),
- sh->conf->file_mode);
- if (retval < 0) {
- goto cleanup;
- }
+ retval = copy_file_if_exists(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC_LOCAL),
+ semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC_LOCAL),
+ sh->conf->file_mode);
+ if (retval < 0) {
+ goto cleanup;
}
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
- if (access(path, F_OK) == 0) {
- retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC),
- semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC),
- sh->conf->file_mode);
- if (retval < 0) {
- goto cleanup;
- }
+ retval = copy_file_if_exists(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC),
+ semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC),
+ sh->conf->file_mode);
+ if (retval < 0) {
+ goto cleanup;
}
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
- if (access(path, F_OK) == 0) {
- retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS),
- semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_SEUSERS),
- sh->conf->file_mode);
- if (retval < 0) {
- goto cleanup;
- }
+ retval = copy_file_if_exists(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS),
+ semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_SEUSERS),
+ sh->conf->file_mode);
+ if (retval < 0) {
+ goto cleanup;
}
/* run genhomedircon if its enabled, this should be the last operation
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/3] libsemanage: replace access() checks to make setuid programs work
2018-03-06 11:58 libsemanage: Perform access check using euid instead of uid v2 Vit Mojzis
2018-03-06 11:58 ` [PATCH 1/3] libsemanage: remove access() check to make setuid programs work Vit Mojzis
2018-03-06 11:58 ` [PATCH 2/3] " Vit Mojzis
@ 2018-03-06 11:58 ` Vit Mojzis
2018-03-07 14:59 ` Stephen Smalley
2 siblings, 1 reply; 19+ messages in thread
From: Vit Mojzis @ 2018-03-06 11:58 UTC (permalink / raw)
To: selinux
access() uses real UID instead of effective UID which causes false
negative checks in setuid programs.
Replace access(,F_OK) (i.e. tests for file existence) by stat().
And access(,R_OK) by fopen(,"r")
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
---
libsemanage/src/direct_api.c | 133 +++++++++++++++++++++++++--------------
libsemanage/src/semanage_store.c | 16 ++++-
2 files changed, 99 insertions(+), 50 deletions(-)
diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index 49cac14f..4e2af810 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -140,6 +140,7 @@ int semanage_direct_is_managed(semanage_handle_t * sh)
int semanage_direct_connect(semanage_handle_t * sh)
{
const char *path;
+ struct stat sb;
if (semanage_check_init(sh, sh->conf->store_root_path))
goto err;
@@ -302,10 +303,16 @@ int semanage_direct_connect(semanage_handle_t * sh)
/* set the disable dontaudit value */
path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_DISABLE_DONTAUDIT);
- if (access(path, F_OK) == 0)
+
+ if (stat(path, &sb) == 0)
sepol_set_disable_dontaudit(sh->sepolh, 1);
- else
+ else if (errno == ENOENT) {
+ /* The file does not exist */
sepol_set_disable_dontaudit(sh->sepolh, 0);
+ } else {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ goto err;
+ }
return STATUS_SUCCESS;
@@ -1139,6 +1146,7 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
int status = 0;
int i;
char cil_path[PATH_MAX];
+ struct stat sb;
assert(sh);
assert(modinfos);
@@ -1155,9 +1163,13 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
}
if (semanage_get_ignore_module_cache(sh) == 0 &&
- access(cil_path, F_OK) == 0) {
+ (status = stat(cil_path, &sb)) == 0) {
continue;
}
+ if (status != 0 && errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", cil_path, strerror(errno));
+ goto cleanup; //an error in the "stat" call
+ }
status = semanage_compile_module(sh, &modinfos[i]);
if (status < 0) {
@@ -1196,6 +1208,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
struct cil_db *cildb = NULL;
semanage_module_info_t *modinfos = NULL;
mode_t mask = umask(0077);
+ struct stat sb;
int do_rebuild, do_write_kernel, do_install;
int fcontexts_modified, ports_modified, seusers_modified,
@@ -1234,10 +1247,15 @@ static int semanage_direct_commit(semanage_handle_t * sh)
/* Create or remove the disable_dontaudit flag file. */
path = semanage_path(SEMANAGE_TMP, SEMANAGE_DISABLE_DONTAUDIT);
- if (access(path, F_OK) == 0)
+ if (stat(path, &sb) == 0)
do_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1);
- else
+ else if (errno == ENOENT) {
+ /* The file does not exist */
do_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1);
+ } else {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ goto cleanup;
+ }
if (sepol_get_disable_dontaudit(sh->sepolh) == 1) {
FILE *touch;
touch = fopen(path, "w");
@@ -1259,10 +1277,16 @@ static int semanage_direct_commit(semanage_handle_t * sh)
/* Create or remove the preserve_tunables flag file. */
path = semanage_path(SEMANAGE_TMP, SEMANAGE_PRESERVE_TUNABLES);
- if (access(path, F_OK) == 0)
+ if (stat(path, &sb) == 0)
do_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1);
- else
+ else if (errno == ENOENT) {
+ /* The file does not exist */
do_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1);
+ } else {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ goto cleanup;
+ }
+
if (sepol_get_preserve_tunables(sh->sepolh) == 1) {
FILE *touch;
touch = fopen(path, "w");
@@ -1299,40 +1323,24 @@ static int semanage_direct_commit(semanage_handle_t * sh)
* a rebuild.
*/
if (!do_rebuild) {
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_KERNEL);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
- }
-
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
- }
-
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
- }
-
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_LINKED);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
- }
-
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
- }
+ int files[] = {SEMANAGE_STORE_KERNEL,
+ SEMANAGE_STORE_FC,
+ SEMANAGE_STORE_SEUSERS,
+ SEMANAGE_LINKED,
+ SEMANAGE_SEUSERS_LINKED,
+ SEMANAGE_USERS_EXTRA_LINKED};
+
+ for (i = 0; i < (int) sizeof(files); i++) {
+ path = semanage_path(SEMANAGE_TMP, files[i]);
+ if (stat(path, &sb) != 0) {
+ if (errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ goto cleanup;
+ }
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
+ do_rebuild = 1;
+ goto rebuild;
+ }
}
}
@@ -1465,7 +1473,7 @@ rebuild:
goto cleanup;
path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
- if (access(path, F_OK) == 0) {
+ if (stat(path, &sb) == 0) {
retval = semanage_copy_file(path,
semanage_path(SEMANAGE_TMP,
SEMANAGE_STORE_SEUSERS),
@@ -1473,12 +1481,18 @@ rebuild:
if (retval < 0)
goto cleanup;
pseusers->dtable->drop_cache(pseusers->dbase);
- } else {
+ } else if (errno == ENOENT) {
+ /* The file does not exist */
pseusers->dtable->clear(sh, pseusers->dbase);
+ } else {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ goto cleanup;
}
+
+
path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
- if (access(path, F_OK) == 0) {
+ if (stat(path, &sb) == 0) {
retval = semanage_copy_file(path,
semanage_path(SEMANAGE_TMP,
SEMANAGE_USERS_EXTRA),
@@ -1486,8 +1500,12 @@ rebuild:
if (retval < 0)
goto cleanup;
pusers_extra->dtable->drop_cache(pusers_extra->dbase);
- } else {
+ } else if (errno == ENOENT) {
+ /* The file does not exist */
pusers_extra->dtable->clear(sh, pusers_extra->dbase);
+ } else {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ goto cleanup;
}
}
@@ -1817,6 +1835,7 @@ static int semanage_direct_extract(semanage_handle_t * sh,
ssize_t _data_len;
char *_data;
int compressed;
+ struct stat sb;
/* get path of module */
rc = semanage_module_get_path(
@@ -1829,8 +1848,8 @@ static int semanage_direct_extract(semanage_handle_t * sh,
goto cleanup;
}
- if (access(module_path, F_OK) != 0) {
- ERR(sh, "Module does not exist: %s", module_path);
+ if (stat(module_path, &sb) != 0) {
+ ERR(sh, "Unable to access %s: %s\n", module_path, strerror(errno));
rc = -1;
goto cleanup;
}
@@ -1859,7 +1878,12 @@ static int semanage_direct_extract(semanage_handle_t * sh,
goto cleanup;
}
- if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && access(input_file, F_OK) != 0) {
+ if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && stat(input_file, &sb) != 0) {
+ if (errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", input_file, strerror(errno));
+ goto cleanup;
+ }
+
rc = semanage_compile_module(sh, _modinfo);
if (rc < 0) {
goto cleanup;
@@ -2004,6 +2028,12 @@ static int semanage_direct_get_enabled(semanage_handle_t *sh,
}
if (stat(path, &sb) < 0) {
+ if (errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ status = -1;
+ goto cleanup;
+ }
+
*enabled = 1;
}
else {
@@ -2330,6 +2360,12 @@ static int semanage_direct_get_module_info(semanage_handle_t *sh,
/* set enabled/disabled status */
if (stat(fn, &sb) < 0) {
+ if (errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", fn, strerror(errno));
+ status = -1;
+ goto cleanup;
+ }
+
ret = semanage_module_info_set_enabled(sh, *modinfo, 1);
if (ret != 0) {
status = -1;
@@ -2738,6 +2774,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
int status = 0;
int ret = 0;
int type;
+ struct stat sb;
char path[PATH_MAX];
mode_t mask = umask(0077);
@@ -2838,7 +2875,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
goto cleanup;
}
- if (access(path, F_OK) == 0) {
+ if (stat(path, &sb) == 0) {
ret = unlink(path);
if (ret != 0) {
ERR(sh, "Error while removing cached CIL file %s: %s", path, strerror(errno));
diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c
index 4bd1d651..10f8b306 100644
--- a/libsemanage/src/semanage_store.c
+++ b/libsemanage/src/semanage_store.c
@@ -514,6 +514,7 @@ char *semanage_conf_path(void)
{
char *semanage_conf = NULL;
int len;
+ struct stat sb;
len = strlen(semanage_root()) + strlen(selinux_path()) + strlen(SEMANAGE_CONF_FILE);
semanage_conf = calloc(len + 1, sizeof(char));
@@ -522,7 +523,12 @@ char *semanage_conf_path(void)
snprintf(semanage_conf, len + 1, "%s%s%s", semanage_root(), selinux_path(),
SEMANAGE_CONF_FILE);
- if (access(semanage_conf, R_OK) != 0) {
+ if (stat(semanage_conf, &sb) != 0) {
+ if (errno != ENOENT) {
+ /* semanage_conf exists but can't be accessed */
+ return NULL;
+ }
+
snprintf(semanage_conf, len + 1, "%s%s", selinux_path(), SEMANAGE_CONF_FILE);
}
@@ -1508,8 +1514,14 @@ int semanage_split_fc(semanage_handle_t * sh)
static int sefcontext_compile(semanage_handle_t * sh, const char *path) {
int r;
+ struct stat sb;
+
+ if (stat(path, &sb) < 0) {
+ if (errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ return -1;
+ }
- if (access(path, F_OK) != 0) {
return 0;
}
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] libsemanage: replace access() checks to make setuid programs work
2018-03-06 11:58 ` [PATCH 3/3] libsemanage: replace access() checks " Vit Mojzis
@ 2018-03-07 14:59 ` Stephen Smalley
2018-03-08 20:24 ` Stephen Smalley
2018-03-09 13:14 ` Vit Mojzis
0 siblings, 2 replies; 19+ messages in thread
From: Stephen Smalley @ 2018-03-07 14:59 UTC (permalink / raw)
To: Vit Mojzis, selinux
On 03/06/2018 06:58 AM, Vit Mojzis wrote:
> access() uses real UID instead of effective UID which causes false
> negative checks in setuid programs.
> Replace access(,F_OK) (i.e. tests for file existence) by stat().
> And access(,R_OK) by fopen(,"r")
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
>
> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
> ---
> libsemanage/src/direct_api.c | 133 +++++++++++++++++++++++++--------------
> libsemanage/src/semanage_store.c | 16 ++++-
> 2 files changed, 99 insertions(+), 50 deletions(-)
>
> diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
> index 49cac14f..4e2af810 100644
> --- a/libsemanage/src/direct_api.c
> +++ b/libsemanage/src/direct_api.c
> @@ -140,6 +140,7 @@ int semanage_direct_is_managed(semanage_handle_t * sh)
> int semanage_direct_connect(semanage_handle_t * sh)
> {
> const char *path;
> + struct stat sb;
>
> if (semanage_check_init(sh, sh->conf->store_root_path))
> goto err;
> @@ -302,10 +303,16 @@ int semanage_direct_connect(semanage_handle_t * sh)
>
> /* set the disable dontaudit value */
> path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_DISABLE_DONTAUDIT);
> - if (access(path, F_OK) == 0)
> +
> + if (stat(path, &sb) == 0)
> sepol_set_disable_dontaudit(sh->sepolh, 1);
> - else
> + else if (errno == ENOENT) {
> + /* The file does not exist */
> sepol_set_disable_dontaudit(sh->sepolh, 0);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + goto err;
> + }
>
> return STATUS_SUCCESS;
>
> @@ -1139,6 +1146,7 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
> int status = 0;
> int i;
> char cil_path[PATH_MAX];
> + struct stat sb;
>
> assert(sh);
> assert(modinfos);
> @@ -1155,9 +1163,13 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
> }
>
> if (semanage_get_ignore_module_cache(sh) == 0 &&
> - access(cil_path, F_OK) == 0) {
> + (status = stat(cil_path, &sb)) == 0) {
> continue;
> }
> + if (status != 0 && errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", cil_path, strerror(errno));
> + goto cleanup; //an error in the "stat" call
> + }
>
> status = semanage_compile_module(sh, &modinfos[i]);
> if (status < 0) {
> @@ -1196,6 +1208,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
> struct cil_db *cildb = NULL;
> semanage_module_info_t *modinfos = NULL;
> mode_t mask = umask(0077);
> + struct stat sb;
>
> int do_rebuild, do_write_kernel, do_install;
> int fcontexts_modified, ports_modified, seusers_modified,
> @@ -1234,10 +1247,15 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>
> /* Create or remove the disable_dontaudit flag file. */
> path = semanage_path(SEMANAGE_TMP, SEMANAGE_DISABLE_DONTAUDIT);
> - if (access(path, F_OK) == 0)
> + if (stat(path, &sb) == 0)
> do_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1);
> - else
> + else if (errno == ENOENT) {
> + /* The file does not exist */
> do_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
We need to ensure that retval is set in all of these cases. Safest to always set it before goto cleanup,
either explicitly to -1 or to return value of stat() call above.
> + goto cleanup;
> + }
> if (sepol_get_disable_dontaudit(sh->sepolh) == 1) {
> FILE *touch;
> touch = fopen(path, "w");
> @@ -1259,10 +1277,16 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>
> /* Create or remove the preserve_tunables flag file. */
> path = semanage_path(SEMANAGE_TMP, SEMANAGE_PRESERVE_TUNABLES);
> - if (access(path, F_OK) == 0)
> + if (stat(path, &sb) == 0)
> do_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1);
> - else
> + else if (errno == ENOENT) {
> + /* The file does not exist */
> do_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + goto cleanup;
> + }
Ditto
> +
> if (sepol_get_preserve_tunables(sh->sepolh) == 1) {
> FILE *touch;
> touch = fopen(path, "w");
> @@ -1299,40 +1323,24 @@ static int semanage_direct_commit(semanage_handle_t * sh)
> * a rebuild.
> */
> if (!do_rebuild) {
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_KERNEL);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> -
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> -
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> -
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_LINKED);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> -
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> + int files[] = {SEMANAGE_STORE_KERNEL,
> + SEMANAGE_STORE_FC,
> + SEMANAGE_STORE_SEUSERS,
> + SEMANAGE_LINKED,
> + SEMANAGE_SEUSERS_LINKED,
> + SEMANAGE_USERS_EXTRA_LINKED};
> +
> + for (i = 0; i < (int) sizeof(files); i++) {
> + path = semanage_path(SEMANAGE_TMP, files[i]);
> + if (stat(path, &sb) != 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + goto cleanup;
Ditto
> + }
>
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> + do_rebuild = 1;
> + goto rebuild;
> + }
> }
> }
>
> @@ -1465,7 +1473,7 @@ rebuild:
> goto cleanup;
>
> path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
> - if (access(path, F_OK) == 0) {
> + if (stat(path, &sb) == 0) {
> retval = semanage_copy_file(path,
> semanage_path(SEMANAGE_TMP,
> SEMANAGE_STORE_SEUSERS),
> @@ -1473,12 +1481,18 @@ rebuild:
> if (retval < 0)
> goto cleanup;
> pseusers->dtable->drop_cache(pseusers->dbase);
> - } else {
> + } else if (errno == ENOENT) {
> + /* The file does not exist */
> pseusers->dtable->clear(sh, pseusers->dbase);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + goto cleanup;
Ditto
> }
>
> +
> +
> path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
> - if (access(path, F_OK) == 0) {
> + if (stat(path, &sb) == 0) {
> retval = semanage_copy_file(path,
> semanage_path(SEMANAGE_TMP,
> SEMANAGE_USERS_EXTRA),
> @@ -1486,8 +1500,12 @@ rebuild:
> if (retval < 0)
> goto cleanup;
> pusers_extra->dtable->drop_cache(pusers_extra->dbase);
> - } else {
> + } else if (errno == ENOENT) {
> + /* The file does not exist */
> pusers_extra->dtable->clear(sh, pusers_extra->dbase);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + goto cleanup;
Ditto
> }
> }
>
> @@ -1817,6 +1835,7 @@ static int semanage_direct_extract(semanage_handle_t * sh,
> ssize_t _data_len;
> char *_data;
> int compressed;
> + struct stat sb;
>
> /* get path of module */
> rc = semanage_module_get_path(
> @@ -1829,8 +1848,8 @@ static int semanage_direct_extract(semanage_handle_t * sh,
> goto cleanup;
> }
>
> - if (access(module_path, F_OK) != 0) {
> - ERR(sh, "Module does not exist: %s", module_path);
> + if (stat(module_path, &sb) != 0) {
> + ERR(sh, "Unable to access %s: %s\n", module_path, strerror(errno));
> rc = -1;
> goto cleanup;
> }
> @@ -1859,7 +1878,12 @@ static int semanage_direct_extract(semanage_handle_t * sh,
> goto cleanup;
> }
>
> - if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && access(input_file, F_OK) != 0) {
> + if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && stat(input_file, &sb) != 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", input_file, strerror(errno));
Ditto
> + goto cleanup;
> +
> +
> rc = semanage_compile_module(sh, _modinfo);
> if (rc < 0) {
> goto cleanup;
> @@ -2004,6 +2028,12 @@ static int semanage_direct_get_enabled(semanage_handle_t *sh,
> }
>
> if (stat(path, &sb) < 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + status = -1;
> + goto cleanup;
> + }
> +
> *enabled = 1;
> }
> else {
> @@ -2330,6 +2360,12 @@ static int semanage_direct_get_module_info(semanage_handle_t *sh,
>
> /* set enabled/disabled status */
> if (stat(fn, &sb) < 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", fn, strerror(errno));
> + status = -1;
> + goto cleanup;
> + }
> +
> ret = semanage_module_info_set_enabled(sh, *modinfo, 1);
> if (ret != 0) {
> status = -1;
> @@ -2738,6 +2774,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
> int status = 0;
> int ret = 0;
> int type;
> + struct stat sb;
>
> char path[PATH_MAX];
> mode_t mask = umask(0077);
> @@ -2838,7 +2875,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
> goto cleanup;
> }
>
> - if (access(path, F_OK) == 0) {
> + if (stat(path, &sb) == 0) {
> ret = unlink(path);
> if (ret != 0) {
> ERR(sh, "Error while removing cached CIL file %s: %s", path, strerror(errno));
> diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c
> index 4bd1d651..10f8b306 100644
> --- a/libsemanage/src/semanage_store.c
> +++ b/libsemanage/src/semanage_store.c
> @@ -514,6 +514,7 @@ char *semanage_conf_path(void)
> {
> char *semanage_conf = NULL;
> int len;
> + struct stat sb;
>
> len = strlen(semanage_root()) + strlen(selinux_path()) + strlen(SEMANAGE_CONF_FILE);
> semanage_conf = calloc(len + 1, sizeof(char));
> @@ -522,7 +523,12 @@ char *semanage_conf_path(void)
> snprintf(semanage_conf, len + 1, "%s%s%s", semanage_root(), selinux_path(),
> SEMANAGE_CONF_FILE);
>
> - if (access(semanage_conf, R_OK) != 0) {
> + if (stat(semanage_conf, &sb) != 0) {
> + if (errno != ENOENT) {
> + /* semanage_conf exists but can't be accessed */
> + return NULL;
Not sure this is what we want. I think we'd just return semanage_conf here, and then let the caller fail when it tries to access it, so that we get a useful error message with the pathname.
> + }
> +
> snprintf(semanage_conf, len + 1, "%s%s", selinux_path(), SEMANAGE_CONF_FILE);
> }
>
> @@ -1508,8 +1514,14 @@ int semanage_split_fc(semanage_handle_t * sh)
> static int sefcontext_compile(semanage_handle_t * sh, const char *path) {
>
> int r;
> + struct stat sb;
> +
> + if (stat(path, &sb) < 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + return -1;
> + }
>
> - if (access(path, F_OK) != 0) {
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] libsemanage: remove access() check to make setuid programs work
2018-03-06 11:58 ` [PATCH 2/3] " Vit Mojzis
@ 2018-03-08 19:54 ` Stephen Smalley
0 siblings, 0 replies; 19+ messages in thread
From: Stephen Smalley @ 2018-03-08 19:54 UTC (permalink / raw)
To: Vit Mojzis, selinux
On 03/06/2018 06:58 AM, Vit Mojzis wrote:
> F_OK access checks only work properly as long as all directories along
> the path are accessible to real user running the program.
> Replace F_OK access checks by testing return value of open, write, etc.
Applied patches 1 and 2 (not 3 as per my comments).
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
>
> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
> ---
> libsemanage/src/direct_api.c | 47 ++++++++++++++++++++++----------------------
> 1 file changed, 23 insertions(+), 24 deletions(-)
>
> diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
> index b7899d68..49cac14f 100644
> --- a/libsemanage/src/direct_api.c
> +++ b/libsemanage/src/direct_api.c
> @@ -1171,6 +1171,14 @@ cleanup:
> return status;
> }
>
> +/* Copies a file from src to dst. If dst already exists then
> + * overwrite it. If source doesn't exist then return success.
> + * Returns 0 on success, -1 on error. */
> +static int copy_file_if_exists(const char *src, const char *dst, mode_t mode){
> + int rc = semanage_copy_file(src, dst, mode);
> + return (rc < 0 && errno != ENOENT) ? rc : 0;
> +}
> +
> /********************* direct API functions ********************/
>
> /* Commits all changes in sandbox to the actual kernel policy.
> @@ -1563,34 +1571,25 @@ rebuild:
> goto cleanup;
> }
>
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC_LOCAL);
> - if (access(path, F_OK) == 0) {
> - retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC_LOCAL),
> - semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC_LOCAL),
> - sh->conf->file_mode);
> - if (retval < 0) {
> - goto cleanup;
> - }
> + retval = copy_file_if_exists(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC_LOCAL),
> + semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC_LOCAL),
> + sh->conf->file_mode);
> + if (retval < 0) {
> + goto cleanup;
> }
>
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
> - if (access(path, F_OK) == 0) {
> - retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC),
> - semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC),
> - sh->conf->file_mode);
> - if (retval < 0) {
> - goto cleanup;
> - }
> + retval = copy_file_if_exists(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC),
> + semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_FC),
> + sh->conf->file_mode);
> + if (retval < 0) {
> + goto cleanup;
> }
>
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
> - if (access(path, F_OK) == 0) {
> - retval = semanage_copy_file(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS),
> - semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_SEUSERS),
> - sh->conf->file_mode);
> - if (retval < 0) {
> - goto cleanup;
> - }
> + retval = copy_file_if_exists(semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS),
> + semanage_final_path(SEMANAGE_FINAL_TMP, SEMANAGE_SEUSERS),
> + sh->conf->file_mode);
> + if (retval < 0) {
> + goto cleanup;
> }
>
> /* run genhomedircon if its enabled, this should be the last operation
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] libsemanage: replace access() checks to make setuid programs work
2018-03-07 14:59 ` Stephen Smalley
@ 2018-03-08 20:24 ` Stephen Smalley
2018-03-09 13:14 ` Vit Mojzis
1 sibling, 0 replies; 19+ messages in thread
From: Stephen Smalley @ 2018-03-08 20:24 UTC (permalink / raw)
To: Vit Mojzis, selinux
On 03/07/2018 09:59 AM, Stephen Smalley wrote:
> On 03/06/2018 06:58 AM, Vit Mojzis wrote:
>> access() uses real UID instead of effective UID which causes false
>> negative checks in setuid programs.
>> Replace access(,F_OK) (i.e. tests for file existence) by stat().
>> And access(,R_OK) by fopen(,"r")
Also, in addition to fixing the code, please update the patch description since the last line is no longer accurate.
>>
>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
>>
>> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
>> ---
>> libsemanage/src/direct_api.c | 133 +++++++++++++++++++++++++--------------
>> libsemanage/src/semanage_store.c | 16 ++++-
>> 2 files changed, 99 insertions(+), 50 deletions(-)
>>
>> diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
>> index 49cac14f..4e2af810 100644
>> --- a/libsemanage/src/direct_api.c
>> +++ b/libsemanage/src/direct_api.c
>> @@ -140,6 +140,7 @@ int semanage_direct_is_managed(semanage_handle_t * sh)
>> int semanage_direct_connect(semanage_handle_t * sh)
>> {
>> const char *path;
>> + struct stat sb;
>>
>> if (semanage_check_init(sh, sh->conf->store_root_path))
>> goto err;
>> @@ -302,10 +303,16 @@ int semanage_direct_connect(semanage_handle_t * sh)
>>
>> /* set the disable dontaudit value */
>> path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_DISABLE_DONTAUDIT);
>> - if (access(path, F_OK) == 0)
>> +
>> + if (stat(path, &sb) == 0)
>> sepol_set_disable_dontaudit(sh->sepolh, 1);
>> - else
>> + else if (errno == ENOENT) {
>> + /* The file does not exist */
>> sepol_set_disable_dontaudit(sh->sepolh, 0);
>> + } else {
>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>> + goto err;
>> + }
>>
>> return STATUS_SUCCESS;
>>
>> @@ -1139,6 +1146,7 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
>> int status = 0;
>> int i;
>> char cil_path[PATH_MAX];
>> + struct stat sb;
>>
>> assert(sh);
>> assert(modinfos);
>> @@ -1155,9 +1163,13 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
>> }
>>
>> if (semanage_get_ignore_module_cache(sh) == 0 &&
>> - access(cil_path, F_OK) == 0) {
>> + (status = stat(cil_path, &sb)) == 0) {
>> continue;
>> }
>> + if (status != 0 && errno != ENOENT) {
>> + ERR(sh, "Unable to access %s: %s\n", cil_path, strerror(errno));
>> + goto cleanup; //an error in the "stat" call
>> + }
>>
>> status = semanage_compile_module(sh, &modinfos[i]);
>> if (status < 0) {
>> @@ -1196,6 +1208,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>> struct cil_db *cildb = NULL;
>> semanage_module_info_t *modinfos = NULL;
>> mode_t mask = umask(0077);
>> + struct stat sb;
>>
>> int do_rebuild, do_write_kernel, do_install;
>> int fcontexts_modified, ports_modified, seusers_modified,
>> @@ -1234,10 +1247,15 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>>
>> /* Create or remove the disable_dontaudit flag file. */
>> path = semanage_path(SEMANAGE_TMP, SEMANAGE_DISABLE_DONTAUDIT);
>> - if (access(path, F_OK) == 0)
>> + if (stat(path, &sb) == 0)
>> do_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1);
>> - else
>> + else if (errno == ENOENT) {
>> + /* The file does not exist */
>> do_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1);
>> + } else {
>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>
> We need to ensure that retval is set in all of these cases. Safest to always set it before goto cleanup,
> either explicitly to -1 or to return value of stat() call above.
>
>> + goto cleanup;
>> + }
>> if (sepol_get_disable_dontaudit(sh->sepolh) == 1) {
>> FILE *touch;
>> touch = fopen(path, "w");
>> @@ -1259,10 +1277,16 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>>
>> /* Create or remove the preserve_tunables flag file. */
>> path = semanage_path(SEMANAGE_TMP, SEMANAGE_PRESERVE_TUNABLES);
>> - if (access(path, F_OK) == 0)
>> + if (stat(path, &sb) == 0)
>> do_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1);
>> - else
>> + else if (errno == ENOENT) {
>> + /* The file does not exist */
>> do_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1);
>> + } else {
>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>> + goto cleanup;
>> + }
>
> Ditto
>
>> +
>> if (sepol_get_preserve_tunables(sh->sepolh) == 1) {
>> FILE *touch;
>> touch = fopen(path, "w");
>> @@ -1299,40 +1323,24 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>> * a rebuild.
>> */
>> if (!do_rebuild) {
>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_KERNEL);
>> - if (access(path, F_OK) != 0) {
>> - do_rebuild = 1;
>> - goto rebuild;
>> - }
>> -
>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
>> - if (access(path, F_OK) != 0) {
>> - do_rebuild = 1;
>> - goto rebuild;
>> - }
>> -
>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
>> - if (access(path, F_OK) != 0) {
>> - do_rebuild = 1;
>> - goto rebuild;
>> - }
>> -
>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_LINKED);
>> - if (access(path, F_OK) != 0) {
>> - do_rebuild = 1;
>> - goto rebuild;
>> - }
>> -
>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
>> - if (access(path, F_OK) != 0) {
>> - do_rebuild = 1;
>> - goto rebuild;
>> - }
>> + int files[] = {SEMANAGE_STORE_KERNEL,
>> + SEMANAGE_STORE_FC,
>> + SEMANAGE_STORE_SEUSERS,
>> + SEMANAGE_LINKED,
>> + SEMANAGE_SEUSERS_LINKED,
>> + SEMANAGE_USERS_EXTRA_LINKED};
>> +
>> + for (i = 0; i < (int) sizeof(files); i++) {
>> + path = semanage_path(SEMANAGE_TMP, files[i]);
>> + if (stat(path, &sb) != 0) {
>> + if (errno != ENOENT) {
>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>> + goto cleanup;
>
> Ditto
>
>> + }
>>
>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
>> - if (access(path, F_OK) != 0) {
>> - do_rebuild = 1;
>> - goto rebuild;
>> + do_rebuild = 1;
>> + goto rebuild;
>> + }
>> }
>> }
>>
>> @@ -1465,7 +1473,7 @@ rebuild:
>> goto cleanup;
>>
>> path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
>> - if (access(path, F_OK) == 0) {
>> + if (stat(path, &sb) == 0) {
>> retval = semanage_copy_file(path,
>> semanage_path(SEMANAGE_TMP,
>> SEMANAGE_STORE_SEUSERS),
>> @@ -1473,12 +1481,18 @@ rebuild:
>> if (retval < 0)
>> goto cleanup;
>> pseusers->dtable->drop_cache(pseusers->dbase);
>> - } else {
>> + } else if (errno == ENOENT) {
>> + /* The file does not exist */
>> pseusers->dtable->clear(sh, pseusers->dbase);
>> + } else {
>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>> + goto cleanup;
>
> Ditto
>
>> }
>>
>> +
>> +
>> path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
>> - if (access(path, F_OK) == 0) {
>> + if (stat(path, &sb) == 0) {
>> retval = semanage_copy_file(path,
>> semanage_path(SEMANAGE_TMP,
>> SEMANAGE_USERS_EXTRA),
>> @@ -1486,8 +1500,12 @@ rebuild:
>> if (retval < 0)
>> goto cleanup;
>> pusers_extra->dtable->drop_cache(pusers_extra->dbase);
>> - } else {
>> + } else if (errno == ENOENT) {
>> + /* The file does not exist */
>> pusers_extra->dtable->clear(sh, pusers_extra->dbase);
>> + } else {
>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>> + goto cleanup;
>
> Ditto
>
>> }
>> }
>>
>> @@ -1817,6 +1835,7 @@ static int semanage_direct_extract(semanage_handle_t * sh,
>> ssize_t _data_len;
>> char *_data;
>> int compressed;
>> + struct stat sb;
>>
>> /* get path of module */
>> rc = semanage_module_get_path(
>> @@ -1829,8 +1848,8 @@ static int semanage_direct_extract(semanage_handle_t * sh,
>> goto cleanup;
>> }
>>
>> - if (access(module_path, F_OK) != 0) {
>> - ERR(sh, "Module does not exist: %s", module_path);
>> + if (stat(module_path, &sb) != 0) {
>> + ERR(sh, "Unable to access %s: %s\n", module_path, strerror(errno));
>> rc = -1;
>> goto cleanup;
>> }
>> @@ -1859,7 +1878,12 @@ static int semanage_direct_extract(semanage_handle_t * sh,
>> goto cleanup;
>> }
>>
>> - if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && access(input_file, F_OK) != 0) {
>> + if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && stat(input_file, &sb) != 0) {
>> + if (errno != ENOENT) {
>> + ERR(sh, "Unable to access %s: %s\n", input_file, strerror(errno));
>
> Ditto
>
>> + goto cleanup;
>> +
>> +
>> rc = semanage_compile_module(sh, _modinfo);
>> if (rc < 0) {
>> goto cleanup;
>> @@ -2004,6 +2028,12 @@ static int semanage_direct_get_enabled(semanage_handle_t *sh,
>> }
>>
>> if (stat(path, &sb) < 0) {
>> + if (errno != ENOENT) {
>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>> + status = -1;
>> + goto cleanup;
>> + }
>> +
>> *enabled = 1;
>> }
>> else {
>> @@ -2330,6 +2360,12 @@ static int semanage_direct_get_module_info(semanage_handle_t *sh,
>>
>> /* set enabled/disabled status */
>> if (stat(fn, &sb) < 0) {
>> + if (errno != ENOENT) {
>> + ERR(sh, "Unable to access %s: %s\n", fn, strerror(errno));
>> + status = -1;
>> + goto cleanup;
>> + }
>> +
>> ret = semanage_module_info_set_enabled(sh, *modinfo, 1);
>> if (ret != 0) {
>> status = -1;
>> @@ -2738,6 +2774,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
>> int status = 0;
>> int ret = 0;
>> int type;
>> + struct stat sb;
>>
>> char path[PATH_MAX];
>> mode_t mask = umask(0077);
>> @@ -2838,7 +2875,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
>> goto cleanup;
>> }
>>
>> - if (access(path, F_OK) == 0) {
>> + if (stat(path, &sb) == 0) {
>> ret = unlink(path);
>> if (ret != 0) {
>> ERR(sh, "Error while removing cached CIL file %s: %s", path, strerror(errno));
>> diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c
>> index 4bd1d651..10f8b306 100644
>> --- a/libsemanage/src/semanage_store.c
>> +++ b/libsemanage/src/semanage_store.c
>> @@ -514,6 +514,7 @@ char *semanage_conf_path(void)
>> {
>> char *semanage_conf = NULL;
>> int len;
>> + struct stat sb;
>>
>> len = strlen(semanage_root()) + strlen(selinux_path()) + strlen(SEMANAGE_CONF_FILE);
>> semanage_conf = calloc(len + 1, sizeof(char));
>> @@ -522,7 +523,12 @@ char *semanage_conf_path(void)
>> snprintf(semanage_conf, len + 1, "%s%s%s", semanage_root(), selinux_path(),
>> SEMANAGE_CONF_FILE);
>>
>> - if (access(semanage_conf, R_OK) != 0) {
>> + if (stat(semanage_conf, &sb) != 0) {
>> + if (errno != ENOENT) {
>> + /* semanage_conf exists but can't be accessed */
>> + return NULL;
>
> Not sure this is what we want. I think we'd just return semanage_conf here, and then let the caller fail when it tries to access it, so that we get a useful error message with the pathname.
>
>> + }
>> +
>> snprintf(semanage_conf, len + 1, "%s%s", selinux_path(), SEMANAGE_CONF_FILE);
>> }
>>
>> @@ -1508,8 +1514,14 @@ int semanage_split_fc(semanage_handle_t * sh)
>> static int sefcontext_compile(semanage_handle_t * sh, const char *path) {
>>
>> int r;
>> + struct stat sb;
>> +
>> + if (stat(path, &sb) < 0) {
>> + if (errno != ENOENT) {
>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>> + return -1;
>> + }
>>
>> - if (access(path, F_OK) != 0) {
>> return 0;
>> }
>>
>>
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] libsemanage: replace access() checks to make setuid programs work
2018-03-07 14:59 ` Stephen Smalley
2018-03-08 20:24 ` Stephen Smalley
@ 2018-03-09 13:14 ` Vit Mojzis
2018-03-09 13:28 ` Stephen Smalley
1 sibling, 1 reply; 19+ messages in thread
From: Vit Mojzis @ 2018-03-09 13:14 UTC (permalink / raw)
To: Stephen Smalley, selinux
On 7.3.2018 15:59, Stephen Smalley wrote:
> On 03/06/2018 06:58 AM, Vit Mojzis wrote:
>> access() uses real UID instead of effective UID which causes false
>> negative checks in setuid programs.
>> Replace access(,F_OK) (i.e. tests for file existence) by stat().
>> And access(,R_OK) by fopen(,"r")
>>
>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
>>
>> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
>> ---
>> libsemanage/src/direct_api.c | 133 +++++++++++++++++++++++++--------------
>> libsemanage/src/semanage_store.c | 16 ++++-
>> 2 files changed, 99 insertions(+), 50 deletions(-)
>>
>> diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
>> index 49cac14f..4e2af810 100644
>> --- a/libsemanage/src/direct_api.c
>> +++ b/libsemanage/src/direct_api.c
>> @@ -140,6 +140,7 @@ int semanage_direct_is_managed(semanage_handle_t * sh)
>> int semanage_direct_connect(semanage_handle_t * sh)
>> {
>> const char *path;
>> + struct stat sb;
>>
>> if (semanage_check_init(sh, sh->conf->store_root_path))
>> goto err;
>> @@ -302,10 +303,16 @@ int semanage_direct_connect(semanage_handle_t * sh)
>>
>> /* set the disable dontaudit value */
>> path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_DISABLE_DONTAUDIT);
>> - if (access(path, F_OK) == 0)
>> +
>> + if (stat(path, &sb) == 0)
>> sepol_set_disable_dontaudit(sh->sepolh, 1);
>> - else
>> + else if (errno == ENOENT) {
>> + /* The file does not exist */
>> sepol_set_disable_dontaudit(sh->sepolh, 0);
>> + } else {
>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>> + goto err;
>> + }
>>
>> return STATUS_SUCCESS;
>>
>> @@ -1139,6 +1146,7 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
>> int status = 0;
>> int i;
>> char cil_path[PATH_MAX];
>> + struct stat sb;
>>
>> assert(sh);
>> assert(modinfos);
>> @@ -1155,9 +1163,13 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
>> }
>>
>> if (semanage_get_ignore_module_cache(sh) == 0 &&
>> - access(cil_path, F_OK) == 0) {
>> + (status = stat(cil_path, &sb)) == 0) {
>> continue;
>> }
>> + if (status != 0 && errno != ENOENT) {
>> + ERR(sh, "Unable to access %s: %s\n", cil_path, strerror(errno));
>> + goto cleanup; //an error in the "stat" call
>> + }
>>
>> status = semanage_compile_module(sh, &modinfos[i]);
>> if (status < 0) {
>> @@ -1196,6 +1208,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>> struct cil_db *cildb = NULL;
>> semanage_module_info_t *modinfos = NULL;
>> mode_t mask = umask(0077);
>> + struct stat sb;
>>
>> int do_rebuild, do_write_kernel, do_install;
>> int fcontexts_modified, ports_modified, seusers_modified,
>> @@ -1234,10 +1247,15 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>>
>> /* Create or remove the disable_dontaudit flag file. */
>> path = semanage_path(SEMANAGE_TMP, SEMANAGE_DISABLE_DONTAUDIT);
>> - if (access(path, F_OK) == 0)
>> + if (stat(path, &sb) == 0)
>> do_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1);
>> - else
>> + else if (errno == ENOENT) {
>> + /* The file does not exist */
>> do_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1);
>> + } else {
>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> We need to ensure that retval is set in all of these cases. Safest to always set it before goto cleanup,
> either explicitly to -1 or to return value of stat() call above.
Thank you.
The retval is actually initialized to -1 (and stat() returns -1 for any
error) so it's not necessary to set it here. It will be necessary a
little bit down the road after the first assignment to retval, you're right.
Should I set it to -1 here anyway to make it less prone to breaking due
to future changes in the code (what about the other ERR calls in this
section of the code)?
>> + goto cleanup;
>> + }
>> if (sepol_get_disable_dontaudit(sh->sepolh) == 1) {
>> FILE *touch;
>> touch = fopen(path, "w");
>> @@ -1259,10 +1277,16 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>>
>> /* Create or remove the preserve_tunables flag file. */
>> path = semanage_path(SEMANAGE_TMP, SEMANAGE_PRESERVE_TUNABLES);
>> - if (access(path, F_OK) == 0)
>> + if (stat(path, &sb) == 0)
>> do_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1);
>> - else
>> + else if (errno == ENOENT) {
>> + /* The file does not exist */
>> do_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1);
>> + } else {
>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>> + goto cleanup;
>> + }
> Ditto
>
>> +
>> if (sepol_get_preserve_tunables(sh->sepolh) == 1) {
>> FILE *touch;
>> touch = fopen(path, "w");
>> @@ -1299,40 +1323,24 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>> * a rebuild.
>> */
>> if (!do_rebuild) {
>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_KERNEL);
>> - if (access(path, F_OK) != 0) {
>> - do_rebuild = 1;
>> - goto rebuild;
>> - }
>> -
>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
>> - if (access(path, F_OK) != 0) {
>> - do_rebuild = 1;
>> - goto rebuild;
>> - }
>> -
>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
>> - if (access(path, F_OK) != 0) {
>> - do_rebuild = 1;
>> - goto rebuild;
>> - }
>> -
>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_LINKED);
>> - if (access(path, F_OK) != 0) {
>> - do_rebuild = 1;
>> - goto rebuild;
>> - }
>> -
>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
>> - if (access(path, F_OK) != 0) {
>> - do_rebuild = 1;
>> - goto rebuild;
>> - }
>> + int files[] = {SEMANAGE_STORE_KERNEL,
>> + SEMANAGE_STORE_FC,
>> + SEMANAGE_STORE_SEUSERS,
>> + SEMANAGE_LINKED,
>> + SEMANAGE_SEUSERS_LINKED,
>> + SEMANAGE_USERS_EXTRA_LINKED};
>> +
>> + for (i = 0; i < (int) sizeof(files); i++) {
>> + path = semanage_path(SEMANAGE_TMP, files[i]);
>> + if (stat(path, &sb) != 0) {
>> + if (errno != ENOENT) {
>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>> + goto cleanup;
> Ditto
>
>> + }
>>
>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
>> - if (access(path, F_OK) != 0) {
>> - do_rebuild = 1;
>> - goto rebuild;
>> + do_rebuild = 1;
>> + goto rebuild;
>> + }
>> }
>> }
>>
>> @@ -1465,7 +1473,7 @@ rebuild:
>> goto cleanup;
>>
>> path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
>> - if (access(path, F_OK) == 0) {
>> + if (stat(path, &sb) == 0) {
>> retval = semanage_copy_file(path,
>> semanage_path(SEMANAGE_TMP,
>> SEMANAGE_STORE_SEUSERS),
>> @@ -1473,12 +1481,18 @@ rebuild:
>> if (retval < 0)
>> goto cleanup;
>> pseusers->dtable->drop_cache(pseusers->dbase);
>> - } else {
>> + } else if (errno == ENOENT) {
>> + /* The file does not exist */
>> pseusers->dtable->clear(sh, pseusers->dbase);
>> + } else {
>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>> + goto cleanup;
> Ditto
>
>> }
>>
>> +
>> +
>> path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
>> - if (access(path, F_OK) == 0) {
>> + if (stat(path, &sb) == 0) {
>> retval = semanage_copy_file(path,
>> semanage_path(SEMANAGE_TMP,
>> SEMANAGE_USERS_EXTRA),
>> @@ -1486,8 +1500,12 @@ rebuild:
>> if (retval < 0)
>> goto cleanup;
>> pusers_extra->dtable->drop_cache(pusers_extra->dbase);
>> - } else {
>> + } else if (errno == ENOENT) {
>> + /* The file does not exist */
>> pusers_extra->dtable->clear(sh, pusers_extra->dbase);
>> + } else {
>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>> + goto cleanup;
> Ditto
>
>> }
>> }
>>
>> @@ -1817,6 +1835,7 @@ static int semanage_direct_extract(semanage_handle_t * sh,
>> ssize_t _data_len;
>> char *_data;
>> int compressed;
>> + struct stat sb;
>>
>> /* get path of module */
>> rc = semanage_module_get_path(
>> @@ -1829,8 +1848,8 @@ static int semanage_direct_extract(semanage_handle_t * sh,
>> goto cleanup;
>> }
>>
>> - if (access(module_path, F_OK) != 0) {
>> - ERR(sh, "Module does not exist: %s", module_path);
>> + if (stat(module_path, &sb) != 0) {
>> + ERR(sh, "Unable to access %s: %s\n", module_path, strerror(errno));
>> rc = -1;
>> goto cleanup;
>> }
>> @@ -1859,7 +1878,12 @@ static int semanage_direct_extract(semanage_handle_t * sh,
>> goto cleanup;
>> }
>>
>> - if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && access(input_file, F_OK) != 0) {
>> + if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && stat(input_file, &sb) != 0) {
>> + if (errno != ENOENT) {
>> + ERR(sh, "Unable to access %s: %s\n", input_file, strerror(errno));
> Ditto
>
>> + goto cleanup;
>> +
>> +
>> rc = semanage_compile_module(sh, _modinfo);
>> if (rc < 0) {
>> goto cleanup;
>> @@ -2004,6 +2028,12 @@ static int semanage_direct_get_enabled(semanage_handle_t *sh,
>> }
>>
>> if (stat(path, &sb) < 0) {
>> + if (errno != ENOENT) {
>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>> + status = -1;
>> + goto cleanup;
>> + }
>> +
>> *enabled = 1;
>> }
>> else {
>> @@ -2330,6 +2360,12 @@ static int semanage_direct_get_module_info(semanage_handle_t *sh,
>>
>> /* set enabled/disabled status */
>> if (stat(fn, &sb) < 0) {
>> + if (errno != ENOENT) {
>> + ERR(sh, "Unable to access %s: %s\n", fn, strerror(errno));
>> + status = -1;
>> + goto cleanup;
>> + }
>> +
>> ret = semanage_module_info_set_enabled(sh, *modinfo, 1);
>> if (ret != 0) {
>> status = -1;
>> @@ -2738,6 +2774,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
>> int status = 0;
>> int ret = 0;
>> int type;
>> + struct stat sb;
>>
>> char path[PATH_MAX];
>> mode_t mask = umask(0077);
>> @@ -2838,7 +2875,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
>> goto cleanup;
>> }
>>
>> - if (access(path, F_OK) == 0) {
>> + if (stat(path, &sb) == 0) {
>> ret = unlink(path);
>> if (ret != 0) {
>> ERR(sh, "Error while removing cached CIL file %s: %s", path, strerror(errno));
>> diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c
>> index 4bd1d651..10f8b306 100644
>> --- a/libsemanage/src/semanage_store.c
>> +++ b/libsemanage/src/semanage_store.c
>> @@ -514,6 +514,7 @@ char *semanage_conf_path(void)
>> {
>> char *semanage_conf = NULL;
>> int len;
>> + struct stat sb;
>>
>> len = strlen(semanage_root()) + strlen(selinux_path()) + strlen(SEMANAGE_CONF_FILE);
>> semanage_conf = calloc(len + 1, sizeof(char));
>> @@ -522,7 +523,12 @@ char *semanage_conf_path(void)
>> snprintf(semanage_conf, len + 1, "%s%s%s", semanage_root(), selinux_path(),
>> SEMANAGE_CONF_FILE);
>>
>> - if (access(semanage_conf, R_OK) != 0) {
>> + if (stat(semanage_conf, &sb) != 0) {
>> + if (errno != ENOENT) {
>> + /* semanage_conf exists but can't be accessed */
>> + return NULL;
> Not sure this is what we want. I think we'd just return semanage_conf here, and then let the caller fail when it tries to access it, so that we get a useful error message with the pathname.
>
>> + }
>> +
>> snprintf(semanage_conf, len + 1, "%s%s", selinux_path(), SEMANAGE_CONF_FILE);
>> }
>>
>> @@ -1508,8 +1514,14 @@ int semanage_split_fc(semanage_handle_t * sh)
>> static int sefcontext_compile(semanage_handle_t * sh, const char *path) {
>>
>> int r;
>> + struct stat sb;
>> +
>> + if (stat(path, &sb) < 0) {
>> + if (errno != ENOENT) {
>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>> + return -1;
>> + }
>>
>> - if (access(path, F_OK) != 0) {
>> return 0;
>> }
>>
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] libsemanage: replace access() checks to make setuid programs work
2018-03-09 13:14 ` Vit Mojzis
@ 2018-03-09 13:28 ` Stephen Smalley
2018-03-09 13:36 ` Stephen Smalley
0 siblings, 1 reply; 19+ messages in thread
From: Stephen Smalley @ 2018-03-09 13:28 UTC (permalink / raw)
To: Vit Mojzis, selinux
On 03/09/2018 08:14 AM, Vit Mojzis wrote:
>
>
> On 7.3.2018 15:59, Stephen Smalley wrote:
>> On 03/06/2018 06:58 AM, Vit Mojzis wrote:
>>> access() uses real UID instead of effective UID which causes false
>>> negative checks in setuid programs.
>>> Replace access(,F_OK) (i.e. tests for file existence) by stat().
>>> And access(,R_OK) by fopen(,"r")
>>>
>>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
>>>
>>> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
>>> ---
>>> libsemanage/src/direct_api.c | 133 +++++++++++++++++++++++++--------------
>>> libsemanage/src/semanage_store.c | 16 ++++-
>>> 2 files changed, 99 insertions(+), 50 deletions(-)
>>>
>>> diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
>>> index 49cac14f..4e2af810 100644
>>> --- a/libsemanage/src/direct_api.c
>>> +++ b/libsemanage/src/direct_api.c
>>> @@ -140,6 +140,7 @@ int semanage_direct_is_managed(semanage_handle_t * sh)
>>> int semanage_direct_connect(semanage_handle_t * sh)
>>> {
>>> const char *path;
>>> + struct stat sb;
>>> if (semanage_check_init(sh, sh->conf->store_root_path))
>>> goto err;
>>> @@ -302,10 +303,16 @@ int semanage_direct_connect(semanage_handle_t * sh)
>>> /* set the disable dontaudit value */
>>> path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_DISABLE_DONTAUDIT);
>>> - if (access(path, F_OK) == 0)
>>> +
>>> + if (stat(path, &sb) == 0)
>>> sepol_set_disable_dontaudit(sh->sepolh, 1);
>>> - else
>>> + else if (errno == ENOENT) {
>>> + /* The file does not exist */
>>> sepol_set_disable_dontaudit(sh->sepolh, 0);
>>> + } else {
>>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>>> + goto err;
>>> + }
>>> return STATUS_SUCCESS;
>>> @@ -1139,6 +1146,7 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
>>> int status = 0;
>>> int i;
>>> char cil_path[PATH_MAX];
>>> + struct stat sb;
>>> assert(sh);
>>> assert(modinfos);
>>> @@ -1155,9 +1163,13 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
>>> }
>>> if (semanage_get_ignore_module_cache(sh) == 0 &&
>>> - access(cil_path, F_OK) == 0) {
>>> + (status = stat(cil_path, &sb)) == 0) {
>>> continue;
>>> }
>>> + if (status != 0 && errno != ENOENT) {
>>> + ERR(sh, "Unable to access %s: %s\n", cil_path, strerror(errno));
>>> + goto cleanup; //an error in the "stat" call
>>> + }
>>> status = semanage_compile_module(sh, &modinfos[i]);
>>> if (status < 0) {
>>> @@ -1196,6 +1208,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>>> struct cil_db *cildb = NULL;
>>> semanage_module_info_t *modinfos = NULL;
>>> mode_t mask = umask(0077);
>>> + struct stat sb;
>>> int do_rebuild, do_write_kernel, do_install;
>>> int fcontexts_modified, ports_modified, seusers_modified,
>>> @@ -1234,10 +1247,15 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>>> /* Create or remove the disable_dontaudit flag file. */
>>> path = semanage_path(SEMANAGE_TMP, SEMANAGE_DISABLE_DONTAUDIT);
>>> - if (access(path, F_OK) == 0)
>>> + if (stat(path, &sb) == 0)
>>> do_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1);
>>> - else
>>> + else if (errno == ENOENT) {
>>> + /* The file does not exist */
>>> do_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1);
>>> + } else {
>>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>> We need to ensure that retval is set in all of these cases. Safest to always set it before goto cleanup,
>> either explicitly to -1 or to return value of stat() call above.
> Thank you.
> The retval is actually initialized to -1 (and stat() returns -1 for any error) so it's not necessary to set it here. It will be necessary a little bit down the road after the first assignment to retval, you're right.
> Should I set it to -1 here anyway to make it less prone to breaking due to future changes in the code (what about the other ERR calls in this section of the code)?
Yes, I would do that for safety here and in any other case where we didn't just set retval and test it. IOW, any case where we have:
retval = foo(args);
if (retval < 0) {
ERR(...)
goto cleanup;
}
is fine as is, but any case where we have:
if (some-condition-other-than-retval-less-than-0) {
ERR(...)
goto cleanup;
}
likely ought to explicitly set retval.
If we have too many of those, then maybe we ought to have an err: label that sets retval to -1 explicitly and goto it instead.
>
>>> + goto cleanup;
>>> + }
>>> if (sepol_get_disable_dontaudit(sh->sepolh) == 1) {
>>> FILE *touch;
>>> touch = fopen(path, "w");
>>> @@ -1259,10 +1277,16 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>>> /* Create or remove the preserve_tunables flag file. */
>>> path = semanage_path(SEMANAGE_TMP, SEMANAGE_PRESERVE_TUNABLES);
>>> - if (access(path, F_OK) == 0)
>>> + if (stat(path, &sb) == 0)
>>> do_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1);
>>> - else
>>> + else if (errno == ENOENT) {
>>> + /* The file does not exist */
>>> do_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1);
>>> + } else {
>>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>>> + goto cleanup;
>>> + }
>> Ditto
>>
>>> +
>>> if (sepol_get_preserve_tunables(sh->sepolh) == 1) {
>>> FILE *touch;
>>> touch = fopen(path, "w");
>>> @@ -1299,40 +1323,24 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>>> * a rebuild.
>>> */
>>> if (!do_rebuild) {
>>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_KERNEL);
>>> - if (access(path, F_OK) != 0) {
>>> - do_rebuild = 1;
>>> - goto rebuild;
>>> - }
>>> -
>>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
>>> - if (access(path, F_OK) != 0) {
>>> - do_rebuild = 1;
>>> - goto rebuild;
>>> - }
>>> -
>>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
>>> - if (access(path, F_OK) != 0) {
>>> - do_rebuild = 1;
>>> - goto rebuild;
>>> - }
>>> -
>>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_LINKED);
>>> - if (access(path, F_OK) != 0) {
>>> - do_rebuild = 1;
>>> - goto rebuild;
>>> - }
>>> -
>>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
>>> - if (access(path, F_OK) != 0) {
>>> - do_rebuild = 1;
>>> - goto rebuild;
>>> - }
>>> + int files[] = {SEMANAGE_STORE_KERNEL,
>>> + SEMANAGE_STORE_FC,
>>> + SEMANAGE_STORE_SEUSERS,
>>> + SEMANAGE_LINKED,
>>> + SEMANAGE_SEUSERS_LINKED,
>>> + SEMANAGE_USERS_EXTRA_LINKED};
>>> +
>>> + for (i = 0; i < (int) sizeof(files); i++) {
>>> + path = semanage_path(SEMANAGE_TMP, files[i]);
>>> + if (stat(path, &sb) != 0) {
>>> + if (errno != ENOENT) {
>>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>>> + goto cleanup;
>> Ditto
>>
>>> + }
>>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
>>> - if (access(path, F_OK) != 0) {
>>> - do_rebuild = 1;
>>> - goto rebuild;
>>> + do_rebuild = 1;
>>> + goto rebuild;
>>> + }
>>> }
>>> }
>>> @@ -1465,7 +1473,7 @@ rebuild:
>>> goto cleanup;
>>> path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
>>> - if (access(path, F_OK) == 0) {
>>> + if (stat(path, &sb) == 0) {
>>> retval = semanage_copy_file(path,
>>> semanage_path(SEMANAGE_TMP,
>>> SEMANAGE_STORE_SEUSERS),
>>> @@ -1473,12 +1481,18 @@ rebuild:
>>> if (retval < 0)
>>> goto cleanup;
>>> pseusers->dtable->drop_cache(pseusers->dbase);
>>> - } else {
>>> + } else if (errno == ENOENT) {
>>> + /* The file does not exist */
>>> pseusers->dtable->clear(sh, pseusers->dbase);
>>> + } else {
>>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>>> + goto cleanup;
>> Ditto
>>
>>> }
>>> +
>>> +
>>> path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
>>> - if (access(path, F_OK) == 0) {
>>> + if (stat(path, &sb) == 0) {
>>> retval = semanage_copy_file(path,
>>> semanage_path(SEMANAGE_TMP,
>>> SEMANAGE_USERS_EXTRA),
>>> @@ -1486,8 +1500,12 @@ rebuild:
>>> if (retval < 0)
>>> goto cleanup;
>>> pusers_extra->dtable->drop_cache(pusers_extra->dbase);
>>> - } else {
>>> + } else if (errno == ENOENT) {
>>> + /* The file does not exist */
>>> pusers_extra->dtable->clear(sh, pusers_extra->dbase);
>>> + } else {
>>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>>> + goto cleanup;
>> Ditto
>>
>>> }
>>> }
>>> @@ -1817,6 +1835,7 @@ static int semanage_direct_extract(semanage_handle_t * sh,
>>> ssize_t _data_len;
>>> char *_data;
>>> int compressed;
>>> + struct stat sb;
>>> /* get path of module */
>>> rc = semanage_module_get_path(
>>> @@ -1829,8 +1848,8 @@ static int semanage_direct_extract(semanage_handle_t * sh,
>>> goto cleanup;
>>> }
>>> - if (access(module_path, F_OK) != 0) {
>>> - ERR(sh, "Module does not exist: %s", module_path);
>>> + if (stat(module_path, &sb) != 0) {
>>> + ERR(sh, "Unable to access %s: %s\n", module_path, strerror(errno));
>>> rc = -1;
>>> goto cleanup;
>>> }
>>> @@ -1859,7 +1878,12 @@ static int semanage_direct_extract(semanage_handle_t * sh,
>>> goto cleanup;
>>> }
>>> - if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && access(input_file, F_OK) != 0) {
>>> + if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && stat(input_file, &sb) != 0) {
>>> + if (errno != ENOENT) {
>>> + ERR(sh, "Unable to access %s: %s\n", input_file, strerror(errno));
>> Ditto
>>
>>> + goto cleanup;
>>> +
>>> +
>>> rc = semanage_compile_module(sh, _modinfo);
>>> if (rc < 0) {
>>> goto cleanup;
>>> @@ -2004,6 +2028,12 @@ static int semanage_direct_get_enabled(semanage_handle_t *sh,
>>> }
>>> if (stat(path, &sb) < 0) {
>>> + if (errno != ENOENT) {
>>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>>> + status = -1;
>>> + goto cleanup;
>>> + }
>>> +
>>> *enabled = 1;
>>> }
>>> else {
>>> @@ -2330,6 +2360,12 @@ static int semanage_direct_get_module_info(semanage_handle_t *sh,
>>> /* set enabled/disabled status */
>>> if (stat(fn, &sb) < 0) {
>>> + if (errno != ENOENT) {
>>> + ERR(sh, "Unable to access %s: %s\n", fn, strerror(errno));
>>> + status = -1;
>>> + goto cleanup;
>>> + }
>>> +
>>> ret = semanage_module_info_set_enabled(sh, *modinfo, 1);
>>> if (ret != 0) {
>>> status = -1;
>>> @@ -2738,6 +2774,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
>>> int status = 0;
>>> int ret = 0;
>>> int type;
>>> + struct stat sb;
>>> char path[PATH_MAX];
>>> mode_t mask = umask(0077);
>>> @@ -2838,7 +2875,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
>>> goto cleanup;
>>> }
>>> - if (access(path, F_OK) == 0) {
>>> + if (stat(path, &sb) == 0) {
>>> ret = unlink(path);
>>> if (ret != 0) {
>>> ERR(sh, "Error while removing cached CIL file %s: %s", path, strerror(errno));
>>> diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c
>>> index 4bd1d651..10f8b306 100644
>>> --- a/libsemanage/src/semanage_store.c
>>> +++ b/libsemanage/src/semanage_store.c
>>> @@ -514,6 +514,7 @@ char *semanage_conf_path(void)
>>> {
>>> char *semanage_conf = NULL;
>>> int len;
>>> + struct stat sb;
>>> len = strlen(semanage_root()) + strlen(selinux_path()) + strlen(SEMANAGE_CONF_FILE);
>>> semanage_conf = calloc(len + 1, sizeof(char));
>>> @@ -522,7 +523,12 @@ char *semanage_conf_path(void)
>>> snprintf(semanage_conf, len + 1, "%s%s%s", semanage_root(), selinux_path(),
>>> SEMANAGE_CONF_FILE);
>>> - if (access(semanage_conf, R_OK) != 0) {
>>> + if (stat(semanage_conf, &sb) != 0) {
>>> + if (errno != ENOENT) {
>>> + /* semanage_conf exists but can't be accessed */
>>> + return NULL;
>> Not sure this is what we want. I think we'd just return semanage_conf here, and then let the caller fail when it tries to access it, so that we get a useful error message with the pathname.
>>
>>> + }
>>> +
>>> snprintf(semanage_conf, len + 1, "%s%s", selinux_path(), SEMANAGE_CONF_FILE);
>>> }
>>> @@ -1508,8 +1514,14 @@ int semanage_split_fc(semanage_handle_t * sh)
>>> static int sefcontext_compile(semanage_handle_t * sh, const char *path) {
>>> int r;
>>> + struct stat sb;
>>> +
>>> + if (stat(path, &sb) < 0) {
>>> + if (errno != ENOENT) {
>>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>>> + return -1;
>>> + }
>>> - if (access(path, F_OK) != 0) {
>>> return 0;
>>> }
>>>
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] libsemanage: replace access() checks to make setuid programs work
2018-03-09 13:28 ` Stephen Smalley
@ 2018-03-09 13:36 ` Stephen Smalley
2018-03-09 15:21 ` [PATCH] " Vit Mojzis
0 siblings, 1 reply; 19+ messages in thread
From: Stephen Smalley @ 2018-03-09 13:36 UTC (permalink / raw)
To: Vit Mojzis, selinux
On 03/09/2018 08:28 AM, Stephen Smalley wrote:
> On 03/09/2018 08:14 AM, Vit Mojzis wrote:
>>
>>
>> On 7.3.2018 15:59, Stephen Smalley wrote:
>>> On 03/06/2018 06:58 AM, Vit Mojzis wrote:
>>>> access() uses real UID instead of effective UID which causes false
>>>> negative checks in setuid programs.
>>>> Replace access(,F_OK) (i.e. tests for file existence) by stat().
>>>> And access(,R_OK) by fopen(,"r")
>>>>
>>>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
>>>>
>>>> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
>>>> ---
>>>> libsemanage/src/direct_api.c | 133 +++++++++++++++++++++++++--------------
>>>> libsemanage/src/semanage_store.c | 16 ++++-
>>>> 2 files changed, 99 insertions(+), 50 deletions(-)
>>>>
>>>> diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
>>>> index 49cac14f..4e2af810 100644
>>>> --- a/libsemanage/src/direct_api.c
>>>> +++ b/libsemanage/src/direct_api.c
>>>> @@ -140,6 +140,7 @@ int semanage_direct_is_managed(semanage_handle_t * sh)
>>>> int semanage_direct_connect(semanage_handle_t * sh)
>>>> {
>>>> const char *path;
>>>> + struct stat sb;
>>>> if (semanage_check_init(sh, sh->conf->store_root_path))
>>>> goto err;
>>>> @@ -302,10 +303,16 @@ int semanage_direct_connect(semanage_handle_t * sh)
>>>> /* set the disable dontaudit value */
>>>> path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_DISABLE_DONTAUDIT);
>>>> - if (access(path, F_OK) == 0)
>>>> +
>>>> + if (stat(path, &sb) == 0)
>>>> sepol_set_disable_dontaudit(sh->sepolh, 1);
>>>> - else
>>>> + else if (errno == ENOENT) {
>>>> + /* The file does not exist */
>>>> sepol_set_disable_dontaudit(sh->sepolh, 0);
>>>> + } else {
>>>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>>>> + goto err;
>>>> + }
>>>> return STATUS_SUCCESS;
>>>> @@ -1139,6 +1146,7 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
>>>> int status = 0;
>>>> int i;
>>>> char cil_path[PATH_MAX];
>>>> + struct stat sb;
>>>> assert(sh);
>>>> assert(modinfos);
>>>> @@ -1155,9 +1163,13 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
>>>> }
>>>> if (semanage_get_ignore_module_cache(sh) == 0 &&
>>>> - access(cil_path, F_OK) == 0) {
>>>> + (status = stat(cil_path, &sb)) == 0) {
>>>> continue;
>>>> }
>>>> + if (status != 0 && errno != ENOENT) {
>>>> + ERR(sh, "Unable to access %s: %s\n", cil_path, strerror(errno));
>>>> + goto cleanup; //an error in the "stat" call
>>>> + }
>>>> status = semanage_compile_module(sh, &modinfos[i]);
>>>> if (status < 0) {
>>>> @@ -1196,6 +1208,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>>>> struct cil_db *cildb = NULL;
>>>> semanage_module_info_t *modinfos = NULL;
>>>> mode_t mask = umask(0077);
>>>> + struct stat sb;
>>>> int do_rebuild, do_write_kernel, do_install;
>>>> int fcontexts_modified, ports_modified, seusers_modified,
>>>> @@ -1234,10 +1247,15 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>>>> /* Create or remove the disable_dontaudit flag file. */
>>>> path = semanage_path(SEMANAGE_TMP, SEMANAGE_DISABLE_DONTAUDIT);
>>>> - if (access(path, F_OK) == 0)
>>>> + if (stat(path, &sb) == 0)
>>>> do_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1);
>>>> - else
>>>> + else if (errno == ENOENT) {
>>>> + /* The file does not exist */
>>>> do_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1);
>>>> + } else {
>>>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>>> We need to ensure that retval is set in all of these cases. Safest to always set it before goto cleanup,
>>> either explicitly to -1 or to return value of stat() call above.
>> Thank you.
>> The retval is actually initialized to -1 (and stat() returns -1 for any error) so it's not necessary to set it here. It will be necessary a little bit down the road after the first assignment to retval, you're right.
>> Should I set it to -1 here anyway to make it less prone to breaking due to future changes in the code (what about the other ERR calls in this section of the code)?
>
> Yes, I would do that for safety here and in any other case where we didn't just set retval and test it. IOW, any case where we have:
> retval = foo(args);
> if (retval < 0) {
> ERR(...)
> goto cleanup;
> }
> is fine as is, but any case where we have:
> if (some-condition-other-than-retval-less-than-0) {
> ERR(...)
> goto cleanup;
> }
> likely ought to explicitly set retval.
>
> If we have too many of those, then maybe we ought to have an err: label that sets retval to -1 explicitly and goto it instead.
That said, you don't have to fix cases that you didn't introduce in your patch; that's up to you, and you can fix pre-existing cases via a separate patch if desired.
>
>>
>>>> + goto cleanup;
>>>> + }
>>>> if (sepol_get_disable_dontaudit(sh->sepolh) == 1) {
>>>> FILE *touch;
>>>> touch = fopen(path, "w");
>>>> @@ -1259,10 +1277,16 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>>>> /* Create or remove the preserve_tunables flag file. */
>>>> path = semanage_path(SEMANAGE_TMP, SEMANAGE_PRESERVE_TUNABLES);
>>>> - if (access(path, F_OK) == 0)
>>>> + if (stat(path, &sb) == 0)
>>>> do_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1);
>>>> - else
>>>> + else if (errno == ENOENT) {
>>>> + /* The file does not exist */
>>>> do_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1);
>>>> + } else {
>>>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>>>> + goto cleanup;
>>>> + }
>>> Ditto
>>>
>>>> +
>>>> if (sepol_get_preserve_tunables(sh->sepolh) == 1) {
>>>> FILE *touch;
>>>> touch = fopen(path, "w");
>>>> @@ -1299,40 +1323,24 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>>>> * a rebuild.
>>>> */
>>>> if (!do_rebuild) {
>>>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_KERNEL);
>>>> - if (access(path, F_OK) != 0) {
>>>> - do_rebuild = 1;
>>>> - goto rebuild;
>>>> - }
>>>> -
>>>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
>>>> - if (access(path, F_OK) != 0) {
>>>> - do_rebuild = 1;
>>>> - goto rebuild;
>>>> - }
>>>> -
>>>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
>>>> - if (access(path, F_OK) != 0) {
>>>> - do_rebuild = 1;
>>>> - goto rebuild;
>>>> - }
>>>> -
>>>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_LINKED);
>>>> - if (access(path, F_OK) != 0) {
>>>> - do_rebuild = 1;
>>>> - goto rebuild;
>>>> - }
>>>> -
>>>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
>>>> - if (access(path, F_OK) != 0) {
>>>> - do_rebuild = 1;
>>>> - goto rebuild;
>>>> - }
>>>> + int files[] = {SEMANAGE_STORE_KERNEL,
>>>> + SEMANAGE_STORE_FC,
>>>> + SEMANAGE_STORE_SEUSERS,
>>>> + SEMANAGE_LINKED,
>>>> + SEMANAGE_SEUSERS_LINKED,
>>>> + SEMANAGE_USERS_EXTRA_LINKED};
>>>> +
>>>> + for (i = 0; i < (int) sizeof(files); i++) {
>>>> + path = semanage_path(SEMANAGE_TMP, files[i]);
>>>> + if (stat(path, &sb) != 0) {
>>>> + if (errno != ENOENT) {
>>>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>>>> + goto cleanup;
>>> Ditto
>>>
>>>> + }
>>>> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
>>>> - if (access(path, F_OK) != 0) {
>>>> - do_rebuild = 1;
>>>> - goto rebuild;
>>>> + do_rebuild = 1;
>>>> + goto rebuild;
>>>> + }
>>>> }
>>>> }
>>>> @@ -1465,7 +1473,7 @@ rebuild:
>>>> goto cleanup;
>>>> path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
>>>> - if (access(path, F_OK) == 0) {
>>>> + if (stat(path, &sb) == 0) {
>>>> retval = semanage_copy_file(path,
>>>> semanage_path(SEMANAGE_TMP,
>>>> SEMANAGE_STORE_SEUSERS),
>>>> @@ -1473,12 +1481,18 @@ rebuild:
>>>> if (retval < 0)
>>>> goto cleanup;
>>>> pseusers->dtable->drop_cache(pseusers->dbase);
>>>> - } else {
>>>> + } else if (errno == ENOENT) {
>>>> + /* The file does not exist */
>>>> pseusers->dtable->clear(sh, pseusers->dbase);
>>>> + } else {
>>>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>>>> + goto cleanup;
>>> Ditto
>>>
>>>> }
>>>> +
>>>> +
>>>> path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
>>>> - if (access(path, F_OK) == 0) {
>>>> + if (stat(path, &sb) == 0) {
>>>> retval = semanage_copy_file(path,
>>>> semanage_path(SEMANAGE_TMP,
>>>> SEMANAGE_USERS_EXTRA),
>>>> @@ -1486,8 +1500,12 @@ rebuild:
>>>> if (retval < 0)
>>>> goto cleanup;
>>>> pusers_extra->dtable->drop_cache(pusers_extra->dbase);
>>>> - } else {
>>>> + } else if (errno == ENOENT) {
>>>> + /* The file does not exist */
>>>> pusers_extra->dtable->clear(sh, pusers_extra->dbase);
>>>> + } else {
>>>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>>>> + goto cleanup;
>>> Ditto
>>>
>>>> }
>>>> }
>>>> @@ -1817,6 +1835,7 @@ static int semanage_direct_extract(semanage_handle_t * sh,
>>>> ssize_t _data_len;
>>>> char *_data;
>>>> int compressed;
>>>> + struct stat sb;
>>>> /* get path of module */
>>>> rc = semanage_module_get_path(
>>>> @@ -1829,8 +1848,8 @@ static int semanage_direct_extract(semanage_handle_t * sh,
>>>> goto cleanup;
>>>> }
>>>> - if (access(module_path, F_OK) != 0) {
>>>> - ERR(sh, "Module does not exist: %s", module_path);
>>>> + if (stat(module_path, &sb) != 0) {
>>>> + ERR(sh, "Unable to access %s: %s\n", module_path, strerror(errno));
>>>> rc = -1;
>>>> goto cleanup;
>>>> }
>>>> @@ -1859,7 +1878,12 @@ static int semanage_direct_extract(semanage_handle_t * sh,
>>>> goto cleanup;
>>>> }
>>>> - if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && access(input_file, F_OK) != 0) {
>>>> + if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && stat(input_file, &sb) != 0) {
>>>> + if (errno != ENOENT) {
>>>> + ERR(sh, "Unable to access %s: %s\n", input_file, strerror(errno));
>>> Ditto
>>>
>>>> + goto cleanup;
>>>> +
>>>> +
>>>> rc = semanage_compile_module(sh, _modinfo);
>>>> if (rc < 0) {
>>>> goto cleanup;
>>>> @@ -2004,6 +2028,12 @@ static int semanage_direct_get_enabled(semanage_handle_t *sh,
>>>> }
>>>> if (stat(path, &sb) < 0) {
>>>> + if (errno != ENOENT) {
>>>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>>>> + status = -1;
>>>> + goto cleanup;
>>>> + }
>>>> +
>>>> *enabled = 1;
>>>> }
>>>> else {
>>>> @@ -2330,6 +2360,12 @@ static int semanage_direct_get_module_info(semanage_handle_t *sh,
>>>> /* set enabled/disabled status */
>>>> if (stat(fn, &sb) < 0) {
>>>> + if (errno != ENOENT) {
>>>> + ERR(sh, "Unable to access %s: %s\n", fn, strerror(errno));
>>>> + status = -1;
>>>> + goto cleanup;
>>>> + }
>>>> +
>>>> ret = semanage_module_info_set_enabled(sh, *modinfo, 1);
>>>> if (ret != 0) {
>>>> status = -1;
>>>> @@ -2738,6 +2774,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
>>>> int status = 0;
>>>> int ret = 0;
>>>> int type;
>>>> + struct stat sb;
>>>> char path[PATH_MAX];
>>>> mode_t mask = umask(0077);
>>>> @@ -2838,7 +2875,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
>>>> goto cleanup;
>>>> }
>>>> - if (access(path, F_OK) == 0) {
>>>> + if (stat(path, &sb) == 0) {
>>>> ret = unlink(path);
>>>> if (ret != 0) {
>>>> ERR(sh, "Error while removing cached CIL file %s: %s", path, strerror(errno));
>>>> diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c
>>>> index 4bd1d651..10f8b306 100644
>>>> --- a/libsemanage/src/semanage_store.c
>>>> +++ b/libsemanage/src/semanage_store.c
>>>> @@ -514,6 +514,7 @@ char *semanage_conf_path(void)
>>>> {
>>>> char *semanage_conf = NULL;
>>>> int len;
>>>> + struct stat sb;
>>>> len = strlen(semanage_root()) + strlen(selinux_path()) + strlen(SEMANAGE_CONF_FILE);
>>>> semanage_conf = calloc(len + 1, sizeof(char));
>>>> @@ -522,7 +523,12 @@ char *semanage_conf_path(void)
>>>> snprintf(semanage_conf, len + 1, "%s%s%s", semanage_root(), selinux_path(),
>>>> SEMANAGE_CONF_FILE);
>>>> - if (access(semanage_conf, R_OK) != 0) {
>>>> + if (stat(semanage_conf, &sb) != 0) {
>>>> + if (errno != ENOENT) {
>>>> + /* semanage_conf exists but can't be accessed */
>>>> + return NULL;
>>> Not sure this is what we want. I think we'd just return semanage_conf here, and then let the caller fail when it tries to access it, so that we get a useful error message with the pathname.
>>>
>>>> + }
>>>> +
>>>> snprintf(semanage_conf, len + 1, "%s%s", selinux_path(), SEMANAGE_CONF_FILE);
>>>> }
>>>> @@ -1508,8 +1514,14 @@ int semanage_split_fc(semanage_handle_t * sh)
>>>> static int sefcontext_compile(semanage_handle_t * sh, const char *path) {
>>>> int r;
>>>> + struct stat sb;
>>>> +
>>>> + if (stat(path, &sb) < 0) {
>>>> + if (errno != ENOENT) {
>>>> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
>>>> + return -1;
>>>> + }
>>>> - if (access(path, F_OK) != 0) {
>>>> return 0;
>>>> }
>>>>
>>
>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] libsemanage: replace access() checks to make setuid programs work
2018-03-09 13:36 ` Stephen Smalley
@ 2018-03-09 15:21 ` Vit Mojzis
2018-03-09 15:31 ` Stephen Smalley
0 siblings, 1 reply; 19+ messages in thread
From: Vit Mojzis @ 2018-03-09 15:21 UTC (permalink / raw)
To: selinux
access() uses real UID instead of effective UID which causes false
negative checks in setuid programs.
Replace access() calls (mostly tests for file existence) by stat().
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
---
libsemanage/src/direct_api.c | 137 +++++++++++++++++++++++++--------------
libsemanage/src/semanage_store.c | 11 +++-
2 files changed, 98 insertions(+), 50 deletions(-)
diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index 92d7517d..439122df 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -140,6 +140,7 @@ int semanage_direct_is_managed(semanage_handle_t * sh)
int semanage_direct_connect(semanage_handle_t * sh)
{
const char *path;
+ struct stat sb;
if (semanage_check_init(sh, sh->conf->store_root_path))
goto err;
@@ -302,10 +303,16 @@ int semanage_direct_connect(semanage_handle_t * sh)
/* set the disable dontaudit value */
path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_DISABLE_DONTAUDIT);
- if (access(path, F_OK) == 0)
+
+ if (stat(path, &sb) == 0)
sepol_set_disable_dontaudit(sh->sepolh, 1);
- else
+ else if (errno == ENOENT) {
+ /* The file does not exist */
sepol_set_disable_dontaudit(sh->sepolh, 0);
+ } else {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ goto err;
+ }
return STATUS_SUCCESS;
@@ -1139,6 +1146,7 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
int status = 0;
int i;
char cil_path[PATH_MAX];
+ struct stat sb;
assert(sh);
assert(modinfos);
@@ -1155,9 +1163,13 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
}
if (semanage_get_ignore_module_cache(sh) == 0 &&
- access(cil_path, F_OK) == 0) {
+ (status = stat(cil_path, &sb)) == 0) {
continue;
}
+ if (status != 0 && errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", cil_path, strerror(errno));
+ goto cleanup; //an error in the "stat" call
+ }
status = semanage_compile_module(sh, &modinfos[i]);
if (status < 0) {
@@ -1196,6 +1208,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
struct cil_db *cildb = NULL;
semanage_module_info_t *modinfos = NULL;
mode_t mask = umask(0077);
+ struct stat sb;
int do_rebuild, do_write_kernel, do_install;
int fcontexts_modified, ports_modified, seusers_modified,
@@ -1234,10 +1247,16 @@ static int semanage_direct_commit(semanage_handle_t * sh)
/* Create or remove the disable_dontaudit flag file. */
path = semanage_path(SEMANAGE_TMP, SEMANAGE_DISABLE_DONTAUDIT);
- if (access(path, F_OK) == 0)
+ if (stat(path, &sb) == 0)
do_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1);
- else
+ else if (errno == ENOENT) {
+ /* The file does not exist */
do_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1);
+ } else {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ retval = -1;
+ goto cleanup;
+ }
if (sepol_get_disable_dontaudit(sh->sepolh) == 1) {
FILE *touch;
touch = fopen(path, "w");
@@ -1259,10 +1278,17 @@ static int semanage_direct_commit(semanage_handle_t * sh)
/* Create or remove the preserve_tunables flag file. */
path = semanage_path(SEMANAGE_TMP, SEMANAGE_PRESERVE_TUNABLES);
- if (access(path, F_OK) == 0)
+ if (stat(path, &sb) == 0)
do_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1);
- else
+ else if (errno == ENOENT) {
+ /* The file does not exist */
do_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1);
+ } else {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ retval = -1;
+ goto cleanup;
+ }
+
if (sepol_get_preserve_tunables(sh->sepolh) == 1) {
FILE *touch;
touch = fopen(path, "w");
@@ -1299,40 +1325,25 @@ static int semanage_direct_commit(semanage_handle_t * sh)
* a rebuild.
*/
if (!do_rebuild) {
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_KERNEL);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
- }
-
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
- }
-
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
- }
-
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_LINKED);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
- }
-
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
- }
+ int files[] = {SEMANAGE_STORE_KERNEL,
+ SEMANAGE_STORE_FC,
+ SEMANAGE_STORE_SEUSERS,
+ SEMANAGE_LINKED,
+ SEMANAGE_SEUSERS_LINKED,
+ SEMANAGE_USERS_EXTRA_LINKED};
+
+ for (i = 0; i < (int) sizeof(files); i++) {
+ path = semanage_path(SEMANAGE_TMP, files[i]);
+ if (stat(path, &sb) != 0) {
+ if (errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ retval = -1;
+ goto cleanup;
+ }
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
+ do_rebuild = 1;
+ goto rebuild;
+ }
}
}
@@ -1465,7 +1476,7 @@ rebuild:
goto cleanup;
path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
- if (access(path, F_OK) == 0) {
+ if (stat(path, &sb) == 0) {
retval = semanage_copy_file(path,
semanage_path(SEMANAGE_TMP,
SEMANAGE_STORE_SEUSERS),
@@ -1473,12 +1484,17 @@ rebuild:
if (retval < 0)
goto cleanup;
pseusers->dtable->drop_cache(pseusers->dbase);
- } else {
+ } else if (errno == ENOENT) {
+ /* The file does not exist */
pseusers->dtable->clear(sh, pseusers->dbase);
+ } else {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ retval = -1;
+ goto cleanup;
}
path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
- if (access(path, F_OK) == 0) {
+ if (stat(path, &sb) == 0) {
retval = semanage_copy_file(path,
semanage_path(SEMANAGE_TMP,
SEMANAGE_USERS_EXTRA),
@@ -1486,8 +1502,13 @@ rebuild:
if (retval < 0)
goto cleanup;
pusers_extra->dtable->drop_cache(pusers_extra->dbase);
- } else {
+ } else if (errno == ENOENT) {
+ /* The file does not exist */
pusers_extra->dtable->clear(sh, pusers_extra->dbase);
+ } else {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ retval = -1;
+ goto cleanup;
}
}
@@ -1817,6 +1838,7 @@ static int semanage_direct_extract(semanage_handle_t * sh,
ssize_t _data_len;
char *_data;
int compressed;
+ struct stat sb;
/* get path of module */
rc = semanage_module_get_path(
@@ -1829,8 +1851,8 @@ static int semanage_direct_extract(semanage_handle_t * sh,
goto cleanup;
}
- if (access(module_path, F_OK) != 0) {
- ERR(sh, "Module does not exist: %s", module_path);
+ if (stat(module_path, &sb) != 0) {
+ ERR(sh, "Unable to access %s: %s\n", module_path, strerror(errno));
rc = -1;
goto cleanup;
}
@@ -1859,7 +1881,13 @@ static int semanage_direct_extract(semanage_handle_t * sh,
goto cleanup;
}
- if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && access(input_file, F_OK) != 0) {
+ if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && stat(input_file, &sb) != 0) {
+ if (errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", input_file, strerror(errno));
+ rc = -1;
+ goto cleanup;
+ }
+
rc = semanage_compile_module(sh, _modinfo);
if (rc < 0) {
goto cleanup;
@@ -2004,6 +2032,12 @@ static int semanage_direct_get_enabled(semanage_handle_t *sh,
}
if (stat(path, &sb) < 0) {
+ if (errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ status = -1;
+ goto cleanup;
+ }
+
*enabled = 1;
}
else {
@@ -2330,6 +2364,12 @@ static int semanage_direct_get_module_info(semanage_handle_t *sh,
/* set enabled/disabled status */
if (stat(fn, &sb) < 0) {
+ if (errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", fn, strerror(errno));
+ status = -1;
+ goto cleanup;
+ }
+
ret = semanage_module_info_set_enabled(sh, *modinfo, 1);
if (ret != 0) {
status = -1;
@@ -2738,6 +2778,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
int status = 0;
int ret = 0;
int type;
+ struct stat sb;
char path[PATH_MAX];
mode_t mask = umask(0077);
@@ -2838,7 +2879,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
goto cleanup;
}
- if (access(path, F_OK) == 0) {
+ if (stat(path, &sb) == 0) {
ret = unlink(path);
if (ret != 0) {
ERR(sh, "Error while removing cached CIL file %s: %s", path, strerror(errno));
diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c
index 4bd1d651..1e71ab67 100644
--- a/libsemanage/src/semanage_store.c
+++ b/libsemanage/src/semanage_store.c
@@ -514,6 +514,7 @@ char *semanage_conf_path(void)
{
char *semanage_conf = NULL;
int len;
+ struct stat sb;
len = strlen(semanage_root()) + strlen(selinux_path()) + strlen(SEMANAGE_CONF_FILE);
semanage_conf = calloc(len + 1, sizeof(char));
@@ -522,7 +523,7 @@ char *semanage_conf_path(void)
snprintf(semanage_conf, len + 1, "%s%s%s", semanage_root(), selinux_path(),
SEMANAGE_CONF_FILE);
- if (access(semanage_conf, R_OK) != 0) {
+ if (stat(semanage_conf, &sb) != 0 && errno == ENONET) {
snprintf(semanage_conf, len + 1, "%s%s", selinux_path(), SEMANAGE_CONF_FILE);
}
@@ -1508,8 +1509,14 @@ int semanage_split_fc(semanage_handle_t * sh)
static int sefcontext_compile(semanage_handle_t * sh, const char *path) {
int r;
+ struct stat sb;
+
+ if (stat(path, &sb) < 0) {
+ if (errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ return -1;
+ }
- if (access(path, F_OK) != 0) {
return 0;
}
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] libsemanage: replace access() checks to make setuid programs work
2018-03-09 15:21 ` [PATCH] " Vit Mojzis
@ 2018-03-09 15:31 ` Stephen Smalley
2018-03-09 15:39 ` Vit Mojzis
0 siblings, 1 reply; 19+ messages in thread
From: Stephen Smalley @ 2018-03-09 15:31 UTC (permalink / raw)
To: Vit Mojzis, selinux
On 03/09/2018 10:21 AM, Vit Mojzis wrote:
> access() uses real UID instead of effective UID which causes false
> negative checks in setuid programs.
> Replace access() calls (mostly tests for file existence) by stat().
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
>
> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
> ---
> libsemanage/src/direct_api.c | 137 +++++++++++++++++++++++++--------------
> libsemanage/src/semanage_store.c | 11 +++-
> 2 files changed, 98 insertions(+), 50 deletions(-)
>
> diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
> index 92d7517d..439122df 100644
> --- a/libsemanage/src/direct_api.c
> +++ b/libsemanage/src/direct_api.c
> @@ -140,6 +140,7 @@ int semanage_direct_is_managed(semanage_handle_t * sh)
> int semanage_direct_connect(semanage_handle_t * sh)
> {
> const char *path;
> + struct stat sb;
>
> if (semanage_check_init(sh, sh->conf->store_root_path))
> goto err;
> @@ -302,10 +303,16 @@ int semanage_direct_connect(semanage_handle_t * sh)
>
> /* set the disable dontaudit value */
> path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_DISABLE_DONTAUDIT);
> - if (access(path, F_OK) == 0)
> +
> + if (stat(path, &sb) == 0)
> sepol_set_disable_dontaudit(sh->sepolh, 1);
> - else
> + else if (errno == ENOENT) {
> + /* The file does not exist */
> sepol_set_disable_dontaudit(sh->sepolh, 0);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + goto err;
> + }
>
> return STATUS_SUCCESS;
>
> @@ -1139,6 +1146,7 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
> int status = 0;
> int i;
> char cil_path[PATH_MAX];
> + struct stat sb;
>
> assert(sh);
> assert(modinfos);
> @@ -1155,9 +1163,13 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
> }
>
> if (semanage_get_ignore_module_cache(sh) == 0 &&
> - access(cil_path, F_OK) == 0) {
> + (status = stat(cil_path, &sb)) == 0) {
> continue;
> }
> + if (status != 0 && errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", cil_path, strerror(errno));
> + goto cleanup; //an error in the "stat" call
> + }
>
> status = semanage_compile_module(sh, &modinfos[i]);
> if (status < 0) {
> @@ -1196,6 +1208,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
> struct cil_db *cildb = NULL;
> semanage_module_info_t *modinfos = NULL;
> mode_t mask = umask(0077);
> + struct stat sb;
>
> int do_rebuild, do_write_kernel, do_install;
> int fcontexts_modified, ports_modified, seusers_modified,
> @@ -1234,10 +1247,16 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>
> /* Create or remove the disable_dontaudit flag file. */
> path = semanage_path(SEMANAGE_TMP, SEMANAGE_DISABLE_DONTAUDIT);
> - if (access(path, F_OK) == 0)
> + if (stat(path, &sb) == 0)
> do_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1);
> - else
> + else if (errno == ENOENT) {
> + /* The file does not exist */
> do_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + retval = -1;
> + goto cleanup;
> + }
> if (sepol_get_disable_dontaudit(sh->sepolh) == 1) {
> FILE *touch;
> touch = fopen(path, "w");
> @@ -1259,10 +1278,17 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>
> /* Create or remove the preserve_tunables flag file. */
> path = semanage_path(SEMANAGE_TMP, SEMANAGE_PRESERVE_TUNABLES);
> - if (access(path, F_OK) == 0)
> + if (stat(path, &sb) == 0)
> do_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1);
> - else
> + else if (errno == ENOENT) {
> + /* The file does not exist */
> do_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + retval = -1;
> + goto cleanup;
> + }
> +
> if (sepol_get_preserve_tunables(sh->sepolh) == 1) {
> FILE *touch;
> touch = fopen(path, "w");
> @@ -1299,40 +1325,25 @@ static int semanage_direct_commit(semanage_handle_t * sh)
> * a rebuild.
> */
> if (!do_rebuild) {
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_KERNEL);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> -
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> -
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> -
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_LINKED);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> -
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> + int files[] = {SEMANAGE_STORE_KERNEL,
> + SEMANAGE_STORE_FC,
> + SEMANAGE_STORE_SEUSERS,
> + SEMANAGE_LINKED,
> + SEMANAGE_SEUSERS_LINKED,
> + SEMANAGE_USERS_EXTRA_LINKED};
> +
> + for (i = 0; i < (int) sizeof(files); i++) {
> + path = semanage_path(SEMANAGE_TMP, files[i]);
> + if (stat(path, &sb) != 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + retval = -1;
> + goto cleanup;
> + }
>
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> + do_rebuild = 1;
> + goto rebuild;
> + }
> }
> }
>
> @@ -1465,7 +1476,7 @@ rebuild:
> goto cleanup;
>
> path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
> - if (access(path, F_OK) == 0) {
> + if (stat(path, &sb) == 0) {
> retval = semanage_copy_file(path,
> semanage_path(SEMANAGE_TMP,
> SEMANAGE_STORE_SEUSERS),
> @@ -1473,12 +1484,17 @@ rebuild:
> if (retval < 0)
> goto cleanup;
> pseusers->dtable->drop_cache(pseusers->dbase);
> - } else {
> + } else if (errno == ENOENT) {
> + /* The file does not exist */
> pseusers->dtable->clear(sh, pseusers->dbase);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + retval = -1;
> + goto cleanup;
> }
>
> path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
> - if (access(path, F_OK) == 0) {
> + if (stat(path, &sb) == 0) {
> retval = semanage_copy_file(path,
> semanage_path(SEMANAGE_TMP,
> SEMANAGE_USERS_EXTRA),
> @@ -1486,8 +1502,13 @@ rebuild:
> if (retval < 0)
> goto cleanup;
> pusers_extra->dtable->drop_cache(pusers_extra->dbase);
> - } else {
> + } else if (errno == ENOENT) {
> + /* The file does not exist */
> pusers_extra->dtable->clear(sh, pusers_extra->dbase);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + retval = -1;
> + goto cleanup;
> }
> }
>
> @@ -1817,6 +1838,7 @@ static int semanage_direct_extract(semanage_handle_t * sh,
> ssize_t _data_len;
> char *_data;
> int compressed;
> + struct stat sb;
>
> /* get path of module */
> rc = semanage_module_get_path(
> @@ -1829,8 +1851,8 @@ static int semanage_direct_extract(semanage_handle_t * sh,
> goto cleanup;
> }
>
> - if (access(module_path, F_OK) != 0) {
> - ERR(sh, "Module does not exist: %s", module_path);
> + if (stat(module_path, &sb) != 0) {
> + ERR(sh, "Unable to access %s: %s\n", module_path, strerror(errno));
> rc = -1;
> goto cleanup;
> }
> @@ -1859,7 +1881,13 @@ static int semanage_direct_extract(semanage_handle_t * sh,
> goto cleanup;
> }
>
> - if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && access(input_file, F_OK) != 0) {
> + if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && stat(input_file, &sb) != 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", input_file, strerror(errno));
> + rc = -1;
> + goto cleanup;
> + }
> +
> rc = semanage_compile_module(sh, _modinfo);
> if (rc < 0) {
> goto cleanup;
> @@ -2004,6 +2032,12 @@ static int semanage_direct_get_enabled(semanage_handle_t *sh,
> }
>
> if (stat(path, &sb) < 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + status = -1;
> + goto cleanup;
> + }
> +
> *enabled = 1;
> }
> else {
> @@ -2330,6 +2364,12 @@ static int semanage_direct_get_module_info(semanage_handle_t *sh,
>
> /* set enabled/disabled status */
> if (stat(fn, &sb) < 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", fn, strerror(errno));
> + status = -1;
> + goto cleanup;
> + }
> +
> ret = semanage_module_info_set_enabled(sh, *modinfo, 1);
> if (ret != 0) {
> status = -1;
> @@ -2738,6 +2778,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
> int status = 0;
> int ret = 0;
> int type;
> + struct stat sb;
>
> char path[PATH_MAX];
> mode_t mask = umask(0077);
> @@ -2838,7 +2879,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
> goto cleanup;
> }
>
> - if (access(path, F_OK) == 0) {
> + if (stat(path, &sb) == 0) {
> ret = unlink(path);
> if (ret != 0) {
> ERR(sh, "Error while removing cached CIL file %s: %s", path, strerror(errno));
> diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c
> index 4bd1d651..1e71ab67 100644
> --- a/libsemanage/src/semanage_store.c
> +++ b/libsemanage/src/semanage_store.c
> @@ -514,6 +514,7 @@ char *semanage_conf_path(void)
> {
> char *semanage_conf = NULL;
> int len;
> + struct stat sb;
>
> len = strlen(semanage_root()) + strlen(selinux_path()) + strlen(SEMANAGE_CONF_FILE);
> semanage_conf = calloc(len + 1, sizeof(char));
> @@ -522,7 +523,7 @@ char *semanage_conf_path(void)
> snprintf(semanage_conf, len + 1, "%s%s%s", semanage_root(), selinux_path(),
> SEMANAGE_CONF_FILE);
>
> - if (access(semanage_conf, R_OK) != 0) {
> + if (stat(semanage_conf, &sb) != 0 && errno == ENONET) {
Typo: ENONET vs ENOENT.
> snprintf(semanage_conf, len + 1, "%s%s", selinux_path(), SEMANAGE_CONF_FILE);
> }
>
> @@ -1508,8 +1509,14 @@ int semanage_split_fc(semanage_handle_t * sh)
> static int sefcontext_compile(semanage_handle_t * sh, const char *path) {
>
> int r;
> + struct stat sb;
> +
> + if (stat(path, &sb) < 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + return -1;
> + }
>
> - if (access(path, F_OK) != 0) {
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] libsemanage: replace access() checks to make setuid programs work
2018-03-09 15:31 ` Stephen Smalley
@ 2018-03-09 15:39 ` Vit Mojzis
2018-03-09 15:51 ` Stephen Smalley
2018-03-17 7:26 ` Petr Lautrbach
0 siblings, 2 replies; 19+ messages in thread
From: Vit Mojzis @ 2018-03-09 15:39 UTC (permalink / raw)
To: selinux
access() uses real UID instead of effective UID which causes false
negative checks in setuid programs.
Replace access() calls (mostly tests for file existence) by stat().
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
---
libsemanage/src/direct_api.c | 137 +++++++++++++++++++++++++--------------
libsemanage/src/semanage_store.c | 11 +++-
2 files changed, 98 insertions(+), 50 deletions(-)
diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index 92d7517d..439122df 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -140,6 +140,7 @@ int semanage_direct_is_managed(semanage_handle_t * sh)
int semanage_direct_connect(semanage_handle_t * sh)
{
const char *path;
+ struct stat sb;
if (semanage_check_init(sh, sh->conf->store_root_path))
goto err;
@@ -302,10 +303,16 @@ int semanage_direct_connect(semanage_handle_t * sh)
/* set the disable dontaudit value */
path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_DISABLE_DONTAUDIT);
- if (access(path, F_OK) == 0)
+
+ if (stat(path, &sb) == 0)
sepol_set_disable_dontaudit(sh->sepolh, 1);
- else
+ else if (errno == ENOENT) {
+ /* The file does not exist */
sepol_set_disable_dontaudit(sh->sepolh, 0);
+ } else {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ goto err;
+ }
return STATUS_SUCCESS;
@@ -1139,6 +1146,7 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
int status = 0;
int i;
char cil_path[PATH_MAX];
+ struct stat sb;
assert(sh);
assert(modinfos);
@@ -1155,9 +1163,13 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
}
if (semanage_get_ignore_module_cache(sh) == 0 &&
- access(cil_path, F_OK) == 0) {
+ (status = stat(cil_path, &sb)) == 0) {
continue;
}
+ if (status != 0 && errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", cil_path, strerror(errno));
+ goto cleanup; //an error in the "stat" call
+ }
status = semanage_compile_module(sh, &modinfos[i]);
if (status < 0) {
@@ -1196,6 +1208,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
struct cil_db *cildb = NULL;
semanage_module_info_t *modinfos = NULL;
mode_t mask = umask(0077);
+ struct stat sb;
int do_rebuild, do_write_kernel, do_install;
int fcontexts_modified, ports_modified, seusers_modified,
@@ -1234,10 +1247,16 @@ static int semanage_direct_commit(semanage_handle_t * sh)
/* Create or remove the disable_dontaudit flag file. */
path = semanage_path(SEMANAGE_TMP, SEMANAGE_DISABLE_DONTAUDIT);
- if (access(path, F_OK) == 0)
+ if (stat(path, &sb) == 0)
do_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1);
- else
+ else if (errno == ENOENT) {
+ /* The file does not exist */
do_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1);
+ } else {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ retval = -1;
+ goto cleanup;
+ }
if (sepol_get_disable_dontaudit(sh->sepolh) == 1) {
FILE *touch;
touch = fopen(path, "w");
@@ -1259,10 +1278,17 @@ static int semanage_direct_commit(semanage_handle_t * sh)
/* Create or remove the preserve_tunables flag file. */
path = semanage_path(SEMANAGE_TMP, SEMANAGE_PRESERVE_TUNABLES);
- if (access(path, F_OK) == 0)
+ if (stat(path, &sb) == 0)
do_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1);
- else
+ else if (errno == ENOENT) {
+ /* The file does not exist */
do_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1);
+ } else {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ retval = -1;
+ goto cleanup;
+ }
+
if (sepol_get_preserve_tunables(sh->sepolh) == 1) {
FILE *touch;
touch = fopen(path, "w");
@@ -1299,40 +1325,25 @@ static int semanage_direct_commit(semanage_handle_t * sh)
* a rebuild.
*/
if (!do_rebuild) {
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_KERNEL);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
- }
-
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
- }
-
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
- }
-
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_LINKED);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
- }
-
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
- }
+ int files[] = {SEMANAGE_STORE_KERNEL,
+ SEMANAGE_STORE_FC,
+ SEMANAGE_STORE_SEUSERS,
+ SEMANAGE_LINKED,
+ SEMANAGE_SEUSERS_LINKED,
+ SEMANAGE_USERS_EXTRA_LINKED};
+
+ for (i = 0; i < (int) sizeof(files); i++) {
+ path = semanage_path(SEMANAGE_TMP, files[i]);
+ if (stat(path, &sb) != 0) {
+ if (errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ retval = -1;
+ goto cleanup;
+ }
- path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
- if (access(path, F_OK) != 0) {
- do_rebuild = 1;
- goto rebuild;
+ do_rebuild = 1;
+ goto rebuild;
+ }
}
}
@@ -1465,7 +1476,7 @@ rebuild:
goto cleanup;
path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
- if (access(path, F_OK) == 0) {
+ if (stat(path, &sb) == 0) {
retval = semanage_copy_file(path,
semanage_path(SEMANAGE_TMP,
SEMANAGE_STORE_SEUSERS),
@@ -1473,12 +1484,17 @@ rebuild:
if (retval < 0)
goto cleanup;
pseusers->dtable->drop_cache(pseusers->dbase);
- } else {
+ } else if (errno == ENOENT) {
+ /* The file does not exist */
pseusers->dtable->clear(sh, pseusers->dbase);
+ } else {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ retval = -1;
+ goto cleanup;
}
path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
- if (access(path, F_OK) == 0) {
+ if (stat(path, &sb) == 0) {
retval = semanage_copy_file(path,
semanage_path(SEMANAGE_TMP,
SEMANAGE_USERS_EXTRA),
@@ -1486,8 +1502,13 @@ rebuild:
if (retval < 0)
goto cleanup;
pusers_extra->dtable->drop_cache(pusers_extra->dbase);
- } else {
+ } else if (errno == ENOENT) {
+ /* The file does not exist */
pusers_extra->dtable->clear(sh, pusers_extra->dbase);
+ } else {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ retval = -1;
+ goto cleanup;
}
}
@@ -1817,6 +1838,7 @@ static int semanage_direct_extract(semanage_handle_t * sh,
ssize_t _data_len;
char *_data;
int compressed;
+ struct stat sb;
/* get path of module */
rc = semanage_module_get_path(
@@ -1829,8 +1851,8 @@ static int semanage_direct_extract(semanage_handle_t * sh,
goto cleanup;
}
- if (access(module_path, F_OK) != 0) {
- ERR(sh, "Module does not exist: %s", module_path);
+ if (stat(module_path, &sb) != 0) {
+ ERR(sh, "Unable to access %s: %s\n", module_path, strerror(errno));
rc = -1;
goto cleanup;
}
@@ -1859,7 +1881,13 @@ static int semanage_direct_extract(semanage_handle_t * sh,
goto cleanup;
}
- if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && access(input_file, F_OK) != 0) {
+ if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && stat(input_file, &sb) != 0) {
+ if (errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", input_file, strerror(errno));
+ rc = -1;
+ goto cleanup;
+ }
+
rc = semanage_compile_module(sh, _modinfo);
if (rc < 0) {
goto cleanup;
@@ -2004,6 +2032,12 @@ static int semanage_direct_get_enabled(semanage_handle_t *sh,
}
if (stat(path, &sb) < 0) {
+ if (errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ status = -1;
+ goto cleanup;
+ }
+
*enabled = 1;
}
else {
@@ -2330,6 +2364,12 @@ static int semanage_direct_get_module_info(semanage_handle_t *sh,
/* set enabled/disabled status */
if (stat(fn, &sb) < 0) {
+ if (errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", fn, strerror(errno));
+ status = -1;
+ goto cleanup;
+ }
+
ret = semanage_module_info_set_enabled(sh, *modinfo, 1);
if (ret != 0) {
status = -1;
@@ -2738,6 +2778,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
int status = 0;
int ret = 0;
int type;
+ struct stat sb;
char path[PATH_MAX];
mode_t mask = umask(0077);
@@ -2838,7 +2879,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
goto cleanup;
}
- if (access(path, F_OK) == 0) {
+ if (stat(path, &sb) == 0) {
ret = unlink(path);
if (ret != 0) {
ERR(sh, "Error while removing cached CIL file %s: %s", path, strerror(errno));
diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c
index 4bd1d651..14ad99c1 100644
--- a/libsemanage/src/semanage_store.c
+++ b/libsemanage/src/semanage_store.c
@@ -514,6 +514,7 @@ char *semanage_conf_path(void)
{
char *semanage_conf = NULL;
int len;
+ struct stat sb;
len = strlen(semanage_root()) + strlen(selinux_path()) + strlen(SEMANAGE_CONF_FILE);
semanage_conf = calloc(len + 1, sizeof(char));
@@ -522,7 +523,7 @@ char *semanage_conf_path(void)
snprintf(semanage_conf, len + 1, "%s%s%s", semanage_root(), selinux_path(),
SEMANAGE_CONF_FILE);
- if (access(semanage_conf, R_OK) != 0) {
+ if (stat(semanage_conf, &sb) != 0 && errno == ENOENT) {
snprintf(semanage_conf, len + 1, "%s%s", selinux_path(), SEMANAGE_CONF_FILE);
}
@@ -1508,8 +1509,14 @@ int semanage_split_fc(semanage_handle_t * sh)
static int sefcontext_compile(semanage_handle_t * sh, const char *path) {
int r;
+ struct stat sb;
+
+ if (stat(path, &sb) < 0) {
+ if (errno != ENOENT) {
+ ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
+ return -1;
+ }
- if (access(path, F_OK) != 0) {
return 0;
}
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] libsemanage: replace access() checks to make setuid programs work
2018-03-09 15:39 ` Vit Mojzis
@ 2018-03-09 15:51 ` Stephen Smalley
2018-03-13 10:58 ` Petr Lautrbach
2018-03-17 7:26 ` Petr Lautrbach
1 sibling, 1 reply; 19+ messages in thread
From: Stephen Smalley @ 2018-03-09 15:51 UTC (permalink / raw)
To: Vit Mojzis, selinux
On 03/09/2018 10:39 AM, Vit Mojzis wrote:
> access() uses real UID instead of effective UID which causes false
> negative checks in setuid programs.
> Replace access() calls (mostly tests for file existence) by stat().
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
Thanks, I've put this up as a PR for testing here:
https://github.com/SELinuxProject/selinux/pull/84
I won't be around next week so someone else can merge it or I will get to it when I return.
>
> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
> ---
> libsemanage/src/direct_api.c | 137 +++++++++++++++++++++++++--------------
> libsemanage/src/semanage_store.c | 11 +++-
> 2 files changed, 98 insertions(+), 50 deletions(-)
>
> diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
> index 92d7517d..439122df 100644
> --- a/libsemanage/src/direct_api.c
> +++ b/libsemanage/src/direct_api.c
> @@ -140,6 +140,7 @@ int semanage_direct_is_managed(semanage_handle_t * sh)
> int semanage_direct_connect(semanage_handle_t * sh)
> {
> const char *path;
> + struct stat sb;
>
> if (semanage_check_init(sh, sh->conf->store_root_path))
> goto err;
> @@ -302,10 +303,16 @@ int semanage_direct_connect(semanage_handle_t * sh)
>
> /* set the disable dontaudit value */
> path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_DISABLE_DONTAUDIT);
> - if (access(path, F_OK) == 0)
> +
> + if (stat(path, &sb) == 0)
> sepol_set_disable_dontaudit(sh->sepolh, 1);
> - else
> + else if (errno == ENOENT) {
> + /* The file does not exist */
> sepol_set_disable_dontaudit(sh->sepolh, 0);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + goto err;
> + }
>
> return STATUS_SUCCESS;
>
> @@ -1139,6 +1146,7 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
> int status = 0;
> int i;
> char cil_path[PATH_MAX];
> + struct stat sb;
>
> assert(sh);
> assert(modinfos);
> @@ -1155,9 +1163,13 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
> }
>
> if (semanage_get_ignore_module_cache(sh) == 0 &&
> - access(cil_path, F_OK) == 0) {
> + (status = stat(cil_path, &sb)) == 0) {
> continue;
> }
> + if (status != 0 && errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", cil_path, strerror(errno));
> + goto cleanup; //an error in the "stat" call
> + }
>
> status = semanage_compile_module(sh, &modinfos[i]);
> if (status < 0) {
> @@ -1196,6 +1208,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
> struct cil_db *cildb = NULL;
> semanage_module_info_t *modinfos = NULL;
> mode_t mask = umask(0077);
> + struct stat sb;
>
> int do_rebuild, do_write_kernel, do_install;
> int fcontexts_modified, ports_modified, seusers_modified,
> @@ -1234,10 +1247,16 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>
> /* Create or remove the disable_dontaudit flag file. */
> path = semanage_path(SEMANAGE_TMP, SEMANAGE_DISABLE_DONTAUDIT);
> - if (access(path, F_OK) == 0)
> + if (stat(path, &sb) == 0)
> do_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1);
> - else
> + else if (errno == ENOENT) {
> + /* The file does not exist */
> do_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + retval = -1;
> + goto cleanup;
> + }
> if (sepol_get_disable_dontaudit(sh->sepolh) == 1) {
> FILE *touch;
> touch = fopen(path, "w");
> @@ -1259,10 +1278,17 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>
> /* Create or remove the preserve_tunables flag file. */
> path = semanage_path(SEMANAGE_TMP, SEMANAGE_PRESERVE_TUNABLES);
> - if (access(path, F_OK) == 0)
> + if (stat(path, &sb) == 0)
> do_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1);
> - else
> + else if (errno == ENOENT) {
> + /* The file does not exist */
> do_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + retval = -1;
> + goto cleanup;
> + }
> +
> if (sepol_get_preserve_tunables(sh->sepolh) == 1) {
> FILE *touch;
> touch = fopen(path, "w");
> @@ -1299,40 +1325,25 @@ static int semanage_direct_commit(semanage_handle_t * sh)
> * a rebuild.
> */
> if (!do_rebuild) {
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_KERNEL);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> -
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> -
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> -
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_LINKED);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> -
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> + int files[] = {SEMANAGE_STORE_KERNEL,
> + SEMANAGE_STORE_FC,
> + SEMANAGE_STORE_SEUSERS,
> + SEMANAGE_LINKED,
> + SEMANAGE_SEUSERS_LINKED,
> + SEMANAGE_USERS_EXTRA_LINKED};
> +
> + for (i = 0; i < (int) sizeof(files); i++) {
> + path = semanage_path(SEMANAGE_TMP, files[i]);
> + if (stat(path, &sb) != 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + retval = -1;
> + goto cleanup;
> + }
>
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> + do_rebuild = 1;
> + goto rebuild;
> + }
> }
> }
>
> @@ -1465,7 +1476,7 @@ rebuild:
> goto cleanup;
>
> path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
> - if (access(path, F_OK) == 0) {
> + if (stat(path, &sb) == 0) {
> retval = semanage_copy_file(path,
> semanage_path(SEMANAGE_TMP,
> SEMANAGE_STORE_SEUSERS),
> @@ -1473,12 +1484,17 @@ rebuild:
> if (retval < 0)
> goto cleanup;
> pseusers->dtable->drop_cache(pseusers->dbase);
> - } else {
> + } else if (errno == ENOENT) {
> + /* The file does not exist */
> pseusers->dtable->clear(sh, pseusers->dbase);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + retval = -1;
> + goto cleanup;
> }
>
> path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
> - if (access(path, F_OK) == 0) {
> + if (stat(path, &sb) == 0) {
> retval = semanage_copy_file(path,
> semanage_path(SEMANAGE_TMP,
> SEMANAGE_USERS_EXTRA),
> @@ -1486,8 +1502,13 @@ rebuild:
> if (retval < 0)
> goto cleanup;
> pusers_extra->dtable->drop_cache(pusers_extra->dbase);
> - } else {
> + } else if (errno == ENOENT) {
> + /* The file does not exist */
> pusers_extra->dtable->clear(sh, pusers_extra->dbase);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + retval = -1;
> + goto cleanup;
> }
> }
>
> @@ -1817,6 +1838,7 @@ static int semanage_direct_extract(semanage_handle_t * sh,
> ssize_t _data_len;
> char *_data;
> int compressed;
> + struct stat sb;
>
> /* get path of module */
> rc = semanage_module_get_path(
> @@ -1829,8 +1851,8 @@ static int semanage_direct_extract(semanage_handle_t * sh,
> goto cleanup;
> }
>
> - if (access(module_path, F_OK) != 0) {
> - ERR(sh, "Module does not exist: %s", module_path);
> + if (stat(module_path, &sb) != 0) {
> + ERR(sh, "Unable to access %s: %s\n", module_path, strerror(errno));
> rc = -1;
> goto cleanup;
> }
> @@ -1859,7 +1881,13 @@ static int semanage_direct_extract(semanage_handle_t * sh,
> goto cleanup;
> }
>
> - if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && access(input_file, F_OK) != 0) {
> + if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && stat(input_file, &sb) != 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", input_file, strerror(errno));
> + rc = -1;
> + goto cleanup;
> + }
> +
> rc = semanage_compile_module(sh, _modinfo);
> if (rc < 0) {
> goto cleanup;
> @@ -2004,6 +2032,12 @@ static int semanage_direct_get_enabled(semanage_handle_t *sh,
> }
>
> if (stat(path, &sb) < 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + status = -1;
> + goto cleanup;
> + }
> +
> *enabled = 1;
> }
> else {
> @@ -2330,6 +2364,12 @@ static int semanage_direct_get_module_info(semanage_handle_t *sh,
>
> /* set enabled/disabled status */
> if (stat(fn, &sb) < 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", fn, strerror(errno));
> + status = -1;
> + goto cleanup;
> + }
> +
> ret = semanage_module_info_set_enabled(sh, *modinfo, 1);
> if (ret != 0) {
> status = -1;
> @@ -2738,6 +2778,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
> int status = 0;
> int ret = 0;
> int type;
> + struct stat sb;
>
> char path[PATH_MAX];
> mode_t mask = umask(0077);
> @@ -2838,7 +2879,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
> goto cleanup;
> }
>
> - if (access(path, F_OK) == 0) {
> + if (stat(path, &sb) == 0) {
> ret = unlink(path);
> if (ret != 0) {
> ERR(sh, "Error while removing cached CIL file %s: %s", path, strerror(errno));
> diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c
> index 4bd1d651..14ad99c1 100644
> --- a/libsemanage/src/semanage_store.c
> +++ b/libsemanage/src/semanage_store.c
> @@ -514,6 +514,7 @@ char *semanage_conf_path(void)
> {
> char *semanage_conf = NULL;
> int len;
> + struct stat sb;
>
> len = strlen(semanage_root()) + strlen(selinux_path()) + strlen(SEMANAGE_CONF_FILE);
> semanage_conf = calloc(len + 1, sizeof(char));
> @@ -522,7 +523,7 @@ char *semanage_conf_path(void)
> snprintf(semanage_conf, len + 1, "%s%s%s", semanage_root(), selinux_path(),
> SEMANAGE_CONF_FILE);
>
> - if (access(semanage_conf, R_OK) != 0) {
> + if (stat(semanage_conf, &sb) != 0 && errno == ENOENT) {
> snprintf(semanage_conf, len + 1, "%s%s", selinux_path(), SEMANAGE_CONF_FILE);
> }
>
> @@ -1508,8 +1509,14 @@ int semanage_split_fc(semanage_handle_t * sh)
> static int sefcontext_compile(semanage_handle_t * sh, const char *path) {
>
> int r;
> + struct stat sb;
> +
> + if (stat(path, &sb) < 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + return -1;
> + }
>
> - if (access(path, F_OK) != 0) {
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] libsemanage: replace access() checks to make setuid programs work
2018-03-09 15:51 ` Stephen Smalley
@ 2018-03-13 10:58 ` Petr Lautrbach
0 siblings, 0 replies; 19+ messages in thread
From: Petr Lautrbach @ 2018-03-13 10:58 UTC (permalink / raw)
To: Stephen Smalley; +Cc: Vit Mojzis, selinux
[-- Attachment #1: Type: text/plain, Size: 11950 bytes --]
On Fri, Mar 09, 2018 at 10:51:20AM -0500, Stephen Smalley wrote:
> On 03/09/2018 10:39 AM, Vit Mojzis wrote:
> > access() uses real UID instead of effective UID which causes false
> > negative checks in setuid programs.
> > Replace access() calls (mostly tests for file existence) by stat().
> >
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
>
> Thanks, I've put this up as a PR for testing here:
> https://github.com/SELinuxProject/selinux/pull/84
>
> I won't be around next week so someone else can merge it or I will get to it when I return.
This is merged now. Thanks!
> >
> > Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
> > ---
> > libsemanage/src/direct_api.c | 137 +++++++++++++++++++++++++--------------
> > libsemanage/src/semanage_store.c | 11 +++-
> > 2 files changed, 98 insertions(+), 50 deletions(-)
> >
> > diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
> > index 92d7517d..439122df 100644
> > --- a/libsemanage/src/direct_api.c
> > +++ b/libsemanage/src/direct_api.c
> > @@ -140,6 +140,7 @@ int semanage_direct_is_managed(semanage_handle_t * sh)
> > int semanage_direct_connect(semanage_handle_t * sh)
> > {
> > const char *path;
> > + struct stat sb;
> >
> > if (semanage_check_init(sh, sh->conf->store_root_path))
> > goto err;
> > @@ -302,10 +303,16 @@ int semanage_direct_connect(semanage_handle_t * sh)
> >
> > /* set the disable dontaudit value */
> > path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_DISABLE_DONTAUDIT);
> > - if (access(path, F_OK) == 0)
> > +
> > + if (stat(path, &sb) == 0)
> > sepol_set_disable_dontaudit(sh->sepolh, 1);
> > - else
> > + else if (errno == ENOENT) {
> > + /* The file does not exist */
> > sepol_set_disable_dontaudit(sh->sepolh, 0);
> > + } else {
> > + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> > + goto err;
> > + }
> >
> > return STATUS_SUCCESS;
> >
> > @@ -1139,6 +1146,7 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
> > int status = 0;
> > int i;
> > char cil_path[PATH_MAX];
> > + struct stat sb;
> >
> > assert(sh);
> > assert(modinfos);
> > @@ -1155,9 +1163,13 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
> > }
> >
> > if (semanage_get_ignore_module_cache(sh) == 0 &&
> > - access(cil_path, F_OK) == 0) {
> > + (status = stat(cil_path, &sb)) == 0) {
> > continue;
> > }
> > + if (status != 0 && errno != ENOENT) {
> > + ERR(sh, "Unable to access %s: %s\n", cil_path, strerror(errno));
> > + goto cleanup; //an error in the "stat" call
> > + }
> >
> > status = semanage_compile_module(sh, &modinfos[i]);
> > if (status < 0) {
> > @@ -1196,6 +1208,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
> > struct cil_db *cildb = NULL;
> > semanage_module_info_t *modinfos = NULL;
> > mode_t mask = umask(0077);
> > + struct stat sb;
> >
> > int do_rebuild, do_write_kernel, do_install;
> > int fcontexts_modified, ports_modified, seusers_modified,
> > @@ -1234,10 +1247,16 @@ static int semanage_direct_commit(semanage_handle_t * sh)
> >
> > /* Create or remove the disable_dontaudit flag file. */
> > path = semanage_path(SEMANAGE_TMP, SEMANAGE_DISABLE_DONTAUDIT);
> > - if (access(path, F_OK) == 0)
> > + if (stat(path, &sb) == 0)
> > do_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1);
> > - else
> > + else if (errno == ENOENT) {
> > + /* The file does not exist */
> > do_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1);
> > + } else {
> > + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> > + retval = -1;
> > + goto cleanup;
> > + }
> > if (sepol_get_disable_dontaudit(sh->sepolh) == 1) {
> > FILE *touch;
> > touch = fopen(path, "w");
> > @@ -1259,10 +1278,17 @@ static int semanage_direct_commit(semanage_handle_t * sh)
> >
> > /* Create or remove the preserve_tunables flag file. */
> > path = semanage_path(SEMANAGE_TMP, SEMANAGE_PRESERVE_TUNABLES);
> > - if (access(path, F_OK) == 0)
> > + if (stat(path, &sb) == 0)
> > do_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1);
> > - else
> > + else if (errno == ENOENT) {
> > + /* The file does not exist */
> > do_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1);
> > + } else {
> > + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> > + retval = -1;
> > + goto cleanup;
> > + }
> > +
> > if (sepol_get_preserve_tunables(sh->sepolh) == 1) {
> > FILE *touch;
> > touch = fopen(path, "w");
> > @@ -1299,40 +1325,25 @@ static int semanage_direct_commit(semanage_handle_t * sh)
> > * a rebuild.
> > */
> > if (!do_rebuild) {
> > - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_KERNEL);
> > - if (access(path, F_OK) != 0) {
> > - do_rebuild = 1;
> > - goto rebuild;
> > - }
> > -
> > - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
> > - if (access(path, F_OK) != 0) {
> > - do_rebuild = 1;
> > - goto rebuild;
> > - }
> > -
> > - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
> > - if (access(path, F_OK) != 0) {
> > - do_rebuild = 1;
> > - goto rebuild;
> > - }
> > -
> > - path = semanage_path(SEMANAGE_TMP, SEMANAGE_LINKED);
> > - if (access(path, F_OK) != 0) {
> > - do_rebuild = 1;
> > - goto rebuild;
> > - }
> > -
> > - path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
> > - if (access(path, F_OK) != 0) {
> > - do_rebuild = 1;
> > - goto rebuild;
> > - }
> > + int files[] = {SEMANAGE_STORE_KERNEL,
> > + SEMANAGE_STORE_FC,
> > + SEMANAGE_STORE_SEUSERS,
> > + SEMANAGE_LINKED,
> > + SEMANAGE_SEUSERS_LINKED,
> > + SEMANAGE_USERS_EXTRA_LINKED};
> > +
> > + for (i = 0; i < (int) sizeof(files); i++) {
> > + path = semanage_path(SEMANAGE_TMP, files[i]);
> > + if (stat(path, &sb) != 0) {
> > + if (errno != ENOENT) {
> > + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> > + retval = -1;
> > + goto cleanup;
> > + }
> >
> > - path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
> > - if (access(path, F_OK) != 0) {
> > - do_rebuild = 1;
> > - goto rebuild;
> > + do_rebuild = 1;
> > + goto rebuild;
> > + }
> > }
> > }
> >
> > @@ -1465,7 +1476,7 @@ rebuild:
> > goto cleanup;
> >
> > path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
> > - if (access(path, F_OK) == 0) {
> > + if (stat(path, &sb) == 0) {
> > retval = semanage_copy_file(path,
> > semanage_path(SEMANAGE_TMP,
> > SEMANAGE_STORE_SEUSERS),
> > @@ -1473,12 +1484,17 @@ rebuild:
> > if (retval < 0)
> > goto cleanup;
> > pseusers->dtable->drop_cache(pseusers->dbase);
> > - } else {
> > + } else if (errno == ENOENT) {
> > + /* The file does not exist */
> > pseusers->dtable->clear(sh, pseusers->dbase);
> > + } else {
> > + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> > + retval = -1;
> > + goto cleanup;
> > }
> >
> > path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
> > - if (access(path, F_OK) == 0) {
> > + if (stat(path, &sb) == 0) {
> > retval = semanage_copy_file(path,
> > semanage_path(SEMANAGE_TMP,
> > SEMANAGE_USERS_EXTRA),
> > @@ -1486,8 +1502,13 @@ rebuild:
> > if (retval < 0)
> > goto cleanup;
> > pusers_extra->dtable->drop_cache(pusers_extra->dbase);
> > - } else {
> > + } else if (errno == ENOENT) {
> > + /* The file does not exist */
> > pusers_extra->dtable->clear(sh, pusers_extra->dbase);
> > + } else {
> > + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> > + retval = -1;
> > + goto cleanup;
> > }
> > }
> >
> > @@ -1817,6 +1838,7 @@ static int semanage_direct_extract(semanage_handle_t * sh,
> > ssize_t _data_len;
> > char *_data;
> > int compressed;
> > + struct stat sb;
> >
> > /* get path of module */
> > rc = semanage_module_get_path(
> > @@ -1829,8 +1851,8 @@ static int semanage_direct_extract(semanage_handle_t * sh,
> > goto cleanup;
> > }
> >
> > - if (access(module_path, F_OK) != 0) {
> > - ERR(sh, "Module does not exist: %s", module_path);
> > + if (stat(module_path, &sb) != 0) {
> > + ERR(sh, "Unable to access %s: %s\n", module_path, strerror(errno));
> > rc = -1;
> > goto cleanup;
> > }
> > @@ -1859,7 +1881,13 @@ static int semanage_direct_extract(semanage_handle_t * sh,
> > goto cleanup;
> > }
> >
> > - if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && access(input_file, F_OK) != 0) {
> > + if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && stat(input_file, &sb) != 0) {
> > + if (errno != ENOENT) {
> > + ERR(sh, "Unable to access %s: %s\n", input_file, strerror(errno));
> > + rc = -1;
> > + goto cleanup;
> > + }
> > +
> > rc = semanage_compile_module(sh, _modinfo);
> > if (rc < 0) {
> > goto cleanup;
> > @@ -2004,6 +2032,12 @@ static int semanage_direct_get_enabled(semanage_handle_t *sh,
> > }
> >
> > if (stat(path, &sb) < 0) {
> > + if (errno != ENOENT) {
> > + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> > + status = -1;
> > + goto cleanup;
> > + }
> > +
> > *enabled = 1;
> > }
> > else {
> > @@ -2330,6 +2364,12 @@ static int semanage_direct_get_module_info(semanage_handle_t *sh,
> >
> > /* set enabled/disabled status */
> > if (stat(fn, &sb) < 0) {
> > + if (errno != ENOENT) {
> > + ERR(sh, "Unable to access %s: %s\n", fn, strerror(errno));
> > + status = -1;
> > + goto cleanup;
> > + }
> > +
> > ret = semanage_module_info_set_enabled(sh, *modinfo, 1);
> > if (ret != 0) {
> > status = -1;
> > @@ -2738,6 +2778,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
> > int status = 0;
> > int ret = 0;
> > int type;
> > + struct stat sb;
> >
> > char path[PATH_MAX];
> > mode_t mask = umask(0077);
> > @@ -2838,7 +2879,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
> > goto cleanup;
> > }
> >
> > - if (access(path, F_OK) == 0) {
> > + if (stat(path, &sb) == 0) {
> > ret = unlink(path);
> > if (ret != 0) {
> > ERR(sh, "Error while removing cached CIL file %s: %s", path, strerror(errno));
> > diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c
> > index 4bd1d651..14ad99c1 100644
> > --- a/libsemanage/src/semanage_store.c
> > +++ b/libsemanage/src/semanage_store.c
> > @@ -514,6 +514,7 @@ char *semanage_conf_path(void)
> > {
> > char *semanage_conf = NULL;
> > int len;
> > + struct stat sb;
> >
> > len = strlen(semanage_root()) + strlen(selinux_path()) + strlen(SEMANAGE_CONF_FILE);
> > semanage_conf = calloc(len + 1, sizeof(char));
> > @@ -522,7 +523,7 @@ char *semanage_conf_path(void)
> > snprintf(semanage_conf, len + 1, "%s%s%s", semanage_root(), selinux_path(),
> > SEMANAGE_CONF_FILE);
> >
> > - if (access(semanage_conf, R_OK) != 0) {
> > + if (stat(semanage_conf, &sb) != 0 && errno == ENOENT) {
> > snprintf(semanage_conf, len + 1, "%s%s", selinux_path(), SEMANAGE_CONF_FILE);
> > }
> >
> > @@ -1508,8 +1509,14 @@ int semanage_split_fc(semanage_handle_t * sh)
> > static int sefcontext_compile(semanage_handle_t * sh, const char *path) {
> >
> > int r;
> > + struct stat sb;
> > +
> > + if (stat(path, &sb) < 0) {
> > + if (errno != ENOENT) {
> > + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> > + return -1;
> > + }
> >
> > - if (access(path, F_OK) != 0) {
> > return 0;
> > }
> >
> >
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] libsemanage: replace access() checks to make setuid programs work
2018-03-09 15:39 ` Vit Mojzis
2018-03-09 15:51 ` Stephen Smalley
@ 2018-03-17 7:26 ` Petr Lautrbach
2018-03-19 14:46 ` [PATCH] libsemanage/direct_api.c: Fix iterating over array Vit Mojzis
1 sibling, 1 reply; 19+ messages in thread
From: Petr Lautrbach @ 2018-03-17 7:26 UTC (permalink / raw)
To: Vit Mojzis; +Cc: selinux
[-- Attachment #1: Type: text/plain, Size: 11775 bytes --]
On Fri, Mar 09, 2018 at 04:39:44PM +0100, Vit Mojzis wrote:
> access() uses real UID instead of effective UID which causes false
> negative checks in setuid programs.
> Replace access() calls (mostly tests for file existence) by stat().
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1186431
>
> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
> ---
> libsemanage/src/direct_api.c | 137 +++++++++++++++++++++++++--------------
> libsemanage/src/semanage_store.c | 11 +++-
> 2 files changed, 98 insertions(+), 50 deletions(-)
>
> diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
> index 92d7517d..439122df 100644
> --- a/libsemanage/src/direct_api.c
> +++ b/libsemanage/src/direct_api.c
> @@ -140,6 +140,7 @@ int semanage_direct_is_managed(semanage_handle_t * sh)
> int semanage_direct_connect(semanage_handle_t * sh)
> {
> const char *path;
> + struct stat sb;
>
> if (semanage_check_init(sh, sh->conf->store_root_path))
> goto err;
> @@ -302,10 +303,16 @@ int semanage_direct_connect(semanage_handle_t * sh)
>
> /* set the disable dontaudit value */
> path = semanage_path(SEMANAGE_ACTIVE, SEMANAGE_DISABLE_DONTAUDIT);
> - if (access(path, F_OK) == 0)
> +
> + if (stat(path, &sb) == 0)
> sepol_set_disable_dontaudit(sh->sepolh, 1);
> - else
> + else if (errno == ENOENT) {
> + /* The file does not exist */
> sepol_set_disable_dontaudit(sh->sepolh, 0);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + goto err;
> + }
>
> return STATUS_SUCCESS;
>
> @@ -1139,6 +1146,7 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
> int status = 0;
> int i;
> char cil_path[PATH_MAX];
> + struct stat sb;
>
> assert(sh);
> assert(modinfos);
> @@ -1155,9 +1163,13 @@ static int semanage_compile_hll_modules(semanage_handle_t *sh,
> }
>
> if (semanage_get_ignore_module_cache(sh) == 0 &&
> - access(cil_path, F_OK) == 0) {
> + (status = stat(cil_path, &sb)) == 0) {
> continue;
> }
> + if (status != 0 && errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", cil_path, strerror(errno));
> + goto cleanup; //an error in the "stat" call
> + }
>
> status = semanage_compile_module(sh, &modinfos[i]);
> if (status < 0) {
> @@ -1196,6 +1208,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
> struct cil_db *cildb = NULL;
> semanage_module_info_t *modinfos = NULL;
> mode_t mask = umask(0077);
> + struct stat sb;
>
> int do_rebuild, do_write_kernel, do_install;
> int fcontexts_modified, ports_modified, seusers_modified,
> @@ -1234,10 +1247,16 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>
> /* Create or remove the disable_dontaudit flag file. */
> path = semanage_path(SEMANAGE_TMP, SEMANAGE_DISABLE_DONTAUDIT);
> - if (access(path, F_OK) == 0)
> + if (stat(path, &sb) == 0)
> do_rebuild |= !(sepol_get_disable_dontaudit(sh->sepolh) == 1);
> - else
> + else if (errno == ENOENT) {
> + /* The file does not exist */
> do_rebuild |= (sepol_get_disable_dontaudit(sh->sepolh) == 1);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + retval = -1;
> + goto cleanup;
> + }
> if (sepol_get_disable_dontaudit(sh->sepolh) == 1) {
> FILE *touch;
> touch = fopen(path, "w");
> @@ -1259,10 +1278,17 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>
> /* Create or remove the preserve_tunables flag file. */
> path = semanage_path(SEMANAGE_TMP, SEMANAGE_PRESERVE_TUNABLES);
> - if (access(path, F_OK) == 0)
> + if (stat(path, &sb) == 0)
> do_rebuild |= !(sepol_get_preserve_tunables(sh->sepolh) == 1);
> - else
> + else if (errno == ENOENT) {
> + /* The file does not exist */
> do_rebuild |= (sepol_get_preserve_tunables(sh->sepolh) == 1);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + retval = -1;
> + goto cleanup;
> + }
> +
> if (sepol_get_preserve_tunables(sh->sepolh) == 1) {
> FILE *touch;
> touch = fopen(path, "w");
> @@ -1299,40 +1325,25 @@ static int semanage_direct_commit(semanage_handle_t * sh)
> * a rebuild.
> */
> if (!do_rebuild) {
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_KERNEL);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> -
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_FC);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> -
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_STORE_SEUSERS);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> -
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_LINKED);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> -
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> - }
> + int files[] = {SEMANAGE_STORE_KERNEL,
> + SEMANAGE_STORE_FC,
> + SEMANAGE_STORE_SEUSERS,
> + SEMANAGE_LINKED,
> + SEMANAGE_SEUSERS_LINKED,
> + SEMANAGE_USERS_EXTRA_LINKED};
> +
> + for (i = 0; i < (int) sizeof(files); i++) {
The size of the files needs to be divided by size of int, otherwise it causes segfault:
# semanage fcontext -m -t httpd_log_t "/var/opt/quest/vas/vasd(/
[1] 3063 segmentation fault (core dumped) semanage fcontext -m -t httpd_log_t "/var/opt/quest/vas/vasd(/.*)?"
backtrace:
#0 0x00007fffe7901f9b in semanage_path (store=store@entry=SEMANAGE_TMP, path_name=3918632424) at semanage_store.c:487
#1 0x00007fffe78f4be1 in semanage_direct_commit (sh=0x55555840da00) at direct_api.c:1336
#2 0x00007fffe78f9f71 in semanage_commit (sh=0x55555840da00) at handle.c:428
Something like this should fix it:
#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
for (i = 0; i < ARRAY_SIZE(files); i++) {
...
> + path = semanage_path(SEMANAGE_TMP, files[i]);
> + if (stat(path, &sb) != 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + retval = -1;
> + goto cleanup;
> + }
>
> - path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
> - if (access(path, F_OK) != 0) {
> - do_rebuild = 1;
> - goto rebuild;
> + do_rebuild = 1;
> + goto rebuild;
> + }
> }
> }
>
> @@ -1465,7 +1476,7 @@ rebuild:
> goto cleanup;
>
> path = semanage_path(SEMANAGE_TMP, SEMANAGE_SEUSERS_LINKED);
> - if (access(path, F_OK) == 0) {
> + if (stat(path, &sb) == 0) {
> retval = semanage_copy_file(path,
> semanage_path(SEMANAGE_TMP,
> SEMANAGE_STORE_SEUSERS),
> @@ -1473,12 +1484,17 @@ rebuild:
> if (retval < 0)
> goto cleanup;
> pseusers->dtable->drop_cache(pseusers->dbase);
> - } else {
> + } else if (errno == ENOENT) {
> + /* The file does not exist */
> pseusers->dtable->clear(sh, pseusers->dbase);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + retval = -1;
> + goto cleanup;
> }
>
> path = semanage_path(SEMANAGE_TMP, SEMANAGE_USERS_EXTRA_LINKED);
> - if (access(path, F_OK) == 0) {
> + if (stat(path, &sb) == 0) {
> retval = semanage_copy_file(path,
> semanage_path(SEMANAGE_TMP,
> SEMANAGE_USERS_EXTRA),
> @@ -1486,8 +1502,13 @@ rebuild:
> if (retval < 0)
> goto cleanup;
> pusers_extra->dtable->drop_cache(pusers_extra->dbase);
> - } else {
> + } else if (errno == ENOENT) {
> + /* The file does not exist */
> pusers_extra->dtable->clear(sh, pusers_extra->dbase);
> + } else {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + retval = -1;
> + goto cleanup;
> }
> }
>
> @@ -1817,6 +1838,7 @@ static int semanage_direct_extract(semanage_handle_t * sh,
> ssize_t _data_len;
> char *_data;
> int compressed;
> + struct stat sb;
>
> /* get path of module */
> rc = semanage_module_get_path(
> @@ -1829,8 +1851,8 @@ static int semanage_direct_extract(semanage_handle_t * sh,
> goto cleanup;
> }
>
> - if (access(module_path, F_OK) != 0) {
> - ERR(sh, "Module does not exist: %s", module_path);
> + if (stat(module_path, &sb) != 0) {
> + ERR(sh, "Unable to access %s: %s\n", module_path, strerror(errno));
> rc = -1;
> goto cleanup;
> }
> @@ -1859,7 +1881,13 @@ static int semanage_direct_extract(semanage_handle_t * sh,
> goto cleanup;
> }
>
> - if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && access(input_file, F_OK) != 0) {
> + if (extract_cil == 1 && strcmp(_modinfo->lang_ext, "cil") && stat(input_file, &sb) != 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", input_file, strerror(errno));
> + rc = -1;
> + goto cleanup;
> + }
> +
> rc = semanage_compile_module(sh, _modinfo);
> if (rc < 0) {
> goto cleanup;
> @@ -2004,6 +2032,12 @@ static int semanage_direct_get_enabled(semanage_handle_t *sh,
> }
>
> if (stat(path, &sb) < 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + status = -1;
> + goto cleanup;
> + }
> +
> *enabled = 1;
> }
> else {
> @@ -2330,6 +2364,12 @@ static int semanage_direct_get_module_info(semanage_handle_t *sh,
>
> /* set enabled/disabled status */
> if (stat(fn, &sb) < 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", fn, strerror(errno));
> + status = -1;
> + goto cleanup;
> + }
> +
> ret = semanage_module_info_set_enabled(sh, *modinfo, 1);
> if (ret != 0) {
> status = -1;
> @@ -2738,6 +2778,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
> int status = 0;
> int ret = 0;
> int type;
> + struct stat sb;
>
> char path[PATH_MAX];
> mode_t mask = umask(0077);
> @@ -2838,7 +2879,7 @@ static int semanage_direct_install_info(semanage_handle_t *sh,
> goto cleanup;
> }
>
> - if (access(path, F_OK) == 0) {
> + if (stat(path, &sb) == 0) {
> ret = unlink(path);
> if (ret != 0) {
> ERR(sh, "Error while removing cached CIL file %s: %s", path, strerror(errno));
> diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c
> index 4bd1d651..14ad99c1 100644
> --- a/libsemanage/src/semanage_store.c
> +++ b/libsemanage/src/semanage_store.c
> @@ -514,6 +514,7 @@ char *semanage_conf_path(void)
> {
> char *semanage_conf = NULL;
> int len;
> + struct stat sb;
>
> len = strlen(semanage_root()) + strlen(selinux_path()) + strlen(SEMANAGE_CONF_FILE);
> semanage_conf = calloc(len + 1, sizeof(char));
> @@ -522,7 +523,7 @@ char *semanage_conf_path(void)
> snprintf(semanage_conf, len + 1, "%s%s%s", semanage_root(), selinux_path(),
> SEMANAGE_CONF_FILE);
>
> - if (access(semanage_conf, R_OK) != 0) {
> + if (stat(semanage_conf, &sb) != 0 && errno == ENOENT) {
> snprintf(semanage_conf, len + 1, "%s%s", selinux_path(), SEMANAGE_CONF_FILE);
> }
>
> @@ -1508,8 +1509,14 @@ int semanage_split_fc(semanage_handle_t * sh)
> static int sefcontext_compile(semanage_handle_t * sh, const char *path) {
>
> int r;
> + struct stat sb;
> +
> + if (stat(path, &sb) < 0) {
> + if (errno != ENOENT) {
> + ERR(sh, "Unable to access %s: %s\n", path, strerror(errno));
> + return -1;
> + }
>
> - if (access(path, F_OK) != 0) {
> return 0;
> }
>
> --
> 2.14.3
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] libsemanage/direct_api.c: Fix iterating over array
2018-03-17 7:26 ` Petr Lautrbach
@ 2018-03-19 14:46 ` Vit Mojzis
2018-03-19 15:19 ` William Roberts
0 siblings, 1 reply; 19+ messages in thread
From: Vit Mojzis @ 2018-03-19 14:46 UTC (permalink / raw)
To: selinux
Fix sizeof calculation in array iteration introduced by commit
6bb8282c4cf66e93daa9684dbe9c75bb6b1e09a7
"libsemanage: replace access() checks to make setuid programs work"
Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
---
libsemanage/src/direct_api.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index 439122df..e7ec952f 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -60,6 +60,7 @@
#define PIPE_READ 0
#define PIPE_WRITE 1
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
static void semanage_direct_destroy(semanage_handle_t * sh);
static int semanage_direct_disconnect(semanage_handle_t * sh);
@@ -1332,7 +1333,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
SEMANAGE_SEUSERS_LINKED,
SEMANAGE_USERS_EXTRA_LINKED};
- for (i = 0; i < (int) sizeof(files); i++) {
+ for (i = 0; i < (int) ARRAY_SIZE(files); i++) {
path = semanage_path(SEMANAGE_TMP, files[i]);
if (stat(path, &sb) != 0) {
if (errno != ENOENT) {
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] libsemanage/direct_api.c: Fix iterating over array
2018-03-19 14:46 ` [PATCH] libsemanage/direct_api.c: Fix iterating over array Vit Mojzis
@ 2018-03-19 15:19 ` William Roberts
2018-03-19 16:18 ` William Roberts
0 siblings, 1 reply; 19+ messages in thread
From: William Roberts @ 2018-03-19 15:19 UTC (permalink / raw)
To: Vit Mojzis; +Cc: selinux
On Mon, Mar 19, 2018 at 7:46 AM, Vit Mojzis <vmojzis@redhat.com> wrote:
> Fix sizeof calculation in array iteration introduced by commit
> 6bb8282c4cf66e93daa9684dbe9c75bb6b1e09a7
> "libsemanage: replace access() checks to make setuid programs work"
>
> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
> ---
> libsemanage/src/direct_api.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
> index 439122df..e7ec952f 100644
> --- a/libsemanage/src/direct_api.c
> +++ b/libsemanage/src/direct_api.c
> @@ -60,6 +60,7 @@
>
> #define PIPE_READ 0
> #define PIPE_WRITE 1
> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>
> static void semanage_direct_destroy(semanage_handle_t * sh);
> static int semanage_direct_disconnect(semanage_handle_t * sh);
> @@ -1332,7 +1333,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
> SEMANAGE_SEUSERS_LINKED,
> SEMANAGE_USERS_EXTRA_LINKED};
>
> - for (i = 0; i < (int) sizeof(files); i++) {
> + for (i = 0; i < (int) ARRAY_SIZE(files); i++) {
> path = semanage_path(SEMANAGE_TMP, files[i]);
> if (stat(path, &sb) != 0) {
> if (errno != ENOENT) {
> --
> 2.14.3
>
>
ack
Just noticing: that code has i as an int, which it probably should be size_t and
the files array as an array of int's, it should probably use the
enum type.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] libsemanage/direct_api.c: Fix iterating over array
2018-03-19 15:19 ` William Roberts
@ 2018-03-19 16:18 ` William Roberts
0 siblings, 0 replies; 19+ messages in thread
From: William Roberts @ 2018-03-19 16:18 UTC (permalink / raw)
To: Vit Mojzis; +Cc: selinux
On Mon, Mar 19, 2018 at 8:19 AM, William Roberts
<bill.c.roberts@gmail.com> wrote:
> On Mon, Mar 19, 2018 at 7:46 AM, Vit Mojzis <vmojzis@redhat.com> wrote:
>> Fix sizeof calculation in array iteration introduced by commit
>> 6bb8282c4cf66e93daa9684dbe9c75bb6b1e09a7
>> "libsemanage: replace access() checks to make setuid programs work"
>>
>> Signed-off-by: Vit Mojzis <vmojzis@redhat.com>
>> ---
>> libsemanage/src/direct_api.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
>> index 439122df..e7ec952f 100644
>> --- a/libsemanage/src/direct_api.c
>> +++ b/libsemanage/src/direct_api.c
>> @@ -60,6 +60,7 @@
>>
>> #define PIPE_READ 0
>> #define PIPE_WRITE 1
>> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>>
>> static void semanage_direct_destroy(semanage_handle_t * sh);
>> static int semanage_direct_disconnect(semanage_handle_t * sh);
>> @@ -1332,7 +1333,7 @@ static int semanage_direct_commit(semanage_handle_t * sh)
>> SEMANAGE_SEUSERS_LINKED,
>> SEMANAGE_USERS_EXTRA_LINKED};
>>
>> - for (i = 0; i < (int) sizeof(files); i++) {
>> + for (i = 0; i < (int) ARRAY_SIZE(files); i++) {
>> path = semanage_path(SEMANAGE_TMP, files[i]);
>> if (stat(path, &sb) != 0) {
>> if (errno != ENOENT) {
>> --
>> 2.14.3
>>
>>
> ack
Thanks, merged:
https://github.com/SELinuxProject/selinux/pull/87
>
> Just noticing: that code has i as an int, which it probably should be size_t and
> the files array as an array of int's, it should probably use the
> enum type.
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2018-03-19 16:18 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-06 11:58 libsemanage: Perform access check using euid instead of uid v2 Vit Mojzis
2018-03-06 11:58 ` [PATCH 1/3] libsemanage: remove access() check to make setuid programs work Vit Mojzis
2018-03-06 11:58 ` [PATCH 2/3] " Vit Mojzis
2018-03-08 19:54 ` Stephen Smalley
2018-03-06 11:58 ` [PATCH 3/3] libsemanage: replace access() checks " Vit Mojzis
2018-03-07 14:59 ` Stephen Smalley
2018-03-08 20:24 ` Stephen Smalley
2018-03-09 13:14 ` Vit Mojzis
2018-03-09 13:28 ` Stephen Smalley
2018-03-09 13:36 ` Stephen Smalley
2018-03-09 15:21 ` [PATCH] " Vit Mojzis
2018-03-09 15:31 ` Stephen Smalley
2018-03-09 15:39 ` Vit Mojzis
2018-03-09 15:51 ` Stephen Smalley
2018-03-13 10:58 ` Petr Lautrbach
2018-03-17 7:26 ` Petr Lautrbach
2018-03-19 14:46 ` [PATCH] libsemanage/direct_api.c: Fix iterating over array Vit Mojzis
2018-03-19 15:19 ` William Roberts
2018-03-19 16:18 ` William Roberts
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.