~kennylevinsen/greetd-devel

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
1

[PATCH greetd] For FUSE-based home folders, chdir into them as the user instead of root.

Details
Message ID
<166922457976.13139.5866227689620072039-0@git.sr.ht>
DKIM signature
missing
Download raw message
Patch: +10 -11
From: Felix Lechner <felix.lechner@lease-up.com>

By default, filesystems user-mounted via FUSE are not accessible to
root. [1] Such user mounts have been common for encrypted home folders
since 2003. [2][3][4] This change accommodates users with those home
folders.

Greetd previously sent affected users into the root directory ("/")
because their home folders were inaccessible to root.

Now the directory operation occurs after a user's privileges were
assumed. Users find themselves in their home folders after logging in.

Since the call to PAM's open_session now takes place before any change
of folders, the value of the current directory is no longer being
exposed to the modules via the environment variable $PWD, but the PAM
environment is distinct from the process environment.

This commit was tested on Guix without commit 424ecac4 since the Rust
crate for nix 0.20 was not immediately available there.

[1] https://unix.stackexchange.com/a/17423
[2] https://en.wikipedia.org/wiki/Comparison_of_disk_encryption_software
[3] https://en.wikipedia.org/wiki/EncFS
[4] https://nuetzlich.net/gocryptfs/
---
 greetd/src/session/worker.rs | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/greetd/src/session/worker.rs b/greetd/src/session/worker.rs
index 20432a1..ddf0701 100644
--- a/greetd/src/session/worker.rs
+++ b/greetd/src/session/worker.rs
@@ -171,16 +171,6 @@ fn worker(sock: &UnixDatagram) -> Result<(), Error> {
    let uid = Uid::from_raw(user.uid());
    let gid = Gid::from_raw(user.primary_group_id());

    // Change working directory
    let pwd = match env::set_current_dir(home) {
        Ok(_) => home,
        Err(_) => {
            env::set_current_dir("/")
                .map_err(|e| format!("unable to set working directory: {}", e))?;
            "/"
        }
    };

    // PAM has to be provided a bunch of environment variables before
    // open_session. We pass any environment variables from our greeter
    // through here as well. This allows them to affect PAM (more
@@ -193,7 +183,6 @@ fn worker(sock: &UnixDatagram) -> Result<(), Error> {
        format!("LOGNAME={}", username),
        format!("HOME={}", home),
        format!("SHELL={}", shell),
        format!("PWD={}", pwd),
        format!("GREETD_SOCK={}", env::var("GREETD_SOCK").unwrap()),
        format!(
            "TERM={}",
@@ -242,6 +231,16 @@ fn worker(sock: &UnixDatagram) -> Result<(), Error> {
            // death signal, which is why we do this here.
            prctl(PrctlOption::SET_PDEATHSIG(libc::SIGTERM)).expect("unable to set death signal");

            // Change working directory
            let pwd = match env::set_current_dir(home) {
                Ok(_) => home,
                Err(_) => {
                    env::set_current_dir("/")
                        .map_err(|e| format!("unable to set working directory: {}", e))?;
                    "/"
                }
            };

            // Run
            let cpath = CString::new("/bin/sh").unwrap();
            execve(
-- 
2.34.5
Details
Message ID
<0M3XLR.D5M5QKVNAH7S2@kl.wtf>
In-Reply-To
<166922457976.13139.5866227689620072039-0@git.sr.ht> (view parent)
DKIM signature
pass
Download raw message
On Tue, Nov 22 2022 at 01:21:51 PM -08:00:00, ~lechner 
<lechner@git.sr.ht> wrote:
> +            // Change working directory
> +            let pwd = match env::set_current_dir(home) {
> +                Ok(_) => home,
> +                Err(_) => {
> +                    env::set_current_dir("/")
> +                        .map_err(|e| format!("unable to set working 
> directory: {}", e))?;

Once we have entered the child after fork(2), we must not return. 
Instead we should either successfully exec the new process, or exit 
with an error. We have so far used `expect` and `unwrap` to just panic 
if there is an error.

On the other hand, maybe we should just skip the fallback chdir to "/", 
and just leave the user in whatever greetd's cwd was (which is likely 
"/" anyway). In that case we can just ignore errors from chdir(home) 
altogether, making the code simpler.
Reply to thread Export thread (mbox)