[PATCH v2 1/1] copy_file(): Revise completion of SELinux security context preserve/set.

[Chris PeBenito]

The existing setfscreatecon() at the beginning of copy_file() is the secure
method for setting the context of new files, but it doesn't apply to
existing files. Change the setfilecon() to only run on preexisting files.

v2:
* Use bb_simple_perror_msg() where needed.
* Change setfilecon() to fsetfilecon() to eliminate race between the
  dst_file open and file relabel, in case an attacker tries to replace
  dst_file so their file can get the setfilecon instead of the intended
  file.
* Initialize con variable.

Signed-off-by: Chris PeBenito <chpebeni at linux.microsoft.com>
---
 libbb/copy_file.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/libbb/copy_file.c b/libbb/copy_file.c
index 49d1ec9c6..4e5db563f 100644
--- a/libbb/copy_file.c
+++ b/libbb/copy_file.c
@@ -325,19 +325,22 @@ int FAST_FUNC copy_file(const char *source, const char *dest, int flags)
 		if ((flags & (FILEUTILS_PRESERVE_SECURITY_CONTEXT|FILEUTILS_SET_SECURITY_CONTEXT))
 		 && is_selinux_enabled() > 0
 		) {
-			security_context_t con;
-			if (getfscreatecon(&con) == -1) {
+			/* Failure to preserve the security context isn't fatal here since
+			 * the copy has been done at this point. */
+			security_context_t con = NULL;
+			if (getfscreatecon(&con) < 0)
 				bb_simple_perror_msg("getfscreatecon");
-				return -1;
-			}
-			if (con) {
-				if (setfilecon(dest, con) == -1) {
-					bb_perror_msg("setfilecon:%s,%s", dest, con);
-					freecon(con);
-					return -1;
-				}
-				freecon(con);
-			}
+
+			if (setfscreatecon(NULL) < 0)
+				bb_simple_perror_msg("can't reset fscreate");
+
+			/* setfscreatecon() only works when a file is created. If dest
+			 * preexisted, use fsetfilecon instead */
+			if (con && dest_exists)
+				if (fsetfilecon(dst_fd, con) < 0)
+					bb_perror_msg("fsetfilecon:%s,%s", dest, con);
+
+			freecon(con);
 		}
 #endif
 #if ENABLE_FEATURE_CP_REFLINK
-- 
2.21.1