From 07e2c0ce14cdb0748908e8e1b08829a1a4199eed Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Sun, 12 Jul 2015 17:25:53 +0530 Subject: [PATCH] Don't use actions to check if service is running - To check whether a service is running does not require root privileges. This can directly be done from a module without any action. - Since actions are allowed to be run using sudo, introducing unnecessary sub-commands increases attack surface. - Simple functions calls are unnecessarily being converted to command line invocations and involve parsing response. - There is a lot of repeated code because of this that can be eliminated. - To generalize this, we need to make all non-root system operations directly from module instead of delegating to action commands. --- actions/deluge | 9 --------- actions/mumble | 9 --------- actions/pagekite | 6 ------ actions/privoxy | 8 -------- actions/tor | 8 -------- actions/transmission | 9 --------- plinth/action_utils.py | 5 ++++- plinth/modules/deluge/views.py | 6 ++---- plinth/modules/mumble/views.py | 6 ++---- plinth/modules/pagekite/utils.py | 4 ++-- plinth/modules/privoxy/views.py | 6 ++---- plinth/modules/tor/tor.py | 5 ++--- plinth/modules/transmission/views.py | 6 ++---- 13 files changed, 16 insertions(+), 71 deletions(-) diff --git a/actions/deluge b/actions/deluge index 3dc21d975..1aae2cd47 100755 --- a/actions/deluge +++ b/actions/deluge @@ -65,10 +65,6 @@ def parse_arguments(): # Disable deluge-web site and stop deluge-web subparsers.add_parser('disable', help='Disable deluge-web site') - # Get whether deluge-web is running - subparsers.add_parser('is-running', - help='Get whether deluge-web is running') - return parser.parse_args() @@ -94,11 +90,6 @@ def subcommand_disable(_): disable() -def subcommand_is_running(_): - """Get whether deluge-web is running.""" - print('yes' if action_utils.service_is_running('deluge-web') else 'no') - - def enable(): """Start and enable deluge-web service.""" action_utils.service_enable('deluge-web') diff --git a/actions/mumble b/actions/mumble index 86bdd2505..22dc4ffd5 100755 --- a/actions/mumble +++ b/actions/mumble @@ -45,10 +45,6 @@ def parse_arguments(): # Disable service subparsers.add_parser('disable', help='Disable Mumble service') - # Get whether daemon is running - subparsers.add_parser('is-running', - help='Get whether Mumble daemon is running') - return parser.parse_args() @@ -95,11 +91,6 @@ def set_service_enable(enable): file.writelines(lines) -def subcommand_is_running(_): - """Get whether server is running.""" - print('yes' if action_utils.service_is_running('mumble-server') else 'no') - - def main(): """Parse arguments and perform all duties.""" arguments = parse_arguments() diff --git a/actions/pagekite b/actions/pagekite index 437cb8854..9bb1165b8 100755 --- a/actions/pagekite +++ b/actions/pagekite @@ -49,7 +49,6 @@ def parse_arguments(): subparsers = parser.add_subparsers(dest='subcommand', help='Sub command') # Enable/disable the pagekite service - subparsers.add_parser('is-running', help='Get whether PakeKite is running') subparsers.add_parser('start-and-enable', help='Enable PageKite service') subparsers.add_parser('stop-and-disable', help='Disable PageKite service') subparsers.add_parser('restart', help='Restart PageKite service') @@ -81,11 +80,6 @@ def parse_arguments(): return parser.parse_args() -def subcommand_is_running(_): - """Print whether pagekite is enabled (yes or no)""" - print('yes' if action_utils.service_is_running('pagekite') else 'no') - - def subcommand_restart(_): """Restart the pagekite service""" action_utils.service_restart('pagekite') diff --git a/actions/privoxy b/actions/privoxy index 2b44bb5d6..293add297 100755 --- a/actions/privoxy +++ b/actions/privoxy @@ -41,8 +41,6 @@ def parse_arguments(): help='Get whether Privoxy service is enabled') subparsers.add_parser('enable', help='Enable Privoxy service') subparsers.add_parser('disable', help='Disable Privoxy service') - subparsers.add_parser('is-running', - help='Get whether Privoxy daemon is running') return parser.parse_args() @@ -83,12 +81,6 @@ def subcommand_disable(_): action_utils.service_disable('privoxy') -def subcommand_is_running(_): - """Get whether server is running.""" - running = action_utils.service_is_running('privoxy') - print('yes' if running else 'no') - - def main(): """Parse arguments and perform all duties.""" arguments = parse_arguments() diff --git a/actions/tor b/actions/tor index e0a7e36cc..22fab6238 100755 --- a/actions/tor +++ b/actions/tor @@ -34,9 +34,6 @@ def parse_arguments(): parser = argparse.ArgumentParser() subparsers = parser.add_subparsers(dest='subcommand', help='Sub command') - # Get whether Tor is running - subparsers.add_parser('is-running', help='Get whether Tor is running') - # Enable and start the service subparsers.add_parser('enable', help='Enable and start Tor service') @@ -55,11 +52,6 @@ def parse_arguments(): return parser.parse_args() -def subcommand_is_running(_): - """Get whether Tor is running""" - print('yes' if action_utils.service_is_running('tor') else 'no') - - def subcommand_enable(_): """Enable and start the service.""" action_utils.service_enable('tor') diff --git a/actions/transmission b/actions/transmission index fc30dfd1a..8ba8aeeeb 100755 --- a/actions/transmission +++ b/actions/transmission @@ -47,10 +47,6 @@ def parse_arguments(): # Disable service subparsers.add_parser('disable', help='Disable Transmission service') - # Get whether daemon is running - subparsers.add_parser('is-running', - help='Get whether Transmission daemon is running') - # Merge given JSON configration with existing merge_configuration = subparsers.add_parser( 'merge-configuration', @@ -108,11 +104,6 @@ def set_service_enable(enable): file_handle.writelines(lines) -def subcommand_is_running(_): - """Get whether Transmission is running.""" - print('yes' if action_utils.service_is_running('transmission-daemon') else 'no') - - def subcommand_merge_configuration(arguments): """Merge given JSON configuration with existing configuration.""" configuration = arguments.configuration diff --git a/plinth/action_utils.py b/plinth/action_utils.py index 79b348007..d14b8db90 100644 --- a/plinth/action_utils.py +++ b/plinth/action_utils.py @@ -23,7 +23,10 @@ import subprocess def service_is_running(servicename): - """Evaluates whether a service is currently running. Returns boolean""" + """Return whether a service is currently running. + + Does not need to run as root. + """ try: subprocess.check_output(['systemctl', 'status', servicename]) return True diff --git a/plinth/modules/deluge/views.py b/plinth/modules/deluge/views.py index c73818c06..25115ac51 100644 --- a/plinth/modules/deluge/views.py +++ b/plinth/modules/deluge/views.py @@ -25,6 +25,7 @@ from gettext import gettext as _ from .forms import DelugeForm from plinth import actions +from plinth import action_utils from plinth import package from plinth.modules import deluge @@ -57,11 +58,8 @@ def get_status(): output = actions.run('deluge', ['get-enabled']) enabled = (output.strip() == 'yes') - output = actions.run('deluge', ['is-running']) - is_running = (output.strip() == 'yes') - status = {'enabled': enabled, - 'is_running': is_running} + 'is_running': action_utils.service_is_running('deluge-web')} return status diff --git a/plinth/modules/mumble/views.py b/plinth/modules/mumble/views.py index fbb5e57f3..5a9753d18 100644 --- a/plinth/modules/mumble/views.py +++ b/plinth/modules/mumble/views.py @@ -26,6 +26,7 @@ import logging from .forms import MumbleForm from plinth import actions +from plinth import action_utils from plinth import package from plinth.modules import mumble @@ -65,11 +66,8 @@ def get_status(): output = actions.run('mumble', ['get-enabled']) enabled = (output.strip() == 'yes') - output = actions.superuser_run('mumble', ['is-running']) - is_running = (output.strip() == 'yes') - status = {'enabled': enabled, - 'is_running': is_running} + 'is_running': action_utils.service_is_running('mumble')} return status diff --git a/plinth/modules/pagekite/utils.py b/plinth/modules/pagekite/utils.py index 7886112ec..849e6af60 100644 --- a/plinth/modules/pagekite/utils.py +++ b/plinth/modules/pagekite/utils.py @@ -21,6 +21,7 @@ import logging import os from plinth import actions +from plinth import action_utils LOGGER = logging.getLogger(__name__) @@ -91,8 +92,7 @@ def get_pagekite_config(): # PageKite service enabled/disabled # This assumes that if pagekite is running it's also enabled as a service - output = run(['is-running']) - status['enabled'] = (output.split()[0] == 'yes') + status['enabled'] = action_utils.service_is_running('pagekite') # PageKite kite details status.update(get_kite_details()) diff --git a/plinth/modules/privoxy/views.py b/plinth/modules/privoxy/views.py index 27e948136..aeedd4d25 100644 --- a/plinth/modules/privoxy/views.py +++ b/plinth/modules/privoxy/views.py @@ -26,6 +26,7 @@ import logging from .forms import PrivoxyForm from plinth import actions +from plinth import action_utils from plinth import package from plinth.modules import privoxy @@ -65,11 +66,8 @@ def get_status(): output = actions.run('privoxy', ['get-enabled']) enabled = (output.strip() == 'yes') - output = actions.superuser_run('privoxy', ['is-running']) - is_running = (output.strip() == 'yes') - status = {'enabled': enabled, - 'is_running': is_running} + 'is_running': action_utils.service_is_running('privoxy')} return status diff --git a/plinth/modules/tor/tor.py b/plinth/modules/tor/tor.py index 1ee106c21..5f6eba542 100644 --- a/plinth/modules/tor/tor.py +++ b/plinth/modules/tor/tor.py @@ -25,6 +25,7 @@ from django.template.response import TemplateResponse from gettext import gettext as _ from plinth import actions +from plinth import action_utils from plinth import cfg from plinth import package @@ -71,8 +72,6 @@ def index(request): def get_status(): """Return the current status""" - is_running = actions.superuser_run('tor', ['is-running']).strip() == 'yes' - output = actions.superuser_run('tor-get-ports') port_info = output.split('\n') ports = {} @@ -99,7 +98,7 @@ def get_status(): hs_hostname = hs_info[0] hs_ports = hs_info[1] - return {'is_running': is_running, + return {'is_running': action_utils.service_is_running('tor'), 'ports': ports, 'hs_enabled': hs_enabled, 'hs_hostname': hs_hostname, diff --git a/plinth/modules/transmission/views.py b/plinth/modules/transmission/views.py index ab3fbd529..11658d4bc 100644 --- a/plinth/modules/transmission/views.py +++ b/plinth/modules/transmission/views.py @@ -28,6 +28,7 @@ import socket from .forms import TransmissionForm from plinth import actions +from plinth import action_utils from plinth import package from plinth.modules import transmission @@ -69,15 +70,12 @@ def get_status(): output = actions.run('transmission', ['get-enabled']) enabled = (output.strip() == 'yes') - output = actions.superuser_run('transmission', ['is-running']) - is_running = (output.strip() == 'yes') - configuration = open(TRANSMISSION_CONFIG, 'r').read() status = json.loads(configuration) status = {key.translate(str.maketrans({'-': '_'})): value for key, value in status.items()} status['enabled'] = enabled - status['is_running'] = is_running + status['is_running'] = action_utils.service_is_running('transmission-daemon') status['hostname'] = socket.gethostname() return status