Thanks for the comments, I have attached a patch with the malloc replaced with 
xmalloc (and a comment). The reason I didn't use xstrdup instead of xmalloc is 
because the security capability bytes can contain null bytes, for this reason I 
am also unsure of how I would go about completing a sanity check on the length 
(past the length given in the pax header). 

The result of the bloat-o-meter is given below:
function                                              old        new    delta
.rodata                                         159429  159563    +134
get_header_tar                                1772      1903    +131
data_extract_all                               1036      1154    +118
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 3/0 up/down: 383/0)             Total: 383 bytes
   text           data       bss     dec         hex filename
1031002   17003    1880 1049885  10051d busybox_old
1031401   17003    1880 1050284  1006ac busybox_unstripped

This was generated with busybox config created with 'make defconfig', and then 
also enabling the suggested feature 'FEATURE_TAR_SEC_CAPABILITY' in the 'make 
bloatcheck'.

Thanks
________________________________________
From: Bernhard Reutner-Fischer <rep.dot....@gmail.com>
Sent: Wednesday, January 8, 2020 11:34 PM
To: busybox@busybox.net; Cole Dishington; busybox@busybox.net
Subject: Re: [PATCH] - Adds TAR_SEC_CAPABILITY feature to tar

On 7 January 2020 01:25:17 CET, Cole Dishington 
<cole.dishing...@alliedtelesis.co.nz> wrote:
>Package: busybox
>
>Version: v131.1
>Severity: wishlist
>
>
>This patch adds the ability for tar to extract security capabilities,
>stored in pax headers. I have tested this on Linux Ubuntu x86_64 and it
>works. I have tested it against GNU tar and the outputs are the same
>(except for the use of setxattrat vs setxattr in a warning message). I
>have ran the tests in the testsuite directory, and nothing breaks.
>
>
>File capabilities divide the permissions of superuser into distinct
>units, these finer grained permissions aid in securing systems. For
>this reason it would be good to get these added to busybox tar.

Please send patches with signed-off-by line.
Also, please use xmalloc instead of malloc (or xstrdup for that matter).
Furthermore, wouldn't you want to sanity check the length?
Bonus points if you add bloat-o-meter output to your patch submission; make 
baseline; apply the patch and make bloatcheck.

TIA,
Signed-off-by: Cole Dishington <cole.dishing...@alliedtelesis.co.nz>
diff --git a/archival/libarchive/data_extract_all.c b/archival/libarchive/data_extract_all.c
index 4c95db4a6..24d3c91a1 100644
--- a/archival/libarchive/data_extract_all.c
+++ b/archival/libarchive/data_extract_all.c
@@ -2,9 +2,47 @@
 /*
  * Licensed under GPLv2 or later, see file LICENSE in this source tree.
  */
+#if ENABLE_FEATURE_TAR_SEC_CAPABILITY
+#include <sys/types.h>
+#include <sys/xattr.h>
+#endif
 #include "libbb.h"
 #include "bb_archive.h"
 
+#if ENABLE_FEATURE_TAR_SEC_CAPABILITY
+static inline void FAST_FUNC data_apply_capability(file_header_t *file_header, const char *filename, bool is_symlnk)
+{
+#  define SEC_CAPABILITY_FILE_CONTEXT_KEYWORD "security.capability"
+	if (file_header->sec_cap && file_header->sec_cap_len > 0) {
+		const char *sysname = "setxattr";
+		int ret = -1;
+
+		/* lsetxattr works on normal files but GNU tar 1.32 only uses it
+		 * for symlinks.
+		 */
+		if (is_symlnk) {
+			sysname = "lsetxattr";
+			ret = lsetxattr(filename, SEC_CAPABILITY_FILE_CONTEXT_KEYWORD,
+							file_header->sec_cap,
+							file_header->sec_cap_len, 0);
+		} else {
+			ret = setxattr(filename, SEC_CAPABILITY_FILE_CONTEXT_KEYWORD,
+							file_header->sec_cap,
+							file_header->sec_cap_len, 0);
+		}
+
+		if (ret == -1) {
+			bb_error_msg("%s: Cannot set '" SEC_CAPABILITY_FILE_CONTEXT_KEYWORD "' extended attribute for file '%s'", sysname, filename);
+		}
+		free(file_header->sec_cap);
+		file_header->sec_cap = NULL;
+		file_header->sec_cap_len = 0;
+	}
+}
+#else
+static inline void FAST_FUNC data_apply_capability(const file_header_t *file_header, const char *filename, bool is_symlnk) {}
+#endif
+
 void FAST_FUNC data_extract_all(archive_handle_t *archive_handle)
 {
 	file_header_t *file_header = archive_handle->file_header;
@@ -245,6 +283,12 @@ void FAST_FUNC data_extract_all(archive_handle_t *archive_handle)
 		}
 	}
 
+	/**
+	 * the file security capability must be applied after
+	 * chmod (as per GNU tar)
+	 */
+	data_apply_capability(file_header, dst_name, (file_header->mode & S_IFMT) == S_IFLNK);
+
  ret: ;
 #if ENABLE_FEATURE_TAR_SELINUX
 	if (sctx) {
diff --git a/archival/libarchive/get_header_tar.c b/archival/libarchive/get_header_tar.c
index 52fa4554a..47f17ca23 100644
--- a/archival/libarchive/get_header_tar.c
+++ b/archival/libarchive/get_header_tar.c
@@ -60,7 +60,7 @@ static unsigned long long getOctal(char *str, int len)
 }
 #define GET_OCTAL(a) getOctal((a), sizeof(a))
 
-#define TAR_EXTD (ENABLE_FEATURE_TAR_GNU_EXTENSIONS || ENABLE_FEATURE_TAR_SELINUX)
+#define TAR_EXTD (ENABLE_FEATURE_TAR_GNU_EXTENSIONS || ENABLE_FEATURE_TAR_SELINUX || ENABLE_FEATURE_TAR_SEC_CAPABILITY)
 #if !TAR_EXTD
 #define process_pax_hdr(archive_handle, sz, global) \
 	process_pax_hdr(archive_handle, sz)
@@ -129,6 +129,25 @@ static void process_pax_hdr(archive_handle_t *archive_handle, unsigned sz, int g
 		}
 # endif
 
+# if ENABLE_FEATURE_TAR_SEC_CAPABILITY
+		if (!global) {
+		/* Scan for security capabilities, via "SCHILY.xattr.security.capability".
+		 * This is what GNU tar uses.
+		 */
+#  define SEC_CAPABILITY_TAR_CONTEXT_KEYWORD "SCHILY.xattr.security.capability"
+			if (is_prefixed_with(value, SEC_CAPABILITY_TAR_CONTEXT_KEYWORD"=")) {
+				free(archive_handle->tar__sec_cap);
+				value += sizeof(SEC_CAPABILITY_TAR_CONTEXT_KEYWORD"=") - 1;
+				archive_handle->tar__sec_cap_len = (size_t)(p - value) - 1;
+				/* Use xmalloc rather than xstrdup as security capability can
+				 * contain NULL bytes. */
+				archive_handle->tar__sec_cap = xmalloc(archive_handle->tar__sec_cap_len);
+				archive_handle->tar__sec_cap = memcpy(archive_handle->tar__sec_cap, value, archive_handle->tar__sec_cap_len);
+				continue;
+			}
+		}
+#endif
+
 # if ENABLE_FEATURE_TAR_SELINUX
 		/* Scan for SELinux contexts, via "RHT.security.selinux" keyword.
 		 * This is what Red Hat's patched version of tar uses.
@@ -383,6 +402,12 @@ char FAST_FUNC get_header_tar(archive_handle_t *archive_handle)
 		if ((uoff_t)file_header->size > 0xfffff) /* paranoia */
 			goto skip_ext_hdr;
 		process_pax_hdr(archive_handle, file_header->size, (tar_typeflag == 'g'));
+#if ENABLE_FEATURE_TAR_SEC_CAPABILITY
+		file_header->sec_cap = archive_handle->tar__sec_cap;
+		file_header->sec_cap_len = archive_handle->tar__sec_cap_len;
+		archive_handle->tar__sec_cap = NULL;
+		archive_handle->tar__sec_cap_len = 0;
+#endif
 		goto again_after_align;
 #if ENABLE_FEATURE_TAR_GNU_EXTENSIONS
 /* See http://www.gnu.org/software/tar/manual/html_node/Extensions.html */
diff --git a/archival/tar.c b/archival/tar.c
index 3ef89fb0a..c158ec121 100644
--- a/archival/tar.c
+++ b/archival/tar.c
@@ -110,6 +110,14 @@
 //config:	help
 //config:	With this option busybox supports restoring SELinux labels
 //config:	when extracting files from tar archives.
+//config:
+//config:config FEATURE_TAR_SEC_CAPABILITY
+//config:	bool "Support extracting security capability"
+//config:	default n
+//config:	depends on TAR
+//config:	help
+//config:	With this option busybox supports restoring security capability
+//config:	when extracting files from tar archives.
 
 //applet:IF_TAR(APPLET(tar, BB_DIR_BIN, BB_SUID_DROP))
 
diff --git a/include/bb_archive.h b/include/bb_archive.h
index 9b1db5b3e..40d9394fc 100644
--- a/include/bb_archive.h
+++ b/include/bb_archive.h
@@ -41,6 +41,10 @@ typedef struct file_header_t {
 	mode_t mode;
 	time_t mtime;
 	dev_t device;
+#if ENABLE_FEATURE_TAR_SEC_CAPABILITY
+	char *sec_cap;
+	size_t *sec_cap_len;
+#endif
 } file_header_t;
 
 struct hardlinks_t;
@@ -121,6 +125,10 @@ typedef struct archive_handle_t {
 	char *ar__long_names;
 	unsigned ar__long_name_size;
 #endif
+#if ENABLE_FEATURE_TAR_SEC_CAPABILITY
+	char *tar__sec_cap;
+	size_t tar__sec_cap_len;
+#endif
 } archive_handle_t;
 /* bits in ah_flags */
 #define ARCHIVE_RESTORE_DATE        (1 << 0)
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to