Re: [SSSD] [PATCHES] DYNDNS: support mult. interfaces for dyndns_iface opt
On 07/09/2015 11:15 AM, Pavel Reichl wrote: Hello, please see attached patches. 1st patch adds return value ENOENT to sss_iface_addr_list_get() so I can provide more concrete debug message for missing interface or if interface is not suitable (missing IP address) 2nd patch: * I introduced new public function sss_iface_addr_concatenate(), I'm aware that this function is probably not reusable but I needed to work around that 'struct sss_iface_addr' in defined in source file only. * I had troubles with correctly handling creating talloc hiearchy of IPs of different interfaces. I decided to use first address of first found interface as a parent talloc context for other interfaces. I attached talloc report output to illustrate this. 1. full talloc report on 'struct sdap_dyndns_get_addrs_state' (total 16 bytes in 1 blocks) 2. full talloc report on 'struct sdap_dyndns_get_addrs_state' (total 376 bytes in 19 blocks) 3. struct sss_iface_addr contains360 bytes in 18 blocks (ref 0) 0xbc0650 4. struct sss_iface_addr contains120 bytes in 6 blocks (ref 0) 0xbecee0 5. struct sss_iface_addr contains 80 bytes in 4 blocks (ref 0) 0xbeb920 6. struct sss_iface_addr contains 40 bytes in 2 blocks (ref 0) 0xbd03f0 7. ../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks (ref 0) 0xbd0470 8. ../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks (ref 0) 0xbeb9a0 9. ../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks (ref 0) 0xbecf60 10. struct sss_iface_addr contains120 bytes in 6 blocks (ref 0) 0xbd0640 11. struct sss_iface_addr contains 80 bytes in 4 blocks (ref 0) 0xbd19a0 12. struct sss_iface_addr contains 40 bytes in 2 blocks (ref 0) 0xbcfb00 13. ../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks (ref 0) 0xbed300 14. ../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks (ref 0) 0xbd1a20 15. ../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks (ref 0) 0xbd06c0 16. struct sss_iface_addr contains 80 bytes in 4 blocks (ref 0) 0xbd0eb0 17. struct sss_iface_addr contains 40 bytes in 2 blocks (ref 0) 0xbd1900 18. ../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks (ref 0) 0xbec4f0 19. ../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks (ref 0) 0xbeca10 20. ../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks (ref 0) 0xbe6ae0 * I was thinking whether it would be a good idea to handle the case when processing of interfaces provided in dyndns_iface yields no address at all by continuing to detect DYNDNS address from LDAP connection? Thanks! ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel While reading #2558 I noticed that there is a further request relating to these patches - mbasti asks If there could be some mean how to send IPs of all interfaces? Is this a good idea in general? I suppose I could implement it with some special value for 'dyndns_iface' - ideally the special value would contain characters prohibited to be part of interface name if there are such... ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] PAM: Add pam environment variable AUTH_DOMAIN to pam reply
On Thu, Jul 09, 2015 at 11:50:06AM +0200, Jan Pazdziora wrote: On Thu, Jul 09, 2015 at 11:33:11AM +0200, Sumit Bose wrote: Most probable because ad...@example.test is the Kerberos principal of your user. If SSSD cannot find a matching user name and the name contains an '@' it tries to find a Kerberos principal which matches the full given name. But realms are case sensitive, aren't they? So while it should work for ad...@example.test, it should not for ad...@example.test. you are absolutely correct, not only realms but the whole principal is case-sensitive. But we want to play nice with AD users and make it easy to log in with the alternative domain suffix feature (some call it log in with email address because they often look the same) which is often used in larger forests. Since AD treats the names and principals case-in-sensitive most AD user will not know the correct spelling and so we treat them case-in-sensitive in SSSD as well. bye, Sumit -- Jan Pazdziora Senior Principal Software Engineer, Identity Management Engineering, Red Hat ___ 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] DYNDNS: support mult. interfaces for dyndns_iface opt
Hello,
please see attached patches.
1st patch adds return value ENOENT to sss_iface_addr_list_get() so I can
provide more concrete debug message for missing interface or if
interface is not suitable (missing IP address)
2nd patch:
* I introduced new public function sss_iface_addr_concatenate(), I'm
aware that this function is probably not reusable but I needed to work
around that 'struct sss_iface_addr' in defined in source file only.
* I had troubles with correctly handling creating talloc hiearchy of IPs
of different interfaces. I decided to use first address of first found
interface as a parent talloc context for other interfaces. I attached
talloc report output to illustrate this.
1.
full talloc report on 'struct sdap_dyndns_get_addrs_state' (total
16 bytes in 1 blocks)
2.
full talloc report on 'struct sdap_dyndns_get_addrs_state' (total
376 bytes in 19 blocks)
3.
struct sss_iface_addr contains360 bytes in 18
blocks (ref 0) 0xbc0650
4.
struct sss_iface_addr contains120 bytes in 6 blocks
(ref 0) 0xbecee0
5.
struct sss_iface_addr contains 80 bytes in 4 blocks
(ref 0) 0xbeb920
6.
struct sss_iface_addr contains 40 bytes in 2
blocks (ref 0) 0xbd03f0
7.
../src/providers/dp_dyndns.c:219 contains 16 bytes in
1 blocks (ref 0) 0xbd0470
8.
../src/providers/dp_dyndns.c:219 contains 16 bytes in 1
blocks (ref 0) 0xbeb9a0
9.
../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks
(ref 0) 0xbecf60
10.
struct sss_iface_addr contains120 bytes in 6 blocks
(ref 0) 0xbd0640
11.
struct sss_iface_addr contains 80 bytes in 4 blocks
(ref 0) 0xbd19a0
12.
struct sss_iface_addr contains 40 bytes in 2
blocks (ref 0) 0xbcfb00
13.
../src/providers/dp_dyndns.c:219 contains 16 bytes in
1 blocks (ref 0) 0xbed300
14.
../src/providers/dp_dyndns.c:219 contains 16 bytes in 1
blocks (ref 0) 0xbd1a20
15.
../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks
(ref 0) 0xbd06c0
16.
struct sss_iface_addr contains 80 bytes in 4 blocks
(ref 0) 0xbd0eb0
17.
struct sss_iface_addr contains 40 bytes in 2 blocks
(ref 0) 0xbd1900
18.
../src/providers/dp_dyndns.c:219 contains 16 bytes in 1
blocks (ref 0) 0xbec4f0
19.
../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks
(ref 0) 0xbeca10
20.
../src/providers/dp_dyndns.c:219 contains 16 bytes in 1 blocks
(ref 0) 0xbe6ae0
* I was thinking whether it would be a good idea to handle the case when
processing of interfaces provided in dyndns_iface yields no address at
all by continuing to detect DYNDNS address from LDAP connection?
Thanks!
From 9ac270075b6532e96394543136a27cbf8a51cca6 Mon Sep 17 00:00:00 2001
From: Pavel Reichl prei...@redhat.com
Date: Wed, 8 Jul 2015 09:01:24 -0400
Subject: [PATCH 1/2] DYNDNS: sss_iface_addr_list_get return ENOENT
If none of eligible interfaces matches ifname then ENOENT is returned.
Resolves:
https://fedorahosted.org/sssd/ticket/2558
---
src/providers/dp_dyndns.c| 8 +++-
src/providers/ldap/sdap_dyndns.c | 4
src/tests/cmocka/test_dyndns.c | 20
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/src/providers/dp_dyndns.c b/src/providers/dp_dyndns.c
index 1cac3d0fae2454cea823ed640a4325f27580353f..9552e83d8d4ba615c157c9bae275c8ab867ec274 100644
--- a/src/providers/dp_dyndns.c
+++ b/src/providers/dp_dyndns.c
@@ -222,7 +222,13 @@ sss_iface_addr_list_get(TALLOC_CTX *mem_ctx, const char *ifname,
}
}
-ret = EOK;
+if (addrlist != NULL) {
+/* OK, some result was found */
+ret = EOK;
+} else {
+/* No result was found */
+ret = ENOENT;
+}
*_addrlist = addrlist;
done:
freeifaddrs(ifaces);
diff --git a/src/providers/ldap/sdap_dyndns.c b/src/providers/ldap/sdap_dyndns.c
index 0d9c9205792062378aa25aad6ac706058001433a..bb98f8e1ec6852b04e9ea3e8d5807fecc7e6958d 100644
--- a/src/providers/ldap/sdap_dyndns.c
+++ b/src/providers/ldap/sdap_dyndns.c
@@ -504,6 +504,10 @@ sdap_dyndns_get_addrs_send(TALLOC_CTX *mem_ctx,
if (ret != EOK) {
DEBUG(SSSDBG_OP_FAILURE,
Cannot get list of addresses from interface %s\n, iface);
+/* non critical failure */
+if (ret == ENOENT) {
+ret = EOK;
+}
}
/* We're done. Just fake an async request completion */
goto done;
diff --git a/src/tests/cmocka/test_dyndns.c b/src/tests/cmocka/test_dyndns.c
index 689e333d4e68c4a2582894d06b8b7b20c76b9be8..3214e90c063ea9a4cf6f6bc6507bf4e37b7d23a4 100644
--- a/src/tests/cmocka/test_dyndns.c
+++ b/src/tests/cmocka/test_dyndns.c
@@ -247,6 +247,23 @@ void
Re: [SSSD] [PATCH] UTIL: Function 2string for enum sss_cli_command
Hi!
There is my repaired patch. All of yours comments were helpful.
I renamed the function to sss_cmd2str(), but maybe it could be
sss_cli_cmd2str(). I am not sure with it, but if it is better, I will
rename it again.
Petr
On 07/08/2015 03:26 PM, Sumit Bose wrote:
On Wed, Jul 08, 2015 at 02:13:42PM +0200, Petr Cech wrote:
Hi!
https://fedorahosted.org/sssd/ticket/2703
It's my first patch to this ticket. It is simple transforming of number of
command to the string.
Hi Petr,
welcome and thank you for your first patch. Besides Pavel's suggestions
I have some general comments as well.
- There is pamcmd2str() which does a similar job for the backend code
but I think it is becoming redundant with your patch. Can you remove
this call and use your's where appropriate?
- I haven't tested it, but I'm pretty sure that the PAM module pam_sss
which is build from pam_sss.c and some other files is broken in debug
mode with your patch because sss_log.c is not used when building it and
hence sss_cli_command_2string() will be undefined. You do not see this
during compilation or even during 'make check' because the 'D' macro
is only evaluate if PAM_DEBUG is defined during compilation. If you
run something like 'make CFLAGS+=-DPAM_DEBUG check' the dlopen test
should fail with your patch.
Since the PAM module pam_sss.so might be loaded by any kind of
processes at runtime we try to keep it as simple as possible and try
to add as few dependencies as possible. If you search the Makefile.am
for pam_sss_la_SOURCES you will see that besides source files from the
sss_client directory we only add atomic_io.c and authtok-utils.c which
both contain only a single function with no special dependencies.
I would suggest that you put sss_cli_command_2string() in a file on
its own similar like atomic_io.c or authtok-utils.c. And add this file
to pam_sss_la_SOURCES and libsss_debug_la_SOURCES in Makefile.am. I
leave it up to you to decide what would be a good place for this file.
The sss_client directory because the enum sss_cli_command is defined
here as well or the util directory because the main usage for it is in
the SSSD code and not in the pam_sss module.
bye,
Sumit
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
From 8b3ae05fc97f548256dc8b72863183b9dc9a539a Mon Sep 17 00:00:00 2001
From: Petr Cech pc...@redhat.com
Date: Wed, 8 Jul 2015 07:17:28 -0400
Subject: [PATCH] UTIL: Function 2string for enum sss_cli_command
Improvement of debug messages.
Instead of:(0x0400): Running command [17]...
We could see:(0x0400): Running command [17][SSS_NSS_GETPWNAM]...
Resolves:
https://fedorahosted.org/sssd/ticket/2703
---
Makefile.am | 4 +-
src/providers/dp_pam_data_util.c | 27 +
src/responder/nss/nsssrv_cmd.c | 30 +++---
src/sss_client/pam_sss.c | 6 +-
src/tools/tools_mc_util.c| 4 +-
src/util/sss_cli_cmd.c | 219 +++
src/util/sss_cli_cmd.h | 9 ++
src/util/sss_log.c | 2 -
8 files changed, 256 insertions(+), 45 deletions(-)
create mode 100644 src/util/sss_cli_cmd.c
create mode 100644 src/util/sss_cli_cmd.h
diff --git a/Makefile.am b/Makefile.am
index b8cbc6df23ded1edb945a709b6dbe1c44eb54017..f16b8ebdb4dd66c2d193c19bd8355782f4de4c9a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -678,7 +678,8 @@ endif
pkglib_LTLIBRARIES += libsss_debug.la
libsss_debug_la_SOURCES = \
src/util/debug.c \
-src/util/sss_log.c
+src/util/sss_log.c \
+src/util/sss_cli_cmd.c
libsss_debug_la_LIBADD = \
$(SYSLOG_LIBS)
libsss_debug_la_LDFLAGS = \
@@ -2654,6 +2655,7 @@ pam_sss_la_SOURCES = \
src/sss_client/sss_cli.h \
src/util/atomic_io.c \
src/util/authtok-utils.c \
+src/util/sss_cli_cmd.c \
src/sss_client/sss_pam_macros.h \
src/sss_client/sss_pam_compat.h
diff --git a/src/providers/dp_pam_data_util.c b/src/providers/dp_pam_data_util.c
index 8724bf936f3f46fb8393c8a3da57215a73b4191a..10e91f5f7286db5e76ad98b6c7519f2482d006db 100644
--- a/src/providers/dp_pam_data_util.c
+++ b/src/providers/dp_pam_data_util.c
@@ -23,33 +23,10 @@
*/
#include providers/data_provider.h
-
+#include util/sss_cli_cmd.h
#define PAM_SAFE_ITEM(item) item ? item : not set
-static const char *pamcmd2str(int cmd) {
-switch (cmd) {
-case SSS_PAM_AUTHENTICATE:
-return PAM_AUTHENTICATE;
-case SSS_PAM_SETCRED:
-return PAM_SETCRED;
-case SSS_PAM_ACCT_MGMT:
-return PAM_ACCT_MGMT;
-case SSS_PAM_OPEN_SESSION:
-return PAM_OPEN_SESSION;
-case SSS_PAM_CLOSE_SESSION:
-return PAM_CLOSE_SESSION;
-case SSS_PAM_CHAUTHTOK:
-return PAM_CHAUTHTOK;
-case SSS_PAM_CHAUTHTOK_PRELIM:
-return PAM_CHAUTHTOK_PRELIM;
-case SSS_PAM_PREAUTH:
-
Re: [SSSD] [PATCH] PAM: Add pam environment variable AUTH_DOMAIN to pam reply
On Tue, Jun 30, 2015 at 02:09:31PM +0200, Sumit Bose wrote: It does the right thing and I'm able to get the value back via pam_getenv(pamh, PAM_ENV_AUTH_DOMAIN) in my Apache module. My only concern is that the domain name as returned by sssd is lowercase which does not really match the realm as seen by say mod_auth_kerb or mod_auth_gssapi. But I guess uppercasing the string is up to consumer of that value. hm, the ticket said domain and not realm and unfortunately there might be cases where the upper-case domain name does not match the realm used for authentication. I've tried to check the behaviour with ssh and it's even more confusing. I have IPA-enrolled machine, IPA domain example.test, realm EXAMPLE.TEST. I've tried to isolate the SSSD domain namespace from the rest. I've changed [domain/example.test] to [domain/xxexample.test] and domains = example.test to domains = xxexample.test in sssd.conf, and I've set use_fully_qualified_names = True. My expectation is that the canonical username of the user will be $u...@xxexample.test. That is true, however when I kinit admin, all the following commands ssh ad...@xxexample.test@client.example.tst id ssh ad...@xxexample.test@client.example.tst id ssh ad...@example.test@client.example.tst id ssh ad...@example.test@client.example.tst id print uid=193940(ad...@xxexample.test) ... So it's nice that the canonical fully qualified name uses the SSSD domain (the same which I expect PAM stack to return in PAM_ENV_AUTH_DOMAIN), but: why am I able to authenticate as ad...@example.test@client.example.tst even if there is no example.test domain defined in sssd.conf anymore? -- Jan Pazdziora Senior Principal Software Engineer, Identity Management Engineering, Red Hat ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] PAM: Add pam environment variable AUTH_DOMAIN to pam reply
On Thu, Jul 09, 2015 at 11:27:14AM +0200, Jan Pazdziora wrote: On Tue, Jun 30, 2015 at 02:09:31PM +0200, Sumit Bose wrote: It does the right thing and I'm able to get the value back via pam_getenv(pamh, PAM_ENV_AUTH_DOMAIN) in my Apache module. My only concern is that the domain name as returned by sssd is lowercase which does not really match the realm as seen by say mod_auth_kerb or mod_auth_gssapi. But I guess uppercasing the string is up to consumer of that value. hm, the ticket said domain and not realm and unfortunately there might be cases where the upper-case domain name does not match the realm used for authentication. I've tried to check the behaviour with ssh and it's even more confusing. I have IPA-enrolled machine, IPA domain example.test, realm EXAMPLE.TEST. I've tried to isolate the SSSD domain namespace from the rest. I've changed [domain/example.test] to [domain/xxexample.test] and domains = example.test to domains = xxexample.test in sssd.conf, and I've set use_fully_qualified_names = True. My expectation is that the canonical username of the user will be $u...@xxexample.test. That is true, however when I kinit admin, all the following commands ssh ad...@xxexample.test@client.example.tst id ssh ad...@xxexample.test@client.example.tst id ssh ad...@example.test@client.example.tst id ssh ad...@example.test@client.example.tst id print uid=193940(ad...@xxexample.test) ... So it's nice that the canonical fully qualified name uses the SSSD domain (the same which I expect PAM stack to return in PAM_ENV_AUTH_DOMAIN), but: why am I able to authenticate as ad...@example.test@client.example.tst even if there is no example.test domain defined in sssd.conf anymore? Most probable because ad...@example.test is the Kerberos principal of your user. If SSSD cannot find a matching user name and the name contains an '@' it tries to find a Kerberos principal which matches the full given name. HTH bye, Sumit -- Jan Pazdziora Senior Principal Software Engineer, Identity Management Engineering, Red Hat ___ 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
Re: [SSSD] [PATCH] PAM: Add pam environment variable AUTH_DOMAIN to pam reply
On Thu, Jul 09, 2015 at 11:33:11AM +0200, Sumit Bose wrote: Most probable because ad...@example.test is the Kerberos principal of your user. If SSSD cannot find a matching user name and the name contains an '@' it tries to find a Kerberos principal which matches the full given name. But realms are case sensitive, aren't they? So while it should work for ad...@example.test, it should not for ad...@example.test. -- Jan Pazdziora Senior Principal Software Engineer, Identity Management Engineering, Red Hat ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Re: [SSSD] [PATCH] PAM: Add pam environment variable AUTH_DOMAIN to pam reply
On Tue, Jun 30, 2015 at 02:15:27PM +0200, Jan Pazdziora wrote: If for the applications realm is also useful, then a) the app can query the sssd dbus interface for the matching realm for the domain b) we can also return the realm, although the expectations should be set right in documentation that not all domain types have the realm info (LDAP domain for example) So how about the reverse mapping? Going from realm to SSSD domain / section? Currently for example in mod_authnz_pam we consider them interchangeable -- we just take the principal name (either fully qualified or not) and pass it to pam_sss.so via pam_acct_mgmt to evaluate the access check. Should we get the SSSD domain / section value for that? I've filed https://fedorahosted.org/sssd/ticket/2709 for some way to get the SSSD domain based on the realm. -- Jan Pazdziora Senior Principal Software Engineer, Identity Management Engineering, Red Hat ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
