Bug#369575: [Pkg-cryptsetup-devel] Bug#369575:

[David Härdeman]

On Fri, Jun 02, 2006 at 11:59:11PM +0200, Jonas Meurer wrote:
>On 02/06/2006 Tobias Gruetzmacher wrote:
>> On Fri, Jun 02, 2006 at 02:42:52PM +0200, Jonas Meurer wrote:
>> > ok, i prepared a fix, and uploaded i386 packages (1.0.3-2) to
>> > http://people.debian.org/~mejo/cryptsetup/
>> > 
>> > tobias, could you check if these fix your bug?
>> 
>> No, it does not. I think the problem is that the previously used getpass
>> wrote directly to /dev/tty, therefore circumventing input/output
>> redirection of the parent process. I think pmount redirects stdout
>> somewhere and the password prompt of cryptsetup gets lost. I suspect
>> this is really a pmount bug and not a problem with cryptsetup...
>
>i'm not sure about that. the cryptsetup prompt is a critical output, it
>should always appear, even if stdout and stderr are redirected.

I'm not sure whether stderr or tty is "better", but getpass uses tty 
first and stderr second, so the replacement should do the same

>i think that this is the reason why getpass was used before.
>
>david: are you capable of writing a patch that reverts this, and writes
>the prompt directly to /dev/tty again?
>maybe STDERR just needs to be changed to /dev/proc/self/fd/0?

I've attached a new version of 01_terminal_timeout.dpatch which uses 
/dev/tty if possible and stderr otherwise. I haven't tested it though, 
so perhaps it would be best if Jonas could make a new test package and 
Tobias could verify that I didn't screw things up? 

(The other dpatches probably need refreshing as well)

>and stderr is still a better solution than stdout, don't you think so?

Yes, getpass used stderr, so we should as well.

Regards,
David
-------------- next part --------------
#! /bin/sh -e
# 01_terminal_timeout.dpatch by Andres Salomon <dilinger at debian.org>
#
## DP: restore terminal state before timeout

if [ $# -ne 1 ]; then
    echo >&2 "`basename $0`: script expects -patch|-unpatch as argument"
    exit 1
fi
case "$1" in
    -patch) patch -f --no-backup-if-mismatch -p1 < $0;;
    -unpatch) patch -f --no-backup-if-mismatch -R -p1 < $0;;
    *)
        echo >&2 "`basename $0`: script expects -patch|-unpatch as argument"
        exit 1;;
esac

exit 0

@DPATCH@
diff -urNad cryptsetup-1.0.3~/lib/setup.c cryptsetup-1.0.3/lib/setup.c
--- cryptsetup-1.0.3~/lib/setup.c	2006-06-04 14:34:17.000000000 +0200
+++ cryptsetup-1.0.3/lib/setup.c	2006-06-04 14:46:45.000000000 +0200
@@ -7,6 +7,7 @@
 #include <sys/mman.h>
 #include <fcntl.h>
 #include <unistd.h>
+#include <termios.h>
 #include <errno.h>
 #include <signal.h>
 #include <assert.h>
@@ -24,12 +25,6 @@
 static int memory_unsafe = 0;
 static char *default_backend = NULL;
 
-static void catch_alarm(int sig_num)
-{
-       fprintf(stderr, "Operation timed out. Exiting.\n");
-       exit(0);
-}
-
 static int setup_enter(struct setup_backend *backend)
 {
 	int r;
@@ -72,6 +67,75 @@
 	return 0;
 }
 
+static int untimed_read(int fd, char *pass, size_t maxlen)
+{
+	ssize_t i;
+
+	i = read(fd, pass, maxlen);
+	if (i > 0) {
+		pass[i-1] = '\0';
+		i = 0;
+	}
+	return i;
+}
+
+static int timed_read(int fd, char *pass, size_t maxlen, long timeout)
+{
+	struct timeval t;
+	fd_set fds;
+	int failed = -1;
+
+	FD_ZERO(&fds);
+	FD_SET(fd, &fds);
+	t.tv_sec = timeout;
+	t.tv_usec = 0;
+
+	if (select(fd+1, &fds, NULL, NULL, &t) > 0)
+		failed = untimed_read(fd, pass, maxlen);
+	else
+		fprintf(stderr, "Operation timed out.\n");
+	return failed;
+}
+
+static int interactive_pass(const char *prompt, char *pass, size_t maxlen,
+		long timeout)
+{
+	struct termios orig, tmp;
+	int failed = -1;
+	int infd, outfd;
+
+	if (maxlen < 1)
+		goto out_err;
+
+	/* Read and write to /dev/tty if available */
+	if ((infd = outfd = open("/dev/tty", O_RDWR)) == -1) {
+		infd = STDIN_FILENO;
+		outfd = STDERR_FILENO;
+	}
+
+	if (tcgetattr(infd, &orig)) {
+		set_error("Unable to get terminal");
+		goto out_err;
+	}
+	memcpy(&tmp, &orig, sizeof(tmp));
+	tmp.c_lflag &= ~ECHO;
+
+	write(outfd, prompt, strlen(prompt));
+	tcsetattr(infd, TCSAFLUSH, &tmp);
+	if (timeout)
+		failed = timed_read(infd, pass, maxlen, timeout);
+	else
+		failed = untimed_read(infd, pass, maxlen);
+	tcsetattr(infd, TCSAFLUSH, &orig);
+
+out_err:
+	if (!failed)
+		write(outfd, "\n", 1);
+	if (infd != STDIN_FILENO)
+		close(infd);
+	return failed;
+}
+
 /*
  * Password reading behaviour matrix of get_key
  * 
@@ -109,32 +173,23 @@
 		newline_stop = 1;
 	}	
 
-	signal(SIGALRM, catch_alarm);
-	if(options->timeout) {
-		alarm(options->timeout);
-	} else {
-		alarm(0);
-	}
-	
 	/* Interactive case */
 	if(isatty(fd)) {
-		char *pass2;
-		
-		pass2 = getpass(prompt);
-		if (!pass2) {
+		int i;
+
+		pass = safe_alloc(512);
+		if (!pass || (i = interactive_pass(prompt, pass, 512, options->timeout))) {
 			set_error("Error reading passphrase");
 			goto out_err;
 		}
-		pass = safe_strdup(pass2);
-		memset(pass2, 0, strlen(pass2));
-		
 		if (verify || verify_if_possible) {
-			char *pass_verify = getpass("Verify passphrase: ");
-			if (!pass_verify || strcmp(pass, pass_verify) != 0) {
+			char pass_verify[512];
+			i = interactive_pass("Verify passphrase: ", pass_verify, sizeof(pass_verify), options->timeout);
+			if (i || strcmp(pass, pass_verify) != 0) {
 				set_error("Passphrases do not match");
 				goto out_err;
 			}
-			memset(pass_verify, 0, strlen(pass_verify));
+			memset(pass_verify, 0, sizeof(pass_verify));
 		}
 		*passLen = strlen(pass);
 		*key = pass;
@@ -187,7 +242,6 @@
 		pass[i] = 0;
 		*key = pass;
 		*passLen = i;
-		alarm(0);
 	}
 
 	return isatty(fd); /* Return true, when password reading can be tried on interactive fds */