About the TTY of the sudo command
Hi, I see in kimchid SystemD service file we assign a TTY to it just to avoid the sudo TTY missing error. I think in some setups other than SystemD the admin might also decide to require TTY for sudo and modify the sudo config, so the current solution is just a workaround for RHEL family distroes. Sometimes it's useful for a daemon provides a console interface on a TTY, but kimchid is not in this type. Maybe it's better to run sudo in a temporary PTY and avoid assigning a TTY to kimchid. Actually I'm fine with the current solution. Just happened to see to this problem and thought maybe we can improve it a little bit.
From: Zhou Zheng Sheng <zhshzhou@linux.vnet.ibm.com> In commit 456018a, we assign a TTY to kimchid in the SystemD service file. This is because sudo requires a TTY by default for security reason. Usually Linux daemon detaches from controlling terminal so that it makes use of the SIGHUP signal to reload its settings. If the daemon has a console interface for the admin to interact (like a QEMU monitor), it can have a TTY to run the console interface. At lease for now, Kimchi is a traditional plain daemon that needs no TTY. This patch creates a temporary PTY and assigns the slave to the sudo process as the controlling terminal each time it runs sudo. In this way we do not have to assign a TTY to kimchid. Maybe it's more portable than specifying TTY in a SystemD service file. Before running sudo, it creates a subprocess to make sure the child is not the session leader. Then the child creates a new session and PYT, and sets the slave as the controlling termial. At last the child calls sudo. The result from the child is delivered to the parent via a shared memory object. Signed-off-by: Zhou Zheng Sheng <zhshzhou@linux.vnet.ibm.com> --- contrib/kimchid.service.fedora | 3 --- src/kimchi/auth.py | 24 ++++++++++++++++++++---- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/contrib/kimchid.service.fedora b/contrib/kimchid.service.fedora index 5f274fb..7abe49b 100644 --- a/contrib/kimchid.service.fedora +++ b/contrib/kimchid.service.fedora @@ -8,9 +8,6 @@ Type=simple ExecStart=/usr/bin/kimchid ExecStop=/bin/kill -TERM $MAINPID EnvironmentFile=/etc/kimchi/kimchi.conf -StandardInput=tty -StandardOutput=tty -TTYPath=/dev/tty11 [Install] WantedBy=multi-user.target diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index b042d73..9f6f2f4 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -19,9 +19,14 @@ import base64 import cherrypy +import fcntl import grp +import multiprocessing +import os import PAM +import pty import re +import termios import time @@ -55,6 +60,18 @@ class User(object): return self.user[USER_GROUPS] def has_sudo(self): + result = multiprocessing.Value('i', 0, lock=False) + p = multiprocessing.Process(target=self._has_sudo, args=(result,)) + p.start() + p.join() + self.user[USER_SUDO] = bool(result.value) + return self.user[USER_SUDO] + + def _has_sudo(self, result): + _master, slave = pty.openpty() + os.setsid() + fcntl.ioctl(slave, termios.TIOCSCTTY, 0) + out, err, exit = run_command(['sudo', '-l', '-U', self.user[USER_ID], 'sudo']) if exit == 0: @@ -67,15 +84,14 @@ class User(object): self.user[USER_ID]]) for line in out.split('\n'): if line and re.search("(ALL)", line): - self.user[USER_SUDO] = True + result.value = 1 debug("User %s can run any command with sudo" % - self.user[USER_ID]) - return self.user[USER_SUDO] + result.value) + return debug("User %s can only run some commands with sudo" % self.user[USER_ID]) else: debug("User %s is not allowed to run sudo" % self.user[USER_ID]) - return self.user[USER_SUDO] def get_user(self): return self.user -- 1.8.5.3
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 03/18/2014 01:48 PM, zhshzhou@linux.vnet.ibm.com wrote:
From: Zhou Zheng Sheng <zhshzhou@linux.vnet.ibm.com>
In commit 456018a, we assign a TTY to kimchid in the SystemD service file. This is because sudo requires a TTY by default for security reason.
Usually Linux daemon detaches from controlling terminal so that it makes use of the SIGHUP signal to reload its settings. If the daemon has a console interface for the admin to interact (like a QEMU monitor), it can have a TTY to run the console interface. At lease for now, Kimchi is a traditional plain daemon that needs no TTY.
This patch creates a temporary PTY and assigns the slave to the sudo process as the controlling terminal each time it runs sudo. In this way we do not have to assign a TTY to kimchid. Maybe it's more portable than specifying TTY in a SystemD service file.
Before running sudo, it creates a subprocess to make sure the child is not the session leader. Then the child creates a new session and PYT, and sets the slave as the controlling termial. At last the child calls sudo. The result from the child is delivered to the parent via a shared memory object.
Signed-off-by: Zhou Zheng Sheng <zhshzhou@linux.vnet.ibm.com> --- contrib/kimchid.service.fedora | 3 --- src/kimchi/auth.py | 24 ++++++++++++++++++++---- 2 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/contrib/kimchid.service.fedora b/contrib/kimchid.service.fedora index 5f274fb..7abe49b 100644 --- a/contrib/kimchid.service.fedora +++ b/contrib/kimchid.service.fedora @@ -8,9 +8,6 @@ Type=simple ExecStart=/usr/bin/kimchid ExecStop=/bin/kill -TERM $MAINPID EnvironmentFile=/etc/kimchi/kimchi.conf -StandardInput=tty -StandardOutput=tty -TTYPath=/dev/tty11
[Install] WantedBy=multi-user.target diff --git a/src/kimchi/auth.py b/src/kimchi/auth.py index b042d73..9f6f2f4 100644 --- a/src/kimchi/auth.py +++ b/src/kimchi/auth.py @@ -19,9 +19,14 @@
import base64 import cherrypy +import fcntl import grp +import multiprocessing +import os import PAM +import pty import re +import termios import time
@@ -55,6 +60,18 @@ class User(object): return self.user[USER_GROUPS]
def has_sudo(self): + result = multiprocessing.Value('i', 0, lock=False) + p = multiprocessing.Process(target=self._has_sudo, args=(result,)) + p.start() + p.join() + self.user[USER_SUDO] = bool(result.value) + return self.user[USER_SUDO] + + def _has_sudo(self, result): + _master, slave = pty.openpty() + os.setsid() + fcntl.ioctl(slave, termios.TIOCSCTTY, 0) + out, err, exit = run_command(['sudo', '-l', '-U', self.user[USER_ID], 'sudo']) if exit == 0: @@ -67,15 +84,14 @@ class User(object): self.user[USER_ID]]) for line in out.split('\n'): if line and re.search("(ALL)", line): - self.user[USER_SUDO] = True + result.value = 1 debug("User %s can run any command with sudo" % - self.user[USER_ID]) - return self.user[USER_SUDO] + result.value) + return debug("User %s can only run some commands with sudo" % self.user[USER_ID]) else: debug("User %s is not allowed to run sudo" % self.user[USER_ID]) - return self.user[USER_SUDO]
def get_user(self): return self.user
Applied. Thanks. Regards, Aline Manera
participants (2)
-
Aline Manera -
zhshzhou＠linux.vnet.ibm.com