[kbd] [PATCH] vlock: Handle tty dying

[kbd] [PATCH] vlock: Handle tty dying

[Kyle Manna]

[-- Attachment #1: Type: text/plain, Size: 720 bytes --]

Hi guys,

I ran into a problem where if vlock was running over ssh and the ssh
session died my system logs would get spammed by the now orphaned vlock
process on Arch Linux:

    Sep 25 20:51:12 hostname vlock[31336]: pam_unix(vlock:auth): auth could not identify password for [user]
    Sep 25 20:51:13 hostname vlock[31336]: pam_unix(vlock:auth): auth could not identify password for [user]
    Sep 25 20:51:14 hostname vlock[31336]: pam_unix(vlock:auth): auth could not identify password for [user]

I was able to re-create this in many other situations (ssh, xterm
window/tab, tmux window/pane, etc).

The patch know checks for a tty on stdin where PAM will likely ask for
the password and exits if it's not found.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-vlock-Handle-tty-dying.patch --]
[-- Type: text/x-diff, Size: 1158 bytes --]

From ec97bfb838ff45a1f7c3a640cb29f7eeb2718db3 Mon Sep 17 00:00:00 2001
From: Kyle Manna <kyle@kylemanna.com>
Date: Thu, 25 Sep 2014 22:22:17 -0700
Subject: [PATCH] vlock: Handle tty dying

* Gracefully exit when the parent tty dies.
* Typical causes of death include closing:
    * terminal tab or window
    * shell
    * ssh session
* If no tty exists, I'm not sure what vlock would be locking.

Signed-off-by: Kyle Manna <kyle@kylemanna.com>
---
 src/vlock/auth.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/vlock/auth.c b/src/vlock/auth.c
index da135ce..79b1319 100644
--- a/src/vlock/auth.c
+++ b/src/vlock/auth.c
@@ -67,6 +67,17 @@ get_password (pam_handle_t * pamh, const char *username, const char *tty)
 		int     rc;
 		const char *msg;
 
+		/* Ensure that the process has a tty.  If the parent had died,
+		 * stdin/out are likely no longer valid and PAM will fail continuously.
+		 */
+		if (isatty(STDIN_FILENO) != 1)
+		{
+			syslog (LOG_INFO,
+				"TTY %s disappeared for %s by (uid=%u)",
+				tty, username, uid);
+			return EXIT_FAILURE;
+		}
+
 		if (!pamh)
 		{
 			pamh = init_pam (username, tty, 1);
-- 
2.1.1

Re: [kbd] [PATCH] vlock: Handle tty dying

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]

Hi,

On Fri, Sep 26, 2014 at 05:42:14AM +0000, Kyle Manna wrote:
> Hi guys,
> 
> I ran into a problem where if vlock was running over ssh and the ssh
> session died my system logs would get spammed by the now orphaned vlock
> process on Arch Linux:
> 
>     Sep 25 20:51:12 hostname vlock[31336]: pam_unix(vlock:auth): auth could not identify password for [user]
>     Sep 25 20:51:13 hostname vlock[31336]: pam_unix(vlock:auth): auth could not identify password for [user]
>     Sep 25 20:51:14 hostname vlock[31336]: pam_unix(vlock:auth): auth could not identify password for [user]
> 
> I was able to re-create this in many other situations (ssh, xterm
> window/tab, tmux window/pane, etc).
> 
> The patch know checks for a tty on stdin where PAM will likely ask for
> the password and exits if it's not found.

I've never seen this myself because in systems where I use vlock
pam_authenticate consistently returns PAM_INCOMPLETE in such cases.

I'll shortly post patches to handle these cases gracefully.


-- 
ldv

[-- Attachment #2: Type: application/pgp-signature, Size: 181 bytes --]

[kbd] [PATCH 1/2] vlock: move some code around

[Dmitry V. Levin]

Move the code that handles PAM_MAXTRIES and PAM_ABORT cases before the
code that handles PAM_INCOMPLETE case.  This no-op change is only needed
to make the next change easier to read.
---
 src/vlock/auth.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/vlock/auth.c b/src/vlock/auth.c
index da135ce..dc47ee8 100644
--- a/src/vlock/auth.c
+++ b/src/vlock/auth.c
@@ -130,6 +130,19 @@ get_password (pam_handle_t * pamh, const char *username, const char *tty)
 					locked_name (), tty, username, uid);
 				return EXIT_SUCCESS;
 
+			case PAM_MAXTRIES:
+			case PAM_ABORT:
+				msg = pam_strerror (pamh, rc);
+				/* Log the fact of failure. */
+				syslog (LOG_WARNING, "%s", msg);
+				printf ("%s.\n\n\n", msg);
+				fflush (stdout);
+				msg = 0;
+				pam_end (pamh, rc);
+				pamh = 0;
+				sleep (LONG_DELAY);
+				break;
+
 			case PAM_INCOMPLETE:
 				/*
 				 * EOF encountered on read?
@@ -149,19 +162,6 @@ get_password (pam_handle_t * pamh, const char *username, const char *tty)
 					locked_name (), tty, username, uid);
 				return EXIT_FAILURE;
 
-			case PAM_MAXTRIES:
-			case PAM_ABORT:
-				msg = pam_strerror (pamh, rc);
-				/* Log the fact of failure. */
-				syslog (LOG_WARNING, "%s", msg);
-				printf ("%s.\n\n\n", msg);
-				fflush (stdout);
-				msg = 0;
-				pam_end (pamh, rc);
-				pamh = 0;
-				sleep (LONG_DELAY);
-				break;
-
 			default:
 				printf ("%s.\n\n\n", pam_strerror (pamh, rc));
 				fflush (stdout);
-- 
ldv

[kbd] [PATCH 2/2] vlock: handle disappearing ttys gracefully

[Dmitry V. Levin]

vlock used to check for disappearing ttys after PAM_INCOMPLETE error
returned by pam_authenticate.  This change extends the check to cover
other non-fatal error codes that could be returned by pam_authenticate.

Reported-by: Kyle Manna <kyle@kylemanna.com>
---
 src/vlock/auth.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/vlock/auth.c b/src/vlock/auth.c
index dc47ee8..eddce51 100644
--- a/src/vlock/auth.c
+++ b/src/vlock/auth.c
@@ -143,7 +143,9 @@ get_password (pam_handle_t * pamh, const char *username, const char *tty)
 				sleep (LONG_DELAY);
 				break;
 
-			case PAM_INCOMPLETE:
+			default:
+				printf ("%s.\n\n\n", pam_strerror (pamh, rc));
+				fflush (stdout);
 				/*
 				 * EOF encountered on read?
 				 * If not on VT, check stdin.
@@ -161,11 +163,6 @@ get_password (pam_handle_t * pamh, const char *username, const char *tty)
 					"Cancelled lock of %s on %s for %s by (uid=%u)",
 					locked_name (), tty, username, uid);
 				return EXIT_FAILURE;
-
-			default:
-				printf ("%s.\n\n\n", pam_strerror (pamh, rc));
-				fflush (stdout);
-				sleep (SHORT_DELAY);
 		}
 	}
 }

-- 
ldv

Re: [kbd] [PATCH] vlock: Handle tty dying

[Alexey Gladkov]

23.01.2015 20:46, Dmitry V. Levin пишет:
> Hi,
> 
> On Fri, Sep 26, 2014 at 05:42:14AM +0000, Kyle Manna wrote:
>> Hi guys,
>>
>> I ran into a problem where if vlock was running over ssh and the ssh
>> session died my system logs would get spammed by the now orphaned vlock
>> process on Arch Linux:
>>
>>     Sep 25 20:51:12 hostname vlock[31336]: pam_unix(vlock:auth): auth could not identify password for [user]
>>     Sep 25 20:51:13 hostname vlock[31336]: pam_unix(vlock:auth): auth could not identify password for [user]
>>     Sep 25 20:51:14 hostname vlock[31336]: pam_unix(vlock:auth): auth could not identify password for [user]
>>
>> I was able to re-create this in many other situations (ssh, xterm
>> window/tab, tmux window/pane, etc).
>>
>> The patch know checks for a tty on stdin where PAM will likely ask for
>> the password and exits if it's not found.
> 
> I've never seen this myself because in systems where I use vlock
> pam_authenticate consistently returns PAM_INCOMPLETE in such cases.
> 
> I'll shortly post patches to handle these cases gracefully.

Applied. Thanks a lot!

-- 
Rgrds, legion