[SSSD] [PATCHES] KDC proxy related fixes

Pavel Březina pbrezina at redhat.com
Thu Aug 6 10:23:47 UTC 2015


On 08/04/2015 06:18 PM, Jakub Hrozek wrote:
> On Tue, Aug 04, 2015 at 10:17:58AM +0300, Alexander Bokovoy wrote:
>> On Mon, 03 Aug 2015, Christian Heimes wrote:
>>> On 2015-07-31 18:19, Sumit Bose wrote:
>>>> On Fri, Jul 31, 2015 at 11:34:23AM +0200, Sumit Bose wrote:
>>>>> Hi,
>>>>>
>>>>> it turned out that some of the current SSSD behaviour does not fit well
>>>>> if a KDC proxy is configured, see
>>>>> https://fedorahosted.org/sssd/ticket/2652 and
>>>>> https://fedorahosted.org/sssd/ticket/2700 for details.
>>>>>
>>>>> The first patch in this series introduces a new call which checks if a
>>>>> KDC proxy is configured as suggested in the tickets. The other two
>>>>> patches aim to fix the respective ticket.
>>>>>
>>>>> bye,
>>>>> Sumit
>>>>
>>>> Please find attached a new version of the patches. They fix a memory
>>>> leak found by Christian in the first patch and contain a different
>>>> version of the third patch because the original version didn't fix the
>>>> issue Alexander was seeing. There is only a minor change compared to the
>>>> version Alexander tested, krb5.conf is not checked unconditionally but
>>>> only if the state is offline.
>>>>
>>>> There was another comment by Christian on irc. Currently the patches
>>>> only check the kdc config entry. In theory if would be possible that for
>>>> the kdc a direct connection is used while the admin_server is configured
>>>> via a proxy. Since this is expected to be an un-common configuration I
>>>> hope it can be added later. To solve this I think
>>>> sss_krb5_realm_has_proxy() should get a second option indication if kdc
>>>> or admin_server should be checked. Depending on the type of request,
>>>> (pre-)auth or change password, or info file, kdcinfo or kpasswdinfo,
>>>> sss_krb5_realm_has_proxy() should be called with the matching option.
>>>
>>> sss_krb5_realm_has_proxy() looks good to me. IMHO it's fine to just
>>> check kdc for https for now.
>> I agree.
>>
>> I would start with this patchset and then improve on it sequentially.
>
> I would like to squash these changes to the first patch since Sumit is

Go ahead. But it looks like there is some indentation error?

> on vacation:
> diff --git a/Makefile.am b/Makefile.am
> index 5345d90..8b64317 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -366,6 +366,7 @@ dist_noinst_SCRIPTS = \
>       src/tests/pysss_murmur-test.py2.sh \
>       src/tests/pysss_murmur-test.py3.sh \
>       src/tests/python-test.py \
> +    src/tests/krb5_proxy_check_test_data.conf \
>       $(NULL)
>
>   dist_noinst_DATA = \
> diff --git a/src/util/sss_krb5.c b/src/util/sss_krb5.c
> index 38e379c..2e128db 100644
> --- a/src/util/sss_krb5.c
> +++ b/src/util/sss_krb5.c
> @@ -1076,7 +1076,7 @@ krb5_error_code sss_krb5_kt_have_content(krb5_context context,
>
>   bool sss_krb5_realm_has_proxy(const char *realm)
>   {
> -    krb5_context context;
> +    krb5_context context = NULL;
>       krb5_error_code kerr;
>       struct _profile_t *profile = NULL;
>       const char  *profile_path[4] = {"realms", NULL, "kdc", NULL};
>
> ACK from me code-wise on the other two patches. I know they work because
> I saw Alexander test them :-)




More information about the sssd-devel mailing list