pkg_add broken by POLA breakage in tar
Revs.1.2-1.3 of tar/src/extract.c break pkg_add (not to mention probably
thousands of user scripts that are no more careful than pkg_add) in
-current and RELENG_4:
% RCS file: /home/ncvs/src/contrib/tar/src/extract.c,v
% Working file: extract.c
% head: 1.4
% branch:
% locks: strict
% access list:
% symbolic names:
% RELENG_4: 1.4.0.2
% TAR_v1_13_25: 1.1.1.1
% FSF: 1.1.1
% keyword substitution: kv
% total revisions: 6; selected revisions: 6
% description:
% ...
%
% revision 1.3
% date: 2002/06/07 06:02:35; author: sobomax; state: Exp; lines: +1 -1
% Disabling automatic --same-owner option when running as uid 0 along with
% the --same-permissions was an overkill, so put it back. This is consistent
% with what our old tar did.
%
% Suggested by: dillon
%
% revision 1.2
% date: 2002/06/07 00:03:23; author: sobomax; state: Exp; lines: +4 -0
% IMO it was a quite ugly idea that if we are running as uid 0 then we can
% safely ignore current umask(2) and assume that permissions should be set
% right like in the archive. Not only it violates POLA, but introduces
^
% huge potential security vulnerability, particularly for ports, where
% many popular archives come with 777 files and dirs.
%
Actually, it is the change violates POLA, and breaks everything that
depends on the historical and still documented behaviour. (The man
page even says that (almost) all permissions are restored even in the
!root case (it says this indirectly by saying that all attributes are
restored if possible and not mentioning the umask or root). The info
page is better.)
This bug showed up as breakage in mutt. mutt uses a setgid utility
named mutt_dotlock to lock /var/mail/*, so it fails to download mail
if mutt_dotlock's setgid bit is lost on extraction. It is probably
another bug that mutt_dotlock attempts to create a temporary file in
/var/mail instead of using flock().
Fixes:
(1) Change pkg_add and thousands of user scripts to use tar -p. This
may reopen security holes closed by respecting the umask.
%%%
Index: extract.c
===
RCS file: /home/ncvs/src/usr.sbin/pkg_install/add/extract.c,v
retrieving revision 1.33
diff -u -2 -r1.33 extract.c
--- extract.c 11 May 2002 04:17:54 - 1.33
+++ extract.c 1 Aug 2002 10:26:10 -
@@ -33,5 +33,5 @@
#define PUSHOUT(todir) /* push out string */ \
if (where_count (int)sizeof(STARTSTRING)-1) { \
- strcat(where_args, |tar --unlink -xf - -C ); \
+ strcat(where_args, |tar --unlink -pxf - -C ); \
strcat(where_args, todir); \
if (system(where_args)) { \
%%%
(2) Restore standard gnu tar behaviour by backing out extract.c revs 1.2-1.3.
%%%
Index: extract.c
===
RCS file: /home/ncvs/src/contrib/tar/src/extract.c,v
retrieving revision 1.4
diff -u -2 -r1.4 extract.c
--- extract.c 3 Jul 2002 12:44:31 - 1.4
+++ extract.c 1 Aug 2002 10:44:34 -
@@ -113,7 +113,5 @@
{
we_are_root = geteuid () == 0;
-#ifndef __FreeBSD__
same_permissions_option += we_are_root;
-#endif
same_owner_option += we_are_root;
xalloc_fail_func = extract_finish;
%%%
Bruce
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: pkg_add broken by POLA breakage in tar
Bruce Evans wrote:
Revs.1.2-1.3 of tar/src/extract.c break pkg_add (not to mention probably
thousands of user scripts that are no more careful than pkg_add) in
-current and RELENG_4:
Are you sure? My own investigation at the time of the commit showed
that old tar shipped with FreeBSD, was adjusting permissions of
extracting files when running as uid 0 according to current umask
settings, so that IMO 1.2-1.3 actually restored POLA, not broke it.
-Maxim
% RCS file: /home/ncvs/src/contrib/tar/src/extract.c,v
% Working file: extract.c
% head: 1.4
% branch:
% locks: strict
% access list:
% symbolic names:
% RELENG_4: 1.4.0.2
% TAR_v1_13_25: 1.1.1.1
% FSF: 1.1.1
% keyword substitution: kv
% total revisions: 6; selected revisions: 6
% description:
% ...
%
% revision 1.3
% date: 2002/06/07 06:02:35; author: sobomax; state: Exp; lines: +1 -1
% Disabling automatic --same-owner option when running as uid 0 along with
% the --same-permissions was an overkill, so put it back. This is consistent
% with what our old tar did.
%
% Suggested by: dillon
%
% revision 1.2
% date: 2002/06/07 00:03:23; author: sobomax; state: Exp; lines: +4 -0
% IMO it was a quite ugly idea that if we are running as uid 0 then we can
% safely ignore current umask(2) and assume that permissions should be set
% right like in the archive. Not only it violates POLA, but introduces
^
% huge potential security vulnerability, particularly for ports, where
% many popular archives come with 777 files and dirs.
%
Actually, it is the change violates POLA, and breaks everything that
depends on the historical and still documented behaviour. (The man
page even says that (almost) all permissions are restored even in the
!root case (it says this indirectly by saying that all attributes are
restored if possible and not mentioning the umask or root). The info
page is better.)
This bug showed up as breakage in mutt. mutt uses a setgid utility
named mutt_dotlock to lock /var/mail/*, so it fails to download mail
if mutt_dotlock's setgid bit is lost on extraction. It is probably
another bug that mutt_dotlock attempts to create a temporary file in
/var/mail instead of using flock().
Fixes:
(1) Change pkg_add and thousands of user scripts to use tar -p. This
may reopen security holes closed by respecting the umask.
%%%
Index: extract.c
===
RCS file: /home/ncvs/src/usr.sbin/pkg_install/add/extract.c,v
retrieving revision 1.33
diff -u -2 -r1.33 extract.c
--- extract.c 11 May 2002 04:17:54 - 1.33
+++ extract.c 1 Aug 2002 10:26:10 -
@@ -33,5 +33,5 @@
#define PUSHOUT(todir) /* push out string */ \
if (where_count (int)sizeof(STARTSTRING)-1) { \
- strcat(where_args, |tar --unlink -xf - -C ); \
+ strcat(where_args, |tar --unlink -pxf - -C ); \
strcat(where_args, todir); \
if (system(where_args)) { \
%%%
(2) Restore standard gnu tar behaviour by backing out extract.c revs 1.2-1.3.
%%%
Index: extract.c
===
RCS file: /home/ncvs/src/contrib/tar/src/extract.c,v
retrieving revision 1.4
diff -u -2 -r1.4 extract.c
--- extract.c 3 Jul 2002 12:44:31 - 1.4
+++ extract.c 1 Aug 2002 10:44:34 -
@@ -113,7 +113,5 @@
{
we_are_root = geteuid () == 0;
-#ifndef __FreeBSD__
same_permissions_option += we_are_root;
-#endif
same_owner_option += we_are_root;
xalloc_fail_func = extract_finish;
%%%
Bruce
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: pkg_add broken by POLA breakage in tar
Maxim Sobolev wrote:
Bruce Evans wrote:
Revs.1.2-1.3 of tar/src/extract.c break pkg_add (not to mention probably
thousands of user scripts that are no more careful than pkg_add) in
-current and RELENG_4:
Are you sure? My own investigation at the time of the commit showed
that old tar shipped with FreeBSD, was adjusting permissions of
extracting files when running as uid 0 according to current umask
settings, so that IMO 1.2-1.3 actually restored POLA, not broke it.
Need evidence? Here it is:
# cvs co -D 10 months ago src/gnu/usr.bin/tar
[...]
# cd src/gnu/usr.bin/tar
# make
[...]
# mkdir foo
# touch foo/bar
# chmod 777 foo
# chmod 777 foo/bar
# ./tar cvf foo.tar foo
foo/
foo/bar
# rm -rf foo
# ./tar xvf foo.tar
foo/
foo/bar
root@notebook# ls -l | grep foo
drwxr-xr-x 2 root wheel 512 1 âþú 19:01 foo/
-rw-r--r-- 1 root wheel 10240 1 âþú 19:01 foo.tar
root@notebook# ls -l foo
total 0
-rwxr-xr-x 1 root wheel 0 1 âþú 19:01 bar*
# umask
0022
#
-Maxim
-Maxim
% RCS file: /home/ncvs/src/contrib/tar/src/extract.c,v
% Working file: extract.c
% head: 1.4
% branch:
% locks: strict
% access list:
% symbolic names:
% RELENG_4: 1.4.0.2
% TAR_v1_13_25: 1.1.1.1
% FSF: 1.1.1
% keyword substitution: kv
% total revisions: 6; selected revisions: 6
% description:
% ...
%
% revision 1.3
% date: 2002/06/07 06:02:35; author: sobomax; state: Exp; lines: +1 -1
% Disabling automatic --same-owner option when running as uid 0 along with
% the --same-permissions was an overkill, so put it back. This is consistent
% with what our old tar did.
%
% Suggested by: dillon
%
% revision 1.2
% date: 2002/06/07 00:03:23; author: sobomax; state: Exp; lines: +4 -0
% IMO it was a quite ugly idea that if we are running as uid 0 then we can
% safely ignore current umask(2) and assume that permissions should be set
% right like in the archive. Not only it violates POLA, but introduces
^
% huge potential security vulnerability, particularly for ports, where
% many popular archives come with 777 files and dirs.
%
Actually, it is the change violates POLA, and breaks everything that
depends on the historical and still documented behaviour. (The man
page even says that (almost) all permissions are restored even in the
!root case (it says this indirectly by saying that all attributes are
restored if possible and not mentioning the umask or root). The info
page is better.)
This bug showed up as breakage in mutt. mutt uses a setgid utility
named mutt_dotlock to lock /var/mail/*, so it fails to download mail
if mutt_dotlock's setgid bit is lost on extraction. It is probably
another bug that mutt_dotlock attempts to create a temporary file in
/var/mail instead of using flock().
Fixes:
(1) Change pkg_add and thousands of user scripts to use tar -p. This
may reopen security holes closed by respecting the umask.
%%%
Index: extract.c
===
RCS file: /home/ncvs/src/usr.sbin/pkg_install/add/extract.c,v
retrieving revision 1.33
diff -u -2 -r1.33 extract.c
--- extract.c 11 May 2002 04:17:54 - 1.33
+++ extract.c 1 Aug 2002 10:26:10 -
@@ -33,5 +33,5 @@
#define PUSHOUT(todir) /* push out string */ \
if (where_count (int)sizeof(STARTSTRING)-1) { \
- strcat(where_args, |tar --unlink -xf - -C ); \
+ strcat(where_args, |tar --unlink -pxf - -C ); \
strcat(where_args, todir); \
if (system(where_args)) { \
%%%
(2) Restore standard gnu tar behaviour by backing out extract.c revs 1.2-1.3.
%%%
Index: extract.c
===
RCS file: /home/ncvs/src/contrib/tar/src/extract.c,v
retrieving revision 1.4
diff -u -2 -r1.4 extract.c
--- extract.c 3 Jul 2002 12:44:31 - 1.4
+++ extract.c 1 Aug 2002 10:44:34 -
@@ -113,7 +113,5 @@
{
we_are_root = geteuid () == 0;
-#ifndef __FreeBSD__
same_permissions_option += we_are_root;
-#endif
same_owner_option += we_are_root;
xalloc_fail_func = extract_finish;
%%%
Bruce
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
Re: pkg_add broken by POLA breakage in tar
Maxim Sobolev wrote:
Maxim Sobolev wrote:
Bruce Evans wrote:
Revs.1.2-1.3 of tar/src/extract.c break pkg_add (not to mention probably
thousands of user scripts that are no more careful than pkg_add) in
-current and RELENG_4:
Are you sure? My own investigation at the time of the commit showed
that old tar shipped with FreeBSD, was adjusting permissions of
extracting files when running as uid 0 according to current umask
settings, so that IMO 1.2-1.3 actually restored POLA, not broke it.
OK, further investigation shows that the problem is likely that unlike
the old one, the new tar doesn't preserve suid/sgid bits on
extraction, and it is what probably needs to be fixed instead.
Need evidence? Here it is:
# cvs co -D 10 months ago src/gnu/usr.bin/tar
[...]
# cd src/gnu/usr.bin/tar
# make
[...]
# mkdir foo
# touch foo/bar
# chmod 777 foo
# chmod 777 foo/bar
# ./tar cvf foo.tar foo
foo/
foo/bar
# rm -rf foo
# ./tar xvf foo.tar
foo/
foo/bar
root@notebook# ls -l | grep foo
drwxr-xr-x 2 root wheel 512 1 âþú 19:01 foo/
-rw-r--r-- 1 root wheel 10240 1 âþú 19:01 foo.tar
root@notebook# ls -l foo
total 0
-rwxr-xr-x 1 root wheel 0 1 âþú 19:01 bar*
# umask
0022
#
-Maxim
-Maxim
% RCS file: /home/ncvs/src/contrib/tar/src/extract.c,v
% Working file: extract.c
% head: 1.4
% branch:
% locks: strict
% access list:
% symbolic names:
% RELENG_4: 1.4.0.2
% TAR_v1_13_25: 1.1.1.1
% FSF: 1.1.1
% keyword substitution: kv
% total revisions: 6; selected revisions: 6
% description:
% ...
%
% revision 1.3
% date: 2002/06/07 06:02:35; author: sobomax; state: Exp; lines: +1 -1
% Disabling automatic --same-owner option when running as uid 0 along with
% the --same-permissions was an overkill, so put it back. This is consistent
% with what our old tar did.
%
% Suggested by: dillon
%
% revision 1.2
% date: 2002/06/07 00:03:23; author: sobomax; state: Exp; lines: +4 -0
% IMO it was a quite ugly idea that if we are running as uid 0 then we can
% safely ignore current umask(2) and assume that permissions should be set
% right like in the archive. Not only it violates POLA, but introduces
^
% huge potential security vulnerability, particularly for ports, where
% many popular archives come with 777 files and dirs.
%
Actually, it is the change violates POLA, and breaks everything that
depends on the historical and still documented behaviour. (The man
page even says that (almost) all permissions are restored even in the
!root case (it says this indirectly by saying that all attributes are
restored if possible and not mentioning the umask or root). The info
page is better.)
This bug showed up as breakage in mutt. mutt uses a setgid utility
named mutt_dotlock to lock /var/mail/*, so it fails to download mail
if mutt_dotlock's setgid bit is lost on extraction. It is probably
another bug that mutt_dotlock attempts to create a temporary file in
/var/mail instead of using flock().
Fixes:
(1) Change pkg_add and thousands of user scripts to use tar -p. This
may reopen security holes closed by respecting the umask.
%%%
Index: extract.c
===
RCS file: /home/ncvs/src/usr.sbin/pkg_install/add/extract.c,v
retrieving revision 1.33
diff -u -2 -r1.33 extract.c
--- extract.c 11 May 2002 04:17:54 - 1.33
+++ extract.c 1 Aug 2002 10:26:10 -
@@ -33,5 +33,5 @@
#define PUSHOUT(todir) /* push out string */ \
if (where_count (int)sizeof(STARTSTRING)-1) { \
- strcat(where_args, |tar --unlink -xf - -C ); \
+ strcat(where_args, |tar --unlink -pxf - -C ); \
strcat(where_args, todir); \
if (system(where_args)) { \
%%%
(2) Restore standard gnu tar behaviour by backing out extract.c revs 1.2-1.3.
%%%
Index: extract.c
===
RCS file: /home/ncvs/src/contrib/tar/src/extract.c,v
retrieving revision 1.4
diff -u -2 -r1.4 extract.c
--- extract.c 3 Jul 2002 12:44:31 - 1.4
+++ extract.c 1 Aug 2002 10:44:34 -
@@ -113,7 +113,5 @@
{
we_are_root = geteuid () == 0;
-#ifndef __FreeBSD__
same_permissions_option += we_are_root;
-#endif
same_owner_option += we_are_root;
xalloc_fail_func = extract_finish;
%%%
Bruce
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe freebsd-current in the body of the message
To Unsubscribe: send mail to [EMAIL PROTECTED]
with unsubscribe
Re: pkg_add broken by POLA breakage in tar
My recollection matches what Bruce says (and I have been using unix since when version 7 was the latest and greatest). At least the SUN OS 5.6 man page I could locate online says this: The o function modifier is only valid with the x function. p Restore the named files to their original modes, and ACLs if applicable, ignoring the present umask(1). This is the default behavior if invoked as super-user with the x function letter specified. If super-user, SETUID and sticky information are also extracted, and files are restored with their original owners and permissions, rather than owned by root. This superuser behavior is what allows one to use tar as an archiving program. To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: pkg_add broken by POLA breakage in tar
Bakul Shah wrote: My recollection matches what Bruce says (and I have been using unix since when version 7 was the latest and greatest). At least the SUN OS 5.6 man page I could locate online says this: The o function modifier is only valid with the x function. p Restore the named files to their original modes, and ACLs if applicable, ignoring the present umask(1). This is the default behavior if invoked as super-user with the x function letter specified. If super-user, SETUID and sticky information are also extracted, and files are restored with their original owners and permissions, rather than owned by root. This superuser behavior is what allows one to use tar as an archiving program. Well, OK, now I am really confused. So what should we be bound to? To the POLA (old GNU tar in 4.6-release and downward was not fully preserving permissions unless -p is specified, even when invoked by root)? Or to what other systems do? Bruce, what do you think? -Maxim To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: pkg_add broken by POLA breakage in tar
On Thu, 1 Aug 2002, Maxim Sobolev wrote: Maxim Sobolev wrote: Maxim Sobolev wrote: Bruce Evans wrote: Revs.1.2-1.3 of tar/src/extract.c break pkg_add (not to mention probably thousands of user scripts that are no more careful than pkg_add) in -current and RELENG_4: Are you sure? My own investigation at the time of the commit showed Oops, apparently not ... that old tar shipped with FreeBSD, was adjusting permissions of extracting files when running as uid 0 according to current umask settings, so that IMO 1.2-1.3 actually restored POLA, not broke it. OK, further investigation shows that the problem is likely that unlike the old one, the new tar doesn't preserve suid/sgid bits on extraction, and it is what probably needs to be fixed instead. Need evidence? Here it is: ... Sorry, I didn't test it at runtime. I don't really like either changing the Gnu/historical behaviour for root or preserving set*id bits while not preserving other attributes, but since this seems have 10 years of precedence in FreeBSD it doesn't break POLA. Bruce To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
