Improve The Security Model

[newtoallofthis123]

Below PR aims to simplify and improve swhkd's security model. Please refer to the changelog for full details

Changes made

It would be difficult to layout all of the changes made in the codebase in a single changelog, however, the following are the major changes that can be easily noticable in the codebase. For the full changes, please refer to the Github repository.

1. Seperate Env Module: refer: All of the env parsing and feed has been categorized into a seperate module called environ.rs. This module is responsible for parsing the environment variables and providing them to the daemon. The env from the server is just a newline separated string of key=value pairs. These are parsed, deduped and then fed to the daemon.

2. Retire SWHKS command execution: refer: The SWHKS command execution has been retired and instead, the daemon now uses su to execute the commands in the user space. This means that the daemon is now less reliant on the server and hence the security concerns are mitigated.

3. Daemonize the server and env sending: refer: The server has been daemonized and all of it's output has been redirected to /dev/null. Moreover, the actual function to send the env to the daemon has been added. Any connection to the server now results in the server sending the env.

4. Event based env refresh: refer: The server now has an event based cron job that sends the env to the daemon every 650ms by default. However, the user can specify a custom timeout using the -r flag.

5. Config file better defaults: refer: The config file location now correctly defaults to ~/.config/swhkd/swhkdrc thanks to the environ refresh from the server. The problem was that the daemon was running in root space with the root user's env, so it was not able to find the config file that was stored in the user space. So, just for the config file location, the daemon now requests the server for the env and then uses su to read the config file.

6. Channel Based Communication: refer: The daemon now uses a channel based communication to communicate with the thread. The thread is spawned at the beginning and is valid throughout the lifetime of the daemon. A mpsc channel of a good default of 100 is used to communicate between the daemon and the thread. This means that the daemon can now execute the commands in the user space without spawning a new thread everytime.

7. Server Instance Tracking: refer: The server now detects if it is already running and if it is, it doesn't start a new instance. This is usefull because it has been daemonized and hence can be running in the background when a new connection is made.

Final Flow

The final flow of the daemon is as follows: The daemon is launched in the root space and the server is launched in the user space. This is reminiscent of the old IPC model as such:

./swhks && doas ./swhkd

The doas or sudo can be skipped by making the swhkd binary a setuid binary. This can be done by running the following command:

sudo chown root:root swhkd
sudo chmod u+s swhkd

Right after this is done, the first connection to the server is made and the server sends the env to the daemon. This information is stored in the env struct instance and this is exchanged and valid throughout the process life cycle. The XDG_CONFIG_HOME is also set to ~/.config/swhkd and the config file is read from there if it exists. A thread is spawned that is valid throughout the lifetime of the daemon. The thread is also de-escalated to the user space. The thread can communicate with the daemon through a channel.

Next, the daemon starts listening for the key events. When a key event is detected, the daemon just sends it to the thread through the channel. Concurrently, there is a cron job that sends the env to the daemon every 650ms by default. This ensures that the env is always fresh and the daemon can always execute the commands in the user space.

[Shinyzenith]

Hi! Ci is broken. Can you resolve the issue?

[newtoallofthis123]

Yep I messed up a small line :)

[newtoallofthis123]

@Shinyzenith It should be all fixed now :)

[Shinyzenith]

I still do not understand why we are adding 120ms, we were gonna subtract 10% as delta right?

[newtoallofthis123]

I'm sorry I had not updated what I was working on This is the latest commit

[newtoallofthis123]

@Shinyzenith this should work :)

[Shinyzenith]

Hi, have you documented all your changes? Some parts of the code still feel awkward to me, we'll speak personally on that later

[Shinyzenith]

Eg:

    let _ = daemon(true, false);

Why? This line doesn't make any sense, the function name doesn't tell me what it is intuitively by just looking at it once. What are the arguments you're passing? It doesn't make much sense.

Edit: ah this is unistd::daemon, I was searching for fn daemon and that led to the confusion. Probably should just call it unistd::daemon as it adds more context.

[newtoallofthis123]

I can leave more comments if that is helpful. Especially near the areas where the tokio spawns and env exchanges happen. You are right it should have more context.

[newtoallofthis123]

As for documentation, the description for the pr is up to date, however I will also leave some helpful comments in the places i made changes to

[Shinyzenith]

@newtoallofthis123 In terms of documentation, I mean updating readme and man pages with relevant information. The comments are an important part too. Thanks for taking note.

[newtoallofthis123]

That makes sense 😅 I'll update them accordingly

[newtoallofthis123]

@Shinyzenith Updated the docs :)

[Shinyzenith]

Why did you remove logging from swhks?

[newtoallofthis123]

Ever since the command execution was taken out from it, the log file is no longer used in swhks. I can probably add it back and use it for logging when the environment is sent?

[Shinyzenith]

Also your commit messages are horrendous sadly. We need to rebase / work on getting those improved.

[newtoallofthis123]

Yeah I am very bad at commit messages 😅 I'll try rebasing and stacking them so that they make sense in a chronological way.

[newtoallofthis123]

Closed and ported to #270