Debian Bug report logs - #505071
login tty mis-determination (see bug#332198)

version graph

Package: login; Maintainer for login is Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>; Source for login is src:shadow.

Reported by: Paul Szabo <psz@maths.usyd.edu.au>

Date: Sun, 9 Nov 2008 07:33:01 UTC

Severity: normal

Found in version shadow/1:4.0.18.1-7

Fixed in version 1:4.1.4.1-1

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

Bug is archived. No further changes may be made.

Toggle useless messages

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sun, 09 Nov 2008 07:33:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
New Bug report received and forwarded. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sun, 09 Nov 2008 07:33:04 GMT) Full text and rfc822 format available.

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

From: Paul Szabo <psz@maths.usyd.edu.au>
To: Debian Bug Tracking System <submit@bugs.debian.org>
Subject: login tty mis-determination (see bug#332198)
Date: Sun, 09 Nov 2008 18:26:39 +1100
Package: login
Version: 1:4.0.18.1-7
Severity: normal

(I wanted to send this to  332198@bugs.debian.org  but that was not
accepted, surely because that is closed/archived.)

I found in my logs (I think first occurrence of such mis-behaviour):

Nov  8 05:50:09 rome in.telnetd[21060]: connect from psz@bari.maths.usyd.edu.au (129.78.69.145) 
Nov  8 05:50:12 rome login[21062]: (pam_unix) session opened for user root by (uid=0) 
Nov  8 05:50:12 rome login[21062]: can't stat(`/dev/smb/39'): errno 2  
Nov  8 05:50:12 rome login[21062]: unable to determine TTY name, got /dev/smb/39  

Surely that Samba device is wrong for a telnet session...

Hope this helps in tacking down the cause of this bug.

Cheers,

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia


-- System Information:
Debian Release: 4.0
  APT prefers stable
  APT policy: (500, 'stable')
Architecture: i386 (i686)
Shell:  /bin/sh linked to /bin/bash
Kernel: Linux 2.6.24-pk03.02-svr
Locale: LANG=C, LC_CTYPE=C (charmap=ANSI_X3.4-1968)

Versions of packages login depends on:
ii  libc6                  2.3.6.ds1-13etch7 GNU C Library: Shared libraries
ii  libpam-modules         0.79-5            Pluggable Authentication Modules f
ii  libpam-runtime         0.79-5            Runtime support for the PAM librar
ii  libpam0g               0.79-5            Pluggable Authentication Modules l

login recommends no packages.

-- no debconf information




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sun, 09 Nov 2008 12:09:15 GMT) Full text and rfc822 format available.

Acknowledgement sent to Nicolas François <nicolas.francois@centraliens.net>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sun, 09 Nov 2008 12:09:16 GMT) Full text and rfc822 format available.

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

From: Nicolas François <nicolas.francois@centraliens.net>
To: Paul Szabo <psz@maths.usyd.edu.au>, 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: login tty mis-determination (see bug#332198)
Date: Sun, 9 Nov 2008 13:06:30 +0100
Hello,

First of all, this issue was already discussed, and the main problem was
that we were not able to reproduce it.
Are you currently able to reproduce it?

That would help us a lot, since this would allow testing instrumentation
of login to find the root cause.

Would you agree testing some patches?

On Sun, Nov 09, 2008 at 06:26:39PM +1100, psz@maths.usyd.edu.au wrote:
> Package: login
> Version: 1:4.0.18.1-7
> Severity: normal
> 
> (I wanted to send this to  332198@bugs.debian.org  but that was not
> accepted, surely because that is closed/archived.)
> 
> I found in my logs (I think first occurrence of such mis-behaviour):
> 
> Nov  8 05:50:09 rome in.telnetd[21060]: connect from psz@bari.maths.usyd.edu.au (129.78.69.145) 
> Nov  8 05:50:12 rome login[21062]: (pam_unix) session opened for user root by (uid=0) 
> Nov  8 05:50:12 rome login[21062]: can't stat(`/dev/smb/39'): errno 2  
> Nov  8 05:50:12 rome login[21062]: unable to determine TTY name, got /dev/smb/39  
> 
> Surely that Samba device is wrong for a telnet session...

You logged in with telnet, right?

Do you know the version of telnet you are using?
Do you know if telnet creates a utmp entry before calling login?

What might be happening is that telnet do not create a utmp entry, and an
old one from samba is reused (in checkutmp). This should be very rare also
because the bug would occur only if the same pid is reused. 

Best Regards,
-- 
Nekral




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sun, 09 Nov 2008 21:54:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sun, 09 Nov 2008 21:54:04 GMT) Full text and rfc822 format available.

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

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org, nicolas.francois@centraliens.net
Subject: Re: [Pkg-shadow-devel] Bug#505071: login tty mis-determination (see bug#332198)
Date: Mon, 10 Nov 2008 08:51:53 +1100
Dear Nicolas (Nekral?),

> First of all, this issue was already discussed, and the main problem was
> that we were not able to reproduce it.

Yes, I am aware of bug #332198.

> Are you currently able to reproduce it?

Have not yet attempted to actively reproduce, have observed one
occurrence of "spontaneous" bad behaviour.

> That would help us a lot, since this would allow testing instrumentation
> of login to find the root cause.
> Would you agree testing some patches?

Yes, would be happy to test.

>> I found in my logs (I think first occurrence of such mis-behaviour):
>> 
>> Nov  8 05:50:09 rome in.telnetd[21060]: connect from psz@bari.maths.usyd.edu.au (129.78.69.145) 
>> Nov  8 05:50:12 rome login[21062]: (pam_unix) session opened for user root by (uid=0) 
>> Nov  8 05:50:12 rome login[21062]: can't stat(`/dev/smb/39'): errno 2  
>> Nov  8 05:50:12 rome login[21062]: unable to determine TTY name, got /dev/smb/39  
>> 
>> Surely that Samba device is wrong for a telnet session...
>
> You logged in with telnet, right?
> Do you know the version of telnet you are using?
> Do you know if telnet creates a utmp entry before calling login?

Yes, with telnet; version 0.17-34 (debian etch); surely it cannot
possibly create utmp (telnet runs on bari, telnetd on rome).

> What might be happening is that telnet do not create a utmp entry, and an
> old one from samba is reused (in checkutmp). This should be very rare also
> because the bug would occur only if the same pid is reused. 

Yes, I agree that this is a re-use of an old "unclosed" utmp entry.
(Samba is in the habit of leaving such unclosed entries.) My logs show
(much earlier than the above-quoted lines):

Nov  7 00:52:02 rome samba[21062]: Connect IPC_ for smbguest from p706f (p706f.pc.maths.usyd.edu.au, 129.78.223.215) 

and I did not notice other utmp uses for the same PID in between.

---

Seems to me that the picking of utent in checkutmp by PID (and type?)
only is naive, should pick by line (or id) also, in fact pick by the
is_my_tty checks.

---

File src/login.c has line 87
  extern struct utmp utent;
whereas file libmisc/utmp.c has line 48
  struct utmp utent;
without extern: is that correct?

---

Other comments. Am worried that relying on utmp correctness is a
security risk: conceptually because group utmp would become
root-equivalent, and practically because of shenanigans with utmp
writing e.g. bugs #329156 #330907.

In file libmisc/chowntty.c :
- line 51: should the call
    (stat (tty, &by_name))
  be changed to lstat? Avoid being fooled by symlinks.
- line 66: is the check
    (by_name.st_rdev != by_fd.st_rdev)
  sufficient: can it be fooled with symlinks or hardlinks?
- lines 122,123: should chown(tty,...) and chmod(tty,...) be changed to
  fchown(0,...) and fchmod(0,...)? Avoid being fooled by symlinks and
  races.

Seems to me that as things stand, writing a suitable utmp entry, would
trick login into chowning an arbitrary file. Should I attempt to write
an exploit/demo?

---

Cheers,

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sun, 09 Nov 2008 23:24:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Nicolas François <nicolas.francois@centraliens.net>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sun, 09 Nov 2008 23:24:05 GMT) Full text and rfc822 format available.

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

From: Nicolas François <nicolas.francois@centraliens.net>
To: Paul Szabo <psz@maths.usyd.edu.au>, 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: Bug#505071: login tty mis-determination (see bug#332198)
Date: Mon, 10 Nov 2008 00:20:56 +0100
Hi,

Thanks for your answer.
The culprit is now confirmed.

On Mon, Nov 10, 2008 at 08:51:53AM +1100, psz@maths.usyd.edu.au wrote:
> Dear Nicolas (Nekral?),
> 
> > First of all, this issue was already discussed, and the main problem was
> > that we were not able to reproduce it.
> 
> Yes, I am aware of bug #332198.
> 
> > Are you currently able to reproduce it?
> 
> Have not yet attempted to actively reproduce, have observed one
> occurrence of "spontaneous" bad behaviour.
> 
> > That would help us a lot, since this would allow testing instrumentation
> > of login to find the root cause.
> > Would you agree testing some patches?
> 
> Yes, would be happy to test.
> 
> >> I found in my logs (I think first occurrence of such mis-behaviour):
> >> 
> >> Nov  8 05:50:09 rome in.telnetd[21060]: connect from psz@bari.maths.usyd.edu.au (129.78.69.145) 
> >> Nov  8 05:50:12 rome login[21062]: (pam_unix) session opened for user root by (uid=0) 
> >> Nov  8 05:50:12 rome login[21062]: can't stat(`/dev/smb/39'): errno 2  
> >> Nov  8 05:50:12 rome login[21062]: unable to determine TTY name, got /dev/smb/39  
> >> 
> >> Surely that Samba device is wrong for a telnet session...
> >
> > You logged in with telnet, right?
> > Do you know the version of telnet you are using?
> > Do you know if telnet creates a utmp entry before calling login?
> 
> Yes, with telnet; version 0.17-34 (debian etch); surely it cannot
> possibly create utmp (telnet runs on bari, telnetd on rome).

Well, I meant telnetd should have inserted a utmp entry, on teh server
side.

> > What might be happening is that telnet do not create a utmp entry, and an
> > old one from samba is reused (in checkutmp). This should be very rare also
> > because the bug would occur only if the same pid is reused. 
> 
> Yes, I agree that this is a re-use of an old "unclosed" utmp entry.
> (Samba is in the habit of leaving such unclosed entries.) My logs show
> (much earlier than the above-quoted lines):
> 
> Nov  7 00:52:02 rome samba[21062]: Connect IPC_ for smbguest from p706f (p706f.pc.maths.usyd.edu.au, 129.78.223.215) 
> 
> and I did not notice other utmp uses for the same PID in between.
> 
> ---
> 
> Seems to me that the picking of utent in checkutmp by PID (and type?)
> only is naive, should pick by line (or id) also, in fact pick by the
> is_my_tty checks.

I agree with you that the utmp handling in shadow is not clean, and might
have a security implication.

I fear I won't have time to work on it in the next 2/3 weeks.

I think checking for the line might be good if the line is known, as well
as the user if possible.

> File src/login.c has line 87
>   extern struct utmp utent;
> whereas file libmisc/utmp.c has line 48
>   struct utmp utent;
> without extern: is that correct?

I think that's the expected behavior, however, I would prefer to avoid
such hidden communication between the modules.

> ---
> 
> Other comments. Am worried that relying on utmp correctness is a
> security risk: conceptually because group utmp would become
> root-equivalent, and practically because of shenanigans with utmp
> writing e.g. bugs #329156 #330907.
> 
> In file libmisc/chowntty.c :
> - line 51: should the call
>     (stat (tty, &by_name))
>   be changed to lstat? Avoid being fooled by symlinks.
> - line 66: is the check
>     (by_name.st_rdev != by_fd.st_rdev)
>   sufficient: can it be fooled with symlinks or hardlinks?
> - lines 122,123: should chown(tty,...) and chmod(tty,...) be changed to
>   fchown(0,...) and fchmod(0,...)? Avoid being fooled by symlinks and
>   races.
> 
> Seems to me that as things stand, writing a suitable utmp entry, would
> trick login into chowning an arbitrary file. Should I attempt to write
> an exploit/demo?

That would be nice to check if it would be possible to chown /etc/shadow
by cheating utmp.

A fake demo would be nice.
(by "fake demo", I mean that you do not have to find a way to guess the
PID, but can recompile a new login which use an hardcoded utmp entry in
checkutmp; that would be sufficient since we already know the utmp entry
selection is wrong and can be cheated)

I hope is_my_tty protects it, but I did not checked at all the complete
path.

Cheers,
-- 
Nekral




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Mon, 10 Nov 2008 00:06:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Mon, 10 Nov 2008 00:06:02 GMT) Full text and rfc822 format available.

Message #25 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org, nicolas.francois@centraliens.net
Subject: Re: [Pkg-shadow-devel] Bug#505071: Bug#505071: login tty mis-determination (see bug#332198)
Date: Mon, 10 Nov 2008 11:04:12 +1100
Dear Nekral,

>> Seems to me that as things stand, writing a suitable utmp entry, would
>> trick login into chowning an arbitrary file. Should I attempt to write
>> an exploit/demo?
>
> That would be nice to check if it would be possible to chown /etc/shadow
> by cheating utmp.
>
> A fake demo would be nice.
> (by "fake demo", I mean that you do not have to find a way to guess the
> PID, but can recompile a new login which use an hardcoded utmp entry in
> checkutmp; that would be sufficient since we already know the utmp entry
> selection is wrong and can be cheated)
>
> I hope is_my_tty protects it, but I did not checked at all the complete
> path.

I expect the following would work:
Predict what PID and tty will be used by login. (This is rather simple:
surely the next available ones, maybe current tty.) For sake of example,
say these are PID=123 and tty=/dev/pts/1.
Pre-create a symlink  /tmp/x -> /dev/pts/1  and write an utmp entry
with PID=123, line=/tmp/x, type=LOGIN_PROCESS.
Run login. While login is running, change /tmp/x to point to /etc/shadow.
We win the race if the change is done after stat(tty,...) within
is_my_tty and before chown(tty,...) in chown_tty.

Hope this is sufficient...

Cheers, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Mon, 10 Nov 2008 10:24:04 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Mon, 10 Nov 2008 10:24:05 GMT) Full text and rfc822 format available.

Message #30 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org, nicolas.francois@centraliens.net
Subject: Re: [Pkg-shadow-devel] Bug#505071: Bug#505071: login tty mis-determination (see bug#332198)
Date: Mon, 10 Nov 2008 21:15:58 +1100
Dear Nekral,

I have not yet written an exploit/PoC/demo, but think it should be
rather easy to do. Looking at the recent DSA-1500 also, I ask you to
change the severity of this bug to "critical - root security hole",
and of course to fix things quickly. (I would change the severity
myself, but I think as a result of bug #299007 am not allowed.)

Thanks, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Mon, 10 Nov 2008 11:39:13 GMT) Full text and rfc822 format available.

Acknowledgement sent to Nicolas François <nicolas.francois@centraliens.net>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Mon, 10 Nov 2008 11:39:13 GMT) Full text and rfc822 format available.

Message #35 received at 505071@bugs.debian.org (full text, mbox):

From: Nicolas François <nicolas.francois@centraliens.net>
To: psz@maths.usyd.edu.au
Cc: 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: Bug#505071: login tty mis-determination (see bug#332198)
Date: Mon, 10 Nov 2008 12:17:01 +0100
Hello,

I think there are two different bugs:

 * one is that login relies on the utmp entry with the current PID
   In my opinion, this cannot be exploited because is_my_tty will detect
   it.

 * The other one is that between is_my_tty and chown, there is a race
   condition.
   Changing chown (tty, ...) to fchown (0, ...) might work and might be
   sufficient.

The first bug is not critical.

The second one should be fixed for Lenny, but tested first.

Best Regards,
-- 
Nekral




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Mon, 10 Nov 2008 23:21:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Mon, 10 Nov 2008 23:21:11 GMT) Full text and rfc822 format available.

Message #40 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: nicolas.francois@centraliens.net
Cc: 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: Bug#505071: login tty mis-determination (see bug#332198)
Date: Tue, 11 Nov 2008 07:36:18 +1100
Dear Nekral,

Curious way of counting bugs. What do you mean exploitable: to do what?
(Surely is_my_tty cannot protect, being buggy itself.)

As I see things, the following bugs are present:

- bad selection of utmp entry [often choosing wrong]
- is_my_tty uses stat [should be lstat]
- is_my_tty compares rdev only [should also test dev ino etc]
- maybe is_my_tty should scrutinize path [ensure directory components
  are root-owned and safe]
- race between is_my_tty checks and chown
- chown of unsafe path [should be fchown anyway]

As things are, it is exploitable to elevate privileges from group utmp
to root. It is also buggy, often failing for legitimate use. Fixing all
bugs would be best; fixing some may already render it "safe" against
exploitation, and/or restore functionality.

Please, fix soon. Please change severity.

Cheers,

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Mon, 10 Nov 2008 23:27:13 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Mon, 10 Nov 2008 23:27:13 GMT) Full text and rfc822 format available.

Message #45 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: nicolas.francois@centraliens.net
Cc: 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: Bug#505071: login tty mis-determination (see bug#332198)
Date: Tue, 11 Nov 2008 07:39:50 +1100
Dear Nekral,

Sorry, I missed your comment:

> ... should be fixed for Lenny ...

No. Should be fixed now, for etch. Needs a DSA.

Cheers,

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Mon, 10 Nov 2008 23:54:07 GMT) Full text and rfc822 format available.

Acknowledgement sent to Nicolas François <nicolas.francois@centraliens.net>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Mon, 10 Nov 2008 23:54:07 GMT) Full text and rfc822 format available.

Message #50 received at 505071@bugs.debian.org (full text, mbox):

From: Nicolas François <nicolas.francois@centraliens.net>
To: psz@maths.usyd.edu.au
Cc: 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: Bug#505071: login tty mis-determination (see bug#332198)
Date: Tue, 11 Nov 2008 00:52:57 +0100
On Tue, Nov 11, 2008 at 07:36:18AM +1100, psz@maths.usyd.edu.au wrote:
> 
> Curious way of counting bugs. What do you mean exploitable: to do what?
> (Surely is_my_tty cannot protect, being buggy itself.)
> 
> As I see things, the following bugs are present:
> 
> - bad selection of utmp entry [often choosing wrong]

Often is arguable.
2 reports in 10 years.

> - is_my_tty uses stat [should be lstat]

I'm not sure lstat is right.
If the caller of login puts the name of a symbolic link for any reason in
utmp, I don't think that should be a failure.

> - is_my_tty compares rdev only [should also test dev ino etc]

I don't think the device or the inode is relevant.
If the major and minor of the device are identical, then they indicate the
same device.

> - maybe is_my_tty should scrutinize path [ensure directory components
>   are root-owned and safe]

Same as lstat, I don't think the paths have to match.

> - race between is_my_tty checks and chown

Yes.

> - chown of unsafe path [should be fchown anyway]

Except for the race, I don't think the path in unsafe.

> As things are, it is exploitable to elevate privileges from group utmp
> to root. It is also buggy, often failing for legitimate use. Fixing all
> bugs would be best; fixing some may already render it "safe" against
> exploitation, and/or restore functionality.

I currently think is_my_tty should be removed. checkutmp should check that
ut_line matches with the current tty, and return a file descriptor

Best Regards,
-- 
Nekral




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Tue, 11 Nov 2008 00:51:06 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Tue, 11 Nov 2008 00:51:07 GMT) Full text and rfc822 format available.

Message #55 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: nicolas.francois@centraliens.net
Cc: 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: Bug#505071: login tty mis-determination (see bug#332198)
Date: Tue, 11 Nov 2008 11:47:53 +1100
Dear Nekral,

> Often is arguable.

Are not computers meant to be infallible and perfect?

---

Privileged programs should be strict on what they accept.

Paths are un-safe unless you verify that all directories above are
root-owned and not group or world writeable.

---

How you count bugs, how you fix the issues, is up to you (does not have
to be to my liking). Please fix soon, before someone writes an exploit.

Cheers,

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Tue, 11 Nov 2008 11:04:37 GMT) Full text and rfc822 format available.

Acknowledgement sent to Nicolas François <nicolas.francois@centraliens.net>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Tue, 11 Nov 2008 11:04:38 GMT) Full text and rfc822 format available.

Message #60 received at 505071@bugs.debian.org (full text, mbox):

From: Nicolas François <nicolas.francois@centraliens.net>
To: psz@maths.usyd.edu.au
Cc: 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: login tty mis-determination (see bug#332198)
Date: Tue, 11 Nov 2008 12:00:28 +0100
clone 505071 -1
retitle -1 symlink attack in login leading to arbitrary file ownership
tags -1 security
severity -1 serious
tags -1 patch
thanks

Somebody with write access to the utmp database can create the conditions
for a symlink attack in login, leading to gaining ownership of an
arbitrary file.

Proposed fix: Changing chown (tty, ...) to fchown (0, ...) in chowntty()

Best Regards,
-- 
Nekral




Bug 505071 cloned as bug 505271. Request was from Nicolas François <nicolas.francois@centraliens.net> to control@bugs.debian.org. (Tue, 11 Nov 2008 11:04:39 GMT) Full text and rfc822 format available.

Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Tue, 11 Nov 2008 12:15:07 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Tue, 11 Nov 2008 12:15:08 GMT) Full text and rfc822 format available.

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

From: Paul Szabo <psz@maths.usyd.edu.au>
To: nicolas.francois@centraliens.net
Cc: 505071@bugs.debian.org, 505271@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: login tty mis-determination (see bug#332198)
Date: Tue, 11 Nov 2008 23:13:21 +1100
Dear Nekral,

> Proposed fix: Changing chown (tty, ...) to fchown (0, ...) in chowntty()

Surely you meant to change chmod to fchmod also. (I know this is
nit-picking, but best to be sure...)

Thanks, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sun, 23 Nov 2008 05:57:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sun, 23 Nov 2008 05:57:06 GMT) Full text and rfc822 format available.

Message #72 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org, 505271@bugs.debian.org, nicolas.francois@centraliens.net
Subject: Bug#505071 and Bug#505271 comments
Date: Sun, 23 Nov 2008 16:56:17 +1100
Random comments about bugs 505071 and 505271.

Group utmp was introduced so terminal emulators could be setgid instead
of needing setuid root, to prevent bugs in them to escalate to root
access. Terminal emulators are generally not written with security in
mind, being the "more features the better" type of programs. This bug
negates the benefit of the group utmp separation. Any data controllable
by group utmp should be treated as insecure or possibly hostile.

Seems that login attempts to pick the "right" line of the utmp file.
Funny idea, seeing how pututline will whack the entry "anywhere"
(depending on ut_id which are rather arbitrary and irrelevant).

If login wanted to sanitize left-over utmp entries, then should set what
ttyname thinks is the correct ut_line (and what it thinks is a sensible
ut_id), not perpetuate "wrong" settings.

Privileged programs should not attempt to clean up utmp, root can do
that at his leisure e.g. with "echo -n '' > /var/run/utmp".

Cheers,

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Fri, 23 Jan 2009 02:12:06 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Fri, 23 Jan 2009 02:12:06 GMT) Full text and rfc822 format available.

Message #77 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org
Subject: Re: Bug#505071: login tty mis-determination (see bug#332198)
Date: Fri, 23 Jan 2009 13:10:24 +1100
We had discussed whether login fails often due to left-over utmp
entries. I guess that depends on how likely it is that processes die
without cleanup; and how important it is that login should "work".

I now see that there is the possibility for a DoS attack, by filling
up utmp with left-over entries to cover all PIDs (or a significant
proportion of PID space). For example, we can cause one left-over
entry with

  run xterm and within that xterm use "kill -9 $PPID"

and might try that repeatedly to exhaust PID space; except xterm
reuses ptys and re-writes utmp entries, and the pty space is smaller
than PID space...

Until this is fixed, utmp should be sanitized regularly to remove
unused entries. I now use the following, often (i.e. nightly).

Cheers,

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia


---

#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <stdio.h>
#include <utmp.h>
#include <time.h>
/*
#include <signal.h>
#include <errno.h>
*/

int main( int argc, char *argv[])
{
char *usage = "Usage:\n\n\
  clear-dead-utmp-entries [-quiet] [-really]\n\n\
to clear left-over utmp entries.\n\n";

int debug = 0;
int quiet = 0;
int really = 0;
int cleared = 0;
int clearable = 0;
int header = 0;
int i;
struct utmp *utmpp;
struct tm *tmmp;
#define buflen 256
char buf[buflen];
struct stat strst;

for (i=1; i<argc; i++) {
  if (!strcmp(argv[i], "-debug")) {
    debug++;
  }
  else if (!strcmp(argv[i], "-quiet")) {
    quiet++;
  }
  else if (!strcmp(argv[i], "-really")) {
    really++;
  }
  else {
    printf ("\nBad option %s. %s", argv[i], usage);
    return 1;
  }
}

setutent();

while (utmpp = getutent()) {
  if (utmpp->ut_type != INIT_PROCESS &&
      utmpp->ut_type != LOGIN_PROCESS &&
      utmpp->ut_type != USER_PROCESS) continue;
  if (utmpp->ut_pid == 0) continue;

  /* Is this PID "alive"? */
  /* Seems that /usr/bin/who uses
  if (kill(utmpp->ut_pid, 0) < 0 && errno == ESRCH) continue;
     but "plain" psz gets EPERM, while root gets success with ENOENT.
  */
  sprintf(buf, "/proc/%d", (int)utmpp->ut_pid);
  if (!stat(buf, &strst)) continue;

  tmmp = localtime(&utmpp->ut_time);
  strftime(buf, buflen, "%b %d %H:%M", tmmp);
  if (debug) {
    printf ("\nutmp line:\n");
    printf ("  ut_user: %.*s\n", sizeof(utmpp->ut_user), utmpp->ut_user);
    printf ("  ut_id:   %.*s\n", sizeof(utmpp->ut_id), utmpp->ut_id);
    printf ("  ut_line: %.*s\n", sizeof(utmpp->ut_line), utmpp->ut_line);
    printf ("  ut_type: %d\n", (int)utmpp->ut_type);
    printf ("  ut_pid:  %d\n", (int)utmpp->ut_pid);
    printf ("  e_term:  %d\n", (int)utmpp->ut_exit.e_termination);
    printf ("  e_exit:  %d\n", (int)utmpp->ut_exit.e_exit);
    printf ("  ut_time: %s\n", buf);
    printf ("  ut_host: %.*s\n", sizeof(utmpp->ut_host), utmpp->ut_host);
    printf ("\n");
  }

  if (!header) {
    printf ("T PID   Name      Line    Time         Hostname\n");
    header++;
  }
  printf ("%-2d%-6d%-10.10s%-8.8s%-13.13s%.40s\n",
    (int)utmpp->ut_type,
    (int)utmpp->ut_pid,
    utmpp->ut_user,
    utmpp->ut_line,
    buf,
    utmpp->ut_host);

  if (!really) {
    clearable++;
    continue;
  }

  utmpp->ut_type = DEAD_PROCESS;
  if (! pututline(utmpp)) {
    printf ("\nERROR: Cannot clear utmp entry above\n\n");
    break;
  };
  cleared++;
}

if (cleared==1) {
  printf ("Cleared one utmp entry shown above\n");
}
else if (cleared>1) {
  printf ("Cleared %d utmp entries shown above\n", cleared);
}
else {
  if (clearable) printf ("No utmp entries cleared, need '-really' option for that\n");
  else if (!quiet) printf ("No utmp entries cleared\n");
}

}




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Fri, 23 Jan 2009 03:03:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Fri, 23 Jan 2009 03:03:07 GMT) Full text and rfc822 format available.

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

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org
Subject: Re: Bug#505071: login tty mis-determination (see bug#332198)
Date: Fri, 23 Jan 2009 13:58:26 +1100
Hmm... could we use Samba for a DoS against login? On a PC log in to
Samba, then "kill -9 PID-of-my-smbd" to leave one utmp entry behind.
Samba will automatically re-spawn a new smbd, then kill that... I do
not yet know how large is the ut_id space used by samba (whether this
could exhaust a significant proportion of PID space).

Cheers,

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Fri, 23 Jan 2009 21:27:04 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Fri, 23 Jan 2009 21:27:05 GMT) Full text and rfc822 format available.

Message #87 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org
Subject: Re: Bug#505071: login tty mis-determination (see bug#332198)
Date: Sat, 24 Jan 2009 08:09:41 +1100
I wrote:

> ... we can cause one left-over entry with [xterm] ... except xterm
> reuses ptys and re-writes utmp entries ...

We can arrange to hog the pty but release the PID with

  run xterm, and within that xterm use
  bash -c 'trap "" 11; sleep 600 &'; kill -11 $PPID

Then waste a few PIDs with something like
  perl -e 'foreach (1..32000) { system "/bin/false" }'
so the "next PID" will be what we want; then do the xterm again and
repeat until we have a contiguous block of PIDs in the utmp file.
Spin the "next PID" to be within that range, and we have a DoS
against the next few login attempts.

I do not know what practical uses this could have: lock out root so
cannot observe our activities?

Cheers,

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Thu, 29 Jan 2009 19:21:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Thu, 29 Jan 2009 19:21:02 GMT) Full text and rfc822 format available.

Message #92 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org
Subject: Re: Bug#505071: login tty mis-determination (see bug#332198)
Date: Fri, 30 Jan 2009 06:17:39 +1100
Another (unrelated?) query. Login re-writes, or writes, a utmp entry.
Should not it remove that entry on exit? I do not think telnetd does
anything with utmp.

Thanks,

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Tags added: pending Request was from Nicolas FRANCOIS (Nekral) <nicolas.francois@centraliens.net> to control@bugs.debian.org. (Sun, 05 Apr 2009 23:03:12 GMT) Full text and rfc822 format available.

Reply sent to Nicolas FRANCOIS (Nekral) <nicolas.francois@centraliens.net>:
You have taken responsibility. (Tue, 14 Apr 2009 22:48:05 GMT) Full text and rfc822 format available.

Notification sent to Paul Szabo <psz@maths.usyd.edu.au>:
Bug acknowledged by developer. (Tue, 14 Apr 2009 22:48:05 GMT) Full text and rfc822 format available.

Message #99 received at 505071-close@bugs.debian.org (full text, mbox):

From: Nicolas FRANCOIS (Nekral) <nicolas.francois@centraliens.net>
To: 505071-close@bugs.debian.org
Subject: Bug#505071: fixed in shadow 1:4.1.3-1
Date: Tue, 14 Apr 2009 22:47:12 +0000
Source: shadow
Source-Version: 1:4.1.3-1

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.1.3-1_i386.deb
  to pool/main/s/shadow/login_4.1.3-1_i386.deb
passwd_4.1.3-1_i386.deb
  to pool/main/s/shadow/passwd_4.1.3-1_i386.deb
shadow_4.1.3-1.diff.gz
  to pool/main/s/shadow/shadow_4.1.3-1.diff.gz
shadow_4.1.3-1.dsc
  to pool/main/s/shadow/shadow_4.1.3-1.dsc
shadow_4.1.3.orig.tar.gz
  to pool/main/s/shadow/shadow_4.1.3.orig.tar.gz



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 505071@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Nicolas FRANCOIS (Nekral) <nicolas.francois@centraliens.net> (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.8
Date: Tue, 14 Apr 2009 23:33:22 +0200
Source: shadow
Binary: passwd login
Architecture: source i386
Version: 1:4.1.3-1
Distribution: unstable
Urgency: low
Maintainer: Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>
Changed-By: Nicolas FRANCOIS (Nekral) <nicolas.francois@centraliens.net>
Description: 
 login      - system login tools
 passwd     - change and administer password and group data
Closes: 474318 487105 498788 499578 505071 508785 511739 511961 513252 517809 523621 523798
Changes: 
 shadow (1:4.1.3-1) unstable; urgency=low
 .
   * The "" release.
   * New upstream release:
     - Fix possible login DOS. Closes: #505071
     - Fix gpasswd and username with 32 characters. Closes: #508785
     - Fix typo in nologin(8). Closes: #513252
     - Remove old features from passwd(1). Closes: #499578
     - login: Close passwd while waiting for exit. Closes: #474318
     - login: fix the count of login failures. Closes: #498788
     - Remove patches applied upstream (4.1.2):
       + debian/patches/434_login_stop_checking_args_after--
       + debian/patches/491_configure.in_friendly_selinux_detection
       + debian/patches/487_passwd_chauthtok_failed_message
       + debian/patches/406_vipw_resume_properly
       + debian/patches/414_remove-unwise-advices
       + debian/patches/300_SHA_crypt_method
       + debian/patches/301_manpages_missing_options
       + debian/patches/415_login_put-echoctl-back
       + debian/patches/431_su_uid_0_not_root
     - Remove patches applied upstream (4.1.3):
       + debian/patches/200_Czech_binary_translation
       + debian/patches/302_remove_non_translated_polish_manpages
       + debian/patches/494_passwd_lock-no_account_lock
       + debian/patches/200_Czech_binary_translation
       + debian/patches/494_passwd_lock-no_account_lock
     - Updated patches:
       + debian/patches/431_su_uid_0_not_root
       + debian/patches/463_login_delay_obeys_to_PAM
       + debian/patches/008_su_get_PAM_username
       + debian/patches/302_vim_selinux_support
       + debian/patches/008_login_log_failure_in_FTMP
       + debian/patches/429_login_FAILLOG_ENAB
       + debian/patches/428_grpck_add_prune_option
       + debian/patches/401_cppw_src.dpatch
       + debian/patches/506_relaxed_usernames
       + debian/patches/463_login_delay_obeys_to_PAM
       + debian/patches/542_useradd-O_option
     - Translations
       + New Kazakh translation. Closes: #517809
       + Updated Slovak translation. Closes: #523621
   * debian/patches/454_userdel_no_MAIL_FILE: Patch removed. If MAIL_FILE is
     defined, the mailbox is not in MAIL_SPOOL_DIR.
   * debian/patches/506_relaxed_usernames: Use an extra paragraph for the note
     on username with a '/'.
   * debian/patches/504_undef_USE_PAM.nolibpam,
     debian/patches/504_undef_USE_PAM.dpatch, debian/rules: Patches removed.
     Replaced by the --disable-account-tools-setuid configure option.
   * debian/control: changed the "Replaces" on manpages-zh to a versioned
     one on 1.5.1-1
   * debian/control: drop all Replaces on manpages-* when the version is
     prior to Etch
   * Versioned Replaces on manpages-tr (<<1..5) as conflicting manpages have
     been removed in that package
   * debian/patches/402_cppw_selinux: Add SE Linux support for cppw / cpgr.
   * debian/patches/900_testsuite_groupmems, debian/patches/901_testsuite_gcov:
     Added patches, only intended to be used in the testsuite.
   * debian/securetty.linux: Added ttyPZ0, ttyPZ1, ttyPZ2, ttyPZ3 for PowerMac
     machines.  Closes: #511739
   * debian/patches/579_chowntty_debug: Removed. With the fix for 505071 and
     505271, this additional debug information is no more needed.
   * debian/patches/507_32char_grnames.dpatch: Patch removed. Replaced by the
     --with-group-name-max-length=32 configure option.
   * debian/patches/592_manpages_typos: No more needed.
   * debian/patches/401_cppw_src.dpatch: Call fsync before closing the backup
     file descriptor. This ensures that the backup file will be available on
     the storage medium.
   * debian/securetty.linux: Removed devfs devices. Usage of devfs enabled
     kernel in Lenny was not supported. Closes: #511961
   * debian/login.defs: Added /usr/local/games/ to ENV_PATH (for regular
     users). Closes: #487105
   * debian/patches/200_bin_nb: Updated Norwegian Bokmål translation.
     Closes: #523798
   * debian/login.defs: Update GID_MIN to 1000. This is more consistent with
     UID_MIN, SYS_GID_MAX and the usage of the same ID for UID and GIDs. This
     should also be more consistent with the assignment of system group IDs
     starting from GID_MAX and going down.
Checksums-Sha1: 
 33524f2d5fe86dcbeb0ae0c1b2e650c976c66909 1545 shadow_4.1.3-1.dsc
 36461090d62c5e262fe9306490d815025604aa80 2669037 shadow_4.1.3.orig.tar.gz
 822dabee0160bf1479bf6e49625f799ab3cc8931 126656 shadow_4.1.3-1.diff.gz
 d1db90de0dbef2d3ba2947f628ae17cd0f976450 929314 passwd_4.1.3-1_i386.deb
 402ae8d5e2d1a183cbf7fa91930185e92592ccb0 673684 login_4.1.3-1_i386.deb
Checksums-Sha256: 
 d5e6e7a382a233d4c7833d4e2b251238f2de1b7550aab3178c01b874dda71ebd 1545 shadow_4.1.3-1.dsc
 fef065c4317631068e55b04460eea058afe8e0d155372562672a22a013addd7a 2669037 shadow_4.1.3.orig.tar.gz
 67c9fc554708b0d8d377965ef3feb300e2ef61aecfe11dee23124226651dab38 126656 shadow_4.1.3-1.diff.gz
 955666a44752a2c4aebef12404b74fe4717daed8049f551ef1b06f53530b8cc5 929314 passwd_4.1.3-1_i386.deb
 6b8e3e33952a630fa2a73bde93cd5cd52de1f630a760cd9887f56f9ae27db5ea 673684 login_4.1.3-1_i386.deb
Files: 
 e0a639aa479ef186a31d4d39fc67dada 1545 admin required shadow_4.1.3-1.dsc
 d77a393dfd48746e7d63d38f6d5a1053 2669037 admin required shadow_4.1.3.orig.tar.gz
 ebe323127df8307060bee7916c30ea0b 126656 admin required shadow_4.1.3-1.diff.gz
 836ea47407e376ef22094d96c6cdc14e 929314 admin required passwd_4.1.3-1_i386.deb
 687bcf4bc7d540e2865386c9e85072f7 673684 admin required login_4.1.3-1_i386.deb

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

iEYEARECAAYFAknlDuUACgkQWgo5mup89a0VHQCdFFSk1PoFMU6SiBIZoRSiCK9b
zmgAn1dw0ANOFPuhz6nJl8oE7ve0yr2K
=xBHh
-----END PGP SIGNATURE-----





Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Thu, 16 Apr 2009 10:03:03 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Thu, 16 Apr 2009 10:03:03 GMT) Full text and rfc822 format available.

Message #104 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org
Cc: team@security.debian.org
Subject: Re: Bug#505071 closed ... fixed in shadow 1:4.1.3-1
Date: Thu, 16 Apr 2009 19:59:17 +1000
Dear Nicolas,

> We believe that the bug you reported is fixed in ...
> login_4.1.3-1_i386.deb ...

The untrusted ut_line is now internally used for utmp only (so there
should be no security issues there), but is passed to PAM as PAM_TTY.
Thus an attacker could:
 - cause securetty checks to fail resulting in a DoS, or
 - bypass or trick some checks in pam_time or pam_group.
Please let me know if you require further details.

[Am puzzled that the bug embodied in is_my_tty() was left, and by the
insistence to use ut_line in preference to ttyname().]

Please re-open the bug.

Cheers, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Thu, 16 Apr 2009 13:30:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Nicolas François <nicolas.francois@centraliens.net>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Thu, 16 Apr 2009 13:30:02 GMT) Full text and rfc822 format available.

Message #109 received at 505071@bugs.debian.org (full text, mbox):

From: Nicolas François <nicolas.francois@centraliens.net>
To: Paul Szabo <psz@maths.usyd.edu.au>, 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Thu, 16 Apr 2009 15:23:55 +0200
Hello,

On Thu, Apr 16, 2009 at 07:59:17PM +1000, psz@maths.usyd.edu.au wrote:
> 
> > We believe that the bug you reported is fixed in ...
> > login_4.1.3-1_i386.deb ...
> 
> The untrusted ut_line is now internally used for utmp only (so there
> should be no security issues there), but is passed to PAM as PAM_TTY.

Please state more clearly why it's untrusted, and why it's a problem.

This statement as such is useless, and as it seems you already have dig
into it, it should be easy for you report your findings.
If you have ideas on what should be done (or even better, patches), they
are welcomed. If you want to change the utmp handling of login, that's
fine, but please explain.

If I have to look again in the history of the bug, in the source, come back
to your statement, make a proposal, close the bug, receive this kind of
comment, etc. it is a waste of time.

> Thus an attacker could:
>  - cause securetty checks to fail resulting in a DoS, or
>  - bypass or trick some checks in pam_time or pam_group.
> Please let me know if you require further details.

Yes, further details would be welcomed.

> [Am puzzled that the bug embodied in is_my_tty() was left, and by the
> insistence to use ut_line in preference to ttyname().]

Am puzzled about that comment.

> Please re-open the bug.

Please do if it's the same bug (but with a rational).
Please open a new bug if it's another bug (explanations are also needed,
and proposals are welcomed)

Best Regards,
-- 
Nekral




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Thu, 16 Apr 2009 22:06:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Thu, 16 Apr 2009 22:06:05 GMT) Full text and rfc822 format available.

Message #114 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org, nicolas.francois@centraliens.net
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Fri, 17 Apr 2009 07:55:23 +1000
Dear Nicolas,

> Please state more clearly ...
> If I have to look again ... it is a waste of time.

That discussion is not fruitful.

> If you have ... patches, they are welcomed.

Please see below. The patch of src/login.c is essential for security;
I would prefer to use the libmisc/utmp.c patch also.

Hmm... am now thinking that hostname (PAM_RHOST) may also be dodgy.

>> Please re-open the bug.
> Please do ...

I do not think I can re-open (would not know how, and I think am banned
from doing control things since the kerfuffle in #299007).

Cheers, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia


--- src/login.c.bak	2009-04-17 07:00:50.000000000 +1000
+++ src/login.c	2009-04-17 07:30:51.000000000 +1000
@@ -479,7 +479,14 @@
 	 * entry (will not overwrite remote hostname).  --marekm
 	 */
 	checkutmp (!amroot);
-	STRFCPY (tty, utent.ut_line);
+	/*
+	 * PSz 17 Apr 09 Though we may handle ut_line correctly (for Linux),
+	 * we should not trust PAM_TTY to its vagaries...
+	 *STRFCPY (tty, utent.ut_line);
+	 */
+	tmp = ttyname (0);
+	if (NULL == tmp) { tmp = "UNKNOWN"; }
+	STRFCPY (tty, tmp);
 #ifndef USE_PAM
 	is_console = console (tty);
 #endif
--- libmisc/utmp.c.bak	2008-11-23 10:56:10.000000000 +1100
+++ libmisc/utmp.c	2009-04-17 07:53:08.000000000 +1000
@@ -127,23 +127,43 @@
 			(void) puts (NO_UTENT);
 			exit (EXIT_FAILURE);
 		}
-		line = ttyname (0);
-		if (NULL == line) {
-			(void) puts (NO_TTY);
-			exit (EXIT_FAILURE);
-		}
-		if (strncmp (line, "/dev/", 5) == 0) {
-			line += 5;
-		}
+/*
+ *		line = ttyname (0);
+ *		if (NULL == line) {
+ *			(void) puts (NO_TTY);
+ *			exit (EXIT_FAILURE);
+ *		}
+ *		if (strncmp (line, "/dev/", 5) == 0) {
+ *			line += 5;
+ *		}
+ */
 		memset ((void *) &utent, 0, sizeof utent);
 		utent.ut_type = LOGIN_PROCESS;
 		utent.ut_pid = pid;
-		strncpy (utent.ut_line, line, sizeof utent.ut_line);
-		/* XXX - assumes /dev/tty?? or /dev/pts/?? */
-		strncpy (utent.ut_id, utent.ut_line + 3, sizeof utent.ut_id);
+/*
+ *		strncpy (utent.ut_line, line, sizeof utent.ut_line);
+ *		* XXX - assumes /dev/tty?? or /dev/pts/?? *
+ *		strncpy (utent.ut_id, utent.ut_line + 3, sizeof utent.ut_id);
+ */
 		strcpy (utent.ut_user, "LOGIN");
 		utent.ut_time = time (NULL);
 	}
+	/*
+	 * PSz 17 Apr 09 Sanitize ut_line and ut_id anyway... so why
+	 * did we bother with getutent and is_my_tty: for ut_host that
+	 * we cannot trust either?
+	 */
+	line = ttyname (0);
+	if (NULL == line) {
+		(void) puts (NO_TTY);
+		exit (EXIT_FAILURE);
+	}
+	if (strncmp (line, "/dev/", 5) == 0) {
+		line += 5;
+	}
+	strncpy (utent.ut_line, line, sizeof utent.ut_line);
+	/* XXX - assumes /dev/tty?? or /dev/pts/?? */
+	strncpy (utent.ut_id, utent.ut_line + 3, sizeof utent.ut_id);
 }
 
 #elif defined(LOGIN_PROCESS)




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Fri, 17 Apr 2009 01:21:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Fri, 17 Apr 2009 01:21:02 GMT) Full text and rfc822 format available.

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

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org, nicolas.francois@centraliens.net
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Fri, 17 Apr 2009 11:18:16 +1000
>> Thus an attacker could:
>>  - cause securetty checks to fail resulting in a DoS, or
>>  - bypass or trick some checks in pam_time or pam_group.
> Please state more clearly ...

We have seen how utmp entries can be "fudged", left behind, with or
without access to group utmp.

Suppose a utmp entry is "fabricated" with "correct" PID etc, and ut_line
set to /tmp/x and /tmp/x made a symlink to the "correct" tty. That entry
will then be used by login; it will set PAM_TTY to /tmp/x, which will
fail securetty checks: resulting in a DoS.

Suppose we see pam_time or pam_group allowing something to (e.g.) tty0.
Then we "fabricate" a utmp entry with ut_line set to /tmp/tty0 and make
/tmp/tty0 point to our tty. Login will set PAM_TTY to /tmp/tty0 and PAM
will give us the goodies.

Please let me know if the above is unclear or insufficient.

Cheers, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Fri, 17 Apr 2009 20:48:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Fri, 17 Apr 2009 20:48:05 GMT) Full text and rfc822 format available.

Message #124 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org, nicolas.francois@centraliens.net
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Sat, 18 Apr 2009 06:45:26 +1000
Dear Nicolas,

> ... If you want to change the utmp handling ...

Will think about that, and get back to you in a few days.

Cheers, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Fri, 17 Apr 2009 21:23:30 GMT) Full text and rfc822 format available.

Acknowledgement sent to Nicolas François <nicolas.francois@centraliens.net>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Fri, 17 Apr 2009 21:23:30 GMT) Full text and rfc822 format available.

Message #129 received at 505071@bugs.debian.org (full text, mbox):

From: Nicolas François <nicolas.francois@centraliens.net>
To: psz@maths.usyd.edu.au
Cc: 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Fri, 17 Apr 2009 23:21:44 +0200
reopen 505071
thanks

Hello,

On Fri, Apr 17, 2009 at 07:55:23AM +1000, psz@maths.usyd.edu.au wrote:
> 
> Please see below. The patch of src/login.c is essential for security;
> I would prefer to use the libmisc/utmp.c patch also.

I changed src/login.c

in libmisc/utmp.c, I only sanitized ut_line.

Is it necessary to reset ut_id?
There isn't a single/standard way to define ut_id. If the caller of login
did not use the same algorithm (ut_line+3), then a new entry will be added
in utmp.
What would be the consequences of a wrong/forged ut_id?


If all fields are reset, then, yes we could remove the getutent() loop.

> Hmm... am now thinking that hostname (PAM_RHOST) may also be dodgy.

utent.ut_host is only used to set:
 * fromhost (only used for SYSLOG)
 * failent

A forged ut_host does not seems critical.

> I do not think I can re-open (would not know how, and I think am banned
> from doing control things since the kerfuffle in #299007).

I would be really surprised that you would be banned from the BTS (I only
heard about one case in the past).

Instructions are there:
http://www.debian.org/Bugs/server-control


Best Regards,
-- 
Nekral




Bug reopened, originator not changed. Request was from Nicolas François <nicolas.francois@centraliens.net> to control@bugs.debian.org. (Fri, 17 Apr 2009 21:27:14 GMT) Full text and rfc822 format available.

Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sat, 18 Apr 2009 06:36:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sat, 18 Apr 2009 06:36:02 GMT) Full text and rfc822 format available.

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

From: Paul Szabo <psz@maths.usyd.edu.au>
To: nicolas.francois@centraliens.net
Cc: 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Sat, 18 Apr 2009 16:28:57 +1000
Dear Nicolas,

Is shadow used on non-Linux or non-PAM machines: do you need to worry
about code within the likes of
#ifndef USE_PAM
...
#endif
#ifdef __linux__
#else
...
#endif
?

If yes, I guess the final endpwent() group in login.c should be moved up
to before setup_uid_gid().

Thanks, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sat, 18 Apr 2009 08:15:06 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sat, 18 Apr 2009 08:15:06 GMT) Full text and rfc822 format available.

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

From: Paul Szabo <psz@maths.usyd.edu.au>
To: nicolas.francois@centraliens.net
Cc: 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Sat, 18 Apr 2009 18:13:12 +1000
Dear Nicolas,

Sorry but it occurs to me I was not clear enough in my previous message.
My question of "is shadow used on non-Linux or non-PAM machines" relates
to utmp handling: does it need to handle those cases also?

Thanks, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sat, 18 Apr 2009 12:30:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Nicolas François <nicolas.francois@centraliens.net>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sat, 18 Apr 2009 12:30:02 GMT) Full text and rfc822 format available.

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

From: Nicolas François <nicolas.francois@centraliens.net>
To: Paul Szabo <psz@maths.usyd.edu.au>, 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Sat, 18 Apr 2009 14:28:18 +0200
Hello,

On Sat, Apr 18, 2009 at 04:28:57PM +1000, psz@maths.usyd.edu.au wrote:
> 
> Is shadow used on non-Linux or non-PAM machines: do you need to worry
> about code within the likes of
> #ifndef USE_PAM
> ...
> #endif
> #ifdef __linux__
> #else
> ...
> #endif
> ?

shadow is used on non-PAM machines.

I know that the previous maintainer used shadow on non-Linux machine
(Solaris or OpenSolaris?), although I'm not sure it's widely distributed
(other distributions / UNIXes have shadow tool equivalents)

Getting rid of __linux__ for utmp.c would be nice. I don't think it really
makes sense to make a difference based on linux. I would prefer to make a
difference based on the availability of fields in the utmp structure (e.g.
IIRC BSD system have no ut_id field, which could change the way entries
should be searched / registered).
One difference with linux might be that it supports both UTMP and UTMPX,
but they are equivalent so there is no need to do both UTMP and UTMPX.
In that case, I would just prefer to remove HAVE_UTMPX from linux builds,
but have a single source in utmp.c

> If yes, I guess the final endpwent() group in login.c should be moved up
> to before setup_uid_gid().

Agreed (even "if no"). The necessary structures should have been copied
before (this would have to be checked).

Best Regards,
-- 
Nekral




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sat, 18 Apr 2009 23:09:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sat, 18 Apr 2009 23:09:02 GMT) Full text and rfc822 format available.

Message #151 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org, nicolas.francois@centraliens.net
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Sun, 19 Apr 2009 09:06:38 +1000
Dear Nicolas,

> I changed src/login.c
> in libmisc/utmp.c, I only sanitized ut_line.

Thanks. New patches (replacing my previous ones) below, including the
move of endpwent(), and more verbose comments. The patch for login.c
is essential. The patch for utmp.c is mostly "as you wish"; there is a
missed endutent() there, am not sure whether correct or how important.

> utent.ut_host is only used to ...
> A forged ut_host does not seems critical.

Thanks, I agree.

Further comments:

Seems to me that su.c needs endgrent() for when SU_WHEEL_ONLY (this is
not a security issue).

In newgrp.c also, endpwent() needs to be moved to before the UID/GID
change (as was in login.c); I do not give patches. - Do these moves
warrant a DSA?

Cheers, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia



--- src/login.c.old	2009-04-17 07:00:50.000000000 +1000
+++ src/login.c	2009-04-19 08:54:00.000000000 +1000
@@ -449,6 +449,7 @@
 #else
 	int delay;
 	struct spwd *spwd = NULL;
+	struct spwd spent;
 #endif
 	/*
 	 * Some quick initialization.
@@ -464,6 +465,16 @@
 
 	amroot = (getuid () == 0);
 	Prog = Basename (argv[0]);
+	/*
+	 * PSz 18 Apr 09 Will this "work" unless we are root?
+	 * Should we test
+	if (geteuid() != 0) {
+		fprintf (stderr, _("%s: Cannot possibly work without effective root\n"), Prog);
+		exit (1);
+	}
+	 * ?
+	 * Debian /bin/login is not setuid, cannot work without amroot.
+	 */
 
 	process_flags (argc, argv);
 
@@ -477,9 +488,22 @@
 	 * even if your getty is broken, or if something corrupts utmp,
 	 * but users must "exec login" which will use the existing utmp
 	 * entry (will not overwrite remote hostname).  --marekm
+	 * PSz 18 Apr 09 As seen in
+	 *   http://bugs.debian.org/505071
+	 *   http://bugs.debian.org/505271
+	 * we cannot trust utmp: should set our own, new entry.
+	 * On Debian at least, we will never succeed without amroot
+	 * (should have exited already?), should never be picky...
 	 */
 	checkutmp (!amroot);
-	STRFCPY (tty, utent.ut_line);
+	/*
+	 * PSz 17 Apr 09 Though we may handle ut_line correctly (for Linux),
+	 * we should not trust PAM_TTY to its vagaries...
+	 *STRFCPY (tty, utent.ut_line);
+	 */
+	tmp = ttyname (0);
+	if (NULL == tmp) { tmp = "UNKNOWN"; }
+	STRFCPY (tty, tmp);
 #ifndef USE_PAM
 	is_console = console (tty);
 #endif
@@ -496,6 +520,12 @@
 		 * create the utmp entry and fill in ut_addr. 
 		 * gethostbyname() is not 100% reliable (the remote host may
 		 * be unknown, etc.).  --marekm
+		 * PSz 18 Apr 09 As commented above we cannot trust
+		 * utmp, should have set our new entry: should now fill
+		 * in ut_addr and ut_host. Even if gethostbyname() fails
+		 * because forward lookup is not provided, and whether
+		 * this lookup will get back to the original IP...
+		 * (should telnetd have refused for such hosts?).
 		 */
 		he = gethostbyname (hostname);
 		if (NULL != he) {
@@ -648,6 +678,11 @@
 	/*
 	 * hostname & tty are either set to NULL or their correct values,
 	 * depending on how much we know.
+	 * PSz 18 Apr 09 PAM_RHOST and PAM_TTY must be kept safe:
+	 * PAM_RHOST is used e.g. in pam_rhosts_auth. We only ever set
+	 * hostname from rflg or hflg not from ut_host, so is safe.
+	 * PAM_TTY is used e.g. in pam_time and pam_group. We set tty
+	 * from ttyname not from ut_line, so is safe.
 	 */
 	retcode = pam_set_item (pamh, PAM_RHOST, hostname);
 	PAM_FAIL_CHECK;
@@ -1176,6 +1211,19 @@
 		}
 	}
 
+/* PSz 18 Apr 09 Must do this before changing UID, before user access */
+/*
+ * Copy spwd so can use in agecheck later, though I do not think that
+ * endspent() would invalidate the previously obtained reference
+ */
+	spent = *spwd;
+	endpwent ();		/* stop access to password file */
+	endgrent ();		/* stop access to group file */
+	endspent ();		/* stop access to shadow passwd file */
+#ifdef	SHADOWGRP
+	endsgent ();		/* stop access to shadow group file */
+#endif
+
 	/* We call set_groups() above because this clobbers pam_groups.so */
 #ifndef USE_PAM
 	if (setup_uid_gid (&pwent, is_console))
@@ -1250,7 +1298,8 @@
 #endif
 			printf (".\n");
 		}
-		agecheck (spwd);
+		/* agecheck (spwd); */
+		agecheck (&spent);
 
 		mailcheck ();	/* report on the status of mail */
 #endif				/* !USE_PAM */
@@ -1269,12 +1318,15 @@
 	(void) signal (SIGHUP, SIG_DFL);	/* added this.  --marekm */
 	(void) signal (SIGINT, SIG_DFL);	/* default interrupt signal */
 
-	endpwent ();		/* stop access to password file */
-	endgrent ();		/* stop access to group file */
-	endspent ();		/* stop access to shadow passwd file */
-#ifdef	SHADOWGRP
-	endsgent ();		/* stop access to shadow group file */
-#endif
+/*
+ * PSz 18 Apr 09 Must do this before changing UID, before user access
+ *	endpwent ();		* stop access to password file *
+ *	endgrent ();		* stop access to group file *
+ *	endspent ();		* stop access to shadow passwd file *
+ * #ifdef	SHADOWGRP
+ *	endsgent ();		* stop access to shadow group file *
+ * #endif
+ */
 	if (0 == pwent.pw_uid) {
 		SYSLOG ((LOG_NOTICE, "ROOT LOGIN %s", fromhost));
 	} else if (getdef_bool ("LOG_OK_LOGINS")) {
--- libmisc/utmp.c.old	2008-11-23 10:56:10.000000000 +1100
+++ libmisc/utmp.c	2009-04-19 08:09:22.000000000 +1000
@@ -68,17 +68,27 @@
 		snprintf (full_tty, sizeof full_tty, "/dev/%s", tty);
 		tty = full_tty;
 	}
-
-	if (   (stat (tty, &by_name) != 0)
-	    || (fstat (STDIN_FILENO, &by_fd) != 0)) {
-		return false;
-	}
-
-	if (by_name.st_rdev != by_fd.st_rdev) {
+	/*
+	 * PSz 19 Apr 09 Might as well just compare to ttyname():
+	 * code below may be too complex (or unsafe/raceable).
+	 */
+	if (strcmp(tty,ttyname(0))) {
 		return false;
 	} else {
 		return true;
 	}
+/*
+ *	if (   (stat (tty, &by_name) != 0)
+ *	    || (fstat (STDIN_FILENO, &by_fd) != 0)) {
+ *		return false;
+ *	}
+ *
+ *	if (by_name.st_rdev != by_fd.st_rdev) {
+ *		return false;
+ *	} else {
+ *		return true;
+ *	}
+ */
 }
 
 /*
@@ -101,10 +111,29 @@
 {
 	char *line;
 	struct utmp *ut;
+	char reuseid[32];
 	pid_t pid = getpid ();
 
 	setutent ();
 
+	/*
+	 * PSz 19 Apr 09 I would like login.c to do its own utmp
+	 * handling, setting up utmp/wtmp from scratch and un-setting
+	 * them at exit. Did not do so because:
+	 * - do not have "working examples" in front of me, there are
+	 *   too many different systems to support (am lazy);
+	 * - change would require cooperation from telnetd etc, there
+	 *   would be duplication of entries until they stopped doing
+	 *   utmp.
+	 * We re-use ut_id (if found below), but set everything else
+	 * ourselves. Re-using ut_id avoids duplication of entries,
+	 * and is "safe". By re-writing utmp completely, we may lose
+	 * "important" data like login time or remote host: but only
+	 * when these were set already, and user done a new login;
+	 * this cannot happen when /bin/login is not setuid, cannot
+	 * happen on Debian.
+	 */
+
 	/* First, try to find a valid utmp entry for this process.  */
 	while ((ut = getutent ()) != NULL) {
 		if (   (ut->ut_pid == pid)
@@ -121,29 +150,56 @@
 
 	/* If there is one, just use it, otherwise create a new one. */
 	if (NULL != ut) {
-		utent = *ut;
+		strncpy (reuseid, utent.ut_id, sizeof reuseid);
 	} else {
 		if (picky) {
 			(void) puts (NO_UTENT);
 			exit (EXIT_FAILURE);
 		}
-		line = ttyname (0);
-		if (NULL == line) {
-			(void) puts (NO_TTY);
-			exit (EXIT_FAILURE);
-		}
-		if (strncmp (line, "/dev/", 5) == 0) {
-			line += 5;
-		}
-		memset ((void *) &utent, 0, sizeof utent);
-		utent.ut_type = LOGIN_PROCESS;
-		utent.ut_pid = pid;
-		strncpy (utent.ut_line, line, sizeof utent.ut_line);
-		/* XXX - assumes /dev/tty?? or /dev/pts/?? */
+		reuseid[0] = '\0';
+/*
+ *		line = ttyname (0);
+ *		if (NULL == line) {
+ *			(void) puts (NO_TTY);
+ *			exit (EXIT_FAILURE);
+ *		}
+ *		if (strncmp (line, "/dev/", 5) == 0) {
+ *			line += 5;
+ *		}
+ *		memset ((void *) &utent, 0, sizeof utent);
+ *		utent.ut_type = LOGIN_PROCESS;
+ *		utent.ut_pid = pid;
+ *		strncpy (utent.ut_line, line, sizeof utent.ut_line);
+ *		* XXX - assumes /dev/tty?? or /dev/pts/?? *
+ *		strncpy (utent.ut_id, utent.ut_line + 3, sizeof utent.ut_id);
+ *		strcpy (utent.ut_user, "LOGIN");
+ *		utent.ut_time = time (NULL);
+ */
+	}
+	line = ttyname (0);
+	if (NULL == line) {
+		(void) puts (NO_TTY);
+		exit (EXIT_FAILURE);
+	}
+	if (strncmp (line, "/dev/", 5) == 0) {
+		line += 5;
+	}
+	memset ((void *) &utent, 0, sizeof utent);
+	utent.ut_type = LOGIN_PROCESS;
+	utent.ut_pid = pid;
+	strncpy (utent.ut_line, line, sizeof utent.ut_line);
+	/* XXX - assumes /dev/tty?? or /dev/pts/?? */
+	if ('\0' != reuseid[0]) {
+		strncpy (utent.ut_id, reuseid, sizeof utent.ut_id);
+	} else {
 		strncpy (utent.ut_id, utent.ut_line + 3, sizeof utent.ut_id);
-		strcpy (utent.ut_user, "LOGIN");
-		utent.ut_time = time (NULL);
 	}
+	strcpy (utent.ut_user, "LOGIN");
+	utent.ut_time = time (NULL);
+
+	/* PSz 19 Apr 09 Seems we missed endutent()...
+	 * not important, would be done later in setutmp */
+	endutent ();
 }
 
 #elif defined(LOGIN_PROCESS)
@@ -438,6 +494,10 @@
 	utxent = utxline;
 	utent = utline;
 
+	/* PSz 19 Apr 09 Seems we missed endutent()... */
+	endutxent ();
+	endutent ();
+
 	return err;
 }
 




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sun, 19 Apr 2009 05:27:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sun, 19 Apr 2009 05:27:02 GMT) Full text and rfc822 format available.

Message #156 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org, nicolas.francois@centraliens.net
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Sun, 19 Apr 2009 15:23:44 +1000
Dear Nicolas,

> ... Do these [entspent] moves warrant a DSA?

Maybe not. Testing, it seems that getspnam() does not leave an open file
descriptor, but setspent() would. (I do not know what /bin/login does
exactly.)

Cheers, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sun, 19 Apr 2009 13:00:09 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sun, 19 Apr 2009 13:00:09 GMT) Full text and rfc822 format available.

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

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org, nicolas.francois@centraliens.net
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Sun, 19 Apr 2009 22:53:50 +1000
Dear Nicolas,

I wrote:

>> ... Do these [entspent] moves warrant a DSA?
> Maybe not. Testing, it seems that getspnam() does not leave an open file
> descriptor, but setspent() would. (I do not know what /bin/login does
> exactly.)

Now testing, seems that just before the endspent() etc calls, login has
a file descriptor open on /etc/passwd but does not have one for
/etc/shadow. Seems there is no security issue. (Is this weird behaviour
in libc?)

Since I do not know how getspent() or endspent() work, I now wonder
whether chunks of /etc/shadow (other than the line for right user) could
be found in process memory, before or after endspent(). Have so far
failed to read /proc/self/mem in my test program, and wonder if that
feature works in my kernel...

Cheers, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sun, 19 Apr 2009 13:48:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Nicolas François <nicolas.francois@centraliens.net>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sun, 19 Apr 2009 13:48:05 GMT) Full text and rfc822 format available.

Message #166 received at 505071@bugs.debian.org (full text, mbox):

From: Nicolas François <nicolas.francois@centraliens.net>
To: psz@maths.usyd.edu.au, 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Sun, 19 Apr 2009 15:47:11 +0200
Hello,

On Sun, Apr 19, 2009 at 09:06:38AM +1000, psz@maths.usyd.edu.au wrote:
> 
> Thanks. New patches (replacing my previous ones) below, including the
> move of endpwent(), and more verbose comments. The patch for login.c
> is essential. The patch for utmp.c is mostly "as you wish"; there is a
> missed endutent() there, am not sure whether correct or how important.

I've added the missing endutent / endutxent.

> Seems to me that su.c needs endgrent() for when SU_WHEEL_ONLY (this is
> not a security issue).
> 
> In newgrp.c also, endpwent() needs to be moved to before the UID/GID
> change (as was in login.c); I do not give patches. - Do these moves
> warrant a DSA?

I don't think there is a need for a DSA.
With a glibc, the user/group databases are opened with the O_CLOEXEC flag.
The file descriptor will not be available to the exec'ed processes.

Regarding login.c, I think it is still fine to "close" the files (with
endspent et al.), in case another implementation is used.

This still have to be checked / done.



Here are some comments on the patches:

>  	amroot = (getuid () == 0);
>  	Prog = Basename (argv[0]);
> +	/*
> +	 * PSz 18 Apr 09 Will this "work" unless we are root?
> +	 * Should we test
> +	if (geteuid() != 0) {
> +		fprintf (stderr, _("%s: Cannot possibly work without effective root\n"), Prog);
> +		exit (1);
> +	}
> +	 * ?
> +	 * Debian /bin/login is not setuid, cannot work without amroot.
> +	 */

Debian used to have a setuid /bin/login. Some people may still want to
have it setuid, so I would prefer to avoid requiring to run login as root.

>  	 * even if your getty is broken, or if something corrupts utmp,
>  	 * but users must "exec login" which will use the existing utmp
>  	 * entry (will not overwrite remote hostname).  --marekm
> +	 * PSz 18 Apr 09 As seen in
> +	 *   http://bugs.debian.org/505071
> +	 *   http://bugs.debian.org/505271
> +	 * we cannot trust utmp: should set our own, new entry.
> +	 * On Debian at least, we will never succeed without amroot
> +	 * (should have exited already?), should never be picky...
>  	 */
>  	checkutmp (!amroot);

My point would be: In case login is setuid, shall we require that it is
called with "exec login". That would be my preference.

Then, how to enforce this? (note the point is not to enforce this is all
cases, but to make sure regular user will not leave a opened session).

It might be better to change the login/utmp logic to:

	if (!amroot)
		checkutmp (); /* Make sure there is a valid enough entry
		               * in utmp */

	getutmp (&utent);      /* get the sanitized existing utmp entry or
	                        * default entry */
#if USE_UTMPX
	getutmpx (&utxent);
#endif

	...

	setutmp (&utent);
#if USE_UTMPX
	setutmpx (&utxent);
#endif

That could remove the need for __linux__ in utmp.c and would remove the
global utent and utxent.

>  	/*
>  	 * hostname & tty are either set to NULL or their correct values,
>  	 * depending on how much we know.
> +	 * PSz 18 Apr 09 PAM_RHOST and PAM_TTY must be kept safe:
> +	 * PAM_RHOST is used e.g. in pam_rhosts_auth. We only ever set
> +	 * hostname from rflg or hflg not from ut_host, so is safe.
> +	 * PAM_TTY is used e.g. in pam_time and pam_group. We set tty
> +	 * from ttyname not from ut_line, so is safe.
>  	 */

I agree on putting a comment here to avoid setting PAM_RHOST differently in
future.

(The other comments were not meant to appear in the code, right?)

> --- libmisc/utmp.c.old	2008-11-23 10:56:10.000000000 +1100
> +++ libmisc/utmp.c	2009-04-19 08:09:22.000000000 +1000
> @@ -68,17 +68,27 @@
>  		snprintf (full_tty, sizeof full_tty, "/dev/%s", tty);
>  		tty = full_tty;
>  	}
> -
> -	if (   (stat (tty, &by_name) != 0)
> -	    || (fstat (STDIN_FILENO, &by_fd) != 0)) {
> -		return false;
> -	}
> -
> -	if (by_name.st_rdev != by_fd.st_rdev) {
> +	/*
> +	 * PSz 19 Apr 09 Might as well just compare to ttyname():
> +	 * code below may be too complex (or unsafe/raceable).
> +	 */
> +	if (strcmp(tty,ttyname(0))) {
>  		return false;
>  	} else {
>  		return true;
>  	}

Let's do it, but I'm still unsure about the consequences of this.
i.e. was it a feature to be able to have a getty using a device not in
/dev

Let's see if somebody notices and complains.

Best Regards,
-- 
Nekral




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sun, 19 Apr 2009 15:48:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Nicolas François <nicolas.francois@centraliens.net>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sun, 19 Apr 2009 15:48:02 GMT) Full text and rfc822 format available.

Message #171 received at 505071@bugs.debian.org (full text, mbox):

From: Nicolas François <nicolas.francois@centraliens.net>
To: psz@maths.usyd.edu.au
Cc: 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Sun, 19 Apr 2009 17:47:35 +0200
On Sun, Apr 19, 2009 at 10:53:50PM +1000, psz@maths.usyd.edu.au wrote:
> 
> Now testing, seems that just before the endspent() etc calls, login has
> a file descriptor open on /etc/passwd but does not have one for
> /etc/shadow. Seems there is no security issue. (Is this weird behaviour
> in libc?)

There are no call to setspent or getspent in shadow, so I'm not really
surprised.

> Since I do not know how getspent() or endspent() work, I now wonder
> whether chunks of /etc/shadow (other than the line for right user) could
> be found in process memory, before or after endspent(). Have so far
> failed to read /proc/self/mem in my test program, and wonder if that
> feature works in my kernel...

Only getspnam would have to be checked.
The problem probably depends on the libc.

Best Regards,
-- 
Nekral




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sun, 19 Apr 2009 21:46:13 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sun, 19 Apr 2009 21:46:37 GMT) Full text and rfc822 format available.

Message #176 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org, nicolas.francois@centraliens.net
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Mon, 20 Apr 2009 07:35:57 +1000
Dear Nicolas,

> I've added the missing endutent / endutxent.

Thanks.

>> Seems to me that su.c needs endgrent() for when SU_WHEEL_ONLY (this is
>> not a security issue).
>> 
>> In newgrp.c also, endpwent() needs to be moved to before the UID/GID
>> change (as was in login.c); I do not give patches. - Do these moves
>> warrant a DSA?
>
> I don't think there is a need for a DSA.
> With a glibc, the user/group databases are opened with the O_CLOEXEC flag.
> The file descriptor will not be available to the exec'ed processes.
>
> Regarding login.c, I think it is still fine to "close" the files (with
> endspent et al.), in case another implementation is used.
>
> This still have to be checked / done.

Once you change UID, the user may interact with the process; the file
descriptor must be closed before the user gets a chance. Anyway you must
not rely on O_CLOEXEC that you did not set yourself. (Admittedly, the
window of opportunity between setuid and endspent is small, and it may
be hard to interact with a process that just gave up privileges so is
not yet "fully" user. But, races are easy to win.)

There may be no need for DSA, because in fact login never had a file
descriptor for /etc/shadow. Other (non-Debian) machines may have a
different libc and not so lucky.

> Here are some comments on the patches:
>
>>  	amroot = (getuid () == 0);
>>  	Prog = Basename (argv[0]);
>> +	/*
>> +	 * PSz 18 Apr 09 Will this "work" unless we are root?
>> +	 * Should we test
>> +	if (geteuid() != 0) {
>> +		fprintf (stderr, _("%s: Cannot possibly work without effective root\n"), Prog);
>> +		exit (1);
>> +	}
>> +	 * ?
>> +	 * Debian /bin/login is not setuid, cannot work without amroot.
>> +	 */
>
> Debian used to have a setuid /bin/login. Some people may still want to
> have it setuid, so I would prefer to avoid requiring to run login as root.

The above would check EUID, would allow that (if uncommented).

> My point would be: In case login is setuid, shall we require that it is
> called with "exec login". That would be my preference.
>
> Then, how to enforce this? (note the point is not to enforce this is all
> cases, but to make sure regular user will not leave a opened session).
>
> It might be better to change the login/utmp logic to:
>
> 	if (!amroot)
> 		checkutmp (); /* Make sure there is a valid enough entry
> 		               * in utmp */
>
> 	getutmp (&utent);      /* get the sanitized existing utmp entry or
> 	                        * default entry */
> #if USE_UTMPX
> 	getutmpx (&utxent);
> #endif
>
> 	...
>
> 	setutmp (&utent);
> #if USE_UTMPX
> 	setutmpx (&utxent);
> #endif
>
> That could remove the need for __linux__ in utmp.c and would remove the
> global utent and utxent.

Sorry but I do not know. (I guess that would be OK, with plenty of
checking/sanitizing after getutmp(), not assuming that it would return
the just-checked entry, but double-checking to make sure.)

>>  	/*
>>  	 * hostname & tty are either set to NULL or their correct values,
>>  	 * depending on how much we know.
>> +	 * PSz 18 Apr 09 PAM_RHOST and PAM_TTY must be kept safe:
>> +	 * PAM_RHOST is used e.g. in pam_rhosts_auth. We only ever set
>> +	 * hostname from rflg or hflg not from ut_host, so is safe.
>> +	 * PAM_TTY is used e.g. in pam_time and pam_group. We set tty
>> +	 * from ttyname not from ut_line, so is safe.
>>  	 */
>
> I agree on putting a comment here to avoid setting PAM_RHOST differently in
> future.

Thanks.

> (The other comments were not meant to appear in the code, right?)

I do not know. I like comments, for when (others?) reviewing the code.

>> --- libmisc/utmp.c.old	2008-11-23 10:56:10.000000000 +1100
>> +++ libmisc/utmp.c	2009-04-19 08:09:22.000000000 +1000
>> @@ -68,17 +68,27 @@
>>  		snprintf (full_tty, sizeof full_tty, "/dev/%s", tty);
>>  		tty = full_tty;
>>  	}
>> -
>> -	if (   (stat (tty, &by_name) != 0)
>> -	    || (fstat (STDIN_FILENO, &by_fd) != 0)) {
>> -		return false;
>> -	}
>> -
>> -	if (by_name.st_rdev != by_fd.st_rdev) {
>> +	/*
>> +	 * PSz 19 Apr 09 Might as well just compare to ttyname():
>> +	 * code below may be too complex (or unsafe/raceable).
>> +	 */
>> +	if (strcmp(tty,ttyname(0))) {
>>  		return false;
>>  	} else {
>>  		return true;
>>  	}
>
> Let's do it, but I'm still unsure about the consequences of this.
> i.e. was it a feature to be able to have a getty using a device not in
> /dev

Thanks.

>> Now testing, seems that just before the endspent() etc calls, login has
>> a file descriptor open on /etc/passwd but does not have one for
>> /etc/shadow. Seems there is no security issue. (Is this weird behaviour
>> in libc?)
>
> There are no call to setspent or getspent in shadow, so I'm not really
> surprised.

But... I do not see setpwent() either within /bin/login sources (though
is in the binary), and /etc/passwd is open; the man pages suggest that
both /etc/passwd and /etc/shadow should be open "internally" until
endXXent, even without explicit setXXent.

>> Since I do not know how getspent() or endspent() work, I now wonder
>> whether chunks of /etc/shadow (other than the line for right user) could
>> be found in process memory, before or after endspent(). Have so far
>> failed to read /proc/self/mem in my test program, and wonder if that
>> feature works in my kernel...
>
> Only getspnam would have to be checked.
> The problem probably depends on the libc.

Yes. Would be nice for getspnam() to scrub memory areas; surely
endspent() should. (Tried looking in libc sources, and got confused...)
Since login uses these, it should make sure they do the "right thing"
(maybe by doing the memory scrubbing itself).

Cheers, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Tue, 21 Apr 2009 08:27:09 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Tue, 21 Apr 2009 08:27:09 GMT) Full text and rfc822 format available.

Message #181 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org, nicolas.francois@centraliens.net
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Tue, 21 Apr 2009 18:24:06 +1000
I now got my /proc/self/mem-reading test program to work, and it
seems that neither getspnam() nor getspent() leave "garbage" in
memory: no security risk. - Am wondering if the call to endspent()
is needed, when apparently there is no setspent() anywhere; still,
it can do no harm (other than to confuse fools like me).

Cheers, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Tue, 21 Apr 2009 22:54:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Nicolas François <nicolas.francois@centraliens.net>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Tue, 21 Apr 2009 22:54:02 GMT) Full text and rfc822 format available.

Message #186 received at 505071@bugs.debian.org (full text, mbox):

From: Nicolas François <nicolas.francois@centraliens.net>
To: psz@maths.usyd.edu.au, 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Wed, 22 Apr 2009 00:52:06 +0200
Hello,

I've committed a new utmp handling for login.
I also hope it fixes other concerns raised in the thread.

I would appreciate if you could have a look:
http://svn.debian.org/viewsvn/pkg-shadow/upstream/trunk/

Thanks in advance,
-- 
Nekral 




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Wed, 22 Apr 2009 00:42:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Wed, 22 Apr 2009 00:42:02 GMT) Full text and rfc822 format available.

Message #191 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org, nicolas.francois@centraliens.net
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Wed, 22 Apr 2009 10:41:21 +1000
Dear Nicolas,

Had a quick look at src/login.c and libmisc/utmp.c, and see the "plain"
(not security) issues below. - The enspent() block should be moved up
in newgrp.c also.

Cheers, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia


--- login.c.bak	2009-04-22 09:34:08.000000000 +1000
+++ login.c	2009-04-22 10:32:53.000000000 +1000
@@ -533,6 +533,7 @@
 		(void) puts (_("No utmp entry.  You must exec \"login\" from the lowest level \"sh\""));
 		exit (1);
 	}
+	/* Take note that utent may be NULL after this... */
 
 	tmptty = ttyname (0);
 	if (NULL == tmptty) {
@@ -625,15 +626,13 @@
 
 	if (rflg || hflg) {
 		cp = hostname;
-	} else {
 #ifdef	HAVE_STRUCT_UTMP_UT_HOST
-		if ('\0' != utent->ut_host[0]) {
-			cp = utent->ut_host;
-		} else
+/* Take care with utent that may be NULL */
+	} else if (utent != NULL && '\0' != utent->ut_host[0]) {
+		cp = utent->ut_host;
 #endif				/* HAVE_STRUCT_UTMP_UT_HOST */
-		{
-			cp = "";
-		}
+	} else {
+		cp = "";
 	}
 
 	if ('\0' != *cp) {
--- utmp.c.bak	2009-04-22 09:34:34.000000000 +1000
+++ utmp.c	2009-04-22 10:32:55.000000000 +1000
@@ -93,6 +93,7 @@
  *	Return NULL if no entries exist in utmp for the current process.
  */
 struct utmp *get_current_utmp (void)
+/* May return NULL. Should it return an "empty" (zeroed) something instead? */
 {
 	struct utmp *ut;
 	struct utmp *ret = NULL;
@@ -109,6 +110,13 @@
 		    && (   (LOGIN_PROCESS == ut->ut_type)
 		        || (USER_PROCESS  == ut->ut_type))
 #endif
+/*
+ * We use HAVE_STRUCT_UTMP_UT_ID above (surely utmp has that??!!)
+ * Should we use
+ *   HAVE_STRUCT_UTMP_UT_LINE below, or use
+ *   HAVE_STRUCT_UTMP_UT_PID above
+ * (are defined anywhere?)
+ */
 		    /* A process may have failed to close an entry
 		     * Check if this entry refers to the current tty */
 		    && is_my_tty (ut->ut_line)) {
@@ -117,6 +125,7 @@
 	}
 
 	if (NULL != ut) {
+		/* malloc, or xmalloc as everywhere else? */
 		ret = malloc (sizeof (*ret));
 		memcpy (ret, ut, sizeof (*ret));
 	}
@@ -200,6 +209,7 @@
 
 	assert (NULL != name);
 	assert (NULL != line);
+	/* BUG: utent in login.c may be NULL (in prepare_utmpx also) */
 	assert (NULL != ut);
 
 
@@ -234,6 +244,7 @@
 	strncpy (utent->ut_line, line,      sizeof (utent->ut_line));
 #ifdef HAVE_STRUCT_UTMP_UT_ID
 	strncpy (utent->ut_id,   ut->ut_id, sizeof (utent->ut_id));
+	/* BUG: set "correct" ut_id based on "line", in case we did not get any (in prepare_utmpx also) */
 #endif				/* HAVE_STRUCT_UTMP_UT_ID */
 #ifdef HAVE_STRUCT_UTMP_UT_NAME
 	strncpy (utent->ut_name, name,      sizeof (utent->ut_name));




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sat, 25 Apr 2009 12:03:05 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sat, 25 Apr 2009 12:03:05 GMT) Full text and rfc822 format available.

Message #196 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org, nicolas.francois@centraliens.net
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Sat, 25 Apr 2009 21:57:41 +1000
Dear Nicolas,

Comments on (snippet of code comments, and your words):

>>  	 * but users must "exec login" which will use the existing utmp
>>  	 * entry (will not overwrite remote hostname).  --marekm
>
> My point would be: In case login is setuid, shall we require that it is
> called with "exec login". That would be my preference.
>
> Then, how to enforce this? (note the point is not to enforce this is all
> cases, but to make sure regular user will not leave a opened session).

If login is not setuid then it cannot be used in that fashion anyway.
So this is not about current Debian or Ubuntu.

In my experience, if users want something, they will get it with some
"worse" means. If we do not let them run login directly, then they will
run telnet instead which is probably much more wasteful; they will not
use "exec login" or su, because are not familiar with those. There is
not much point in protecting users from own foolishness.

But mainly, "exec login" cannot possibly work in a PAM environment, but
will fail/die and "lose" the user session; users should not be tricked
into doing that. Presumably the user logged in with login (e.g. telnet,
may not apply for ssh or xterm); then login done a fork before running
the shell; any utmp entry refers to the PID of the parent login.
Incidentally, seems rather wasteful to have login waiting to
pam_close_session and telnetd waiting to clear utmp.

Cheers, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sun, 26 Apr 2009 16:42:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Nicolas François <nicolas.francois@centraliens.net>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sun, 26 Apr 2009 16:42:02 GMT) Full text and rfc822 format available.

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

From: Nicolas François <nicolas.francois@centraliens.net>
To: psz@maths.usyd.edu.au
Cc: 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Sun, 26 Apr 2009 18:38:17 +0200
On Sat, Apr 25, 2009 at 09:57:41PM +1000, psz@maths.usyd.edu.au wrote:
> Dear Nicolas,
> 
> Comments on (snippet of code comments, and your words):
> 
> >>  	 * but users must "exec login" which will use the existing utmp
> >>  	 * entry (will not overwrite remote hostname).  --marekm
> >
> > My point would be: In case login is setuid, shall we require that it is
> > called with "exec login". That would be my preference.
> >
> > Then, how to enforce this? (note the point is not to enforce this is all
> > cases, but to make sure regular user will not leave a opened session).
> 
> If login is not setuid then it cannot be used in that fashion anyway.
> So this is not about current Debian or Ubuntu.

I would prefer to keep this feature, even if not used on Debian and
Ubuntu. Some users may be relying on it.

> In my experience, if users want something, they will get it with some
> "worse" means. If we do not let them run login directly, then they will
> run telnet instead which is probably much more wasteful; they will not
> use "exec login" or su, because are not familiar with those. There is
> not much point in protecting users from own foolishness.



> But mainly, "exec login" cannot possibly work in a PAM environment, but
> will fail/die and "lose" the user session; users should not be tricked
> into doing that. Presumably the user logged in with login (e.g. telnet,
> may not apply for ssh or xterm); then login done a fork before running
> the shell; any utmp entry refers to the PID of the parent login.

I don't get your point.
At least when login was setuid on debian, "exec login" used to work on PAM
environments.

> Incidentally, seems rather wasteful to have login waiting to
> pam_close_session and telnetd waiting to clear utmp.

What would you recommend?

There are some UNIX where pam_close_session has to be done by the caller
of login. But this requires a general policy to be applied consistently on
many packages. I don't think this will be possible to enforce this on the
tools used in Linux. So having login wait to pam_close_session is
currently the only solution.

Regarding UTMP, I don't know if it would be valid to close the entry on
exit. Even if login did it, I'm also not sure telnetd wouldn't have to do
it again anyway (there are other login implementation).

Best Regards,
-- 
Nekral




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sun, 26 Apr 2009 17:30:06 GMT) Full text and rfc822 format available.

Acknowledgement sent to Nicolas François <nicolas.francois@centraliens.net>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sun, 26 Apr 2009 17:30:06 GMT) Full text and rfc822 format available.

Message #206 received at 505071@bugs.debian.org (full text, mbox):

From: Nicolas François <nicolas.francois@centraliens.net>
To: Paul Szabo <psz@maths.usyd.edu.au>, 505071@bugs.debian.org
Subject: Re: Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Sun, 26 Apr 2009 19:13:00 +0200
Hello,

On Wed, Apr 22, 2009 at 10:41:21AM +1000, psz@maths.usyd.edu.au wrote:
> 
> Had a quick look at src/login.c and libmisc/utmp.c, and see the "plain"
> (not security) issues below. - The enspent() block should be moved up
> in newgrp.c also.

Thanks for checking this.
They should be fixed now.

Best Regards,
-- 
Nekral




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Sun, 26 Apr 2009 20:54:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Sun, 26 Apr 2009 20:54:03 GMT) Full text and rfc822 format available.

Message #211 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: nicolas.francois@centraliens.net
Cc: 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Mon, 27 Apr 2009 06:51:11 +1000
Dear Nicolas,

>> But mainly, "exec login" cannot possibly work in a PAM environment, but
>> will fail/die and "lose" the user session; users should not be tricked
>> into doing that. Presumably the user logged in with login (e.g. telnet,
>> may not apply for ssh or xterm); then login done a fork before running
>> the shell; any utmp entry refers to the PID of the parent login.
>
> I don't get your point.
> At least when login was setuid on debian, "exec login" used to work on PAM
> environments.

Please do the simple test (which "works" regardless whether login is
setuid or not):

anyone@anywhere:~$ /usr/bin/telnet bari
Trying 129.78.69.145...
Connected to bari.maths.usyd.edu.au.
Escape character is '^]'.
Debian GNU/Linux 4.0
bari.maths.usyd.edu.au login: psz
Password: 
...
psz@bari:~$ /bin/login
No utmp entry.  You must exec "login" from the lowest level "sh"
psz@bari:~$ exec /bin/login
No utmp entry.  You must exec "login" from the lowest level "sh"
Connection closed by foreign host.
anyone@anywhere:~$ 

Surely your memory of "used to work" is wrong?

Cheers, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Mon, 27 Apr 2009 00:06:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Nicolas François <nicolas.francois@centraliens.net>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Mon, 27 Apr 2009 00:06:02 GMT) Full text and rfc822 format available.

Message #216 received at 505071@bugs.debian.org (full text, mbox):

From: Nicolas François <nicolas.francois@centraliens.net>
To: psz@maths.usyd.edu.au
Cc: 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Mon, 27 Apr 2009 02:03:48 +0200
On Mon, Apr 27, 2009 at 06:51:11AM +1000, psz@maths.usyd.edu.au wrote:
> Dear Nicolas,
> 
> >> But mainly, "exec login" cannot possibly work in a PAM environment, but
> >> will fail/die and "lose" the user session; users should not be tricked
> >> into doing that. Presumably the user logged in with login (e.g. telnet,
> >> may not apply for ssh or xterm); then login done a fork before running
> >> the shell; any utmp entry refers to the PID of the parent login.
> >
> > I don't get your point.
> > At least when login was setuid on debian, "exec login" used to work on PAM
> > environments.
> 
> Please do the simple test (which "works" regardless whether login is
> setuid or not):
> 
> anyone@anywhere:~$ /usr/bin/telnet bari
[...]
> 
> Surely your memory of "used to work" is wrong?

It works when you use a tool which sets UTMP before it provides the shell.
telnetd does not, but for example xterm does.

Best Regards,
-- 
Nekral




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Mon, 27 Apr 2009 00:27:04 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Mon, 27 Apr 2009 00:27:04 GMT) Full text and rfc822 format available.

Message #221 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: nicolas.francois@centraliens.net
Cc: 505071@bugs.debian.org
Subject: Re: [Pkg-shadow-devel] Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Mon, 27 Apr 2009 10:25:58 +1000
Dear Nicolas,

> It works when you use a tool which sets UTMP before it provides the shell.
> telnetd does not, but for example xterm does.

More to the point: login does not. To belabour the point: if you "come"
from login (e.g. via telnet), then login provides a utmp entry, but that
refers to the PID of login which is the parent of your shell; using
"exec login" does not "see" that (correctly set) utmp entry.

I checked and "exec login" does not work from an ssh login either.
Yes, I know it would work from an xterm.

In other words: your favourite tool is (should be) login; it does
provide an utmp entry, but not in a way that it can use it itself.
We could say that login is self-contradictory, inconsistent.

Cheers, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Information forwarded to debian-bugs-dist@lists.debian.org, Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>:
Bug#505071; Package login. (Mon, 27 Apr 2009 02:48:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Paul Szabo <psz@maths.usyd.edu.au>:
Extra info received and forwarded to list. Copy sent to Shadow package maintainers <pkg-shadow-devel@lists.alioth.debian.org>. (Mon, 27 Apr 2009 02:48:02 GMT) Full text and rfc822 format available.

Message #226 received at 505071@bugs.debian.org (full text, mbox):

From: Paul Szabo <psz@maths.usyd.edu.au>
To: 505071@bugs.debian.org, nicolas.francois@centraliens.net
Subject: Re: Bug#505071: closed ... fixed in shadow 1:4.1.3-1
Date: Mon, 27 Apr 2009 12:40:48 +1000
Dear Nicolas,

> They should be fixed now.

Thanks, things seem OK now.

Cheers, Paul

Paul Szabo   psz@maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia




Reply sent to Nicolas François <nicolas.francois@centraliens.net>:
You have taken responsibility. (Sat, 18 Jul 2009 17:06:06 GMT) Full text and rfc822 format available.

Notification sent to Paul Szabo <psz@maths.usyd.edu.au>:
Bug acknowledged by developer. (Sat, 18 Jul 2009 17:06:06 GMT) Full text and rfc822 format available.

Message #231 received at 505071-done@bugs.debian.org (full text, mbox):

From: Nicolas François <nicolas.francois@centraliens.net>
To: 505071-done@bugs.debian.org
Subject: Re: Bug#505071: login tty mis-determination (see bug#332198)
Date: Sat, 18 Jul 2009 19:03:35 +0200
Version: 1:4.1.4.1-1

On Mon, Apr 27, 2009 at 12:40:48PM +1000, Paul Szabo wrote:
> 
> > They should be fixed now.
> 
> Thanks, things seem OK now.

Paul, I'm closing now.
This seems to be fixed.
There were other topics raised in this bug. I also think they were dealt
with.

Reading the backlog (last mail "Mon, 27 Apr 2009 10:25:58 +1000"),
login should also be fixed regarding the last issue as it reports to UTMP
the right PID.

Best Regards,
-- 
Nekral




Bug archived. Request was from Debbugs Internal Request <owner@bugs.debian.org> to internal_control@bugs.debian.org. (Sun, 16 Aug 2009 07:35:32 GMT) Full text and rfc822 format available.

Send a report that this bug log contains spam.


Debian bug tracking system administrator <owner@bugs.debian.org>. Last modified: Mon Apr 21 10:21:10 2014; Machine Name: buxtehude.debian.org

Debian Bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.