[capsicum] xz sandboxing



Hi folks,

I played with capsicum and decided to take a look at xz from 10.1-RELEASE.

The code is pretty much laid out, and quite modular. I decided to attempt
a non-intrusive way to sandbox it.

Here's the diff:

--- file_io.c.orig	2015-01-28 21:18:41.000000000 +0400
+++ file_io.c	2015-01-28 21:51:32.000000000 +0400
@@ -13,6 +13,7 @@
 #include "private.h"
 
 #include <fcntl.h>
+#include <sys/capability.h>
 
 #ifdef TUKLIB_DOSLIKE
 #	include <io.h>
@@ -289,6 +290,8 @@ io_copy_attrs(const file_pair *pair)
 static bool
 io_open_src_real(file_pair *pair)
 {
+	cap_rights_t rights;
+
 	// There's nothing to open when reading from stdin.
 	if (pair->src_name == stdin_filename) {
 		pair->src_fd = STDIN_FILENO;
@@ -358,6 +361,11 @@ io_open_src_real(file_pair *pair)
 		pair->src_fd = open(pair->src_name, flags);
 	} while (pair->src_fd == -1 && errno == EINTR && !user_abort);
 
+	cap_rights_init(&rights, CAP_READ, CAP_LOOKUP, CAP_FSTAT, CAP_SEEK,
+			CAP_FCNTL);
+	if (cap_rights_limit(pair->src_fd, &rights) < 0 && errno != ENOSYS)
+		message_warning(_("unable to limit src_fd"));
+	
 	if (!reg_files_only)
 		signals_block();
 
@@ -569,6 +577,8 @@ io_close_src(file_pair *pair, bool succe
 static bool
 io_open_dest_real(file_pair *pair)
 {
+	cap_rights_t rights;
+
 	if (opt_stdout || pair->src_fd == STDIN_FILENO) {
 		// We don't modify or free() this.
 		pair->dest_name = (char *)"(stdout)";
@@ -601,6 +611,12 @@ io_open_dest_real(file_pair *pair)
 			free(pair->dest_name);
 			return true;
 		}
+		cap_rights_init(&rights, CAP_READ, CAP_LOOKUP, CAP_FSTAT,
+				CAP_SEEK, CAP_WRITE, CAP_FCHOWN, CAP_FCHMOD, 
+				CAP_FUTIMES, CAP_FCNTL);
+		if (cap_rights_limit(pair->dest_fd, &rights) < 0 && errno !=
+		    ENOSYS)
+		    message_warning(("unable to limit dest fd"));
 	}
 
 	// If this really fails... well, we have a safe fallback.



--- coder.c.orig	2015-01-28 21:18:28.000000000 +0400
+++ coder.c	2015-01-28 21:36:50.000000000 +0400
@@ -11,7 +11,7 @@
 ///////////////////////////////////////////////////////////////////////////////
 
 #include "private.h"
-
+#include <sys/capability.h>
 
 /// Return value type for coder_init().
 enum coder_init_ret {
@@ -648,6 +648,8 @@ coder_run(const char *filename)
 			// Don't open the destination file when --test
 			// is used.
 			if (opt_mode == MODE_TEST || !io_open_dest(pair)) {
+				if (cap_enter() < 0)
+					message_error("cap_enter() failed");
 				// Initialize the progress indicator.
 				const uint64_t in_size
 						= pair->src_st.st_size <= 0

---------

It's possible to go further, and initiase the compression/decomp engine after
opening the destination file descriptor and setting the fd limits. However,
this goes against the wishes of the original author.

(He may consider re-evaluating his opinion, after reading about capsicum).

Second point: there's a race that occurs between unlink and close. Going
through the ktrace log shows that it closes the file descriptor before
unlinking.

I think that with capsicum enabled in xz, this code should be removed as it
generates a warning each time.

Sample run:

[logan at logan ~]$ ktrace xz 1M 
xz: 1M: File seems to have been moved, not removing
[logan at logan ~]$ file 1M
1M: data
[logan at logan ~]$ du -h 1M
1.0M	1M
[logan at logan ~]$ du -h 1M.xz
4.0K	1M.xz


The advantage of capsicum here, is that it sandboxes the decompression/
compression code which involves dangerous buffer, and mixed data type
operations that may have vulnerabilities.

As usual, feedback welcomed !

Kind regards,
//Logan
C-x-C-c




This archive was generated by a fusion of Pipermail (Mailman edition) and MHonArc.