[PULL] su

[PULL] su

[Ludwig Nussel]

Hi,

coreutils isn't really interested in maintaining su anymore.
Distributions have to add custom patches to fix bugs and add
features. The worst offender is probably the pam patch. Fortunately
we managed to sync the patch between openSUSE and Fedora meanwhile.
Over time the risk that the patches divert again increases though.
Since util-linux now contains a common login program it looks like a
good new home for su too.

To integrate su into util-linux I've extracted the history of the su
program until the last GPLv2 version (they've changed to GPLv3
meanwhile) from coreutils' git and merged it with util-linux.
Then I've added the patches used in Fedora and openSUSE and fixed the
files to actually build.

You can find the result in branch su-v1 (ff6b15d) at
git@github.com:lnussel/util-linux.git

I've also filed a pull request on github:
https://github.com/karelzak/util-linux/pull/10

Originally I had planned to implement separate tty allocation for
the child program on top of that to get rid of the evil setsid
patch. It turned out to be more complicated than I thought though. The
change would be too massive to backport anyways I guess. So the next
step would be to rip out the non-pam legacy stuff and clean up the code
to make it readable again.

cu
Ludwig

-- 
 (o_   Ludwig Nussel
 //\
 V_/_  http://www.suse.de/
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg) 

Re: [PULL] su

[Karel Zak]

On Thu, May 24, 2012 at 02:58:01PM +0200, Ludwig Nussel wrote:
> coreutils isn't really interested in maintaining su anymore.
> Distributions have to add custom patches to fix bugs and add
> features. The worst offender is probably the pam patch. Fortunately
> we managed to sync the patch between openSUSE and Fedora meanwhile.
> Over time the risk that the patches divert again increases though.
> Since util-linux now contains a common login program it looks like a
> good new home for su too.
> 
> To integrate su into util-linux I've extracted the history of the su
> program until the last GPLv2 version (they've changed to GPLv3
> meanwhile) from coreutils' git and merged it with util-linux.
> Then I've added the patches used in Fedora and openSUSE and fixed the
> files to actually build.

 Good work, thanks.

> You can find the result in branch su-v1 (ff6b15d) at
> git@github.com:lnussel/util-linux.git
> 
> I've also filed a pull request on github:
> https://github.com/karelzak/util-linux/pull/10
> 
> Originally I had planned to implement separate tty allocation for
> the child program on top of that to get rid of the evil setsid
> patch. It turned out to be more complicated than I thought though. The
> change would be too massive to backport anyways I guess. So the next
> step would be to rip out the non-pam legacy stuff and clean up the code
> to make it readable again.

 It would be nice to cleanup the code (remove non-PAM stuff) before
 merge. Can you do this in your su branch? Please.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PULL] su

[Ludwig Nussel]

Karel Zak wrote:
> On Thu, May 24, 2012 at 02:58:01PM +0200, Ludwig Nussel wrote:
>> coreutils isn't really interested in maintaining su anymore.
>> Distributions have to add custom patches to fix bugs and add
>> features. The worst offender is probably the pam patch. Fortunately
>> we managed to sync the patch between openSUSE and Fedora meanwhile.
>> Over time the risk that the patches divert again increases though.
>> Since util-linux now contains a common login program it looks like a
>> good new home for su too.
> [...]
>  It would be nice to cleanup the code (remove non-PAM stuff) before
>  merge. Can you do this in your su branch? Please.

Done.

cu
Ludwig

-- 
 (o_   Ludwig Nussel
 //\
 V_/_  http://www.suse.de/
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg) 

Re: [PULL] su

[Karel Zak]

On Thu, May 24, 2012 at 02:58:01PM +0200, Ludwig Nussel wrote:
> I've also filed a pull request on github:
> https://github.com/karelzak/util-linux/pull/10

 Merged, but you forgot man page ;-) I don't think we need whole
 history for the man page, so just copy and edit any existing su.1
 man page and send it as a patch.

 Thanks.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

su(1) wish list

[Karel Zak]

On Tue, May 29, 2012 at 11:06:09AM +0200, Karel Zak wrote:
> On Thu, May 24, 2012 at 02:58:01PM +0200, Ludwig Nussel wrote:
> > I've also filed a pull request on github:
> > https://github.com/karelzak/util-linux/pull/10
> 
>  Merged, but you forgot man page ;-) I don't think we need whole
>  history for the man page, so just copy and edit any existing su.1
>  man page and send it as a patch.

another issue:

  - why su.c expects "PATH" and "SUPATH" login default variables, but
    login.c expects "ENV_PATH" and "ENV_SUPATH"?

    See also login.def man page from shadow-utils where ENV_* is
    documented (for login as well as for su).

    IMHO we need to share the configuration between login and su.


and wish list:

  - remove MAX(), use max() from c.h
  - use err(), errx() and warn() and warnx() rather than error()
  - share init_environ() from login.c with su.c
  - cleanup usage()
  - move longopts[] to main
  - remove horrible PAM_BAIL macro, see is_pam_failure() in login.c
  - remove #if 0 code

  - simplify xsetenv(), move it to include/env.h as inline function,
    and use it in su.c, login.c etc.

    IMHO we can use code without putenv(), something like

       if (setenv(name, value, ow) != 0)
          err(EXIT_FAILURE, "failed to set %s variable", name);



    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: su(1) wish list

[Ludwig Nussel]

Karel Zak wrote:
> On Tue, May 29, 2012 at 11:06:09AM +0200, Karel Zak wrote:
>> On Thu, May 24, 2012 at 02:58:01PM +0200, Ludwig Nussel wrote:
>>> I've also filed a pull request on github:
>>> https://github.com/karelzak/util-linux/pull/10
>>
>>  Merged, but you forgot man page ;-) I don't think we need whole
>>  history for the man page, so just copy and edit any existing su.1
>>  man page and send it as a patch.

The su man page from coreutils is pretty brief. More docu is in the
info page. The license is the FDL, would that be ok or would a new
man page from scratch be better? Or maybe the one from shadow-utils?

> another issue:
> 
>   - why su.c expects "PATH" and "SUPATH" login default variables, but
>     login.c expects "ENV_PATH" and "ENV_SUPATH"?
> 
>     See also login.def man page from shadow-utils where ENV_* is
>     documented (for login as well as for su).
> 
>     IMHO we need to share the configuration between login and su.

No idea why it is different. Probably legacy stuff. I see no reason
why to use different values than login though.

> and wish list:

ACK

cu
Ludwig

-- 
 (o_   Ludwig Nussel
 //\
 V_/_  http://www.suse.de/
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg) 

RE: su(1) wish list

[Voelker, Bernhard]

Ludwig Nussel wrote:

> The su man page from coreutils is pretty brief. More docu is in the
> info page.

Coreutil's manpages are generated from --help output. It's intension
is just to have a brief overview over the behavior and the options.
Therefore it's almost always better to look into the info pages.
AFAIR that's GNU-ish.

Have a nice day,
Berny

Re: su(1) wish list

[Karel Zak]

On Tue, May 29, 2012 at 02:15:29PM +0200, Ludwig Nussel wrote:
> Karel Zak wrote:
> > On Tue, May 29, 2012 at 11:06:09AM +0200, Karel Zak wrote:
> >> On Thu, May 24, 2012 at 02:58:01PM +0200, Ludwig Nussel wrote:
> >>> I've also filed a pull request on github:
> >>> https://github.com/karelzak/util-linux/pull/10
> >>
> >>  Merged, but you forgot man page ;-) I don't think we need whole
> >>  history for the man page, so just copy and edit any existing su.1
> >>  man page and send it as a patch.
> 
> The su man page from coreutils is pretty brief. More docu is in the
> info page. The license is the FDL, would that be ok or would a new
> man page from scratch be better? Or maybe the one from shadow-utils?

 It should be pretty simple to write a new man page from scratch --
 as an example see some new man pages in util-linux (e.g. lsblk, lslocks, ...)

> No idea why it is different. Probably legacy stuff. I see no reason
> why to use different values than login though.

 OK, I'll (or Sami in his cleanup) will change it. Thanks.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com