On Fri, 2018-01-19 at 12:04 -0800, William Roberts wrote:
> Richard, are you going to respin this?
Yes - Next week
>
> On Tue, Jan 16, 2018 at 9:35 AM, William Roberts
> <bill.c.roberts@xxxxxxxxx> wrote:
> > On Tue, Jan 16, 2018 at 8:00 AM, Stephen Smalley <sds@xxxxxxxxxxxxx
> > > wrote:
> > > On Tue, 2018-01-16 at 07:47 -0800, William Roberts wrote:
> > > > On Mon, Jan 15, 2018 at 9:32 AM, Stephen Smalley
> > > > <stephen.smalley@xxxxxxxxx> wrote:
> > > > > On Jan 14, 2018 10:36 AM, "Richard Haines" <richard_c_haines@
> > > > > btinte
> > > > > rnet.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?
> > > >
> > > > 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.
> >
> > > 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.
> >
> > >
> > > 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.
> >
> >
> >
> > >
> > > >
> > > > >
> > > > >
> > > > > Signed-off-by: Richard Haines <richard_c_haines@xxxxxxxxxxxxx
> > > > > 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_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_FINA
> > > > > L_TOPL
> > > > > EVEL));
> > > > > 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_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
>
>
>
