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 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 :-) _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel