debian review of this software

[anarcat]

Hi,

As part of the process of including this software into the Debian project, I have performed a small review of the software, which I copy here as I figured you might find it interesting and cross-reference issues i have opened here during that process. Feel free to close that issue silently whenever, a copy will be available in BTS #873955.

At first look, the program looks well-written. It uses sqlalchemy for storage in a SQLite database and while it doesn't have any unit tests, it looks like it has a sound design. I didn't review the cocoa or Windows code (specifically sniff_cocoa.py and sniff_win.py), so the following applies only to the X version.

Entries can be encrypted in the database. The program uses Python's getpass module to prompt for passwords on the commandline and Tk's Entry element in the GUI, which probably leaks the password all over the memory space (and may show it to the user). The pinentry program should be used instead.

But That password is problematic in the first place since you need to either type it every time you start the program (which is annoying) or hardcode it on the commandline (which is insecure) or the config file (which is pointless). A better approach would be to use OpenPGP to encrypt the database - filed an issue (#155) about this.

Furthermore, Blowfish to encrypt data stored in the database. Because of its 64-bit blocksize, Blowfish is considered problematic for large file encryption as it's vulnerable to birthday attacks. Furthermore, MD5 is used to derive a key from the user password, which also shows its age. A better approach would be to use a more standard SQLite encryption approach like SQLcipher. An ideal design, in my mind, would be to have an AES-encrypted SQLite database with a strong key encrypted with a user password using a proper key derivation function (KDF) like Argon, scrypt or PBKDF2 (probably in that order). Also filed an issue (#159) about this.

So that's for the security of the encrypted storage: basically, it's showing its age, but should be good enough for casual attackers. As I mentioned before, a better option would be to not store keystrokes text by default and explicitly force the user to enable this by hand if they really want a keylogger running at all time. I have also made a PR (#158) for that feature.

The keylogger itself uses what looks like Python's Xlib and relies on the RECORD extension which will stream all keystrokes for all clients to the application, regardless of whether or not keylogging is enabled. This will force administrators to enable that possible security liability on the X server in order for users to use this app, but that's a fundamental limitation of X more than an issue with this particular app.

Finally, while I cannot vouch for the software without a more thorough review, I can say that I have read through the code and didn't find any obvious "leakage" out of the SQLite storage. It doesn't look like the program sends keystrokes on the network or publicly-readable files.

As for Debian packaging, that should be fairly straightforward: all dependencies (lockfile, sqlalchemy, keyring, xlib, although the latter is missing from requirements #157) are already in Debian and it's a fairly normal Python program. I have filed a PR to remove the Makefile (#156) to avoid possible debhelper confusion.

I'll be using the program a little more to figure out if there are any other gotchas and upload if all is well.

Thanks for finding that awesome software. :)