[SSSD] [PATCH] AD GPO: Fix incorrect sAMAccountName selection

Jakub Hrozek jhrozek at redhat.com
Thu Sep 25 08:56:36 UTC 2014


On Wed, Sep 24, 2014 at 11:10:00AM -0400, Stephen Gallagher wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> We were assuming that the ad_hostname value would match the
> sAMAccountName attribute, but in practice this was almost never the
> case on a properly-configured system.
> 
> Microsoft's convention is that the sAMAccountName is always the
> portion of the FQDN before the first dot, so this patch makes that
> same assumption.
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1
> 
> iEYEARECAAYFAlQi3sgACgkQeiVVYja6o6OJDwCfRgV66Mp/oLWfNXXamXMC7S1i
> KYwAoK4H6eogurMto2oUVM6V9pVDbZ0C
> =1Nj3
> -----END PGP SIGNATURE-----

> From c179806c27ce6d25137306ba7bb37ecfae573c3b Mon Sep 17 00:00:00 2001
> From: Stephen Gallagher <sgallagh at redhat.com>
> Date: Tue, 23 Sep 2014 17:44:41 -0400
> Subject: [PATCH] AD GPO: Fix incorrect sAMAccountName selection
> 
> ---
>  src/providers/ad/ad_gpo.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
> index de4d44166b85ccd85ed36bcb11f0596e0020af11..745af8b2786a5d6c71a2a3eb6c1448a61c151019 100644
> --- a/src/providers/ad/ad_gpo.c
> +++ b/src/providers/ad/ad_gpo.c
> @@ -1479,6 +1479,8 @@ ad_gpo_connect_done(struct tevent_req *subreq)
>      struct tevent_req *req;
>      struct ad_gpo_access_state *state;
>      char *filter;
> +    char *hostname;
> +    char *shortname;
>      char *sam_account_name;
>      char *domain_dn;
>      int dp_error;
> @@ -1519,7 +1521,21 @@ ad_gpo_connect_done(struct tevent_req *subreq)
>          }
>      }
>  
> -    sam_account_name = talloc_asprintf(state, "%s$", state->ad_hostname);
> +    hostname = talloc_strdup(state, state->ad_hostname);
> +    if (hostname == NULL) {
> +        ret = ENOMEM;
> +        goto done;
> +    }
> +    shortname = strtok(hostname, ".");
> +    if (shortname == NULL) {
> +        /* This should never fail; if there's no dot,
> +         * it should return the full string.
> +         */
> +        ret = EIO;
> +        goto done;
> +    }
> +    sam_account_name = talloc_asprintf(state, "%s$", hostname);
> +    talloc_zfree(hostname);
>      if (sam_account_name == NULL) {
>          ret = ENOMEM;
>          goto done;

The fix works, but we already have code that does pretty much the same
for principal selection -- check out get_primary() in sss_krb5.c. Would
it be better to split out lines 38 to 53 from get_primary() into a
separate function and use it in ad_gpo.c, too, to save code duplication?

> @@ -1548,6 +1564,8 @@ ad_gpo_connect_done(struct tevent_req *subreq)
>          goto done;
>      }
>  
> +    talloc_zfree(sam_account_name);
> +
>      subreq = sdap_get_generic_send(state, state->ev, state->opts,
>                                     sdap_id_op_handle(state->sdap_op),
>                                     domain_dn, LDAP_SCOPE_SUBTREE,
> -- 
> 2.1.0
> 


> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel




More information about the sssd-devel mailing list