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 <sb...@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
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
