openssh bugs - Dec 2007 - [Bug 1419] New: Fix PTY handling on Mac OS X

https://bugzilla.mindrot.org/show_bug.cgi?id=1419

           Summary: Fix PTY handling on Mac OS X
    Classification: Unclassified
           Product: Portable OpenSSH
           Version: 4.7p1
          Platform: Other
        OS/Version: Mac OS X
            Status: NEW
          Severity: normal
          Priority: P2
         Component: ssh
        AssignedTo: bitbucket at mindrot.org
        ReportedBy: vgiffin at apple.com


Created an attachment (id=1415)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1415)
Corrects PTY handling.

Attached are two patches for building OpenSSH 4.7p1 on Mac OS X.

These patches correct PTY handling on Mac OS X.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419





--- Comment #1 from Disco Vince Giffin <vgiffin at apple.com>  2007-12-21
15:45:38 ---
Created an attachment (id=1416)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1416)
A PTY permission fix.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419


Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at zip.com.au




--- Comment #2 from Darren Tucker <dtucker at zip.com.au>  2007-12-31
21:56:19 ---
Could you please explain why these are needed?  Does ttyname not work
on OS X?  And why do you want to not change the ownership of the ptys?

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419





--- Comment #3 from Disco Vince Giffin <vgiffin at apple.com>  2008-01-08
05:14:35 ---
(In reply to comment #2)
> Could you please explain why these are needed?  Does ttyname not work
> on OS X?  And why do you want to not change the ownership of the ptys?
I've spoken with the author of these patches and done some testing.

The first patch (for ttyname) is no longer needed.

The second (permission fix) patch bypasses setowner.  We don't need to
call setowner because the ownership and permissions are already setup
when we allocate a cloning pty.  This patch was added (and is needed)
because when thing close down, and the last file descriptor to the pty
is closed, the kernel removes it from /dev.  So when sshd (thinking the
pty is still there) tries to reset the ownership and permissions, it
gets an error because it isn't there anymore.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419





--- Comment #4 from Darren Tucker <dtucker at zip.com.au>  2008-01-08
15:07:59 ---
(In reply to comment #3)
> I've spoken with the author of these patches and done some testing.
> 
> The first patch (for ttyname) is no longer needed.
OK then I'll ignore that one.
> The second (permission fix) patch bypasses setowner.  We don't need to
> call setowner because the ownership and permissions are already setup
> when we allocate a cloning pty.
That means pty_setowner is a no-op?  So is the ifdef even required?

There's another call to pty_setowner in monitor.c which will be called
when privsep is enabled (the default) so the ifdef around pty_setowner
will only have an effect if privsep is off or it's root logging in.
> This patch was added (and is needed)
> because when thing close down, and the last file descriptor to the pty
> is closed, the kernel removes it from /dev.  So when sshd (thinking the
> pty is still there) tries to reset the ownership and permissions, it
> gets an error because it isn't there anymore.
OK that sounds reasonable but it probably belongs in sshpty.c with the
rest of the pty handling code.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419


Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
Attachment #1416 is|0                           |1
           obsolete|                            |
Attachment #1415 is|0                           |1
           obsolete|                            |




--- Comment #5 from Darren Tucker <dtucker at zip.com.au>  2008-01-08
15:16:30 ---
Created an attachment (id=1439)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1439)
Make pty_release a no-op on OS X

If my assumptions are correct, this patch should do the right thing on
OS X.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419


Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org




--- Comment #6 from Damien Miller <djm at mindrot.org>  2008-01-20
06:55:14 ---
Where/how does __APPLE_PRIVPTY__ set set?

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419





--- Comment #7 from Disco Vince Giffin <vgiffin at apple.com>  2008-01-22
07:52:29 ---
(In reply to comment #5)
> Created an attachment (id=1439)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1439)
[details]
> Make pty_release a no-op on OS X
> 
> If my assumptions are correct, this patch should do the right thing on
> OS X.
This patch does work for Mac OS X (10.5.1).

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419





--- Comment #8 from Disco Vince Giffin <vgiffin at apple.com>  2008-01-22
07:56:01 ---
(In reply to comment #6)
> Where/how does __APPLE_PRIVPTY__ set set?
This is set (or not) with an Extra_Configure_Flags in our top level
Makefile, depending on what platform we are building for.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419





--- Comment #9 from Darren Tucker <dtucker at zip.com.au>  2008-01-22
15:24:34 ---
(In reply to comment #8)
> (In reply to comment #6)
> > Where/how does __APPLE_PRIVPTY__ set set?
> 
> This is set (or not) with an Extra_Configure_Flags in our top level
> Makefile, depending on what platform we are building for.
So that means that people building OpenSSH from source themselves on
those platforms will not have the #define and thus get the wrong
behaviour?

How can one reliably tell which platforms/versions need that need this?

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419





--- Comment #10 from Disco Vince Giffin <vgiffin at apple.com>  2008-01-23
10:07:12 ---
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #6)
> > > Where/how does __APPLE_PRIVPTY__ set set?
> > 
> > This is set (or not) with an Extra_Configure_Flags in our top level
> > Makefile, depending on what platform we are building for.
> 
> So that means that people building OpenSSH from source themselves on
> those platforms will not have the #define and thus get the wrong
> behaviour?
> 
> How can one reliably tell which platforms/versions need that need this?
Good question.  We could use AvailabilityMacros:

#ifdef __APPLE__
#include <AvailabilityMacros.h>
#if (MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_5)
#define __APPLE_PRIVPTY__
#endif
#endif

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419


Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |1481




-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419


Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1439|0                           |1
        is obsolete|                            |
   Attachment #1593|                            |ok?
               Flag|                            |




--- Comment #11 from Damien Miller <djm at mindrot.org>  2009-01-21
16:11:52 ---
Created an attachment (id=1593)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1593)
pty_release => noop with feature test macro

Like this?

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419


Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1593|0                           |1
        is obsolete|                            |
   Attachment #1593|ok?                         |
               Flag|                            |
   Attachment #1594|                            |ok?
               Flag|                            |




--- Comment #12 from Damien Miller <djm at mindrot.org>  2009-01-21
16:39:28 ---
Created an attachment (id=1594)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1594)
unbroken diff

oops, that last diff was made of fail.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419





--- Comment #13 from Damien Miller <djm at mindrot.org>  2009-02-02
09:22:01 ---
(In reply to comment #12)
> Created an attachment (id=1594)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1594)
[details]
> unbroken diff
Could someone from Apple confirm that attachment 1594 is correct? I'd
like to get this in for openssh-5.2.

--- Comment #14 from Damien Miller <djm at mindrot.org>  2009-02-02
09:22:49 ---
(In reply to comment #12)
> Created an attachment (id=1594)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1594)
[details]
> unbroken diff
Could someone from Apple confirm that attachment 1594 is correct? I'd
like to get this in for openssh-5.2.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419





--- Comment #15 from Disco Vince Giffin <vgiffin at apple.com>  2009-02-03
13:48:22 ---
(In reply to comment #14)
> (In reply to comment #12)
> > Created an attachment (id=1594)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=1594) [details]
[details]
> > unbroken diff
> 
> Could someone from Apple confirm that attachment 1594 [details] is correct?
I'd
> like to get this in for openssh-5.2.
This works for me (on 10.5.6).

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419


Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED




--- Comment #16 from Damien Miller <djm at mindrot.org>  2009-02-12
12:19:56 ---
patch applied - will be in openssh-5.2. Thanks!

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1419


Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED




--- Comment #17 from Damien Miller <djm at mindrot.org>  2009-02-23
13:35:41 ---
Close bugs fixed/reviewed for openssh-5.2 release

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.