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

Pavel Březina pbrezina at redhat.com
Fri Jan 11 10:43:12 UTC 2013


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.




More information about the sssd-devel mailing list