[SSSD] [PATCH] SPEC: Workaround for build with rpm 4.13
ehlo,
patch is attached.
LS
From 4599b88989ee56343c25554b3198689279d2be0a Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik lsleb...@redhat.com
Date: Fri, 31 Jul 2015 14:09:25 +0200
Subject: [PATCH] SPEC: Workaround for build with rpm 4.13
If the tarball is generated with minimal dependencies extracted from spec file
then translated manual pages are not generated due to missing script po4a.
This step is not necessary for regular nightly/developer builds.
The tarball is created faster without such step. However rpm = 4.13
will fail due to empty manifest file.
Resolves:
https://fedorahosted.org/sssd/ticket/2738
---
contrib/sssd.spec.in | 3 +++
1 file changed, 3 insertions(+)
diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
index
0606e5e4f6a2f5ff7677b1e142816e459bb5805a..5ca26495bf5cd21da0f652d8f5ff78aa3b0c9067
100644
--- a/contrib/sssd.spec.in
+++ b/contrib/sssd.spec.in
@@ -12,6 +12,9 @@
%define __provides_exclude_from %{python2_sitearch}/.*\.so$
%define __provides_exclude_from %{python3_sitearch}/.*\.so$
+# workaround for rpm 4.13
+%define _empty_manifest_terminate_build 0
+
%if (0%{?fedora} || 0%{?rhel} = 7)
%global use_systemd 1
%endif
--
2.4.6
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] mmap_cache: Override functions for initgr mmap cache
On 07/30/2015 05:23 PM, Michal Židek wrote: On 07/30/2015 05:06 PM, Michal Židek wrote: On 07/30/2015 04:15 PM, Lukas Slebodnik wrote: On (29/07/15 17:05), Lukas Slebodnik wrote: Attached is a new patch set. I also contains fix for #2712 and not just for #2716. Otherwise there would be a crash. So it would be better push them together. Integration test were failing sporadically due to bug in clien code. 3rd patch fix this problem. Other patches are the same. LS Integration tests still do not work for me even with this version. $ contrib/ci/run -n -r autoreconf: success 00:00:14 ci-autoreconf.log DEBUG BUILD: ci-build-debug configure: success 00:00:10 ci-build-debug/ci-configure.log make-tests: success 00:02:18 ci-build-debug/ci-make-tests.log make-check-valgrind:success 00:01:41 ci-build-debug/ci-make-check-valgrind.log make-intgcheck: failure 00:01:55 ci-build-debug/ci-make-intgcheck.log FAILURE See sanitized log. These are the newly added _with_mc tests. Michal Ah... nevermind, I had exported SSS_NSS_USE_MEMCACHE=no in the terminal where I was running the tests. On the other hand this proves that your tests really catch situation when memcache does not work :) Thank you for expanding the tests coverage. Tentative ACK to the patches (will fully ack when CI results finish). I filled a ticket to track one issue, but it does not seem to be related to the memcache. https://fedorahosted.org/sssd/ticket/2741 Michal -- Senior Principal Intern ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] mmap_cache: Override functions for initgr mmap cache
On 07/31/2015 04:54 PM, Michal Židek wrote: On 07/30/2015 05:23 PM, Michal Židek wrote: On 07/30/2015 05:06 PM, Michal Židek wrote: On 07/30/2015 04:15 PM, Lukas Slebodnik wrote: On (29/07/15 17:05), Lukas Slebodnik wrote: Attached is a new patch set. I also contains fix for #2712 and not just for #2716. Otherwise there would be a crash. So it would be better push them together. Integration test were failing sporadically due to bug in clien code. 3rd patch fix this problem. Other patches are the same. LS Integration tests still do not work for me even with this version. $ contrib/ci/run -n -r autoreconf: success 00:00:14 ci-autoreconf.log DEBUG BUILD: ci-build-debug configure: success 00:00:10 ci-build-debug/ci-configure.log make-tests: success 00:02:18 ci-build-debug/ci-make-tests.log make-check-valgrind:success 00:01:41 ci-build-debug/ci-make-check-valgrind.log make-intgcheck: failure 00:01:55 ci-build-debug/ci-make-intgcheck.log FAILURE See sanitized log. These are the newly added _with_mc tests. Michal Ah... nevermind, I had exported SSS_NSS_USE_MEMCACHE=no in the terminal where I was running the tests. On the other hand this proves that your tests really catch situation when memcache does not work :) Thank you for expanding the tests coverage. Tentative ACK to the patches (will fully ack when CI results finish). I filled a ticket to track one issue, but it does not seem to be related to the memcache. https://fedorahosted.org/sssd/ticket/2741 Well, it is related to the memcache. But not that memcache itself would not work. We somewhere miss call to clear the memcache in the code branch that 'su' triggers. Michal -- Senior Principal Intern ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCHES] KDC proxy related fixes
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.
bye,
Sumit
From 907bf732bc42c8a9ad558a46415eb73d31368196 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Thu, 30 Jul 2015 16:52:42 +0200
Subject: [PATCH 1/3] krb5 utils: add sss_krb5_realm_has_proxy()
---
src/tests/krb5_proxy_check_test_data.conf | 8 +
src/tests/krb5_utils-tests.c | 17 +
src/util/sss_krb5.c | 57 +++
src/util/sss_krb5.h | 2 ++
4 files changed, 84 insertions(+)
create mode 100644 src/tests/krb5_proxy_check_test_data.conf
diff --git a/src/tests/krb5_proxy_check_test_data.conf
b/src/tests/krb5_proxy_check_test_data.conf
new file mode 100644
index
..eb74dbfa47d643668688d5c789b5962698c3d17c
--- /dev/null
+++ b/src/tests/krb5_proxy_check_test_data.conf
@@ -0,0 +1,8 @@
+[realms]
+ REALM = {
+kdc = hello
+ }
+
+ REALM_PROXY = {
+kdc = https://hello
+ }
diff --git a/src/tests/krb5_utils-tests.c b/src/tests/krb5_utils-tests.c
index
650ed48592768c214156d5274e654a447be98e36..9a25b09cdc136651a7117327036dd51b8ff23606
100644
--- a/src/tests/krb5_utils-tests.c
+++ b/src/tests/krb5_utils-tests.c
@@ -684,6 +684,22 @@ START_TEST(test_parse_krb5_map_user)
}
END_TEST
+START_TEST(test_sss_krb5_realm_has_proxy)
+{
+krb5_error_code kerr;
+long perr;
+
+fail_unless(sss_krb5_realm_has_proxy(NULL) == false);
+
+setenv(KRB5_CONFIG, /dev/null, 1);
+fail_unless(sss_krb5_realm_has_proxy(REALM) == false);
+
+setenv(KRB5_CONFIG,
ABS_SRC_DIR/src/tests/krb5_proxy_check_test_data.conf, 1);
+fail_unless(sss_krb5_realm_has_proxy(REALM) == false);
+fail_unless(sss_krb5_realm_has_proxy(REALM_PROXY) == true);
+}
+END_TEST
+
Suite *krb5_utils_suite (void)
{
Suite *s = suite_create (krb5_utils);
@@ -723,6 +739,7 @@ Suite *krb5_utils_suite (void)
TCase *tc_krb5_helpers = tcase_create(Helper functions);
tcase_add_test(tc_krb5_helpers, test_compare_principal_realm);
tcase_add_test(tc_krb5_helpers, test_parse_krb5_map_user);
+tcase_add_test(tc_krb5_helpers, test_sss_krb5_realm_has_proxy);
suite_add_tcase(s, tc_krb5_helpers);
return s;
diff --git a/src/util/sss_krb5.c b/src/util/sss_krb5.c
index
e5c2121da575b174c8b6a9a90835f2c97f807f37..38e379cf95bb4772ca11daa513a80b2e480c88c4
100644
--- a/src/util/sss_krb5.c
+++ b/src/util/sss_krb5.c
@@ -20,6 +20,7 @@
#include stdio.h
#include errno.h
#include talloc.h
+#include profile.h
#include config.h
@@ -1069,3 +1070,59 @@ krb5_error_code sss_krb5_kt_have_content(krb5_context
context,
return 0;
#endif
}
+
+#define KDC_PROXY_INDICATOR https://;
+#define KDC_PROXY_INDICATOR_LEN (sizeof(KDC_PROXY_INDICATOR) - 1)
+
+bool sss_krb5_realm_has_proxy(const char *realm)
+{
+krb5_context context;
+krb5_error_code kerr;
+struct _profile_t *profile = NULL;
+const char *profile_path[4] = {realms, NULL, kdc, NULL};
+char **list = NULL;
+bool res = false;
+size_t c;
+
+if (realm == NULL) {
+return false;
+}
+
+kerr = krb5_init_context(context);
+if (kerr != 0) {
+DEBUG(SSSDBG_OP_FAILURE, krb5_init_context failed.\n);
+return false;
+}
+
+kerr = krb5_get_profile(context, profile);
+if (kerr != 0) {
+DEBUG(SSSDBG_OP_FAILURE, krb5_get_profile failed.\n);
+goto done;
+}
+
+profile_path[1] = realm;
+
+kerr = profile_get_values(profile, profile_path, list);
+if (kerr != 0) {
+
Re: [SSSD] [PATCH] mmap_cache: Override functions for initgr mmap cache
On (31/07/15 16:54), Michal Židek wrote: On 07/30/2015 05:23 PM, Michal Židek wrote: On 07/30/2015 05:06 PM, Michal Židek wrote: On 07/30/2015 04:15 PM, Lukas Slebodnik wrote: On (29/07/15 17:05), Lukas Slebodnik wrote: Attached is a new patch set. I also contains fix for #2712 and not just for #2716. Otherwise there would be a crash. So it would be better push them together. Integration test were failing sporadically due to bug in clien code. 3rd patch fix this problem. Other patches are the same. LS Integration tests still do not work for me even with this version. $ contrib/ci/run -n -r autoreconf: success 00:00:14 ci-autoreconf.log DEBUG BUILD: ci-build-debug configure: success 00:00:10 ci-build-debug/ci-configure.log make-tests: success 00:02:18 ci-build-debug/ci-make-tests.log make-check-valgrind:success 00:01:41 ci-build-debug/ci-make-check-valgrind.log make-intgcheck: failure 00:01:55 ci-build-debug/ci-make-intgcheck.log FAILURE See sanitized log. These are the newly added _with_mc tests. Michal Ah... nevermind, I had exported SSS_NSS_USE_MEMCACHE=no in the terminal where I was running the tests. On the other hand this proves that your tests really catch situation when memcache does not work :) Thank you for expanding the tests coverage. Tentative ACK to the patches (will fully ack when CI results finish). I filled a ticket to track one issue, but it does not seem to be related to the memcache. https://fedorahosted.org/sssd/ticket/2741 Ticket #2741 seems to be duplicate of #2716 It means that regression is not fixed an patches canot be pushed LS ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCHES] Add support for Smartcard authentication
On Thu, Jul 30, 2015 at 09:27:31PM +0200, Jakub Hrozek wrote: On Thu, Jul 30, 2015 at 05:24:27PM +0200, Sumit Bose wrote: On Thu, Jul 30, 2015 at 11:10:33AM +0200, Jakub Hrozek wrote: On Wed, Jul 29, 2015 at 03:34:32PM +0200, Jakub Hrozek wrote: On Tue, Jul 28, 2015 at 10:43:13PM +0200, Sumit Bose wrote: Hi, I started the review, but because the patches are quite big, I will send my comments in batches. I hope that's fine. Thank you for the review. CI failed in distcheck. The failure was due to distcheck's tendency to make the complete source tree read-only and NSS's tendency open all databases in read-write more. I added the needed read-onl options to the NSS code. Coverity found some issues that I forwarded to Sumit. Thank you, I fixed them and hopefully did not create new ones. Coverity is clean this time, but CI did not pass on Debian due to build failure of a test: http://sssd-ci.duckdns.org/logs/job/19/78/debian_testing/ci-build-debug/ci-make-tests.log Normally this is caused by a missing dependency, which the Fedora/RHEL linker can satisfy. Please find attached a new set which should fix the debian build (only the last patch changed). As you suspected it was a missing dependency in Makefile.am. But this make me realize that I didn't implement a OpenSSL/libcrypto version of cert_to_ssh_key() which would break the OpenSSL build. I added an initial version without certificate validation. If you don't like it it can simply replaced by 'return ENOSUPP'. I'm fine with the code as long as you keep the TODOs at least in some todo list of yours. ACK code wise, also my regression tests went fine. I'm only waiting for the automated tests to finish. CI passed, the rawhide failure is unrelated: http://sssd-ci.idm.lab.eng.brq.redhat.com:8080/job/ci/2015/ ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] IPA: Remove MPG groups if getgrgid was called before getpw()
On Thu, Jul 30, 2015 at 06:23:03PM +0200, Sumit Bose wrote: On Thu, Jul 30, 2015 at 06:20:31PM +0200, Sumit Bose wrote: On Wed, Jul 22, 2015 at 12:18:07PM +0200, Jakub Hrozek wrote: On Wed, Jul 22, 2015 at 10:01:31AM +0200, Sumit Bose wrote: On Tue, Jul 21, 2015 at 09:41:46PM +0200, Jakub Hrozek wrote: Hi, the attached patch fixes https://fedorahosted.org/sssd/ticket/2724 for me. I haven't yet heard back from the affected user, but they do have test packages available. The patch is working as expected and fixes the issue for me as well. Although it might not look as the most elegant way I think the approach is ok because otherwise we would be to check for every group by GID request if the GID might belong to a MPG which would require a user lookup. This would slow down the requests where the GID belong to a normal (non-MPG) group. Nevertheless I wonder if we should check if the group name matches the user name before deleting the group by GID to be on the safe side? OK, done. Thank you. The patch looks good and is working as advertised, ACK. sorry, forgot to say that CI is still running, I'll send the result when it is done. CI basically passed http://sssd-ci.duckdns.org/logs/job/20/16/summary.html There is an unrealted issue in rawhide: RPM build errors: error: Empty %files file /builddir/build/BUILD/sssd-1.13.1/sssd_client.lang For details see http://sssd-ci.duckdns.org/logs/job/20/16/fedora_rawhide/ci-build-debug/ci-mock-result/build.log so once again, ACK. bye, Sumit btw the function is really long. Would you oppose to splitting it into smaller functions that save user/group/... ? (We need tests for that, though..) no, feel free to open tickets to track this. bye, Sumit ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
[SSSD] [PATCHES] KDC proxy related fixes
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
From 19a0e94b702c9813439f12c7017fb6b6e22000cf Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Thu, 30 Jul 2015 16:52:42 +0200
Subject: [PATCH 1/3] krb5 utils: add sss_krb5_realm_has_proxy()
---
src/tests/krb5_proxy_check_test_data.conf | 8 +
src/tests/krb5_utils-tests.c | 17 +
src/util/sss_krb5.c | 57 +++
src/util/sss_krb5.h | 2 ++
4 files changed, 84 insertions(+)
create mode 100644 src/tests/krb5_proxy_check_test_data.conf
diff --git a/src/tests/krb5_proxy_check_test_data.conf
b/src/tests/krb5_proxy_check_test_data.conf
new file mode 100644
index
..eb74dbfa47d643668688d5c789b5962698c3d17c
--- /dev/null
+++ b/src/tests/krb5_proxy_check_test_data.conf
@@ -0,0 +1,8 @@
+[realms]
+ REALM = {
+kdc = hello
+ }
+
+ REALM_PROXY = {
+kdc = https://hello
+ }
diff --git a/src/tests/krb5_utils-tests.c b/src/tests/krb5_utils-tests.c
index
650ed48592768c214156d5274e654a447be98e36..9a25b09cdc136651a7117327036dd51b8ff23606
100644
--- a/src/tests/krb5_utils-tests.c
+++ b/src/tests/krb5_utils-tests.c
@@ -684,6 +684,22 @@ START_TEST(test_parse_krb5_map_user)
}
END_TEST
+START_TEST(test_sss_krb5_realm_has_proxy)
+{
+krb5_error_code kerr;
+long perr;
+
+fail_unless(sss_krb5_realm_has_proxy(NULL) == false);
+
+setenv(KRB5_CONFIG, /dev/null, 1);
+fail_unless(sss_krb5_realm_has_proxy(REALM) == false);
+
+setenv(KRB5_CONFIG,
ABS_SRC_DIR/src/tests/krb5_proxy_check_test_data.conf, 1);
+fail_unless(sss_krb5_realm_has_proxy(REALM) == false);
+fail_unless(sss_krb5_realm_has_proxy(REALM_PROXY) == true);
+}
+END_TEST
+
Suite *krb5_utils_suite (void)
{
Suite *s = suite_create (krb5_utils);
@@ -723,6 +739,7 @@ Suite *krb5_utils_suite (void)
TCase *tc_krb5_helpers = tcase_create(Helper functions);
tcase_add_test(tc_krb5_helpers, test_compare_principal_realm);
tcase_add_test(tc_krb5_helpers, test_parse_krb5_map_user);
+tcase_add_test(tc_krb5_helpers, test_sss_krb5_realm_has_proxy);
suite_add_tcase(s, tc_krb5_helpers);
return s;
diff --git a/src/util/sss_krb5.c b/src/util/sss_krb5.c
index
e5c2121da575b174c8b6a9a90835f2c97f807f37..cc11b961ca198fb1840d678cb0cd585b4e9e17fb
100644
--- a/src/util/sss_krb5.c
+++ b/src/util/sss_krb5.c
@@ -20,6 +20,7 @@
#include stdio.h
#include errno.h
#include talloc.h
+#include profile.h
#include config.h
@@ -1069,3 +1070,59 @@ krb5_error_code sss_krb5_kt_have_content(krb5_context
context,
return 0;
#endif
}
+
+#define KDC_PROXY_INDICATOR https://;
+#define KDC_PROXY_INDICATOR_LEN (sizeof(KDC_PROXY_INDICATOR) - 1)
+
+bool sss_krb5_realm_has_proxy(const char *realm)
+{
+krb5_context context;
+krb5_error_code kerr;
+struct _profile_t *profile = NULL;
+const char *profile_path[4] = {realms, NULL, kdc, NULL};
+char **list = NULL;
+bool res = false;
+size_t c;
+
+if (realm == NULL) {
+return false;
+}
+
+kerr = krb5_init_context(context);
+if (kerr != 0) {
+DEBUG(SSSDBG_OP_FAILURE, krb5_init_context failed.\n);
+return false;
+}
+
+kerr = krb5_get_profile(context, profile);
+if (kerr != 0) {
+DEBUG(SSSDBG_OP_FAILURE, krb5_get_profile failed.\n);
+return false;
+}
+
+profile_path[1] = realm;
+
+kerr = profile_get_values(profile, profile_path, list);
+if (kerr != 0) {
+DEBUG(SSSDBG_OP_FAILURE, profile_get_values failed.\n);
+goto done;
+}
+
+for (c = 0; list[c] != NULL; c++) {
+if (strncasecmp(KDC_PROXY_INDICATOR, list[c],
+KDC_PROXY_INDICATOR_LEN) == 0) {
+DEBUG(SSSDBG_TRACE_ALL,
+ Found KDC Proxy indicator [%s] in [%s].\n,
+ KDC_PROXY_INDICATOR, list[c]);
+res = true;
+break;
+}
+}
+
+done:
+profile_free_list(list);
+profile_release(profile);
+krb5_free_context(context);
+
+return res;
+}
diff --git a/src/util/sss_krb5.h b/src/util/sss_krb5.h
index
462dbbe0bda8969432c8ac2f062a0123c1f098f0..fdaeb49314764e096448f342a054dc6938f0c248
100644
--- a/src/util/sss_krb5.h
+++ b/src/util/sss_krb5.h
@@ -189,4 +189,6 @@ sss_krb5_get_primary(TALLOC_CTX *mem_ctx,
krb5_error_code sss_krb5_kt_have_content(krb5_context context,
krb5_keytab keytab);
+
+bool sss_krb5_realm_has_proxy(const char *realm);
#endif /* __SSS_KRB5_H__ */
--
2.1.0
From
Re: [SSSD] [PATCH] IPA: Remove MPG groups if getgrgid was called before getpw()
On Thu, Jul 30, 2015 at 06:20:31PM +0200, Sumit Bose wrote: On Wed, Jul 22, 2015 at 12:18:07PM +0200, Jakub Hrozek wrote: On Wed, Jul 22, 2015 at 10:01:31AM +0200, Sumit Bose wrote: On Tue, Jul 21, 2015 at 09:41:46PM +0200, Jakub Hrozek wrote: Hi, the attached patch fixes https://fedorahosted.org/sssd/ticket/2724 for me. I haven't yet heard back from the affected user, but they do have test packages available. The patch is working as expected and fixes the issue for me as well. Although it might not look as the most elegant way I think the approach is ok because otherwise we would be to check for every group by GID request if the GID might belong to a MPG which would require a user lookup. This would slow down the requests where the GID belong to a normal (non-MPG) group. Nevertheless I wonder if we should check if the group name matches the user name before deleting the group by GID to be on the safe side? OK, done. Thank you. The patch looks good and is working as advertised, ACK. master: 6fe057efb981ee4b45dcadf131c03f8501fce28d btw the function is really long. Would you oppose to splitting it into smaller functions that save user/group/... ? (We need tests for that, though..) no, feel free to open tickets to track this. https://fedorahosted.org/sssd/ticket/2739 ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCHES] KDC proxy related fixes
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
Hi,
this is not a self-review, I just want the other developers to check
something which I think looks odd.
diff --git a/src/providers/krb5/krb5_child_handler.c
b/src/providers/krb5/krb5_child_handler.c
index
4e453b02d5f4e57b8f5a8f48d7be2434654c0f7e..da5d2707894511a97b7f13eef888f359907b41e0
100644
--- a/src/providers/krb5/krb5_child_handler.c
+++ b/src/providers/krb5/krb5_child_handler.c
@@ -177,7 +177,12 @@ static errno_t create_send_buffer(struct krb5child_req
*kr,
SAFEALIGN_COPY_UINT32(buf-data[rp], kr-uid, rp);
SAFEALIGN_COPY_UINT32(buf-data[rp], kr-gid, rp);
SAFEALIGN_COPY_UINT32(buf-data[rp], validate, rp);
-SAFEALIGN_COPY_UINT32(buf-data[rp], kr-is_offline, rp);
+if (sss_krb5_realm_has_proxy(dp_opt_get_cstring(kr-krb5_ctx-opts,
+KRB5_REALM))) {
+SAFEALIGN_SET_UINT32(buf-data[rp], 0, rp);
+} else {
+SAFEALIGN_COPY_UINT32(buf-data[rp], kr-is_offline, rp);
I didn't change the line above, just moved it. is_offline is defined as
bool in struct krb5child_req. I think we were just lucky the we didn't
run into issues here so far. I think it would be more clean to use
something like:
SAFEALIGN_SET_UINT32(buf-data[rp], (kr-is_offline ? 1 : 0), rp);
to make sure we do not depend on memory layout. I haven't modified it in
the patch because I didn't want to change the existing behaviour.
bye,
Sumit
+}
SAFEALIGN_COPY_UINT32(buf-data[rp], send_pac, rp);
SAFEALIGN_COPY_UINT32(buf-data[rp], use_enterprise_principal, rp);
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCHES] Add support for Smartcard authentication
On Fri, Jul 31, 2015 at 09:28:13AM +0200, Jakub Hrozek wrote: On Thu, Jul 30, 2015 at 09:27:31PM +0200, Jakub Hrozek wrote: On Thu, Jul 30, 2015 at 05:24:27PM +0200, Sumit Bose wrote: On Thu, Jul 30, 2015 at 11:10:33AM +0200, Jakub Hrozek wrote: On Wed, Jul 29, 2015 at 03:34:32PM +0200, Jakub Hrozek wrote: On Tue, Jul 28, 2015 at 10:43:13PM +0200, Sumit Bose wrote: Hi, I started the review, but because the patches are quite big, I will send my comments in batches. I hope that's fine. Thank you for the review. CI failed in distcheck. The failure was due to distcheck's tendency to make the complete source tree read-only and NSS's tendency open all databases in read-write more. I added the needed read-onl options to the NSS code. Coverity found some issues that I forwarded to Sumit. Thank you, I fixed them and hopefully did not create new ones. Coverity is clean this time, but CI did not pass on Debian due to build failure of a test: http://sssd-ci.duckdns.org/logs/job/19/78/debian_testing/ci-build-debug/ci-make-tests.log Normally this is caused by a missing dependency, which the Fedora/RHEL linker can satisfy. Please find attached a new set which should fix the debian build (only the last patch changed). As you suspected it was a missing dependency in Makefile.am. But this make me realize that I didn't implement a OpenSSL/libcrypto version of cert_to_ssh_key() which would break the OpenSSL build. I added an initial version without certificate validation. If you don't like it it can simply replaced by 'return ENOSUPP'. I'm fine with the code as long as you keep the TODOs at least in some todo list of yours. ACK code wise, also my regression tests went fine. I'm only waiting for the automated tests to finish. CI passed, the rawhide failure is unrelated: http://sssd-ci.idm.lab.eng.brq.redhat.com:8080/job/ci/2015/ Pushed to master: * 4de84af23db74e13e867985c9093f394c9fa8d51 * 5242964d275d0b2e96c9b0d1f8a9958c85d566fc * a8d887323f83984679a7d9b827a70146656bb7b2 * 10703cd558016685ee778e333f1d4490238d46e7 * 35f3a213e0f0f2c60e9b5f095a05388e21092ae2 * 45726939a48e605b0166521f94300ae04981a3a7 * 0d5bb38364a6976e9c85d6349aa13a04d181a090 ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] SDAP: rename SDAP_CACHE_PURGE_TIMEOUT
On Thu, Jul 30, 2015 at 02:02:06PM +0200, Petr Cech wrote: On 07/29/2015 08:51 PM, Jakub Hrozek wrote: On Wed, Jul 29, 2015 at 10:19:33AM +0200, Pavel Reichl wrote: Hello, please see trivial patch attached. While I was investigating case I found that to access value of 'ldap_purge_cache_timeout' option I need to use enum value SDAP_CACHE_PURGE_TIMEOUT. I consider this to be a bad name (swap of cache and purge) as I took me additional time to find this out. I think that proposed name is better. Unless somebody feels strongly against the patch I think it could be reviewed by our new colleague. Yes, I assigned the review to Petr. Thanks! ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel Hi, I went through the code, the replacement was done consistently everywhere. I built it successfully. CI tests: http://sssd-ci.duckdns.org/logs/commit/0e/84d48733ed84948e52d62e9f7ca6f40dd7366c/1995/summary.html (Failing is not relevant to the patch.) = ACK Petr * master: 4b1a46396caf656095e5f5e90d43996bdeaba0f3 ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCHES] KDC proxy related fixes
On Fri, Jul 31, 2015 at 11:46:37AM +0200, 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
Hi,
this is not a self-review, I just want the other developers to check
something which I think looks odd.
diff --git a/src/providers/krb5/krb5_child_handler.c
b/src/providers/krb5/krb5_child_handler.c
index
4e453b02d5f4e57b8f5a8f48d7be2434654c0f7e..da5d2707894511a97b7f13eef888f359907b41e0
100644
--- a/src/providers/krb5/krb5_child_handler.c
+++ b/src/providers/krb5/krb5_child_handler.c
@@ -177,7 +177,12 @@ static errno_t create_send_buffer(struct krb5child_req
*kr,
SAFEALIGN_COPY_UINT32(buf-data[rp], kr-uid, rp);
SAFEALIGN_COPY_UINT32(buf-data[rp], kr-gid, rp);
SAFEALIGN_COPY_UINT32(buf-data[rp], validate, rp);
-SAFEALIGN_COPY_UINT32(buf-data[rp], kr-is_offline, rp);
+if (sss_krb5_realm_has_proxy(dp_opt_get_cstring(kr-krb5_ctx-opts,
+KRB5_REALM))) {
+SAFEALIGN_SET_UINT32(buf-data[rp], 0, rp);
+} else {
+SAFEALIGN_COPY_UINT32(buf-data[rp], kr-is_offline, rp);
I didn't change the line above, just moved it. is_offline is defined as
bool in struct krb5child_req. I think we were just lucky the we didn't
run into issues here so far. I think it would be more clean to use
something like:
SAFEALIGN_SET_UINT32(buf-data[rp], (kr-is_offline ? 1 : 0), rp);
to make sure we do not depend on memory layout. I haven't modified it in
the patch because I didn't want to change the existing behaviour.
I agree we should change it. btw on the krb5_child.c side, it's uint32_t
as appropriate.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
