* Re: [PATCH nfs-utils] gssd: Check for AD style machine principal name
[not found] ` <1277729907-14299-1-git-send-email-timo.aaltonen-uOixanVlb7U@public.gmane.org>
@ 2010-08-06 8:59 ` Timo Aaltonen
2010-08-06 17:18 ` J. Bruce Fields
0 siblings, 1 reply; 3+ messages in thread
From: Timo Aaltonen @ 2010-08-06 8:59 UTC (permalink / raw)
To: linux-nfs; +Cc: steved
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH nfs-utils] gssd: Check for AD style machine principal name
2010-08-06 8:59 ` [PATCH nfs-utils] gssd: Check for AD style machine principal name Timo Aaltonen
@ 2010-08-06 17:18 ` J. Bruce Fields
2010-08-07 12:48 ` Timo Aaltonen
0 siblings, 1 reply; 3+ messages in thread
From: J. Bruce Fields @ 2010-08-06 17:18 UTC (permalink / raw)
To: Timo Aaltonen; +Cc: linux-nfs, steved
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH nfs-utils] gssd: Check for AD style machine principal name
2010-08-06 17:18 ` J. Bruce Fields
@ 2010-08-07 12:48 ` Timo Aaltonen
0 siblings, 0 replies; 3+ messages in thread
From: Timo Aaltonen @ 2010-08-07 12:48 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: linux-nfs, steved
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-08-07 12:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1277729907-14299-1-git-send-email-timo.aaltonen@aalto.fi>
[not found] ` <1277729907-14299-1-git-send-email-timo.aaltonen-uOixanVlb7U@public.gmane.org>
2010-08-06 8:59 ` [PATCH nfs-utils] gssd: Check for AD style machine principal name Timo Aaltonen
2010-08-06 17:18 ` J. Bruce Fields
2010-08-07 12:48 ` Timo Aaltonen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox