Hello,

I'd like to remember the following bug report. I've ported the patches for it to GNU tar 1.29.


Also see https://bugzilla.redhat.com/show_bug.cgi?id=1052876

Storing and restoring ACLs in tar archives should be improved:


(1) tar archive creation with "--numeric-owner" option:

In this case, users are expectiing that the archive does not contain any symbolic owner name, so it can be extracted to an emoty hard disc on a system bootet e. g. by a rescue cd from Redhat. Current sitation is that ACLs still include the symbolic owner and not the numeric owner.


This is quite trivial to fix (not completely contained in the attached patches):

Replace all ocurences of "val = acl_to_text(acl, &len);" by "val = acl_to_any_text(acl, 0, ',', ( numeric_owner_option?TEXT_NUMERIC_IDS:0));" and followed by "len=strlen(val);" after the "if (!val)" error-handling.



Effect: Numeric owner is stored.

I'd like to note that this improvement is essential to me.



(2a) tar archive creation without "--numeric-owner" option:

In GNU tar 1.26, for every file the owner is stored both, symbolic and numeric. I would expect that ACLs are stored in both ways, too. star shows us how to do that:


star stores the numeric owner in a forth field of an acl: (e.g. "u:msteinbo:rwx:500").




(2b) tar extract should use the 4th field (discussed in point 2) in presence of "-numeric-owner".


This together with point (2) enabled users to restore an backup created without numeric owner option on a clean hard disc without passwd entries for the users (let's assume that /etc/passwd is contained in the archive so the operation makes sense).

I'd like to mention that this point would increase star compatibility a lot.



(see also http://lists.gnu.org/archive/html/bug-tar/2013-03/msg00021.html
upstream maintainer "Paul Eggert" says in
http://lists.gnu.org/archive/html/bug-tar/2013-04/msg00024.html:
"That sort of thing all sounds reasonable, I guess. I'd like Sergey's opinion though.".



Greetings from Germany

Markus Steinborn
GNU gv maintainer

>From f4118bdc56da613046caa6e19981ca4f9cd9cf43 Mon Sep 17 00:00:00 2001
From: Markus Steinborn <[email protected]>
Date: Tue, 17 Dec 2013 21:26:21 +0100
Subject: [PATCH 1/3] tar extract should use the 4th field (discussed in point
 2) in presence of "-numeric-owner".

This together with point (2) enabled users to restore an backup created
without numeric owner option on a clean hard disc without passwd entries for
the users (let's assume that /etc/passwd is contained in the archive so the
operation makes sense).
---
 src/xattrs.c | 253 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 252 insertions(+), 1 deletion(-)

diff --git a/src/xattrs.c b/src/xattrs.c
index 8e56168..634eeaf 100644
--- a/src/xattrs.c
+++ b/src/xattrs.c
@@ -191,7 +191,7 @@ fixup_extra_acl_fields (char *ptr)
         src += strcspn (src, "\n,");
 
       if ((*src == '\n') || (*src == ','))
-        *dst++ = *src++;        /* also done when dst == src, but that's ok */
+        *(dst++) = *src++;        /* also done when dst == src, but that's ok */
     }
   if (src != dst)
     *dst = 0;
@@ -199,6 +199,253 @@ fixup_extra_acl_fields (char *ptr)
   return ptr;
 }
 
+void acl_check_ids(char* acltext, char* infotext)
+{
+  int state = 1;
+  char *src = acltext;
+  char *dst = infotext;
+  char username[100], acc[4], userid[20];
+  char *tmp = 0;
+
+
+  while (1)
+  {
+     if (state == 1)
+     {
+        switch (*src)
+	{
+	   case ' ':
+	   case 13:
+	   case 10:
+	      break;
+	      
+	   case '#':
+	      state = 5;
+	      break;
+	   
+	   case 'u':
+	      state = 2;
+	      *(dst++) = 'u';
+	      break;
+	   case 'g':
+	      state = 3;
+	      *(dst++) = 'g';
+	      break;
+           case 'o':
+	      state = 4;
+	      *(dst++) = 'o';
+	      break;
+           case 'm':
+	      state = 22;
+	      *(dst++) = 'm';
+	      break;
+	   default:
+	      state = 0;
+	}
+     }
+     else if (state == 5)
+     {
+        if (*src == 13) state = 1;
+	else if (*src == 0) { state = 20; *tmp = 0; continue; }
+     }
+     
+     else if (state == 2)
+     {
+        if (*src == ':') { state = 17; *(dst++) = ':'; *username=0; *acc=0; *userid=0; tmp=username; }
+	else if (*src == 's') state = 6;
+	else state = 0;
+     }
+     else if (state == 6)
+     {
+	if (*src == 'e') state = 7;
+	else state = 0;
+     }
+     else if (state == 7)
+     {
+	if (*src == 'r') state = 8;
+	else state = 0;
+     }
+     else if (state == 8)
+     {
+	if (*src == ':') { state = 17; *(dst++) = ':'; *username=0; *acc=0; *userid=0; tmp=username; }
+	else state = 0;
+     }
+
+     else if (state == 3)
+     {
+        if (*src == ':') { state = 17; *(dst++) = ':'; *username=0; *acc=0; *userid=0; tmp=username; }
+	else if (*src == 'r') state = 9;
+	else state = 0;
+     }
+     else if (state == 9)
+     {
+	if (*src == 'o') state = 10;
+	else state = 0;
+     }
+     else if (state == 10)
+     {
+	if (*src == 'u') state = 11;
+	else state = 0;
+     }
+     else if (state == 11)
+     {
+	if (*src == 'p') state = 12;
+	else state = 0;
+     }
+     else if (state == 12)
+     {
+	if (*src == ':') { state = 17; *(dst++) = ':'; *username=0; *acc=0; *userid=0; tmp=username; }
+	else state = 0;
+     }
+
+     else if (state == 4)
+     {
+        if (*src == ':') { state = 17; *(dst++) = ':'; *username=0; *acc=0; *userid=0; tmp=username; }
+	else if (*src == 't') state = 13;
+	else state = 0;
+     }
+     else if (state == 13)
+     {
+	if (*src == 'h') state = 14;
+	else state = 0;
+     }
+     else if (state == 14)
+     {
+	if (*src == 'e') state = 15;
+	else state = 0;
+     }
+     else if (state == 15)
+     {
+	if (*src == 'r') state = 16;
+	else state = 0;
+     }
+     else if (state == 16)
+     {
+	if (*src == ':') { state = 17; *(dst++) = ':'; *username=0; *acc=0; *userid=0; tmp=username; }
+	else state = 0;
+     }
+     
+     else if (state == 22)
+     {
+        if (*src == ':') { state = 17; *(dst++) = ':'; *username=0; *acc=0; *userid=0; tmp=username; }
+	else if (*src == 'a') state = 23;
+	else state = 0;
+     }
+     else if (state == 23)
+     {
+	if (*src == 's') state = 24;
+	else state = 0;
+     }
+     else if (state == 24)
+     {
+	if (*src == 'k') state = 25;
+	else state = 0;
+     }
+     else if (state == 25)
+     {
+	if (*src == ':') { state = 17; *(dst++) = ':'; *username=0; *acc=0; *userid=0; tmp=username; }
+	else state = 0;
+     }
+
+     /* 2nd field */
+     else if (state == 17)
+     {
+        if (*src == ':') { state = 18; *tmp = 0; tmp = acc; }
+	else if (*src == 0) { state = 0; *tmp = 0; }
+	else *(tmp++) = *src;
+     }
+     
+     /* 3rd field */
+     else if (state == 18)
+     {
+        if (*src == ' ') { state = 20; *tmp = 0; }
+        else if (*src == '\n') { state = 20; *tmp = 0; }
+	else if (*src == ',') { state = 20; *tmp = 0; }
+        else if (*src == ':') { state = 19; *tmp = 0; tmp = userid; }
+	else if (*src == 0) { state = 20; *tmp = 0; continue; }
+	else *(tmp++) = *src;
+     }
+     
+     /* 4th field */
+     else if (state == 19)
+     {
+        if (*src == ' ') { state = 20; *tmp = 0; }
+        else if (*src == '\n') { state = 20; *tmp = 0; }
+	else if (*src == ',') { state = 20; *tmp = 0; }
+        else if (*src == ':') { state = 20; *tmp = 0; }
+	else if (*src == 0) { state = 20; *tmp = 0; continue; }
+	else *(tmp++) = *src;
+     }
+     
+     /* End of "line" */
+     else if (state == 20)
+     {
+        state = 21;
+	if ( numeric_owner_option && *userid )
+	   tmp = userid;
+	else
+	   tmp = username;
+	while (*tmp)  *(dst++) = *(tmp++);
+	*(dst++) = ':';
+	tmp = acc;
+	while (*tmp)  *(dst++) = *(tmp++);
+	continue;
+     }
+     
+     else if (state == 21)
+     {
+        switch (*src)
+	{
+	   case ' ':
+	   case 13:
+	   case 10:
+	      break;
+	      
+	   case '#':
+	      state = 5;
+	      break;
+	   
+	   case 'u':
+	      state = 2;
+	      *(dst++) = ',';
+	      *(dst++) = 'u';
+	      break;
+	   case 'g':
+	      state = 3;
+	      *(dst++) = ',';
+	      *(dst++) = 'g';
+	      break;
+           case 'o':
+	      state = 4;
+	      *(dst++) = ',';
+	      *(dst++) = 'o';
+	      break;
+           case 'm':
+	      state = 22;
+	      *(dst++) = ',';
+	      *(dst++) = 'm';
+	      break;
+	      
+	   case 0:
+	      break;
+	      
+	   default:
+	      state = 0;
+	}
+     }
+
+     if (!state || !*src)
+        break;
+     else
+        ++src;
+  }
+  
+  *dst = 0;
+  if (!state)
+     *infotext = 0;
+}
+
+/* "system.posix_acl_access" */
 /* Set the "system.posix_acl_access/system.posix_acl_default" extended
    attribute.  Called only when acls_option > 0. */
 static void
@@ -213,6 +460,10 @@ xattrs__acls_set (struct tar_stat_info const *st,
       /* assert (strlen (ptr) == len); */
       ptr = fixup_extra_acl_fields (ptr);
       acl = acl_from_text (ptr);
+      char* tmp = malloc(len+1);
+      acl_check_ids(ptr,tmp);
+      acl = acl_from_text (tmp);
+      free(tmp);
     }
   else if (def)
     {
-- 
2.7.4

>From ef57ab0bc81f1f0c1feaf5395a6d76e9ea3cfd2a Mon Sep 17 00:00:00 2001
From: Markus Steinborn <[email protected]>
Date: Sun, 6 Oct 2013 11:50:07 +0200
Subject: [PATCH 2/3] tar archive creation without "--numeric-owner" option

In GNU tar 1.26, for every file the owner is stored both, symbolic and
numeric. I would expect that ACLs are stored in both ways, too. star shows
us how to do that:

star stores the numeric owner in a forth field of an acl:
(e.g. "u:msteinbo:rwx:500").

ACLs: tar archive creation with "--numeric-owner" option

In this case, users are expectiing that the archive does not contain any
symbolic owner name, so it can be extracted to an emoty hard disc on a
---
 src/xattrs.c | 202 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 200 insertions(+), 2 deletions(-)

diff --git a/src/xattrs.c b/src/xattrs.c
index 634eeaf..9c2901a 100644
--- a/src/xattrs.c
+++ b/src/xattrs.c
@@ -53,6 +53,7 @@ static struct
 #ifdef HAVE_POSIX_ACLS
 # include "acl.h"
 # include <sys/acl.h>
+# include <acl/libacl.h>
 #endif
 
 #ifdef HAVE_POSIX_ACLS
@@ -497,12 +498,179 @@ xattrs__acls_set (struct tar_stat_info const *st,
   acl_free (acl);
 }
 
+bool name2uid(char *name, unsigned long *uidp)
+{
+	struct passwd	*pw;
+
+	if (!*name)
+		return false;
+
+	if ((pw = getpwnam(name)) != NULL) {
+		*uidp = pw->pw_uid;
+		return true;
+	} else {
+		*uidp = 0;
+		return false;
+	}
+}
+
+bool name2gid(char* name, unsigned long *gidp)
+{
+	struct group	*gr;
+
+	if (!*name)
+		return false;
+
+	if ((gr = getgrnam(name)) != NULL) {
+		*gidp = gr->gr_gid;
+		return true;
+	} else {
+		*gidp = 0;
+		return false;
+	}
+}
+
+static bool acl_add_ids(char* acltext, char** retInfotext)
+{
+   int ret, dstlen;
+   char *tmp, *tmp2, *src, *dst; 
+   tmp = malloc( 1+strlen(acltext) );
+   acl_check_ids( acltext, tmp );
+   src=tmp;
+   
+   dstlen = 1024;
+   dst=tmp2=malloc(dstlen);
+   
+   while (*src)
+   {
+      char *end, *end1, *f1, *f2, *f3, *tmpx;
+      int isNum, len;
+      end = src;
+      while (*end && *end != ',')
+         end++;
+      end1 = end;
+      if (*end)
+         *(end++) = 0;
+      f1 = src;
+      f2 = f1;
+      while (*f2 != ':')
+         f2++;
+      *f2 = 0;
+      f3 = ++f2;
+      while (*f3 != ':')
+         f3++;
+      *f3 = 0;
+      ++f3;
+      
+      isNum = 1;
+      tmpx = f2;
+      while (*tmpx)
+      {
+         if (*tmpx < '0' || *tmpx >'9')
+	    isNum = 0;
+	 tmpx++;
+      }
+
+      if (isNum)
+      {
+	    int size, si;
+	    size = snprintf(dst, 0, "%s:%s:%s", f1, f2, f3);
+            si = (dst-tmp2) + size + 1;
+	    
+	    if ( si >= dstlen )
+	    {
+	       int siz = dst-tmp2;
+	       while (si >= dstlen)
+	          dstlen *= 2;
+	       
+	       tmp2 = realloc(tmp2, dstlen);
+	       dst = tmp2+siz;
+	    }
+
+         sprintf(dst, "%s:%s:%s", f1, f2, f3);
+      }
+      else
+      {
+         unsigned long f4 = 0;
+	 int ok = 0;
+	 
+	 if (*f1 == 'u')
+	 {
+	    ok = name2uid(f2, &f4);
+	 }
+	 if (*f1 == 'g')
+	 {
+	    ok = name2gid(f2, &f4);
+	 }
+
+         if (ok)
+	 {
+	    int size, si;
+	    
+	    if (numeric_owner_option)
+	       size = snprintf(dst, 0, "%s:%li:%s", f1, f4, f3);
+	    else
+	       size = snprintf(dst, 0, "%s:%s:%s:%li", f1, f2, f3, f4);
+            si = (dst-tmp2) + size + 1;
+	    
+	    if ( si >= dstlen )
+	    {
+	       int siz = dst-tmp2;
+	       while (si >= dstlen)
+	          dstlen *= 2;
+	       
+	       tmp2 = realloc(tmp2, dstlen);
+	       dst = tmp2+siz;
+	    }
+	    
+	    if (numeric_owner_option)
+	       sprintf(dst, "%s:%li:%s", f1, f4, f3);
+            else
+	       sprintf(dst, "%s:%s:%s:%li", f1, f2, f3, f4);
+	 }
+	 else
+	 {
+	    int size, si;
+	    size = snprintf(dst, 0, "%s:%s:%s", f1, f2, f3);
+            si = (dst-tmp2) + size + 1;
+	    
+	    if ( si >= dstlen )
+	    {
+	       int siz = dst-tmp2;
+	       while (si >= dstlen)
+	          dstlen *= 2;
+	       
+	       tmp2 = realloc(tmp2, dstlen);
+	       dst = tmp2+siz;
+	    }
+
+	    sprintf(dst, "%s:%s:%s", f1, f2, f3);
+	 }
+      }
+      dst +=strlen(dst);
+      if (end != end1)
+         *(dst++) = ',';
+      
+      src = end;
+   }
+   *dst = 0;
+   
+   free(tmp);
+   *retInfotext = strdup(tmp2);
+   free(tmp2);
+   return true;
+}
+
 static void
 xattrs__acls_get_a (int parentfd, const char *file_name,
                     struct tar_stat_info *st,
                     char **ret_ptr, size_t * ret_len)
 {
   char *val = NULL;
+   char* text;
+   char* c;
+   void* toFree = 0;
+
   ssize_t len;
   acl_t acl;
 
@@ -513,7 +681,18 @@ xattrs__acls_get_a (int parentfd, const char *file_name,
       return;
     }
 
-  val = acl_to_text (acl, &len);
+   text = acl_to_text(acl, &len);
+   if (text)
+   {
+      if (!acl_add_ids(text, &val)) {
+		      acl_free((acl_t)text);
+                      *ret_ptr = 0;
+                      *ret_len = 0;
+                      return;
+      }
+      acl_free(text);
+      len = strlen(val);
+   }
   acl_free (acl);
 
   if (!val)
@@ -522,6 +701,8 @@ xattrs__acls_get_a (int parentfd, const char *file_name,
       return;
     }
 
+  len = strlen(val);
+
   *ret_ptr = xstrdup (val);
   *ret_len = len;
 
@@ -535,6 +716,10 @@ xattrs__acls_get_d (int parentfd, char const *file_name,
                     char **ret_ptr, size_t * ret_len)
 {
   char *val = NULL;
+   char* text;
+   char* c;
+   void* toFree = 0;
+
   ssize_t len;
   acl_t acl;
 
@@ -545,7 +730,18 @@ xattrs__acls_get_d (int parentfd, char const *file_name,
       return;
     }
 
-  val = acl_to_text (acl, &len);
+   text = acl_to_text(acl, &len);
+   if (text)
+   {
+      if (!acl_add_ids(text, &val)) {
+		      acl_free((acl_t)text);
+                      *ret_ptr = 0;
+                      *ret_len = 0;
+                      return;
+      }
+      acl_free(text);
+      len = strlen(val);
+   }
   acl_free (acl);
 
   if (!val)
@@ -554,6 +750,8 @@ xattrs__acls_get_d (int parentfd, char const *file_name,
       return;
     }
 
+   len = strlen(val);
+
   *ret_ptr = xstrdup (val);
   *ret_len = len;
 
-- 
2.7.4

>From 94dd14e7d2b08d1cdaf1f65e8b63c8a6f7669209 Mon Sep 17 00:00:00 2001
From: Markus Steinborn <[email protected]>
Date: Tue, 17 Dec 2013 15:26:17 +0100
Subject: [PATCH 3/3] Do not store filesystem dependent xattrs (i.e. ACLs etc)

---
 src/tar.c     |  4 +++-
 src/xattrs.c  |  9 +++++++++
 src/xheader.c | 11 ++++++++++-
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/tar.c b/src/tar.c
index ba24c43..c667ccd 100644
--- a/src/tar.c
+++ b/src/tar.c
@@ -1985,7 +1985,9 @@ parse_opt (int key, char *arg, struct argp_state *state)
       break;
 
     case XATTR_OPTION:
-      set_xattr_option (1);
+      set_archive_format ("posix");
+      xattrs_option = xattrs_option >= 0 && xattrs_option < 3 ? xattrs_option+1 : 
+                      xattrs_option < 0 ? 1 : xattrs_option;
       break;
 
     case NO_XATTR_OPTION:
diff --git a/src/xattrs.c b/src/xattrs.c
index 9c2901a..5cd1e3a 100644
--- a/src/xattrs.c
+++ b/src/xattrs.c
@@ -929,6 +929,14 @@ xattrs_xattrs_get (int parentfd, char const *file_name,
               size_t len = strlen (attr);
               ssize_t aret = 0;
 
+              if (strncmp (attr, "user.", strlen("user.")) &&
+                  strncmp (attr, "trusted.", strlen("trusted.")) &&
+                  strncmp (attr, "lustre.", strlen("lustre.")) &&
+		  (xattrs_option != 2 || strncmp (attr, "security.", strlen("security."))) &&
+		  xattrs_option != 3
+		 )
+                goto next_attr; /* only store normal xattrs */
+
               /* Archive all xattrs during creation, decide at extraction time
                * which ones are of interest/use for the target filesystem. */
               while (((fd == 0)
@@ -946,6 +954,7 @@ xattrs_xattrs_get (int parentfd, char const *file_name,
                 call_arg_warn ((fd == 0) ? "lgetxattrat"
                                : "fgetxattr", file_name);
 
+next_attr:
               attr += len + 1;
               xret -= len + 1;
             }
diff --git a/src/xheader.c b/src/xheader.c
index 8dda580..a5d026a 100644
--- a/src/xheader.c
+++ b/src/xheader.c
@@ -1792,7 +1792,16 @@ struct xhdr_tab const xhdr_tab[] = {
   /* We are storing all extended attributes using this rule even if some of them
      were stored by some previous rule (duplicates) -- we just have to make sure
      they are restored *only once* during extraction later on. */
-  { "SCHILY.xattr", xattr_coder, xattr_decoder, 0, true },
+
+  /* xattrs use the star format.  note we only save some variants... */
+  { "SCHILY.xattr.user",    xattr_coder, xattr_decoder, 0, true },
+  { "SCHILY.xattr.security",    xattr_coder, xattr_decoder, 0, true },
+  { "SCHILY.xattr.trusted", xattr_coder, xattr_decoder, 0, true },
+  { "SCHILY.xattr.lustre",  xattr_coder, xattr_decoder, 0, true },
+//  { "SCHILY.xattr.security.NTACL", xattr_coder, xattr_decoder, 0, true },
+
+  /* ignore everything else in the xattr namespaces... */
+  { "SCHILY.xattr",         dummy_coder, dummy_decoder, 0, true },
 
   { NULL, NULL, NULL, 0, false }
 };
-- 
2.7.4

Reply via email to