[SSSD] [sssd PR#947][comment] tests: fix race conditions in integration tests
URL: https://github.com/SSSD/sssd/pull/947 Title: #947: tests: fix race conditions in integration tests spbnick commented: """ @pbrezina, Hi Pavel :) I don't really remember, sorry, but looking at the code now it might be to avoid racing with SSSD initialization, while making the change before the timeout expires. I.e. to have the database change between the SSSD initialization and the first timeout, perhaps? """ See the full comment at https://github.com/SSSD/sssd/pull/947#issuecomment-559474330 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#346][comment] intg: Disable add_remove tests
URL: https://github.com/SSSD/sssd/pull/346 Title: #346: intg: Disable add_remove tests spbnick commented: """ Sure, go ahead and push it. My comment is just my preference. I can go on to explaining the reasoning, but I wouldn't want to force it on you and delay your work. """ See the full comment at https://github.com/SSSD/sssd/pull/346#issuecomment-321742523 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#346][comment] intg: Disable add_remove tests
URL: https://github.com/SSSD/sssd/pull/346 Title: #346: intg: Disable add_remove tests spbnick commented: """ This is a good move, and looks fine to me, except I would prefer to have a comment in front of each disabled test saying it is disabled and briefly explaining why (including a reference to the issue perhaps). """ See the full comment at https://github.com/SSSD/sssd/pull/346#issuecomment-321588154 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#341][comment] CACHE_REQ_SR_OVERLAY: Fix coverity warning
URL: https://github.com/SSSD/sssd/pull/341 Title: #341: CACHE_REQ_SR_OVERLAY: Fix coverity warning spbnick commented: """ Yes, this is absolutely correct, thank you, Fabiano, ACK! """ See the full comment at https://github.com/SSSD/sssd/pull/341#issuecomment-318995775 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Woo-hoo :D! Thanks a lot for all the work, Pavel, Lukas and Jakub :)! """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-318342564 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ CI result: http://sssd-ci.duckdns.org/logs/commit/17/e2bb8c81901721fc35f7807f8c000aea950c27/7063/summary.html """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-305196182 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Rebased, and swapped `cache_req_process_result` and `cache_req_done`, as requested. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-305174143 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ I'm on it, thanks for review @pbrezina! """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-305148541 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ This update rebases on latest master and also changes "tlog-rec" to "tlog-rec-session", as the latter is now supposed to be used as the login shell in upstream tlog. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-302695875 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ CI results: http://sssd-ci.duckdns.org/logs/job/69/46/summary.html """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-301045414 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Alright, here is another version of the patchset. Changes include: * abort SSSD startup if session recording is enabled, but session recording shell is not present, or not executable * add configure option disabling session recording (`--without-session-recording`); * fix the `--with-session-recording-shell` configure option; * rename `pam_reply_export_shell` to `pam_reply_sr_export_shell` as it deals with session recording exclusively. If we decide we don't need the `--without-session-recording` configure option, we can just drop the last patch. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-298836574 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Yes, that's what I meant. Only perhaps we can just use `--with-session-recording-shell=/bin/false` and not have any scripts. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-298629188 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Integration tests don't actually need tlog to verify that SSSD's job is done properly. Also, tlog is not going to be officially present for a while yet on most distros we test. OTOH, nothing prevents users installing it there themselves and so SSSD functionality needs to be verified. Ignoring absence of tlog would not be a security issue, but only a functionality issue, because recorded users simply won't be able to login. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-298612435 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ There is one other problem: integration tests. If we make SSSD fail on startup when it doesn't find tlog-rec, then we can't run SSSD as is in integration tests. That is without requiring tlog-rec be installed for integration testing, which is hassle, as tlog is only present in Fedora so far. So, either we make it a warning, which will be harder to notice for users, or perhaps add a configure option to make the session recording shell something that we know always exists, like `/bin/true`. I don't like the complications of the latter, but I think it might be better for user. Pavel, Lukas? """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-298608203 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Yeah, perhaps not starting SSSD can help. Still, there is the issue of tlog not being available for RHEL at all, at least not from official channels. Should SSSD have session recording documented and supported then? """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-298063944 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Sure, a weak dependency sounds right. I would not like disabling SSSD or session recording in case tlog is missing. If it can be fixed by just installing tlog, then we shouldn't break SSSD, and we should let users fix the situation with as little fuss as possible. I.e. without the need to restart SSSD. I wouldn't say it's a misconfiguration of SSSD, it's a system misconfiguration, and it's not SSSD's business to abort because of that. As far as SSSD is concerned its configuration is valid. However, I think SSSD can shout in the log that tlog is missing. The question is at which point. Maybe on PAM session setup? """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-298024703 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Sure, a weak dependency sounds right. I would not like disabling SSSD or session recording in case tlog is missing. If it can be fixed by just installing tlog, then we shouldn't break SSSD, and let users fix the situation with as little fuss as possible. I.e. without the need to restart SSSD. I wouldn't say it's a misconfiguration of SSSD, it's a system misconfiguration, and it's not SSSD's business to abort because of that. As far as SSSD is concerned its configuration is valid. However, I think SSSD can shout in the log that tlog is missing. The question is at which point. Maybe on PAM session setup? """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-298024703 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Sure, a weak dependency sounds right. I would not like disabling SSSD or session recording in case tlog is missing. If it can be fixed by just installing tlog, then we shouldn't break SSSD, and let users fix the situation with as little fuss as possible. I.e. without the need to restart SSSD. I wouldn't say it's a misconfiguration of SSSD, it's a system misconfiguration, and it's not SSSD's business to abort because of that. As far as SSSD is concerned its configuration is valid. However, I think SSSD can shout in the log that tlog is missing. The question is at which point. Maybe on PAM session setup? """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-298024703 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Ah, I see. I understand what you meant regarding enable-files-domain. I'll try to explain what we're trying to do with tlog. First of all, session recording should always be disabled by default. No matter if tlog is present or not. Next, SSSD will function perfectly, also no matter if tlog is present or not, even if session recording is enabled and configured. The only problem is the users won't be able to login if session recording is enabled for them and tlog is not installed. A little recap on how session recording setup works in SSSD: you get to choose if session recording is disabled for all, enabled for all, or enabled for some users/groups. In the latter case you get to choose which users/groups have it enabled. For enabled users SSSD makes NSS report their shell as tlog, and puts the original shell into an environment variable when PAM sets up the session, so tlog can start the original shell. So if tlog is not installed, users simply get non-existing shell. I was thinking we could just have session recording support built and documented in manpages unconditionally, and only add it to SSSD dependencies on distros where tlog is present. However, I worry that RHEL customers will then be misled, because I expect tlog is not going to be available for RHEL for a while. So the question is do we build/document session recording conditionally, selected at configure time, or not. Also, do we add tlog to dependencies if we have session recording support built. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-297999419 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Ah, I see. I understand what you meant regarding enable-files-domain. I'll try to explain what we're trying to do with tlog. First of all, session recording should always be disabled by default. No matter if tlog is present or not. Next, SSSD will function perfectly, also no matter if tlog is present or not, even if session recording is enabled and configured. The only problem is the users won't be able to login if session recording is enabled for them and tlog is not installed. A little recap on how session recording setup works in SSSD: you get to choose if session recording is disabled for all, enabled for all, or enabled for some users/groups. In the latter case you get to choose which users/groups have it enabled. For enabled users SSSD makes NSS report their shell as tlog, and puts the original shell into an environment variable when PAM sets up the session, so tlog can start the original shell. So if tlog is not installed, users simply get non-existing shell. I was thinking we could just have session recording support built and documented in manpages unconditionally, and only add it to SSSD dependencies on distros where tlog is present. However, I worry that RHEL customers will then be misled, because I expect tlog is not going to be available for RHEL for a while. So the question is do we build/document session recording conditionally, selected at configure time, or not. Also, do we add tlog to dependencies if we have session recording support built. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-297999419 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Hi Lukas, I'm sorry I don't quite understand. I agree that SSSD would have a runtime dependency on tlog to use this feature, i.e. tlog would need to be installed to work. However, how does this come from SSSD needing to compile on el7? Also what part of enable_files_domain we can reuse? Pavel, perhaps you understand and can explain? Thank you. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-297973256 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Pavel, I tried to address your latest comments. I had a difficulty with the last one, though, the comment regarding "CACHE_REQ: Pull sessionRecording attrs from initgr". I think I fixed the order, but I'm not sure if you wanted me to change any names, and if so, to what. Please explain what you need me do, if I did it wrong. I made the draft manpage updates too, please take a look. I think perhaps we shouldn't do any conditional building of tlog support, but I worry what RHEL customers will say when they know tlog is not available in RHEL yet. Perhaps we can get it in EPEL? Another thing: I'll likely replace `tlog-rec` with something else, perhaps something called `tlog-session`, because I want to have `tlog-rec` purely for recording and not track user sessions. Just a heads-up. If you'd like details, see Scribery/tlog#92. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-297766450 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Alright, I'll work on the manpages as well. Another thing: should we also make a configure option for enabling session recording, so that we can have this enabled and have an RPM dependency in Fedora, where tlog is already available, and disabled in RHEL and everywhere else, until it is available there? """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-297404214 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ BTW, should I perhaps include an update to the sssd.conf man page? """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-297017382 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Hi Pavel, thank you for your review! I'll be addressing your comments soon, but for now here is how to test this. The patches add support for a new section in sssd.conf: `session_recording`. The section can have up to three options: `scope`, `users`, and `groups`. The `scope` option accepts one of three values: `none`, `all`, and `some`. They mean "no users will be recorded", "all users will be recorded", and "some (specified) users and/or groups will be recorded", respectively. If `scope` is set to `some`, then the `users` option accepts a whitespace-separated list of users to have session recording enabled, same goes for `groups` option, but only for groups. Enabling session recording for a user should result in SSSD reporting user shell as `/usr/bin/tlog-rec` through NSS, and exporting `TLOG_REC_SHELL` environment variable during PAM session setup. That variable should contain the actual user shell. Testing for those should be sufficient, but if you'd like, you can get tlog here: https://github.com/Scribery/tlog You can either build and install it yourself, or use an RPM from the latest release: https://github.com/Scribery/tlog/releases/tag/v3 You can also take a look at the tests in `src/tests/intg/test_session_recording.py` for configuration examples. Please let me know if you needed some other information. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-297016412 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Pavel, I tried to address all your comments, and also added the fix you made to data provider initialization regarding overrides. I also improved the tests. This is ready for another review. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-296623885 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Pavel, after your suggestion on IRC I replaced `sysdb_initgroups` with `sysdb_initgroups_with_views` in the data provider, and also switched to using `sss_view_ldb_msg_find_attr_as_uint64` and `sysdb_getgrgid_with_views` for retrieving the primary group, but the result is the same. All the test_session_recording_some_groups_overridden* tests fail, there is no trace of group overrides being in effect. I suspect I'm missing something basic. Do you have any suggestions? Thank you. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-295768680 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Pavel, after your suggestion on IRC I replaced `sysdb_initgroups` with `sysdb_initgroups_with_views` in the data provider, and also switched to using `sss_view_ldb_msg_find_attr_as_uint64` and `sysdb_getgrgid_with_views` for retrieving the primary group, but the result is the same. All the test_session_recording_some_groups_overridden* tests fail, there is no trace of group overrides being in effect. I suspect I'm missing something basic. Do you have any suggestions? Thank you. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-295768680 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Thank you. Yes, I think it answers my question. I'll do another sysdb_initgroups in the postprocess function. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-294827141 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Thanks, Pavel. I'm still confused, though. At the moment we invoke `create_initgr_ctx` with an `ldb_result` returned from `sysdb_initgroups`. If `sysdb_initgroups` returned `ENOENT`, or produced `res->count == 0`, we will have nothing to supply `create_initgr_ctx` with, and even if it worked, `dp_req_initgr_pp_sr_overlay` would have nothing to match against. I guess I don't understand the whole mechanism: first we get the result from the cache, then we do the request. Doesn't make sense to me. Could you please explain what's going on here? Thank you """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-293816585 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Thanks, Pavel! Is it ensured that the primary group name will always be in sysdb at that point? """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-293521162 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136
Title: #136: Tlog integration
spbnick commented:
"""
Pavel, another question: aren't we skipping the request altogether in
dp_initgroups, instead of just the post-processing callback here:
```C
ret = sysdb_initgroups(sbus_req, domain, data->filter_value, );
if (ret == ENOENT || (ret == EOK && res->count == 0)) {
/* There is no point in contacting NSS responder. Proceed as usual. */
return EAGAIN;
} else if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get initgroups [%d]: %s\n",
ret, sss_strerror(ret));
goto done;
}
```
"""
See the full comment at
https://github.com/SSSD/sssd/pull/136#issuecomment-293271093
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136
Title: #136: Tlog integration
spbnick commented:
"""
Pavel, another question: aren't we skipping the request altogether in
dp_initgroups, instead of just the post-processing callback here:
```C
ret = sysdb_initgroups(sbus_req, domain, data->filter_value, );
if (ret == ENOENT || (ret == EOK && res->count == 0)) {
/* There is no point in contacting NSS responder. Proceed as usual. */
return EAGAIN;
} else if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Unable to get initgroups [%d]: %s\n",
ret, sss_strerror(ret));
goto done;
}
```
"""
See the full comment at
https://github.com/SSSD/sssd/pull/136#issuecomment-293271093
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ > Primary group is specified with gidNumber and suplementary groups are > specified with memberUid/memberOf. Primary group may or may not be part of > suplementary groups so yes, you need to special case it here. Pavel, how can we get the primary group name here, in case it is not in the supplementary groups? """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-293244825 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Pardon for the review request, Pavel. Trying to figure out how they work on GitHub. Please ignore. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-293225994 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Hi Pavel, thanks a lot for a thorough review and for your kind words! I'll be fixing the code according to your comments and will answer your question below. > I would welcome a little bit shorter enum values for scope, e.g. > SESSION_RECORDING_SCOPE_NONE. Or do you think that the CONF word is important? Not really, I can rename the module to just "session_recording.c" and remove "_CONF" from the names. It's an artifact from the time there were more "session_recording" modules in the same directory, and, anyway, SSSD is not adhering to "module name is the symbol name prefix" policy anyway. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-292191969 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ A better CI result: http://sssd-ci.duckdns.org/logs/job/66/48/summary.html """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-290197456 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Alright, this one includes PAM exporting the original shell as well. One thing that bothers me about the implementation is that now all responders are reading the shell settings from the NSS section. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-290127604 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][synchronized] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136
Author: spbnick
Title: #136: Tlog integration
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/136/head:pr136
git checkout pr136
From 22256f94283bce43698b903f6ccb93e58031784c Mon Sep 17 00:00:00 2001
From: Nikolai Kondrashov <nikolai.kondras...@redhat.com>
Date: Fri, 24 Mar 2017 16:24:22 +0200
Subject: [PATCH 01/17] CACHE_REQ: Propagate num_results to cache_req_state
The num_results field in struct cache_req_state was only set in case of
well-known objects, set it also for the regular results for uniformity,
and for later use by session recording code.
---
src/responder/common/cache_req/cache_req.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/responder/common/cache_req/cache_req.c b/src/responder/common/cache_req/cache_req.c
index 4831263..96e7a26 100644
--- a/src/responder/common/cache_req/cache_req.c
+++ b/src/responder/common/cache_req/cache_req.c
@@ -548,7 +548,8 @@ static void cache_req_search_domains_done(struct tevent_req *subreq)
static errno_t
cache_req_search_domains_recv(TALLOC_CTX *mem_ctx,
struct tevent_req *req,
- struct cache_req_result ***_results)
+ struct cache_req_result ***_results,
+ size_t *_num_results)
{
struct cache_req_search_domains_state *state;
@@ -559,6 +560,9 @@ cache_req_search_domains_recv(TALLOC_CTX *mem_ctx,
if (_results != NULL) {
*_results = talloc_steal(mem_ctx, state->results);
}
+if (_num_results != NULL) {
+*_num_results = state->num_results;
+}
return EOK;
}
@@ -866,7 +870,8 @@ static void cache_req_done(struct tevent_req *subreq)
req = tevent_req_callback_data(subreq, struct tevent_req);
state = tevent_req_data(req, struct cache_req_state);
-ret = cache_req_search_domains_recv(state, subreq, >results);
+ret = cache_req_search_domains_recv(state, subreq,
+>results, >num_results);
talloc_zfree(subreq);
if (ret == ENOENT && state->first_iteration) {
From 842c010e7cf5bb2599fa851ec91ab3e5fd369da2 Mon Sep 17 00:00:00 2001
From: Nikolai Kondrashov <nikolai.kondras...@redhat.com>
Date: Wed, 22 Mar 2017 14:32:35 +0200
Subject: [PATCH 02/17] NSS: Move output name formatting to utils
Move NSS nss_get_name_from_msg and the core of sized_output_name to the
utils to make them available to provider and other responders.
---
src/responder/nss/nss_protocol_grent.c | 3 +-
src/responder/nss/nss_protocol_pwent.c | 2 +-
src/responder/nss/nss_protocol_sid.c | 3 +-
src/responder/nss/nss_utils.c | 55 +--
src/util/sss_nss.c | 68 ++
src/util/sss_nss.h | 12 ++
6 files changed, 94 insertions(+), 49 deletions(-)
diff --git a/src/responder/nss/nss_protocol_grent.c b/src/responder/nss/nss_protocol_grent.c
index 283ab9f..5f208e0 100644
--- a/src/responder/nss/nss_protocol_grent.c
+++ b/src/responder/nss/nss_protocol_grent.c
@@ -19,6 +19,7 @@
*/
#include "responder/nss/nss_protocol.h"
+#include "util/sss_nss.h"
static errno_t
nss_get_grent(TALLOC_CTX *mem_ctx,
@@ -41,7 +42,7 @@ nss_get_grent(TALLOC_CTX *mem_ctx,
}
/* Get fields. */
-name = nss_get_name_from_msg(domain, msg);
+name = sss_nss_get_name_from_msg(domain, msg);
gid = sss_view_ldb_msg_find_attr_as_uint64(domain, msg, SYSDB_GIDNUM, 0);
if (name == NULL || gid == 0) {
diff --git a/src/responder/nss/nss_protocol_pwent.c b/src/responder/nss/nss_protocol_pwent.c
index edda9d3..c0b8e79 100644
--- a/src/responder/nss/nss_protocol_pwent.c
+++ b/src/responder/nss/nss_protocol_pwent.c
@@ -225,7 +225,7 @@ nss_get_pwent(TALLOC_CTX *mem_ctx,
/* Get fields. */
upn = ldb_msg_find_attr_as_string(msg, SYSDB_UPN, NULL);
-name = nss_get_name_from_msg(domain, msg);
+name = sss_nss_get_name_from_msg(domain, msg);
gid = nss_get_gid(domain, msg);
uid = sss_view_ldb_msg_find_attr_as_uint64(domain, msg, SYSDB_UIDNUM, 0);
diff --git a/src/responder/nss/nss_protocol_sid.c b/src/responder/nss/nss_protocol_sid.c
index a6a4e27..37bbec5 100644
--- a/src/responder/nss/nss_protocol_sid.c
+++ b/src/responder/nss/nss_protocol_sid.c
@@ -19,6 +19,7 @@
*/
#include "util/crypto/sss_crypto.h"
+#include "util/sss_nss.h"
#include "responder/nss/nss_protocol.h"
static errno_t
@@ -532,7 +533,7 @@ nss_protocol_fill_name_list(struct nss_ctx *nss_ctx,
return ret;
}
-tmp_str = nss_get_name_from_msg(result->domain, result->msgs[c]);
+tmp_str = sss_nss_get_name_from_msg(result->domain, result->msgs[c]);
if (tmp_str == NULL) {
[SSSD] [sssd PR#136][synchronized] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136
Author: spbnick
Title: #136: Tlog integration
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/136/head:pr136
git checkout pr136
From 871c2b596de3ae5238750ba6bd4a008a40e98138 Mon Sep 17 00:00:00 2001
From: Nikolai Kondrashov <nikolai.kondras...@redhat.com>
Date: Fri, 24 Mar 2017 16:24:22 +0200
Subject: [PATCH 01/14] CACHE_REQ: Propagate num_results to cache_req_state
The num_results field in struct cache_req_state was only set in case of
well-known objects, set it also for the regular results for uniformity,
and for later use by session recording code.
---
src/responder/common/cache_req/cache_req.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/responder/common/cache_req/cache_req.c b/src/responder/common/cache_req/cache_req.c
index aca150d..077282e 100644
--- a/src/responder/common/cache_req/cache_req.c
+++ b/src/responder/common/cache_req/cache_req.c
@@ -536,7 +536,8 @@ static void cache_req_search_domains_done(struct tevent_req *subreq)
static errno_t
cache_req_search_domains_recv(TALLOC_CTX *mem_ctx,
struct tevent_req *req,
- struct cache_req_result ***_results)
+ struct cache_req_result ***_results,
+ size_t *_num_results)
{
struct cache_req_search_domains_state *state;
@@ -547,6 +548,9 @@ cache_req_search_domains_recv(TALLOC_CTX *mem_ctx,
if (_results != NULL) {
*_results = talloc_steal(mem_ctx, state->results);
}
+if (_num_results != NULL) {
+*_num_results = state->num_results;
+}
return EOK;
}
@@ -851,7 +855,8 @@ static void cache_req_done(struct tevent_req *subreq)
req = tevent_req_callback_data(subreq, struct tevent_req);
state = tevent_req_data(req, struct cache_req_state);
-ret = cache_req_search_domains_recv(state, subreq, >results);
+ret = cache_req_search_domains_recv(state, subreq,
+>results, >num_results);
talloc_zfree(subreq);
if (ret == ENOENT && state->first_iteration) {
From 81d03785289ba25615a68a75d74c9d6051e19392 Mon Sep 17 00:00:00 2001
From: Nikolai Kondrashov <nikolai.kondras...@redhat.com>
Date: Wed, 22 Mar 2017 14:32:35 +0200
Subject: [PATCH 02/14] NSS: Move output name formatting to utils
Move NSS nss_get_name_from_msg and the core of sized_output_name to the
utils to make them available to provider and other responders.
---
src/responder/nss/nss_protocol_grent.c | 3 +-
src/responder/nss/nss_protocol_pwent.c | 2 +-
src/responder/nss/nss_protocol_sid.c | 3 +-
src/responder/nss/nss_utils.c | 55 +--
src/util/sss_nss.c | 68 ++
src/util/sss_nss.h | 12 ++
6 files changed, 94 insertions(+), 49 deletions(-)
diff --git a/src/responder/nss/nss_protocol_grent.c b/src/responder/nss/nss_protocol_grent.c
index 283ab9f..5f208e0 100644
--- a/src/responder/nss/nss_protocol_grent.c
+++ b/src/responder/nss/nss_protocol_grent.c
@@ -19,6 +19,7 @@
*/
#include "responder/nss/nss_protocol.h"
+#include "util/sss_nss.h"
static errno_t
nss_get_grent(TALLOC_CTX *mem_ctx,
@@ -41,7 +42,7 @@ nss_get_grent(TALLOC_CTX *mem_ctx,
}
/* Get fields. */
-name = nss_get_name_from_msg(domain, msg);
+name = sss_nss_get_name_from_msg(domain, msg);
gid = sss_view_ldb_msg_find_attr_as_uint64(domain, msg, SYSDB_GIDNUM, 0);
if (name == NULL || gid == 0) {
diff --git a/src/responder/nss/nss_protocol_pwent.c b/src/responder/nss/nss_protocol_pwent.c
index edda9d3..c0b8e79 100644
--- a/src/responder/nss/nss_protocol_pwent.c
+++ b/src/responder/nss/nss_protocol_pwent.c
@@ -225,7 +225,7 @@ nss_get_pwent(TALLOC_CTX *mem_ctx,
/* Get fields. */
upn = ldb_msg_find_attr_as_string(msg, SYSDB_UPN, NULL);
-name = nss_get_name_from_msg(domain, msg);
+name = sss_nss_get_name_from_msg(domain, msg);
gid = nss_get_gid(domain, msg);
uid = sss_view_ldb_msg_find_attr_as_uint64(domain, msg, SYSDB_UIDNUM, 0);
diff --git a/src/responder/nss/nss_protocol_sid.c b/src/responder/nss/nss_protocol_sid.c
index a6a4e27..37bbec5 100644
--- a/src/responder/nss/nss_protocol_sid.c
+++ b/src/responder/nss/nss_protocol_sid.c
@@ -19,6 +19,7 @@
*/
#include "util/crypto/sss_crypto.h"
+#include "util/sss_nss.h"
#include "responder/nss/nss_protocol.h"
static errno_t
@@ -532,7 +533,7 @@ nss_protocol_fill_name_list(struct nss_ctx *nss_ctx,
return ret;
}
-tmp_str = nss_get_name_from_msg(result->domain, result->msgs[c]);
+tmp_str = sss_nss_get_name_from_msg(result->domain, result->msgs[c]);
if (tmp_str == NULL) {
[SSSD] [sssd PR#136][comment] Tlog integration
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration spbnick commented: """ Alright. Aside from `dp_req_initgr_pp` not letting us report a failure, I think this is close to what we need, and is ready for a proper review. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-289480431 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][edited] Tlog integration WIP
URL: https://github.com/SSSD/sssd/pull/136 Author: spbnick Title: #136: Tlog integration WIP Action: edited Changed field: title Original value: """ Tlog integration WIP """ ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration WIP
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration WIP spbnick commented: """ @sumit-bose, @pbrezina, could you please take a look? I don't need a detailed review yet, just feedback on approach. Thanks! """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-289364772 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration WIP
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration WIP spbnick commented: """ Alright, this version loads override_space, and only does initgr request if there are groups to match. I think I'll maybe add checking for SYSDB_INITGR_EXPIRE to avoid firing initgr for entries which haven't expired yet. """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-289055189 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][synchronized] Tlog integration WIP
URL: https://github.com/SSSD/sssd/pull/136
Author: spbnick
Title: #136: Tlog integration WIP
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/136/head:pr136
git checkout pr136
From 32d6411ef99a1a2abd78314cc7131af8ddb69db7 Mon Sep 17 00:00:00 2001
From: Nikolai Kondrashov <nikolai.kondras...@redhat.com>
Date: Fri, 24 Mar 2017 16:24:22 +0200
Subject: [PATCH 01/14] CACHE_REQ: Propagate num_results to cache_req_state
The num_results field in struct cache_req_state was only set in case of
well-known objects, set it also for the regular results for uniformity,
and for later use by session recording code.
---
src/responder/common/cache_req/cache_req.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/responder/common/cache_req/cache_req.c b/src/responder/common/cache_req/cache_req.c
index aca150d..077282e 100644
--- a/src/responder/common/cache_req/cache_req.c
+++ b/src/responder/common/cache_req/cache_req.c
@@ -536,7 +536,8 @@ static void cache_req_search_domains_done(struct tevent_req *subreq)
static errno_t
cache_req_search_domains_recv(TALLOC_CTX *mem_ctx,
struct tevent_req *req,
- struct cache_req_result ***_results)
+ struct cache_req_result ***_results,
+ size_t *_num_results)
{
struct cache_req_search_domains_state *state;
@@ -547,6 +548,9 @@ cache_req_search_domains_recv(TALLOC_CTX *mem_ctx,
if (_results != NULL) {
*_results = talloc_steal(mem_ctx, state->results);
}
+if (_num_results != NULL) {
+*_num_results = state->num_results;
+}
return EOK;
}
@@ -851,7 +855,8 @@ static void cache_req_done(struct tevent_req *subreq)
req = tevent_req_callback_data(subreq, struct tevent_req);
state = tevent_req_data(req, struct cache_req_state);
-ret = cache_req_search_domains_recv(state, subreq, >results);
+ret = cache_req_search_domains_recv(state, subreq,
+>results, >num_results);
talloc_zfree(subreq);
if (ret == ENOENT && state->first_iteration) {
From 6519d96c520aca0bd30c20d9cc40ce4e74f5b7cd Mon Sep 17 00:00:00 2001
From: Nikolai Kondrashov <nikolai.kondras...@redhat.com>
Date: Wed, 22 Mar 2017 14:32:35 +0200
Subject: [PATCH 02/14] NSS: Move output name formatting to utils
Move NSS nss_get_name_from_msg and the core of sized_output_name to the
utils to make them available to provider and other responders.
---
src/responder/nss/nss_protocol_grent.c | 3 +-
src/responder/nss/nss_protocol_pwent.c | 2 +-
src/responder/nss/nss_utils.c | 55 +--
src/util/sss_nss.c | 68 ++
src/util/sss_nss.h | 10 +
5 files changed, 90 insertions(+), 48 deletions(-)
diff --git a/src/responder/nss/nss_protocol_grent.c b/src/responder/nss/nss_protocol_grent.c
index 283ab9f..5f208e0 100644
--- a/src/responder/nss/nss_protocol_grent.c
+++ b/src/responder/nss/nss_protocol_grent.c
@@ -19,6 +19,7 @@
*/
#include "responder/nss/nss_protocol.h"
+#include "util/sss_nss.h"
static errno_t
nss_get_grent(TALLOC_CTX *mem_ctx,
@@ -41,7 +42,7 @@ nss_get_grent(TALLOC_CTX *mem_ctx,
}
/* Get fields. */
-name = nss_get_name_from_msg(domain, msg);
+name = sss_nss_get_name_from_msg(domain, msg);
gid = sss_view_ldb_msg_find_attr_as_uint64(domain, msg, SYSDB_GIDNUM, 0);
if (name == NULL || gid == 0) {
diff --git a/src/responder/nss/nss_protocol_pwent.c b/src/responder/nss/nss_protocol_pwent.c
index edda9d3..c0b8e79 100644
--- a/src/responder/nss/nss_protocol_pwent.c
+++ b/src/responder/nss/nss_protocol_pwent.c
@@ -225,7 +225,7 @@ nss_get_pwent(TALLOC_CTX *mem_ctx,
/* Get fields. */
upn = ldb_msg_find_attr_as_string(msg, SYSDB_UPN, NULL);
-name = nss_get_name_from_msg(domain, msg);
+name = sss_nss_get_name_from_msg(domain, msg);
gid = nss_get_gid(domain, msg);
uid = sss_view_ldb_msg_find_attr_as_uint64(domain, msg, SYSDB_UIDNUM, 0);
diff --git a/src/responder/nss/nss_utils.c b/src/responder/nss/nss_utils.c
index f839930..42fe33e 100644
--- a/src/responder/nss/nss_utils.c
+++ b/src/responder/nss/nss_utils.c
@@ -22,37 +22,11 @@
#include
#include "util/util.h"
+#include "util/sss_nss.h"
#include "confdb/confdb.h"
#include "responder/common/responder.h"
#include "responder/nss/nss_private.h"
-const char *
-nss_get_name_from_msg(struct sss_domain_info *domain,
- struct ldb_message *msg)
-{
-const char *name;
-
-/* If domain has a view associated we return overridden name
- * if possible. */
-if (DOM_HAS_VIEWS(domain)) {
-name = ldb_msg_find_attr_as_string(m
[SSSD] [sssd PR#136][comment] Tlog integration WIP
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration WIP spbnick commented: """ One thing bothering me is that `dp_req_initgr_pp` doesn't seem well suited for what we're doing. I.e. nothing cares if it fails. Can we do something about it? Change the interface, put that code somewhere else? """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-288413469 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][comment] Tlog integration WIP
URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration WIP spbnick commented: """ I've pushed a draft rewrite using cache_req and data provider. The functionality is very basic and is mostly a proof-of-concept with no intent to be efficient. For each entry returned for a request for user information in cache_req, it fires off an initgr request. On the data provider side, that initgr request is post-processed to include a "sessionRecording" attribute, if selective session recording is enabled. That attribute specifies if the user name, or names of the groups it's member of, match any of the user or group names in the session recording configuration. Back in cache_req, that attribute is copied over the returned entry. Once the entries get to NSS, if unconditional session recording is enabled (scope = all), or if selective session recording is enabled (scope = some) and the entry has sessionRecording attribute set to true, the user shell is replaced with the session recording shell. Things still to do: * retrieve and use override_space instead of hardcoding * make sure initgr is fired only if there are groups to match against and SYSDB_INITGR_EXPIRE has expired * things I missed (please tell!) Regarding the second item, isn't cache_req already ensuring that initgr request is only sent when SYSDB_INITGR_EXPIRE is sent, so we don't have to do anything about that? I'd be glad to hear @pbrezina and @sumit-bose opinions on this so far. Thanks! """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-288411475 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][synchronized] Tlog integration WIP
URL: https://github.com/SSSD/sssd/pull/136
Author: spbnick
Title: #136: Tlog integration WIP
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/136/head:pr136
git checkout pr136
From 462f3f323f3ef9aa2efcb08a6a6ba8a3c58b9a3b Mon Sep 17 00:00:00 2001
From: Nikolai Kondrashov <nikolai.kondras...@redhat.com>
Date: Wed, 22 Mar 2017 14:32:35 +0200
Subject: [PATCH 01/11] NSS: Move output name formatting to utils
Move NSS nss_get_name_from_msg and the core of sized_output_name to the
utils to make them available to provider and other responders.
---
src/responder/nss/nss_protocol_grent.c | 3 +-
src/responder/nss/nss_protocol_pwent.c | 2 +-
src/responder/nss/nss_utils.c | 55 +--
src/util/sss_nss.c | 68 ++
src/util/sss_nss.h | 10 +
5 files changed, 90 insertions(+), 48 deletions(-)
diff --git a/src/responder/nss/nss_protocol_grent.c b/src/responder/nss/nss_protocol_grent.c
index 283ab9f..5f208e0 100644
--- a/src/responder/nss/nss_protocol_grent.c
+++ b/src/responder/nss/nss_protocol_grent.c
@@ -19,6 +19,7 @@
*/
#include "responder/nss/nss_protocol.h"
+#include "util/sss_nss.h"
static errno_t
nss_get_grent(TALLOC_CTX *mem_ctx,
@@ -41,7 +42,7 @@ nss_get_grent(TALLOC_CTX *mem_ctx,
}
/* Get fields. */
-name = nss_get_name_from_msg(domain, msg);
+name = sss_nss_get_name_from_msg(domain, msg);
gid = sss_view_ldb_msg_find_attr_as_uint64(domain, msg, SYSDB_GIDNUM, 0);
if (name == NULL || gid == 0) {
diff --git a/src/responder/nss/nss_protocol_pwent.c b/src/responder/nss/nss_protocol_pwent.c
index edda9d3..c0b8e79 100644
--- a/src/responder/nss/nss_protocol_pwent.c
+++ b/src/responder/nss/nss_protocol_pwent.c
@@ -225,7 +225,7 @@ nss_get_pwent(TALLOC_CTX *mem_ctx,
/* Get fields. */
upn = ldb_msg_find_attr_as_string(msg, SYSDB_UPN, NULL);
-name = nss_get_name_from_msg(domain, msg);
+name = sss_nss_get_name_from_msg(domain, msg);
gid = nss_get_gid(domain, msg);
uid = sss_view_ldb_msg_find_attr_as_uint64(domain, msg, SYSDB_UIDNUM, 0);
diff --git a/src/responder/nss/nss_utils.c b/src/responder/nss/nss_utils.c
index f839930..42fe33e 100644
--- a/src/responder/nss/nss_utils.c
+++ b/src/responder/nss/nss_utils.c
@@ -22,37 +22,11 @@
#include
#include "util/util.h"
+#include "util/sss_nss.h"
#include "confdb/confdb.h"
#include "responder/common/responder.h"
#include "responder/nss/nss_private.h"
-const char *
-nss_get_name_from_msg(struct sss_domain_info *domain,
- struct ldb_message *msg)
-{
-const char *name;
-
-/* If domain has a view associated we return overridden name
- * if possible. */
-if (DOM_HAS_VIEWS(domain)) {
-name = ldb_msg_find_attr_as_string(msg, OVERRIDE_PREFIX SYSDB_NAME,
- NULL);
-if (name != NULL) {
-return name;
-}
-}
-
-/* Otherwise we try to return name override from
- * Default Truest View for trusted users. */
-name = ldb_msg_find_attr_as_string(msg, SYSDB_DEFAULT_OVERRIDE_NAME, NULL);
-if (name != NULL) {
-return name;
-}
-
-/* If no override is found we return the original name. */
-return ldb_msg_find_attr_as_string(msg, SYSDB_NAME, NULL);
-}
-
int sized_output_name(TALLOC_CTX *mem_ctx,
struct resp_ctx *rctx,
const char *orig_name,
@@ -61,7 +35,7 @@ int sized_output_name(TALLOC_CTX *mem_ctx,
{
TALLOC_CTX *tmp_ctx = NULL;
errno_t ret;
-char *username;
+char *name_str;
struct sized_string *name;
tmp_ctx = talloc_new(NULL);
@@ -69,30 +43,19 @@ int sized_output_name(TALLOC_CTX *mem_ctx,
return ENOMEM;
}
-username = sss_output_name(tmp_ctx, orig_name, name_dom->case_preserve,
- rctx->override_space);
-if (username == NULL) {
-ret = EIO;
-goto done;
-}
-
-if (name_dom->fqnames) {
-username = sss_tc_fqname(tmp_ctx, name_dom->names, name_dom, username);
-if (username == NULL) {
-DEBUG(SSSDBG_CRIT_FAILURE, "sss_replace_space failed\n");
-ret = EIO;
-goto done;
-}
-}
-
name = talloc_zero(tmp_ctx, struct sized_string);
if (name == NULL) {
ret = ENOMEM;
goto done;
}
-to_sized_string(name, username);
-name->str = talloc_steal(name, username);
+ret = sss_nss_output_name(mem_ctx, name_dom, orig_name,
+ rctx->override_space, _str);
+if (ret != EOK) {
+goto done;
+}
+
+to_sized_string(name, name_str);
*_name = talloc_steal(mem_ctx, name);
ret = EOK;
done:
diff --git a/src/u
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups spbnick commented: """ All my comments were addressed, ACK on my points. Thanks, @fidencio! """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-282267591 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups spbnick commented: """ @fidencio Sorry, I was able to understand very little from the comment, so I can't really suggest a better commit message. """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-281073850 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#141][comment] PAM: Use cache_req to perform initgroups lookups
URL: https://github.com/SSSD/sssd/pull/141 Title: #141: PAM: Use cache_req to perform initgroups lookups spbnick commented: """ I'll try to review this. """ See the full comment at https://github.com/SSSD/sssd/pull/141#issuecomment-279670819 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#136][opened] Tlog integration WIP
URL: https://github.com/SSSD/sssd/pull/136
Author: spbnick
Title: #136: Tlog integration WIP
Action: opened
PR body:
"""
@lslebodn, @pbrezina, this is the work-in-progress tlog integration patchset
I'd like to work on with you.
This is not for merging as it is. We can go over it when we meet :)
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/136/head:pr136
git checkout pr136
From 50a5ef8822e8573136f3e1a4841adc6f4a4fa566 Mon Sep 17 00:00:00 2001
From: Nikolai Kondrashov <nikolai.kondras...@redhat.com>
Date: Tue, 20 Dec 2016 10:16:47 +0200
Subject: [PATCH 01/11] config: Add session_recording section
Add information on "session_recording" config section, having three
options: "scope", "users", and "groups".
The section is intended for disabling session recording ("scope = none",
default), enabling session recording for all users ("scope = all"), and
enabling it for some specific users and/or groups ("scope = some",
"users = ", "groups = ").
---
src/confdb/confdb.h | 6 ++
src/config/SSSDConfigTest.py | 6 --
src/config/etc/sssd.api.conf | 6 ++
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 9055048..0794451 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -159,6 +159,12 @@
#define CONFDB_IFP_USER_ATTR_LIST "user_attributes"
#define CONFDB_IFP_WILDCARD_LIMIT "wildcard_limit"
+/* Session Recording */
+#define CONFDB_SESSION_RECORDING_CONF_ENTRY "config/session_recording"
+#define CONFDB_SESSION_RECORDING_SCOPE "scope"
+#define CONFDB_SESSION_RECORDING_USERS "users"
+#define CONFDB_SESSION_RECORDING_GROUPS "groups"
+
/* Domains */
#define CONFDB_DOMAIN_PATH_TMPL "config/domain/%s"
#define CONFDB_DOMAIN_BASEDN "cn=domain,cn=config"
diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py
index 0da5d63..c4dc65a 100755
--- a/src/config/SSSDConfigTest.py
+++ b/src/config/SSSDConfigTest.py
@@ -1354,7 +1354,8 @@ def testNewConfig(self):
'ssh',
'pac',
'ifp',
-'secrets']
+'secrets',
+'session_recording']
for section in control_list:
self.assertTrue(sssdconfig.has_section(section),
"Section [%s] missing" %
@@ -1448,7 +1449,8 @@ def testListServices(self):
'ssh',
'pac',
'ifp',
-'secrets']
+'secrets',
+'session_recording']
service_list = sssdconfig.list_services()
for service in control_list:
self.assertTrue(service in service_list,
diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf
index 5654006..7fea58c 100644
--- a/src/config/etc/sssd.api.conf
+++ b/src/config/etc/sssd.api.conf
@@ -111,6 +111,12 @@ forward_headers = list, None, false
username = str, None, false
password = str, None, false
+[session_recording]
+# Session recording service
+scope = str, None, false
+users = list, str, false
+groups = list, str, false
+
[provider]
#Available provider types
id_provider = str, None, true
From 08a412dcdd332311cd59e6138563f088de86d7f8 Mon Sep 17 00:00:00 2001
From: Nikolai Kondrashov <nikolai.kondras...@redhat.com>
Date: Thu, 11 Aug 2016 14:15:55 +0300
Subject: [PATCH 02/11] BUILD: Support configuring session recording shell
Add support for specifying the shell used for recording user sessions,
at configure time.
---
configure.ac | 1 +
src/conf_macros.m4 | 16
2 files changed, 17 insertions(+)
diff --git a/configure.ac b/configure.ac
index 2915046..b9ed9e6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -139,6 +139,7 @@ WITH_SEMANAGE
WITH_AD_GPO_DEFAULT
WITH_GPO_CACHE_PATH
WITH_NOLOGIN_SHELL
+WITH_SESSION_RECORDING_SHELL
WITH_APP_LIBS
WITH_SUDO
WITH_SUDO_LIB_PATH
diff --git a/src/conf_macros.m4 b/src/conf_macros.m4
index 427b0e0..5422a57 100644
--- a/src/conf_macros.m4
+++ b/src/conf_macros.m4
@@ -592,6 +592,22 @@ AC_DEFUN([WITH_NOLOGIN_SHELL],
AC_DEFINE_UNQUOTED(NOLOGIN_SHELL, "$nologin_shell", [The shell used to deny access to users])
])
+AC_DEFUN([WITH_SESSION_RECORDING_SHELL],
+ [ AC_ARG_WITH([session-recording-shell],
+[AC_HELP_STRING([--with-session-recording-shell=PATH],
+[The shell used to record user sessions [/usr/bin/tlog-rec]]
+ )
+]
+ )
+session_recording_shell="/usr/bin/tlog-rec"
+if test x"$with_session_recording_shell" != x; then
+nologin_shell=$with_session_recording_shell
+fi
+AC_SUBST(session_recording_shell)
+AC_D
[SSSD] [sssd PR#89][comment] nss: rewrite nss responder so it uses cache_req
URL: https://github.com/SSSD/sssd/pull/89 Title: #89: nss: rewrite nss responder so it uses cache_req spbnick commented: """ Another question is cache_req going to be used by PAM (if this question even makes sense)? """ See the full comment at https://github.com/SSSD/sssd/pull/89#issuecomment-268206740 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#89][comment] nss: rewrite nss responder so it uses cache_req
URL: https://github.com/SSSD/sssd/pull/89 Title: #89: nss: rewrite nss responder so it uses cache_req spbnick commented: """ Not a review, but rather a question to @pbrezina: why do some "plugin" functions receive both cache_req_data and cache_req, which also references cache_req_data, at the same time? Are those referring to the same cache_req_data? """ See the full comment at https://github.com/SSSD/sssd/pull/89#issuecomment-268204367 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#89][comment] nss: rewrite nss responder so it uses cache_req
URL: https://github.com/SSSD/sssd/pull/89 Title: #89: nss: rewrite nss responder so it uses cache_req spbnick commented: """ In general it is good to submit your patchset to CI with the tag "each", to have each patch tested (instead of only the last one), and catch this kind of thing. """ See the full comment at https://github.com/SSSD/sssd/pull/89#issuecomment-262908628 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#83][comment] TESTS: Check new line at end of file
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file spbnick commented: """ Ah, I see. Then you can put your patterns into a variable and check against them in the loop, similarly to the way it's done above in the script. You can use extended globs (with `shopt -s extglob`), or regexes as before. For globs the test will be if `[[ "$file" != $EXCLUDE_GLOB ]]`, and for regexes it will be `! [[ "$file" =~ $EXCLUDE_REGEX ]]`. """ See the full comment at https://github.com/SSSD/sssd/pull/83#issuecomment-261934231 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#83][comment] TESTS: Check new line at end of file
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file spbnick commented: """ Ah, I see. Then you can put your patterns into a variable and check against them in the loop, similarly to the way it's done above in the script. You can use extended globs (with `shopt -s extglob`), or regexes as before. For globs the test will be if `[[ "$file" != $EXCLUDE_GLOB ]]`, and for regexes it will be `! [[ "$file" ~= $EXCLUDE_REGEX ]]`. """ See the full comment at https://github.com/SSSD/sssd/pull/83#issuecomment-261934231 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#83][comment] TESTS: Check new line at end of file
URL: https://github.com/SSSD/sssd/pull/83 Title: #83: TESTS: Check new line at end of file spbnick commented: """ @lslebodn I left one suggestion, if that's not what you needed, could you please specify in which way it should be "better"? """ See the full comment at https://github.com/SSSD/sssd/pull/83#issuecomment-261919223 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#52][comment] CI: Remove dlopen-test from valgrind blacklist
URL: https://github.com/SSSD/sssd/pull/52 Title: #52: CI: Remove dlopen-test from valgrind blacklist spbnick commented: """ Also, if we stumble into something we can't fix again, we can always mask the test again. """ See the full comment at https://github.com/SSSD/sssd/pull/52#issuecomment-254754136 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#18] [PATCHES] sss_user/groupmod fixes (comment)
spbnick commented on a pull request """ Just a hint: if you avoid putting "PR" in front of the PR#16, then you'll get a link to the actual pull request on the GitHub page, plus the target pull request will have a link back. Like this: #16. """ See the full comment at https://github.com/SSSD/sssd/pull/18#issuecomment-245302411 ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
