I need a trusted C code reviewer. Reviewing at least for non-maliciousness.
It’s impossible to prevent bugs as it’s impossible to tell when something will fail if new code is added or code is removed, but I can definitely tell you personally and on Github Issues for those projects if C code is not cryptographically sound, i.e if it uses insecure PRNG or algorithms.
If it helps you get an idea, I’ve already analyzed the randomness of the bootclockrandomizer (bootclock is safe as it uses /dev/random and uses the shuffle feature of bash, both pretty fast and safe) and have begun analysis of kloak, and it uses libsodium in Pull Request #38 so I am not as worried if this will be included in Whonix, but I am still concerned about the fact that the current version on the Github page uses a insecure RNG that could still fingerprint users. I will still be on the lookout for bugs regarding the mouse obfuscation, but I wanted to just warn you about the RNG and how #38 fixes it depsite not being officially added.
A wholly different idea: Any reason this must be written in C? Could this be rewritten in python or something?
You are right that it could be written in Python if you value ease than being fast, and I have some very promising ways such as pyautogui and keyboard but these both have the consequence of having to be bundled within the python installation on the Whonix system. pyautogui uses some C modules, while keyboard only interacts with pure Python. I’ll definitely look into making it an independent project on my Github page and I fully consent for it to be FOSS for everyone including the Whonix project to use.
Any compile time hardening options seem unrelated and could be moved into its own pull request. These might be generally useful independently of Qubes support.
That is a lot of new C code for Qubes just for support. I am not sure it’s going to work to request upstream projects to add lots of Qubes specific code.
Is there a way to implement Qubes support without needing lots of Qubes specific code in kloak?
Hmm, I was under impression it expect X server input device, not not Linux kernel input device. Qubes GUI agent do not expose the later. It shouldn’t be hard to change that, but still it is some code change. Namely, rewrite this function (feed /dev/uinput instead of X input device).
That’s odd. I used the GUI tool to convert my Whonix distro to the developer repo (to test the new kloak and test for any bugs) and my kloak distribution hasn’t changed?
Yeah there is a lot of other stuff I added in there after the qubes support was done. There were some other features I figured might be nice to have in kloak as well and since it was taking a little while to either be approved or denied, I figured I would get it in in that pull request as well instead of a different pull request for each one. It may be better to move the other changes to another one though. That was my first pull request I’ve ever submitted, so I wasn’t sure what the best practice was.
I almost have this ready to make a pull request, but in the templates, there will need to be a few changes to use it.
user will need to be added to the input group
the environment variable CREATEINPUTDEVICE will be need to be set to 1 before the gui-agent loads
The first change is needed because xinit is running as user and when the input device is created by the qubes gui-agent, the device has a user owner of root and group owner of input, so xinit won’t be able to use the device without the group add.
The second change is needed to make it opt-in so input devices are only created if it’s specifically requested.
I should be able to make a pull request by the end of the day Sunday.
Don’t know about this. This needs to be discussed with Qubes.
Also don’t know if or how Qubes implementation of sys-gui (GUI domain) will interact with this.
This one too.
If needed, then Whonix-Workstation could ship a systemd drop-in file for Qubes gui-agent which sets this environment variable. An alternative mechanism could be a status file.
That’s really good to know, that section checking for the environment variable was only a few lines, so it’s easy to change. I tested checking to see if the /etc/whonix_version file exists and creating it by default in whonix and that works as well.
Is the /etc/whonix_version file likely to always be there across versions? Based on the contents it looks like it, but wanted to make sure. If not, what would be the easiest method for you to implement within the whonix template to indicate that an input device should be created? I’ll design the opt-in around that.
If checking for the existence of /etc/whonix_version is likely to always work between versions, I’ll figure out another opt-in method for other qubes and just always have it create the device for whonix.
marmarek had mentioned in a thread a while ago that it should be limited to Whonix VMs and they may package it as an optional dom0 package. I wasn’t sure if that was specifically referring to kloak, or the input device creation as well. The easier option would be to always have it created and if qubes is open to that, I can have it do that.
But best avoided if upstream (Qubes) doesn’t request this.
Also I guess it should be a neutrally named Qubes status file. Maybe a Qubes service | Qubes OS file. But that’s also only something that upstream can answer.
