Hello,

I have some additional changes on top of my last patch, which added the ability 
to extract security.capability xattr. The new changes include adding the 
--xattr/--no-xattr and --xattr-include/--xattr-exclude commandline arguments 
(seen on GNU tar) to busybox tar. 

I have attached my patch along with a new bloat-o-meter reading.

Thanks
________________________________________
From: busybox <busybox-boun...@busybox.net> on behalf of Cole Dishington 
<cole.dishing...@alliedtelesis.co.nz>
Sent: Thursday, January 9, 2020 9:21 AM
To: busybox@busybox.net
Subject: Re: [PATCH] - Adds TAR_SEC_CAPABILITY feature to tar

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,
function                                             old     new   delta
get_header_tar                                      1772    2034    +262
.rodata                                           159429  159673    +244
data_extract_all                                    1036    1154    +118
tar_longopts                                         314     369     +55
packed_usage                                       33609   33658     +49
tar_main                                            1344    1380     +36
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 6/0 up/down: 764/0)             Total: 764 bytes

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 3142405a3..e383eb396 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_XATTRS_SEC_CAPABILITY
+#include <sys/types.h>
+#include <sys/xattr.h>
+#endif
 #include "libbb.h"
 #include "bb_archive.h"
 
+#if ENABLE_FEATURE_TAR_XATTRS_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 b3131ff2d..a4bf4c37e 100644
--- a/archival/libarchive/get_header_tar.c
+++ b/archival/libarchive/get_header_tar.c
@@ -11,12 +11,52 @@
  *    Opengroup's ustar interchange format,
  *    http://www.opengroup.org/onlinepubs/007904975/utilities/pax.html
  */
+#if ENABLE_FEATURE_TAR_XATTRS_SEC_CAPABILITY
+#include <fnmatch.h>
+#endif
 #include "libbb.h"
 #include "bb_archive.h"
 
 typedef uint32_t aliased_uint32_t FIX_ALIASING;
 typedef off_t    aliased_off_t    FIX_ALIASING;
 
+#if ENABLE_FEATURE_TAR_XATTRS_SEC_CAPABILITY
+static bool FAST_FUNC xattrs_match_masks(const char *kw, const llist_t *masks)
+{
+	const llist_t *iter = masks;
+
+	while (iter) {
+		if (fnmatch (iter->data, kw, 0) == 0) {
+			return true;
+		}
+		iter = (const llist_t *)iter->link;
+	}
+	return false;
+}
+
+static inline bool FAST_FUNC xattrs_kw_included(const char *kw, const llist_t *masks)
+{
+	if (masks) {
+		return xattrs_match_masks (kw, masks);
+	} else {
+# define USER_DOT_PREFIX "user."
+		return strncmp (kw, USER_DOT_PREFIX, sizeof (USER_DOT_PREFIX) - 1) == 0;
+	}
+}
+
+static inline bool FAST_FUNC xattrs_kw_excluded(const char *kw, const llist_t *masks)
+{
+	if (masks) {
+		return xattrs_match_masks (kw, masks);
+	}
+	return false;
+}
+#else
+static bool FAST_FUNC xattrs_match_masks(const char *kw, llist_t *masks) {}
+static inline bool FAST_FUNC xattrs_kw_included(const char *kw, const llist_t *masks) {}
+static inline bool FAST_FUNC xattrs_kw_excluded(const char *kw, const llist_t *masks) {}
+#endif
+
 /* NB: _DESTROYS_ str[len] character! */
 static unsigned long long getOctal(char *str, int len)
 {
@@ -60,7 +100,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_XATTRS_SEC_CAPABILITY)
 #if !TAR_EXTD
 #define process_pax_hdr(archive_handle, sz, global) \
 	process_pax_hdr(archive_handle, sz)
@@ -141,6 +181,35 @@ static void process_pax_hdr(archive_handle_t *archive_handle, unsigned sz, int g
 			continue;
 		}
 # endif
+
+# if ENABLE_FEATURE_TAR_XATTRS_SEC_CAPABILITY
+		if (archive_handle->ah_flags & ARCHIVE_RESTORE_XATTRS) {
+			if (!global) {
+			/* Scan for security capabilities, via "SCHILY.xattr.security.capability".
+			* This is what GNU tar uses.
+			*/
+#  define SCHILY_XATTR_PREFIX "SCHILY.xattr"
+#  define SEC_CAPABILITY_CONTEXT_KEYWORD "security.capability"
+#  define SEC_CAPABILITY_TAR_CONTEXT_KEYWORD SCHILY_XATTR_PREFIX "." SEC_CAPABILITY_CONTEXT_KEYWORD
+				if (is_prefixed_with(value, SEC_CAPABILITY_TAR_CONTEXT_KEYWORD"=")) {
+					/* First evaluate if xattr keyword is included then if it is excluded,
+					 * check the capability base name (excluding SCHILY) like GNU tar.
+					 */
+					if (xattrs_kw_included(SEC_CAPABILITY_CONTEXT_KEYWORD, archive_handle->tar__xattrs_include)
+					 && !xattrs_kw_excluded(SEC_CAPABILITY_CONTEXT_KEYWORD, archive_handle->tar__xattrs_exclude)) {
+						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
 	}
 
 	free(buf);
@@ -383,6 +452,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_XATTRS_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 4ab38db29..bf9b797bc 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_XATTRS_SEC_CAPABILITY
+//config:	bool "Support extracting xattr security capability"
+//config:	default n
+//config:	depends on TAR && FEATURE_TAR_LONG_OPTIONS
+//config:	help
+//config:	With this option busybox supports restoring xattr security
+//config:	capability when extracting files from tar archives.
 
 //applet:IF_TAR(APPLET(tar, BB_DIR_BIN, BB_SUID_DROP))
 
@@ -818,6 +826,12 @@ static llist_t *append_file_list_to_list(llist_t *list)
 //usage:     "\n	--exclude PATTERN	Glob pattern to exclude"
 //usage:	)
 //usage:	)
+//usage:	IF_FEATURE_TAR_XATTRS_SEC_CAPABILITY(
+//usage:	 "\n	--xattrs            	Enable extended attributes support"
+//usage:	 "\n	--no-xattrs         	Disable extended attributes support"
+//usage:	 "\n	--xattrs-exclude MASK	specify the exclude pattern xattr keys"
+//usage:	 "\n	--xattrs-include MASK	specify the include pattern xattr keys"
+//usage:	)
 //usage:
 //usage:#define tar_example_usage
 //usage:       "$ zcat /tmp/tarball.tar.gz | tar -xf -\n"
@@ -853,6 +867,11 @@ enum {
 	OPTBIT_NUMERIC_OWNER,
 	OPTBIT_NOPRESERVE_PERM,
 	OPTBIT_OVERWRITE,
+	IF_FEATURE_TAR_FROM(     OPTBIT_EXCLUDE          ,)
+	IF_FEATURE_TAR_XATTRS_SEC_CAPABILITY(OPTBIT_XATTRS        ,)
+	IF_FEATURE_TAR_XATTRS_SEC_CAPABILITY(OPTBIT_NOXATTRS      ,)
+	IF_FEATURE_TAR_XATTRS_SEC_CAPABILITY(OPTBIT_XATTRS_INCLUDE,)
+	IF_FEATURE_TAR_XATTRS_SEC_CAPABILITY(OPTBIT_XATTRS_EXCLUDE,)
 #endif
 	OPT_TEST         = 1 << 0, // t
 	OPT_EXTRACT      = 1 << 1, // x
@@ -880,6 +899,11 @@ enum {
 	OPT_NUMERIC_OWNER    = IF_FEATURE_TAR_LONG_OPTIONS((1 << OPTBIT_NUMERIC_OWNER  )) + 0, // numeric-owner
 	OPT_NOPRESERVE_PERM  = IF_FEATURE_TAR_LONG_OPTIONS((1 << OPTBIT_NOPRESERVE_PERM)) + 0, // no-same-permissions
 	OPT_OVERWRITE        = IF_FEATURE_TAR_LONG_OPTIONS((1 << OPTBIT_OVERWRITE      )) + 0, // overwrite
+	OPT_EXCLUDE          = IF_FEATURE_TAR_LONG_OPTIONS(IF_FEATURE_TAR_FROM(1 << OPTBIT_EXCLUDE)) + 0, // exclude
+	OPT_XATTRS           = IF_FEATURE_TAR_XATTRS_SEC_CAPABILITY((1 << OPTBIT_XATTRS         )) + 0, // xattrs
+	OPT_NOXATTRS         = IF_FEATURE_TAR_XATTRS_SEC_CAPABILITY((1 << OPTBIT_NOXATTRS       )) + 0, // no-xattrs
+	OPT_XATTRS_INCLUDE   = IF_FEATURE_TAR_XATTRS_SEC_CAPABILITY((1 << OPTBIT_XATTRS_INCLUDE )) + 0, // xattrs-include
+	OPT_XATTRS_EXCLUDE   = IF_FEATURE_TAR_XATTRS_SEC_CAPABILITY((1 << OPTBIT_XATTRS_EXCLUDE )) + 0, // xattrs-exclude
 
 	OPT_ANY_COMPRESS = (OPT_BZIP2 | OPT_LZMA | OPT_GZIP | OPT_XZ | OPT_COMPRESS),
 };
@@ -939,6 +963,12 @@ static const char tar_longopts[] ALIGN1 =
 	/* therefore we have to put it _after_ --no-same-permissions */
 # if ENABLE_FEATURE_TAR_FROM
 	"exclude\0"             Required_argument "\xff"
+# endif
+# if ENABLE_FEATURE_TAR_XATTRS_SEC_CAPABILITY
+	"xattrs\0"              No_argument       "\xe0"
+	"no-xattrs\0"           No_argument       "\xe1"
+	"xattrs-include\0"      Required_argument "\xe2"
+	"xattrs-exclude\0"      Required_argument "\xe3"
 # endif
 	;
 # define GETOPT32 getopt32long
@@ -1024,6 +1054,9 @@ int tar_main(int argc UNUSED_PARAM, char **argv)
 		"tt:vv:" // count -t,-v
 #if ENABLE_FEATURE_TAR_LONG_OPTIONS && ENABLE_FEATURE_TAR_FROM
 		"\xff::" // --exclude=PATTERN is a list
+#endif
+#if ENABLE_FEATURE_TAR_XATTRS_SEC_CAPABILITY
+		"\xe2::\xe3::" // --xattrs-include and xattrs-exclude are lists
 #endif
 		IF_FEATURE_TAR_CREATE("c:") "t:x:" // at least one of these is reqd
 		IF_FEATURE_TAR_CREATE("c--tx:t--cx:x--ct") // mutually exclusive
@@ -1042,6 +1075,10 @@ int tar_main(int argc UNUSED_PARAM, char **argv)
 		IF_FEATURE_TAR_TO_COMMAND(, &(tar_handle->tar__to_command)) // --to-command
 #if ENABLE_FEATURE_TAR_LONG_OPTIONS && ENABLE_FEATURE_TAR_FROM
 		, &excludes // --exclude
+#endif
+#if ENABLE_FEATURE_TAR_XATTRS_SEC_CAPABILITY
+		, &tar_handle->tar__xattrs_include
+		, &tar_handle->tar__xattrs_exclude
 #endif
 		, &verboseFlag // combined count for -t and -v
 		, &verboseFlag // combined count for -t and -v
@@ -1138,6 +1175,14 @@ int tar_main(int argc UNUSED_PARAM, char **argv)
 	tar_handle->accept = append_file_list_to_list(tar_handle->accept);
 #endif
 
+#if ENABLE_FEATURE_TAR_XATTRS_SEC_CAPABILITY
+	if (opt & OPT_XATTRS) {
+		tar_handle->ah_flags |= ARCHIVE_RESTORE_XATTRS;
+	} else {
+		tar_handle->ah_flags &= ~ARCHIVE_RESTORE_XATTRS;
+	}
+#endif
+
 	/* Setup an array of filenames to work with */
 	/* TODO: This is the same as in ar, make a separate function? */
 	while (*argv) {
diff --git a/include/bb_archive.h b/include/bb_archive.h
index 9b1db5b3e..f239c843c 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_XATTRS_SEC_CAPABILITY
+	char *sec_cap;
+	size_t *sec_cap_len;
+#endif
 } file_header_t;
 
 struct hardlinks_t;
@@ -98,6 +102,12 @@ typedef struct archive_handle_t {
 # if ENABLE_FEATURE_TAR_SELINUX
 	char* tar__sctx[2];
 # endif
+# if ENABLE_FEATURE_TAR_XATTRS_SEC_CAPABILITY
+	llist_t *tar__xattrs_include;
+	llist_t *tar__xattrs_exclude;
+	char *tar__sec_cap;
+	size_t tar__sec_cap_len;
+# endif
 #endif
 #if ENABLE_CPIO || ENABLE_RPM2CPIO || ENABLE_RPM
 	uoff_t cpio__blocks;
@@ -135,6 +145,9 @@ typedef struct archive_handle_t {
 #if ENABLE_RPM
 #define ARCHIVE_REPLACE_VIA_RENAME  (1 << 9)
 #endif
+#if ENABLE_FEATURE_TAR_XATTRS_SEC_CAPABILITY
+#define ARCHIVE_RESTORE_XATTRS      (1 << 10)
+#endif
 
 
 /* POSIX tar Header Block, from POSIX 1003.1-1990  */
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to