Re: [Freeipa-devel] [PATCH 0167] ipa-client-install: Configure sudo to use SSSD as data source

2014-05-08 Thread Alexander Bokovoy

On Wed, 07 May 2014, Jakub Hrozek wrote:

On Wed, May 07, 2014 at 05:29:37PM +0200, Tomas Babej wrote:


On 04/30/2014 02:44 PM, Jakub Hrozek wrote:
 On Wed, Apr 30, 2014 at 11:05:52AM +0200, Tomas Babej wrote:
 On 03/24/2014 03:27 PM, Jan Pazdziora wrote:
 On Mon, Mar 24, 2014 at 02:57:30PM +0100, Martin Kosek wrote:
 On 03/24/2014 02:47 PM, Jan Pazdziora wrote:
 On Mon, Mar 03, 2014 at 08:24:41PM +0100, Tomas Babej wrote:
 Hi,

 Makes ipa-client-install configure SSSD as the data provider
 for the sudo service by default. This behaviour can be disabled
 by using --no-sudo flag.

 https://fedorahosted.org/freeipa/ticket/3358
 Ack.

 Applied against ipa-client-3.0.0-37.el6.x86_64, tried without
 --no-sudo and sudo was added to sssd.conf's services list and sudoeers
 added to /etc/nsswitch.conf.

 Rerun with --uninstall and run again with the --no-sudo parameter,
 those settings were not longer there.

 Did you also do the functional test?
 No. I do not want to get dragged into the discussion of having the
 correct sssd and sudo and glibc versions and SELinux and stuff. The
 ticket explicitly talk about setting configuration in config files,
 which the patch does.

 To ack and push this ticket, following
 scenario needs to work:
 Consumption of those configuration changes is really different story,
 isn't it?

 1) IPA clients enroll against IPA server without --no-sudo
 2) IPA client user logs in, types sudo -l, gets all allowed commands
 (prerequisite is of course to have sudo commands defined on the IPA server)
 3) IPA client reboots, IPA client user logs in, types sudo -l, gets all
 allowed commands

 For 2) to work, NIS domain name must be set, nsswitch and SSSD changes 
must be done

 For 3) to work, related systemd service preserving NIS domain name setting
 needs to be enabled
 With the commit message only talking about configuring sssd, I assume
 the NIS domain name mentioned in the ticket will be done by some other
 patch.

 To me, the patch does what is advertised in the commit message, and is
 in line with what the ticket asks to be done.

 Attached are rebased versions of the patches 113 and 167 (which was
 marked as 157 in the thread previously by mistake).

 There is a slight behaviour change in 167, if there is no sudoers line
 in nsswitch.conf, we add both files and sss as sudoers sources.

 I also developed CI test that covers the functionality of the IPA - sudo
 integration feature, which is attached.

 Please note that the last three tests are expected to fail until:

 https://fedorahosted.org/freeipa/ticket/4324

 is fixed.

 --
 Tomas Babej
 Associate Software Engineer | Red Hat | Identity Management
 RHCE | Brno Site | IRC: tbabej | freeipa.org

 Hi,

 I haven't done a thorough review, but the patch looks good to me in
 general -- in other words, seems to cover what I've been doing manually
 for my test setups.

 My only suggestion (maybe for future) would be to split changing the
 nsswitch.conf into its own separate helper class or a function, because
 you might want to do the same change for automount or other services in
 nsswitch.conf.

 But I think this version is OK at the moment.

 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel

I created a rather general function for editing the nsswitch.conf as
requesting.

Updated patch attached.


Thanks, looks good to me, although I haven't done a thorough review.

ACK from my side too. I spent some time today looking through the
Jenkins job with these patches and apart from three failures in the new
sudo test suite everything looks fine. These failures were around dozen
in past two weeks and this patchset reduces them as a work in progress.

Given that the tests only run against in CI and are fresh new, I'm fine
with committing the patchset, the fixes can come next week.

--
/ Alexander Bokovoy

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0167] ipa-client-install: Configure sudo to use SSSD as data source

2014-05-07 Thread Tomas Babej

On 04/30/2014 02:44 PM, Jakub Hrozek wrote:
 On Wed, Apr 30, 2014 at 11:05:52AM +0200, Tomas Babej wrote:
 On 03/24/2014 03:27 PM, Jan Pazdziora wrote:
 On Mon, Mar 24, 2014 at 02:57:30PM +0100, Martin Kosek wrote:
 On 03/24/2014 02:47 PM, Jan Pazdziora wrote:
 On Mon, Mar 03, 2014 at 08:24:41PM +0100, Tomas Babej wrote:
 Hi,

 Makes ipa-client-install configure SSSD as the data provider
 for the sudo service by default. This behaviour can be disabled
 by using --no-sudo flag.

 https://fedorahosted.org/freeipa/ticket/3358
 Ack.

 Applied against ipa-client-3.0.0-37.el6.x86_64, tried without
 --no-sudo and sudo was added to sssd.conf's services list and sudoeers
 added to /etc/nsswitch.conf.

 Rerun with --uninstall and run again with the --no-sudo parameter,
 those settings were not longer there.

 Did you also do the functional test?
 No. I do not want to get dragged into the discussion of having the
 correct sssd and sudo and glibc versions and SELinux and stuff. The
 ticket explicitly talk about setting configuration in config files,
 which the patch does.

 To ack and push this ticket, following
 scenario needs to work:
 Consumption of those configuration changes is really different story,
 isn't it?

 1) IPA clients enroll against IPA server without --no-sudo
 2) IPA client user logs in, types sudo -l, gets all allowed commands
 (prerequisite is of course to have sudo commands defined on the IPA server)
 3) IPA client reboots, IPA client user logs in, types sudo -l, gets all
 allowed commands

 For 2) to work, NIS domain name must be set, nsswitch and SSSD changes 
 must be done

 For 3) to work, related systemd service preserving NIS domain name setting
 needs to be enabled
 With the commit message only talking about configuring sssd, I assume
 the NIS domain name mentioned in the ticket will be done by some other
 patch.

 To me, the patch does what is advertised in the commit message, and is
 in line with what the ticket asks to be done.

 Attached are rebased versions of the patches 113 and 167 (which was
 marked as 157 in the thread previously by mistake).

 There is a slight behaviour change in 167, if there is no sudoers line
 in nsswitch.conf, we add both files and sss as sudoers sources.

 I also developed CI test that covers the functionality of the IPA - sudo
 integration feature, which is attached.

 Please note that the last three tests are expected to fail until:

 https://fedorahosted.org/freeipa/ticket/4324

 is fixed.

 -- 
 Tomas Babej
 Associate Software Engineer | Red Hat | Identity Management
 RHCE | Brno Site | IRC: tbabej | freeipa.org 

 Hi,

 I haven't done a thorough review, but the patch looks good to me in
 general -- in other words, seems to cover what I've been doing manually
 for my test setups.

 My only suggestion (maybe for future) would be to split changing the
 nsswitch.conf into its own separate helper class or a function, because
 you might want to do the same change for automount or other services in
 nsswitch.conf.

 But I think this version is OK at the moment.

 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel

I created a rather general function for editing the nsswitch.conf as
requesting.

Updated patch attached.

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

From d10cf78796e64b68bc59d645d70b6ec2dfff5785 Mon Sep 17 00:00:00 2001
From: Tomas Babej tomasba...@gmail.com
Date: Thu, 21 Nov 2013 13:09:28 +0100
Subject: [PATCH] ipa-client-install: Configure sudo to use SSSD as data source

Makes ipa-client-install configure SSSD as the data provider
for the sudo service by default. This behaviour can be disabled
by using --no-sudo flag.

https://fedorahosted.org/freeipa/ticket/3358
---
 ipa-client/ipa-install/ipa-client-install | 84 ++-
 ipa-client/man/ipa-client-install.1   |  3 ++
 2 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 5fdd51520ba667f240239077a80e328877c99cd7..6fd64d0d940be97ea2d443fe01a4aebe3ce3d661 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -72,6 +72,8 @@ SSH_KNOWNHOSTSFILE = '/var/lib/sss/pubconf/known_hosts'
 
 client_nss_nickname_format = 'IPA Machine Certificate - %s'
 
+NSSWITCH_CONF = '/etc/nsswitch.conf'
+
 def parse_options():
 def validate_ca_cert_file_option(option, opt, value, parser):
 if not os.path.exists(value):
@@ -137,6 +139,9 @@ def parse_options():
   help=do not configure OpenSSH client)
 basic_group.add_option(--no-sshd, dest=conf_sshd, default=True, action=store_false,
   help=do not configure OpenSSH server)
+basic_group.add_option(--no-sudo, dest=conf_sudo, default=True,
+   

Re: [Freeipa-devel] [PATCH 0167] ipa-client-install: Configure sudo to use SSSD as data source

2014-05-07 Thread Jakub Hrozek
On Wed, May 07, 2014 at 05:29:37PM +0200, Tomas Babej wrote:
 
 On 04/30/2014 02:44 PM, Jakub Hrozek wrote:
  On Wed, Apr 30, 2014 at 11:05:52AM +0200, Tomas Babej wrote:
  On 03/24/2014 03:27 PM, Jan Pazdziora wrote:
  On Mon, Mar 24, 2014 at 02:57:30PM +0100, Martin Kosek wrote:
  On 03/24/2014 02:47 PM, Jan Pazdziora wrote:
  On Mon, Mar 03, 2014 at 08:24:41PM +0100, Tomas Babej wrote:
  Hi,
 
  Makes ipa-client-install configure SSSD as the data provider
  for the sudo service by default. This behaviour can be disabled
  by using --no-sudo flag.
 
  https://fedorahosted.org/freeipa/ticket/3358
  Ack.
 
  Applied against ipa-client-3.0.0-37.el6.x86_64, tried without
  --no-sudo and sudo was added to sssd.conf's services list and sudoeers
  added to /etc/nsswitch.conf.
 
  Rerun with --uninstall and run again with the --no-sudo parameter,
  those settings were not longer there.
 
  Did you also do the functional test?
  No. I do not want to get dragged into the discussion of having the
  correct sssd and sudo and glibc versions and SELinux and stuff. The
  ticket explicitly talk about setting configuration in config files,
  which the patch does.
 
  To ack and push this ticket, following
  scenario needs to work:
  Consumption of those configuration changes is really different story,
  isn't it?
 
  1) IPA clients enroll against IPA server without --no-sudo
  2) IPA client user logs in, types sudo -l, gets all allowed commands
  (prerequisite is of course to have sudo commands defined on the IPA 
  server)
  3) IPA client reboots, IPA client user logs in, types sudo -l, gets all
  allowed commands
 
  For 2) to work, NIS domain name must be set, nsswitch and SSSD changes 
  must be done
 
  For 3) to work, related systemd service preserving NIS domain name 
  setting
  needs to be enabled
  With the commit message only talking about configuring sssd, I assume
  the NIS domain name mentioned in the ticket will be done by some other
  patch.
 
  To me, the patch does what is advertised in the commit message, and is
  in line with what the ticket asks to be done.
 
  Attached are rebased versions of the patches 113 and 167 (which was
  marked as 157 in the thread previously by mistake).
 
  There is a slight behaviour change in 167, if there is no sudoers line
  in nsswitch.conf, we add both files and sss as sudoers sources.
 
  I also developed CI test that covers the functionality of the IPA - sudo
  integration feature, which is attached.
 
  Please note that the last three tests are expected to fail until:
 
  https://fedorahosted.org/freeipa/ticket/4324
 
  is fixed.
 
  -- 
  Tomas Babej
  Associate Software Engineer | Red Hat | Identity Management
  RHCE | Brno Site | IRC: tbabej | freeipa.org 
 
  Hi,
 
  I haven't done a thorough review, but the patch looks good to me in
  general -- in other words, seems to cover what I've been doing manually
  for my test setups.
 
  My only suggestion (maybe for future) would be to split changing the
  nsswitch.conf into its own separate helper class or a function, because
  you might want to do the same change for automount or other services in
  nsswitch.conf.
 
  But I think this version is OK at the moment.
 
  ___
  Freeipa-devel mailing list
  Freeipa-devel@redhat.com
  https://www.redhat.com/mailman/listinfo/freeipa-devel
 
 I created a rather general function for editing the nsswitch.conf as
 requesting.
 
 Updated patch attached.

Thanks, looks good to me, although I haven't done a thorough review.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0167] ipa-client-install: Configure sudo to use SSSD as data source

2014-04-30 Thread Tomas Babej
On 03/24/2014 03:27 PM, Jan Pazdziora wrote:
 On Mon, Mar 24, 2014 at 02:57:30PM +0100, Martin Kosek wrote:
 On 03/24/2014 02:47 PM, Jan Pazdziora wrote:
 On Mon, Mar 03, 2014 at 08:24:41PM +0100, Tomas Babej wrote:
 Hi,

 Makes ipa-client-install configure SSSD as the data provider
 for the sudo service by default. This behaviour can be disabled
 by using --no-sudo flag.

 https://fedorahosted.org/freeipa/ticket/3358
 Ack.

 Applied against ipa-client-3.0.0-37.el6.x86_64, tried without
 --no-sudo and sudo was added to sssd.conf's services list and sudoeers
 added to /etc/nsswitch.conf.

 Rerun with --uninstall and run again with the --no-sudo parameter,
 those settings were not longer there.

 Did you also do the functional test?
 No. I do not want to get dragged into the discussion of having the
 correct sssd and sudo and glibc versions and SELinux and stuff. The
 ticket explicitly talk about setting configuration in config files,
 which the patch does.

 To ack and push this ticket, following
 scenario needs to work:
 Consumption of those configuration changes is really different story,
 isn't it?

 1) IPA clients enroll against IPA server without --no-sudo
 2) IPA client user logs in, types sudo -l, gets all allowed commands
 (prerequisite is of course to have sudo commands defined on the IPA server)
 3) IPA client reboots, IPA client user logs in, types sudo -l, gets all
 allowed commands

 For 2) to work, NIS domain name must be set, nsswitch and SSSD changes must 
 be done

 For 3) to work, related systemd service preserving NIS domain name setting
 needs to be enabled
 With the commit message only talking about configuring sssd, I assume
 the NIS domain name mentioned in the ticket will be done by some other
 patch.

 To me, the patch does what is advertised in the commit message, and is
 in line with what the ticket asks to be done.


Attached are rebased versions of the patches 113 and 167 (which was
marked as 157 in the thread previously by mistake).

There is a slight behaviour change in 167, if there is no sudoers line
in nsswitch.conf, we add both files and sss as sudoers sources.

I also developed CI test that covers the functionality of the IPA - sudo
integration feature, which is attached.

Please note that the last three tests are expected to fail until:

https://fedorahosted.org/freeipa/ticket/4324

is fixed.

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

From 0803c29840a79463fa838478bab65a061d779163 Mon Sep 17 00:00:00 2001
From: Tomas Babej tomasba...@gmail.com
Date: Thu, 21 Nov 2013 13:09:28 +0100
Subject: [PATCH] ipa-client-install: Configure sudo to use SSSD as data source

Makes ipa-client-install configure SSSD as the data provider
for the sudo service by default. This behaviour can be disabled
by using --no-sudo flag.

https://fedorahosted.org/freeipa/ticket/3358
---
 ipa-client/ipa-install/ipa-client-install | 58 ++-
 ipa-client/man/ipa-client-install.1   |  3 ++
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 5fdd51520ba667f240239077a80e328877c99cd7..99fed2ab08195c7dfe4fb876ce225bac7868eb3b 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -137,6 +137,9 @@ def parse_options():
   help=do not configure OpenSSH client)
 basic_group.add_option(--no-sshd, dest=conf_sshd, default=True, action=store_false,
   help=do not configure OpenSSH server)
+basic_group.add_option(--no-sudo, dest=conf_sudo, default=True,
+  action=store_false,
+  help=do not configure SSSD as data source for sudo)
 basic_group.add_option(--no-dns-sshfp, dest=create_sshfp, default=True, action=store_false,
   help=do not automatically create DNS SSHFP records)
 basic_group.add_option(--noac, dest=no_ac, default=False, action=store_true,
@@ -1141,6 +1144,59 @@ def configure_sssd_conf(fstore, cli_realm, cli_domain, cli_server, options, clie
 
 sssdconfig.activate_service('ssh')
 
+if options.conf_sudo:
+# Activate the service in the SSSD config
+try:
+sssdconfig.new_service('sudo')
+except SSSDConfig.ServiceAlreadyExists:
+pass
+except SSSDConfig.ServiceNotRecognizedError:
+root_logger.error(Unable to activate the SUDO service in 
+  SSSD config.)
+
+sssdconfig.activate_service('sudo')
+
+# Backup the nsswitch.conf, we're going to edit it now
+NSSWITCH_CONF = '/etc/nsswitch.conf'
+fstore.backup_file(NSSWITCH_CONF)
+
+conf = ipaclient.ipachangeconf.IPAChangeConf(IPA Installer)
+conf.setOptionAssignment(':')
+
+# Determine if nsswitch already contains files for sudoers or 

Re: [Freeipa-devel] [PATCH 0167] ipa-client-install: Configure sudo to use SSSD as data source

2014-04-30 Thread Jakub Hrozek
On Wed, Apr 30, 2014 at 11:05:52AM +0200, Tomas Babej wrote:
 On 03/24/2014 03:27 PM, Jan Pazdziora wrote:
  On Mon, Mar 24, 2014 at 02:57:30PM +0100, Martin Kosek wrote:
  On 03/24/2014 02:47 PM, Jan Pazdziora wrote:
  On Mon, Mar 03, 2014 at 08:24:41PM +0100, Tomas Babej wrote:
  Hi,
 
  Makes ipa-client-install configure SSSD as the data provider
  for the sudo service by default. This behaviour can be disabled
  by using --no-sudo flag.
 
  https://fedorahosted.org/freeipa/ticket/3358
  Ack.
 
  Applied against ipa-client-3.0.0-37.el6.x86_64, tried without
  --no-sudo and sudo was added to sssd.conf's services list and sudoeers
  added to /etc/nsswitch.conf.
 
  Rerun with --uninstall and run again with the --no-sudo parameter,
  those settings were not longer there.
 
  Did you also do the functional test?
  No. I do not want to get dragged into the discussion of having the
  correct sssd and sudo and glibc versions and SELinux and stuff. The
  ticket explicitly talk about setting configuration in config files,
  which the patch does.
 
  To ack and push this ticket, following
  scenario needs to work:
  Consumption of those configuration changes is really different story,
  isn't it?
 
  1) IPA clients enroll against IPA server without --no-sudo
  2) IPA client user logs in, types sudo -l, gets all allowed commands
  (prerequisite is of course to have sudo commands defined on the IPA server)
  3) IPA client reboots, IPA client user logs in, types sudo -l, gets all
  allowed commands
 
  For 2) to work, NIS domain name must be set, nsswitch and SSSD changes 
  must be done
 
  For 3) to work, related systemd service preserving NIS domain name setting
  needs to be enabled
  With the commit message only talking about configuring sssd, I assume
  the NIS domain name mentioned in the ticket will be done by some other
  patch.
 
  To me, the patch does what is advertised in the commit message, and is
  in line with what the ticket asks to be done.
 
 
 Attached are rebased versions of the patches 113 and 167 (which was
 marked as 157 in the thread previously by mistake).
 
 There is a slight behaviour change in 167, if there is no sudoers line
 in nsswitch.conf, we add both files and sss as sudoers sources.
 
 I also developed CI test that covers the functionality of the IPA - sudo
 integration feature, which is attached.
 
 Please note that the last three tests are expected to fail until:
 
 https://fedorahosted.org/freeipa/ticket/4324
 
 is fixed.
 
 -- 
 Tomas Babej
 Associate Software Engineer | Red Hat | Identity Management
 RHCE | Brno Site | IRC: tbabej | freeipa.org 
 

Hi,

I haven't done a thorough review, but the patch looks good to me in
general -- in other words, seems to cover what I've been doing manually
for my test setups.

My only suggestion (maybe for future) would be to split changing the
nsswitch.conf into its own separate helper class or a function, because
you might want to do the same change for automount or other services in
nsswitch.conf.

But I think this version is OK at the moment.

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel