[PATCH v3] fixes account locked when using winbind refresh tickets

[Andreas Schneider]

On Monday, 15 January 2018 16:11:14 CET Ralph Böhme via samba-technical wrote:
> Hi David,
> 
> On Mon, Jan 15, 2018 at 07:52:40AM -0700, David Mulder wrote:
> > Touch ups recommended by Andreas (null initialize, helper variables, etc).
> 
> thanks, much better! :)
> 
> Can you please also simplify a few if conditions and use early exits, eg you
> have
> 
> +       at_ptr = strchr(entry->principal_name, '@');
> +       if (at_ptr != NULL) {
> +               int strlen = at_ptr - entry->principal_name;
> +               sam = talloc_strndup(ctx, entry->principal_name, strlen);
> +       } else {
> +               DBG_DEBUG("Could not determine samAccountName from %s\n",
> +                         entry->principal_name);
> +               goto fail;
> +       }
> 
> Simpler:
> 
> +       int strlen;
>         ...
> +       at_ptr = strchr(entry->principal_name, '@');
> +       if (at_ptr == NULL) {
> +               DBG_DEBUG("Could not determine samAccountName from %s\n",
> +                         entry->principal_name);
> +               goto fail;
> +       }
> +
> +       strlen = at_ptr - entry->principal_name;
> +       sam = talloc_strndup(ctx, entry->principal_name, strlen);
> 
> Similar for the hunk that adds ads_do_search_all().
> 
> I guess is too much asking for a refactoring of
> krb5_ticket_refresh_handler() as part of the fix? This function is pure
> horror.

And I've spotted something else:

+static uint64_t fetch_last_password_change(struct WINBINDD_CCACHE_ENTRY 
*entry)
+{
+	TALLOC_CTX *ctx = talloc_new(entry);

Please use:

	TALLOC_CTX *frame = talloc_stackframe();

This is cheaper than talloc_new().


	Andreas

-- 
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             asn at samba.org
www.samba.org