Re: [PATCH nfs-utils] gssd: Check for AD style machine principal name

Re: [PATCH nfs-utils] gssd: Check for AD style machine principal name

[Timo Aaltonen]

On Mon, 28 Jun 2010, Timo Aaltonen wrote:

> On a MS Active Directory client the service principals cannot be used
> for authentication. Add a check for the default machine principal name
> so that krb5 auth works out of the box.
>
> Signed-off-by: Timo Aaltonen <timo.aaltonen-uOixanVlb7U@public.gmane.org>
> ---
>
> Resending, the previous try didn't get any comments.

Ping? Should it add a new option to try the default AD principal to get 
accepted?

> utils/gssd/krb5_util.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 58 insertions(+), 0 deletions(-)
>
> diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
> index dccbeb6..686ef3b 100644
> --- a/utils/gssd/krb5_util.c
> +++ b/utils/gssd/krb5_util.c
> @@ -737,6 +737,29 @@ gssd_search_krb5_keytab(krb5_context context, krb5_keytab kt,
> }
>
> /*
> + * Convert the hostname to machine principal name as created
> + * by MS Active Directory.
> +*/
> +
> +static char *
> +hostname_to_adprinc(char *name)
> +{
> +	int i = 0;
> +	int len = strlen(name);
> +	char *buf;
> +	if ((buf = malloc(len+2))) {
> +		while(i < len) {
> +			buf[i] = toupper(name[i]);
> +			i++;
> +		}
> +		buf[i++] = '$';
> +		buf[i] = 0;
> +		return buf;
> +	}
> +	return NULL;
> +}
> +
> +/*
>  * Find a keytab entry to use for a given target hostname.
>  * Tries to find the most appropriate keytab to use given the
>  * name of the host we are trying to connect with.
> @@ -754,6 +777,7 @@ find_keytab_entry(krb5_context context, krb5_keytab kt, const char *hostname,
> 	char *k5err = NULL;
> 	int tried_all = 0, tried_default = 0;
> 	krb5_principal princ;
> +	char *adprinc = NULL;
>
>
> 	/* Get full target hostname */
> @@ -769,6 +793,9 @@ find_keytab_entry(krb5_context context, krb5_keytab kt, const char *hostname,
> 		printerr(1, "%s while getting local hostname\n", k5err);
> 		goto out;
> 	}
> +	/* Convert to Active Directory machine principal name */
> +	adprinc = hostname_to_adprinc(myhostname);
> +
> 	retval = get_full_hostname(myhostname, myhostname, sizeof(myhostname));
> 	if (retval)
> 		goto out;
> @@ -812,6 +839,36 @@ find_keytab_entry(krb5_context context, krb5_keytab kt, const char *hostname,
> 			break;
> 		if (strcmp(realm, default_realm) == 0)
> 			tried_default = 1;
> +		/* First try the Active Directory style machine principal */
> +		if (adprinc != NULL) {
> +			code = krb5_build_principal_ext(context, &princ,
> +							strlen(realm),
> +							realm,
> +							strlen(adprinc),
> +							adprinc,
> +							NULL);
> +			if (code) {
> +				k5err = gssd_k5_err_msg(context, code);
> +				printerr(1, "%s while building principal for "
> +					 "'%s@%s'\n", k5err,
> +					 adprinc, realm);
> +			}
> +			code = krb5_kt_get_entry(context, kt, princ, 0, 0, kte);
> +			krb5_free_principal(context, princ);
> +			if (code) {
> +				k5err = gssd_k5_err_msg(context, code);
> +				printerr(3, "%s while getting keytab entry for "
> +					 "'%s@%s'\n", k5err,
> +					 adprinc, realm);
> +			} else {
> +				printerr(3, "Success getting keytab entry for "
> +					 "'%s@%s'\n",
> +					 adprinc, realm);
> +				retval = 0;
> +				goto out;
> +			}
> +			retval =code;
> +		}
> 		for (j = 0; svcnames[j] != NULL; j++) {
> 			code = krb5_build_principal_ext(context, &princ,
> 							strlen(realm),
> @@ -870,6 +927,7 @@ out:
> 	if (realmnames)
> 		krb5_free_host_realm(context, realmnames);
> 	free(k5err);
> +	free(adprinc);
> 	return retval;
> }
>
> -- 
> 1.7.0.4
>
>

-- 
Timo Aaltonen
Systems Specialist, Aalto IT
tel. +358-9-47024317, mobile: +358-50-5918781
http://users.tkk.fi/~tjaalton

Re: [PATCH nfs-utils] gssd: Check for AD style machine principal name

[J. Bruce Fields]

On Fri, Aug 06, 2010 at 11:59:10AM +0300, Timo Aaltonen wrote:
> On Mon, 28 Jun 2010, Timo Aaltonen wrote:
> 
> >On a MS Active Directory client the service principals cannot be used
> >for authentication. Add a check for the default machine principal name
> >so that krb5 auth works out of the box.
> >
> >Signed-off-by: Timo Aaltonen <timo.aaltonen@aalto.fi>
> >---
> >
> >Resending, the previous try didn't get any comments.
> 
> Ping? Should it add a new option to try the default AD principal to
> get accepted?

find_keytab_entry() is already quite long, has multiple nested loops,
and I find the control flow hard to follow.  This piles a little more
on.

Could you look into moving some of that code into logically-named helper
functions and clarifying the control flow?  Ideally  you could do that
in a preliminary patch that cleaned up the existing code, followed by an
update of this patch.  That might be easier to review.

--b.

> 
> >utils/gssd/krb5_util.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++++
> >1 files changed, 58 insertions(+), 0 deletions(-)
> >
> >diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
> >index dccbeb6..686ef3b 100644
> >--- a/utils/gssd/krb5_util.c
> >+++ b/utils/gssd/krb5_util.c
> >@@ -737,6 +737,29 @@ gssd_search_krb5_keytab(krb5_context context, krb5_keytab kt,
> >}
> >
> >/*
> >+ * Convert the hostname to machine principal name as created
> >+ * by MS Active Directory.
> >+*/
> >+
> >+static char *
> >+hostname_to_adprinc(char *name)
> >+{
> >+	int i = 0;
> >+	int len = strlen(name);
> >+	char *buf;
> >+	if ((buf = malloc(len+2))) {
> >+		while(i < len) {
> >+			buf[i] = toupper(name[i]);
> >+			i++;
> >+		}
> >+		buf[i++] = '$';
> >+		buf[i] = 0;
> >+		return buf;
> >+	}
> >+	return NULL;
> >+}
> >+
> >+/*
> > * Find a keytab entry to use for a given target hostname.
> > * Tries to find the most appropriate keytab to use given the
> > * name of the host we are trying to connect with.
> >@@ -754,6 +777,7 @@ find_keytab_entry(krb5_context context, krb5_keytab kt, const char *hostname,
> >	char *k5err = NULL;
> >	int tried_all = 0, tried_default = 0;
> >	krb5_principal princ;
> >+	char *adprinc = NULL;
> >
> >
> >	/* Get full target hostname */
> >@@ -769,6 +793,9 @@ find_keytab_entry(krb5_context context, krb5_keytab kt, const char *hostname,
> >		printerr(1, "%s while getting local hostname\n", k5err);
> >		goto out;
> >	}
> >+	/* Convert to Active Directory machine principal name */
> >+	adprinc = hostname_to_adprinc(myhostname);
> >+
> >	retval = get_full_hostname(myhostname, myhostname, sizeof(myhostname));
> >	if (retval)
> >		goto out;
> >@@ -812,6 +839,36 @@ find_keytab_entry(krb5_context context, krb5_keytab kt, const char *hostname,
> >			break;
> >		if (strcmp(realm, default_realm) == 0)
> >			tried_default = 1;
> >+		/* First try the Active Directory style machine principal */
> >+		if (adprinc != NULL) {
> >+			code = krb5_build_principal_ext(context, &princ,
> >+							strlen(realm),
> >+							realm,
> >+							strlen(adprinc),
> >+							adprinc,
> >+							NULL);
> >+			if (code) {
> >+				k5err = gssd_k5_err_msg(context, code);
> >+				printerr(1, "%s while building principal for "
> >+					 "'%s@%s'\n", k5err,
> >+					 adprinc, realm);
> >+			}
> >+			code = krb5_kt_get_entry(context, kt, princ, 0, 0, kte);
> >+			krb5_free_principal(context, princ);
> >+			if (code) {
> >+				k5err = gssd_k5_err_msg(context, code);
> >+				printerr(3, "%s while getting keytab entry for "
> >+					 "'%s@%s'\n", k5err,
> >+					 adprinc, realm);
> >+			} else {
> >+				printerr(3, "Success getting keytab entry for "
> >+					 "'%s@%s'\n",
> >+					 adprinc, realm);
> >+				retval = 0;
> >+				goto out;
> >+			}
> >+			retval =code;
> >+		}
> >		for (j = 0; svcnames[j] != NULL; j++) {
> >			code = krb5_build_principal_ext(context, &princ,
> >							strlen(realm),
> >@@ -870,6 +927,7 @@ out:
> >	if (realmnames)
> >		krb5_free_host_realm(context, realmnames);
> >	free(k5err);
> >+	free(adprinc);
> >	return retval;
> >}
> >
> >-- 
> >1.7.0.4
> >
> >
> 
> -- 
> Timo Aaltonen
> Systems Specialist, Aalto IT
> tel. +358-9-47024317, mobile: +358-50-5918781
> http://users.tkk.fi/~tjaalton
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH nfs-utils] gssd: Check for AD style machine principal name

[Timo Aaltonen]

On Fri, 6 Aug 2010, J. Bruce Fields wrote:

> On Fri, Aug 06, 2010 at 11:59:10AM +0300, Timo Aaltonen wrote:
>> On Mon, 28 Jun 2010, Timo Aaltonen wrote:
>>
>>> On a MS Active Directory client the service principals cannot be used
>>> for authentication. Add a check for the default machine principal name
>>> so that krb5 auth works out of the box.
>>>
>>> Signed-off-by: Timo Aaltonen <timo.aaltonen-uOixanVlb7U@public.gmane.org>
>>> ---
>>>
>>> Resending, the previous try didn't get any comments.
>>
>> Ping? Should it add a new option to try the default AD principal to
>> get accepted?
>
> find_keytab_entry() is already quite long, has multiple nested loops,
> and I find the control flow hard to follow.  This piles a little more
> on.
>
> Could you look into moving some of that code into logically-named helper
> functions and clarifying the control flow?  Ideally  you could do that
> in a preliminary patch that cleaned up the existing code, followed by an
> update of this patch.  That might be easier to review.

Yeah, you're right. I'll look into it and post patches to clean it up.



>>> utils/gssd/krb5_util.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 files changed, 58 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/utils/gssd/krb5_util.c b/utils/gssd/krb5_util.c
>>> index dccbeb6..686ef3b 100644
>>> --- a/utils/gssd/krb5_util.c
>>> +++ b/utils/gssd/krb5_util.c
>>> @@ -737,6 +737,29 @@ gssd_search_krb5_keytab(krb5_context context, krb5_keytab kt,
>>> }
>>>
>>> /*
>>> + * Convert the hostname to machine principal name as created
>>> + * by MS Active Directory.
>>> +*/
>>> +
>>> +static char *
>>> +hostname_to_adprinc(char *name)
>>> +{
>>> +	int i = 0;
>>> +	int len = strlen(name);
>>> +	char *buf;
>>> +	if ((buf = malloc(len+2))) {
>>> +		while(i < len) {
>>> +			buf[i] = toupper(name[i]);
>>> +			i++;
>>> +		}
>>> +		buf[i++] = '$';
>>> +		buf[i] = 0;
>>> +		return buf;
>>> +	}
>>> +	return NULL;
>>> +}
>>> +
>>> +/*
>>> * Find a keytab entry to use for a given target hostname.
>>> * Tries to find the most appropriate keytab to use given the
>>> * name of the host we are trying to connect with.
>>> @@ -754,6 +777,7 @@ find_keytab_entry(krb5_context context, krb5_keytab kt, const char *hostname,
>>> 	char *k5err = NULL;
>>> 	int tried_all = 0, tried_default = 0;
>>> 	krb5_principal princ;
>>> +	char *adprinc = NULL;
>>>
>>>
>>> 	/* Get full target hostname */
>>> @@ -769,6 +793,9 @@ find_keytab_entry(krb5_context context, krb5_keytab kt, const char *hostname,
>>> 		printerr(1, "%s while getting local hostname\n", k5err);
>>> 		goto out;
>>> 	}
>>> +	/* Convert to Active Directory machine principal name */
>>> +	adprinc = hostname_to_adprinc(myhostname);
>>> +
>>> 	retval = get_full_hostname(myhostname, myhostname, sizeof(myhostname));
>>> 	if (retval)
>>> 		goto out;
>>> @@ -812,6 +839,36 @@ find_keytab_entry(krb5_context context, krb5_keytab kt, const char *hostname,
>>> 			break;
>>> 		if (strcmp(realm, default_realm) == 0)
>>> 			tried_default = 1;
>>> +		/* First try the Active Directory style machine principal */
>>> +		if (adprinc != NULL) {
>>> +			code = krb5_build_principal_ext(context, &princ,
>>> +							strlen(realm),
>>> +							realm,
>>> +							strlen(adprinc),
>>> +							adprinc,
>>> +							NULL);
>>> +			if (code) {
>>> +				k5err = gssd_k5_err_msg(context, code);
>>> +				printerr(1, "%s while building principal for "
>>> +					 "'%s@%s'\n", k5err,
>>> +					 adprinc, realm);
>>> +			}
>>> +			code = krb5_kt_get_entry(context, kt, princ, 0, 0, kte);
>>> +			krb5_free_principal(context, princ);
>>> +			if (code) {
>>> +				k5err = gssd_k5_err_msg(context, code);
>>> +				printerr(3, "%s while getting keytab entry for "
>>> +					 "'%s@%s'\n", k5err,
>>> +					 adprinc, realm);
>>> +			} else {
>>> +				printerr(3, "Success getting keytab entry for "
>>> +					 "'%s@%s'\n",
>>> +					 adprinc, realm);
>>> +				retval = 0;
>>> +				goto out;
>>> +			}
>>> +			retval =code;
>>> +		}
>>> 		for (j = 0; svcnames[j] != NULL; j++) {
>>> 			code = krb5_build_principal_ext(context, &princ,
>>> 							strlen(realm),
>>> @@ -870,6 +927,7 @@ out:
>>> 	if (realmnames)
>>> 		krb5_free_host_realm(context, realmnames);
>>> 	free(k5err);
>>> +	free(adprinc);
>>> 	return retval;
>>> }
>>>
>>> --
>>> 1.7.0.4
>>>
>>>
>>
>> --
>> Timo Aaltonen
>> Systems Specialist, Aalto IT
>> tel. +358-9-47024317, mobile: +358-50-5918781
>> http://users.tkk.fi/~tjaalton
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

-- 
Timo Aaltonen
Systems Specialist, Aalto IT
tel. +358-9-47024317, mobile: +358-50-5918781
http://users.tkk.fi/~tjaalton