Env Problem

GitHub

Summary: Possible approaches to fix the env reading issue of SWHKD

Marked: Tue Jun 04 2024

Category: Other

Reviewed: ✅

User priveledge management and handling the environment all at once is a bit of a tricky problem. The current implementation of SWHKD uses the pkexec command to run the child process with elevated privileges, however, this comes with the problem that the environment variables are not passed to the child process.

To solve this, I’ve been thinking of a few approaches:

1. Fork a Child process to userspace and construct env:

The first approach would be quite straightforward. We just fork into a new child process, use out perms module to de-escalate the child process to the user’s privileges Then, we can construct the environment variables in the child process and run the command. This can be transferred to the parent process through a shared memory area or easier yet, a simple channel.

It would look something like this:

let (mut env_send, env_rev) = mpsc::channel::<environ::Env>();

match unsafe { fork() } {
    Ok(ForkResult::Parent { .. }) => {
    }
    Ok(ForkResult::Child) => {
        if setuid(Uid::from_raw(user_id)).is_err() {
            eprintln!("Failed to set UID.");
            std::process::exit(1);
        }
        let command =
            Command::new("env").output().expect("Failed to execute command");
        let env = String::from_utf8(command.stdout).unwrap();
        env_send.send(env).unwrap();
    }
    Err(_) => {
        eprintln!("Fork failed.");
        std::process::exit(1);
    }
}

let env = env_rev.recv().unwrap();
println!("{:#?}", env);

On the surface, this seems to be quite promising and a viable approach to the problem. However, in practice, in implementation has one major flaw.

It uses the env command to get the environment variables of the parent process. The env command is a shell command that prints the environment variables of the current shell. It does this by reading the /proc/self/environ file. It turns out however, that when you actually use sudo or pkexec to run a command, the environment variables are not passed to the child process. It instead forks into a new root process and runs the command there.

Here lies our problem. Moreover, when we make a child process, it inherits the environment of the parent process. So even if we are running in the userspace, it would make no difference. Bummer.

2. Parse the .profile file

This is a rather naive approach and I am including it here just for the sake of completeness. One approach is to parse the .profile file of the user and construct the environment variables from there. These are all persistent environment variables that are set when the user logs in. However, these are not guaranteed to be the same as the environment variables of the login shell.

Not only just this approach, but any approach that involves reading the environment variables from a persistant source involves the same problem, the fact that the environment variables are subject to change. The best way to get them is when the process is actually run, not before, nor after. Hence, this is the reason so many programs use the standard env command to get the environment variables or just read off the /proc/self/environ file.

3. Use IPC

This brings me to what seems to be the most viable and sane approach to me, using IPC. We already have a server daemon that is running in the userspace by default. The plan was to kinda phase out the daemon or make it a null main process, so that it is not a breaking change.

However, the server can still have some use. Most uses currently using SWHKD are already using the SWHKS and SWHKD anyways.

So, we can rewrite the server to do the following:

  • Listen on a socket
  • Accept a connection
  • Read the environment variables from the userspace it is running in
  • Send the environment variables to the client

The client can then parse the environment variables through whatever means it wants to and run the command. The server can be so simple, it can just send a , seperated string of the environment variables.

Here’s what it would look like:

// Server

let listener = UnixListener::bind("/tmp/swhkd.sock").unwrap();

for stream in listener.incoming() {
    match stream {
        Ok(mut stream) => {
            let env = std::env::vars().collect::<Vec<(String, String)>>();
            let env_string = env.iter().map(|(k, v)| format!("{}={}", k, v))
                .collect::<Vec<String>>().join(",");
            stream.write(env_string.as_bytes()).unwrap();
        }
        Err(e) => {
            eprintln!("Error: {}", e);
        }
    }
}

Now the client can connect to the server and read the environment variables as a string and parse them.

// Client

let stream = UnixStream::connect("/tmp/swhkd.sock").unwrap();
let mut buffer = [0; 1024];

stream.read(&mut buffer).unwrap();
let env_string = String::from_utf8_lossy(&buffer);
let env: Vec<(String, String)> = env_string.split(",").map(|x| {
    let mut split = x.split("=");
    (split.next().unwrap().to_string(), split.next().unwrap().to_string())
}).collect();

println!("{:#?}", env);

This should work quite well and is a very clean and simple solution to the problem. Added bonus, we can also design this in such a way that the server can be optional. As in if the user want to read the env, which would be the only function of the server, atleast for now, they can run the server daemon along with the client.

However, this would cause no vulnerabilities as the server is running in the userspace and is not doing anything at all, just passing in the environment variables. We can also sanitize these variables before sending them to the client, hence the chance of the process escalation CVE can be mitigated.

Possible Improvements:

  • Whenever the server is run, it can also write the evironment variables to a file, these can act as the temporary cache for the environment variables, so the server need not be run everytime.

  • This approach does sound the most viable, however, experimenting with the child process would also be helpful.