Skip to contentSkip to conversation

Fix implementation of MLS file relabel attributes

PATCH New Chad Hanson 2 participants 4 messages Dec 11, 2017 – Dec 13, 2017

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix implementation of MLS file relabel attributes

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.

---
 policy/mls                   |  2 +-
 policy/modules/kernel/mls.if | 28 ++++++++++++++++++++++++----
 policy/modules/kernel/mls.te |  3 ++-
 3 files changed, 27 insertions(+), 6 deletions(-)
policy/mls +1 -1
index 2dadd205..73ff301b 100644
@@ -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 }
policy/modules/kernel/mls.if +24 -4
index b09c0a5a..2e2bebc2 100644
@@ -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;
 ')
 
 ########################################
policy/modules/kernel/mls.te +2 -1
index ad74e81f..7c50e75c 100644
@@ -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;
Signature · 1 line
2.14.1
#1 lore raw

PATCH 1/1 Fix implementation of MLS file relabel attributes

Quoted 14 lines · Chad Hanson via refpolicy

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.

---
  policy/mls                   |  2 +-
  policy/modules/kernel/mls.if | 28 ++++++++++++++++++++++++----
  policy/modules/kernel/mls.te |  3 ++-
  3 files changed, 27 insertions(+), 6 deletions(-)
policy/mls +0 -0
index 2dadd205..73ff301b 100644

policy/mls mlsconstrain { file lnk_file fifo_file } { create relabelto } · line 75

Quoted 5 lines
policy/mls +1 -1
@@ -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.

Quoted 46 lines
policy/mls +0 -0
  # 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 }
policy/modules/kernel/mls.if +21 -1
index b09c0a5a..2e2bebc2 100644
@@ -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/>
  #

policy/modules/kernel/mls.if interface(`mls_file_relabel_to_clearance',` · line 132

Quoted 10 lines
policy/modules/kernel/mls.if +3 -3
-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.

Quoted 18 lines
policy/modules/kernel/mls.if +0 -0
  ########################################
policy/modules/kernel/mls.te +2 -1
index ad74e81f..7c50e75c 100644
@@ -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;
Signature · 1 line
Chris PeBenito
#2 lore raw

PATCH 1/1 Fix implementation of MLS file relabel attributes

Quoted 10 lines · Chris PeBenito
Quoted 9 lines · Chad Hanson via refpolicy

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.

---
 policy/mls                   |  2 +-
 policy/modules/kernel/mls.if | 28 ++++++++++++++++++++++++----
 policy/modules/kernel/mls.te |  3 ++-
 3 files changed, 27 insertions(+), 6 deletions(-)
Quoted 15 lines

policy/mls mlsconstrain { file lnk_file fifo_file } { create relabelto } · line 75

Quoted 10 lines
policy/mls +1 -1
index 2dadd205..73ff301b 100644
@@ -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.

Quoted 49 lines
Quoted 49 lines
policy/mls +0 -0
 # 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 }
policy/modules/kernel/mls.if +22 -3
index b09c0a5a..2e2bebc2 100644
@@ -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;
Quoted 9 lines

policy/modules/kernel/mls.if interface(`mls_file_relabel_to_clearance',` · line 132

Quoted 5 lines
policy/modules/kernel/mls.if +2 -1
+		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.

Quoted 21 lines
Quoted 17 lines
policy/modules/kernel/mls.if +0 -0
 ########################################
policy/modules/kernel/mls.te +2 -1
index ad74e81f..7c50e75c 100644
@@ -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;
 
Signature · 1 line
Chris PeBenito
#3 lore raw

PATCH 1/1 Fix implementation of MLS file relabel attributes

Quoted 27 lines · Chad Hanson
Quoted 10 lines · Chris PeBenito
Quoted 9 lines · Chad Hanson via refpolicy

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.

---
  policy/mls                   |  2 +-
  policy/modules/kernel/mls.if | 28 ++++++++++++++++++++++++----
  policy/modules/kernel/mls.te |  3 ++-
  3 files changed, 27 insertions(+), 6 deletions(-)
Quoted 15 lines

policy/mls mlsconstrain { file lnk_file fifo_file } { create relabelto } · line 75

Quoted 10 lines
policy/mls +1 -1
index 2dadd205..73ff301b 100644
@@ -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.

Quoted 8 lines

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.

Signature · 1 line
Chris PeBenito