On Fri, Apr 08, 2016 at 07:31:59PM +0300, Nikolai Kondrashov wrote:
On 04/06/2016 02:06 PM, Sumit Bose wrote:
>On Wed, Apr 06, 2016 at 12:59:32PM +0300, Nikolai Kondrashov wrote:
>>On 03/18/2016 07:56 PM, Nikolai Kondrashov wrote:
>>>The attached patch adds exporting of the original (non-overridden) user
shell
>>>to tlog-rec, during the PAM session opening. The shell is exported via
adding
>>>variable "TLOG_REC_SHELL" to the user's environment.
>>
>>Bump, tlog still needs this!
>
>Hi Nikolai,
>
>sorry for the delay. The patch looks good in general and is working as
>expected. Currently the environment variable is set unconditionally which
>is good for testing but it might be irritating for some users that there
>is a new environment variable they do not know about and might be useless
>for them. Would it be possible to set the variable only if tloc is
>enabled?
Sure, it should be possible. However, we don't have any "tlog enabling" so
far
and my impression was that it's undesirable at this stage. Jakub, what are
your thoughts on this?
>I wonder if it would makes sense to add the cached user object to preq
>in pam_check_user_search() to avoid the lookup in
>pam_reply_export_shell(). The data is already allocated on preq and as
>far as I can see never freed explicitly, so it wouldn't even cost more
>memory.
Sure, that would be nice. However it's really hard for me to tell where that
would come from, where it's actually retrieved and what's the lifetime would
be. I really miss documentation there.
Could you suggest the change, perhaps?
sure, please have a look at attached (untested) patch. With this you start in
pam_reply_export_shell() with
+ shell = ldb_msg_find_attr_as_string(preq->user_obj, SYSDB_SHELL, NULL);
+ if (shell == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "user has no shell\n");
+ ret = ENOENT;
+ goto done;
+ }
....
HTH
bye,
Sumit
Thank you!
Nick
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org