Debian Bug report logs - #276419
su appends the positional args to the command line

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: "Helmut Waitzmann" (Debian Bug Tracking System) <Helmut.Waitzmann@web.de>

To: Debian Bug Tracking System <submit@bugs.debian.org>

Subject: su appends the positional args to the command line

Date: Wed, 13 Oct 2004 22:10:56 +0200

Package: login
Version: 20000902-12
Severity: important
File: /bin/su


The manual page su(1) says:

   NAME
	  su - Change user ID or become super-user

   SYNOPSIS
	  su [OPTS] [-] [username [ARGS]]


Description of the bug:

su appends the ARGS to the command line rather than simply passing
them to execvp (see execvp(3)) as an argument vector (char *const
argv[]) when executing the shell to be run.

To expose the error, run the following command:

   $ su --shell=/bin/sh -c 'printf :%q:\\n ${1+"$@"}' "$USER" \
      sh su concatenates the "shell's" positional parameters \
      rather than passing them to execvp

which produces the output (type in your password):

   Password: 
   sh: -c: line 1: unexpected EOF while looking for matching `''
   sh: -c: line 2: syntax error: unexpected end of file

Quoting the manual page su(1),

   Any arguments supplied after the username will be passed to the
   invoked shell (shell must support the -c command line option in
   order for a command to be passed to it).

the expected output would be:

   :su:
   :concatenates:
   :the:
   :shell\'s:
   :positional:
   :parameters:
   :rather:
   :than:
   :passing:
   :them:
   :to:
   :execvp:

as can be seen when running the following command (which should be
equivalent with respect to running the shell):

   $ /bin/sh -c 'printf :%q:\\n ${1+"$@"}' \
      sh su concatenates the "shell's" positional parameters \
      rather than passing them to execvp


-- System Information
Debian Release: 3.0
Architecture: i386
Kernel: Linux kugelfisch 2.4.18-bf2.4 #1 Son Apr 14 09:53:28 CEST 2002 i686
Locale: LANG=C, LC_CTYPE=de_DE

Versions of packages login depends on:
ii  libc6                         2.2.5-11.5 GNU C Library: Shared libraries an
ii  libpam-modules                0.72-35    Pluggable Authentication Modules f
ii  libpam0g                      0.72-35    Pluggable Authentication Modules l

-- 
Wenn Sie mir E-Mail schreiben, stellen |  When writing me e-mail, please
Sie bitte vor meine E-Mail-Adresse     |  precede my e-mail address with
meinen Vor- und Nachnamen, etwa so:    |  my full name, like
Helmut Waitzmann <xxx@example.net>, (Helmut Waitzmann) xxx@example.net

Message #10 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Nicolas François <nicolas.francois@centraliens.net>

To: Debian Bug Tracking System <276419@bugs.debian.org>

Subject: Re: Bug#276419: su appends the positional args to the command line

Date: Wed, 23 Mar 2005 23:43:13 +0100

Package: login
Version: 1:4.0.3-31sarge1
Followup-For: Bug #276419

Hello,

I had a look at this bug and came up with this patch.

This implementation is much simpler, so I wonder if I did not forget
something.


For the impatient, it could also have been solve by a proper escape:
$ su --shell=/bin/sh -c 'printf :%q:\\n ${1+"$@"}' "$USER" \
    sh su concatenates the "shell\'s" positional parameters \
    rather than passing them to execvp

or

$ su --shell=/bin/sh -c 'printf :%q:\\n ${1+"$@"}' "$USER" \
    sh su concatenates the "\"shell\'s\"" positional parameters \
    rather than passing them to execvp


Best Regards,
-- 
Nekral

Message #15 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Nicolas François <nicolas.francois@centraliens.net>

To: Debian Bug Tracking System <276419@bugs.debian.org>

Subject: Re: Bug#276419: su appends the positional args to the command line

Date: Wed, 23 Mar 2005 23:46:30 +0100

[Message part 1 (text/plain, inline)]

Hi,

I'm sure you all want to see it;)

-- 
Nekral

[su.c.patch (text/plain, attachment)]

Message #20 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 276419-submitter@bugs.debian.org, 276419@bugs.debian.org

Subject: Overflated severity?

Date: Thu, 24 Mar 2005 19:14:57 +0100

I'm afraid I don't really see the rationale behind the severity of
this bug report.

Does it really deserve the "important" severity because it "has a
major effect on the usability of a package, without rendering it
completely unusable to everyone"?

My current opinion is that it does not fit this definition which makes
me think that it should have its severity lowered to "normal".

After all, all common use of su are not affected by this bug, which is
triggered in a rather specific situation.

A patch has been proposed for solving it. Please test it if you have
such possibility.

See http://bugs.debian.org/276419


-- 

Message #28 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 276419@bugs.debian.org

Cc: control@bugs.debian.org

Subject: Re: [Pkg-shadow-devel] Bug#276419: Overflated severity?

Date: Fri, 25 Mar 2005 07:37:06 +0100

tags 276419 fixed-upstream
thanks

> Agreed. Since there is a very simple fix (escape of arguments, which
> people used to shell programming should be able to achieve), a normal (or
> even minor) severity could be used.

Let's see if the submitter has some input.

PS to people in the pkg-shadow-devel list : when answering to threads
which come from the BTS (mails sent to nnnnnn@bugs.debian.org), please
use the bug address and NOT the mailing list address.

When answering to the list, only the list members will see the
discussion. Answering to the bug number will archive the discussion in
the bug log. In both cases, you'll receive the answer as the
maintainer address for the package is....the mailing list..:-)

Also, in cases where the bug submitter was CC'ed (such as here where
we want his/her input), please keep him/her CC'ed. Remember that mails
sent to a given bug in Debian BTS do NOT go to the bug submitter.

> > Also, I already had a look at this bug some time ago
> > (half a year?). As far as I remember, the bug is fixed
> > in upstream -- need to re-check.
> 
> Upstream's code for run_shell is very different (lots of PAM stuff) and
> use the arguments the same way as my patch.
> 
> I also tested it to make sure, and (with the exception that --shell is not
> supported), it works.
> 
> If anybody change the severity, it could also be tagged fixed-upstream


Done (feel free to do so in such cases...we are ALL maintainers of the
package)

> 
> 
> BTW, do you think the options supported by the Debian's su will be needed
> after Sarge (currently it support --command, --preserve-environment and
> --shell, but IMHO upstream's su has no option).


Well, this will be part of the game "what to do with Debian specific
patches". Let's first finish the bug triage.

Message #41 received at 276419@bugs.debian.org (full text, mbox, reply):

From: "Helmut Waitzmann" (Debian Bug Tracking System) <Helmut.Waitzmann@web.de>

To: Christian Perrier <bubulle@debian.org>

Cc: 276419@bugs.debian.org, control@bugs.debian.org

Subject: Re: [Pkg-shadow-devel] Bug#276419: Overflated severity?

Date: Thu, 31 Mar 2005 02:22:22 +0200

Christian Perrier <bubulle@debian.org> quotes (I don't know whom):

>> Since there is a very simple fix (escape of arguments, which
>> people used to shell programming should be able to achieve),

... which is not so simple, if the arguments are to be computed by some
program, for instance:

$ find . \! \( -exec chown -- root:other '{}' \; \) -o -print0 | \
   xargs -0 -e -r -- sh -c \
   'exec su -- - root \
   '\''for f; do test -O "$f" \! -L "$f" && chmod -- u-s,go=u-w "$f"; done'\'' \
   -sh ${1+"$@"} < /dev/tty' sh

... or is impossible if one wants to supply invocation shell options when
invoking a shell

$ su -- - "$LOGNAME" -x

, a script file or positional parameters and no commandline.
-- 
Wenn Sie mir E-Mail schreiben, stellen |  When writing me e-mail, please
Sie bitte vor meine E-Mail-Adresse     |  precede my e-mail address with
meinen Vor- und Nachnamen, etwa so:    |  my full name, like
Helmut Waitzmann <xxx@example.net>, (Helmut Waitzmann) xxx@example.net

Message #46 received at 276419@bugs.debian.org (full text, mbox, reply):

From: "Helmut Waitzmann" (Debian Bug Tracking System) <Helmut.Waitzmann@web.de>

To: Christian Perrier <bubulle@debian.org>

Cc: 276419-quiet@bugs.debian.org, 276419-submitter@bugs.debian.org, 276419@bugs.debian.org

Subject: Re: Bug#276419: Overflated severity?

Date: Thu, 31 Mar 2005 02:22:27 +0200

[Message part 1 (text/plain, inline)]

Christian Perrier <bubulle@debian.org> writes:

>I'm afraid I don't really see the rationale behind the severity of
>this bug report.
>
>Does it really deserve the "important" severity because it "has a
>major effect on the usability of a package, without rendering it
>completely unusable to everyone"?
>
>My current opinion is that it does not fit this definition which makes
>me think that it should have its severity lowered to "normal".
>
>After all, all common use of su are not affected by this bug, which is
>triggered in a rather specific situation.

Is this situation specific enough?

$ find some files to be archived -print0 | xargs -0 -e -r -- \
  sh -c 'exec su -- - archiver ${1+"$@"} < /dev/tty' sh

(The user "archiver" has got a special "shell": a program which expects
file names of files to be archived as positional parameters).

Nicolas François' patch solves the commandline problem.

A second problem, which is related to the first (erroneous processing
of the shell's arguments), but afaics is not solved by his patch:

I like to use su when I'm going to change and test the configuration of
my login shell.  To see, what's going on, when doing a login I'd like to
start the command

$ su -- - "$LOGNAME" -x

in order to get a login shell that echoes the commands it's going to
execute.

However, something is going wrong:

$ su -- - "$LOGNAME" -x
Password: 
-su: option `-c' requires an argument

According to the manual page su(1) and to the Linux Standard Base
Specification 1.3 and 2.0.1, su will process its option arguments and
pass all ARGS following the username to the shell to be started.  In this
case the ARGS would be the single argument "-x":  The login shell will be
invoked with the positional arguments vector { "-su", "-x", NULL }.

su supplies the arguments vector { "-su", "-c", "-x", NULL } to the shell
to be invoked and breaks those requirements (which is afair a severe
violation of the Debian policy).

The Problem lies at lines 209 to 210 of Nicolas François' patched version
of su.c:

  if (command || additional_args)
    args[argno++] = "-c";

This patch (applied after Nicolas François' patch) should correct that:

[commandline.2.patch (text/x-patch, attachment)]

[Message part 3 (text/plain, inline)]

There are some minor corrections included:

* Compute the exact size of the argument vector, so don't allocate a
  longer one than is necessary.

* The function 'elements' doesn't dereference a NULL pointer argument.


I'd like to see the function 'elements' returning a value of type size_t
rather than of type int.

The following patch reflects this.

[size_t.patch (text/x-patch, attachment)]

[Message part 5 (text/plain, inline)]

su.c does include <sys/types.h>.  It is necessary, to include <stddef.h>
in order to have size_t defined?  If the answer is "yes", then the last
patch won't be enough.  If one inserts a '#include <stddef.h>', what
makefiles, autoconffiles, etc. will have to be changed, too?

If you like, apply this patch after the one above.


As I couldn't compile neither Nicolas François' nor the following patches
(Debian Woody lacks automake1.7), this patches are untested.  Would you
please have a look at them?
-- 
Wenn Sie mir E-Mail schreiben, stellen |  When writing me e-mail, please
Sie bitte vor meine E-Mail-Adresse     |  precede my e-mail address with
meinen Vor- und Nachnamen, etwa so:    |  my full name, like
Helmut Waitzmann <xxx@example.net>, (Helmut Waitzmann) xxx@example.net

Message #59 received at 276419-quiet@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: Helmut Waitzmann <Helmut.Waitzmann@web.de>

Cc: 276419-quiet@bugs.debian.org, 276419-submitter@bugs.debian.org

Subject: Re: Bug#276419: Overflated severity?

Date: Thu, 31 Mar 2005 07:07:28 +0200

> >My current opinion is that it does not fit this definition which makes
> >me think that it should have its severity lowered to "normal".
> >
> >After all, all common use of su are not affected by this bug, which is
> >triggered in a rather specific situation.
> 
> Is this situation specific enough?

I still think that we are at the limits between normal and important
severities...

However, given that I give this BR more "importance" than several
other bugs in shadow bug log, I agree to keep the "important" severity.


> According to the manual page su(1) and to the Linux Standard Base
> Specification 1.3 and 2.0.1, su will process its option arguments and
> pass all ARGS following the username to the shell to be started.  In this
> case the ARGS would be the single argument "-x":  The login shell will be
> invoked with the positional arguments vector { "-su", "-x", NULL }.


I don't really see any policy requirement broken here. I just see
advertised behaviour (LSB compliance of su) to be somewhat not
complete. So, obviously, I will oppose any severity inflation to
"serious", of course....

> 
> su supplies the arguments vector { "-su", "-c", "-x", NULL } to the shell
> to be invoked and breaks those requirements (which is afair a severe
> violation of the Debian policy).
> 
> The Problem lies at lines 209 to 210 of Nicolas François' patched version
> of su.c:
> 
>   if (command || additional_args)
>     args[argno++] = "-c";
> 
> This patch (applied after Nicolas François' patch) should correct that:


> As I couldn't compile neither Nicolas François' nor the following patches
> (Debian Woody lacks automake1.7), this patches are untested.  Would you
> please have a look at them?


Nicolas, this is a mission for you:

-gather the needed patches for #276419
-build a patch to upstream sources and commit it in the "sid" branch
*as a dpatch patch* for this bug
-build a -32 version from all this
-report here...:-)

I hereby nominate Nicolas as owner of this bug. Nico, Thou shalt not
sleep until Thou have found the solution...:-)

Message #67 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Nicolas François <nicolas.francois@centraliens.net>

To: 276419@bugs.debian.org, 276419-submitter@bugs.debian.org

Cc: control@bugs.debian.org

Subject: Re: Bug#276419: Overflated severity?

Date: Wed, 22 Jun 2005 22:24:14 +0200

tags 276419 pending
thanks

Hello Helmut,

Thanks a lot for your patches, and sorry for the late reply (mails sent to
-quiet@bugs.debian.org are not sent to the mailing list).

On Thu, Mar 31, 2005 at 02:22:27AM +0200, Helmut Waitzmann wrote:
> Nicolas François' patch solves the commandline problem.
> 
> A second problem, which is related to the first (erroneous processing
> of the shell's arguments), but afaics is not solved by his patch:
> 
> I like to use su when I'm going to change and test the configuration of
> my login shell.  To see, what's going on, when doing a login I'd like to
> start the command
> 
> $ su -- - "$LOGNAME" -x
> 
> in order to get a login shell that echoes the commands it's going to
> execute.
[...]
> The Problem lies at lines 209 to 210 of Nicolas François' patched version
> of su.c:
> 
>   if (command || additional_args)
>     args[argno++] = "-c";
> 
> This patch (applied after Nicolas François' patch) should correct that:
[...commandline.2.patch...]

This patch is applied to the Debian's shadow repository and should appear
in the next release.

> I'd like to see the function 'elements' returning a value of type size_t
> rather than of type int.
> 
> The following patch reflects this.
[...size_t.patch...]

I've not applied this one because I didn't want to scatter the Debian
patch (we are trying to update Debain's shadow to the lattest upstream,
and the size of the patch matters).

Kind Regards,
-- 
Nekral

Message #77 received at 276419-close@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 276419-close@bugs.debian.org

Subject: Bug#276419: fixed in shadow 1:4.0.3-36

Date: Tue, 05 Jul 2005 16:02:32 -0400

Source: shadow
Source-Version: 1:4.0.3-36

We believe that the bug you reported is fixed in the latest version of
shadow, which is due to be installed in the Debian FTP archive:

login_4.0.3-36_i386.deb
  to pool/main/s/shadow/login_4.0.3-36_i386.deb
passwd_4.0.3-36_i386.deb
  to pool/main/s/shadow/passwd_4.0.3-36_i386.deb
shadow_4.0.3-36.diff.gz
  to pool/main/s/shadow/shadow_4.0.3-36.diff.gz
shadow_4.0.3-36.dsc
  to pool/main/s/shadow/shadow_4.0.3-36.dsc



A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 276419@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Christian Perrier <bubulle@debian.org> (supplier of updated shadow package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Format: 1.7
Date: Mon, 20 Jun 2005 23:37:56 +0300
Source: shadow
Binary: login passwd
Architecture: source i386
Version: 1:4.0.3-36
Distribution: unstable
Urgency: low
Maintainer: Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>
Changed-By: Christian Perrier <bubulle@debian.org>
Description: 
 login      - system login tools
 passwd     - change and administer password and group data
Closes: 75181 78961 87301 109279 192849 219321 244754 245332 248150 256732 261490 266281 269583 276419 286258 286616 287410 288106 288827 290842 298060 298773 304350 309408 312428 312429 312430 312431 312471 314303 314407 314423 314539 314727 315362 315372 315375 315378 315391 315407 315426 315429 315434 315483 315567 315727 315767 315783 315809 315812 315840 315972 316026
Changes: 
 shadow (1:4.0.3-36) unstable; urgency=low
 .
   * Debian specific programs fixes:
     - Re-enable logging and displaying failures on login when login is
       compiled with PAM and when FAILLOG_ENAB is set to yes. And create the
       faillog file if it does not exist on postinst (as on Woody).
       Closes: #192849
     - do not localize login's syslog messages.
   * Debian packaging fixes:
     - Fix FTBFS with new dpkg 1.13 and use a correct dpkg-architecture
       invocation. Closes: #314407
     - Add a comment about potential sensitive information exposure
       when LOG_UNKFAIL_ENAB is set in login.defs
       Closes: #298773
     - Remove limits.5 and limits.conf.5 man pages which do not
       reflect the way we deal with limits in Debian
       Closes: #288106, #244754
     - debian/login.defs:
       - Make SU_PATH and PATH consistent with the values used in /etc/profile
         Closes: #286616
       - Comment the UMASK setting which is more confusing than useful
         as it only affects console logins. Better use pam_umask instead
         Closes: #314539, #248150
       - Add a comment about "appropriate" values for umask
         Closes: #269583
       - Correct the assertion about the variable defined by QMAIL_DIR
         which is MAILDIR, not MAIL
         Closes: #109279
       - Move the PASS_MAX_LEN variable at the end of login.defs as this
         is obsoleted when using PAM
         Closes: #87301
     - debian/passwd.config:
       - Re-enable the password confirmation question at critical priority
         Closes: #304350
       - Do no prompt again for the login name when the two passwords don't
         match while creating a new user
         Closes: #245332
     - debian/add-shell.sh, debian/remove-shell.sh, debian/shadowconfig.sh,
       debian/passwd.config, debian/passwd.postinst:
       - checked for bashisms, replaced "#!/bin/bash" with "#!/bin/sh",
         Closes: #315767
       - replaced "test XXX -a YYY" XSI:isms with "test XXX && test YYY",
         for rationale see:
         http://www.opengroup.org/onlinepubs/009695399/utilities/test.html
       - replaced all unneeded "egrep"s with basic "grep"s
         Closes: #256732
     - debian/rules:
       Remove the setuid bit on login
       Closes: #298060
     - debian/passwd.templates:
       Templates rewrite to shorten them down a little and make them DTSG
       compliant. Give more details about what the user's full name is used
       for.
       Closes: #287410
     - Updated to Standards: 3.6.2 (checked)
   * Debconf translation updates:
     - Estonian added. Closes: #312471
     - Basque updated. Closes: #314303
     - Malagasy updated. Closes: #290842
     - Punjabi updated. Closes: #315372
     - Danish updated. Closes: #315378
     - Polish updated. Closes: #315391
     - Japanese updated. Closes: #315407
     - Brazilian Portuguese updated. Closes: #315426
     - Czech updated. Closes: #315429
     - Spanish updated. Closes: #315434
     - Lithuanian updated. Closes: #315483
     - Galician updated. Closes: #315362
     - Portuguese updated. Closes: #315375
     - Simplified Chinese updated. Closes: #315567
     - French updated
     - Ukrainian updated. Closes: #315727
     - Welsh updated. Closes: #315809
     - Slovak updated. Closes: #315812
     - Romanian updated. Closes: #315783
     - Finnish updated. Closes: #315972
     - Catalan updated. Closes: #316026
   * Man pages translation updates:
     - Remove the too outdated Korean translation of newgrp.1
       which doesn't even mention sg
       Closes: #261490
   * Man pages correction for Debian specific issues:
     - 402_usermod.8-system-users-range-286258:
       Document the system user range from 0 to 999 in Debian
       Closes: #286258
   * Upstream bugs not fixed in upstream releases or CVS:
     - 423_su_pass_args_without_concatenation
       Thanks to Helmut Waitzmann.
       Closes: #276419
       * pass the argument to the shell or command without concatenation
         before the call to exec.
       * If no command is provided, the arguments after the username are for
         the shell, no -c has to be appended.
     - 008_su_ignore_SIGINT
       * Also ignore SIGQUIT in su to avoid defeating the delay.
         The gain in security is very minor.
         Closes: #288827
     - 424_pwck.8_quiet_option
       pwck(8): document the -q option. Closes: #309408
     - 425_lastlog_8_sparse
       lastlog(8): Document that lastlog is a sparse file, and don't need to be
       rotated. Closes: #219321
     - 426_grpck_group-gshadow_members_consistency
       * (grpck) warn for inconsistencies between members in /etc/group and gshadow
         Closes: #75181
       * (pwck and grpck) warn and propose a fix for entries present in the
         regular /etc/group or /etc/passwd files and not in shadow/gshadow.
     - 427_chage_expiry_0
       Fix chage display in the case of null expiry fields (do not display
       Never, but 01 Jan 1970)
       Closes: #78961
   * Upstream bugs already fixed in upstream releases or CVS:
     - Corrected typos in chfn.1. Closes: #312428
     - Corrected typos in gshadow.5. Closes: #312429
     - Corrected typos in shadow.5. Closes: #312430
     - Corrected typos in grpck.8. Closes: #312431
     - Added patch (356th) for su to propagate SIGSTOP up and SIGCONT down.
       Added similar patch (357th) for newgrp. Both changes only affect
       operation with CLOSE_SESSION set to yes (in /etc/login.defs).
       Closes: #314727
   * Translation updates:
     - debian/patches/010_more-i18ned-messages
       - More messages are translatable. We will deal with the translation
         updates after syncing with upstream.
         Closes: #266281
     - debian/patches/114_eu:
       - Basque translation update. Closes: #314423
     - debian/patches/132_vi.dpatch:
       - Vietnamese translation update. Closes: #315840
Files: 
 2b951dfb5a5258b06dbf4cc9c1c10a9b 843 base required shadow_4.0.3-36.dsc
 c282dd24f1a680566120ef684f5c0386 1405333 base required shadow_4.0.3-36.diff.gz
 c3e579b2641ed0587fa4d8a2fb00e56c 504416 base required passwd_4.0.3-36_i386.deb
 9608524e0d057f7cbe832b35bde32f2e 590616 base required login_4.0.3-36_i386.deb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCyuJO1OXtrMAUPS0RAh8zAKCdD/46/ukzdT+o7jJwPZYJ/ZnP2QCeImF4
ZIx948C5htLynLJrbekYXn4=
=Mslh
-----END PGP SIGNATURE-----

Message #82 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Nicolas François <nicolas.francois@centraliens.net>

To: 276419@bugs.debian.org, 276419-submitter@bugs.debian.org

Subject: Re: Bug#276419: su appends the positional args to the command line

Date: Fri, 8 Jul 2005 15:03:32 +0200

Hello Helmut,

a patch was included in the 4.0.3-36 release, but this one breaks some
scripts (see #317264).
Thus, I'm considering to revert this patch and fix your bug by documenting
the su behavior in its man page.

Here are some details on the issues introduced by the patch:

 * arguments are no more concatenated to provide only one string to the -c
   option of the shell:
   if some arguments are provided after the command provided by -c, these
   arguments are provided to the shell that interprets the command, not to
   the command itself.
   For example:
     $ bash -c 'echo $@' a b c
   will print "b c"
   but:
     $ bash -c echo a b c
   will only display an empty line
 * -c is no more provided to the shell when it is not provided to su:
   This break invocation of su like:
     $ su $LOGNAME bash
     /bin/bash: /bin/bash: cannot execute binary file

As the old behaviors are assumed by some packages (at least pbuilder,
and probably others), I think it is better to revert the patch, and just
document the fact that -c is always provided to the invoked shell (if
there are additional arguments), and that the arguments are provided in a
concatenated form to the -c flag. I will also indicate that since this
command line will be interpreted by the shell, an additional level of
escape may be needed.


Do you agree with this?


To fix your issues, there will still be the solutions of:
  * escaping quotes
  * writing the script in a file, so that there is no argument

  * use su -- - "$LOGNAME" bash -x
    instead of su -- - "$LOGNAME" -x

PS: I'm BCC'ing you to your address without nospam, sorry if you receive
this mail twice)

Best Regards,
-- 
Nekral

Message #90 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Alexander Gattin <arg@online.com.ua>

To: 276419@bugs.debian.org

Cc: 276419-submitter@bugs.debian.org

Subject: Re: [Pkg-shadow-devel] Bug#276419: su appends the positional args to the command line

Date: Fri, 8 Jul 2005 23:48:03 +0300

Hi!

On Fri, Jul 08, 2005 at 03:03:32PM +0200, Nicolas François wrote:
> a patch was included in the 4.0.3-36 release, but this one breaks some
> scripts (see #317264).

Yes, but really those scripts are buggy, not new su.

> Thus, I'm considering to revert this patch and fix your bug by documenting
> the su behavior in its man page.

Please, don't do this, because it will make our su
different from the rest of Unix world.
(see e.g. http://www.freebsd.org/cgi/man.cgi?query=su or
http://docs.sun.com/app/docs/doc/816-5166/6mbb1kqhg?a=view)

> Here are some details on the issues introduced by the patch:
> 
>  * arguments are no more concatenated to provide only one string to the -c
>    option of the shell:

Right behaviour.

>  * -c is no more provided to the shell when it is not provided to su:

Again, absolutely correct behaviour.

>    This break invocation of su like:
>      $ su $LOGNAME bash

And what should this mean? `su luser bash -i`?
or `su luser bash -c`? ;)

> As the old behaviors are assumed by some packages (at least pbuilder,
> and probably others),

Those are buggy and just need to be fixes ASAP.

> I think it is better to revert the patch, and just
> document the fact that -c is always provided to the invoked shell

Return to broken state and explicitly document that
Debian has broken su? Please, don't do this!

> Do you agree with this?

I guess that Helmut will disagree.

-- 
WBR,
xrgtn

Message #98 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 276419@bugs.debian.org

Cc: 276419-submitter@bugs.debian.org

Subject: Re: Bug#276419: [Pkg-shadow-devel] Bug#276419: su appends the positional args to the command line

Date: Sat, 9 Jul 2005 06:55:47 +0200

> > I think it is better to revert the patch, and just
> > document the fact that -c is always provided to the invoked shell
> 
> Return to broken state and explicitly document that
> Debian has broken su? Please, don't do this!


Well, we need some decision here and here I come with my DD
hat.

I have followed the work on this bug and, first of all, I want to
thank you guys for the involved time and the neurones your burned on
that case...:-)

Now for the decision : it seems that the change, even if justified by
technical reasons, is very likely to break other packages and maybe as
well some system administrators scripts, badly written or not.

We cannot afford this.

Debian has a longstanding reputation of stability (YES, even unstable)
and reliability. Breaking things without warning is *not* the Debian
Way and this is exactly what we did with this change (of course, non
intentionnally).

So, we have to leave time to our fellow maintainers to fix their
scripts/packages/work/whatever. We also have to find the better way to
do so. I have some ideas but others may pop up in the future.

So, first of all, we REVERT THE PATCH. Don't take it badly, Alex,
nothing pro or against you but this is, I think, the less worse
decision.

The bug will be reopened, as a reminder, even if we fix su.

Then we communicate with other Debian developers so that scripts which
assume the "incorrect" behaviour are fixed. If this involves a
transition (ie fixes cannot be made unless we change su), we organise
it. Being the origin of the needed changes, this is our duty.

To achieve this, we need a document which explains the behaviour
changes. This document MUST be as simple as possible and clearly
explain the issues, even for people who are not very aware of su
internals.

All this (managing the change) will be made AFTER we merge with
upstream.


We will make an urgent upload with reveted su behaviour and Nicolas
documentation in the man page. This change will also include the FTBFS
fix for kFreeBSD and Hurd. I will work on it at Debconf if I can't
sleep..:-)

Message #106 received at 276419@bugs.debian.org (full text, mbox, reply):

From: "Helmut Waitzmann" (Debian Bug Tracking System) <Helmut.Waitzmann@web.de>

To: Nicolas FranXXois <nicolas.francois@centraliens.net>

Cc: 276419-quiet@bugs.debian.org, 276419@bugs.debian.org, 276419-submitter@bugs.debian.org

Subject: Re: Bug#276419: su appends the positional args to the command line

Date: Mon, 11 Jul 2005 17:44:01 +0200

Nicolas François <nicolas.francois@centraliens.net> writes:

>a patch was included in the 4.0.3-36 release, but this one breaks some
>scripts (see #317264).

which are broken and need to be fixed.  See below.

>Thus, I'm considering to revert this patch and fix your bug by documenting
>the su behavior in its man page.
>
>Here are some details on the issues introduced by the patch:
>
> * arguments are no more concatenated to provide only one string to the -c
>   option of the shell:
>   if some arguments are provided after the command provided by -c, these
>   arguments are provided to the shell that interprets the command, not to
>   the command itself.
>   For example:
>     $ bash -c 'echo $@' a b c
>   will print "b c"
>   but:
>     $ bash -c echo a b c
>   will only display an empty line

This behavior corresponds to the Debian manual page, the Linux Standard
Base and concurres with the behavior of Fedora Linux' and HPUX's su (and
if I remember correctly, solaris' su, too) and is correct behavior.

Passing additional arguments to su and banking on concatenation to the
command line is broken usage of su:  Neither Debian's nor Fedora's nor
HPUX's su's manual page nor the Linux Standard Base supports this.  The
rule is simple:  If you want su to pass a command line with concatenated
additional args, then simply concatenate them to the command line before
passing it to su.

> * -c is no more provided to the shell when it is not provided to su:
>   This break invocation of su like:
>     $ su $LOGNAME bash
>     /bin/bash: /bin/bash: cannot execute binary file

The same is true here.

Passing a command line to su without a preceding "-c" is broken usage of
su:  Neither Debian's nor Fedora's nor HPUX's su's manual page nor the
Linux Standard Base supports this.  The rule is simple:  If you want su to
provide a "-c" option to the shell, then simply provide it to su.

>As the old behaviors are assumed by some packages (at least pbuilder,
>and probably others), 

Those packages are broken and should be fixed:

1. If there is a command line to be passed, precede it with "-c".

2. If there are additional arguments to be concatenated to the command
   line, concatenate them to the command line.

Then those packages will work both with old and new su and there will be
no need to

>revert the patch, and just document the fact that -c is always provided
>to the invoked shell (if there are additional arguments), and that the
>arguments are provided in a concatenated form to the -c flag.

>Do you agree with this?

As it breaks the specifications of Linux Standard Base, I don't.  Please,
don't revert to a broken su.  That would make Debian's su different from
any other su in UNIX-like operating systems and restrict it to a very
small subset of invocations it will serve.

On the other hand, the invocation

$ su - someone -c 'for arg; do process "$arg"; done' *

, which meets the specifications, will fail with old su, as it will be
treated like

$ (set x *
   shift
   exec su - someone -c 'for arg; do process "$arg"; done '"$*"
   )

, which will not have the intended behavior as long as at least one
additional argument is supplied.  And there is no remedy, no matter how
many levels of escape are applied.

What do you think about creating a su-old-behavior package, which depends
on su (and will work with both old and new su) and contains a wrapper
executable (shell script, perl script or binary) called e.g. old-su, to
provide the old behavior of su using new (or old) su?

Then all fixing to be done to packages assuming the old behavior of su
would be to substitute all calls to su with calls to old-su and making a
dependency to the su-old-behavior package.

Do you think it is worth while doing that?

>To fix your issues, there will still be the solutions of:

>  * use su -- - "$LOGNAME" bash -x
>    instead of su -- - "$LOGNAME" -x

This is not a solution.  Using new su:

$ su -- - "$LOGNAME" -x

will start a login shell in debug mode, whereas using old su

$ su -- - "$LOGNAME" bash -x

will start a login shell which will start a bash in debug mode.  It will
not help me debugging my login shell, especially when I want to pass
positional arguments to an interactive login shell (to be used in
"$HOME"/.profile).

>PS: I'm BCC'ing you to your address without nospam, sorry if you receive
>this mail twice)

As long as 276419-submitter@bugs.debian.org is included in the list of
recipients or

Helmut Waitzmann (Debian Bug Tracking System) <Helmut.Waitzmann@web.de>

is included in the revealed list of recipients (i.e. BCC will not
work), I'll receive that mail.

Best regards,

Helmut
-- 
Wenn Sie mir E-Mail schreiben, stellen |  When writing me e-mail, please
Sie bitte vor meine E-Mail-Adresse     |  precede my e-mail address with
meinen Vor- und Nachnamen, etwa so:    |  my full name, like
Helmut Waitzmann <xxx@example.net>, (Helmut Waitzmann) xxx@example.net

Message #119 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: Helmut Waitzmann <Helmut.Waitzmann@web.de>, 276419@bugs.debian.org

Subject: Re: [Pkg-shadow-devel] Bug#276419: su appends the positional args to the command line

Date: Tue, 12 Jul 2005 10:19:34 +0200

> As it breaks the specifications of Linux Standard Base, I don't.  Please,
> don't revert to a broken su.  That would make Debian's su different from
> any other su in UNIX-like operating systems and restrict it to a very
> small subset of invocations it will serve.

Actually, we did revert to a broken su.

While seeing that several things were broken by the change, I decided
to go one step back and just take time for other people to be able to
fix thei packages.

So, the plan is have one of us writing a small document explaining the
issue, then post to -devel with it, then start a long flamewar, then
have the offending packages fixed to be able to work both ways if they
can....then upload a fixed shadow.

The summertime is probably not very well suited for this, so it may
take a few weeks to complete the process, anyhow.

> What do you think about creating a su-old-behavior package, which depends
> on su (and will work with both old and new su) and contains a wrapper
> executable (shell script, perl script or binary) called e.g. old-su, to
> provide the old behavior of su using new (or old) su?

Your suggestion sounds interesting as well. I will need to put the
"pbuilder and others" people attention to this and make them think
about this transition and the possibility you suggested to have a
transitory period of time. My only concern is that such temporary
package might sound as a bit overkill to solve this problem.

I'm opened and hearing from all of you, folks.

Message #126 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Martin Quinson <martin.quinson@loria.fr>

To: Helmut Waitzmann <Helmut.Waitzmann@web.de>, 276419@bugs.debian.org

Subject: Re: Bug#276419: [Pkg-shadow-devel] Bug#276419: su appends the positional args to the command line

Date: Fri, 22 Jul 2005 23:15:41 +0200

[Message part 1 (text/plain, inline)]

On Tue, Jul 12, 2005 at 10:19:34AM +0200, Christian Perrier wrote:
> 
> > As it breaks the specifications of Linux Standard Base, I don't.  Please,
> > don't revert to a broken su.  That would make Debian's su different from
> > any other su in UNIX-like operating systems and restrict it to a very
> > small subset of invocations it will serve.
> 
> Actually, we did revert to a broken su.

I think that this is a bad thing (TM). Hopefully you'll change your mind and
help debian getting rid of this long standing bug :)


I don't feel the need for overkilling mecanism here. I feel that Helmut
perfectly demonstrated at least twice why this bug has to be fixed. I
believe that the patches to packages relying on the old broken behaviour are
both quite rare (debian only) and quite simple to fix.

With sarge just released, it looks like a perfect time to go for this
transition. C++ ABI transition seems faaar much intrusive (and somehow
discutable, but it must be my anti-c++ religious position) to me than this
*fix*.

I'd say that we need to summarize the rationnal and how to adapt packages to
the new, fixed behaviour on, say, 100 lines, post it to
debian-devel-announce under the title "su behaviour transition" or so, wait
2 weeks, and upload a fixed version.

If you guys(*) agree, I'll try to do the summary and send it myself. I'm
still in vacations, but I'm really a geek, I'm affraid.

Bye, Mt.

(*): hehe. Forget about "les gars", it's just like "tutoyer".

[signature.asc (application/pgp-signature, inline)]

Message #131 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Nicolas François <nicolas.francois@centraliens.net>

To: Helmut Waitzmann <Helmut.Waitzmann@web.de>, 276419@bugs.debian.org

Subject: Re: Bug#276419: [Pkg-shadow-devel] Bug#276419: su appends the positional args to the command line

Date: Sun, 24 Jul 2005 22:21:19 +0200

Hi,

On Fri, Jul 22, 2005 at 11:15:41PM +0200, Martin Quinson wrote:
> I don't feel the need for overkilling mecanism here. I feel that Helmut
> perfectly demonstrated at least twice why this bug has to be fixed. I
> believe that the patches to packages relying on the old broken behaviour are
> both quite rare (debian only) and quite simple to fix.

Yes, both rare and in debian only scripts.
The biggest issue is to detect them (currently pbuilder and dchroot, maybe
another *chroot).


> I'd say that we need to summarize the rationnal and how to adapt packages to
> the new, fixed behaviour on, say, 100 lines, post it to
> debian-devel-announce under the title "su behaviour transition" or so, wait
> 2 weeks, and upload a fixed version.
> 
> If you guys(*) agree, I'll try to do the summary and send it myself. I'm
> still in vacations, but I'm really a geek, I'm affraid.

I do agree.


Here are some ideas:
(The "patch" here is the one that stop the concatenation of the arguments,
and also stop adding a -c)

It makes Debian's su more compatible with BSD, SUN, Gentoo, Redhat's ones.
(slackware or suze behave like Debian; shell-utils' su should be checked)
Some may argue that Debian's su also support some options others don't (so
the compatibility won't be complete anyway). Also FreeBSD su has a
completly different -c option.

The developer better knows how to quote than su (when we are concatenating
the arguments, we can mess the quotes).

Removing the -c when additional arguments are provided permits to add
options to the shell.

It makes the program compatible with its documentation (well, with the
patch revertion, the documentation has been changed to reflect the program
behavior).

-- 
Nekral, back to Lapland, and fighting with mosquitos

Message #136 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 276419@bugs.debian.org

Cc: Helmut Waitzmann <Helmut.Waitzmann@web.de>

Subject: Re: Bug#276419: [Pkg-shadow-devel] Bug#276419: su appends the positional args to the command line

Date: Mon, 25 Jul 2005 07:41:00 +0200

> > Actually, we did revert to a broken su.
> 
> I think that this is a bad thing (TM). Hopefully you'll change your mind and
> help debian getting rid of this long standing bug :)

Again and again and again, please read the whole rationale. After
doing the change, it rapidly turned out that it broke some important
packages.

Whether they are wrong or not is not relevant now: the consequence
was, as Junichi Uekawa pointed, that we broke packages such as
pbuilder without prior warning. We did not intend to do so of course
and, as we are nice to our fellow Debian developer, we don't want to
break their work without warning.

Moreover, unstable being in the middle of an important transition
(namely GCC 4.0 with ABI changes), this was absolutely NOT the moment
to break useful build tools.

> I don't feel the need for overkilling mecanism here. I feel that Helmut
> perfectly demonstrated at least twice why this bug has to be fixed. I

I DON'T NEED TO BE CONVINCED AGAIN.

OK, have I yelled enough now? :-)

But, pals, seeing everyone trying to "convince" someone who is already
convinced is kinda painful.

We will make this transition and, as I explained already, we will make
it barely the way you described.

What I fear a little is the discussion that is likely to follow in
-develwith people nitpicking the arguments we have for doing the
change.

*Here* I expect you, shadow maintainers AND Helmut, to help me in
explaining with the Right Arguments, why we have to change this. You
all know that I'm too technically challenged for doing so.

> With sarge just released, it looks like a perfect time to go for this
> transition. C++ ABI transition seems faaar much intrusive (and somehow
> discutable, but it must be my anti-c++ religious position) to me than this
> *fix*.
> 
> I'd say that we need to summarize the rationnal and how to adapt packages to
> the new, fixed behaviour on, say, 100 lines, post it to
> debian-devel-announce under the title "su behaviour transition" or so, wait
> 2 weeks, and upload a fixed version.


Sure. But not during August while many DD's are VAC (and not only
French ones). We should make the announcement ASAP. Idealistically
before I leave for holidays on Aug 1st for something like 3 weeks.

This announcement would mention September 15th as target release date
for the change, leaving people enough time to preventively adapt their
work.

The announcement must be as short as possible and be very clear bout
the consequence. Deep technical details and rationale should be left
to an appendix.

Message #141 received at 276419@bugs.debian.org (full text, mbox, reply):

From: "Helmut Waitzmann" (Debian Bug Tracking System) <Helmut.Waitzmann@web.de>

To: Christian Perrier <bubulle@debian.org>

Cc: 276419@bugs.debian.org

Subject: Re: Bug#276419: [Pkg-shadow-devel] Bug#276419: su appends the positional args to the command line

Date: Sun, 31 Jul 2005 04:35:39 +0200

[Message part 1 (text/plain, inline)]

Christian Perrier <bubulle@debian.org> writes:

>We will make this transition

[...]

>What I fear a little is the discussion that is likely to follow in
>-develwith people nitpicking the arguments we have for doing the
>change.
>
>*Here* I expect you, shadow maintainers AND Helmut, to help me in
>explaining with the Right Arguments, why we have to change this. 

Quoting the manual page:

   SU(1)

   NAME
	  su - Change user ID or become super-user

   SYNOPSIS
	  su [OPTS] [-] [username [ARGS]]


What are the pros and cons in doing the change to new su?

To be able to use arbitrary shell invocation options, e.g. start a login
shell in debug mode:

$ su -- - username -x

Start a login shell with additional positional parameters, which can be
examined in "$HOME"/.profile:

$ su -- - username -s XAUTHORITY="$XAUTHORITY"

These are impossible with old su.  So it turns out to be a PRO.



With old su,

$ su -- root cat /etc/shadow

works as expected.  With new su, it won't work.  To make it work with new
su, one has to concatenate the commandline by oneself and pass it as a
parameter to the '-c' option:

$ su -c cat\ /etc/shadow -- root

This is related to problems with proper commandline quoting that have
been reported, as can be seen for example in the posts to
<317264@bugs.debian.org> with message-ids
<874qb6mgdj.dancerj@netfort.gr.jp>,
<lflr7e5ny9h.fsf@helmutwaitzmann.news.arcor.de>,
<871x65gktx.dancerj@netfort.gr.jp>, and
<lflk6jvcjj2.fsf@helmutwaitzmann.news.arcor.de>.

Quoting from <lflk6jvcjj2.fsf@helmutwaitzmann.news.arcor.de>:

|Junichi Uekawa <dancer@netfort.gr.jp> writes:

|>> >I have concerns wrt shell quoting,
|>> 
|>> Could you please explain more detailed?
|>
|>The implications of needing quoting means that previous 
|>quoting conventions will need to change.
|>
|>Applications which used to pass quoted text to su, and 
|>needs to quote differently now need to change dramatically.

This looks like a CON.  But it is a misunderstanding:  With the
transition to new su there is no change in whether or how quoting has to
be done, because old su as well as new su does not do any quoting.

Some people are seduced to the wrong assumptions about the ARGS
processing of old su by two (undocumented) features of old su:  It
concatenates a commandline out of the ARGS, and, if neither the '-c' nor
the '--command' OPTS are supplied, it inserts a '-c' option implicitly.

Noticing the positional ARGS rather than a commandline they suppose that
su doesn't construct a commandline but simply invokes a command.

For example:

$ touch '/tmp/;rm -r /'
$ su -- root ls -- /tmp/\;*

is supposed to do a

execl("/bin/ls", "ls", "--", "/tmp/;rm -r /", NULL)

runtime library call rather than a

execl("/bin/sh", "sh", "-c", "ls -- /tmp/;rm -r /", NULL)

runtime library call.

So, they don't know, that su will just collect the ARGS and concatenate
them with spaces in between as a commandline for the shell's invocation
option '-c'.  Maybe, they don't even know that su will call a shell (to
let it evaluate the commandline) rather than invoking the command by
itself.

Therefore they don't recognize the need for proper quoting of the ARGS to
be concatenated into the commandline that is evaluated by the shell
invoked by the second runtime library call above.

And it is interesting:  The wrong assumptions about quoting are made with
old su rather than with new su, as authors are aware of the need of
quoting, when they have to concatenate the ARGS for themselves.


With new su, there is no easy way to invoke that fatal old su command
from the example above.

$ su root -c 'ls -- /tmp/\;*'

which crosses one's mind first, won't do any harm (with both old and new
su) but is not accurately the same:  The filename pattern is expanded by
root's shell rather than by the invokers shell.  Whereas (only with new su)

$ su -- root -c 'ls -- ${1+"$@"}' sh /tmp/\;*

would have the intended semantics of the original command

$ su -- root ls -- /tmp/\;*

:  The filename pattern is expanded by the invokers' shell and the
filenames are passed as positional parameters to the root shell and can
be accessed by the '${1+"$@"}' construct.  No problem remains with
quoting and the filename '/tmp/;rm -r', as it is not part of the
commandline, can't do any harm.  It will not be evaluated, just passed
unchanged to the argument list of 'ls'.  This turns out to be a PRO.


>You all know that I'm too technically challenged for doing so.
>
>> With sarge just released, it looks like a perfect time to go for this
>> transition. C++ ABI transition seems faaar much intrusive (and somehow
>> discutable, but it must be my anti-c++ religious position) to me than this
>> *fix*.
>> 
>> I'd say that we need to summarize the rationnal and how to adapt packages to
>> the new, fixed behaviour on, say, 100 lines, post it to
>> debian-devel-announce under the title "su behaviour transition" or so, wait
>> 2 weeks, and upload a fixed version.

This is a draft of a su invocation adaption howto:

[Message part 2 (text/plain, attachment)]

[Message part 3 (text/plain, inline)]

       
>Sure. But not during August while many DD's are VAC (and not only
>French ones). We should make the announcement ASAP. Idealistically
>before I leave for holidays on Aug 1st for something like 3 weeks.
>
>This announcement would mention September 15th as target release date
>for the change, leaving people enough time to preventively adapt their
>work.
>
>The announcement must be as short as possible and be very clear bout
>the consequence. 

Is the su adaption howto above too long?  Is it clear enough?

>Deep technical details and rationale should be left to an appendix.

Part of that appendix should be a RTFM to the manual page bash(1):
namely the invocation options '-c' and '-s', and the section about
parameter expansion.
-- 
Wenn Sie mir E-Mail schreiben, stellen |  When writing me e-mail, please
Sie bitte vor meine E-Mail-Adresse     |  precede my e-mail address with
meinen Vor- und Nachnamen, etwa so:    |  my full name, like
Helmut Waitzmann <xxx@example.net>, (Helmut Waitzmann) xxx@example.net

Message #146 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 276419@bugs.debian.org, 276419-submitter@bugs.debian.org

Subject: Is the famous "su appends the positional args to the command line" bug still here?

Date: Tue, 11 Oct 2005 19:03:44 +0200

This bug is tagged "fixed-upstream".

That means that it should no more be here in 4.0.12. Is that right?

The problem here becomes: such change has to go upstream. I can't
imagine we diverge from upstream on that matter...so we first need to
decide whether the bug is still here or not.

The first example given by Helmut in the bug report works as Helmut
expects...but it also works with 4.0.3-39...:-)

The 'su -- - "$LOGNAME" -x' example does not work with either 4.0.3 or
4.0.12.

So, it the bug still here (it seems to be)?

If it isn't anymore, just close the bug...:-)

If it is, then try convincing Tomasz he should adopt the patch(es) proposed

Then build the transition plan with Debian maintainers....


Now time for an aspirin to give some rest to my old brain....

Message #154 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 276419@bugs.debian.org

Subject: Re: [Pkg-shadow-devel] Bug#276419: Is the famous "su appends the positional args to the command line" bug still here?

Date: Wed, 12 Oct 2005 06:59:18 +0200

Rumble....Please answer to the bug address, not the list
address....Otherwise discussions can't be followed in the BTS when
coming back later on bugs.

This is why I entirely quote your message.

Quoting Nicolas François (nicolas.francois@centraliens.net):
> Hi,
> 
> On Tue, Oct 11, 2005 at 07:03:44PM +0200, bubulle@debian.org wrote:
> > This bug is tagged "fixed-upstream".
> > 
> > That means that it should no more be here in 4.0.12. Is that right?
> > 
> > The problem here becomes: such change has to go upstream. I can't
> > imagine we diverge from upstream on that matter...so we first need to
> > decide whether the bug is still here or not.
> > 
> > The first example given by Helmut in the bug report works as Helmut
> > expects...but it also works with 4.0.3-39...:-)
> > 
> > The 'su -- - "$LOGNAME" -x' example does not work with either 4.0.3 or
> > 4.0.12.
> > 
> > So, it the bug still here (it seems to be)?
> > 
> > If it isn't anymore, just close the bug...:-)
> > 
> > If it is, then try convincing Tomasz he should adopt the patch(es) proposed
> > 
> > Then build the transition plan with Debian maintainers....
> 
> This bug should not be fixed currently.
> 
> It is fixed upstream because the submitter thinks upstream behavior is
> correct, whereas Debian's su is not correct.
> 
> We tried to fix it, but this broke pbuilder (remember Junichi blogs?;)
> 
> The plan was to make a summary of the situation to debian-devel, and chose
> whether 423_su_pass_args_without_concatenation should be applied or not.
> This may also mean a transition.
> 
> I have no real opinion:
>  * I prefer upstream behavior
>  * upstream's behavior is also used in other distrib / *nix
>  * upstream behavior breaks some important packages that currently depend
>    on Debian's su behavior.
> 


I was wondering "If the bug is fixed upstream, then by which miracle
is Debian's su behaviour different ? :-)"

We now use upstream.

Then I discovered the two 423 patches....

 enforces the "old" behaviour and is applied

So, now, we enforce the "old" behaviour with
423_su_arguments_are_concatenated to our su while we should actually
just use upstream's behaviour by dropping this patch. Right ?

 423_su_pass_args_without_concatenation was meant for 4.0.3 but should
 actually be useless. Going to the new behaviour is just dropping the
 423_su_arguments_are_concatenated patch. OK?



Helmut wrote a long and detailed HOWTO to help users, admins and
maintainers to adapt their use of su. Fine.


However, putting myself in the skin of a quite clueless maintainer,
the first thought that comes would be "hey, I use "su" in my
package...am I concerned by what these guys are changing"?

So, we need a VERY SHORT, NON TECHNICAL document explaining this.

I think it needs to have something like this structure, with no more
than 3 lines per part:

Introduction
------------
Briefly explain the change and the rationale, pointing at the bug
number

Short details
-------------
Explain, with examples, the new su use cases that need to be adapted.

Needed adaptations
------------------
Point people affected by this changes to the HOWTO (which will be
released along with shadow ASAP, possibly with 4.0.13).

Transition plan
---------------
Explain when the new behaviour will be implemented, giving maintainers
a deadline.

Message #161 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Nicolas François <nicolas.francois@centraliens.net>

To: debian-devel@lists.debian.org

Cc: pbuilder@packages.debian.org, dchroot@packages.debian.org, 276419-submitter@bugs.debian.org, 276419@bugs.debian.org

Subject: Shall Debian's su conform to other implementations

Date: Sun, 6 Nov 2005 23:14:39 +0100

Hi,

In #276419, the bug submitter complained that when a command and some
arguments were passed to su, all these arguments were concatenated, and
provided to the shell -c option.

This behavior differs from su on other systems [0].
This also forbid to pass arguments to the shell [1].

As these behaviors where not documented in the man page, in the code or in
the changelog, we uploded 4.0.3-36 to fix this bug.

Unfortunately, this broke pbuilder (see #317264), and other Debian
packages (e.g. dchroot). So this patch was (at least temporarily) removed,
and the current behavior documented.


We would now like to get rid of this bug. What do you recommend:
 * keep a Debian specific implementation and tag this bug wontfix
 * reapply the patch to fix this bug, and report bugs on the packages that
   uses this "feature"



[0] On other systems su's -c arguments must be quoted:
        su - -c "ls -l /tmp"
    and:
        su -- - "$LOGNAME" ls -l /tmp
    probably only works on Debian.

[1] For example:
        su -- - "$LOGNAME" -x
    Could exec("/bin/sh", ["-x"])

Kind Regards,
-- 
Nekral

Message #169 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Junichi Uekawa <dancer@netfort.gr.jp>

To: debian-devel@lists.debian.org, pbuilder@packages.debian.org, dchroot@packages.debian.org, 276419-submitter@bugs.debian.org, 276419@bugs.debian.org

Subject: Re: [Pbuilder-maint] Shall Debian's su conform to other implementations

Date: Fri, 11 Nov 2005 08:19:25 +0900

Hi,

> Unfortunately, this broke pbuilder (see #317264), and other Debian
> packages (e.g. dchroot). So this patch was (at least temporarily) removed,
> and the current behavior documented.
> 
> 
> We would now like to get rid of this bug. What do you recommend:
>  * keep a Debian specific implementation and tag this bug wontfix
>  * reapply the patch to fix this bug, and report bugs on the packages that
>    uses this "feature"

Could you document and wait until etch release?


regards,
	junichi

Message #177 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: Junichi Uekawa <dancer@netfort.gr.jp>, 276419@bugs.debian.org

Cc: debian-devel@lists.debian.org, pbuilder@packages.debian.org, dchroot@packages.debian.org, 276419-submitter@bugs.debian.org

Subject: Re: [Pkg-shadow-devel] Bug#276419: [Pbuilder-maint] Shall Debian's su conform to other implementations

Date: Fri, 11 Nov 2005 08:02:27 +0100

Quoting Junichi Uekawa (dancer@netfort.gr.jp):

> > We would now like to get rid of this bug. What do you recommend:
> >  * keep a Debian specific implementation and tag this bug wontfix
> >  * reapply the patch to fix this bug, and report bugs on the packages that
> >    uses this "feature"
> 
> Could you document and wait until etch release?


Etch release?

We already delayed this for sarge release.....then tried to fix it
(badly as you know). I don't want bug reports rotting in the BTS. I
have no idea of the shadow devel team healt in more than 1 year and I
prefer we fixed as many bugs as possible while we can.

Are these changes *that* invasive for pbuilder?

Message #185 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Junichi Uekawa <dancer@netfort.gr.jp>

To: Christian Perrier <bubulle@debian.org>

Cc: Junichi Uekawa <dancer@netfort.gr.jp>, 276419@bugs.debian.org, debian-devel@lists.debian.org, pbuilder@packages.debian.org, dchroot@packages.debian.org, 276419-submitter@bugs.debian.org

Subject: Re: [Pkg-shadow-devel] Bug#276419: [Pbuilder-maint] Shall Debian's su conform to other implementations

Date: Fri, 11 Nov 2005 22:09:09 +0900

Hi,

> > > We would now like to get rid of this bug. What do you recommend:
> > >  * keep a Debian specific implementation and tag this bug wontfix
> > >  * reapply the patch to fix this bug, and report bugs on the packages that
> > >    uses this "feature"
> > 
> > Could you document and wait until etch release?
> 
> 
> Etch release?
> 
> We already delayed this for sarge release.....then tried to fix it
> (badly as you know). I don't want bug reports rotting in the BTS. I
> have no idea of the shadow devel team healt in more than 1 year and I
> prefer we fixed as many bugs as possible while we can.
> 
> Are these changes *that* invasive for pbuilder?

The ideal way to approach this is to announce a change, 
document that change, provide some environmental variable
support for that change (like POSIXLY_CORRECT)

Then change.

We've got quite a bit of tools in sarge that doesn't work with
this change, right?



regards,
	junichi
-- 
dancer@{debian.org,netfort.gr.jp}   Debian Project

Message #193 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: Junichi Uekawa <dancer@netfort.gr.jp>, 276419@bugs.debian.org

Cc: 276419-submitter@bugs.debian.org, debian-devel@lists.debian.org, dchroot@packages.debian.org, pbuilder@packages.debian.org

Subject: Re: Bug#276419: [Pkg-shadow-devel] Bug#276419: [Pbuilder-maint] Shall Debian's su conform to other implementations

Date: Fri, 11 Nov 2005 14:52:05 +0100

> The ideal way to approach this is to announce a change, 

Which is what we are doing now. We neglected to do so the first time
(mostly because we didn't anticipate this would break pbuilder so
much)  and this is why we reverted the change very quickly.

> document that change, provide some environmental variable

It will be documented. We have a ready document, written by the
original bug reporter, which documents the changes and gives
recommendations for fixing affected usage of su.

We didn't want to send it now because it's a long and deeply technical
document and we first wanted to get input from people, like you, who
are already aware of the issue.

> support for that change (like POSIXLY_CORRECT)
> 
> Then change.
> 
> We've got quite a bit of tools in sarge that doesn't work with
> this change, right?


From what Nicolas investigated, not that much. He only found dchroot
and pbuilder up to now. Nicolas does not pretend to have a complete
investigation but I trust him when he mentions he tried as widely as
possible.

Message #201 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Junichi Uekawa <dancer@netfort.gr.jp>

To: Christian Perrier <bubulle@debian.org>

Cc: Junichi Uekawa <dancer@netfort.gr.jp>, 276419@bugs.debian.org, 276419-submitter@bugs.debian.org, debian-devel@lists.debian.org, dchroot@packages.debian.org, pbuilder@packages.debian.org

Subject: Re: Bug#276419: [Pkg-shadow-devel] Bug#276419: [Pbuilder-maint] Shall Debian's su conform to other implementations

Date: Fri, 11 Nov 2005 23:07:17 +0900

Hi,

> > support for that change (like POSIXLY_CORRECT)
> > 
> > Then change.
> > 
> > We've got quite a bit of tools in sarge that doesn't work with
> > this change, right?
> 
> 
> >From what Nicolas investigated, not that much. He only found dchroot
> and pbuilder up to now. Nicolas does not pretend to have a complete
> investigation but I trust him when he mentions he tried as widely as
> possible.

FWIW, pbuilder in sid is fixed since 0.129 (17 August 2005),
and I am hoping that will
probagate into some stable backports, so that practically,
pbuilder side is  ready for the new su.


regards,
	junichi

Message #209 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 276419@bugs.debian.org

Cc: 276419-submitter@bugs.debian.org, debian-devel@lists.debian.org, dchroot@packages.debian.org

Subject: Re: Bug#276419: [Pkg-shadow-devel] Bug#276419: [Pbuilder-maint] Shall Debian's su conform to other implementations

Date: Fri, 11 Nov 2005 16:00:42 +0100

> FWIW, pbuilder in sid is fixed since 0.129 (17 August 2005),
> and I am hoping that will
> probagate into some stable backports, so that practically,
> pbuilder side is  ready for the new su.


Well, this is actually great news as pbuilder was by far the main
blocker for this change. Sorry for having bugged you, then.

(removing Junichi and the pbuilder list from CC)

Message #217 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Nicolas François <nicolas.francois@centraliens.net>

To: debian-devel@lists.debian.org

Cc: 276419@bugs.debian.org

Subject: Advices for an su transition

Date: Sun, 20 Nov 2005 23:26:20 +0100

Hello,

On Sun, Nov 06, 2005 at 11:14:39PM +0100, Nicolas François wrote:
> 
> We would now like to get rid of this bug. What do you recommend:
>  * keep a Debian specific implementation and tag this bug wontfix
>  * reapply the patch to fix this bug, and report bugs on the packages that
>    uses this "feature"

I've not seen a strong opposition to the second choice.


We now need some advices to perform this transition.

Junichi proposed to keep the current behavior when an environment variable
is set (I would prefer something different from POSIXLY_CORRECT). The
support for this environment variable could then be dropped after Etch.

The change will have to be announced (here and in NEWS) and documented.
(Basically, -c's argument is a command executed in the shell, other
arguments given after '--' are provided to the shell, not to the command)


Another step is to find which packages won't work with this change, get
them fixed and upload a new login which conflicts with their previous
versions.

I think such softwares won't run with upstream's su (i.e. at least Redhat
and Gentoo), GNU's su (coreutils), FreeBSD's su (and OpenSolaris' su).
So I hope only Debian specific packages or Debian maintainers' scripts
will be affected.

At this time we are aware of:
 * pbuilder (not true anymore)
 * dchroot
   dchroot synopsis is: dchroot [OPTION...] [COMMAND]
   it passes its arguments to su. -c is not provided.
   COMMAND needs to be a single argument.

There are no dependencies on login, so I don't see anything else than a
grep on the 2 letters "su" (\<su\> may help).

IIRC people from debian-audit have some tools to perform such grep on the
source package with some heuristics to extract and patch the sources
(dpatch, cdbs, ...), and ignore the documentation files (e.g. "su" is a
common word in spanish).
I have no idea of the size of such grep output.


Does anybody have another idea?

Kind Regards,
-- 
Nekral

Message #222 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Bill Allombert <ballombe@master.debian.org>

To: debian-devel@lists.debian.org

Cc: 276419@bugs.debian.org

Subject: Re: Advices for an su transition

Date: Sun, 20 Nov 2005 17:44:46 -0600

On Sun, Nov 20, 2005 at 11:26:20PM +0100, Nicolas Fran?ois wrote:
> IIRC people from debian-audit have some tools to perform such grep on the
> source package with some heuristics to extract and patch the sources
> (dpatch, cdbs, ...), and ignore the documentation files (e.g. "su" is a
> common word in spanish).
> I have no idea of the size of such grep output.
> 
> 
> Does anybody have another idea?

Yes, you must absolutly check all the maintainer scripts (preinst,etc)
shipped in sarge and preferably also all postrm in woody, else you take
the risk of breaking sarge to etch upgrade.

I can do it for you if you want.

Cheers,
-- 
Bill. <ballombe@debian.org>

Imagine a large red swirl here.

Message #227 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Bill Allombert <ballombe@master.debian.org>

To: debian-devel@lists.debian.org

Cc: 276419@bugs.debian.org

Subject: Re: Advices for an su transition

Date: Sun, 20 Nov 2005 18:22:16 -0600

On Sun, Nov 20, 2005 at 05:44:46PM -0600, Bill Allombert wrote:
> On Sun, Nov 20, 2005 at 11:26:20PM +0100, Nicolas Fran?ois wrote:
> > IIRC people from debian-audit have some tools to perform such grep on the
> > source package with some heuristics to extract and patch the sources
> > (dpatch, cdbs, ...), and ignore the documentation files (e.g. "su" is a
> > common word in spanish).
> > I have no idea of the size of such grep output.
> > 
> > 
> > Does anybody have another idea?
> 
> Yes, you must absolutly check all the maintainer scripts (preinst,etc)
> shipped in sarge and preferably also all postrm in woody, else you take
> the risk of breaking sarge to etch upgrade.
> 
> I can do it for you if you want.

Actually looks here: <http://merkel.debian.org/~ballombe/>

The full data is available on merkel.debian.org in ~ballombe/menu/supackages.

Cheers,
-- 
Bill. <ballombe@debian.org>

Imagine a large red swirl here.

Message #232 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Nicolas François <nicolas.francois@centraliens.net>

To: debian-devel@lists.debian.org

Cc: Bill Allombert <ballombe@debian.org>, 276419@bugs.debian.org

Subject: Re: Advices for an su transition

Date: Mon, 21 Nov 2005 23:19:54 +0100

On Sun, Nov 20, 2005 at 06:22:16PM -0600, Bill Allombert wrote:
> 
> Actually looks here: <http://merkel.debian.org/~ballombe/>
> 
> The full data is available on merkel.debian.org in ~ballombe/menu/supackages.

Thanks a lot!

At least all these maintainer scripts seems OK.

I can probably check all the native packages now.

Best Regards,
-- 
Nekral

Message #237 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 276419@bugs.debian.org

Subject: Do we fix this in 4.0.14-4?

Date: Thu, 26 Jan 2006 07:44:37 +0100

The upload of 4.0.14-4 will happen soon.

We have the opportunity of fixing this bug now. From Nicolas survey,
it does not seem likely to harm critical things (inclusing sarge to
etch upgrades, see thread back in November), so the only actions
(besides stopping to apply the 423 patch) are communication:

-publish an announcement in d-d
-include a technical document in /usr/share/doc/passwd

There was a proposal for POSIXLY_CORRECT variable....I'm not deeply
convinced to add this complexity unless it proves really useful
(backword compatibility...but with what?).

If the patch that adds this is ready then we can go for it. If it
isn't and you feel it's needed, then convince me..:)




-- 

Message #242 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 276419@bugs.debian.org

Subject: Re: [Pkg-shadow-devel] Bug#276419: Do we fix this in 4.0.14-4?

Date: Thu, 26 Jan 2006 18:01:38 +0100

> > -publish an announcement in d-d
> > -include a technical document in /usr/share/doc/passwd
> 
> I think it is still too early to call it a survey.
> I need more time to check:
>  * the Debian native packages
>  * the packages that uses cron
>  * the packages that install an init.d
>  * the maintainer scripts
>    (based on the selction of Bill Allombert)
> 
> I've started the 3 first points.

OK. "need more time" noticed. So, this will obviously not be for this
release.

> > If the patch that adds this is ready then we can go for it. If it
> > isn't and you feel it's needed, then convince me..:)
> 
> I'm not sure, but "the patch that adds this" is probably just the removal
> of a debian patch.

Yes. But if we need a variable to revert the behaviour to the "old
behaviour" then it needs to be implemented somewhere, no?



Anyway, bug solution delayed....maybe some day, this bug will be the
last one...:)

Message #247 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Nicolas François <nicolas.francois@centraliens.net>

To: 276419@bugs.debian.org

Subject: Re: Bug#276419: transition anouncement proposal

Date: Sat, 28 Jan 2006 23:31:39 +0100

Hello,

Here is a proposal for the transition announcement.

We need to find if the transitionned login must conflict with all the
packages which are not following the new synopsis.
(There was a discussion about it on debian-devel, but IIRC no strong
point)

I will make a patch to enable swicthing to the old behavior with an
environment variable (SU_NO_SHELL_ARGS).


==========================================================================

Introduction
============
As reported in #276419, shadow's su [1] doesn't permit to specify options
to the invoked shell and doesn't respect quoted arguments.
We plan to revert this behavior and follow su's documentation and other
implementations.


Short details
=============
Packages passing a command in argument to su must use su's -c option
and must quote the command if it contains a space.
For example:
  su - root -c "ls -l /"

The following commands won't work anymore:
  su - root -c ls -l /
  su - root "ls -l /"
  su - root ls -l /

There will be no problems for backports. -c can be used and arguments
quoted, with the past and future versions.

Needed adaptations
==================
We tried to find the packages that will be affected by this transition.
We did not audit the full archive, but focused on [2]:
 * maintainer scripts
 * packages with an init.d script (based on a sid Contents-i386)
 * packages with an cron script (based on a sid Contents-i386)
 * native packages (on sid i386)
(In general, archives embedded in source packages were not chacked)

Package needing changes
-----------------------
amavisd-new-2.3.3/debian/amavisd-new.cron.daily
backupninja-0.9.2/handlers/pgsql
backupninja-0.9.2/handlers/mysql
backupninja-0.9.2/examples/example.rdiff
echolot-2.1.8/debian/echolot.init
gnunet-0.7.0b/contrib/init_gnunet_ubuntu
yiff-2.14.2/build_and_install
python-4suite-0.99cvs20051115/debian/python-4suite-server.init.d
samhain-2.0.10a/init/samhain.start.in

To be checked
-------------
debget-1.5/debget

maybe
-----
cedar-backup2-2.7.2/CedarBackup2/peer.py (depends on executeCommand)
nut-2.0.2/scripts/HP-UX/nut-drvctl.sh (maybe not used on Debian)
usermin-1.160/cron/config-aix (maybe not used on Debian)
courier-0.52.1/courier.lpspec(.in)? (maybe not used on Debian)
courier-0.52.1/courier.spec(.in)? (maybe not used on Debian)
murasaki-0.8.11/scripts/printer (su $USER -c $CMD, $CMD may have a space)
nut-2.0.2/scripts/HP-UX/nut-drvctl.sh (maybe not used on Debian)
nut-2.0.2/scripts/HP-UX/nut-upsd.sh (maybe not used on Debian)
usermin-1.160/web-lib-funcs.pl
usermin-1.160/shell/index.cgi
usermin-1.160/fetchmail/check.pl
usermin-1.160/commands/run.cgi
usermin-1.160/postgresql/postgresql-lib.pl
webmin-1.230/web-lib-funcs.pl
webmin-1.230/cron/config-aix
webmin-1.230/custom/run.cgi

In comments or documentation
----------------------------
cyrus21-imapd-2.1.18/debian/cyrus21-common.postinst
lprng-3.8.28/DOCS/LPRng-Reference.html
lprng-3.8.28/DOCS/LPRng-Reference.sgml
lprng-3.8.28/DOCS/LPRng-Reference-Multipart/x9198.htm
remstats-1.0.13a/INSTALL
remstats-1.0.13a/docs/book.tex (and other formats)
remstats-1.0.13a/docs/install-user.pod
remstats-1.0.13a/docs/install.pod
remstats-1.0.13a/docs/install.txt
bricolage-1.8.8/bin/bric_ftpd
dhis-client-5.3/README
kdenetwork-3.5.0/kopete/protocols/meanwhile/README
pop-before-smtp-1.36/contrib/README.rootless-install
sqlrelay-0.36.4/doc/gettingstarted/interbase.html
debpool-0.2.2/debian/README.User

Transition plan
===============
Date?

environment variable to restore the previous behavior

conflict with the above packages?


Other recommandation
====================
You should follow the following synopsis for your su commands.
(This will give you more chance to be portable and to work on
POSIXLY_CORRECT environments)

    su [options] [-] [username [args]]

[args] are arguments passed to the shell

Specifically:
 * It is preferable to provide -c in [args] rather than in [options].
 * su - root -p doesn't work on if the POSIXLY_CORRECT environment
   variable is set.

axyl-2.1.9/db/postgres/install-db.sh
dwww-1.9.26/dwww-format-man
findutils-4.2.26/locate/updatedb.sh
interchange-5.3.2/debian/interchange.cron.daily
interchange-5.3.2/scripts/restart.PL
popularity-contest-1.31/debian/cron.weekly
popularity-contest-1.31/FAQ
powersave-0.9.25/scripts/wm_shutdown
powersave-0.9.25/scripts/do_screen_saver
powersave-0.9.25/scripts/wm_logout
powersave-0.9.25/scripts/x_helper_functions
usermin-1.160/web-lib-funcs.pl
usermin-1.160/commands/run.cgi
webmin: ditto



[1] The su used on Debian, unless for The Hurd (which uses coreutils's su).

[2] The rational is that we consider there is a greater chance to find
    problems on debian specific packages/scripts since it would have fail
    on other OS (on RedHat, Gentoo, Mandriva, SunOS).
    Probably 10% of the archive was audited.

[3] Thanks to Bill Allombert 
    http://lists.debian.org/debian-devel/2005/11/msg01215.html

==========================================================================

We can make it smaller by removing the "Other recommandation" and
"In comments or documentation" sections.

The "To be checked" and "maybe" sections need to be checked (or maybe we
can keep them like that and let the maintainers check).

Kind Regards,
-- 
Nekral

Message #252 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: Nicolas François <nicolas.francois@centraliens.net>, 276419@bugs.debian.org

Subject: Re: [Pkg-shadow-devel] Bug#276419: transition anouncement proposal

Date: Mon, 27 Feb 2006 07:17:02 +0100

Quoting Nicolas François (nicolas.francois@centraliens.net):
> Hello,
> 
> Here is a proposal for the transition announcement.

No mor echanges proposed so this is the final version.

> 
> We need to find if the transitionned login must conflict with all the
> packages which are not following the new synopsis.
> (There was a discussion about it on debian-devel, but IIRC no strong
> point)

I would say "only those that use the new synopsis in their maintainer scripts"

> 
> I will make a patch to enable swicthing to the old behavior with an
> environment variable (SU_NO_SHELL_ARGS).


That one is still to happen.

I'm urging this a little as we have to know that we're not *that* far
from etch release...so we really must work on fixing 276419 right
now....

Message #257 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Nicolas François <nicolas.francois@centraliens.net>

To: debian-devel@lists.debian.org

Cc: 276419@bugs.debian.org

Subject: Announcing changes in su

Date: Sun, 5 Mar 2006 02:06:45 +0100

Hello,

Introduction
============
As reported in #276419, su in the login Debian package doesn't permit to
specify options to the invoked shell and doesn't respect quoted arguments.
We plan to revert this behavior and follow su's documentation and other
implementations.


Short details
=============
Packages passing a command in argument to su must use su's -c option
and must quote the command if it contains a space.
For example:
  su - root -c "ls -l /"

The following commands won't work anymore:
  su - root -c ls -l /
  su - root "ls -l /"
  su - root ls -l /

There will be no problems for backports. -c can be used and arguments
quoted, with the past and future versions.

Needed adaptations
==================
We tried to find the packages that will be affected by this transition.
We did not audit the full archive, but focused on [1]:
 * maintainer scripts [2]
 * packages with an init.d script (based on a sid Contents-i386)
 * packages with an cron script (based on a sid Contents-i386)
 * native packages (on sid i386)
(In general, archives embedded in source packages were not checked)

Package needing changes
-----------------------
Micah Anderson <micah@riseup.net>
        backupninja-0.9.2/handlers/pgsql
        backupninja-0.9.2/handlers/mysql
        backupninja-0.9.2/examples/example.rdiff
Raphael Bossek <bossekr@debian.org>
        python-4suite-0.99cvs20051115/debian/python-4suite-server.init.d
Phil Brooke <pjb@debian.org>
        yiff-2.14.2/build_and_install
Arnaud Kyheng <Arnaud.Kyheng@free.fr>
        gnunet-0.7.0b/contrib/init_gnunet_ubuntu
Brian May <bam@debian.org>
        amavisd-new-2.3.3/debian/amavisd-new.cron.daily
Peter Palfrader <weasel@debian.org>
        echolot-2.1.8/debian/echolot.init
Javier Fernandez-Sanguino Pen~a <jfs@computer.org>
        samhain-2.0.10a/init/samhain.start.in

To be checked
-------------
Roderick Schertler <roderick@argon.org>
        debget-1.5/debget
(It should be OK. According to the code, it works with GNU su)

maybe
-----
Stefan Hornburg (Racke) <racke@linuxia.de>
        courier-0.52.1/courier.lpspec(.in)? (maybe not used on Debian)
        courier-0.52.1/courier.spec(.in)? (maybe not used on Debian)
Kenneth J. Pronovici <pronovic@debian.org>
        cedar-backup2-2.7.2/CedarBackup2/peer.py (depends on executeCommand)
Arnaud Quette <aquette@debian.org>
        nut-2.0.2/scripts/HP-UX/nut-drvctl.sh (maybe not used on Debian)
        nut-2.0.2/scripts/HP-UX/nut-upsd.sh (maybe not used on Debian)
Taku YASUI <tach@debian.or.jp>
        murasaki-0.8.11/scripts/printer (su $USER -c $CMD, $CMD may have a space)
Debian Webmin maintainers <webmin-maintainers@lists.alioth.debian.org>
        usermin-1.160/cron/config-aix (maybe not used on Debian)
        usermin-1.160/web-lib-funcs.pl
        usermin-1.160/shell/index.cgi
        usermin-1.160/fetchmail/check.pl
        usermin-1.160/commands/run.cgi
        usermin-1.160/postgresql/postgresql-lib.pl
        webmin-1.230/web-lib-funcs.pl
        webmin-1.230/cron/config-aix
        webmin-1.230/custom/run.cgi

In comments or documentation
----------------------------
Clint Adams <schizo@debian.org>
        bricolage-1.8.8/bin/bric_ftpd
Joel Aelwyn <fenton@debian.org>
        debpool-0.2.2/debian/README.User
Debian Qt/KDE Maintainers <debian-qt-kde@lists.debian.org>
        kdenetwork-3.5.0/kopete/protocols/meanwhile/README
Henrique de Moraes Holschuh <hmh@debian.org>
        cyrus21-imapd-2.1.18/debian/cyrus21-common.postinst
Robert Jordens <jordens@debian.org>
        remstats-1.0.13a/INSTALL
        remstats-1.0.13a/docs/book.tex (and other formats)
        remstats-1.0.13a/docs/install-user.pod
        remstats-1.0.13a/docs/install.pod
        remstats-1.0.13a/docs/install.txt
Matthias Klose <doko@debian.org>
        sqlrelay-0.36.4/doc/gettingstarted/interbase.html
Guus Sliepen <guus@debian.org>
        dhis-client-5.3/README
Craig Small <csmall@debian.org>
        lprng-3.8.28/DOCS/LPRng-Reference.html
        lprng-3.8.28/DOCS/LPRng-Reference.sgml
        lprng-3.8.28/DOCS/LPRng-Reference-Multipart/x9198.htm
Jonas Smedegaard <dr@jones.dk>
        pop-before-smtp-1.36/contrib/README.rootless-install

Transition plan
===============
A package will be first available for testing on experimental.
If you know that your package uses su, it would be nice if you could test
it with the login package (which will be uploaded) on experimental.

The SU_NO_SHELL_ARGS environment variable will restore the previous
behavior. The support for this variable should be dropped after Etch.

login will conflict with the package of the first category. When fixed,
these packages do not need a versionned dependency on login.


Recommandation
==============
You should follow the following synopsis for your su commands.
(This will give you more chance to be portable and to work on
POSIXLY_CORRECT environments)

    su [options] [-] [username [args]]

[args] are arguments passed to the shell

Specifically:
 * It is preferable to provide -c in [args] rather than in [options].
 * su - root -p doesn't work if the POSIXLY_CORRECT environment
   variable is set.

The following packages don't follow these rules:
Stefan Hornburg (Racke) <racke@linuxia.de>
        interchange-5.3.2/debian/interchange.cron.daily
        interchange-5.3.2/scripts/restart.PL
Michael Biebl <biebl@teco.edu>
        powersave-0.9.25/scripts/wm_shutdown
        powersave-0.9.25/scripts/do_screen_saver
        powersave-0.9.25/scripts/wm_logout
        powersave-0.9.25/scripts/x_helper_functions
Popularity Contest Developers <popcon-developers@lists.alioth.debian.org>
        popularity-contest-1.31/debian/cron.weekly
        popularity-contest-1.31/FAQ
Robert Luberda <robert@debian.org>
        dwww-1.9.26/dwww-format-man
Andreas Metzler <ametzler@debian.org>
        findutils-4.2.26/locate/updatedb.sh
Paul Waite <paul@catalyst.net.nz>
        axyl-2.1.9/db/postgres/install-db.sh
Debian Webmin maintainers <webmin-maintainers@lists.alioth.debian.org>
        usermin-1.160/web-lib-funcs.pl
        usermin-1.160/commands/run.cgi
        webmin: ditto



[1] The rationale is that we consider there is a greater chance to find
    problems on Debian specific packages/scripts since it would have fail
    on other OS (on RedHat, Gentoo, Mandriva, SunOS).
    Probably 10% of the archive was audited.

[2] Thanks to Bill Allombert 
    http://lists.debian.org/debian-devel/2005/11/msg01215.html

Kind Regards,
-- 
Nekral

Message #262 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Javier Fernández-Sanguino Peña <jfs@computer.org>

To: debian-devel@lists.debian.org, 276419@bugs.debian.org

Subject: Re: Announcing changes in su

Date: Sun, 5 Mar 2006 02:35:34 +0100

[Message part 1 (text/plain, inline)]

On Sun, Mar 05, 2006 at 02:06:45AM +0100, Nicolas François wrote:
> Javier Fernandez-Sanguino Pen~a <jfs@computer.org>
>         samhain-2.0.10a/init/samhain.start.in

That (upstream) code is not used in the Debian package (the init script used
is  samhain-2.0.10a/debian/samhain.init

Regards

Javier

[signature.asc (application/pgp-signature, inline)]

Message #269 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 276419@bugs.debian.org

Subject: Re: [Pkg-shadow-devel] Bug#276419: Announcing changes in su

Date: Fri, 24 Mar 2006 18:42:48 +0100

[Message part 1 (text/plain, inline)]

> Introduction
> ============
> As reported in #276419, su in the login Debian package doesn't permit to
> specify options to the invoked shell and doesn't respect quoted arguments.
> We plan to revert this behavior and follow su's documentation and other
> implementations.

This is now time to prepare for 4.0.15-2 in unstable, with the
fix.....

So, we will need to warn again maintainers of these packages, in
addition of sending another notice in -devel-announce.


> Package needing changes
> -----------------------
> Micah Anderson <micah@riseup.net>
>         backupninja-0.9.2/handlers/pgsql
>         backupninja-0.9.2/handlers/mysql
>         backupninja-0.9.2/examples/example.rdiff
> Raphael Bossek <bossekr@debian.org>
>         python-4suite-0.99cvs20051115/debian/python-4suite-server.init.d
> Phil Brooke <pjb@debian.org>
>         yiff-2.14.2/build_and_install
> Arnaud Kyheng <Arnaud.Kyheng@free.fr>
>         gnunet-0.7.0b/contrib/init_gnunet_ubuntu
> Brian May <bam@debian.org>
>         amavisd-new-2.3.3/debian/amavisd-new.cron.daily
> Peter Palfrader <weasel@debian.org>
>         echolot-2.1.8/debian/echolot.init



> Javier Fernandez-Sanguino Pen~a <jfs@computer.org>
>         samhain-2.0.10a/init/samhain.start.in

Javier confirmed that samhain is not affected.

> maybe
> -----
> Stefan Hornburg (Racke) <racke@linuxia.de>
>         courier-0.52.1/courier.lpspec(.in)? (maybe not used on Debian)
>         courier-0.52.1/courier.spec(.in)? (maybe not used on Debian)
> Kenneth J. Pronovici <pronovic@debian.org>
>         cedar-backup2-2.7.2/CedarBackup2/peer.py (depends on executeCommand)
> Arnaud Quette <aquette@debian.org>
>         nut-2.0.2/scripts/HP-UX/nut-drvctl.sh (maybe not used on Debian)
>         nut-2.0.2/scripts/HP-UX/nut-upsd.sh (maybe not used on Debian)
> Taku YASUI <tach@debian.or.jp>
>         murasaki-0.8.11/scripts/printer (su $USER -c $CMD, $CMD may have a space)
> Debian Webmin maintainers <webmin-maintainers@lists.alioth.debian.org>
>         usermin-1.160/cron/config-aix (maybe not used on Debian)
>         usermin-1.160/web-lib-funcs.pl
>         usermin-1.160/shell/index.cgi
>         usermin-1.160/fetchmail/check.pl
>         usermin-1.160/commands/run.cgi
>         usermin-1.160/postgresql/postgresql-lib.pl
>         webmin-1.230/web-lib-funcs.pl
>         webmin-1.230/cron/config-aix
>         webmin-1.230/custom/run.cgi


Nicolas, have you been able to check these ones. Otherwise, we will
add their maintainers to the BCC list anyway.



> login will conflict with the package of the first category. When fixed,
> these packages do not need a versionned dependency on login.

We can already set this conflict.


Finally all this should go in a dedicated README file.

[signature.asc (application/pgp-signature, inline)]

Message #274 received at 276419-close@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 276419-close@bugs.debian.org

Subject: Bug#276419: fixed in shadow 1:4.0.15-2

Date: Sun, 02 Apr 2006 10:17:20 -0700

Source: shadow
Source-Version: 1:4.0.15-2

We believe that the bug you reported is fixed in the latest version of
shadow, which is due to be installed in the Debian FTP archive:

login_4.0.15-2_i386.deb
  to pool/main/s/shadow/login_4.0.15-2_i386.deb
passwd_4.0.15-2_i386.deb
  to pool/main/s/shadow/passwd_4.0.15-2_i386.deb
shadow_4.0.15-2.diff.gz
  to pool/main/s/shadow/shadow_4.0.15-2.diff.gz
shadow_4.0.15-2.dsc
  to pool/main/s/shadow/shadow_4.0.15-2.dsc



A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 276419@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Christian Perrier <bubulle@debian.org> (supplier of updated shadow package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Format: 1.7
Date: Sun,  2 Apr 2006 12:45:49 +0200
Source: shadow
Binary: login passwd
Architecture: source i386
Version: 1:4.0.15-2
Distribution: unstable
Urgency: low
Maintainer: Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>
Changed-By: Christian Perrier <bubulle@debian.org>
Description: 
 login      - system login tools
 passwd     - change and administer password and group data
Closes: 276419 355070 359163 360179 360276
Changes: 
 shadow (1:4.0.15-2) unstable; urgency=low
 .
   * The "Pavé d'Auge" release
   * Debian packaging fixes:
     - Only replace manpages-es << 1.55-4. Thanks to Rubén
     - Include chgpasswd in shipped files. Really Closes: #355070
     - parse /etc/default/locale for locale environment variables in login and
       su default PAM configuration files. Thanks to Denis Barbier for the
       patch. Closes: #359163
     - su: Do not concatenate the additional arguments, and support an
           environment variable to revert to the old Debian's su behavior.
           Closes: #276419
           To avoid breaking packages using the old-style way to pass
           arguments, set Conflicts with "gnunet, amavisd-new, python-4suite,
           backupninja (<= 0.9.3-4), echolot (<< 2.1.8-4)"
     - 467_useradd_-r_LSB. Do not forgot to change the owner of the new home
       directory. Closes: #360179
   * Upstream bugs or fixes not already fixed in upstream releases or CVS:
     - 486_chgpasswd.8: add a manpage for chgpasswd.
   * Upstream bugs or fixes fixed in upstream releases or CVS:
     - 492_correct_exit_status_for_run_commands: correct the exit status of su
       when the invoked command fails. Closes: #360276
Files: 
 c53e405de5f3aff5b2f130603984a0a9 964 admin required shadow_4.0.15-2.dsc
 1e02cbcc74f1254f0804032b7eb09ad1 164304 admin required shadow_4.0.15-2.diff.gz
 dd17b3c9717b4cfb8bf6c927b9c215bf 745490 admin required passwd_4.0.15-2_i386.deb
 3df6ae24a264c434f5e280ebe1c43ff5 710842 admin required login_4.0.15-2_i386.deb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)

iD8DBQFEMASO1OXtrMAUPS0RApX2AJ9Dayd00oUFWMS19dMPh7V2YX/YVQCePtTj
wlZNvWu2kl5dWgPh8nD8en0=
=LoT/
-----END PGP SIGNATURE-----

Message #279 received at 276419@bugs.debian.org (full text, mbox, reply):

From: Christian Perrier <bubulle@debian.org>

To: 276419@bugs.debian.org, 276419-submitter@bugs.debian.org

Subject: Re: [Pkg-shadow-devel] Bug#276419: marked as done (su appends the positional args to the command line)

Date: Mon, 3 Apr 2006 07:15:41 +0200

[Message part 1 (text/plain, inline)]

Quoting Debian Bug Tracking System (owner@bugs.debian.org):
> Your message dated Sun, 02 Apr 2006 10:17:20 -0700
> with message-id <E1FQ6CW-0004XX-CC@spohr.debian.org>
> and subject line Bug#276419: fixed in shadow 1:4.0.15-2
> has caused the attached Bug report to be marked as done.


So, we finally got rid of that bug!

I hereby take this opportunity to thank Helmut Waitzmann for
reporting it and be patiently waiting for it to be fixed. Helmut, your
contribution has been very important on that issue.

The transition was not easy to organize and we had to be careful about
not breaking stuff. Nicolas François did a wonderful job investigating
the consequences of the issue and the team learned a lot during this
process.

So, the shadow bug log is ever shrinking and we're not that far from
the Zero Opened Bug Grail....maybe not immediately as I suspect we'll
have a few issues opened because of the "su behaviour transition".

Let's raise a pint^W cheese and celebrate!

[signature.asc (application/pgp-signature, inline)]

Send a report that this bug log contains spam.