7:08 a.m.
URL: https://github.com/SSSD/sssd/pull/127
Author: pbrezina
Title: #127: ssh: use cache_req
Action: opened
PR body:
"""
This patches makes SSH responder use the cache_req interface.
Since this responder uses that same `cache-domain-cache` lookup logic
for host certificates I implemented `host by name` request in `cache_req`.
In order to achieve this I moved data provider lookup function from `cache_req`
code into plugins.
The first two patches fixes minor issues in the SSH responder and should be
pushed to earlier versions as well. The first patch fix a little issue
introduced probably by overrides and the second patch removes name qualification
since it is already qualified in the sysdb since fqname patches.
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/127/head:pr127
git checkout pr127
7:32 a.m.
New subject: [sssd PR#127][synchronized] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Author: pbrezina
Title: #127: ssh: use cache_req
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/127/head:pr127
git checkout pr127
4:28 a.m.
New subject: [sssd PR#127][comment] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Title: #127: ssh: use cache_req
jhrozek commented:
"""
I will review this as these changes make the requested changes to the files provider code
on review easier.
"""
See the full comment at https://github.com/SSSD/sssd/pull/127#issuecomment-276027605
7:51 a.m.
New subject: [sssd PR#127][comment] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Title: #127: ssh: use cache_req
jhrozek commented:
"""
There is a bug when looking up keys for a user from a trusted domain:
```
==3945== Invalid read of size 8
==3945== at 0x4100FE: sss_dp_callback_destructor (responder_dp.c:61)
==3945== by 0x9019C0F: ??? (in /usr/lib64/libtalloc.so.2.1.8)
==3945== by 0x901B7F0: _talloc_free (in /usr/lib64/libtalloc.so.2.1.8)
==3945== by 0x77D4DBF: tevent_req_received (in /usr/lib64/libtevent.so.0.9.30)
==3945== by 0x77D4DF8: ??? (in /usr/lib64/libtevent.so.0.9.30)
==3945== by 0x9019C0F: ??? (in /usr/lib64/libtalloc.so.2.1.8)
==3945== by 0x9019694: ??? (in /usr/lib64/libtalloc.so.2.1.8)
==3945== by 0x901B7F0: _talloc_free (in /usr/lib64/libtalloc.so.2.1.8)
==3945== by 0x77D4DBF: tevent_req_received (in /usr/lib64/libtevent.so.0.9.30)
==3945== by 0x77D4DF8: ??? (in /usr/lib64/libtevent.so.0.9.30)
==3945== by 0x901BADF: _talloc_free (in /usr/lib64/libtalloc.so.2.1.8)
==3945== by 0x405F97: ssh_cmd_get_user_pubkeys_done (ssh_cmd.c:134)
==3945== Address 0xb687390 is 688 bytes inside a block of size 797 free'd
==3945== at 0x4C2ED4A: free (vg_replace_malloc.c:530)
==3945== by 0x901B89A: _talloc_free (in /usr/lib64/libtalloc.so.2.1.8)
==3945== by 0x411047: sss_dp_req_done (responder_dp.c:408)
==3945== by 0x411E34: sss_dp_internal_get_done (responder_dp.c:840)
==3945== by 0x5728391: ??? (in /usr/lib64/libdbus-1.so.3.16.3)
==3945== by 0x572BCDE: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.16.3)
==3945== by 0x4E8A4E8: sbus_dispatch (sssd_dbus_connection.c:96)
==3945== by 0x77D84FF: tevent_common_loop_timer_delay (in
/usr/lib64/libtevent.so.0.9.30)
==3945== by 0x77D9518: ??? (in /usr/lib64/libtevent.so.0.9.30)
==3945== by 0x77D7C06: ??? (in /usr/lib64/libtevent.so.0.9.30)
==3945== by 0x77D3ABC: _tevent_loop_once (in /usr/lib64/libtevent.so.0.9.30)
==3945== by 0x77D3CEA: tevent_common_loop_wait (in /usr/lib64/libtevent.so.0.9.30)
==3945== Block was alloc'd at
==3945== at 0x4C2DB9D: malloc (vg_replace_malloc.c:299)
==3945== by 0x901F2BB: _talloc_pooled_object (in /usr/lib64/libtalloc.so.2.1.8)
==3945== by 0x77D4ACC: _tevent_req_create (in /usr/lib64/libtevent.so.0.9.30)
==3945== by 0x4118B9: sss_dp_internal_get_send (responder_dp.c:689)
==3945== by 0x410C5F: sss_dp_issue_request (responder_dp.c:334)
==3945== by 0x411351: sss_dp_get_account_send (responder_dp.c:528)
==3945== by 0x41A0CB: cache_req_user_by_upn_dp_send (cache_req_user_by_upn.c:102)
==3945== by 0x418266: cache_req_search_dp (cache_req_search.c:289)
==3945== by 0x417FDB: cache_req_search_send (cache_req_search.c:232)
==3945== by 0x416912: cache_req_next_domain (cache_req.c:544)
==3945== by 0x416D00: cache_req_done (cache_req.c:693)
==3945== by 0x77D4473: tevent_common_loop_immediate (in
/usr/lib64/libtevent.so.0.9.30)
```
"""
See the full comment at https://github.com/SSSD/sssd/pull/127#issuecomment-276367683
8:15 a.m.
New subject: [sssd PR#127][+Changes requested] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Title: #127: ssh: use cache_req
Label: +Changes requested
6:21 a.m.
New subject: [sssd PR#127][synchronized] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Author: pbrezina
Title: #127: ssh: use cache_req
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/127/head:pr127
git checkout pr127
6:28 a.m.
New subject: [sssd PR#127][synchronized] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Author: pbrezina
Title: #127: ssh: use cache_req
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/127/head:pr127
git checkout pr127
6:32 a.m.
New subject: [sssd PR#127][comment] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Title: #127: ssh: use cache_req
pbrezina commented:
"""
It should be fixed. Thanks.
"""
See the full comment at https://github.com/SSSD/sssd/pull/127#issuecomment-276646417
10:09 a.m.
New subject: [sssd PR#127][comment] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Title: #127: ssh: use cache_req
jhrozek commented:
"""
The code looks good to me now but I found one regression - if you set
default_domain_suffix to the AD domain and try to look up a host, the ssh responder will
query the AD domain. Since hosts can only be placed in the IPA domain, we should ignore
the default_domain_suffix for host searches.
And one more question is related to the first two patches -- looks like they are
legitimate bugs in the 1.14 codebase, should we push them to 1.14 separately? If yes, I
would prefer to have some better commit message with steps to reproduce the bug or at
least a desription.
"""
See the full comment at https://github.com/SSSD/sssd/pull/127#issuecomment-277001067
6:16 a.m.
New subject: [sssd PR#127][synchronized] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Author: pbrezina
Title: #127: ssh: use cache_req
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/127/head:pr127
git checkout pr127
6:17 a.m.
New subject: [sssd PR#127][comment] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Title: #127: ssh: use cache_req
pbrezina commented:
"""
The code looks good to me now but I found one regression - if you set
default_domain_suffix to the AD domain and try to look up a host, the ssh responder will
query the AD domain. Since hosts can only be placed in the IPA domain, we should ignore
the default_domain_suffix for host searches.
Thanks. Fixed.
And one more question is related to the first two patches -- looks
like they are legitimate bugs in the 1.14 codebase, should we push them to 1.14
separately? If yes, I would prefer to have some better commit message with steps to
reproduce the bug or at least a desription.
Yes, those two patches should be pushed into 1.14. I improved commit message.
"""
See the full comment at https://github.com/SSSD/sssd/pull/127#issuecomment-277233928
10:40 a.m.
New subject: [sssd PR#127][-Changes requested] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Title: #127: ssh: use cache_req
Label: -Changes requested
10:18 a.m.
New subject: [sssd PR#127][comment] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Title: #127: ssh: use cache_req
jhrozek commented:
"""
I'm afraid this is still not fixed correctly. Please check this gdb session when I
requested an IPA host with a default_domain_suffix pointing to a Windows domain:
```
0x00007f0a1cc0d543 in __epoll_wait_nocancel () at ../sysdeps/unix/syscall-template.S:84
84 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
Missing separate debuginfos, use: dnf debuginfo-install sssd-common-1.14.2-2.fc25.x86_64
(gdb) b sysdb_search_ssh_hosts
Breakpoint 1 at 0x7f0a21095b90: file /sssd/src/db/sysdb_ssh.c, line 290.
(gdb) c
Continuing.
Breakpoint 1, sysdb_search_ssh_hosts (mem_ctx=0x20abf60, domain=0x20bb680,
filter=0x20bd6a0 "(name=client.ipa.test)", attrs=0x20b73f0,
num_hosts=0x7fffa2d19770, hosts=0x7fffa2d19768)
at /sssd/src/db/sysdb_ssh.c:290
290 tmp_ctx = talloc_new(NULL);
(gdb) c
Continuing.
Breakpoint 1, sysdb_search_ssh_hosts (mem_ctx=0x20abf60, domain=0x20bb680,
filter=0x20b8440 "(name=client.ipa.test)", attrs=0x20b73f0,
num_hosts=0x7fffa2d198e0, hosts=0x7fffa2d198d8)
at /sssd/src/db/sysdb_ssh.c:290
290 tmp_ctx = talloc_new(NULL);
(gdb) n
291 if (!tmp_ctx) {
(gdb)
295 ret = sysdb_search_custom(tmp_ctx, domain, filter,
(gdb)
298 if (ret != EOK && ret != ENOENT) {
(gdb) p ret
$1 = 2
(gdb) p filter
$2 = 0x20b8440 "(name=client.ipa.test)"
(gdb) p *domain
$3 = {name = 0x20ac820 "win.trust.test", conn_name = 0x20b7d60
"ipa.test", provider = 0x20bc0f0 "ipa", timeout = 0,
enumerate = false, sd_enumerate = 0x0, fqnames = true, mpg = true, ignore_group_members
= false, id_min = 1,
id_max = 4294967295, cache_credentials = true, cache_credentials_min_ff_length = 8,
legacy_passwords = false,
case_sensitive = false, case_preserve = false, override_gid = 0, override_homedir = 0x0,
fallback_homedir = 0x0,
subdomain_homedir = 0x20ab490 "/home/%d/%u", homedir_substr = 0x0,
override_shell = 0x0, default_shell = 0x0,
user_timeout = 5400, group_timeout = 5400, netgroup_timeout = 5400, service_timeout =
5400, autofsmap_timeout = 0,
sudo_timeout = 0, ssh_host_timeout = 0, refresh_expired_interval = 0,
subdomain_refresh_interval = 0,
cached_auth_timeout = 0, pwd_expiration_warning = -1, sysdb = 0x20b31d0, names =
0x20ad590, parent = 0x20aed20,
subdomains = 0x0, realm = 0x20baff0 "WIN.TRUST.TEST", flat_name = 0x20bc490
"WIN",
domain_id = 0x20b6060 "S-1-5-21-1733986419-3120376054-1365464917",
trust_direction = 0, subdomains_last_checked = {
tv_sec = 0, tv_usec = 0}, has_views = false, view_name = 0x20b85f0
"default", prev = 0x0, next = 0x20be700,
state = DOM_ACTIVE, sd_inherit = 0x0, forest = 0x20ae870 "win.trust.test",
forest_root = 0x20bb680,
upn_suffixes = 0x0}
(gdb)
```
"""
See the full comment at https://github.com/SSSD/sssd/pull/127#issuecomment-277731678
10:18 a.m.
New subject: [sssd PR#127][+Changes requested] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Title: #127: ssh: use cache_req
Label: +Changes requested
4:56 a.m.
New subject: [sssd PR#127][comment] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Title: #127: ssh: use cache_req
pbrezina commented:
"""
Try now.
"""
See the full comment at https://github.com/SSSD/sssd/pull/127#issuecomment-277965152
4:56 a.m.
New subject: [sssd PR#127][synchronized] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Author: pbrezina
Title: #127: ssh: use cache_req
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/127/head:pr127
git checkout pr127
5:20 a.m.
New subject: [sssd PR#127][comment] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Title: #127: ssh: use cache_req
jhrozek commented:
"""
On Tue, Feb 07, 2017 at 02:56:00AM -0800, Pavel Březina wrote:
Try now.
thanks, that case is now fixed. I'll run a couple of more tests before
acking.
"""
See the full comment at https://github.com/SSSD/sssd/pull/127#issuecomment-277970321
8:39 a.m.
New subject: [sssd PR#127][+Accepted] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Title: #127: ssh: use cache_req
Label: +Accepted
8:39 a.m.
New subject: [sssd PR#127][-Changes requested] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Title: #127: ssh: use cache_req
Label: -Changes requested
4 a.m.
New subject: [sssd PR#127][comment] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Title: #127: ssh: use cache_req
lslebodn commented:
"""
The commit "cache_req: add api to create ldb_result from message" broke
integration tests:
http://sssd-ci.duckdns.org/logs/job/62/01/summary.html
The following commit fixed that. IMHO changing order should be safe.
I tried to run some downstream tests and they failed; need to find a reason
"""
See the full comment at https://github.com/SSSD/sssd/pull/127#issuecomment-278283530
4:14 a.m.
New subject: [sssd PR#127][comment] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Title: #127: ssh: use cache_req
jhrozek commented:
"""
Well, I *just* pushed the patches:
0b7ded1..a8191ce
That's why I leave the accepted flag for quite some time before pushing..feel free to
just remove it next time if you plan on testing patches more than the reviewee.
"""
See the full comment at https://github.com/SSSD/sssd/pull/127#issuecomment-278286805
4:14 a.m.
New subject: [sssd PR#127][closed] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Author: pbrezina
Title: #127: ssh: use cache_req
Action: closed
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/127/head:pr127
git checkout pr127
3:20 a.m.
New subject: [sssd PR#127][+Pushed] ssh: use cache_req
URL: https://github.com/SSSD/sssd/pull/127
Title: #127: ssh: use cache_req
Label: +Pushed
2633
days inactive
2684
days old
sssd-devel@lists.fedorahosted.org
22 comments
4 participants
participants (4)
-
fidencio -
jhrozek
-
lslebodn
-
pbrezina