On Mon, Jun 23, 2008 at 09:40:32PM +0200, Matthias Kilian wrote:
> On Mon, Jun 23, 2008 at 02:31:55PM +0200, Tobias Ulmer wrote:
> > Index: patches/patch-attacher_c
> > ===================================================================
> > RCS file: /cvs/ports/misc/screen/patches/patch-attacher_c,v
> > retrieving revision 1.1
> > diff -u -r1.1 patch-attacher_c
> > --- patches/patch-attacher_c 14 Oct 2003 23:05:28 -0000 1.1
> > +++ patches/patch-attacher_c 22 Jun 2008 12:08:32 -0000
> > @@ -1,6 +1,6 @@
> > $OpenBSD: patch-attacher_c,v 1.1 2003/10/14 23:05:28 jolan Exp $
> > ---- attacher.c.orig 2003-09-08 09:24:48.000000000 -0500
> > -+++ attacher.c 2003-10-14 14:10:14.000000000 -0500
> > +--- attacher.c.orig Mon Sep 8 16:24:48 2003
> > ++++ attacher.c Sun Jun 22 14:04:14 2008
> > @@ -676,7 +676,7 @@ LockTerminal()
> > setuid(real_uid); /* this should be done already */
> > #endif
> > @@ -10,3 +10,16 @@
> > exit(errno);
> > }
> > if (pid == -1)
> > +@@ -869,8 +869,10 @@ screen_builtin_lck()
> > + errno = 0;
> > + if ((cp1 = getpass(message)) == NULL)
> > + {
> > +- AttacherFinit(SIGARG);
> > +- /* NOTREACHED */
> > ++ if (errno == EINTR) /* interrupted by a signal */
> > ++ continue;
> > ++
> > ++ AttacherFinit(SIGARG); /* fatal error, exit attacher, unlock
> > screen */
> > + }
> > + #ifdef USE_PAM
> > + PAM_conversation.appdata_ptr = cp1;
>
> Is this enough? getpass(3) can bail out on other conditions than EINTR.
>
>
IMHO yes and no. Yes because it seems to be screens design philosophy to
unlock the attacher whenever something goes wrong (It doesn't matter if
it's getpass, PAM or an external locking app).
No because this could certainly be done better, a screen instance that
would stay locked forever looping until you log in and kill the attacher
wouldn't be the end of the world, while a rooted box might be ;)
The patch wasn't intended to fix screens bad design, just to catch that
special case where OpenBSD behaves a little different than other
operating systems.
I havn't looked into the FreeBSD sources, but since they have no patches
there and it doesn't affect them, i assume that their readpassphrase
checks if a non-default signal action is installed, and in this special
case, doesn't return with NULL and EINTR, but with the password typed so
far.
