Bug 195759
| Summary: | x11/slim: Fails to honour login.conf | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Jonathan Chen <jonc> | ||||||
| Component: | Individual Port(s) | Assignee: | freebsd-ports-bugs (Nobody) <ports-bugs> | ||||||
| Status: | Closed FIXED | ||||||||
| Severity: | Affects Some People | CC: | henry.hu.sh, marino, pi | ||||||
| Priority: | --- | Keywords: | needs-qa, patch, patch-ready | ||||||
| Version: | Latest | Flags: | koobs:
maintainer-feedback+
|
||||||
| Hardware: | Any | ||||||||
| OS: | Any | ||||||||
| Attachments: |
|
||||||||
Maintainer CC'd I set LANG in .cshrc which works well. Because login.conf is FreeBSD-specific, slim is not reading from it, and I need to find out which program is in charge of reading from that. For $HOME in PATH, I guess that's caused by the new default slim configuration file, which we copied the path setting from default login.conf, but slim does not resolve variables in its settings. I think that I can just remove the $HOME/bin from the default path, and if I find out which program should set the environment according to login.conf, this should also be handled by that program. I had a quick look, and I believe that patch-switchuser.cpp should make use of setusercontext() in login_class(3); instead of just setlogin(2). (In reply to Jonathan Chen from comment #3) > I had a quick look, and I believe that patch-switchuser.cpp should make use > of setusercontext() in login_class(3); instead of just setlogin(2). Nice catch. It seems like that we can replace multiple calls with a single setusercontext(): setlogin -> LOGIN_SETLOGIN initgroups, setgid -> LOGIN_SETGROUP setuid -> LOGIN_SETUSER and other problems are solved as well: path -> LOGIN_SETPATH lang -> LOGIN_SETENV seems like that we should just use LOGIN_SETALL. To get the login_cap_t, currently I'm using login_getpwclass(). I should use login_getuserclass() at the same time, and query the later one first, and turn to the first one if something is not present in the later one. However, setusercontext() does not support this, so just use login_getpwclass() for now. I've created a patch for this. Please test it. I'm not sure if all these APIs are present on older systems. Created attachment 150621 [details]
use login capabilities database to set up environment
It uses login_getpwclass() and setusercontext(), and falls back to the old method if that fails.
Unfortunately, the patch didn't work. env is: VENDOR=amd LOGNAME=jonc OSTYPE=FreeBSD MACHTYPE=x86_64 XAUTHORITY=/home/jonc/.Xauthority MAIL=/var/mail/jonc PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr/games:/usr/local/sbin:/usr/local/bin HOST=osiris.chen.org.nz DISPLAY=:0.0 PWD=/home/jonc GROUP=core USER=jonc HOME=/home/jonc HOSTTYPE=FreeBSD SHELL=/bin/tcsh The logs are too sparse to make anything out. Perhaps add more log information to see where it's failing? (In reply to Jonathan Chen from comment #6) > Unfortunately, the patch didn't work. env is: > > VENDOR=amd > LOGNAME=jonc > OSTYPE=FreeBSD > MACHTYPE=x86_64 > XAUTHORITY=/home/jonc/.Xauthority > MAIL=/var/mail/jonc > PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr/games:/usr/local/sbin:/usr/local/bin > HOST=osiris.chen.org.nz > DISPLAY=:0.0 > PWD=/home/jonc > GROUP=core > USER=jonc > HOME=/home/jonc > HOSTTYPE=FreeBSD > SHELL=/bin/tcsh > > The logs are too sparse to make anything out. Perhaps add more log > information to see where it's failing? Have you seen anything strange in /var/log/slim.log? No. It's a pretty sparse file. Maybe put more logging in the patch to see what's failing and why? Ok, I know why.
In app.cpp, slim creates child_env, and uses it as the environment of the child.
If PAM is not used, the array child_env is manually created and filled with predefined entries. Else, the array is obtained from the pam module.
So it seems hard to support all the environment variables that setusercontext() supports. Which environment variables do you really need? LANG and PATH and anything else?
setusercontext() supports these env vars:
path PATH
manpath MANPATH
lang LANG
charset MM_CHARSET
timezone TZ
term TERM
For PATH, because there is an option called default_path in slim's configuration file, maybe we should just ignore the login.conf.
For LANG, we can get the entry "lang" from login database and set it.
For TERM, slim sets it from its own version. Anyway, we're using slim so it's not that important.
For other variables(TZ, MM_CHARSET, MANPATH), we can do it in a similar way.
But it might be still better to use setusercontext() because it sets resource limits, priorities, umask and other things.
Created attachment 150706 [details]
2nd ver
Set the environment variables specified in login.conf.
setusercontext() is still called, because it sets other limitations.
It looks good. I checked the environment vars, umask, as well as resource limits. I have noticed that about 15% of the time, my login will fail; and a retry is required. The logs do not indicate any authentication errors. (In reply to Jonathan Chen from comment #12) > I have noticed that about 15% of the time, my login will fail; and a retry > is required. The logs do not indicate any authentication errors. Does it happen without the patch? Authentication is always successful with the patch (barring fumble fingers). With a quick scan of the comments, I believe the maintainer has come up with a patch that should be committed, so let's move this PR to "patch-ready" status. (In reply to John Marino from comment #15) According to user's report, the patch works. After reading setusercontext()'s code, it seems like that .login.conf is also honored. I think that the patch needs security auditing, because it's a patch to a login manager, but I'm not sure who is responsible for this. I don't know either. We don't have a "login manager" guru position identified. Thanks for the update Jonathan, if could include successful build output as an attachment, that will help this issue progress. Highly preferred is: - poudriere testport or bulk -t ouput For instructions on how to set this up, see: https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/testing-poudriere.html OR - make stage && make check-plist && make stage-qa && make package poudriere build-testing@work A commit references this bug: Author: pi Date: Sun Feb 22 14:50:27 UTC 2015 New revision: 379608 URL: https://svnweb.freebsd.org/changeset/ports/379608 Log: x11/slim: fix to honour login.conf, WWW PR: 195759 Requested by: Jonathan Chen <jonc@chen.org.nz> Approved by: Henry Hu <henry.hu.sh@gmail.com> (maintainer) Changes: head/x11/slim/Makefile head/x11/slim/files/patch-CMakeLists.txt head/x11/slim/files/patch-app.cpp head/x11/slim/files/patch-slim.conf head/x11/slim/files/patch-switchuser.cpp head/x11/slim/pkg-descr build-test went fine on 10.1a, 9.3a, 8.4i, so: committed, thanks all! |
I have the set lang within /etc/login.conf: default:\ :lang=en_NZ.UTF-8:\ :passwd_format=sha512:\ ... I've "cap_mkdb /etc/login.conf". In my ~/.xinitrc: env > /tmp/j-env exec mate-session > .xsession-errors 2>&1 The contents of /tmp/j-env after logging in: VENDOR=amd LOGNAME=jonc OSTYPE=FreeBSD MACHTYPE=x86_64 XAUTHORITY=/home/jonc/.Xauthority MAIL=/var/mail/jonc PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr/games:/usr/local/sbin:/usr/local/bin:$HOME/bin HOST=osiris.chen.org.nz DISPLAY=:0.0 PWD=/home/jonc GROUP=core USER=jonc HOME=/home/jonc HOSTTYPE=FreeBSD SHELL=/bin/tcsh XDG_SESSION_COOKIE=8df97606318ac9f41ded527c53c12017-1417896465.683295-2112864763 2 things stand out: 1. LANG is not set. 2. PATH has literal and not evaluated "$HOME/bin" appended. The options on my x11/slim: PAM : on UTF8 : off