Bug#690839: run-parts: By default the umask is set to 022

2012-11-12 Thread Jakub Wilk

* Clint Adams , 2012-11-11, 23:19:
Yes, I'm aware of that. On the other hand: The current behavior seems 
insane to me, unless there would be a good reason for by default 
changing the umask to a less secure setting, but then that should be 
documented.


I agree that the current run-parts behavior is indeed unfortunate.

I agree with your objection though. To avoid that kind of trouble, I'd 
suggest a stepwise change:


* For the next release,


Next being wheezy or jessie? If the former, I'm afraid it's too late.

make run-parts issue a warning that it changes the umask, and that 
this behavior is going to change in the future. Suppress the warning 
if the user sets the umask with -u.


* For the following release, change run-parts to not tamper with the 
umask anymore, and warn about the new situation, unless -u is given.


* Finally remove the warning.

Would that be of any use?


How about something that doesn't take more than a release cycle or 
involve annoying warnings?


1) Identify packages that rely on the current behavior. File bugs.
2) Wait until wheezy is released.
3) Fix run-parts not to fiddle with umask by default. Add Debian.NEWS 
explaining the situation.


--
Jakub Wilk


--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org



Bug#690839: run-parts: By default the umask is set to 022

2012-11-12 Thread Stefan Klinger
Hi,

the attached patch against the 4.3.2 source [1] implements the solution
I've sketched earlier.

On 2012-Nov-11 23:19 (+), Clint Adams wrote with possible deletions:
> >   * For the next release, make run-parts issue a warning that it changes
> > the umask, and that this behavior is going to change in the future.
> > Suppress the warning if the user sets the umask with -u.
> > 
> >   * For the following release, change run-parts to not tamper with the
> > umask anymore, and warn about the new situation, unless -u is given.
> > 
> >   * Finally remove the warning.

For the trasition, I introduced the macro RUNPARTS_FIX_UMASK, which can
be set to

0 - no change
1 - warn about changing umask to 022 <- start here
2 - warn about not changing umask anymore
3 - fixed, please remove unused code

0 is the current version, and when 3 is reached, the macro and the old
code snippets can be removed.

Kind regards,
Stefan


[1] 
http://ftp.de.debian.org/debian/pool/main/d/debianutils/debianutils_4.3.2.tar.gz


-- 
Stefan Klinger  o/klettern
/\/  bis zum
send plaintext only - max size 32kB - no spam \   Abfallen
http://stefan-klinger.de
--- run-parts.old.c	2012-11-12 13:47:22.0 +0100
+++ run-parts.new.c	2012-11-12 14:33:05.0 +0100
@@ -35,6 +35,22 @@
 #define RUNPARTS_ERE 1
 #define RUNPARTS_LSBSYSINIT 100
 
+/* Transition to fix Bug#690839.  Valid values:
+ *   0 - no change
+ *   1 - warn about changing umask
+ *   2 - warn about not changing umask
+ *   3 - fixed, please remove unused code
+ */
+#define RUNPARTS_FIX_UMASK 1
+
+#if RUNPARTS_FIX_UMASK == 1
+char const * warn_umask =
+ "Changing umask to 022.  Try `run-parts --help' for more information.";
+#elif RUNPARTS_FIX_UMASK == 2
+char const * warn_umask =
+ "Leaving umask unchanged.  Try `run-parts --help' for more information.";
+#endif
+
 int test_mode = 0;
 int list_mode = 0;
 int verbose_mode = 0;
@@ -98,7 +114,21 @@ void usage()
 	  "  --lsbsysinitvalidate filenames based on LSB sysinit specs.\n"
 	  "  --new-session   run each script in a separate process session\n"
 	  "  --regex=PATTERN validate filenames based on POSIX ERE pattern PATTERN.\n"
+#if RUNPARTS_FIX_UMASK == 0
 	  "  -u, --umask=UMASK   sets umask to UMASK (octal), default is 022.\n"
+#elif RUNPARTS_FIX_UMASK == 1
+	  "  -u, --umask=UMASK   sets umask to UMASK (octal), default is 022.  Future\n"
+  "  versions of run-parts will not change the umask\n"
+  "  implicitly anymore.  Use -u022 to retain the current\n"
+  "  behavior.\n"
+#elif RUNPARTS_FIX_UMASK == 2
+	  "  -u, --umask=UMASK   sets umask to UMASK (octal).  By default, and diverging\n"
+  "  from historical behavior, the umask is not changed\n"
+  "  implicitly to 022 anymore.\n"
+#elif RUNPARTS_FIX_UMASK == 3
+	  "  -u, --umask=UMASK   sets umask to UMASK (octal).  If omitted, the umask is\n"
+  "  left unchanged.\n"
+#endif
 	  "  -a, --arg=ARGUMENT  pass ARGUMENT to scripts, use once for each argument.\n"
 	  "  -V, --version   output version information and exit.\n"
 	  "  -h, --help  display this help and exit.\n");
@@ -468,7 +498,9 @@ void run_parts(char *dirname)
 int main(int argc, char *argv[])
 {
   custom_ere = NULL;
+#if RUNPARTS_FIX_UMASK < 2
   umask(022);
+#endif
   add_argument(0);
 
   for (;;) {
@@ -503,6 +535,9 @@ int main(int argc, char *argv[])
   break;
 case 'u':
   set_umask();
+#if RUNPARTS_FIX_UMASK > 0  && RUNPARTS_FIX_UMASK < 3
+  warn_umask = 0;
+#endif
   break;
 case 'a':
   add_argument(optarg);
@@ -522,6 +557,10 @@ int main(int argc, char *argv[])
 }
   }
 
+#if RUNPARTS_FIX_UMASK > 0  && RUNPARTS_FIX_UMASK < 3
+  if (warn_umask) fprintf(stderr, "%s\n", warn_umask);
+#endif
+  
   /* We require exactly one argument: the directory name */
   if (optind != (argc - 1)) {
 error("missing operand");


Bug#690839: run-parts: By default the umask is set to 022

2012-11-11 Thread Clint Adams
On Sun, Nov 11, 2012 at 10:35:46PM +0100, Stefan Klinger wrote:
> Yes, I'm aware of that.  On the other hand: The current behavior seems
> insane to me, unless there would be a good reason for by default
> changing the umask to a less secure setting, but then that should be
> documented.
> 
> I agree with your objection though.  To avoid that kind of trouble, I'd
> suggest a stepwise change:
> 
>   * For the next release, make run-parts issue a warning that it changes
> the umask, and that this behavior is going to change in the future.
> Suppress the warning if the user sets the umask with -u.
> 
>   * For the following release, change run-parts to not tamper with the
> umask anymore, and warn about the new situation, unless -u is given.
> 
>   * Finally remove the warning.
> 
> Would that be of any use?

Let's solicit debian-devel's thoughts and opinions.


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org



Bug#690839: run-parts: By default the umask is set to 022

2012-11-11 Thread Stefan Klinger
Thank you for looking into this!

On 2012-Nov-11 16:22 (+), Clint Adams wrote with possible deletions:
> That might surprise anyone relying on the current behavior.

Yes, I'm aware of that.  On the other hand: The current behavior seems
insane to me, unless there would be a good reason for by default
changing the umask to a less secure setting, but then that should be
documented.

I agree with your objection though.  To avoid that kind of trouble, I'd
suggest a stepwise change:

  * For the next release, make run-parts issue a warning that it changes
the umask, and that this behavior is going to change in the future.
Suppress the warning if the user sets the umask with -u.

  * For the following release, change run-parts to not tamper with the
umask anymore, and warn about the new situation, unless -u is given.

  * Finally remove the warning.

Would that be of any use?
Stefan


-- 
Stefan Klinger  o/klettern
/\/  bis zum
send plaintext only - max size 32kB - no spam \   Abfallen
http://stefan-klinger.de


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org



Bug#690839: run-parts: By default the umask is set to 022

2012-11-11 Thread Clint Adams
On Thu, Oct 18, 2012 at 02:03:57PM +0200, Stefan Klinger wrote:
> Suggested fix: By default, run-parts should not change the umask.  The
> user may still use -u022 to get the original behaviour.

That might surprise anyone relying on the current behavior.


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org



Bug#690839: run-parts: By default the umask is set to 022

2012-10-18 Thread Stefan Klinger
Package: debianutils
Version: 4.3.2

Problem description: As the manual for run-parts describes, it sets the
umask to 022 unless specified otherwise.  IMHO this violates the
principle of least surprise: I would (did) not expect any tool to fiddle
with the umask (or any other environment stuff) unless required for it's
operation, or explicitly requested by the user.

Workaround: For the meantime, use `run-parts --umask="$(umask)" ...`.

Suggested fix: By default, run-parts should not change the umask.  The
user may still use -u022 to get the original behaviour.

System information:
$ uname -a
Linux bellbird 3.2.0-3-amd64 #1 SMP Mon Jul 23 02:45:17 UTC 2012 x86_64 
GNU/Linux
$ dpkg -s libc6 | grep ^Version
Version: 2.13-35

Thanks!
Stefan


-- 
Stefan Klinger  o/klettern
/\/  bis zum
send plaintext only - max size 32kB - no spam \   Abfallen
http://stefan-klinger.de


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org