Re: [PATCH] libsemanage: Allow tmp files to be kept if a compile fails

[Richard Haines]

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
>  wrote:
> > On Tue, Jan 16, 2018 at 8:00 AM, Stephen Smalley  > > wrote:
> > > On Tue, 2018-01-16 at 07:47 -0800, William Roberts wrote:
> > > > On Mon, Jan 15, 2018 at 9:32 AM, Stephen Smalley
> > > >  wrote:
> > > > > On Jan 14, 2018 10:36 AM, "Richard 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  > > > > 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 

Re: [PATCH] libsemanage: Allow tmp files to be kept if a compile fails

[William Roberts]

Richard, are you going to respin this?

On Tue, Jan 16, 2018 at 9:35 AM, William Roberts
 wrote:
> On Tue, Jan 16, 2018 at 8:00 AM, Stephen Smalley  wrote:
>> On Tue, 2018-01-16 at 07:47 -0800, William Roberts wrote:
>>> On Mon, Jan 15, 2018 at 9:32 AM, Stephen Smalley
>>>  wrote:
>>> > On Jan 14, 2018 10:36 AM, "Richard Haines" >> > 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 
>>> > ---
>>> >  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  ARG
>>> > @@ -95,6 +95,7 @@ single_opt: module_store
>>> > |   bzip_blocksize
>>> > |   bzip_small
>>> > |   remove_hll
>>> > +   

Re: [PATCH] libsemanage: Allow tmp files to be kept if a compile fails

[William Roberts]

On Tue, Jan 16, 2018 at 8:00 AM, Stephen Smalley  wrote:
> On Tue, 2018-01-16 at 07:47 -0800, William Roberts wrote:
>> On Mon, Jan 15, 2018 at 9:32 AM, Stephen Smalley
>>  wrote:
>> > On Jan 14, 2018 10:36 AM, "Richard Haines" > > 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 
>> > ---
>> >  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  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 

Re: [PATCH] libsemanage: Allow tmp files to be kept if a compile fails

[Stephen Smalley]

On Tue, 2018-01-16 at 07:47 -0800, William Roberts wrote:
> On Mon, Jan 15, 2018 at 9:32 AM, Stephen Smalley
>  wrote:
> > On Jan 14, 2018 10:36 AM, "Richard Haines"  > 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
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.

> 
> > 
> > 
> > Signed-off-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  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) {
> > @@ 

Re: [PATCH] libsemanage: Allow tmp files to be kept if a compile fails

[William Roberts]

On Mon, Jan 15, 2018 at 9:32 AM, Stephen Smalley
 wrote:
> On Jan 14, 2018 10:36 AM, "Richard Haines" 
> 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.

>
>
> Signed-off-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  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-unknownreturn 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);

Re: [PATCH] libsemanage: Allow tmp files to be kept if a compile fails

[Stephen Smalley]

On Jan 14, 2018 10:36 AM, "Richard Haines" 
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 
---
 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  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-unknownreturn 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);
}

/* 

Re: [PATCH] libsemanage: Allow tmp files to be kept if a compile fails

[William Roberts]

On Mon, Jan 15, 2018 at 8:39 AM, Richard Haines
 wrote:
> On Mon, 2018-01-15 at 07:46 -0800, William Roberts wrote:
>> On Sun, Jan 14, 2018 at 7:34 AM, Richard Haines
>>  wrote:
>> > 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
> 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.

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.

>
>>
>> >
>> > Signed-off-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  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)
>> > 

Re: [PATCH] libsemanage: Allow tmp files to be kept if a compile fails

[Petr Lautrbach]

On Mon, Jan 15, 2018 at 07:46:27AM -0800, William Roberts wrote:
> On Sun, Jan 14, 2018 at 7:34 AM, Richard Haines
>  wrote:
> > 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
...


 
> >
> > Signed-off-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  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-unknownreturn 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 

Re: [PATCH] libsemanage: Allow tmp files to be kept if a compile fails

[William Roberts]

On Sun, Jan 14, 2018 at 7:34 AM, Richard Haines
 wrote:
> 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?

>
> Signed-off-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  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-unknownreturn 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 

[PATCH] libsemanage: Allow tmp files to be kept if a compile fails

[Richard Haines]

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 
---
 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  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-unknownreturn 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 */