On 06/05/2012 06:53 PM, Petr Viktorin wrote:
On 06/05/2012 04:18 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 06/05/2012 03:00 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 06/05/2012 10:06 AM, Martin Kosek wrote:
On Mon, 2012-06-04 at 11:51 -0400, Simo Sorce wrote:
On Mon, 2012-06-04 at 17:22 +0200, Petr Viktorin wrote:
An update plugin needed root privileges, and aborted the update
if an
ordinary user user ran it.
With this patch the plugin is skipped with a warning in that case.
https://fedorahosted.org/freeipa/ticket/2621
Hi Petr,
I am not sure I like the proposed solution.
If there is a legitimate reason to run this plugin as non-root (eg
admin
user) then you should change the connection part to try to use
GSSAPI
auth over ldap when non-root, not just throw a warning.
If there is no reason for anyone but root to run this script then we
should just abort if not root IMO.
Simo.
I would keep this script runable for root users only. Regularly, this
should not be run manually but as a part of RPM update which is
done by
root. It is being run manually only when something is broken anyway
and
I am not convinced that non-root users should be involved in such
recovery.
Martin
Thanks for the advice. The attached patch only allows root to run
ipa-ldap-updater.
NACK. It is very handy for developers to be able to run
ipa-ldap-updater
to test update files.
rob
Developers can run it as root, I don't see a problem here.
I'd really rather not. This does nothing requiring root permissions,
it's all done over LDAP. I'd rather trade not running some plugins than
always requiring root.
rob
Thanks for info on how the tool is used. I looked into it deeper.
The proper fix would be to use the ldap2 backend here, instead of the
IPAdmin. That's ticket 2660, and it'll be quite a lot of work to get
ReplicationManager and tools that depend on that ported.
But, I think it makes sense to require root if (and only if) plugins are
run. Justification below. Would that work for your use case?
There are currently three modes ipa-ldap-updater can run in:
1) --upgrade (needs root, runs plugins)
2) no --upgrade, either no files specified or --plugins (doesn't need
root, runs plugins)
3) no --upgrade, specific files specified without --plugins (doesn't
need root, doesn't run plugins)
I propose to make mode 2 require root.
There are two major uses of the script: install/upgrade (first two
modes), and a developer testing update files (third or possibly second
mode). Install/upgrade is always run as root, and the developer usually
doesn't need to run the plugins (if they do, they should run as root
anyway, so that some (parts of) plugins aren't skipped).
Some of the plugins ask to restart the DS. Without root privileges, the
restart (but not the rest of the plugin) is skipped. I think this is
just asking for trouble.
Some plugins (or parts of plugins) don't need root, but I don't think
singling these out and testing both cases is worth the effort.
The attached patch that implements the above. I re-ordered the code a
bit to put the checks before the DM password prompt, so you don't enter
the password only to find out you had to use sudo or different options.
--
Petr³
From a255f639101dc4a16beb96d674dc2aac527a1615 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Wed, 6 Jun 2012 00:44:58 -0400
Subject: [PATCH] Only allow root to run update plugins
Several plugins need restarting the DS (or they currently do
an external bind).
Rather than disabling plugins (possibly partially), refuse
to run them when run as an unprivileged user.
This means running ipa-ldap-updater as non-root requires specifying
a list of files, and omiting the --upgrade and --plugins options.
https://fedorahosted.org/freeipa/ticket/2621
---
install/tools/ipa-ldap-updater | 30 ++++++++++++++++-------------
ipaserver/install/plugins/updateclient.py | 3 ---
2 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/install/tools/ipa-ldap-updater b/install/tools/ipa-ldap-updater
index bd2233a94241c28375b29cc10d60908238b8f176..197b840b07867269340f56e32989820d8af59ae1 100755
--- a/install/tools/ipa-ldap-updater
+++ b/install/tools/ipa-ldap-updater
@@ -83,14 +83,27 @@ def get_dirman_password():
def main():
badsyntax = False
upgradefailed = False
- run_plugins = False
safe_options, options, args = parse_options()
+ run_plugins = options.plugins
+
+ files = []
+ if len(args) > 0:
+ files = args
+
+ if len(files) < 1:
+ run_plugins = True
+
if os.getegid() == 0:
installutils.check_server_configuration()
- elif not os.path.exists('/etc/ipa/default.conf'):
- sys.exit("IPA is not configured on this system.")
+ else:
+ if not os.path.exists('/etc/ipa/default.conf'):
+ sys.exit("IPA is not configured on this system.")
+ if options.upgrade:
+ sys.exit('Upgrade can only be done as root')
+ if run_plugins:
+ sys.exit('Plugins can only be run as root.')
dirman_password = ""
if options.password:
@@ -115,26 +128,17 @@ def main():
api.bootstrap(**cfg)
api.finalize()
- files = []
- if len(args) > 0:
- files = args
-
- if len(files) < 1:
- run_plugins = True
-
updates = None
if options.upgrade:
- if os.getegid() != 0:
- sys.exit('Upgrade can only be done as root')
root_logger.debug('%s was invoked with arguments %s and options: %s' % (sys.argv[0], args, safe_options))
realm = krbV.default_context().default_realm
upgrade = IPAUpgrade(realm, files, live_run=not options.test)
upgrade.create_instance()
modified = upgrade.modified
badsyntax = upgrade.badsyntax
upgradefailed = upgrade.upgradefailed
else:
- ld = LDAPUpdate(dm_password=dirman_password, sub_dict={}, live_run=not options.test, ldapi=options.ldapi, plugins=options.plugins or run_plugins)
+ ld = LDAPUpdate(dm_password=dirman_password, sub_dict={}, live_run=not options.test, ldapi=options.ldapi, plugins=run_plugins)
if len(files) < 1:
files = ld.get_all_files(UPDATES_DIR)
modified = ld.update(files)
diff --git a/ipaserver/install/plugins/updateclient.py b/ipaserver/install/plugins/updateclient.py
index b566d19e7959f6567443ab3093e3a3213022fe34..10d899abcad091a3396d4315d877b5656068775e 100644
--- a/ipaserver/install/plugins/updateclient.py
+++ b/ipaserver/install/plugins/updateclient.py
@@ -155,9 +155,6 @@ def run(self, method, **kw):
return self.Updater[method](**kw) #pylint: disable=E1101
def restart(self, dm_password, live_run):
- if os.getegid() != 0:
- self.log.warn("Not root, skipping restart")
- return
dsrestart = DSRestart()
socket_name = '/var/run/slapd-%s.socket' % \
api.env.realm.replace('.','-')
--
1.7.10.2
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
