Subject: bug#8391: chmod setuid & setguid bits

From: Ondrej Vasik <[email-masked]>
Date: Mon, 05 Mar 2012 15:00:34 +0100

On Fri, 2012-02-24 at 11:47 -0800, Paul Eggert wrote:
> On 02/24/2012 11:33 AM, Ondrej Vasik wrote:
> > Yes, but `chmod @755 DIR' approach will not let you to write a script
> > which will work without modification on RHEL-4,RHEL-5 and RHEL-6
> > machine...
> 
> None of these approaches will let you write a script that will work
> without modification on any POSIX platform.  If one wants to be portable,
> one must use the symbolic notation, not the octal.
> 
> None of these approaches will even let you write a script that will work
> without modification on any RHEL platform.  This is because some RHEL
> platforms use the newer coreutils.
> 
> Still, I take your point that the 5-or-more-digit approach will let you write
> scripts that will run on all POSIX platforms without a diagnostic
> (though perhaps not with the desired effect).  And these scripts will
> run and have the desired effect if you know that your scripts will run
> only on a particular subset of POSIX platforms, one where the effect is
> the one desired.
> 
> How about this idea for a compromise?  Implement both notations, but
> recommend leading '@' for future scripts.  It's more likely that a notation
> like leading-'@' would be adopted by future POSIX versions, since it's a
> pure extension, whereas the 5-or-more-digit approach is incompatible with
> some POSIX systems now.  And if leading-'@' is adopted by POSIX, there would
> eventually be a portable way to do what the requester wants.
> 
> Personally I'd be more inclined to go with a pure '@' solution, since
> it's simpler and the portability gains of the compromise are not all
> that great; but I guess the compromise would be OK too.

Hi,
both notations implemented via changing gnulib modechange. I didn't add
the recommendation for leading '@' yet, as this part will probably need
some rewording to better match the standards of coreutils texinfo
perm.texi documentation anyway and I don't know how this should be
properly written.

Both patches (one for gnulib, one for coreutils documentation and
testsuite) attached.

Greetings,
          Ondrej Vasik

>From 4647a343f5e6c903495fa01a50743af5015552a6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Va=C5=A1=C3=ADk?= <[email-masked]>
Date: Mon, 5 Mar 2012 14:43:39 +0100
Subject: [PATCH] chmod: Clear special bits for octal modes with 5+ digits or preceeded by @.
         * NEWS: Mention the changes.
         * doc/perm.texi (Directory Setuid and Setgid) : Document changes.
         * tests/chmod/setuid : Check the new behaviour by test.
         Suggested by Eric Blake.

---
 NEWS               |    4 ++++
 doc/perm.texi      |   26 +++++++++++++++++++++++---
 tests/chmod/setgid |   24 ++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index 8006669..c0657e2 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,10 @@ GNU coreutils NEWS                                    -*- outline -*-
   split now accepts the --additional-suffix option, to append an
   additional static suffix to output file names.
 
+  chmod, mkdir, install now accept new style of octal mode specification.
+  When octal mode is preceeded by @ or is 5+ digits long with leading zeros,
+  it can clear the set user id and set group id bits on directories.
+
 ** Bug fixes
 
   mv now lets you move a symlink onto a same-inode destination file that
diff --git a/doc/perm.texi b/doc/perm.texi
index 84f8500..d1469a9 100644
--- a/doc/perm.texi
+++ b/doc/perm.texi
@@ -495,7 +495,10 @@ alternative to giving a symbolic mode, you can give an octal (base 8)
 number that represents the mode.
 This number is always interpreted in octal; you do not have to add a
 leading @samp{0}, as you do in C.  Mode @samp{0055} is the same as
-mode @samp{55}.
+mode @samp{55}. However, adding leading zeros to create octal number with
+at least 5 digits means that this mode is taken explicitly - so could
+clear even the set-user-ID and set-group-ID bits of directories. The
+same could be enforced by preceeding the numeric mode by "@@" character.
 
 A numeric mode is usually shorter than the corresponding symbolic
 mode, but it is limited in that normally it cannot take into account the
@@ -559,8 +562,11 @@ bits of directories.  If commands like @command{chmod} and
 mechanisms would be less convenient and it would be harder to share
 files.  Therefore, a command like @command{chmod} does not affect the
 set-user-ID or set-group-ID bits of a directory unless the user
-specifically mentions them in a symbolic mode, or sets them in
-a numeric mode.  For example, on systems that support
+specifically mentions them in a symbolic mode, explicitly enforces the
+mode by using "@@" character before numeric mode or by at least 5 digits
+long octal mode,or when sets them in a numeric mode.
+
+For example, on systems that support
 set-group-ID inheritance:
 
 @example
@@ -598,6 +604,20 @@ explicitly in a symbolic mode, e.g.:
 chmod a-s D
 @end example
 
+If you want force the chmod to change directory mode to exact numeric mode
+(clear the special bits), you could use at least 5 digit octal mode
+or preceed the mode with "@@" character, e.g.:
+
+@example
+# These commands try to clear the set-user-ID
+# and set-group-ID bits of directories D and E and set
+# their permissions to 0755.
+mkdir D E
+chmod 6755 D E
+chmod 00755 D
+chmod @@755 E
+@end example
+
 This behavior is a @acronym{GNU} extension.  Portable scripts should
 not rely on requests to set or clear these bits on directories, as
 @acronym{POSIX} allows implementations to ignore these requests.
diff --git a/tests/chmod/setgid b/tests/chmod/setgid
index eaa9351..6db6794 100755
--- a/tests/chmod/setgid
+++ b/tests/chmod/setgid
@@ -45,4 +45,28 @@ chmod 755 d
 
 case `ls -ld d` in drwxr-sr-x*);; *) fail=1;; esac
 
+# make sure that it doesn't clear the special bits for 4 digit octal mode
+chmod 0755 d
+case `ls -ld d` in drwxr-sr-x*);; *) fail=1;; esac
+
+# make sure that it doesn't clear the special bits for 4 digit octal mode
+# with two leading zeros
+chmod 0055 d
+case `ls -ld d` in d---r-sr-x*);; *) fail=1;; esac
+
+# make sure that it clears the special bits for 5 digit octal mode with
+# leading zero
+chmod 00755 d
+case `ls -ld d` in drwxr-xr-x*);; *) fail=1;; esac
+
+# make sure that it clears the special bits even for more leading zeros
+# octal digits
+chmod 00000755 d
+case `ls -ld d` in drwxr-xr-x*);; *) fail=1;; esac
+
+# make sure that it clears the special bits even for exact mode(@)
+chmod @755 d
+case `ls -ld d` in drwxr-xr-x*);; *) fail=1;; esac
+
+
 Exit $fail
-- 
1.7.1

>From f845b1742ce2f06dd8b6a6fe94e3881f7cd0ff4d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Va=C5=A1=C3=ADk?= <[email-masked]>
Date: Mon, 5 Mar 2012 10:34:13 +0100
Subject: [PATCH] modechange: Allow cleaning of special bits on dirs when requested

As discussed in coreutils bug #8391 and rhbz #691466, some users miss
the functionality of cleaning set user id and set group id dir bits by
exact octal mode.
This functionality was removed by 4e3c4c0478e6e1e81f093b80d8272840a39ca2bd
gnulib commit and this patch allows it for 5+ digits octal modes or
octal modes preceeded by "@" character.

* lib/modechange.c: Allow cleaning of special bits on dirs.
  (enum): Add new mode flag - MODE_EXACT_CHANGE.
  (mode_compile): Use MODE_EXACT_CHANGE for 5+ digits octals and octal
                  mode preceeded by @.
  (mode_adjust): Don't mask the special bit change for MODE_EXACT_CHANGE.
---
 ChangeLog        |    8 ++++++++
 lib/modechange.c |   21 ++++++++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4ec700f..54b9ffb 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2012-03-04  Ondrej Vasik  <[email-masked]>
+	modechange: Allow cleaning of special bits on dirs when requested
+	* lib/modechange.c: Allow cleaning of special bits on dirs.
+	(enum): Add new mode flag - MODE_EXACT_CHANGE.
+	(mode_compile): Use MODE_EXACT_CHANGE for 5+ digits octals and octal
+	                mode preceeded by @.
+	(mode_adjust): Don't mask the special bit change for MODE_EXACT_CHANGE.
+
 2012-03-04  Bruno Haible  <[email-masked]>
 
 	sqrt* tests: More tests.
diff --git a/lib/modechange.c b/lib/modechange.c
index 4ae90ca..a4741ac 100644
--- a/lib/modechange.c
+++ b/lib/modechange.c
@@ -32,6 +32,7 @@
 #include "stat-macros.h"
 #include "xalloc.h"
 #include <stdlib.h>
+#include <string.h>
 
 /* The traditional octal values corresponding to each mode bit.  */
 #define SUID 04000
@@ -91,7 +92,11 @@ enum
     /* Instead of the typical case, copy some existing permissions for
        u, g, or o onto the other two.  Which of u, g, or o is copied
        is determined by which bits are set in the 'value' field.  */
-    MODE_COPY_EXISTING
+    MODE_COPY_EXISTING,
+
+    /* Instead of the typical case, we want to enforce the exact
+       requested mode */
+    MODE_EXACT_CHANGE
   };
 
 /* Description of a mode change.  */
@@ -136,6 +141,10 @@ mode_compile (char const *mode_string)
   /* The array of mode-change directives to be returned.  */
   struct mode_change *mc;
   size_t used = 0;
+  bool exact_change = (strlen(mode_string) > 4 || '@' == *mode_string);
+
+  if ('@' == *mode_string)
+    mode_string++;
 
   if ('0' <= *mode_string && *mode_string < '8')
     {
@@ -156,7 +165,11 @@ mode_compile (char const *mode_string)
 
       mode = octal_to_mode (octal_mode);
       mentioned = (mode & (S_ISUID | S_ISGID)) | S_ISVTX | S_IRWXUGO;
-      return make_node_op_equals (mode, mentioned);
+
+      mc = make_node_op_equals (mode, mentioned);
+      if (exact_change)
+        mc->flag = MODE_EXACT_CHANGE;
+      return mc;
     }
 
   /* Allocate enough space to hold the result.  */
@@ -320,11 +333,13 @@ mode_adjust (mode_t oldmode, bool dir, mode_t umask_value,
     {
       mode_t affected = changes->affected;
       mode_t omit_change =
-        (dir ? S_ISUID | S_ISGID : 0) & ~ changes->mentioned;
+        ((dir && changes->flag != MODE_EXACT_CHANGE) ?
+          S_ISUID | S_ISGID : 0) & ~ changes->mentioned;
       mode_t value = changes->value;
 
       switch (changes->flag)
         {
+        case MODE_EXACT_CHANGE:
         case MODE_ORDINARY_CHANGE:
           break;
 
-- 
1.7.1