On (15/01/16 16:44), 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 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
>
> 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.
>
> 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.
[...]
> +static errno_t get_adcli_extra_args(struct renewal_data *renewal_data)
> +{
> + const char **args;
> + size_t c = 0;
> +
> + renewal_data->prog_path = talloc_strdup(renewal_data, RENEWAL_PROG_PATH);
We don't need to strdup a constant..
> + if (renewal_data->prog_path == NULL) {
> + DEBUG(SSSDBG_OP_FAILURE, "talloc_strdup failed.\n");
> + return ENOMEM;
> + }
> +
> + args = talloc_array(renewal_data, const char *, 7);
[...]
> +static void ad_machine_account_password_renewal_done(struct tevent_req *subreq)
> +{
> + uint8_t *buf;
> + ssize_t buf_len;
> + struct tevent_req *req = tevent_req_callback_data(subreq,
> + struct tevent_req);
> + struct renewal_state *state = tevent_req_data(req, struct renewal_state);
> + int ret;
> +
> + talloc_zfree(state->timeout_handler);
> +
> + ret = read_pipe_recv(subreq, state, &buf, &buf_len);
> + talloc_zfree(subreq);
> + if (ret != EOK) {
> + tevent_req_error(req, ret);
> + return;
> + }
> +
> + DEBUG(SSSDBG_TRACE_LIBS, "--- adcli output start---\n"
> + "%.*s"
> + "---adcli output end---\n",
> + buf_len, buf);
I didn't see this myself, but coverity detected an issue here:
sssd-1.13.90/src/providers/ad/ad_machine_pw_renewal.c: scope_hint: In function
'ad_machine_account_password_renewal_done'
sssd-1.13.90/src/providers/ad/ad_machine_pw_renewal.c:237:5: warning: field precision
specifier '.*' expects argument of type 'int', but argument 6 has type
'ssize_t' [-Wformat=]
# DEBUG(SSSDBG_TRACE_LIBS, "--- adcli output start---\n"
# ^
# 235| }
# 236|
# 237|-> DEBUG(SSSDBG_TRACE_LIBS, "--- adcli output start---\n"
# 238| "%.*s"
# 239| "---adcli output end---\n",
It's not a coverity. It's caused by extra gcc warnings.
Explicit cast to int should remove warning.
LS