Discussion:
[PATCH] libsemanage: Allow tmp files to be kept if a compile fails
Richard Haines
2018-01-14 15:34:36 UTC
Permalink
Add new option to semanage.conf that allows the tmp build files
to be kept for debugging when building policy.

Signed-off-by: Richard Haines <***@btinternet.com>
---
libsemanage/man/man5/semanage.conf.5 | 8 ++++++++
libsemanage/src/conf-parse.y | 15 ++++++++++++++-
libsemanage/src/conf-scan.l | 1 +
libsemanage/src/direct_api.c | 21 ++++++++++++---------
libsemanage/src/semanage_conf.h | 1 +
5 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/libsemanage/man/man5/semanage.conf.5 b/libsemanage/man/man5/semanage.conf.5
index 8f8de55a..10cab65a 100644
--- a/libsemanage/man/man5/semanage.conf.5
+++ b/libsemanage/man/man5/semanage.conf.5
@@ -121,6 +121,14 @@ and by default it is set to "false".
Please note that since this option deletes all HLL files, an updated HLL compiler will not be able to recompile the original HLL file into CIL.
In order to compile the original HLL file into CIL, the same HLL file will need to be reinstalled.

+.TP
+.B retain-tmp
+When set to "true", tmp directories (the sandbox at \fBstore-root/\fR[\fIpolicy-store\fR]\fB/tmp \fRand/or the final policy at \fBstore-root/final/\fR[\fIpolicy-store\fR]) will be retained after compilation to allow debugging of any build errors. Note that on a successful build the sandbox becomes \fBstore-root/\fR[\fIpolicy-store\fR]\fB/active\fR.
+.br
+The
+.B retain-tmp
+option can be set to either "true" or "false" and by default it is set to "false".
+
.SH "SEE ALSO"
.TP
semanage(8)
diff --git a/libsemanage/src/conf-parse.y b/libsemanage/src/conf-parse.y
index b527e893..f098e55d 100644
--- a/libsemanage/src/conf-parse.y
+++ b/libsemanage/src/conf-parse.y
@@ -61,7 +61,7 @@ static int parse_errors;

%token MODULE_STORE VERSION EXPAND_CHECK FILE_MODE SAVE_PREVIOUS SAVE_LINKED TARGET_PLATFORM COMPILER_DIR IGNORE_MODULE_CACHE STORE_ROOT
%token LOAD_POLICY_START SETFILES_START SEFCONTEXT_COMPILE_START DISABLE_GENHOMEDIRCON HANDLE_UNKNOWN USEPASSWD IGNOREDIRS
-%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL
+%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL RETAIN_TMP
%token VERIFY_MOD_START VERIFY_LINKED_START VERIFY_KERNEL_START BLOCK_END
%token PROG_PATH PROG_ARGS
%token <s> ARG
@@ -95,6 +95,7 @@ single_opt: module_store
| bzip_blocksize
| bzip_small
| remove_hll
+ | retain_tmp
;

module_store: MODULE_STORE '=' ARG {
@@ -268,6 +269,17 @@ remove_hll: REMOVE_HLL'=' ARG {
free($3);
}

+retain_tmp: RETAIN_TMP'=' ARG {
+ if (strcasecmp($3, "false") == 0) {
+ current_conf->retain_tmp = 0;
+ } else if (strcasecmp($3, "true") == 0) {
+ current_conf->retain_tmp = 1;
+ } else {
+ yyerror("retain-tmp can only be 'true' or 'false'");
+ }
+ free($3);
+}
+
command_block:
command_start external_opts BLOCK_END {
if (new_external->path == NULL) {
@@ -352,6 +364,7 @@ static int semanage_conf_init(semanage_conf_t * conf)
conf->bzip_small = 0;
conf->ignore_module_cache = 0;
conf->remove_hll = 0;
+ conf->retain_tmp = 0;

conf->save_previous = 0;
conf->save_linked = 0;
diff --git a/libsemanage/src/conf-scan.l b/libsemanage/src/conf-scan.l
index 607bbf0b..e26c3494 100644
--- a/libsemanage/src/conf-scan.l
+++ b/libsemanage/src/conf-scan.l
@@ -54,6 +54,7 @@ handle-unknown return HANDLE_UNKNOWN;
bzip-blocksize return BZIP_BLOCKSIZE;
bzip-small return BZIP_SMALL;
remove-hll return REMOVE_HLL;
+retain-tmp return RETAIN_TMP;
"[load_policy]" return LOAD_POLICY_START;
"[setfiles]" return SETFILES_START;
"[sefcontext_compile]" return SEFCONTEXT_COMPILE_START;
diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index a455612f..5d2a443c 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -326,7 +326,10 @@ static void semanage_direct_destroy(semanage_handle_t * sh
static int semanage_direct_disconnect(semanage_handle_t * sh)
{
/* destroy transaction */
- if (sh->is_in_transaction) {
+ if (sh->is_in_transaction)
+ semanage_release_trans_lock(sh);
+
+ if (!sh->conf->retain_tmp && sh->is_in_transaction) {
/* destroy sandbox */
if (semanage_remove_directory
(semanage_path(SEMANAGE_TMP, SEMANAGE_TOPLEVEL)) < 0) {
@@ -342,7 +345,6 @@ static int semanage_direct_disconnect(semanage_handle_t * sh)
SEMANAGE_FINAL_TOPLEVEL));
return -1;
}
- semanage_release_trans_lock(sh);
}

/* Release object databases: local modifications */
@@ -1639,13 +1641,14 @@ cleanup:

free(fc_buffer);

- /* regardless if the commit was successful or not, remove the
- sandbox if it is still there */
- semanage_remove_directory(semanage_path
- (SEMANAGE_TMP, SEMANAGE_TOPLEVEL));
- semanage_remove_directory(semanage_final_path
- (SEMANAGE_FINAL_TMP,
- SEMANAGE_FINAL_TOPLEVEL));
+ /* Delete sandbox unless requested by semanage.conf */
+ if (!sh->conf->retain_tmp) {
+ semanage_remove_directory(semanage_path
+ (SEMANAGE_TMP, SEMANAGE_TOPLEVEL));
+ semanage_remove_directory(semanage_final_path
+ (SEMANAGE_FINAL_TMP,
+ SEMANAGE_FINAL_TOPLEVEL));
+ }
umask(mask);

return retval;
diff --git a/libsemanage/src/semanage_conf.h b/libsemanage/src/semanage_conf.h
index c99ac8c7..f7bba754 100644
--- a/libsemanage/src/semanage_conf.h
+++ b/libsemanage/src/semanage_conf.h
@@ -46,6 +46,7 @@ typedef struct semanage_conf {
int bzip_blocksize;
int bzip_small;
int remove_hll;
+ int retain_tmp;
int ignore_module_cache;
char *ignoredirs; /* ";" separated of list for genhomedircon to ignore */
struct external_prog *load_policy;
--
2.14.3
William Roberts
2018-01-15 15:46:27 UTC
Permalink
On Sun, Jan 14, 2018 at 7:34 AM, Richard Haines
Post by Richard Haines
Add new option to semanage.conf that allows the tmp build files
to be kept for debugging when building policy.
How do people know where the tmp files are, does something print it out or is it
documented in a manpage somewhere?
Post by Richard Haines
---
libsemanage/man/man5/semanage.conf.5 | 8 ++++++++
libsemanage/src/conf-parse.y | 15 ++++++++++++++-
libsemanage/src/conf-scan.l | 1 +
libsemanage/src/direct_api.c | 21 ++++++++++++---------
libsemanage/src/semanage_conf.h | 1 +
5 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/libsemanage/man/man5/semanage.conf.5 b/libsemanage/man/man5/semanage.conf.5
index 8f8de55a..10cab65a 100644
--- a/libsemanage/man/man5/semanage.conf.5
+++ b/libsemanage/man/man5/semanage.conf.5
@@ -121,6 +121,14 @@ and by default it is set to "false".
Please note that since this option deletes all HLL files, an updated HLL compiler will not be able to recompile the original HLL file into CIL.
In order to compile the original HLL file into CIL, the same HLL file will need to be reinstalled.
+.TP
+.B retain-tmp
+When set to "true", tmp directories (the sandbox at \fBstore-root/\fR[\fIpolicy-store\fR]\fB/tmp \fRand/or the final policy at \fBstore-root/final/\fR[\fIpolicy-store\fR]) will be retained after compilation to allow debugging of any build errors. Note that on a successful build the sandbox becomes \fBstore-root/\fR[\fIpolicy-store\fR]\fB/active\fR.
+.br
+The
+.B retain-tmp
+option can be set to either "true" or "false" and by default it is set to "false".
+
.SH "SEE ALSO"
.TP
semanage(8)
diff --git a/libsemanage/src/conf-parse.y b/libsemanage/src/conf-parse.y
index b527e893..f098e55d 100644
--- a/libsemanage/src/conf-parse.y
+++ b/libsemanage/src/conf-parse.y
@@ -61,7 +61,7 @@ static int parse_errors;
%token MODULE_STORE VERSION EXPAND_CHECK FILE_MODE SAVE_PREVIOUS SAVE_LINKED TARGET_PLATFORM COMPILER_DIR IGNORE_MODULE_CACHE STORE_ROOT
%token LOAD_POLICY_START SETFILES_START SEFCONTEXT_COMPILE_START DISABLE_GENHOMEDIRCON HANDLE_UNKNOWN USEPASSWD IGNOREDIRS
-%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL
+%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL RETAIN_TMP
%token VERIFY_MOD_START VERIFY_LINKED_START VERIFY_KERNEL_START BLOCK_END
%token PROG_PATH PROG_ARGS
%token <s> ARG
@@ -95,6 +95,7 @@ single_opt: module_store
| bzip_blocksize
| bzip_small
| remove_hll
+ | retain_tmp
;
module_store: MODULE_STORE '=' ARG {
@@ -268,6 +269,17 @@ remove_hll: REMOVE_HLL'=' ARG {
free($3);
}
+retain_tmp: RETAIN_TMP'=' ARG {
+ if (strcasecmp($3, "false") == 0) {
+ current_conf->retain_tmp = 0;
+ } else if (strcasecmp($3, "true") == 0) {
+ current_conf->retain_tmp = 1;
+ } else {
+ yyerror("retain-tmp can only be 'true' or 'false'");
+ }
+ free($3);
+}
+
command_start external_opts BLOCK_END {
if (new_external->path == NULL) {
@@ -352,6 +364,7 @@ static int semanage_conf_init(semanage_conf_t * conf)
conf->bzip_small = 0;
conf->ignore_module_cache = 0;
conf->remove_hll = 0;
+ conf->retain_tmp = 0;
conf->save_previous = 0;
conf->save_linked = 0;
diff --git a/libsemanage/src/conf-scan.l b/libsemanage/src/conf-scan.l
index 607bbf0b..e26c3494 100644
--- a/libsemanage/src/conf-scan.l
+++ b/libsemanage/src/conf-scan.l
@@ -54,6 +54,7 @@ handle-unknown return HANDLE_UNKNOWN;
bzip-blocksize return BZIP_BLOCKSIZE;
bzip-small return BZIP_SMALL;
remove-hll return REMOVE_HLL;
+retain-tmp return RETAIN_TMP;
"[load_policy]" return LOAD_POLICY_START;
"[setfiles]" return SETFILES_START;
"[sefcontext_compile]" return SEFCONTEXT_COMPILE_START;
diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index a455612f..5d2a443c 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -326,7 +326,10 @@ static void semanage_direct_destroy(semanage_handle_t * sh
static int semanage_direct_disconnect(semanage_handle_t * sh)
{
/* destroy transaction */
- if (sh->is_in_transaction) {
+ if (sh->is_in_transaction)
+ semanage_release_trans_lock(sh);
+
+ if (!sh->conf->retain_tmp && sh->is_in_transaction) {
/* destroy sandbox */
if (semanage_remove_directory
(semanage_path(SEMANAGE_TMP, SEMANAGE_TOPLEVEL)) < 0) {
@@ -342,7 +345,6 @@ static int semanage_direct_disconnect(semanage_handle_t * sh)
SEMANAGE_FINAL_TOPLEVEL));
return -1;
}
- semanage_release_trans_lock(sh);
}
/* Release object databases: local modifications */
free(fc_buffer);
- /* regardless if the commit was successful or not, remove the
- sandbox if it is still there */
- semanage_remove_directory(semanage_path
- (SEMANAGE_TMP, SEMANAGE_TOPLEVEL));
- semanage_remove_directory(semanage_final_path
- (SEMANAGE_FINAL_TMP,
- SEMANAGE_FINAL_TOPLEVEL));
+ /* Delete sandbox unless requested by semanage.conf */
+ if (!sh->conf->retain_tmp) {
+ semanage_remove_directory(semanage_path
+ (SEMANAGE_TMP, SEMANAGE_TOPLEVEL));
+ semanage_remove_directory(semanage_final_path
+ (SEMANAGE_FINAL_TMP,
+ SEMANAGE_FINAL_TOPLEVEL));
+ }
umask(mask);
return retval;
diff --git a/libsemanage/src/semanage_conf.h b/libsemanage/src/semanage_conf.h
index c99ac8c7..f7bba754 100644
--- a/libsemanage/src/semanage_conf.h
+++ b/libsemanage/src/semanage_conf.h
@@ -46,6 +46,7 @@ typedef struct semanage_conf {
int bzip_blocksize;
int bzip_small;
int remove_hll;
+ int retain_tmp;
int ignore_module_cache;
char *ignoredirs; /* ";" separated of list for genhomedircon to ignore */
struct external_prog *load_policy;
--
2.14.3
--
Respectfully,

William C Roberts
Petr Lautrbach
2018-01-15 16:11:00 UTC
Permalink
Post by William Roberts
On Sun, Jan 14, 2018 at 7:34 AM, Richard Haines
Post by Richard Haines
Add new option to semanage.conf that allows the tmp build files
to be kept for debugging when building policy.
How do people know where the tmp files are, does something print it out or is it
documented in a manpage somewhere?
I usually see them when I try to load a module with some problem, e.g.

# semodule -i myconfined10.cil
neverallow check failed at /var/lib/selinux/targeted/tmp/modules/100/base/cil:13249
...
Post by William Roberts
Post by Richard Haines
---
libsemanage/man/man5/semanage.conf.5 | 8 ++++++++
libsemanage/src/conf-parse.y | 15 ++++++++++++++-
libsemanage/src/conf-scan.l | 1 +
libsemanage/src/direct_api.c | 21 ++++++++++++---------
libsemanage/src/semanage_conf.h | 1 +
5 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/libsemanage/man/man5/semanage.conf.5 b/libsemanage/man/man5/semanage.conf.5
index 8f8de55a..10cab65a 100644
--- a/libsemanage/man/man5/semanage.conf.5
+++ b/libsemanage/man/man5/semanage.conf.5
@@ -121,6 +121,14 @@ and by default it is set to "false".
Please note that since this option deletes all HLL files, an updated HLL compiler will not be able to recompile the original HLL file into CIL.
In order to compile the original HLL file into CIL, the same HLL file will need to be reinstalled.
+.TP
+.B retain-tmp
+When set to "true", tmp directories (the sandbox at \fBstore-root/\fR[\fIpolicy-store\fR]\fB/tmp \fRand/or the final policy at \fBstore-root/final/\fR[\fIpolicy-store\fR]) will be retained after compilation to allow debugging of any build errors. Note that on a successful build the sandbox becomes \fBstore-root/\fR[\fIpolicy-store\fR]\fB/active\fR.
+.br
+The
+.B retain-tmp
+option can be set to either "true" or "false" and by default it is set to "false".
+
.SH "SEE ALSO"
.TP
semanage(8)
diff --git a/libsemanage/src/conf-parse.y b/libsemanage/src/conf-parse.y
index b527e893..f098e55d 100644
--- a/libsemanage/src/conf-parse.y
+++ b/libsemanage/src/conf-parse.y
@@ -61,7 +61,7 @@ static int parse_errors;
%token MODULE_STORE VERSION EXPAND_CHECK FILE_MODE SAVE_PREVIOUS SAVE_LINKED TARGET_PLATFORM COMPILER_DIR IGNORE_MODULE_CACHE STORE_ROOT
%token LOAD_POLICY_START SETFILES_START SEFCONTEXT_COMPILE_START DISABLE_GENHOMEDIRCON HANDLE_UNKNOWN USEPASSWD IGNOREDIRS
-%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL
+%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL RETAIN_TMP
%token VERIFY_MOD_START VERIFY_LINKED_START VERIFY_KERNEL_START BLOCK_END
%token PROG_PATH PROG_ARGS
%token <s> ARG
@@ -95,6 +95,7 @@ single_opt: module_store
| bzip_blocksize
| bzip_small
| remove_hll
+ | retain_tmp
;
module_store: MODULE_STORE '=' ARG {
@@ -268,6 +269,17 @@ remove_hll: REMOVE_HLL'=' ARG {
free($3);
}
+retain_tmp: RETAIN_TMP'=' ARG {
+ if (strcasecmp($3, "false") == 0) {
+ current_conf->retain_tmp = 0;
+ } else if (strcasecmp($3, "true") == 0) {
+ current_conf->retain_tmp = 1;
+ } else {
+ yyerror("retain-tmp can only be 'true' or 'false'");
+ }
+ free($3);
+}
+
command_start external_opts BLOCK_END {
if (new_external->path == NULL) {
@@ -352,6 +364,7 @@ static int semanage_conf_init(semanage_conf_t * conf)
conf->bzip_small = 0;
conf->ignore_module_cache = 0;
conf->remove_hll = 0;
+ conf->retain_tmp = 0;
conf->save_previous = 0;
conf->save_linked = 0;
diff --git a/libsemanage/src/conf-scan.l b/libsemanage/src/conf-scan.l
index 607bbf0b..e26c3494 100644
--- a/libsemanage/src/conf-scan.l
+++ b/libsemanage/src/conf-scan.l
@@ -54,6 +54,7 @@ handle-unknown return HANDLE_UNKNOWN;
bzip-blocksize return BZIP_BLOCKSIZE;
bzip-small return BZIP_SMALL;
remove-hll return REMOVE_HLL;
+retain-tmp return RETAIN_TMP;
"[load_policy]" return LOAD_POLICY_START;
"[setfiles]" return SETFILES_START;
"[sefcontext_compile]" return SEFCONTEXT_COMPILE_START;
diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index a455612f..5d2a443c 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -326,7 +326,10 @@ static void semanage_direct_destroy(semanage_handle_t * sh
static int semanage_direct_disconnect(semanage_handle_t * sh)
{
/* destroy transaction */
- if (sh->is_in_transaction) {
+ if (sh->is_in_transaction)
+ semanage_release_trans_lock(sh);
+
+ if (!sh->conf->retain_tmp && sh->is_in_transaction) {
/* destroy sandbox */
if (semanage_remove_directory
(semanage_path(SEMANAGE_TMP, SEMANAGE_TOPLEVEL)) < 0) {
@@ -342,7 +345,6 @@ static int semanage_direct_disconnect(semanage_handle_t * sh)
SEMANAGE_FINAL_TOPLEVEL));
return -1;
}
- semanage_release_trans_lock(sh);
}
/* Release object databases: local modifications */
free(fc_buffer);
- /* regardless if the commit was successful or not, remove the
- sandbox if it is still there */
- semanage_remove_directory(semanage_path
- (SEMANAGE_TMP, SEMANAGE_TOPLEVEL));
- semanage_remove_directory(semanage_final_path
- (SEMANAGE_FINAL_TMP,
- SEMANAGE_FINAL_TOPLEVEL));
+ /* Delete sandbox unless requested by semanage.conf */
+ if (!sh->conf->retain_tmp) {
+ semanage_remove_directory(semanage_path
+ (SEMANAGE_TMP, SEMANAGE_TOPLEVEL));
+ semanage_remove_directory(semanage_final_path
+ (SEMANAGE_FINAL_TMP,
+ SEMANAGE_FINAL_TOPLEVEL));
+ }
umask(mask);
return retval;
diff --git a/libsemanage/src/semanage_conf.h b/libsemanage/src/semanage_conf.h
index c99ac8c7..f7bba754 100644
--- a/libsemanage/src/semanage_conf.h
+++ b/libsemanage/src/semanage_conf.h
@@ -46,6 +46,7 @@ typedef struct semanage_conf {
int bzip_blocksize;
int bzip_small;
int remove_hll;
+ int retain_tmp;
int ignore_module_cache;
char *ignoredirs; /* ";" separated of list for genhomedircon to ignore */
struct external_prog *load_policy;
--
2.14.3
--
Respectfully,
William C Roberts
Richard Haines
2018-01-15 16:39:40 UTC
Permalink
Post by William Roberts
On Sun, Jan 14, 2018 at 7:34 AM, Richard Haines
Post by Richard Haines
Add new option to semanage.conf that allows the tmp build files
to be kept for debugging when building policy.
How do people know where the tmp files are, does something print it out or is it
documented in a manpage somewhere?
I updated the semanage.conf man page in the patch that states where
they are located. If you think more clarification is required let me
know. I guess the semanage/semodule man pages could do with a reference
to semanage.conf as it does influence the policy build, but that would
be a separate patch.

I added this patch as when running semodule to add a new hand cranked
module, I make a mistake and the following error was displayed showing
the file location:

Failed to resolve typeattributeset statement at
/var/lib/selinux/targeted/tmp/modules/400/local/cil:3

However without the patch it deleted the tmp directory so I could not
find the mistake.
Post by William Roberts
Post by Richard Haines
---
libsemanage/man/man5/semanage.conf.5 | 8 ++++++++
libsemanage/src/conf-parse.y | 15 ++++++++++++++-
libsemanage/src/conf-scan.l | 1 +
libsemanage/src/direct_api.c | 21 ++++++++++++---------
libsemanage/src/semanage_conf.h | 1 +
5 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/libsemanage/man/man5/semanage.conf.5
b/libsemanage/man/man5/semanage.conf.5
index 8f8de55a..10cab65a 100644
--- a/libsemanage/man/man5/semanage.conf.5
+++ b/libsemanage/man/man5/semanage.conf.5
@@ -121,6 +121,14 @@ and by default it is set to "false".
Please note that since this option deletes all HLL files, an
updated HLL compiler will not be able to recompile the original HLL
file into CIL.
In order to compile the original HLL file into CIL, the same HLL
file will need to be reinstalled.
+.TP
+.B retain-tmp
+When set to "true", tmp directories (the sandbox at \fBstore-
root/\fR[\fIpolicy-store\fR]\fB/tmp \fRand/or the final policy at
\fBstore-root/final/\fR[\fIpolicy-store\fR]) will be retained after
compilation to allow debugging of any build errors. Note that on a
successful build the sandbox becomes \fBstore-root/\fR[\fIpolicy-
store\fR]\fB/active\fR.
+.br
+The
+.B retain-tmp
+option can be set to either "true" or "false" and by default it is set to "false".
+
.SH "SEE ALSO"
.TP
semanage(8)
diff --git a/libsemanage/src/conf-parse.y b/libsemanage/src/conf-
parse.y
index b527e893..f098e55d 100644
--- a/libsemanage/src/conf-parse.y
+++ b/libsemanage/src/conf-parse.y
@@ -61,7 +61,7 @@ static int parse_errors;
%token MODULE_STORE VERSION EXPAND_CHECK FILE_MODE SAVE_PREVIOUS
SAVE_LINKED TARGET_PLATFORM COMPILER_DIR IGNORE_MODULE_CACHE
STORE_ROOT
%token LOAD_POLICY_START SETFILES_START SEFCONTEXT_COMPILE_START
DISABLE_GENHOMEDIRCON HANDLE_UNKNOWN USEPASSWD IGNOREDIRS
-%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL
+%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL RETAIN_TMP
%token VERIFY_MOD_START VERIFY_LINKED_START VERIFY_KERNEL_START BLOCK_END
%token PROG_PATH PROG_ARGS
%token <s> ARG
@@ -95,6 +95,7 @@ single_opt: module_store
| bzip_blocksize
| bzip_small
| remove_hll
+ | retain_tmp
;
module_store: MODULE_STORE '=' ARG {
@@ -268,6 +269,17 @@ remove_hll: REMOVE_HLL'=' ARG {
free($3);
}
+retain_tmp: RETAIN_TMP'=' ARG {
+ if (strcasecmp($3, "false") == 0) {
+ current_conf->retain_tmp = 0;
+ } else if (strcasecmp($3, "true") == 0) {
+ current_conf->retain_tmp = 1;
+ } else {
+ yyerror("retain-tmp can only be 'true' or
'false'");
+ }
+ free($3);
+}
+
command_start external_opts BLOCK_END {
if (new_external->path == NULL) {
@@ -352,6 +364,7 @@ static int semanage_conf_init(semanage_conf_t * conf)
conf->bzip_small = 0;
conf->ignore_module_cache = 0;
conf->remove_hll = 0;
+ conf->retain_tmp = 0;
conf->save_previous = 0;
conf->save_linked = 0;
diff --git a/libsemanage/src/conf-scan.l b/libsemanage/src/conf-
scan.l
index 607bbf0b..e26c3494 100644
--- a/libsemanage/src/conf-scan.l
+++ b/libsemanage/src/conf-scan.l
@@ -54,6 +54,7 @@ handle-unknown return HANDLE_UNKNOWN;
bzip-blocksize return BZIP_BLOCKSIZE;
bzip-small return BZIP_SMALL;
remove-hll return REMOVE_HLL;
+retain-tmp return RETAIN_TMP;
"[load_policy]" return LOAD_POLICY_START;
"[setfiles]" return SETFILES_START;
"[sefcontext_compile]" return SEFCONTEXT_COMPILE_START;
diff --git a/libsemanage/src/direct_api.c
b/libsemanage/src/direct_api.c
index a455612f..5d2a443c 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -326,7 +326,10 @@ static void
semanage_direct_destroy(semanage_handle_t * sh
static int semanage_direct_disconnect(semanage_handle_t * sh)
{
/* destroy transaction */
- if (sh->is_in_transaction) {
+ if (sh->is_in_transaction)
+ semanage_release_trans_lock(sh);
+
+ if (!sh->conf->retain_tmp && sh->is_in_transaction) {
/* destroy sandbox */
if (semanage_remove_directory
(semanage_path(SEMANAGE_TMP,
SEMANAGE_TOPLEVEL)) < 0) {
@@ -342,7 +345,6 @@ static int
semanage_direct_disconnect(semanage_handle_t * sh)
SEMANAGE_FINAL_TOPL
EVEL));
return -1;
}
- semanage_release_trans_lock(sh);
}
/* Release object databases: local modifications */
free(fc_buffer);
- /* regardless if the commit was successful or not, remove the
- sandbox if it is still there */
- semanage_remove_directory(semanage_path
- (SEMANAGE_TMP,
SEMANAGE_TOPLEVEL));
- semanage_remove_directory(semanage_final_path
- (SEMANAGE_FINAL_TMP,
- SEMANAGE_FINAL_TOPLEVEL));
+ /* Delete sandbox unless requested by semanage.conf */
+ if (!sh->conf->retain_tmp) {
+ semanage_remove_directory(semanage_path
+ (SEMANAGE_TMP,
SEMANAGE_TOPLEVEL));
+ semanage_remove_directory(semanage_final_path
+ (SEMANAGE_FINAL_TMP,
+ SEMANAGE_FINAL_TOPLEVEL)
);
+ }
umask(mask);
return retval;
diff --git a/libsemanage/src/semanage_conf.h
b/libsemanage/src/semanage_conf.h
index c99ac8c7..f7bba754 100644
--- a/libsemanage/src/semanage_conf.h
+++ b/libsemanage/src/semanage_conf.h
@@ -46,6 +46,7 @@ typedef struct semanage_conf {
int bzip_blocksize;
int bzip_small;
int remove_hll;
+ int retain_tmp;
int ignore_module_cache;
char *ignoredirs; /* ";" separated of list for
genhomedircon to ignore */
struct external_prog *load_policy;
--
2.14.3
William Roberts
2018-01-15 16:49:27 UTC
Permalink
On Mon, Jan 15, 2018 at 8:39 AM, Richard Haines
Post by Richard Haines
Post by William Roberts
On Sun, Jan 14, 2018 at 7:34 AM, Richard Haines
Post by Richard Haines
Add new option to semanage.conf that allows the tmp build files
to be kept for debugging when building policy.
How do people know where the tmp files are, does something print it out or is it
documented in a manpage somewhere?
I updated the semanage.conf man page in the patch that states where
they are located.
I read right past that, thanks for pointing it out. I need all the help I can
get some days.

If you think more clarification is required let me
Post by Richard Haines
know. I guess the semanage/semodule man pages could do with a reference
to semanage.conf as it does influence the policy build, but that would
be a separate patch.
I added this patch as when running semodule to add a new hand cranked
module, I make a mistake and the following error was displayed showing
Failed to resolve typeattributeset statement at
/var/lib/selinux/targeted/tmp/modules/400/local/cil:3
However without the patch it deleted the tmp directory so I could not
find the mistake.
This seems very reasonable. I have had similar issues in Android build
where I have to make sure it doesn't clobber intermediate files.

Ack by me. I am going to let this sit until Wednesday since some
folks have today off in the US and might not be following.

Assuming no complaints from others, ill take to merge Wednesday.
Post by Richard Haines
Post by William Roberts
Post by Richard Haines
---
libsemanage/man/man5/semanage.conf.5 | 8 ++++++++
libsemanage/src/conf-parse.y | 15 ++++++++++++++-
libsemanage/src/conf-scan.l | 1 +
libsemanage/src/direct_api.c | 21 ++++++++++++---------
libsemanage/src/semanage_conf.h | 1 +
5 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/libsemanage/man/man5/semanage.conf.5
b/libsemanage/man/man5/semanage.conf.5
index 8f8de55a..10cab65a 100644
--- a/libsemanage/man/man5/semanage.conf.5
+++ b/libsemanage/man/man5/semanage.conf.5
@@ -121,6 +121,14 @@ and by default it is set to "false".
Please note that since this option deletes all HLL files, an
updated HLL compiler will not be able to recompile the original HLL
file into CIL.
In order to compile the original HLL file into CIL, the same HLL
file will need to be reinstalled.
+.TP
+.B retain-tmp
+When set to "true", tmp directories (the sandbox at \fBstore-
root/\fR[\fIpolicy-store\fR]\fB/tmp \fRand/or the final policy at
\fBstore-root/final/\fR[\fIpolicy-store\fR]) will be retained after
compilation to allow debugging of any build errors. Note that on a
successful build the sandbox becomes \fBstore-root/\fR[\fIpolicy-
store\fR]\fB/active\fR.
+.br
+The
+.B retain-tmp
+option can be set to either "true" or "false" and by default it is set to "false".
+
.SH "SEE ALSO"
.TP
semanage(8)
diff --git a/libsemanage/src/conf-parse.y b/libsemanage/src/conf-
parse.y
index b527e893..f098e55d 100644
--- a/libsemanage/src/conf-parse.y
+++ b/libsemanage/src/conf-parse.y
@@ -61,7 +61,7 @@ static int parse_errors;
%token MODULE_STORE VERSION EXPAND_CHECK FILE_MODE SAVE_PREVIOUS
SAVE_LINKED TARGET_PLATFORM COMPILER_DIR IGNORE_MODULE_CACHE
STORE_ROOT
%token LOAD_POLICY_START SETFILES_START SEFCONTEXT_COMPILE_START
DISABLE_GENHOMEDIRCON HANDLE_UNKNOWN USEPASSWD IGNOREDIRS
-%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL
+%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL RETAIN_TMP
%token VERIFY_MOD_START VERIFY_LINKED_START VERIFY_KERNEL_START BLOCK_END
%token PROG_PATH PROG_ARGS
%token <s> ARG
@@ -95,6 +95,7 @@ single_opt: module_store
| bzip_blocksize
| bzip_small
| remove_hll
+ | retain_tmp
;
module_store: MODULE_STORE '=' ARG {
@@ -268,6 +269,17 @@ remove_hll: REMOVE_HLL'=' ARG {
free($3);
}
+retain_tmp: RETAIN_TMP'=' ARG {
+ if (strcasecmp($3, "false") == 0) {
+ current_conf->retain_tmp = 0;
+ } else if (strcasecmp($3, "true") == 0) {
+ current_conf->retain_tmp = 1;
+ } else {
+ yyerror("retain-tmp can only be 'true' or
'false'");
+ }
+ free($3);
+}
+
command_start external_opts BLOCK_END {
if (new_external->path == NULL) {
@@ -352,6 +364,7 @@ static int semanage_conf_init(semanage_conf_t * conf)
conf->bzip_small = 0;
conf->ignore_module_cache = 0;
conf->remove_hll = 0;
+ conf->retain_tmp = 0;
conf->save_previous = 0;
conf->save_linked = 0;
diff --git a/libsemanage/src/conf-scan.l b/libsemanage/src/conf-
scan.l
index 607bbf0b..e26c3494 100644
--- a/libsemanage/src/conf-scan.l
+++ b/libsemanage/src/conf-scan.l
@@ -54,6 +54,7 @@ handle-unknown return HANDLE_UNKNOWN;
bzip-blocksize return BZIP_BLOCKSIZE;
bzip-small return BZIP_SMALL;
remove-hll return REMOVE_HLL;
+retain-tmp return RETAIN_TMP;
"[load_policy]" return LOAD_POLICY_START;
"[setfiles]" return SETFILES_START;
"[sefcontext_compile]" return SEFCONTEXT_COMPILE_START;
diff --git a/libsemanage/src/direct_api.c
b/libsemanage/src/direct_api.c
index a455612f..5d2a443c 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -326,7 +326,10 @@ static void
semanage_direct_destroy(semanage_handle_t * sh
static int semanage_direct_disconnect(semanage_handle_t * sh)
{
/* destroy transaction */
- if (sh->is_in_transaction) {
+ if (sh->is_in_transaction)
+ semanage_release_trans_lock(sh);
+
+ if (!sh->conf->retain_tmp && sh->is_in_transaction) {
/* destroy sandbox */
if (semanage_remove_directory
(semanage_path(SEMANAGE_TMP,
SEMANAGE_TOPLEVEL)) < 0) {
@@ -342,7 +345,6 @@ static int
semanage_direct_disconnect(semanage_handle_t * sh)
SEMANAGE_FINAL_TOPL
EVEL));
return -1;
}
- semanage_release_trans_lock(sh);
}
/* Release object databases: local modifications */
free(fc_buffer);
- /* regardless if the commit was successful or not, remove the
- sandbox if it is still there */
- semanage_remove_directory(semanage_path
- (SEMANAGE_TMP,
SEMANAGE_TOPLEVEL));
- semanage_remove_directory(semanage_final_path
- (SEMANAGE_FINAL_TMP,
- SEMANAGE_FINAL_TOPLEVEL));
+ /* Delete sandbox unless requested by semanage.conf */
+ if (!sh->conf->retain_tmp) {
+ semanage_remove_directory(semanage_path
+ (SEMANAGE_TMP,
SEMANAGE_TOPLEVEL));
+ semanage_remove_directory(semanage_final_path
+ (SEMANAGE_FINAL_TMP,
+ SEMANAGE_FINAL_TOPLEVEL)
);
+ }
umask(mask);
return retval;
diff --git a/libsemanage/src/semanage_conf.h
b/libsemanage/src/semanage_conf.h
index c99ac8c7..f7bba754 100644
--- a/libsemanage/src/semanage_conf.h
+++ b/libsemanage/src/semanage_conf.h
@@ -46,6 +46,7 @@ typedef struct semanage_conf {
int bzip_blocksize;
int bzip_small;
int remove_hll;
+ int retain_tmp;
int ignore_module_cache;
char *ignoredirs; /* ";" separated of list for
genhomedircon to ignore */
struct external_prog *load_policy;
--
2.14.3
--
Respectfully,

William C Roberts
Stephen Smalley
2018-01-15 17:32:28 UTC
Permalink
On Jan 14, 2018 10:36 AM, "Richard Haines" <***@btinternet.com>
wrote:

Add new option to semanage.conf that allows the tmp build files
to be kept for debugging when building policy.


Would it be better to just retain the files by default if there is an
error?


Signed-off-by: Richard Haines <***@btinternet.com>
---
libsemanage/man/man5/semanage.conf.5 | 8 ++++++++
libsemanage/src/conf-parse.y | 15 ++++++++++++++-
libsemanage/src/conf-scan.l | 1 +
libsemanage/src/direct_api.c | 21 ++++++++++++---------
libsemanage/src/semanage_conf.h | 1 +
5 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/libsemanage/man/man5/semanage.conf.5 b/libsemanage/man/man5/
semanage.conf.5
index 8f8de55a..10cab65a 100644
--- a/libsemanage/man/man5/semanage.conf.5
+++ b/libsemanage/man/man5/semanage.conf.5
@@ -121,6 +121,14 @@ and by default it is set to "false".
Please note that since this option deletes all HLL files, an updated HLL
compiler will not be able to recompile the original HLL file into CIL.
In order to compile the original HLL file into CIL, the same HLL file will
need to be reinstalled.

+.TP
+.B retain-tmp
+When set to "true", tmp directories (the sandbox at
\fBstore-root/\fR[\fIpolicy-store\fR]\fB/tmp \fRand/or the final policy at
\fBstore-root/final/\fR[\fIpolicy-store\fR]) will be retained after
compilation to allow debugging of any build errors. Note that on a
successful build the sandbox becomes \fBstore-root/\fR[\fIpolicy-
store\fR]\fB/active\fR.
+.br
+The
+.B retain-tmp
+option can be set to either "true" or "false" and by default it is set to
"false".
+
.SH "SEE ALSO"
.TP
semanage(8)
diff --git a/libsemanage/src/conf-parse.y b/libsemanage/src/conf-parse.y
index b527e893..f098e55d 100644
--- a/libsemanage/src/conf-parse.y
+++ b/libsemanage/src/conf-parse.y
@@ -61,7 +61,7 @@ static int parse_errors;

%token MODULE_STORE VERSION EXPAND_CHECK FILE_MODE SAVE_PREVIOUS
SAVE_LINKED TARGET_PLATFORM COMPILER_DIR IGNORE_MODULE_CACHE STORE_ROOT
%token LOAD_POLICY_START SETFILES_START SEFCONTEXT_COMPILE_START
DISABLE_GENHOMEDIRCON HANDLE_UNKNOWN USEPASSWD IGNOREDIRS
-%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL
+%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL RETAIN_TMP
%token VERIFY_MOD_START VERIFY_LINKED_START VERIFY_KERNEL_START BLOCK_END
%token PROG_PATH PROG_ARGS
%token <s> ARG
@@ -95,6 +95,7 @@ single_opt: module_store
| bzip_blocksize
| bzip_small
| remove_hll
+ | retain_tmp
;

module_store: MODULE_STORE '=' ARG {
@@ -268,6 +269,17 @@ remove_hll: REMOVE_HLL'=' ARG {
free($3);
}

+retain_tmp: RETAIN_TMP'=' ARG {
+ if (strcasecmp($3, "false") == 0) {
+ current_conf->retain_tmp = 0;
+ } else if (strcasecmp($3, "true") == 0) {
+ current_conf->retain_tmp = 1;
+ } else {
+ yyerror("retain-tmp can only be 'true' or 'false'");
+ }
+ free($3);
+}
+
command_block:
command_start external_opts BLOCK_END {
if (new_external->path == NULL) {
@@ -352,6 +364,7 @@ static int semanage_conf_init(semanage_conf_t * conf)
conf->bzip_small = 0;
conf->ignore_module_cache = 0;
conf->remove_hll = 0;
+ conf->retain_tmp = 0;

conf->save_previous = 0;
conf->save_linked = 0;
diff --git a/libsemanage/src/conf-scan.l b/libsemanage/src/conf-scan.l
index 607bbf0b..e26c3494 100644
--- a/libsemanage/src/conf-scan.l
+++ b/libsemanage/src/conf-scan.l
@@ -54,6 +54,7 @@ handle-unknown return HANDLE_UNKNOWN;
bzip-blocksize return BZIP_BLOCKSIZE;
bzip-small return BZIP_SMALL;
remove-hll return REMOVE_HLL;
+retain-tmp return RETAIN_TMP;
"[load_policy]" return LOAD_POLICY_START;
"[setfiles]" return SETFILES_START;
"[sefcontext_compile]" return SEFCONTEXT_COMPILE_START;
diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index a455612f..5d2a443c 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -326,7 +326,10 @@ static void semanage_direct_destroy(semanage_handle_t
* sh
static int semanage_direct_disconnect(semanage_handle_t * sh)
{
/* destroy transaction */
- if (sh->is_in_transaction) {
+ if (sh->is_in_transaction)
+ semanage_release_trans_lock(sh);


Is it safe to release the trans lock before deleting the sandbox?

+
+ if (!sh->conf->retain_tmp && sh->is_in_transaction) {
/* destroy sandbox */
if (semanage_remove_directory
(semanage_path(SEMANAGE_TMP, SEMANAGE_TOPLEVEL)) < 0) {
@@ -342,7 +345,6 @@ static int semanage_direct_disconnect(semanage_handle_t
* sh)
SEMANAGE_FINAL_TOPLEVEL));
return -1;
}
- semanage_release_trans_lock(sh);
}

/* Release object databases: local modifications */
@@ -1639,13 +1641,14 @@ cleanup:

free(fc_buffer);

- /* regardless if the commit was successful or not, remove the
- sandbox if it is still there */
- semanage_remove_directory(semanage_path
- (SEMANAGE_TMP, SEMANAGE_TOPLEVEL));
- semanage_remove_directory(semanage_final_path
- (SEMANAGE_FINAL_TMP,
- SEMANAGE_FINAL_TOPLEVEL));
+ /* Delete sandbox unless requested by semanage.conf */
+ if (!sh->conf->retain_tmp) {
+ semanage_remove_directory(semanage_path
+ (SEMANAGE_TMP,
SEMANAGE_TOPLEVEL));
+ semanage_remove_directory(semanage_final_path
+ (SEMANAGE_FINAL_TMP,
+ SEMANAGE_FINAL_TOPLEVEL));
+ }
umask(mask);

return retval;
diff --git a/libsemanage/src/semanage_conf.h b/libsemanage/src/semanage_
conf.h
index c99ac8c7..f7bba754 100644
--- a/libsemanage/src/semanage_conf.h
+++ b/libsemanage/src/semanage_conf.h
@@ -46,6 +46,7 @@ typedef struct semanage_conf {
int bzip_blocksize;
int bzip_small;
int remove_hll;
+ int retain_tmp;
int ignore_module_cache;
char *ignoredirs; /* ";" separated of list for genhomedircon
to ignore */
struct external_prog *load_policy;
--
2.14.3
William Roberts
2018-01-16 15:47:49 UTC
Permalink
On Mon, Jan 15, 2018 at 9:32 AM, Stephen Smalley
Post by Richard Haines
Add new option to semanage.conf that allows the tmp build files
to be kept for debugging when building policy.
Would it be better to just retain the files by default if there is an error?
I thought about this as well, my reasoning as to why Richard's approach was
better is that if someone does it N times trying to figure out an issue,
then there would be N piles of files in the tmp folder. This way they
have to opt in to have their tmp folder spammed.
Post by Richard Haines
---
libsemanage/man/man5/semanage.conf.5 | 8 ++++++++
libsemanage/src/conf-parse.y | 15 ++++++++++++++-
libsemanage/src/conf-scan.l | 1 +
libsemanage/src/direct_api.c | 21 ++++++++++++---------
libsemanage/src/semanage_conf.h | 1 +
5 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/libsemanage/man/man5/semanage.conf.5
b/libsemanage/man/man5/semanage.conf.5
index 8f8de55a..10cab65a 100644
--- a/libsemanage/man/man5/semanage.conf.5
+++ b/libsemanage/man/man5/semanage.conf.5
@@ -121,6 +121,14 @@ and by default it is set to "false".
Please note that since this option deletes all HLL files, an updated HLL
compiler will not be able to recompile the original HLL file into CIL.
In order to compile the original HLL file into CIL, the same HLL file will
need to be reinstalled.
+.TP
+.B retain-tmp
+When set to "true", tmp directories (the sandbox at
\fBstore-root/\fR[\fIpolicy-store\fR]\fB/tmp \fRand/or the final policy at
\fBstore-root/final/\fR[\fIpolicy-store\fR]) will be retained after
compilation to allow debugging of any build errors. Note that on a
successful build the sandbox becomes
\fBstore-root/\fR[\fIpolicy-store\fR]\fB/active\fR.
+.br
+The
+.B retain-tmp
+option can be set to either "true" or "false" and by default it is set to "false".
+
.SH "SEE ALSO"
.TP
semanage(8)
diff --git a/libsemanage/src/conf-parse.y b/libsemanage/src/conf-parse.y
index b527e893..f098e55d 100644
--- a/libsemanage/src/conf-parse.y
+++ b/libsemanage/src/conf-parse.y
@@ -61,7 +61,7 @@ static int parse_errors;
%token MODULE_STORE VERSION EXPAND_CHECK FILE_MODE SAVE_PREVIOUS
SAVE_LINKED TARGET_PLATFORM COMPILER_DIR IGNORE_MODULE_CACHE STORE_ROOT
%token LOAD_POLICY_START SETFILES_START SEFCONTEXT_COMPILE_START
DISABLE_GENHOMEDIRCON HANDLE_UNKNOWN USEPASSWD IGNOREDIRS
-%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL
+%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL RETAIN_TMP
%token VERIFY_MOD_START VERIFY_LINKED_START VERIFY_KERNEL_START BLOCK_END
%token PROG_PATH PROG_ARGS
%token <s> ARG
@@ -95,6 +95,7 @@ single_opt: module_store
| bzip_blocksize
| bzip_small
| remove_hll
+ | retain_tmp
;
module_store: MODULE_STORE '=' ARG {
@@ -268,6 +269,17 @@ remove_hll: REMOVE_HLL'=' ARG {
free($3);
}
+retain_tmp: RETAIN_TMP'=' ARG {
+ if (strcasecmp($3, "false") == 0) {
+ current_conf->retain_tmp = 0;
+ } else if (strcasecmp($3, "true") == 0) {
+ current_conf->retain_tmp = 1;
+ } else {
+ yyerror("retain-tmp can only be 'true' or 'false'");
+ }
+ free($3);
+}
+
command_start external_opts BLOCK_END {
if (new_external->path == NULL) {
@@ -352,6 +364,7 @@ static int semanage_conf_init(semanage_conf_t * conf)
conf->bzip_small = 0;
conf->ignore_module_cache = 0;
conf->remove_hll = 0;
+ conf->retain_tmp = 0;
conf->save_previous = 0;
conf->save_linked = 0;
diff --git a/libsemanage/src/conf-scan.l b/libsemanage/src/conf-scan.l
index 607bbf0b..e26c3494 100644
--- a/libsemanage/src/conf-scan.l
+++ b/libsemanage/src/conf-scan.l
@@ -54,6 +54,7 @@ handle-unknown return HANDLE_UNKNOWN;
bzip-blocksize return BZIP_BLOCKSIZE;
bzip-small return BZIP_SMALL;
remove-hll return REMOVE_HLL;
+retain-tmp return RETAIN_TMP;
"[load_policy]" return LOAD_POLICY_START;
"[setfiles]" return SETFILES_START;
"[sefcontext_compile]" return SEFCONTEXT_COMPILE_START;
diff --git a/libsemanage/src/direct_api.c b/libsemanage/src/direct_api.c
index a455612f..5d2a443c 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -326,7 +326,10 @@ static void semanage_direct_destroy(semanage_handle_t * sh
static int semanage_direct_disconnect(semanage_handle_t * sh)
{
/* destroy transaction */
- if (sh->is_in_transaction) {
+ if (sh->is_in_transaction)
+ semanage_release_trans_lock(sh);
Is it safe to release the trans lock before deleting the sandbox?
+
+ if (!sh->conf->retain_tmp && sh->is_in_transaction) {
/* destroy sandbox */
if (semanage_remove_directory
(semanage_path(SEMANAGE_TMP, SEMANAGE_TOPLEVEL)) < 0) {
@@ -342,7 +345,6 @@ static int semanage_direct_disconnect(semanage_handle_t * sh)
SEMANAGE_FINAL_TOPLEVEL));
return -1;
}
- semanage_release_trans_lock(sh);
}
/* Release object databases: local modifications */
free(fc_buffer);
- /* regardless if the commit was successful or not, remove the
- sandbox if it is still there */
- semanage_remove_directory(semanage_path
- (SEMANAGE_TMP, SEMANAGE_TOPLEVEL));
- semanage_remove_directory(semanage_final_path
- (SEMANAGE_FINAL_TMP,
- SEMANAGE_FINAL_TOPLEVEL));
+ /* Delete sandbox unless requested by semanage.conf */
+ if (!sh->conf->retain_tmp) {
+ semanage_remove_directory(semanage_path
+ (SEMANAGE_TMP,
SEMANAGE_TOPLEVEL));
+ semanage_remove_directory(semanage_final_path
+ (SEMANAGE_FINAL_TMP,
+ SEMANAGE_FINAL_TOPLEVEL));
+ }
umask(mask);
return retval;
diff --git a/libsemanage/src/semanage_conf.h
b/libsemanage/src/semanage_conf.h
index c99ac8c7..f7bba754 100644
--- a/libsemanage/src/semanage_conf.h
+++ b/libsemanage/src/semanage_conf.h
@@ -46,6 +46,7 @@ typedef struct semanage_conf {
int bzip_blocksize;
int bzip_small;
int remove_hll;
+ int retain_tmp;
int ignore_module_cache;
char *ignoredirs; /* ";" separated of list for genhomedircon
to ignore */
struct external_prog *load_policy;
--
2.14.3
--
Respectfully,

William C Roberts
Stephen Smalley
2018-01-16 16:00:46 UTC
Permalink
Post by William Roberts
On Mon, Jan 15, 2018 at 9:32 AM, Stephen Smalley
Post by Richard Haines
rnet.com>
Add new option to semanage.conf that allows the tmp build files
to be kept for debugging when building policy.
Would it be better to just retain the files by default if there is an error?
I thought about this as well, my reasoning as to why Richard's
approach was
better is that if someone does it N times trying to figure out an issue,
then there would be N piles of files in the tmp folder. This way they
have to opt in to have their tmp folder spammed.
I believe that the tmp directories are deleted and re-created by
libsemanage each time before use (otherwise we'd have a different
problem with not removing them, since we could end up with a mix of
files from different, incomplete transactions being intermingled
there). So I don't think this would be a problem. It might however
require saving the commit success/failure result in the handle so that
we know in semanage_direct_disconnect() whether or not we should delete
it.

If we truly need to make it optional, then I'd rather have it be an
option flag to semodule and a runtime setting of libsemanage (ala
reload, disable_dontaudit, etc) than a semanage.conf setting, as this
is something a user will want to be able to use without having to edit
a config file, re-run the transaction, and then re-edit the config file
each time. But I'm not convinced we can't just make it the default
behavior whenever the commit fails. Deleting the tmp files
automatically only really makes sense when it succeeds.
Post by William Roberts
Post by Richard Haines
---
 libsemanage/man/man5/semanage.conf.5 |  8 ++++++++
 libsemanage/src/conf-parse.y         | 15 ++++++++++++++-
 libsemanage/src/conf-scan.l          |  1 +
 libsemanage/src/direct_api.c         | 21 ++++++++++++---------
 libsemanage/src/semanage_conf.h      |  1 +
 5 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/libsemanage/man/man5/semanage.conf.5
b/libsemanage/man/man5/semanage.conf.5
index 8f8de55a..10cab65a 100644
--- a/libsemanage/man/man5/semanage.conf.5
+++ b/libsemanage/man/man5/semanage.conf.5
@@ -121,6 +121,14 @@ and by default it is set to "false".
 Please note that since this option deletes all HLL files, an
updated HLL
compiler will not be able to recompile the original HLL file into CIL.
 In order to compile the original HLL file into CIL, the same HLL
file will
need to be reinstalled.
+.TP
+.B retain-tmp
+When set to "true", tmp directories (the sandbox at
\fBstore-root/\fR[\fIpolicy-store\fR]\fB/tmp \fRand/or the final policy at
\fBstore-root/final/\fR[\fIpolicy-store\fR]) will be retained after
compilation to allow debugging of any build errors. Note that on a
successful build the sandbox becomes
\fBstore-root/\fR[\fIpolicy-store\fR]\fB/active\fR.
+.br
+The
+.B retain-tmp
+option can be set to either "true" or "false" and by default it is set to
"false".
+
 .SH "SEE ALSO"
 .TP
 semanage(8)
diff --git a/libsemanage/src/conf-parse.y b/libsemanage/src/conf-
parse.y
index b527e893..f098e55d 100644
--- a/libsemanage/src/conf-parse.y
+++ b/libsemanage/src/conf-parse.y
@@ -61,7 +61,7 @@ static int parse_errors;
 %token MODULE_STORE VERSION EXPAND_CHECK FILE_MODE SAVE_PREVIOUS
SAVE_LINKED TARGET_PLATFORM COMPILER_DIR IGNORE_MODULE_CACHE
STORE_ROOT
 %token LOAD_POLICY_START SETFILES_START SEFCONTEXT_COMPILE_START
DISABLE_GENHOMEDIRCON HANDLE_UNKNOWN USEPASSWD IGNOREDIRS
-%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL
+%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL RETAIN_TMP
 %token VERIFY_MOD_START VERIFY_LINKED_START VERIFY_KERNEL_START
BLOCK_END
 %token PROG_PATH PROG_ARGS
 %token <s> ARG
@@ -95,6 +95,7 @@ single_opt:     module_store
        |       bzip_blocksize
        |       bzip_small
        |       remove_hll
+       |       retain_tmp
         ;
 module_store:   MODULE_STORE '=' ARG {
@@ -268,6 +269,17 @@ remove_hll:  REMOVE_HLL'=' ARG {
        free($3);
 }
+retain_tmp:  RETAIN_TMP'=' ARG {
+       if (strcasecmp($3, "false") == 0) {
+               current_conf->retain_tmp = 0;
+       } else if (strcasecmp($3, "true") == 0) {
+               current_conf->retain_tmp = 1;
+       } else {
+               yyerror("retain-tmp can only be 'true' or
'false'");
+       }
+       free($3);
+}
+
                 command_start external_opts BLOCK_END  {
                         if (new_external->path == NULL) {
@@ -352,6 +364,7 @@ static int semanage_conf_init(semanage_conf_t * conf)
        conf->bzip_small = 0;
        conf->ignore_module_cache = 0;
        conf->remove_hll = 0;
+       conf->retain_tmp = 0;
        conf->save_previous = 0;
        conf->save_linked = 0;
diff --git a/libsemanage/src/conf-scan.l b/libsemanage/src/conf-
scan.l
index 607bbf0b..e26c3494 100644
--- a/libsemanage/src/conf-scan.l
+++ b/libsemanage/src/conf-scan.l
@@ -54,6 +54,7 @@ handle-unknown    return HANDLE_UNKNOWN;
 bzip-blocksize return BZIP_BLOCKSIZE;
 bzip-small     return BZIP_SMALL;
 remove-hll     return REMOVE_HLL;
+retain-tmp     return RETAIN_TMP;
 "[load_policy]"   return LOAD_POLICY_START;
 "[setfiles]"      return SETFILES_START;
 "[sefcontext_compile]"      return SEFCONTEXT_COMPILE_START;
diff --git a/libsemanage/src/direct_api.c
b/libsemanage/src/direct_api.c
index a455612f..5d2a443c 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -326,7 +326,10 @@ static void
semanage_direct_destroy(semanage_handle_t *
sh
 static int semanage_direct_disconnect(semanage_handle_t * sh)
 {
        /* destroy transaction */
-       if (sh->is_in_transaction) {
+       if (sh->is_in_transaction)
+               semanage_release_trans_lock(sh);
Is it safe to release the trans lock before deleting the sandbox?
+
+       if (!sh->conf->retain_tmp && sh->is_in_transaction) {
                /* destroy sandbox */
                if (semanage_remove_directory
                    (semanage_path(SEMANAGE_TMP,
SEMANAGE_TOPLEVEL)) < 0) {
@@ -342,7 +345,6 @@ static int
semanage_direct_disconnect(semanage_handle_t
* sh)
                                                SEMANAGE_FINAL_TOPL
EVEL));
                        return -1;
                }
-               semanage_release_trans_lock(sh);
        }
        /* Release object databases: local modifications */
        free(fc_buffer);
-       /* regardless if the commit was successful or not, remove
the
-          sandbox if it is still there */
-       semanage_remove_directory(semanage_path
-                                 (SEMANAGE_TMP,
SEMANAGE_TOPLEVEL));
-       semanage_remove_directory(semanage_final_path
-                                 (SEMANAGE_FINAL_TMP,
-                                  SEMANAGE_FINAL_TOPLEVEL));
+       /* Delete sandbox unless requested by semanage.conf */
+       if (!sh->conf->retain_tmp) {
+               semanage_remove_directory(semanage_path
+                                         (SEMANAGE_TMP,
SEMANAGE_TOPLEVEL));
+               semanage_remove_directory(semanage_final_path
+                                         (SEMANAGE_FINAL_TMP,
+                                          SEMANAGE_FINAL_TOPLEVEL)
);
+       }
        umask(mask);
        return retval;
diff --git a/libsemanage/src/semanage_conf.h
b/libsemanage/src/semanage_conf.h
index c99ac8c7..f7bba754 100644
--- a/libsemanage/src/semanage_conf.h
+++ b/libsemanage/src/semanage_conf.h
@@ -46,6 +46,7 @@ typedef struct semanage_conf {
        int bzip_blocksize;
        int bzip_small;
        int remove_hll;
+       int retain_tmp;
        int ignore_module_cache;
        char *ignoredirs;       /* ";" separated of list for
genhomedircon
to ignore */
        struct external_prog *load_policy;
--
2.14.3
William Roberts
2018-01-16 17:35:30 UTC
Permalink
Post by Stephen Smalley
Post by William Roberts
On Mon, Jan 15, 2018 at 9:32 AM, Stephen Smalley
Post by Richard Haines
rnet.com>
Add new option to semanage.conf that allows the tmp build files
to be kept for debugging when building policy.
Would it be better to just retain the files by default if there is an error?
I thought about this as well, my reasoning as to why Richard's approach was
better is that if someone does it N times trying to figure out an issue,
then there would be N piles of files in the tmp folder. This way they
have to opt in to have their tmp folder spammed.
I believe that the tmp directories are deleted and re-created by
libsemanage each time before use (otherwise we'd have a different
problem with not removing them, since we could end up with a mix of
files from different, incomplete transactions being intermingled
there). So I don't think this would be a problem. It might however
Oh I see it looks like its just generating a /tmp "store" directory
under the semanage path. I thought that enum was triggering a true
/tmp style thing. I should have looksed closer.
Post by Stephen Smalley
require saving the commit success/failure result in the handle so that
we know in semanage_direct_disconnect() whether or not we should delete
it.
Now that I understand that tid-bit, I think you're right, let's just
leave it on error.
Post by Stephen Smalley
If we truly need to make it optional, then I'd rather have it be an
option flag to semodule and a runtime setting of libsemanage (ala
reload, disable_dontaudit, etc) than a semanage.conf setting, as this
is something a user will want to be able to use without having to edit
a config file, re-run the transaction, and then re-edit the config file
each time. But I'm not convinced we can't just make it the default
behavior whenever the commit fails. Deleting the tmp files
automatically only really makes sense when it succeeds.
Post by William Roberts
Post by Richard Haines
---
libsemanage/man/man5/semanage.conf.5 | 8 ++++++++
libsemanage/src/conf-parse.y | 15 ++++++++++++++-
libsemanage/src/conf-scan.l | 1 +
libsemanage/src/direct_api.c | 21 ++++++++++++---------
libsemanage/src/semanage_conf.h | 1 +
5 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/libsemanage/man/man5/semanage.conf.5
b/libsemanage/man/man5/semanage.conf.5
index 8f8de55a..10cab65a 100644
--- a/libsemanage/man/man5/semanage.conf.5
+++ b/libsemanage/man/man5/semanage.conf.5
@@ -121,6 +121,14 @@ and by default it is set to "false".
Please note that since this option deletes all HLL files, an updated HLL
compiler will not be able to recompile the original HLL file into CIL.
In order to compile the original HLL file into CIL, the same HLL file will
need to be reinstalled.
+.TP
+.B retain-tmp
+When set to "true", tmp directories (the sandbox at
\fBstore-root/\fR[\fIpolicy-store\fR]\fB/tmp \fRand/or the final policy at
\fBstore-root/final/\fR[\fIpolicy-store\fR]) will be retained after
compilation to allow debugging of any build errors. Note that on a
successful build the sandbox becomes
\fBstore-root/\fR[\fIpolicy-store\fR]\fB/active\fR.
+.br
+The
+.B retain-tmp
+option can be set to either "true" or "false" and by default it is set to
"false".
+
.SH "SEE ALSO"
.TP
semanage(8)
diff --git a/libsemanage/src/conf-parse.y b/libsemanage/src/conf-
parse.y
index b527e893..f098e55d 100644
--- a/libsemanage/src/conf-parse.y
+++ b/libsemanage/src/conf-parse.y
@@ -61,7 +61,7 @@ static int parse_errors;
%token MODULE_STORE VERSION EXPAND_CHECK FILE_MODE SAVE_PREVIOUS
SAVE_LINKED TARGET_PLATFORM COMPILER_DIR IGNORE_MODULE_CACHE STORE_ROOT
%token LOAD_POLICY_START SETFILES_START SEFCONTEXT_COMPILE_START
DISABLE_GENHOMEDIRCON HANDLE_UNKNOWN USEPASSWD IGNOREDIRS
-%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL
+%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL RETAIN_TMP
%token VERIFY_MOD_START VERIFY_LINKED_START VERIFY_KERNEL_START BLOCK_END
%token PROG_PATH PROG_ARGS
%token <s> ARG
@@ -95,6 +95,7 @@ single_opt: module_store
| bzip_blocksize
| bzip_small
| remove_hll
+ | retain_tmp
;
module_store: MODULE_STORE '=' ARG {
@@ -268,6 +269,17 @@ remove_hll: REMOVE_HLL'=' ARG {
free($3);
}
+retain_tmp: RETAIN_TMP'=' ARG {
+ if (strcasecmp($3, "false") == 0) {
+ current_conf->retain_tmp = 0;
+ } else if (strcasecmp($3, "true") == 0) {
+ current_conf->retain_tmp = 1;
+ } else {
+ yyerror("retain-tmp can only be 'true' or
'false'");
+ }
+ free($3);
+}
+
command_start external_opts BLOCK_END {
if (new_external->path == NULL) {
@@ -352,6 +364,7 @@ static int semanage_conf_init(semanage_conf_t * conf)
conf->bzip_small = 0;
conf->ignore_module_cache = 0;
conf->remove_hll = 0;
+ conf->retain_tmp = 0;
conf->save_previous = 0;
conf->save_linked = 0;
diff --git a/libsemanage/src/conf-scan.l b/libsemanage/src/conf-
scan.l
index 607bbf0b..e26c3494 100644
--- a/libsemanage/src/conf-scan.l
+++ b/libsemanage/src/conf-scan.l
@@ -54,6 +54,7 @@ handle-unknown return HANDLE_UNKNOWN;
bzip-blocksize return BZIP_BLOCKSIZE;
bzip-small return BZIP_SMALL;
remove-hll return REMOVE_HLL;
+retain-tmp return RETAIN_TMP;
"[load_policy]" return LOAD_POLICY_START;
"[setfiles]" return SETFILES_START;
"[sefcontext_compile]" return SEFCONTEXT_COMPILE_START;
diff --git a/libsemanage/src/direct_api.c
b/libsemanage/src/direct_api.c
index a455612f..5d2a443c 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -326,7 +326,10 @@ static void
semanage_direct_destroy(semanage_handle_t *
sh
static int semanage_direct_disconnect(semanage_handle_t * sh)
{
/* destroy transaction */
- if (sh->is_in_transaction) {
+ if (sh->is_in_transaction)
+ semanage_release_trans_lock(sh);
Is it safe to release the trans lock before deleting the sandbox?
+
+ if (!sh->conf->retain_tmp && sh->is_in_transaction) {
/* destroy sandbox */
if (semanage_remove_directory
(semanage_path(SEMANAGE_TMP,
SEMANAGE_TOPLEVEL)) < 0) {
@@ -342,7 +345,6 @@ static int
semanage_direct_disconnect(semanage_handle_t
* sh)
SEMANAGE_FINAL_TOPL
EVEL));
return -1;
}
- semanage_release_trans_lock(sh);
}
/* Release object databases: local modifications */
free(fc_buffer);
- /* regardless if the commit was successful or not, remove the
- sandbox if it is still there */
- semanage_remove_directory(semanage_path
- (SEMANAGE_TMP,
SEMANAGE_TOPLEVEL));
- semanage_remove_directory(semanage_final_path
- (SEMANAGE_FINAL_TMP,
- SEMANAGE_FINAL_TOPLEVEL));
+ /* Delete sandbox unless requested by semanage.conf */
+ if (!sh->conf->retain_tmp) {
+ semanage_remove_directory(semanage_path
+ (SEMANAGE_TMP,
SEMANAGE_TOPLEVEL));
+ semanage_remove_directory(semanage_final_path
+ (SEMANAGE_FINAL_TMP,
+ SEMANAGE_FINAL_TOPLEVEL)
);
+ }
umask(mask);
return retval;
diff --git a/libsemanage/src/semanage_conf.h
b/libsemanage/src/semanage_conf.h
index c99ac8c7..f7bba754 100644
--- a/libsemanage/src/semanage_conf.h
+++ b/libsemanage/src/semanage_conf.h
@@ -46,6 +46,7 @@ typedef struct semanage_conf {
int bzip_blocksize;
int bzip_small;
int remove_hll;
+ int retain_tmp;
int ignore_module_cache;
char *ignoredirs; /* ";" separated of list for genhomedircon
to ignore */
struct external_prog *load_policy;
--
2.14.3
--
Respectfully,

William C Roberts
William Roberts
2018-01-19 20:04:46 UTC
Permalink
Richard, are you going to respin this?

On Tue, Jan 16, 2018 at 9:35 AM, William Roberts
Post by William Roberts
Post by Stephen Smalley
Post by William Roberts
On Mon, Jan 15, 2018 at 9:32 AM, Stephen Smalley
Post by Richard Haines
rnet.com>
Add new option to semanage.conf that allows the tmp build files
to be kept for debugging when building policy.
Would it be better to just retain the files by default if there is an error?
I thought about this as well, my reasoning as to why Richard's approach was
better is that if someone does it N times trying to figure out an issue,
then there would be N piles of files in the tmp folder. This way they
have to opt in to have their tmp folder spammed.
I believe that the tmp directories are deleted and re-created by
libsemanage each time before use (otherwise we'd have a different
problem with not removing them, since we could end up with a mix of
files from different, incomplete transactions being intermingled
there). So I don't think this would be a problem. It might however
Oh I see it looks like its just generating a /tmp "store" directory
under the semanage path. I thought that enum was triggering a true
/tmp style thing. I should have looksed closer.
Post by Stephen Smalley
require saving the commit success/failure result in the handle so that
we know in semanage_direct_disconnect() whether or not we should delete
it.
Now that I understand that tid-bit, I think you're right, let's just
leave it on error.
Post by Stephen Smalley
If we truly need to make it optional, then I'd rather have it be an
option flag to semodule and a runtime setting of libsemanage (ala
reload, disable_dontaudit, etc) than a semanage.conf setting, as this
is something a user will want to be able to use without having to edit
a config file, re-run the transaction, and then re-edit the config file
each time. But I'm not convinced we can't just make it the default
behavior whenever the commit fails. Deleting the tmp files
automatically only really makes sense when it succeeds.
Post by William Roberts
Post by Richard Haines
---
libsemanage/man/man5/semanage.conf.5 | 8 ++++++++
libsemanage/src/conf-parse.y | 15 ++++++++++++++-
libsemanage/src/conf-scan.l | 1 +
libsemanage/src/direct_api.c | 21 ++++++++++++---------
libsemanage/src/semanage_conf.h | 1 +
5 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/libsemanage/man/man5/semanage.conf.5
b/libsemanage/man/man5/semanage.conf.5
index 8f8de55a..10cab65a 100644
--- a/libsemanage/man/man5/semanage.conf.5
+++ b/libsemanage/man/man5/semanage.conf.5
@@ -121,6 +121,14 @@ and by default it is set to "false".
Please note that since this option deletes all HLL files, an updated HLL
compiler will not be able to recompile the original HLL file into CIL.
In order to compile the original HLL file into CIL, the same HLL file will
need to be reinstalled.
+.TP
+.B retain-tmp
+When set to "true", tmp directories (the sandbox at
\fBstore-root/\fR[\fIpolicy-store\fR]\fB/tmp \fRand/or the final policy at
\fBstore-root/final/\fR[\fIpolicy-store\fR]) will be retained after
compilation to allow debugging of any build errors. Note that on a
successful build the sandbox becomes
\fBstore-root/\fR[\fIpolicy-store\fR]\fB/active\fR.
+.br
+The
+.B retain-tmp
+option can be set to either "true" or "false" and by default it is set to
"false".
+
.SH "SEE ALSO"
.TP
semanage(8)
diff --git a/libsemanage/src/conf-parse.y b/libsemanage/src/conf-
parse.y
index b527e893..f098e55d 100644
--- a/libsemanage/src/conf-parse.y
+++ b/libsemanage/src/conf-parse.y
@@ -61,7 +61,7 @@ static int parse_errors;
%token MODULE_STORE VERSION EXPAND_CHECK FILE_MODE SAVE_PREVIOUS
SAVE_LINKED TARGET_PLATFORM COMPILER_DIR IGNORE_MODULE_CACHE STORE_ROOT
%token LOAD_POLICY_START SETFILES_START SEFCONTEXT_COMPILE_START
DISABLE_GENHOMEDIRCON HANDLE_UNKNOWN USEPASSWD IGNOREDIRS
-%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL
+%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL RETAIN_TMP
%token VERIFY_MOD_START VERIFY_LINKED_START VERIFY_KERNEL_START BLOCK_END
%token PROG_PATH PROG_ARGS
%token <s> ARG
@@ -95,6 +95,7 @@ single_opt: module_store
| bzip_blocksize
| bzip_small
| remove_hll
+ | retain_tmp
;
module_store: MODULE_STORE '=' ARG {
@@ -268,6 +269,17 @@ remove_hll: REMOVE_HLL'=' ARG {
free($3);
}
+retain_tmp: RETAIN_TMP'=' ARG {
+ if (strcasecmp($3, "false") == 0) {
+ current_conf->retain_tmp = 0;
+ } else if (strcasecmp($3, "true") == 0) {
+ current_conf->retain_tmp = 1;
+ } else {
+ yyerror("retain-tmp can only be 'true' or
'false'");
+ }
+ free($3);
+}
+
command_start external_opts BLOCK_END {
if (new_external->path == NULL) {
@@ -352,6 +364,7 @@ static int semanage_conf_init(semanage_conf_t * conf)
conf->bzip_small = 0;
conf->ignore_module_cache = 0;
conf->remove_hll = 0;
+ conf->retain_tmp = 0;
conf->save_previous = 0;
conf->save_linked = 0;
diff --git a/libsemanage/src/conf-scan.l b/libsemanage/src/conf-
scan.l
index 607bbf0b..e26c3494 100644
--- a/libsemanage/src/conf-scan.l
+++ b/libsemanage/src/conf-scan.l
@@ -54,6 +54,7 @@ handle-unknown return HANDLE_UNKNOWN;
bzip-blocksize return BZIP_BLOCKSIZE;
bzip-small return BZIP_SMALL;
remove-hll return REMOVE_HLL;
+retain-tmp return RETAIN_TMP;
"[load_policy]" return LOAD_POLICY_START;
"[setfiles]" return SETFILES_START;
"[sefcontext_compile]" return SEFCONTEXT_COMPILE_START;
diff --git a/libsemanage/src/direct_api.c
b/libsemanage/src/direct_api.c
index a455612f..5d2a443c 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -326,7 +326,10 @@ static void
semanage_direct_destroy(semanage_handle_t *
sh
static int semanage_direct_disconnect(semanage_handle_t * sh)
{
/* destroy transaction */
- if (sh->is_in_transaction) {
+ if (sh->is_in_transaction)
+ semanage_release_trans_lock(sh);
Is it safe to release the trans lock before deleting the sandbox?
+
+ if (!sh->conf->retain_tmp && sh->is_in_transaction) {
/* destroy sandbox */
if (semanage_remove_directory
(semanage_path(SEMANAGE_TMP,
SEMANAGE_TOPLEVEL)) < 0) {
@@ -342,7 +345,6 @@ static int
semanage_direct_disconnect(semanage_handle_t
* sh)
SEMANAGE_FINAL_TOPL
EVEL));
return -1;
}
- semanage_release_trans_lock(sh);
}
/* Release object databases: local modifications */
free(fc_buffer);
- /* regardless if the commit was successful or not, remove the
- sandbox if it is still there */
- semanage_remove_directory(semanage_path
- (SEMANAGE_TMP,
SEMANAGE_TOPLEVEL));
- semanage_remove_directory(semanage_final_path
- (SEMANAGE_FINAL_TMP,
- SEMANAGE_FINAL_TOPLEVEL));
+ /* Delete sandbox unless requested by semanage.conf */
+ if (!sh->conf->retain_tmp) {
+ semanage_remove_directory(semanage_path
+ (SEMANAGE_TMP,
SEMANAGE_TOPLEVEL));
+ semanage_remove_directory(semanage_final_path
+ (SEMANAGE_FINAL_TMP,
+ SEMANAGE_FINAL_TOPLEVEL)
);
+ }
umask(mask);
return retval;
diff --git a/libsemanage/src/semanage_conf.h
b/libsemanage/src/semanage_conf.h
index c99ac8c7..f7bba754 100644
--- a/libsemanage/src/semanage_conf.h
+++ b/libsemanage/src/semanage_conf.h
@@ -46,6 +46,7 @@ typedef struct semanage_conf {
int bzip_blocksize;
int bzip_small;
int remove_hll;
+ int retain_tmp;
int ignore_module_cache;
char *ignoredirs; /* ";" separated of list for genhomedircon
to ignore */
struct external_prog *load_policy;
--
2.14.3
--
Respectfully,
William C Roberts
--
Respectfully,

William C Roberts
Richard Haines
2018-01-20 10:38:03 UTC
Permalink
Post by William Roberts
Richard, are you going to respin this?
Yes - Next week
Post by William Roberts
On Tue, Jan 16, 2018 at 9:35 AM, William Roberts
Post by William Roberts
Post by Stephen Smalley
Post by William Roberts
On Mon, Jan 15, 2018 at 9:32 AM, Stephen Smalley
Post by Richard Haines
btinte
rnet.com>
Add new option to semanage.conf that allows the tmp build files
to be kept for debugging when building policy.
Would it be better to just retain the files by default if
there is
an error?
I thought about this as well, my reasoning as to why Richard's approach was
better is that if someone does it N times trying to figure out
an
issue,
then there would be N piles of files in the tmp folder. This way they
have to opt in to have their tmp folder spammed.
I believe that the tmp directories are deleted and re-created by
libsemanage each time before use (otherwise we'd have a different
problem with not removing them, since we could end up with a mix of
files from different, incomplete transactions being intermingled
there). So I don't think this would be a problem. It might however
Oh I see it looks like its just generating a /tmp "store" directory
under the semanage path. I thought that enum was triggering a true
/tmp style thing. I should have looksed closer.
Post by Stephen Smalley
require saving the commit success/failure result in the handle so that
we know in semanage_direct_disconnect() whether or not we should delete
it.
Now that I understand that tid-bit, I think you're right, let's just
leave it on error.
Post by Stephen Smalley
If we truly need to make it optional, then I'd rather have it be an
option flag to semodule and a runtime setting of libsemanage (ala
reload, disable_dontaudit, etc) than a semanage.conf setting, as this
is something a user will want to be able to use without having to edit
a config file, re-run the transaction, and then re-edit the config file
each time. But I'm not convinced we can't just make it the default
behavior whenever the commit fails. Deleting the tmp files
automatically only really makes sense when it succeeds.
Post by William Roberts
Post by Richard Haines
m>
---
libsemanage/man/man5/semanage.conf.5 | 8 ++++++++
libsemanage/src/conf-parse.y | 15 ++++++++++++++-
libsemanage/src/conf-scan.l | 1 +
libsemanage/src/direct_api.c | 21 ++++++++++++----
-----
libsemanage/src/semanage_conf.h | 1 +
5 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/libsemanage/man/man5/semanage.conf.5
b/libsemanage/man/man5/semanage.conf.5
index 8f8de55a..10cab65a 100644
--- a/libsemanage/man/man5/semanage.conf.5
+++ b/libsemanage/man/man5/semanage.conf.5
@@ -121,6 +121,14 @@ and by default it is set to "false".
Please note that since this option deletes all HLL files, an updated HLL
compiler will not be able to recompile the original HLL file
into
CIL.
In order to compile the original HLL file into CIL, the same
HLL
file will
need to be reinstalled.
+.TP
+.B retain-tmp
+When set to "true", tmp directories (the sandbox at
\fBstore-root/\fR[\fIpolicy-store\fR]\fB/tmp \fRand/or the
final
policy at
\fBstore-root/final/\fR[\fIpolicy-store\fR]) will be retained after
compilation to allow debugging of any build errors. Note that on a
successful build the sandbox becomes
\fBstore-root/\fR[\fIpolicy-store\fR]\fB/active\fR.
+.br
+The
+.B retain-tmp
+option can be set to either "true" or "false" and by default
it is
set to
"false".
+
.SH "SEE ALSO"
.TP
semanage(8)
diff --git a/libsemanage/src/conf-parse.y
b/libsemanage/src/conf-
parse.y
index b527e893..f098e55d 100644
--- a/libsemanage/src/conf-parse.y
+++ b/libsemanage/src/conf-parse.y
@@ -61,7 +61,7 @@ static int parse_errors;
%token MODULE_STORE VERSION EXPAND_CHECK FILE_MODE
SAVE_PREVIOUS
SAVE_LINKED TARGET_PLATFORM COMPILER_DIR IGNORE_MODULE_CACHE STORE_ROOT
%token LOAD_POLICY_START SETFILES_START
SEFCONTEXT_COMPILE_START
DISABLE_GENHOMEDIRCON HANDLE_UNKNOWN USEPASSWD IGNOREDIRS
-%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL
+%token BZIP_BLOCKSIZE BZIP_SMALL REMOVE_HLL RETAIN_TMP
%token VERIFY_MOD_START VERIFY_LINKED_START
VERIFY_KERNEL_START
BLOCK_END
%token PROG_PATH PROG_ARGS
%token <s> ARG
@@ -95,6 +95,7 @@ single_opt: module_store
| bzip_blocksize
| bzip_small
| remove_hll
+ | retain_tmp
;
module_store: MODULE_STORE '=' ARG {
@@ -268,6 +269,17 @@ remove_hll: REMOVE_HLL'=' ARG {
free($3);
}
+retain_tmp: RETAIN_TMP'=' ARG {
+ if (strcasecmp($3, "false") == 0) {
+ current_conf->retain_tmp = 0;
+ } else if (strcasecmp($3, "true") == 0) {
+ current_conf->retain_tmp = 1;
+ } else {
+ yyerror("retain-tmp can only be 'true' or 'false'");
+ }
+ free($3);
+}
+
command_start external_opts BLOCK_END {
if (new_external->path == NULL) {
@@ -352,6 +364,7 @@ static int
semanage_conf_init(semanage_conf_t *
conf)
conf->bzip_small = 0;
conf->ignore_module_cache = 0;
conf->remove_hll = 0;
+ conf->retain_tmp = 0;
conf->save_previous = 0;
conf->save_linked = 0;
diff --git a/libsemanage/src/conf-scan.l
b/libsemanage/src/conf-
scan.l
index 607bbf0b..e26c3494 100644
--- a/libsemanage/src/conf-scan.l
+++ b/libsemanage/src/conf-scan.l
@@ -54,6 +54,7 @@ handle-unknown return HANDLE_UNKNOWN;
bzip-blocksize return BZIP_BLOCKSIZE;
bzip-small return BZIP_SMALL;
remove-hll return REMOVE_HLL;
+retain-tmp return RETAIN_TMP;
"[load_policy]" return LOAD_POLICY_START;
"[setfiles]" return SETFILES_START;
"[sefcontext_compile]" return SEFCONTEXT_COMPILE_START;
diff --git a/libsemanage/src/direct_api.c
b/libsemanage/src/direct_api.c
index a455612f..5d2a443c 100644
--- a/libsemanage/src/direct_api.c
+++ b/libsemanage/src/direct_api.c
@@ -326,7 +326,10 @@ static void
semanage_direct_destroy(semanage_handle_t *
sh
static int semanage_direct_disconnect(semanage_handle_t * sh)
{
/* destroy transaction */
- if (sh->is_in_transaction) {
+ if (sh->is_in_transaction)
+ semanage_release_trans_lock(sh);
Is it safe to release the trans lock before deleting the sandbox?
+
+ if (!sh->conf->retain_tmp && sh->is_in_transaction) {
/* destroy sandbox */
if (semanage_remove_directory
(semanage_path(SEMANAGE_TMP,
SEMANAGE_TOPLEVEL)) < 0) {
@@ -342,7 +345,6 @@ static int
semanage_direct_disconnect(semanage_handle_t
* sh)
SEMANAGE_FINA
L_TOPL
EVEL));
return -1;
}
- semanage_release_trans_lock(sh);
}
/* Release object databases: local modifications */
free(fc_buffer);
- /* regardless if the commit was successful or not,
remove
the
- sandbox if it is still there */
- semanage_remove_directory(semanage_path
- (SEMANAGE_TMP,
SEMANAGE_TOPLEVEL));
- semanage_remove_directory(semanage_final_path
- (SEMANAGE_FINAL_TMP,
- SEMANAGE_FINAL_TOPLEVEL));
+ /* Delete sandbox unless requested by semanage.conf */
+ if (!sh->conf->retain_tmp) {
+ semanage_remove_directory(semanage_path
+ (SEMANAGE_TMP,
SEMANAGE_TOPLEVEL));
+ semanage_remove_directory(semanage_final_path
+ (SEMANAGE_FINAL_TMP
,
+ SEMANAGE_FINAL_TOP
LEVEL)
);
+ }
umask(mask);
return retval;
diff --git a/libsemanage/src/semanage_conf.h
b/libsemanage/src/semanage_conf.h
index c99ac8c7..f7bba754 100644
--- a/libsemanage/src/semanage_conf.h
+++ b/libsemanage/src/semanage_conf.h
@@ -46,6 +46,7 @@ typedef struct semanage_conf {
int bzip_blocksize;
int bzip_small;
int remove_hll;
+ int retain_tmp;
int ignore_module_cache;
char *ignoredirs; /* ";" separated of list for genhomedircon
to ignore */
struct external_prog *load_policy;
--
2.14.3
--
Respectfully,
William C Roberts
Loading...