This patch properly completes the implementation of the MLS file relabel attributes. In the previous patch [http://oss.tresys.com/pipermail/refpolicy/2016-July/008038.html], a new attribute, mlsfilerelabetoclr, was created. There should have been a second attribute, mlsfilerelabel, created instead of overloading mlsfilewrite for this privilege. I concur with creating new attributes for this situation. I have created the patch below.
Signed-off-by: Chad Hanson <[email protected]>
---
policy/mls | 2 +-
policy/modules/kernel/mls.if | 28 ++++++++++++++++++++++++----
policy/modules/kernel/mls.te | 3 ++-
3 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/policy/mls b/policy/mls
index 2dadd205..73ff301b 100644
--- a/policy/mls
+++ b/policy/mls
@@ -72,7 +72,7 @@ mlsconstrain { file lnk_file fifo_file } { create relabelto }
mlsconstrain { dir file lnk_file chr_file blk_file sock_file fifo_file } relabelto
(( h1 dom h2 ) or
(( t1 == mlsfilerelabeltoclr ) and ( h1 dom l2 )) or
- ( t1 == mlsfilewrite ));
+ ( t1 == mlsfilerelabel ));
# the file "read" ops (note the check is dominance of the low level)
mlsconstrain { dir file lnk_file chr_file blk_file sock_file fifo_file } { read getattr execute }
diff --git a/policy/modules/kernel/mls.if b/policy/modules/kernel/mls.if
index b09c0a5a..2e2bebc2 100644
--- a/policy/modules/kernel/mls.if
+++ b/policy/modules/kernel/mls.if
@@ -71,6 +71,26 @@ interface(`mls_file_write_to_clearance',`
typeattribute $1 mlsfilewritetoclr;
')
+########################################
+## <summary>
+## Make specified domain MLS trusted
+## for writing to files at all levels.
+## </summary>
+## <param name="domain">
+## <summary>
+## Domain allowed access.
+## </summary>
+## </param>
+## <rolecap/>
+#
+interface(`mls_file_write_all_levels',`
+ gen_require(`
+ attribute mlsfilewrite;
+ ')
+
+ typeattribute $1 mlsfilewrite;
+')
+
########################################
## <summary>
## Make specified domain MLS trusted
@@ -94,7 +114,7 @@ interface(`mls_file_relabel_to_clearance',`
########################################
## <summary>
## Make specified domain MLS trusted
-## for writing to files at all levels.
+## for relabelto to files at all levels.
## </summary>
## <param name="domain">
## <summary>
@@ -103,12 +123,12 @@ interface(`mls_file_relabel_to_clearance',`
## </param>
## <rolecap/>
#
-interface(`mls_file_write_all_levels',`
+interface(`mls_file_relabel',`
gen_require(`
- attribute mlsfilewrite;
+ attribute mlsfilerelabel;
')
- typeattribute $1 mlsfilewrite;
+ typeattribute $1 mlsfilerelabel;
')
########################################
diff --git a/policy/modules/kernel/mls.te b/policy/modules/kernel/mls.te
index ad74e81f..7c50e75c 100644
--- a/policy/modules/kernel/mls.te
+++ b/policy/modules/kernel/mls.te
@@ -10,9 +10,10 @@ attribute mlsfilereadtoclr;
attribute mlsfilewrite;
attribute mlsfilewritetoclr;
attribute mlsfilewriteinrange;
+attribute mlsfilerelabel;
+attribute mlsfilerelabeltoclr;
attribute mlsfileupgrade;
attribute mlsfiledowngrade;
-attribute mlsfilerelabeltoclr;
attribute mlsnetread;
attribute mlsnetreadtoclr;
--
2.14.1
On 12/10/2017 11:02 PM, Chad Hanson via refpolicy wrote:
> This patch properly completes the implementation of the MLS file relabel attributes. In the previous patch [http://oss.tresys.com/pipermail/refpolicy/2016-July/008038.html], a new attribute, mlsfilerelabetoclr, was created. There should have been a second attribute, mlsfilerelabel, created instead of overloading mlsfilewrite for this privilege. I concur with creating new attributes for this situation. I have created the patch below.
>
> Signed-off-by: Chad Hanson <[email protected]>
> ---
> policy/mls | 2 +-
> policy/modules/kernel/mls.if | 28 ++++++++++++++++++++++++----
> policy/modules/kernel/mls.te | 3 ++-
> 3 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/policy/mls b/policy/mls
> index 2dadd205..73ff301b 100644
> --- a/policy/mls
> +++ b/policy/mls
> @@ -72,7 +72,7 @@ mlsconstrain { file lnk_file fifo_file } { create relabelto }
> mlsconstrain { dir file lnk_file chr_file blk_file sock_file fifo_file } relabelto
> (( h1 dom h2 ) or
> (( t1 == mlsfilerelabeltoclr ) and ( h1 dom l2 )) or
> - ( t1 == mlsfilewrite ));
> + ( t1 == mlsfilerelabel ));
It seems like both conditions should actually be included, not the
mlsfilerelabel replacing the mlsfilewrite. That way, if you use the
latter, it's still comprehensive.
> # the file "read" ops (note the check is dominance of the low level)
> mlsconstrain { dir file lnk_file chr_file blk_file sock_file fifo_file } { read getattr execute }
> diff --git a/policy/modules/kernel/mls.if b/policy/modules/kernel/mls.if
> index b09c0a5a..2e2bebc2 100644
> --- a/policy/modules/kernel/mls.if
> +++ b/policy/modules/kernel/mls.if
> @@ -71,6 +71,26 @@ interface(`mls_file_write_to_clearance',`
> typeattribute $1 mlsfilewritetoclr;
> ')
>
> +########################################
> +## <summary>
> +## Make specified domain MLS trusted
> +## for writing to files at all levels.
> +## </summary>
> +## <param name="domain">
> +## <summary>
> +## Domain allowed access.
> +## </summary>
> +## </param>
> +## <rolecap/>
> +#
> +interface(`mls_file_write_all_levels',`
> + gen_require(`
> + attribute mlsfilewrite;
> + ')
> +
> + typeattribute $1 mlsfilewrite;
> +')
> +
> ########################################
> ## <summary>
> ## Make specified domain MLS trusted
> @@ -94,7 +114,7 @@ interface(`mls_file_relabel_to_clearance',`
> ########################################
> ## <summary>
> ## Make specified domain MLS trusted
> -## for writing to files at all levels.
> +## for relabelto to files at all levels.
> ## </summary>
> ## <param name="domain">
> ## <summary>
> @@ -103,12 +123,12 @@ interface(`mls_file_relabel_to_clearance',`
> ## </param>
> ## <rolecap/>
> #
> -interface(`mls_file_write_all_levels',`
> +interface(`mls_file_relabel',`
> gen_require(`
> - attribute mlsfilewrite;
> + attribute mlsfilerelabel;
> ')
>
> - typeattribute $1 mlsfilewrite;
> + typeattribute $1 mlsfilerelabel;
> ')
Interfaces shouldn't be removed. Instead they should be deprecated....
assuming we go forward with the above change.
> ########################################
> diff --git a/policy/modules/kernel/mls.te b/policy/modules/kernel/mls.te
> index ad74e81f..7c50e75c 100644
> --- a/policy/modules/kernel/mls.te
> +++ b/policy/modules/kernel/mls.te
> @@ -10,9 +10,10 @@ attribute mlsfilereadtoclr;
> attribute mlsfilewrite;
> attribute mlsfilewritetoclr;
> attribute mlsfilewriteinrange;
> +attribute mlsfilerelabel;
> +attribute mlsfilerelabeltoclr;
> attribute mlsfileupgrade;
> attribute mlsfiledowngrade;
> -attribute mlsfilerelabeltoclr;
>
> attribute mlsnetread;
> attribute mlsnetreadtoclr;
>
--
Chris PeBenito
On Mon, Dec 11, 2017 at 07:01:18PM -0500, Chris PeBenito wrote:
> On 12/10/2017 11:02 PM, Chad Hanson via refpolicy wrote:
> >This patch properly completes the implementation of the MLS file relabel attributes. In the previous patch [http://oss.tresys.com/pipermail/refpolicy/2016-July/008038.html], a new attribute, mlsfilerelabetoclr, was created. There should have been a second attribute, mlsfilerelabel, created instead of overloading mlsfilewrite for this privilege. I concur with creating new attributes for this situation. I have created the patch below.
> >
> >Signed-off-by: Chad Hanson <[email protected]>
> >---
> > policy/mls | 2 +-
> > policy/modules/kernel/mls.if | 28 ++++++++++++++++++++++++----
> > policy/modules/kernel/mls.te | 3 ++-
> > 3 files changed, 27 insertions(+), 6 deletions(-)
> >
> >diff --git a/policy/mls b/policy/mls
> >index 2dadd205..73ff301b 100644
> >--- a/policy/mls
> >+++ b/policy/mls
> >@@ -72,7 +72,7 @@ mlsconstrain { file lnk_file fifo_file } { create relabelto }
> > mlsconstrain { dir file lnk_file chr_file blk_file sock_file fifo_file } relabelto
> > (( h1 dom h2 ) or
> > (( t1 == mlsfilerelabeltoclr ) and ( h1 dom l2 )) or
> >- ( t1 == mlsfilewrite ));
> >+ ( t1 == mlsfilerelabel ));
>
> It seems like both conditions should actually be included, not the
> mlsfilerelabel replacing the mlsfilewrite. That way, if you use the
> latter, it's still comprehensive.
>
I would normally agree, I just don't think mlsfilewrite should ever been
allowed for "relabelto" operations. The combination of a new
mlsfilerelabeltoclr combined with mlsfilewrite doesn't make sense. It
should either have been mlsfilewrite[toclr] or mlsfilerelabel[toclr]. My
personal opinion is that relabel is much different than write. In MLS, a
process can write to a file without having read access. A relabel can
lead to new accesses since this is a file attribute change. This wasn't
bypassable originally, that is why a new attribute seems to make sense.
> > # the file "read" ops (note the check is dominance of the low level)
> > mlsconstrain { dir file lnk_file chr_file blk_file sock_file fifo_file } { read getattr execute }
> >diff --git a/policy/modules/kernel/mls.if b/policy/modules/kernel/mls.if
> >index b09c0a5a..2e2bebc2 100644
> >--- a/policy/modules/kernel/mls.if
> >+++ b/policy/modules/kernel/mls.if
> >@@ -71,6 +71,26 @@ interface(`mls_file_write_to_clearance',`
> > typeattribute $1 mlsfilewritetoclr;
> > ')
> >+########################################
> >+## <summary>
> >+## Make specified domain MLS trusted
> >+## for writing to files at all levels.
> >+## </summary>
> >+## <param name="domain">
> >+## <summary>
> >+## Domain allowed access.
> >+## </summary>
> >+## </param>
> >+## <rolecap/>
> >+#
> >+interface(`mls_file_write_all_levels',`
> >+ gen_require(`
> >+ attribute mlsfilewrite;
> >+ ')
> >+
> >+ typeattribute $1 mlsfilewrite;
> >+')
> >+
> > ########################################
> > ## <summary>
> > ## Make specified domain MLS trusted
> >@@ -94,7 +114,7 @@ interface(`mls_file_relabel_to_clearance',`
> > ########################################
> > ## <summary>
> > ## Make specified domain MLS trusted
> >-## for writing to files at all levels.
> >+## for relabelto to files at all levels.
> > ## </summary>
> > ## <param name="domain">
> > ## <summary>
> >@@ -103,12 +123,12 @@ interface(`mls_file_relabel_to_clearance',`
> > ## </param>
> > ## <rolecap/>
> > #
> >-interface(`mls_file_write_all_levels',`
> >+interface(`mls_file_relabel',`
> > gen_require(`
> >- attribute mlsfilewrite;
> >+ attribute mlsfilerelabel;
> > ')
> >- typeattribute $1 mlsfilewrite;
> >+ typeattribute $1 mlsfilerelabel;
> > ')
>
> Interfaces shouldn't be removed. Instead they should be
> deprecated.... assuming we go forward with the above change.
>
I didn't remove an interface here, I just moved one of the new
interfaces to have them together after the mlsfilewrite interfaces
instead of interspersed within. The diff just doesn't reflect that
very well.
> > ########################################
> >diff --git a/policy/modules/kernel/mls.te b/policy/modules/kernel/mls.te
> >index ad74e81f..7c50e75c 100644
> >--- a/policy/modules/kernel/mls.te
> >+++ b/policy/modules/kernel/mls.te
> >@@ -10,9 +10,10 @@ attribute mlsfilereadtoclr;
> > attribute mlsfilewrite;
> > attribute mlsfilewritetoclr;
> > attribute mlsfilewriteinrange;
> >+attribute mlsfilerelabel;
> >+attribute mlsfilerelabeltoclr;
> > attribute mlsfileupgrade;
> > attribute mlsfiledowngrade;
> >-attribute mlsfilerelabeltoclr;
> > attribute mlsnetread;
> > attribute mlsnetreadtoclr;
> >
>
>
> --
> Chris PeBenito
On 12/12/2017 12:49 AM, Chad Hanson wrote:
> On Mon, Dec 11, 2017 at 07:01:18PM -0500, Chris PeBenito wrote:
>> On 12/10/2017 11:02 PM, Chad Hanson via refpolicy wrote:
>>> This patch properly completes the implementation of the MLS file relabel attributes. In the previous patch [http://oss.tresys.com/pipermail/refpolicy/2016-July/008038.html], a new attribute, mlsfilerelabetoclr, was created. There should have been a second attribute, mlsfilerelabel, created instead of overloading mlsfilewrite for this privilege. I concur with creating new attributes for this situation. I have created the patch below.
>>>
>>> Signed-off-by: Chad Hanson <[email protected]>
>>> ---
>>> policy/mls | 2 +-
>>> policy/modules/kernel/mls.if | 28 ++++++++++++++++++++++++----
>>> policy/modules/kernel/mls.te | 3 ++-
>>> 3 files changed, 27 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/policy/mls b/policy/mls
>>> index 2dadd205..73ff301b 100644
>>> --- a/policy/mls
>>> +++ b/policy/mls
>>> @@ -72,7 +72,7 @@ mlsconstrain { file lnk_file fifo_file } { create relabelto }
>>> mlsconstrain { dir file lnk_file chr_file blk_file sock_file fifo_file } relabelto
>>> (( h1 dom h2 ) or
>>> (( t1 == mlsfilerelabeltoclr ) and ( h1 dom l2 )) or
>>> - ( t1 == mlsfilewrite ));
>>> + ( t1 == mlsfilerelabel ));
>>
>> It seems like both conditions should actually be included, not the
>> mlsfilerelabel replacing the mlsfilewrite. That way, if you use the
>> latter, it's still comprehensive.
>>
>
> I would normally agree, I just don't think mlsfilewrite should ever been
> allowed for "relabelto" operations. The combination of a new
> mlsfilerelabeltoclr combined with mlsfilewrite doesn't make sense. It
> should either have been mlsfilewrite[toclr] or mlsfilerelabel[toclr]. My
> personal opinion is that relabel is much different than write. In MLS, a
> process can write to a file without having read access. A relabel can
> lead to new accesses since this is a file attribute change. This wasn't
> bypassable originally, that is why a new attribute seems to make sense.
That makes sense. Merged.
--
Chris PeBenito