* Re: [PATCH] Check for AD style machine principal name [not found] ` <560_1272392934_4BD72CE6_560_482_1_20100427182806.GP30729@fieldses.org> @ 2010-05-05 16:53 ` Timo Aaltonen [not found] ` <alpine.DEB.2.00.1005051934040.3282-9o+TK994UVukEuhtYYxXtw@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Timo Aaltonen @ 2010-05-05 16:53 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs CC:ing the "new" list. On Tue, 27 Apr 2010, J. Bruce Fields wrote: > On Mon, Apr 26, 2010 at 05:00:59PM +0300, timo.aaltonen-uOixanVlb7U@public.gmane.org wrote: >> @@ -737,6 +737,26 @@ 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; >> + char *buf = strdup(name); > > We should handle the possible NULL return. How about not calling hostname_to_adprinc from find_keytab_entry unless myhostname is != NULL? >> + int len = strlen(name); >> + while(i < len) { >> + buf[i] = toupper(buf[i]); >> + i++; >> + } >> + buf[i++] = '$'; >> + buf[i] = 0; > > This is past the end of the buffer. Maybe you want buf to be > strlen(name)+1? Ok, so: char *buf = malloc(sizeof(name)+1); buf = strdup(name); . . works, and should be ok? >> @@ -812,6 +836,35 @@ 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 */ >> + 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); >> + continue; > > Is there an infinite loop here? Not to my knowledge, it's basically the same chunk of code copied from a few lines below, modified to suit the adprinc. -- Timo Aaltonen Systems Specialist IT Services, Aalto University ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <alpine.DEB.2.00.1005051934040.3282-9o+TK994UVukEuhtYYxXtw@public.gmane.org>]
* Re: [PATCH] Check for AD style machine principal name [not found] ` <alpine.DEB.2.00.1005051934040.3282-9o+TK994UVukEuhtYYxXtw@public.gmane.org> @ 2010-05-05 17:36 ` J. Bruce Fields 2010-05-05 17:46 ` Kevin Coffman 1 sibling, 0 replies; 7+ messages in thread From: J. Bruce Fields @ 2010-05-05 17:36 UTC (permalink / raw) To: Timo Aaltonen; +Cc: linux-nfs On Wed, May 05, 2010 at 07:53:33PM +0300, Timo Aaltonen wrote: > > CC:ing the "new" list. > > On Tue, 27 Apr 2010, J. Bruce Fields wrote: > >> On Mon, Apr 26, 2010 at 05:00:59PM +0300, timo.aaltonen-uOixanVlb7U@public.gmane.org wrote: >>> @@ -737,6 +737,26 @@ 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; >>> + char *buf = strdup(name); >> >> We should handle the possible NULL return. > > How about not calling hostname_to_adprinc from find_keytab_entry unless > myhostname is != NULL? I'm not sure what you mean. I'm just pointing out that strdup() can return NULL if the memory allocation fails. > >>> + int len = strlen(name); >>> + while(i < len) { >>> + buf[i] = toupper(buf[i]); >>> + i++; >>> + } >>> + buf[i++] = '$'; >>> + buf[i] = 0; >> >> This is past the end of the buffer. Maybe you want buf to be >> strlen(name)+1? > > Ok, so: > char *buf = malloc(sizeof(name)+1); > buf = strdup(name); > . > . > works, and should be ok? The strdup should be a strcpy. > >>> @@ -812,6 +836,35 @@ 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 */ >>> + 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); >>> + continue; >> >> Is there an infinite loop here? > > Not to my knowledge, We need a more convincing argument than that, unless someone else has the cycles to take this over! --b. > it's basically the same chunk of code copied from a > few lines below, modified to suit the adprinc. > > > -- > Timo Aaltonen > Systems Specialist > IT Services, Aalto University ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Check for AD style machine principal name [not found] ` <alpine.DEB.2.00.1005051934040.3282-9o+TK994UVukEuhtYYxXtw@public.gmane.org> 2010-05-05 17:36 ` J. Bruce Fields @ 2010-05-05 17:46 ` Kevin Coffman [not found] ` <n2s4d569c331005051046t28cd5dch1e5dfe4a73b987bf-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 1 sibling, 1 reply; 7+ messages in thread From: Kevin Coffman @ 2010-05-05 17:46 UTC (permalink / raw) To: Timo Aaltonen; +Cc: J. Bruce Fields, linux-nfs On Wed, May 5, 2010 at 12:53 PM, Timo Aaltonen <timo.aaltonen-uOixanVlb7U@public.gmane.org>= wrote: > > CC:ing the "new" list. > > On Tue, 27 Apr 2010, J. Bruce Fields wrote: > >> On Mon, Apr 26, 2010 at 05:00:59PM +0300, timo.aaltonen-uOixanVlb7U@public.gmane.org wro= te: >>> >>> @@ -737,6 +737,26 @@ gssd_search_krb5_keytab(krb5_context context, >>> krb5_keytab kt, >>> =A0} >>> >>> =A0/* >>> + * Convert the hostname to machine principal name as created >>> + * by MS Active Directory. >>> +*/ >>> + >>> +static char * >>> +hostname_to_adprinc(char *name) >>> +{ >>> + =A0 =A0 =A0 int i =3D 0; >>> + =A0 =A0 =A0 char *buf =3D strdup(name); >> >> We should handle the possible NULL return. > > How about not calling hostname_to_adprinc from find_keytab_entry unle= ss > myhostname is !=3D NULL? I think he meant that strdup() may fail, and return NULL. That has to be handled here, and a possible resulting NULL return from host_to_adprinc() should also be handled below. >>> + =A0 =A0 =A0 int len =3D strlen(name); >>> + =A0 =A0 =A0 while(i < len) { >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 buf[i] =3D toupper(buf[i]); >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 i++; >>> + =A0 =A0 =A0 } >>> + =A0 =A0 =A0 buf[i++] =3D '$'; >>> + =A0 =A0 =A0 buf[i] =3D 0; >> >> This is past the end of the buffer. =A0Maybe you want buf to be >> strlen(name)+1? > > Ok, so: > =A0 =A0 =A0 =A0char *buf =3D malloc(sizeof(name)+1); > =A0 =A0 =A0 =A0buf =3D strdup(name); > =A0 =A0 =A0 =A0. > =A0 =A0 =A0 =A0. > works, and should be ok? > >>> @@ -812,6 +836,35 @@ find_keytab_entry(krb5_context context, krb5_k= eytab >>> kt, const char *hostname, >>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break; >>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (strcmp(realm, default_realm) =3D= =3D 0) >>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tried_default =3D 1; >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* First try the Active Directory sty= le machine principal >>> */ >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 code =3D krb5_build_principal_ext(con= text, &princ, >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 strlen(realm), >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 realm, >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 strlen(adprinc), >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 adprinc, >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 NULL); >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (code) { >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 k5err =3D gssd_k5_err= _msg(context, code); >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 printerr(1, "%s while= building principal for " >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"'= %s@%s'\n", k5err, >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ad= princ, realm); >>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 continue; >> >> Is there an infinite loop here? > > Not to my knowledge, it's basically the same chunk of code copied fro= m a few > lines below, modified to suit the adprinc. The chunk you copied was within a for() loop. You are calling continue within the while(1) loop. ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <n2s4d569c331005051046t28cd5dch1e5dfe4a73b987bf-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] Check for AD style machine principal name [not found] ` <n2s4d569c331005051046t28cd5dch1e5dfe4a73b987bf-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2010-05-05 22:15 ` Timo Aaltonen [not found] ` <alpine.DEB.2.00.1005052347130.3282-9o+TK994UVukEuhtYYxXtw@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Timo Aaltonen @ 2010-05-05 22:15 UTC (permalink / raw) To: Kevin Coffman; +Cc: J. Bruce Fields, linux-nfs On Wed, 5 May 2010, Kevin Coffman wrote: > On Wed, May 5, 2010 at 12:53 PM, Timo Aaltonen <timo.aaltonen@aalto.f= i> wrote: >> >> CC:ing the "new" list. >> >> On Tue, 27 Apr 2010, J. Bruce Fields wrote: >> >>> On Mon, Apr 26, 2010 at 05:00:59PM +0300, timo.aaltonen-uOixanVlb7U@public.gmane.org wr= ote: >>>> >>>> @@ -737,6 +737,26 @@ gssd_search_krb5_keytab(krb5_context context, >>>> krb5_keytab kt, >>>> =A0} >>>> >>>> =A0/* >>>> + * Convert the hostname to machine principal name as created >>>> + * by MS Active Directory. >>>> +*/ >>>> + >>>> +static char * >>>> +hostname_to_adprinc(char *name) >>>> +{ >>>> + =A0 =A0 =A0 int i =3D 0; >>>> + =A0 =A0 =A0 char *buf =3D strdup(name); >>> >>> We should handle the possible NULL return. >> >> How about not calling hostname_to_adprinc from find_keytab_entry unl= ess >> myhostname is !=3D NULL? > > I think he meant that strdup() may fail, and return NULL. That has t= o > be handled here, and a possible resulting NULL return from > host_to_adprinc() should also be handled below. Ahh.. ok so how about this: static char * hostname_to_adprinc(char *name) { int i =3D 0; int len =3D strlen(name); char *buf; if ((buf =3D malloc(len+1))) { strcpy(buf, name); while(i < len) { buf[i] =3D toupper(buf[i]); i++; } buf[i++] =3D '$'; buf[i] =3D 0; return buf; } return NULL; } and then adding 'if (adprinc !=3D NULL) { ... }' to the while loop in=20 find_keytab_entry. That passes my tests. >>>> @@ -812,6 +836,35 @@ find_keytab_entry(krb5_context context, krb5_= keytab >>>> kt, const char *hostname, >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break; >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (strcmp(realm, default_realm) =3D= =3D 0) >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tried_default =3D 1= ; >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* First try the Active Directory st= yle machine principal >>>> */ >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 code =3D krb5_build_principal_ext(co= ntext, &princ, >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 strlen(realm), >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 realm, >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 strlen(adprinc), >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 adprinc, >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 NULL); >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (code) { >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 k5err =3D gssd_k5_er= r_msg(context, code); >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 printerr(1, "%s whil= e building principal for " >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"= '%s@%s'\n", k5err, >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0a= dprinc, realm); >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 continue; >>> >>> Is there an infinite loop here? >> >> Not to my knowledge, it's basically the same chunk of code copied fr= om a few >> lines below, modified to suit the adprinc. > > The chunk you copied was within a for() loop. You are calling > continue within the while(1) loop. Ok, so 'continue' was extra, whoops.. --=20 Timo Aaltonen Systems Specialist IT Services, Aalto University ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <alpine.DEB.2.00.1005052347130.3282-9o+TK994UVukEuhtYYxXtw@public.gmane.org>]
* RE: [PATCH] Check for AD style machine principal name [not found] ` <alpine.DEB.2.00.1005052347130.3282-9o+TK994UVukEuhtYYxXtw@public.gmane.org> @ 2010-05-05 22:25 ` Staubach_Peter [not found] ` <BF3BB6D12298F54B89C8DCC1E4073D80A24712-1Zg0zMUlrbd9m/dOYFj4Yjjd7nCn89gW@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Staubach_Peter @ 2010-05-05 22:25 UTC (permalink / raw) To: timo.aaltonen-uOixanVlb7U, kwc; +Cc: bfields, linux-nfs -----Original Message----- =46rom: linux-nfs-owner@vger.kernel.org [mailto:linux-nfs-owner-fy+rA21nqHI@public.gmane.org= rnel.org] On Behalf Of Timo Aaltonen Sent: Wednesday, May 05, 2010 6:16 PM To: Kevin Coffman Cc: J. Bruce Fields; linux-nfs@vger.kernel.org Subject: Re: [PATCH] Check for AD style machine principal name On Wed, 5 May 2010, Kevin Coffman wrote: > On Wed, May 5, 2010 at 12:53 PM, Timo Aaltonen <timo.aaltonen@aalto.f= i> wrote: >> >> CC:ing the "new" list. >> >> On Tue, 27 Apr 2010, J. Bruce Fields wrote: >> >>> On Mon, Apr 26, 2010 at 05:00:59PM +0300, timo.aaltonen-uOixanVlb7U@public.gmane.org wr= ote: >>>> >>>> @@ -737,6 +737,26 @@ gssd_search_krb5_keytab(krb5_context context, >>>> krb5_keytab kt, >>>> =A0} >>>> >>>> =A0/* >>>> + * Convert the hostname to machine principal name as created >>>> + * by MS Active Directory. >>>> +*/ >>>> + >>>> +static char * >>>> +hostname_to_adprinc(char *name) >>>> +{ >>>> + =A0 =A0 =A0 int i =3D 0; >>>> + =A0 =A0 =A0 char *buf =3D strdup(name); >>> >>> We should handle the possible NULL return. >> >> How about not calling hostname_to_adprinc from find_keytab_entry unl= ess >> myhostname is !=3D NULL? > > I think he meant that strdup() may fail, and return NULL. That has t= o > be handled here, and a possible resulting NULL return from > host_to_adprinc() should also be handled below. Ahh.. ok so how about this: static char * hostname_to_adprinc(char *name) { int i =3D 0; int len =3D strlen(name); char *buf; if ((buf =3D malloc(len+1))) { > Doesn't this need to be "+2" since you are adding a '$' and a '\0' to= the end of the string, neither of which is included in the strlen() ca= lculation? strcpy(buf, name); while(i < len) { buf[i] =3D toupper(buf[i]); i++; } > Instead of copying name to buf and then converting the characters in = buf to uppercase, why not copy and convert in one loop? buf[i++] =3D '$'; buf[i] =3D 0; return buf; } return NULL; } and then adding 'if (adprinc !=3D NULL) { ... }' to the while loop in=20 find_keytab_entry. That passes my tests. >>>> @@ -812,6 +836,35 @@ find_keytab_entry(krb5_context context, krb5_= keytab >>>> kt, const char *hostname, >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break; >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (strcmp(realm, default_realm) =3D= =3D 0) >>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0tried_default =3D 1= ; >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* First try the Active Directory st= yle machine principal >>>> */ >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 code =3D krb5_build_principal_ext(co= ntext, &princ, >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 strlen(realm), >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 realm, >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 strlen(adprinc), >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 adprinc, >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 NULL); >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (code) { >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 k5err =3D gssd_k5_er= r_msg(context, code); >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 printerr(1, "%s whil= e building principal for " >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"= '%s@%s'\n", k5err, >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0a= dprinc, realm); >>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 continue; >>> >>> Is there an infinite loop here? >> >> Not to my knowledge, it's basically the same chunk of code copied fr= om a few >> lines below, modified to suit the adprinc. > > The chunk you copied was within a for() loop. You are calling > continue within the while(1) loop. Ok, so 'continue' was extra, whoops.. --=20 Timo Aaltonen Systems Specialist IT Services, Aalto University -- 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] 7+ messages in thread
[parent not found: <BF3BB6D12298F54B89C8DCC1E4073D80A24712-1Zg0zMUlrbd9m/dOYFj4Yjjd7nCn89gW@public.gmane.org>]
* RE: [PATCH] Check for AD style machine principal name [not found] ` <BF3BB6D12298F54B89C8DCC1E4073D80A24712-1Zg0zMUlrbd9m/dOYFj4Yjjd7nCn89gW@public.gmane.org> @ 2010-05-05 22:33 ` Timo Aaltonen [not found] ` <alpine.DEB.2.00.1005060131080.3282-9o+TK994UVukEuhtYYxXtw@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Timo Aaltonen @ 2010-05-05 22:33 UTC (permalink / raw) To: Staubach_Peter; +Cc: timo.aaltonen-uOixanVlb7U, kwc, bfields, linux-nfs On Thu, 6 May 2010, Staubach_Peter@emc.com wrote: > -----Original Message----- > From: linux-nfs-owner@vger.kernel.org [mailto:linux-nfs-owner-fy+rA21nqHI@public.gmane.org= rnel.org] On Behalf Of Timo Aaltonen > Sent: Wednesday, May 05, 2010 6:16 PM > To: Kevin Coffman > Cc: J. Bruce Fields; linux-nfs@vger.kernel.org > Subject: Re: [PATCH] Check for AD style machine principal name > > On Wed, 5 May 2010, Kevin Coffman wrote: > >> On Wed, May 5, 2010 at 12:53 PM, Timo Aaltonen <timo.aaltonen@aalto.= fi> wrote: >>> >>> CC:ing the "new" list. >>> >>> On Tue, 27 Apr 2010, J. Bruce Fields wrote: >>> >>>> On Mon, Apr 26, 2010 at 05:00:59PM +0300, timo.aaltonen-uOixanVlb7U@public.gmane.org w= rote: >>>>> >>>>> @@ -737,6 +737,26 @@ gssd_search_krb5_keytab(krb5_context context= , >>>>> krb5_keytab kt, >>>>> =A0} >>>>> >>>>> =A0/* >>>>> + * Convert the hostname to machine principal name as created >>>>> + * by MS Active Directory. >>>>> +*/ >>>>> + >>>>> +static char * >>>>> +hostname_to_adprinc(char *name) >>>>> +{ >>>>> + =A0 =A0 =A0 int i =3D 0; >>>>> + =A0 =A0 =A0 char *buf =3D strdup(name); >>>> >>>> We should handle the possible NULL return. >>> >>> How about not calling hostname_to_adprinc from find_keytab_entry un= less >>> myhostname is !=3D NULL? >> >> I think he meant that strdup() may fail, and return NULL. That has = to >> be handled here, and a possible resulting NULL return from >> host_to_adprinc() should also be handled below. > > Ahh.. ok so how about this: > > static char * > hostname_to_adprinc(char *name) > { > int i =3D 0; > int len =3D strlen(name); > char *buf; > if ((buf =3D malloc(len+1))) { > >> Doesn't this need to be "+2" since you are adding a '$' and a '\0' t= o the end of the string, neither of which is included in the strlen() c= alculation? Yep, you're right. Shouldn't adding to the buffer fail then, when the=20 malloc leaves the string short? > strcpy(buf, name); > while(i < len) { > buf[i] =3D toupper(buf[i]); > i++; > } > >> Instead of copying name to buf and then converting the characters in= buf to uppercase, why not copy and convert in one loop? This buf[i] =3D toupper(name[i]); works fine, thanks :) --=20 Timo Aaltonen Systems Specialist IT Services, Aalto University ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <alpine.DEB.2.00.1005060131080.3282-9o+TK994UVukEuhtYYxXtw@public.gmane.org>]
* RE: [PATCH] Check for AD style machine principal name [not found] ` <alpine.DEB.2.00.1005060131080.3282-9o+TK994UVukEuhtYYxXtw@public.gmane.org> @ 2010-05-05 22:43 ` Staubach_Peter 0 siblings, 0 replies; 7+ messages in thread From: Staubach_Peter @ 2010-05-05 22:43 UTC (permalink / raw) To: timo.aaltonen-uOixanVlb7U; +Cc: kwc, bfields, linux-nfs -----Original Message----- =46rom: Timo Aaltonen [mailto:timo.aaltonen-uOixanVlb7U@public.gmane.org]=20 Sent: Wednesday, May 05, 2010 6:34 PM To: Staubach, Peter Cc: timo.aaltonen-uOixanVlb7U@public.gmane.org; kwc@citi.umich.edu; bfields@fieldses.org; l= inux-nfs@vger.kernel.org Subject: RE: [PATCH] Check for AD style machine principal name On Thu, 6 May 2010, Staubach_Peter@emc.com wrote: > -----Original Message----- > From: linux-nfs-owner@vger.kernel.org [mailto:linux-nfs-owner-fy+rA21nqHI@public.gmane.org= rnel.org] On Behalf Of Timo Aaltonen > Sent: Wednesday, May 05, 2010 6:16 PM > To: Kevin Coffman > Cc: J. Bruce Fields; linux-nfs@vger.kernel.org > Subject: Re: [PATCH] Check for AD style machine principal name > > On Wed, 5 May 2010, Kevin Coffman wrote: > >> On Wed, May 5, 2010 at 12:53 PM, Timo Aaltonen <timo.aaltonen@aalto.= fi> wrote: >>> >>> CC:ing the "new" list. >>> >>> On Tue, 27 Apr 2010, J. Bruce Fields wrote: >>> >>>> On Mon, Apr 26, 2010 at 05:00:59PM +0300, timo.aaltonen-uOixanVlb7U@public.gmane.org w= rote: >>>>> >>>>> @@ -737,6 +737,26 @@ gssd_search_krb5_keytab(krb5_context context= , >>>>> krb5_keytab kt, >>>>> =A0} >>>>> >>>>> =A0/* >>>>> + * Convert the hostname to machine principal name as created >>>>> + * by MS Active Directory. >>>>> +*/ >>>>> + >>>>> +static char * >>>>> +hostname_to_adprinc(char *name) >>>>> +{ >>>>> + =A0 =A0 =A0 int i =3D 0; >>>>> + =A0 =A0 =A0 char *buf =3D strdup(name); >>>> >>>> We should handle the possible NULL return. >>> >>> How about not calling hostname_to_adprinc from find_keytab_entry un= less >>> myhostname is !=3D NULL? >> >> I think he meant that strdup() may fail, and return NULL. That has = to >> be handled here, and a possible resulting NULL return from >> host_to_adprinc() should also be handled below. > > Ahh.. ok so how about this: > > static char * > hostname_to_adprinc(char *name) > { > int i =3D 0; > int len =3D strlen(name); > char *buf; > if ((buf =3D malloc(len+1))) { > >> Doesn't this need to be "+2" since you are adding a '$' and a '\0' t= o the end of the string, neither of which is included in the strlen() c= alculation? Yep, you're right. Shouldn't adding to the buffer fail then, when the=20 malloc leaves the string short? >> Nope, you just step on memory that potentially belongs to something = else, causing corruption. ps > strcpy(buf, name); > while(i < len) { > buf[i] =3D toupper(buf[i]); > i++; > } > >> Instead of copying name to buf and then converting the characters in= buf to uppercase, why not copy and convert in one loop? This buf[i] =3D toupper(name[i]); works fine, thanks :) --=20 Timo Aaltonen Systems Specialist IT Services, Aalto University ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-05-05 22:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1272290459-11335-1-git-send-email-timo.aaltonen@aalto.fi>
[not found] ` <560_1272392934_4BD72CE6_560_482_1_20100427182806.GP30729@fieldses.org>
2010-05-05 16:53 ` [PATCH] Check for AD style machine principal name Timo Aaltonen
[not found] ` <alpine.DEB.2.00.1005051934040.3282-9o+TK994UVukEuhtYYxXtw@public.gmane.org>
2010-05-05 17:36 ` J. Bruce Fields
2010-05-05 17:46 ` Kevin Coffman
[not found] ` <n2s4d569c331005051046t28cd5dch1e5dfe4a73b987bf-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-05-05 22:15 ` Timo Aaltonen
[not found] ` <alpine.DEB.2.00.1005052347130.3282-9o+TK994UVukEuhtYYxXtw@public.gmane.org>
2010-05-05 22:25 ` Staubach_Peter
[not found] ` <BF3BB6D12298F54B89C8DCC1E4073D80A24712-1Zg0zMUlrbd9m/dOYFj4Yjjd7nCn89gW@public.gmane.org>
2010-05-05 22:33 ` Timo Aaltonen
[not found] ` <alpine.DEB.2.00.1005060131080.3282-9o+TK994UVukEuhtYYxXtw@public.gmane.org>
2010-05-05 22:43 ` Staubach_Peter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.