viczem / ansible-keepass

Ansible lookup plugin to fetch data from KeePass file
MIT License
116 stars 30 forks source link

Temp files should be in tmpfs #41

Open LecrisUT opened 1 year ago

LecrisUT commented 1 year ago

This one is actually important. If the power is cut and the drive is accessed by a different OS, then the attachments become exposed. Maybe memory-tempfile would be useful here.

https://github.com/viczem/ansible-keepass/blob/237f3a0da53d72b04f2d04bf960e61b7fc8fcd44/plugins/lookup/keepass.py#L349

viczem commented 1 year ago

You are right. But I would not like to add one more dependency. Maybe it would be good to add an optional argument for destination path to save an attachment file. If it not specified, it will be saved to a temporary directory as it was for backwards compatibility.

For example, attachment: "{{ lookup('viczem.keepass.keepass', 'path/to/entry', 'attachments', 'a_file_name', '/path/to/save') }}"

LecrisUT commented 1 year ago

I don't think the /path/to/save is much useful for the lookup. The user could use the module attachment for that.

In the backend, if we don't want to add more dependency, I would say we could try either /run or /var/run if they exist then check for [/var]/run/user/${uid} if it exists, then check if it is a tmpfs. If any of them fail we can then default to the native python temp dir and print a warning message. I believe most distros will support that.

viczem commented 1 year ago

I thought it best to leave it as is.

1) /tmp in many linux distributions by default has tmpfs 2) /run/user/${uid} directory is used by systemd, and not all linux distributions use it. For example, when run ansible in docker container based on Alpine 3) /var and other directories could be mounted from external source 4) there is no point to trust tmpfs because it could be mounted from an external source

So adding an optional argument for destination path as I mentioned above, is a best choice.

LecrisUT commented 1 year ago

I don't think /tmp defaults to tmpfs on non-docker environments. Here is my Fedora partition for reference:

$ df -hT
devtmpfs       devtmpfs  4.0M     0  4.0M   0% /dev
tmpfs          tmpfs      14G  796K   14G   1% /dev/shm
tmpfs          tmpfs     5.5G  2.2M  5.5G   1% /run
/dev/nvme0n1p9 btrfs     599G   87G  511G  15% /
tmpfs          tmpfs      14G  112K   14G   1% /tmp
/dev/nvme0n1p9 btrfs     599G   87G  511G  15% /home
/dev/nvme0n1p8 ext4      974M  320M  587M  36% /boot
/dev/nvme0n1p2 vfat      127M   51M   76M  40% /boot/efi

/run/user/${uid} directory is used by systemd

Indeed and it is a good reason why they do so. Various other components use it, e.g. here is another snapshot of mine:

$ ls /run/user/${UID}/doc/by-app/
chat.schildi.desktop  com.slack.Slack  org.zulip.Zulip  us.zoom.Zoom

You can find attachment files in there for example. Granted there is a limitation of the size allocated, but you should not have GBs of attachment files.

/var and other directories could be mounted from external source

The issue that can be mitigated is when the drives are extracted and re-mounted, e.g. the drives are sold after hardware upgrade. In this case you can trivially get root access and read any files. That would not be the case for the ram mounted tmpfs.

There are of course more complicated setups, e.g. in HPC /tmp can often be shared across compute nodes, MIM attacks on NFS mounts, etc.

I think it would be a good practice default to set it to some /run/user tmpfs over a less standardized /tmp. Of course we can have it fallback like in the docker setups where everything is already containerized.