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
