On Fri, Jan 15, 2016 at 11:45:59AM +0100, Jakub Hrozek wrote:
On Fri, Jan 15, 2016 at 10:44:44AM +0100, Sumit Bose wrote:
> On Thu, Jan 14, 2016 at 05:54:06PM +0100, Sumit Bose wrote:
> > Hi,
> >
> > this patch adds a task to the AD provider which calls adcli on a regular
> > basis to update the machine account password if needed. adcli supports
> > this functionality since version 0.8.0. Adding support other utilities like
> > msktutil shouldn't be hard.
> >
> > Since adcli (and other external tools) does not understand the SSSD
> > default options like --debug_level a change in exec_child_ex() was
> > needed which is covered in the first patch.
> >
> > bye,
> > Sumit
>
> patches are now rebased on current master.
>
> bye,
> Sumit
> From d69bde0f81e92b296d3bdfcd8e5b5dd89cee67f7 Mon Sep 17 00:00:00 2001
> From: Sumit Bose <sbose(a)redhat.com>
> Date: Thu, 14 Jan 2016 13:33:53 +0100
> Subject: [PATCH 1/2] UTIL: allow to skip default options for child processes
Ack but would you agree with squashing in
https://github.com/jhrozek/sssd/commit/bad5f16143ad67b5027d9bc769a846db44...
?
of course, but please do not squash it in but use a separate commit to
get the credits right. If you improve the commit message a bit ACK to
your patch.
> From eee0ca691dbf19942b479122e14f413b45b2ba39 Mon Sep 17 00:00:00 2001
> From: Sumit Bose <sbose(a)redhat.com>
> Date: Tue, 12 Jan 2016 11:05:02 +0100
> Subject: [PATCH 2/2] AD: add task to renew the machine account password if
> needed
So far I only read the code and have some questions..
>
> AD expects its clients to renew the machine account password on a
> regular basis, be default every 30 days. Even if a client does not renew
> the password it might not cause issues because AD does not enforce the
> renewal. But the password age might be used to identify unused machine
> accounts in large environments which might get disabled or deleted
> automatically.
Do you know if the machine might then be disabled right after the 30
days? Would it make sense to try to renew the keytab sooner (maybe after
20 days or so) to factor in a machine that might be offline, maybe
someone travelling with a laptop?
I would expect that if such enforcement is in place there will be a
grace period. If not ad_maximum_machine_account_password_age can be set
to a suitable value for such an environment.
>
> With this patch SSSD calls an external program to check the age of the
> machine account password and renew it if needed. Currently 'adcli' is
> used as external program which is able to renew the password since
> version 0.8.0.
~~~~~~~~~~~~~
What would happen if the user had an older adcli version? Some users run
RHEL-6 with adcli from EPEL. Should a downstream add a versioned
https://dl.fedoraproject.org/pub/epel/6/x86_64/adcli-0.8.0-1.el6.x86_64.rpm :-)
Requirement?
For every downstream which offers adcli-0.8.0 or newer I think it would
make sense to add a Requires to the package containing the AD provider,
because renewing the password is the expected default behaviour of an AD
client.
If an older version of adcli is used SSSD will try to start adcli but
adcli will complain about unknown options. The stderr of adcli is added
to the logs with debug_level 6 or higher.
>
> Resolves
https://fedorahosted.org/sssd/ticket/1041
> ---
> Makefile.am | 1 +
> src/config/SSSDConfig/__init__.py.in | 2 +
> src/config/etc/sssd.api.d/sssd-ad.conf | 2 +
> src/man/sssd-ad.5.xml | 33 +++
> src/providers/ad/ad_common.h | 5 +
> src/providers/ad/ad_init.c | 7 +
> src/providers/ad/ad_machine_pw_renewal.c | 376 +++++++++++++++++++++++++++++++
> src/providers/ad/ad_opts.c | 2 +
> src/util/util_errors.c | 1 +
> src/util/util_errors.h | 1 +
> 10 files changed, 430 insertions(+)
> create mode 100644 src/providers/ad/ad_machine_pw_renewal.c
[...]
> +#include "util/util.h"
> +#include "util/strtonum.h"
> +#include "providers/dp_ptask.h"
> +#include "providers/ad/ad_common.h"
> +
> +#ifndef RENEWAL_PROG_PATH
> +#define RENEWAL_PROG_PATH "/usr/sbin/adcli"
> +#endif
Do you think it would make sense to provide a configure time option as
well for distributions that would package adcli elsewhere (or try to
detect adcli and fall back to this default) ?
yes, but I wanted to wait until I (or someone else :-) will add support
for msktutil as well.
bye,
Sumit