[sssd PR#241][opened] FleetCommander Integration

[sssd PR#347][opened] Fixes...

[sssd PR#211][opened] IFP: Fix of...

fidencio

Thursday, 20 April 2017 Thu, 20 Apr '17

10:36 a.m.

URL: https://github.com/SSSD/sssd/pull/241 Author: fidencio Title: #241: FleetCommander Integration Action: opened PR body: """ This patch series contains: - some refactoring done on access module (and, consequently, on HBAC) in order to reuse a some code in the session module (an, consequently, on FleetCommander code); - the new session module that provides the FleetCommander Integration; """ To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/241/head:pr241 git checkout pr241

Attachments:

sssd-pr-241.patch (text/x-diff — 163.8 KB)

Show replies by thread

fidencio

Friday, 9 June Fri, 9 Jun

9:07 a.m.

New subject: [sssd PR#241][synchronized] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Author: fidencio Title: #241: FleetCommander Integration Action: synchronized To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/241/head:pr241 git checkout pr241

fidencio

Monday, 10 July Mon, 10 Jul

1:34 a.m.

New subject: [sssd PR#241][synchronized] FleetCommander Integration

fidencio

1:39 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration fidencio commented: """ I've updated the patch set accordingly to https://pagure.io/SSSD/sssd/issue/2995#comment-447913 """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-314018566

fidencio

3:12 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration fidencio commented: """ CI: http://sssd-ci.idm.lab.eng.brq.redhat.com:8080/job/ci/7173/ Failures do not seem to be related to this patch set. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-314036112

fidencio

Tuesday, 11 July Tue, 11 Jul

7:55 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration fidencio commented: """ Here [0] you can find some step-by-step on how to test the functionality of those patches. It's important to mention that @olivergs has been doing some functional tests on those patches, using the copr repo provided by [0]. [0]: https://copr.fedorainfracloud.org/coprs/fidencio/deskprofile/ """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-314434411

fidencio

9:42 a.m.

New subject: [sssd PR#241][synchronized] FleetCommander Integration

fidencio

9:44 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration fidencio commented: """ I've updated the patch set and the changes are: - as @jhrozek agreed on the approach proposed by FleetCommander guys, the 2 "fixup" patches got squashed into "DESKPROFILE: Introduce the new IPA session provider"; - updated the link to thefor design page; """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-314467404

fidencio

Wednesday, 26 July Wed, 26 Jul

4:43 p.m.

New subject: [sssd PR#241][synchronized] FleetCommander Integration

jhrozek

Tuesday, 1 August Tue, 1 Aug

4:16 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ There are some issues Coverity found: ``` Error: TOCTOU (CWE-367): sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:157: fs_check_call: Calling function "stat" to perform check on "domain_dir". sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:159: toctou: Calling function "mkdir" that uses "domain_dir" after a check function. This can cause a time-of-check, time-of-use race condition. # 157| ret = stat(domain_dir, &info); # 158| if (ret != EOK) { # 159|-> ret = mkdir(domain_dir, 0755); # 160| if (ret != EOK) { # 161| ret = errno; Error: TOCTOU (CWE-367): sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:214: fs_check_call: Calling function "stat" to perform check on "user_dir". sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:216: toctou: Calling function "mkdir" that uses "user_dir" after a check function. This can cause a time-of-check, time-of-use race condition. # 214| ret = stat(user_dir, &info); # 215| if (ret != EOK) { # 216|-> ret = mkdir(user_dir, 0600); # 217| if (ret != EOK) { # 218| ret = errno; Error: TOCTOU (CWE-367): sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:227: fs_check_call: Calling function "stat" to perform check on "user_dir". sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:247: toctou: Calling function "chown" that uses "user_dir" after a check function. This can cause a time-of-check, time-of-use race condition. # 245| } # 246| # 247|-> ret = chown(user_dir, uid, gid); # 248| if (ret != EOK) { # 249| ret = errno; Error: TOCTOU (CWE-367): sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:687: fs_check_call: Calling function "stat" to perform check on "filepath". sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:702: toctou: Calling function "remove" that uses "filepath" after a check function. This can cause a time-of-check, time-of-use race condition. # 700| } # 701| # 702|-> ret = remove(filepath); # 703| if (ret != EOK) { # 704| ret = errno; Error: RESOURCE_LEAK (CWE-772): sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:659: alloc_fn: Storage is returned from allocation function "opendir". sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:659: var_assign: Assigning: "dir" = storage returned from "opendir(user_dir)". sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:680: noescape: Resource "dir" is not freed or pointed-to in "readdir". sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:725: leaked_storage: Variable "dir" going out of scope leaks the storage it points to. # 723| done: # 724| talloc_free(tmp_ctx); # 725|-> return ret; # 726| } # 727| Error: UNINIT (CWE-457): sssd-1.15.4/src/responder/common/cache_req/cache_req_sr_overlay.c:122: var_decl: Declaring variable "ret" without initializer. sssd-1.15.4/src/responder/common/cache_req/cache_req_sr_overlay.c:209: uninit_use: Using uninitialized value "ret". # 207| done: # 208| talloc_zfree(tmp_ctx); # 209|-> return ret; # 210| } # 211| ``` """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-319315814

jhrozek

4:18 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ In general, to get rid of the TOCTOU issues, the key is using the `*at` and `*fd` family of functions - you might open the top level directory to get a fd and then instead of `stat` call `fstat`. You can see how we deal with that in `src/tools/files.c`. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-319316366

jhrozek

5:20 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ With an old server that has no support for the desktop profiles, I'm getting: ``` (Tue Aug 1 10:19:17 2017) [sssd[be[ipa.test]]] [ipa_deskprofile_get_config_done] (0x0040): Unexpected number of results, expected 1, got 0. (Tue Aug 1 10:19:17 2017) [sssd[be[ipa.test]]] [ipa_pam_session_handler_done] (0x0020): Unable to fetch Desktop Profile rules [22]: Invalid argument ``` """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-319330842

jhrozek

5:21 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ sorry, I hit comment too soon. I think we want to handle that more gracefully and not throw an error. By the way, I'm still not finished with the review of the last patch (the previous ones LGTM), but because I'm running out of time today, I'm already adding the "Changes Requested" keyword. I will continue my review in the evening. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-319331120

jhrozek

5:21 a.m.

New subject: [sssd PR#241][+Changes requested] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration Label: +Changes requested

fidencio

9:53 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration fidencio commented: """ @jhrozek, about the issue when connecting to old server that has no support for the desktop profiles ... In that case we'd always get a reply could as 0. Would be okay, for you, to just return ENOENT in those cases? ipa_pam_session_handler_done() handles well ENOENT :-) AI: Still have to fix the TUCTOU issues. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-319394677

jhrozek

1:03 p.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ I think the error condition is not really as much of a code issue as a user-friendliness issue. Admins of older IPA servers would suddendly see an odd message saying that something failed. I think returning ENOENT or a special code is both fine as long as there is just an informational message that the server doesn't support desktop profiles. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-319449557

fidencio

Monday, 7 August Mon, 7 Aug

8:48 a.m.

New subject: [sssd PR#241][synchronized] FleetCommander Integration

fidencio

8:49 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration fidencio commented: """ @jhrozek, a new version of the patchset has been pushed and hopefully all the comments have been addressed. A few more patches were introduced in this new patchset. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-320668549

fidencio

8:49 a.m.

New subject: [sssd PR#241][-Changes requested] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration Label: -Changes requested

jhrozek

Monday, 14 August Mon, 14 Aug

11:03 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ There are still some Coverity warnings: ``` Error: NEGATIVE_RETURNS (CWE-394): sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:232: negative_return_fn: Function "sss_create_dir("/var/lib/sss/deskprofile", domain, 493U, getuid(), getgid())" returns a negative number. sssd-1.15.4/src/util/files.c:843:5: var_tested_neg: Variable "ret" is negative. sssd-1.15.4/src/util/files.c:878:5: return_negative_variable: Explicitly returning negative variable "ret". sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:232: var_assign: Assigning: signed variable "ret" = "sss_create_dir". sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:235: negative_returns: "ret" is passed to a parameter that cannot be negative. sssd-1.15.4/src/util/util_errors.c:128:5: neg_sink_parm_call: Passing "error" to "strerror", which cannot accept a negative number. # 233| getuid(), getgid()); # 234| if (ret != EOK) { # 235|-> DEBUG(SSSDBG_TRACE_FUNC, # 236| "Failed to create the directory \"%s/%s\" that would be used to " # 237| "store the Desktop Profile rules users' directory [%d]: %s\n", Error: NEGATIVE_RETURNS (CWE-394): sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:250: negative_return_fn: Function "sss_create_dir(domain_dir, shortname, 384U, uid, gid)" returns a negative number. sssd-1.15.4/src/util/files.c:843:5: var_tested_neg: Variable "ret" is negative. sssd-1.15.4/src/util/files.c:878:5: return_negative_variable: Explicitly returning negative variable "ret". sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:250: var_assign: Assigning: signed variable "ret" = "sss_create_dir". sssd-1.15.4/src/providers/ipa/ipa_deskprofile_rules_util.c:252: negative_returns: "ret" is passed to a parameter that cannot be negative. sssd-1.15.4/src/util/util_errors.c:128:5: neg_sink_parm_call: Passing "error" to "strerror", which cannot accept a negative number. # 250| ret = sss_create_dir(domain_dir, shortname, 0600, uid, gid); # 251| if (ret != EOK) { # 252|-> DEBUG(SSSDBG_TRACE_FUNC, # 253| "Failed to create the directory \"%s/%s/%s\" that would be used " # 254| "to store the Desktop Profile rules for the user \"%s\" [%d]: " ``` """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-322232061

jhrozek

11:07 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ I think to fix the warnings, you just need to `errno=ret` before printing the DEBUG message """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-322233270

jhrozek

11:07 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ retest this please """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-322233302

fidencio

1:10 p.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration fidencio commented: """ @jhrozek, about the coverity warnings, I guess the safest path to take is just use int instead of errno_t in that function where the warning happend (and, of course, change the whole chain ...) It's still not fixed locally. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-322265238

fidencio

1:36 p.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration fidencio commented: """ Hmm. That's actually a bad idea and I'm struggling to see a better solution for this. Suggestions are welcome. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-322272067

jhrozek

2:42 p.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ Well, I'm struggling to see how would `sss_create_dir` return -1. All control paths either return a directly assigned return value (ENOMEM) or errno. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-322288785

jhrozek

Tuesday, 15 August Tue, 15 Aug

6:12 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ Since most of the comments were minor issues with little impact on functionality, I went ahead and scheduled regression runs. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-322440673

jhrozek

6:12 a.m.

New subject: [sssd PR#241][+Changes requested] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration Label: +Changes requested

jhrozek

2:40 p.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ btw the downstream regression test passed (job ID 2005455) """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-322566071

fidencio

Thursday, 17 August Thu, 17 Aug

4:11 a.m.

New subject: [sssd PR#241][synchronized] FleetCommander Integration

fidencio

4:16 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration fidencio commented: """ I have updated the patch set and, hopefully, all your comments have been addressed. For a few comments I ended up taking the path I believe is the right one. Let me know whether you need some changes and those will be done according to your answers for the questions I've raised as answers for your comments. This version of the patches includes support for https://pagure.io/SSSD/sssd/issue/3449 and https://github.com/abbra/freeipa-desktop-profile/issues/3. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323014987

fidencio

4:16 a.m.

New subject: [sssd PR#241][-Changes requested] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration Label: -Changes requested

jhrozek

6:25 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ Thank you for the updates. I think only the `permuts` dereference should be changed to happen after the check. In the meantime, I've started CI, Coverity and downstream HBAC tests. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323043057

jhrozek

6:25 a.m.

New subject: [sssd PR#241][+Changes requested] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration Label: +Changes requested

jhrozek

8:45 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ On Thu, Aug 17, 2017 at 04:25:07AM -0700, Jakub Hrozek wrote:

...

Thank you for the updates. I think only the `permuts` dereference should be changed to happen after the check. In the meantime, I've started CI, Coverity and downstream HBAC tests.

All passed by the way. So once the one-liner with the array index check is addressed, I'll be happy about the patches. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323077931

fidencio

10:11 a.m.

New subject: [sssd PR#241][synchronized] FleetCommander Integration

fidencio

10:12 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

fidencio

10:21 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration fidencio commented: """ Patch set updated. This is the patch that I've squashed: ``` From 97f189d3a4fcbbfe453cc6ba8eab8710ca31c4ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio(a)redhat.com> Date: Thu, 17 Aug 2017 13:29:24 +0200 Subject: [PATCH] fixup! DESKPROFILE: Introduce the new IPA session provider --- src/providers/ipa/ipa_deskprofile_rules_util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/providers/ipa/ipa_deskprofile_rules_util.c b/src/providers/ipa/ipa_deskprofile_rules_util.c index 12c6492ba..2ece5a1ee 100644 --- a/src/providers/ipa/ipa_deskprofile_rules_util.c +++ b/src/providers/ipa/ipa_deskprofile_rules_util.c @@ -150,7 +150,7 @@ ipa_deskprofile_get_filename_path(TALLOC_CTX *mem_ctx, extension, NULL, }; - const uint8_t *perms = permuts[config_priority - 1]; + const uint8_t *perms; char *result; errno_t ret; @@ -167,6 +167,8 @@ ipa_deskprofile_get_filename_path(TALLOC_CTX *mem_ctx, goto done; } + perms = permuts[config_priority - 1]; + result = talloc_strdup(tmp_ctx, ""); if (result == NULL) { ret = ENOMEM; -- 2.13.4 ``` and also ... ``` From 8cc50e9b63d6610e7d5c52f57f8238a63f182f7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio(a)redhat.com> Date: Thu, 17 Aug 2017 17:20:35 +0200 Subject: [PATCH] fixup! DESKPROFILE: Introduce the new IPA session provider --- src/providers/ipa/ipa_deskprofile_rules_util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/providers/ipa/ipa_deskprofile_rules_util.c b/src/providers/ipa/ipa_deskprofile_rules_util.c index 2ece5a1ee..1f5b7f9c5 100644 --- a/src/providers/ipa/ipa_deskprofile_rules_util.c +++ b/src/providers/ipa/ipa_deskprofile_rules_util.c @@ -26,6 +26,7 @@ #include <ctype.h> #include <fcntl.h> +#define DESKPROFILE_GLOBAL_POLICY_MIN_VALUE 1 #define DESKPROFILE_GLOBAL_POLICY_MAX_VALUE 24 enum deskprofile_name { @@ -159,7 +160,8 @@ ipa_deskprofile_get_filename_path(TALLOC_CTX *mem_ctx, return ENOMEM; } - if (config_priority > DESKPROFILE_GLOBAL_POLICY_MAX_VALUE) { + if (config_priority < DESKPROFILE_GLOBAL_POLICY_MIN_VALUE || + config_priority > DESKPROFILE_GLOBAL_POLICY_MAX_VALUE) { DEBUG(SSSDBG_CRIT_FAILURE, "The configuration priority has an invalid value: %d!\n", config_priority); -- 2.13.4 ``` """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323102642

fidencio

10:22 a.m.

New subject: [sssd PR#241][synchronized] FleetCommander Integration

fidencio

10:25 a.m.

New subject: [sssd PR#241][-Changes requested] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration Label: -Changes requested

jhrozek

10:44 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ Thank you, code-wise ACK. I'm not going to add the accepted label so that the patches don't get pushed by accident until we hear from the desktop group about results of their testing. Thank you for accomodating all my comments, looking forward to merging the patches! """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323112506

fidencio

5:13 p.m.

New subject: [sssd PR#241][synchronized] FleetCommander Integration

fidencio

5:15 p.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration fidencio commented: """ @jhrozek, I've updated the patch set including **DESKPROFILE: Add ipa_deskprofile_request_interval**, which fix for https://pagure.io/SSSD/sssd/issue/3482 """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323209189

fidencio

5:17 p.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration fidencio commented: """ @jhrozek, I've updated the patch set including **DESKPROFILE: Add ipa_deskprofile_request_interval**, which is a fix for https://pagure.io/SSSD/sssd/issue/3482 """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323209189

fidencio

5:28 p.m.

New subject: [sssd PR#241][synchronized] FleetCommander Integration

pbrezina

Friday, 18 August Fri, 18 Aug

4:26 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration pbrezina commented: """ I just briefly read the code, haven't tried it. ```c @@ -126,10 +126,9 @@ static void choose_target(struct data_provider *provider, name = "PAM Chpass 2nd"; break; case SSS_PAM_OPEN_SESSION: - target = DP_TARGET_SENTINEL; - method = DP_METHOD_SENTINEL; + target = DPT_SESSION; + method = DPM_SESSION_HANDLER; name = "PAM Open Session"; - pd->pam_status = PAM_SUCCESS; break; ... /* Check that target is configured. */ if (target != DP_TARGET_SENTINEL && !dp_target_enabled(provider, NULL, target)) { target = DP_TARGET_SENTINEL; method = DP_METHOD_SENTINEL; pd->pam_status = PAM_MODULE_UNKNOWN; } ``` What happens here if session provider is not set? With this patch we return `PAM_MODULE_UNKNOWN` but it was `PAM_SUCCESS` before. Doesn't this cause any kind of problem? """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323305584

jhrozek

5:23 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ On Fri, Aug 18, 2017 at 09:26:14AM +0000, Pavel Březina wrote:

...

I just briefly read the code, haven't tried it. ```c @@ -126,10 +126,9 @@ static void choose_target(struct data_provider *provider, name = "PAM Chpass 2nd"; break; case SSS_PAM_OPEN_SESSION: - target = DP_TARGET_SENTINEL; - method = DP_METHOD_SENTINEL; + target = DPT_SESSION; + method = DPM_SESSION_HANDLER; name = "PAM Open Session"; - pd->pam_status = PAM_SUCCESS; break; ... /* Check that target is configured. */ if (target != DP_TARGET_SENTINEL && !dp_target_enabled(provider, NULL, target)) { target = DP_TARGET_SENTINEL; method = DP_METHOD_SENTINEL; pd->pam_status = PAM_MODULE_UNKNOWN; } ``` What happens here if session provider is not set? With this patch we return `PAM_MODULE_UNKNOWN` but it was `PAM_SUCCESS` before. Doesn't this cause any kind of problem?

I don't follow. The IPA session provider is enabled by default (all providers except access are derived from the id_provider). Setting session_provider = none gives me: ``` (Fri Aug 18 10:21:17 2017) [sssd[be[ipa.test]]] [dp_req_reply_gen_error] (0x0080): DP Request [PAM Open Session #13]: Finished. Target is not supported with this configuration. pam_dp_send_req returned 0 (Fri Aug 18 10:21:17 2017) [sssd[pam]] [pam_dp_process_reply] (0x0010): Reply error. (Fri Aug 18 10:21:17 2017) [sssd[pam]] [filter_responses] (0x0100): [pam_response_filter] not available, not fatal. ``` Which, except the reply error is also expected, right? """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323317998

fidencio

5:28 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration fidencio commented: """ @jhrozek: @pbrezina told me (on #sssd) to test with `ipa_enable_deskprofile = false`. Here's the result: ``` (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_pam_handler] (0x0100): Got request with the following data (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): command: SSS_PAM_OPEN_SESSION (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): domain: ipa.example (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): user: admin(a)ipa.example (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): service: su-l (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): tty: pts/0 (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): ruser: root (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): rhost: (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): authtok type: 0 (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): newauthtok type: 0 (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): priv: 1 (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): cli_pid: 2324 (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): logon name: not set (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_attach_req] (0x0400): DP Request [PAM Open Session #7]: New request. Flags [0000]. (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_attach_req] (0x0400): Number of active DP request: 1 (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [sss_domain_get_state] (0x1000): Domain ipa.example is Active (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_find_method] (0x0020): Bug: Invalid combination of target [session] and method [9] (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [_dp_req_recv] (0x0400): DP Request [PAM Open Session #7]: Receiving request data. (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_req_destructor] (0x0400): DP Request [PAM Open Session #7]: Request removed. (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_req_destructor] (0x0400): Number of active DP request: 0 (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_req_reply_gen_error] (0x0020): DP Request [PAM Open Session #7]: Finished. Error [1432158209]: Internal Error (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [sbus_dispatch] (0x4000): dbus conn: 0x560901ec30b0 (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [sbus_dispatch] (0x4000): Dispatching. (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [sbus_message_handler] (0x2000): Received SBUS method org.freedesktop.sssd.dataprovider.pamHandler on path /org/freedesktop/sssd/dataprovider (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [sbus_get_sender_id_send] (0x2000): Not a sysbus message, quit (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_pam_handler] (0x0100): Got request with the following data (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): command: SSS_PAM_CLOSE_SESSION (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): domain: ipa.example (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): user: admin(a)ipa.example (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): service: su-l (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): tty: pts/0 (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): ruser: root (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): rhost: (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): authtok type: 0 (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): newauthtok type: 0 (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): priv: 1 (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): cli_pid: 2324 (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): logon name: not set (Fri Aug 18 10:19:45 2017) [sssd[be[ipa.example]]] [dp_pam_reply] (0x1000): DP Request [PAM Close Session]: Sending result [0][ipa.example] ``` Seems that we throw an internal error there. I'll fix it and update the patch to be squashed so @pbrezina can review. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323318997

fidencio

Saturday, 19 August Sat, 19 Aug

5:42 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration fidencio commented: """ @pbrezina: So, here's the patch that solves the issue: ``` From ae60eae181c7a3214d76b3ff00d9e431f060bbc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio(a)redhat.com> Date: Fri, 18 Aug 2017 19:46:20 +0200 Subject: [PATCH] fixup! DESKPROFILE: Introduce the new IPA session provider --- src/providers/ipa/ipa_init.c | 9 --------- src/providers/ipa/ipa_session.c | 10 ++++++++++ 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/providers/ipa/ipa_init.c b/src/providers/ipa/ipa_init.c index b3daa4921..7cae43c06 100644 --- a/src/providers/ipa/ipa_init.c +++ b/src/providers/ipa/ipa_init.c @@ -950,20 +950,11 @@ errno_t sssm_ipa_session_init(TALLOC_CTX *mem_ctx, struct ipa_session_ctx *session_ctx; struct ipa_init_ctx *init_ctx; struct ipa_id_ctx *id_ctx; - bool enabled; errno_t ret; init_ctx = talloc_get_type(module_data, struct ipa_init_ctx); id_ctx = init_ctx->id_ctx; - enabled = dp_opt_get_bool(id_ctx->ipa_options->basic, - IPA_ENABLE_DESKPROFILE); - if (!enabled) { - DEBUG(SSSDBG_TRACE_FUNC, "ipa_enable_deskprofile is set to FALSE\n"); - - return EOK; - } - session_ctx = talloc_zero(mem_ctx, struct ipa_session_ctx); if (session_ctx == NULL) { DEBUG(SSSDBG_CRIT_FAILURE, "talloc_zero() failed.\n"); diff --git a/src/providers/ipa/ipa_session.c b/src/providers/ipa/ipa_session.c index 8559284c9..800ea665a 100644 --- a/src/providers/ipa/ipa_session.c +++ b/src/providers/ipa/ipa_session.c @@ -485,6 +485,7 @@ ipa_pam_session_handler_send(TALLOC_CTX *mem_ctx, struct tevent_req *req; struct tevent_req *subreq; struct ipa_pam_session_handler_state *state; + bool enabled; errno_t ret; DEBUG(SSSDBG_TRACE_FUNC, "Retrieving Desktop Profile rules\n"); @@ -500,6 +501,15 @@ ipa_pam_session_handler_send(TALLOC_CTX *mem_ctx, state->be_ctx = params->be_ctx; state->session_ctx = session_ctx; + enabled = dp_opt_get_bool(session_ctx->ipa_options, + IPA_ENABLE_DESKPROFILE); + if (!enabled) { + ret = EOK; + DEBUG(SSSDBG_TRACE_FUNC, "ipa_enable_deskprofile is set to FALSE\n"); + state->pd->pam_status = PAM_SUCCESS; + goto done; + } + /* Get all the user info that will be needed in order the delete the * user's deskprofile directory from the disk, create the user's directory, * save the fetched rules to the disk and notify the deskprofile client -- 2.13.5 ``` In case we returned EOK from `sssm_ipa_session_init`, having the handlers set was something expected (and it was **not** happening). So, I've decided to do the check about whether the config was enabled or not later on in the code, in the `_send() ` handler. There it's done before any call to ldap or sysdb is done (or, IOW, without messing with performance). Here are the logs when: - `ipa_deskprofile_enable = false`: ``` (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [dp_pam_handler] (0x0100): Got request with the following data (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): command: SSS_PAM_OPEN_SESSION (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): domain: ipa.example (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): user: admin(a)ipa.example (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): service: su-l (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): tty: pts/0 (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): ruser: root (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): rhost: (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): authtok type: 0 (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): newauthtok type: 0 (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): priv: 1 (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): cli_pid: 2083 (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): logon name: not set (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [dp_attach_req] (0x0400): DP Request [PAM Open Session #4]: New request. Flags [0000]. (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [dp_attach_req] (0x0400): Number of active DP request: 1 (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [sss_domain_get_state] (0x1000): Domain ipa.example is Active (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [ipa_pam_session_handler_send] (0x0400): Retrieving Desktop Profile rules (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [ipa_pam_session_handler_send] (0x0400): ipa_enable_deskprofile is set to FALSE (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [dp_req_done] (0x0400): DP Request [PAM Open Session #4]: Request handler finished [0]: Success (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [_dp_req_recv] (0x0400): DP Request [PAM Open Session #4]: Receiving request data. (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [dp_req_destructor] (0x0400): DP Request [PAM Open Session #4]: Request removed. (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [dp_req_destructor] (0x0400): Number of active DP request: 0 (Sat Aug 19 10:27:02 2017) [sssd[be[ipa.example]]] [dp_pam_reply] (0x1000): DP Request [PAM Open Session #4]: Sending result [0][ipa.example] ``` - `session_provider = none`: ``` (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [dp_pam_handler] (0x0100): Got request with the following data (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): command: SSS_PAM_OPEN_SESSION (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): domain: ipa.example (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): user: admin(a)ipa.example (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): service: su-l (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): tty: pts/0 (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): ruser: root (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): rhost: (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): authtok type: 0 (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): newauthtok type: 0 (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): priv: 1 (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): cli_pid: 2247 (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [pam_print_data] (0x0100): logon name: not set (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [dp_attach_req] (0x0400): DP Request [PAM Open Session #2]: New request. Flags [0000]. (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [dp_attach_req] (0x0400): Number of active DP request: 1 (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [sss_domain_get_state] (0x1000): Domain ipa.example is Active (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [dp_find_method] (0x0100): Target [session] is not initialized (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [_dp_req_recv] (0x0400): DP Request [PAM Open Session #2]: Receiving request data. (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [dp_req_destructor] (0x0400): DP Request [PAM Open Session #2]: Request removed. (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [dp_req_destructor] (0x0400): Number of active DP request: 0 (Sat Aug 19 10:29:25 2017) [sssd[be[ipa.example]]] [dp_req_reply_gen_error] (0x0080): DP Request [PAM Open Session #2]: Finished. Target is not supported with this configuration. ``` I'm updating the patch set with the mentioned patch squashed. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323515639

fidencio

5:44 a.m.

New subject: [sssd PR#241][synchronized] FleetCommander Integration

pbrezina

Monday, 21 August Mon, 21 Aug

5:39 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration pbrezina commented: """ I think it is better to set the method only when needed. Try this diff instead of your patch: ```c --- a/src/providers/data_provider/dp_target_auth.c +++ b/src/providers/data_provider/dp_target_auth.c @@ -126,9 +126,16 @@ static void choose_target(struct data_provider *provider, name = "PAM Chpass 2nd"; break; case SSS_PAM_OPEN_SESSION: - target = DPT_SESSION; - method = DPM_SESSION_HANDLER; name = "PAM Open Session"; + if (dp_method_enabled(provider, DPT_SESSION, DPM_SESSION_HANDLER)) { + target = DPT_SESSION; + method = DPM_SESSION_HANDLER; + break; + } + + target = DP_TARGET_SENTINEL; + method = DP_METHOD_SENTINEL; + pd->pam_status = PAM_SUCCESS; break; case SSS_PAM_SETCRED: target = DP_TARGET_SENTINEL; ``` Also, as I read through the patches, I don't see `session_provider` documented anywhere and since `ipa_deskprofile_enable ` defaults to `true` it is basically an equivalent to `session_provider = ipa | not set`. Wouldn't it be better to remove this option and document `session_provider` instead as we do with other providers (e.g. hostid)? """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323710521

fidencio

5:43 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration fidencio commented: """ On Mon, Aug 21, 2017 at 12:39 PM, Pavel Březina <notifications(a)github.com> wrote:

...

I think it is better to set the method only when needed. Try this diff instead of your patch: --- a/src/providers/data_provider/dp_target_auth.c +++ b/src/providers/data_provider/dp_target_auth.c @@ -126,9 +126,16 @@ static void choose_target(struct data_provider *provider, name = "PAM Chpass 2nd"; break; case SSS_PAM_OPEN_SESSION: - target = DPT_SESSION; - method = DPM_SESSION_HANDLER; name = "PAM Open Session"; + if (dp_method_enabled(provider, DPT_SESSION, DPM_SESSION_HANDLER)) { + target = DPT_SESSION; + method = DPM_SESSION_HANDLER; + break; + } + + target = DP_TARGET_SENTINEL; + method = DP_METHOD_SENTINEL; + pd->pam_status = PAM_SUCCESS; break; case SSS_PAM_SETCRED: target = DP_TARGET_SENTINEL; Also, as I read through the patches, I don't see session_provider documented anywhere and since ipa_deskprofile_enable defaults to true it is basically an equivalent to session_provider = ipa | not set. Wouldn't it be better to remove this option and document session_provider instead as we do with other providers (e.g. hostid)?

I like the idea to have only the session_provider option documented/exposed. @jhrozek, would that be okay for you? Then we can drop the ipa_enable_deskprofile option entirely.

...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <https://github.com/SSSD/sssd/pull/241#issuecomment-323710521>;, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAG4etDXxe7hxYhC8DpwTR2... .

""" See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323711365

fidencio

9:06 a.m.

New subject: [sssd PR#241][synchronized] FleetCommander Integration

fidencio

9:28 a.m.

New subject: [sssd PR#241][synchronized] FleetCommander Integration

pbrezina

Tuesday, 22 August Tue, 22 Aug

3:06 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration pbrezina commented: """ Typo in "related": ```xml + <para> + Default: <quote>id_provider</quote> is used if it + is set and can perform session reltted tasks. + </para> ``` This works and it looks good in provider logs but in pam log is a system error which may be confusing. Please add the diff I provided so we can avoid it. ``` 275 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_dp_send_req] (0x0100): Sending request with the following data: 276 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): command: SSS_PAM_OPEN_SESSION 277 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): domain: LDAP.PB 278 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): user: user-1(a)ldap.pb 279 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): service: su 280 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): tty: pts/7 281 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): ruser: pbrezina 282 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): rhost: not set 283 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): authtok type: 0 284 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): newauthtok type: 0 285 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): priv: 0 286 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): cli_pid: 10586 287 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_print_data] (0x0100): logon name: user-1 288 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [sbus_add_timeout] (0x2000): 0xe9e330 289 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_dom_forwarder] (0x0100): pam_dp_send_req returned 0 290 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [sbus_remove_timeout] (0x2000): 0xe9e330 291 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_dp_process_reply] (0x0010): Reply error. 292 (Tue Aug 22 09:59:23 2017) [sssd[pam]] [pam_reply] (0x0200): pam_reply called with result [4]: System error. ``` """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-323951221

fidencio

6:54 a.m.

New subject: [sssd PR#241][synchronized] FleetCommander Integration

fidencio

6:54 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration fidencio commented: """ @pbrezina: patchset has been updated. Thanks for your suggestions! """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-324003707

pbrezina

7:30 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration pbrezina commented: """ On 08/22/2017 01:54 PM, fidencio wrote:

...

@pbrezina <https://github.com/pbrezina>;: patchset has been updated. Thanks for your suggestions!

Ack. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-324011406

olivergs

Monday, 28 August Mon, 28 Aug

7:18 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration olivergs commented: """ Tested the latest build for the patches from @fidencio COPR repo and it worked as expected. On Thu, Aug 17, 2017 at 4:23 PM, fidencio <notifications(a)github.com> wrote:

...

Patch set updated. This is the patch that I've squashed: From 97f189d3a4fcbbfe453cc6ba8eab8710ca31c4ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio(a)redhat.com> Date: Thu, 17 Aug 2017 13:29:24 +0200 Subject: [PATCH] fixup! DESKPROFILE: Introduce the new IPA session provider --- src/providers/ipa/ipa_deskprofile_rules_util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/providers/ipa/ipa_deskprofile_rules_util.c b/src/providers/ipa/ipa_deskprofile_rules_util.c index 12c6492ba..2ece5a1ee 100644 --- a/src/providers/ipa/ipa_deskprofile_rules_util.c +++ b/src/providers/ipa/ipa_deskprofile_rules_util.c @@ -150,7 +150,7 @@ ipa_deskprofile_get_filename_path(TALLOC_CTX *mem_ctx, extension, NULL, }; - const uint8_t *perms = permuts[config_priority - 1]; + const uint8_t *perms; char *result; errno_t ret; @@ -167,6 +167,8 @@ ipa_deskprofile_get_filename_path(TALLOC_CTX *mem_ctx, goto done; } + perms = permuts[config_priority - 1]; + result = talloc_strdup(tmp_ctx, ""); if (result == NULL) { ret = ENOMEM; -- 2.13.4 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/SSSD/sssd/pull/241#issuecomment-323102642>;, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABX4oQhaAXHRCzP43H7qrQ3... .

-- Oliver Gutierrez Associate Software Engineer - Desktop Management tools Red Hat """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-325337915

olivergs

7:19 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration olivergs commented: """ Tested the latest build for the patches from @fidencio COPR repo and it worked as expected. """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-325337915

jhrozek

10:44 a.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ ACK, CI: http://vm-058-233.XXX/logs/job/74/33/summary.html """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-325391191

jhrozek

10:44 a.m.

New subject: [sssd PR#241][+Accepted] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration Label: +Accepted

jhrozek

1:45 p.m.

New subject: [sssd PR#241][comment] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration jhrozek commented: """ * master: 4a311702045b065a97a0c0fc0ccc7a1fc84b38cf 85517b57685809ff96818bbd3e3b4678ac74b461 85a93ca67ae020607006cd035170c9360fb0a450 684a13e8de1526257ca2e40b6bf2e05585d4eaca 4b37ee7d370003514916c793046577ea4b6e736b dd6a4fb9ae4825caf4ccb835f8b8221c96bbb6f5 c9e104f17b6c4cf5741dea9fdbe864619125fab1 9d98e98ab37d86323034e7bc342f196b81fa07bc b054e7d8c43b024ee33e9343b4a15e124861f68c f982039c75ec064894deb676ae53ee57de868590 6f466e0a3d950d21bd750ef53cb93b75dc023f9e 5b93634c7f0e34f69b4cf8fb9b2e77b9179024a7 7c1d1393537dec95e09b83b607ce9d0e8f49584c 18d898d9cb30f298b3a35dc1c1bace95ef4e0b3b ee164913f9c12a557044eb469f4498b9be9a8f50 0f623456437c96f50330fe0ff21afd9638d14e57 d2a0b4a6a220bf9a58c7306c3f673891efc419eb e17e37cd0e2109e7f1bd4ae48edfc8cca85b3f93 21909d3b620d97e81dd946b959a47efe88d2b7d8 9a18f78f38e274f4906af6ef8e1a82d844fde4cc 8a26d32bc9b71e85a42b7832891100a7249f92aa """ See the full comment at https://github.com/SSSD/sssd/pull/241#issuecomment-325442601

jhrozek

1:45 p.m.

New subject: [sssd PR#241][+Pushed] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Title: #241: FleetCommander Integration Label: +Pushed

jhrozek

1:45 p.m.

New subject: [sssd PR#241][closed] FleetCommander Integration

URL: https://github.com/SSSD/sssd/pull/241 Author: fidencio Title: #241: FleetCommander Integration Action: closed To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/241/head:pr241 git checkout pr241

2432

days inactive

2562

days old

sssd-devel@lists.fedorahosted.org

Manage subscription

63 comments

4 participants

tags (0)

participants (4)

fidencio
jhrozek
olivergs
pbrezina

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

[sssd PR#241][opened] FleetCommander Integration