[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: (ITS#6092) correct string problem in guess_service_principal()



Hallvard B Furuseth wrote:
> mikbec@web.de writes:
>> When GSSAPI option GSSAPI_ALLOW_REMOTE_PRINCIPAL is switched on then
>> string provided by "givenstr" will be used as principal name. But
>> length of string counted in "svc_principal_size" is one letter to
>> less.
> 
> svc_principal_size looks correct in that case, but the snprintf is
> wrong.  I can't test, but it should work to pass svc_principal_size
> instead of svc_principal_size-1.  Except snprintf seems pointless here,
> since we already make sure to allocate enough memory to avoid overflow.

Your code looks OK to me ... so there is no need for any code that 
relates to "svc_principal_size" any more.

> 
> I'll take the opportunity to get rid of a gcc -Wformat warning - it
> can't verify the format argument since that is not a string literal.
> 
> Like this.  What's a good name for my new "prefix" variable below?  I
> don't know Kerberos/GSSAPI terminology.

Hmmm ... its a good question. The problem is here the hostname is the 
instance and "ldap/" is the service we are looking for on that host. So 
"prefix" is really the "service" and "dnsHostName" or "host" are the 
service instances .... I think if we call it "spn_instance_prefix" or 
simply "svc_prefix" then this will be a good choice.

Please have a look at:
http://www.gnu.org/software/shishi/manual/html_node/Realm-and-Principal-Naming.html
section 4.3.2 Principal Names. This is a summary of RFC 1510 chapter 7.2 
"Principal names".

The princial name type we are using here is mostly NT-SRV-HST.

> 
> 	const char *prefix;
> 	...
> 	if (allow_remote && givenstr) {
> 		prefix = "";
> 		str = givenstr;
> 	} else {
> 		prefix = "ldap/";
> 		str = (allow_remote && dnsHostName) ? dnsHostName : host;
> 	}
> 
> 	svc_principal = (char*) ldap_memalloc(strlen(prefix) + strlen(str) + 1);
> 	if ( svc_principal == NULL ) {
> 		ld->ld_errno = LDAP_NO_MEMORY;
> 		return ld->ld_errno;
> 	}
> 	sprintf( svc_principal, "%s%s", prefix, str);
>