[PATCH] Fix curses running as root on tty of other user

Stanislav Ochotnicky sochotnicky at redhat.com
Tue Feb 15 15:12:46 CET 2011


After doing "su -", ownership of current tty stays with original
user. If we drop all capabilities we will not be able to open current
tty to setup curses screen.

So we keep ipc_lock together with dac_override capabilities until we
open tty for R/W, then we drop dac_override capability.

This patch also fixes second cap_set_proc call in lock_pool that was
originally "cap_ipc_lock+p", but should probably be
"cap_ipc_lock-e". Original call had no effect because ipc_lock
capability was already permitted. Instead it was supposed to drop
effective capability enabled in first call.

See https://bugzilla.redhat.com/show_bug.cgi?id=676034 for details and
reproducer
---
 pinentry/pinentry-curses.c |   25 ++++++++++++++++++++++++-
 secmem/secmem.c            |    6 ++++--
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/pinentry/pinentry-curses.c b/pinentry/pinentry-curses.c
index 76ddbdd..bfc5013 100644
--- a/pinentry/pinentry-curses.c
+++ b/pinentry/pinentry-curses.c
@@ -93,6 +93,21 @@ struct dialog
 };
 typedef struct dialog *dialog_t;
 
+/* When pinentry-curses runs as root, current tty can be owned by
+  original user (before doing "su -"). We need to preserve
+  dac_override capability to be able to read/write on tty */
+static int tty_cap_setup(int init)
+{
+#if defined(USE_CAPABILITIES)
+  if (init)
+    cap_set_proc( cap_from_text("cap_dac_override+ep") );
+  else
+    cap_set_proc ( cap_from_text("cap_ipc_lock=p") );
+#endif
+  return 0;
+}
+
+
 
 /* Return the next line up to MAXLEN columns wide in START and LEN.
    The first invocation should have 0 as *LEN.  If the line ends with
@@ -635,17 +650,25 @@ dialog_run (pinentry_t pinentry, const char *tty_name, const char *tty_type)
   /* Open the desired terminal if necessary.  */
   if (tty_name)
     {
+      tty_cap_setup(1);
       ttyfi = fopen (tty_name, "r");
       if (!ttyfi)
-	return -1;
+	{
+	  int err = errno;
+	  tty_cap_setup(0);
+	  errno = err;
+	  return -1;
+	}
       ttyfo = fopen (tty_name, "w");
       if (!ttyfo)
 	{
 	  int err = errno;
 	  fclose (ttyfi);
+	  tty_cap_setup(0);
 	  errno = err;
 	  return -1;
 	}
+      tty_cap_setup(0);
       screen = newterm (tty_type, ttyfo, ttyfi);
       set_term (screen);
     }
diff --git a/secmem/secmem.c b/secmem/secmem.c
index fed7e97..b31e270 100644
--- a/secmem/secmem.c
+++ b/secmem/secmem.c
@@ -130,11 +130,13 @@ lock_pool( void *p, size_t n )
 #if defined(USE_CAPABILITIES) && defined(HAVE_MLOCK)
     int err;
 
-    cap_set_proc( cap_from_text("cap_ipc_lock+ep") );
+    /* we have to preserve dac_override to be able to open current tty
+     * even if it is owned by other user (usually after doing su) */
+    cap_set_proc( cap_from_text("cap_ipc_lock+ep cap_dac_override+p") );
     err = mlock( p, n );
     if( err && errno )
 	err = errno;
-    cap_set_proc( cap_from_text("cap_ipc_lock+p") );
+    cap_set_proc( cap_from_text("cap_ipc_lock-e cap_dac_override+p") );
 
     if( err ) {
 	if( errno != EPERM
-- 
1.7.4




More information about the Gpa-dev mailing list