[SSSD] [PATCH] #570-fall back to defaults from krb5.conf if the realm is not specified explicitly in sssd.conf

Ariel Barria olivares73 at hotmail.com
Mon May 13 23:03:29 UTC 2013


thank you very much, your comments were very helpful.

I send patch for review.

> Date: Fri, 11 Jan 2013 11:43:12 +0100
> From: pbrezina at redhat.com
> To: sssd-devel at lists.fedorahosted.org
> Subject: Re: [SSSD] #570-fall back to defaults from krb5.conf if the realm is not specified explicitly in sssd.conf
> 
> On 11/26/2012 06:30 PM, Ariel Barria wrote:
> > Hi.
> > Apologies, I have been working in ticket #570 and I send for your
> > comments before the send a patch final.
> > I was reviewing where else needs to change and I find this places
> > 1 - krb5_auth.c ---> tevent_req *krb5_auth_send(
> > 2 - krb5_utils.c ---> char *expand_ccname_template(
> > 3 - ipa_dyndns.c ---> int create_nsupdate_message(
> > This correct?
> >
> > https://fedorahosted.org/sssd/ticket/570
> >
> > Thanks
> 
> Hi, thank you for contribution. I'm sorry this took so long.
> 
> This is a good start, but I have few comments.
> 
> 1. Can we put this code into a common function instead of copy and
> paste? Say:
> 
> errno_t krb5_get_realm(TALLOC_CTX *mem_ctx, struct dp_option *opts,
>                         char **_realm);
> 
> It will store chosen krb5 realm in *_realm attached to talloc context
> mem_ctx.
> 
> 2. When creating tmp_ctx, don't forget to free it before returning from
> 
> function. When using done/fail label, it should say:
> 
> done:
>      ...
>      talloc_free(tmp_ctx);
>      return ret;
> 
> 3. Initialize krb5_ctx only when you know you're gonna need it. I.e.
> after if (krb5_realm == NULL)
> 
> 4. Additional code that needs to be altered:
> ipa_common.c ipa_get_auth_options()
> krb5_common.c krb5_install_sigterm_handler()
> krb5_common.c krb5_get_simple_upn()
> krb5_common.c expand_ccname_template()
> 
> I went through references to KRB5_REALM option. I didn't include all
> references, some of them seemed that it should not be changed. Can
> somebody check me please?
> 
> 5. But in the end, we don't have to change those functions at all. We
> can chose the realm during provider initialization and use
> dp_opt_set_option() to set the option to chosen value.
> 
> Cheers,
> Pavel.
> 
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
 		 	   		  
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130513/409dafbe/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-fall-back-to-defaults-from-krb5.conf-if-the-realm-is.patch
Type: application/octet-stream
Size: 7148 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130513/409dafbe/attachment.obj>


More information about the sssd-devel mailing list