MichaIng / DietPi

Lightweight justice for your single-board computer!
https://dietpi.com/
GNU General Public License v2.0
4.81k stars 494 forks source link

DietPi-Banner | Enhance security when loading MOTD #4545

Open palmface2 opened 3 years ago

palmface2 commented 3 years ago

I was looking at the dietpi source code and found some things that peaked my interest.

Firstly, a Cron job runs every single day, which downloads apt updates and fetches the MoTD shell script from their servers which gets stored on disk. When a user logs in through ssh for example, that MoTD file gets sourced/executed, which possibly means they can put any code they want in the MoTD file and your system will execute it

Here is where the MoTD file gets sourced: https://github.com/MichaIng/DietPi/blob/master/dietpi/func/dietpi-banner#L228 Here is where the MoTD file gets downloaded inside the daily Cron job: https://github.com/MichaIng/DietPi/blob/master/rootfs/etc/cron.daily/dietpi#L42

Secondly, DietPI downloads a kernel module from a random AWS IP. That kernel module is pre-built and is sometimes modprobe'd. Here is where that kernel module is downloaded: https://github.com/MichaIng/DietPi/blob/112e56c836a34ea70ddf11f89ac5a03253258271/dietpi/patch_file#L2566

Should I be concerned? Has anyone else seen this? Is this of no concern and I'm just overlooking something extremely obvious?

Joulinar commented 3 years ago

Hi,

There is no need to concern. The MOTD is just displaying actual information we like to share. As well we use it to apply some hotfix if needed (extremely rare case). This function is not hidden as it is available on GitHub for everybody to check. As well the content of the MOTD file can be checked by everyone.

The code line you mentioned is a download of an Allo driver. This is the regular Allo download page. The download and Allo driver installation will be done during DietPi update only. This is not hidden as well and clearly shown during the update process.

palmface2 commented 3 years ago

There is no need to concern. The MOTD is just displaying actual information we like to share. As well we use it to apply some hotfix if needed (extremely rare case).

I understand that the DietPi team has no ill intentions, but just the fact that a malicious or potentially broken payload can be pushed to all devices running DietPi doesn't sit right with me - what if the server gets compromised or government agencies *puts on tinfoil hat* get involved?

I wish there was a clearly comprehensive TUI modal in the installer which describes the risks and the benefits, while easily allowing you to disable it.

I also never mentioned that these code snippets/behaviors were hidden inside the source code, as a matter of fact, they can be very easily seen on github...

The code line you mentioned is a download of an Allo driver. This is the regular Allo download page.

I understand that it's a driver download, however, could a comment perhaps clarify better to whom the IP belongs to and where it came from?

MichaIng commented 3 years ago

The concerns are generally valid regarding the MOTD. While we currently use this ourself for single line sed hotfixes, where applicable, I want to replace this with a more transparent live-patch script so that the banner shows that there are patches available for the current release, and allows to review and apply them individually. The MOTD file can at latest then be parsed instead of sourced.

Btw, the MOTD can be disabled via dietpi-banner.

Regarding the Allo driver: This is only applied on Raspberry Pi CM3, which can be used in an Allo USBridge Sig, which requires this driver to make Ethernet and audio work together. A native Raspberry Pi kernel solution is still outstanding:

I already asked Allo whether this driver can be provided via HTTPS and an ordinary domain instead, but didn't get an answer on that so far. When it is installed, however, there is some console output about what it is from and for, isn't it?

Joulinar commented 3 years ago

You could have it simply disabled if you don't like the MOTD. It can be done using dietpi-banner. This way the information will be still stored but not displayed/executed. In fact the MOTD is a simple text file that will be displayed and not some complex code to be executed.

MichaIng commented 3 years ago

This way the information will be still stored but nor displayed/executed.

The file is not even downloaded in this case 😉: https://github.com/MichaIng/DietPi/blob/master/rootfs/etc/cron.daily/dietpi#L39-L43

palmface2 commented 3 years ago

The file is not even downloaded in this case

Nifty! Didn't dig deep enough to find how to disable it...

The main issue is transparency - didn't know about this 'till I decided to look in the code.

I already asked Allo whether this driver can be provided via HTTPS and an ordinary domain instead, but didn't get an answer on that so far.

If I'm not overlooking anything, the driver is available @ https://github.com/allocom/USBridgeSig/tree/master/ethernet Maybe it could be built and placed as a tarball on the dietpi website, and then later downloaded with HTTPS by the script if the user consents to it.

Joulinar commented 3 years ago

This means we would need to watch Allo and duplicate the driver on our side. This doesn't sound efficient.

Options for dietpi-banner are displayed on our online docs https://dietpi.com/docs/dietpi_tools/#dietpi-banner

palmface2 commented 3 years ago

This means we would need to watch Allo and duplicate the driver on our side. This doesn't sound efficient.

Ah ok; got it.

Options for dietpi-banner are displayed on our online docs https://dietpi.com/docs/dietpi_tools/#dietpi-banner

That still doesn't seem enough to me (I used dietpi for the better part of 2 years without ever noticing it...) - as I said, a TUI modal explaining the risks and benefits clearly to the user with a button to disable it and one to enable it at first install would be the best choice. Same with the Allo drivers maybe?

MichaIng commented 3 years ago

In theory we could build the Allo driver ourselves (I did once to test a fix), but actually Allo does a great job to do this for every RPi kernel version once available, even the ones from master branch (e.g. when someone uses rpi-update). So indeed it's overhead, and also Allo does not update their sources in every case (not sure here, but I remember in other cases), so using their builds is the only way we know that we have the intended and compatible driver. I'll ask them once again to host it on a domain with HTTPS access, e.g. also GitHub would work just fine. That would not only be an improvement, security-wise, for our users, but also for other Raspberry Pi + USBridge Sig users, using Raspberry Pi OS, Debian or Ubuntu.

One issue with the MOTD script is, and the initial reason to source it instead of parsing it, was that this way we can provide information targeted to specific boards, versions, with certain software titles installed etc. We don't use it often that way, but there were e.g. important information about grub upgrades on x86 (leading to an unbootable system in cases), security hints for Nextcloud and Pi-hole users, and Raspberry Pi specific notes, I remember. Of course this could be solved differently, by using and parsing certain text markers matching the condition, but using the shell definitely makes this much easier and much more flexible.

I thought about the security impact before, but accepted it taking into account that most installers are just provided the same way as raw scripts, intended to be piped into a shell or python, and the same risk applies to many software-internal updaters etc. The difference is that in case of the MOTD it's not a one-time install script or an updater (which is not always but often used interactively, and often with limited permissions) but downloaded every night and sourced on login as well as root user.

We'll discuss this and see whether we find an alternative safer solution. One idea I just have is to host the MOTD on GitHub as well (a dedicated repo), where it's more transparent and the servers supposed to be harder to compromise? But it's not 100% safe either, as someone could hack my account instead. That's actually easier than hacking our server, now that I think about it, password vs SSH key 😄.

MichaIng commented 3 years ago

First part done by decoupling live patches from MOTD but having them shipped with the repository version file: https://github.com/MichaIng/DietPi/blob/master/.update/version

MichaIng commented 2 years ago

Btw, in the meantime the MOTD is provided by a Cloudflare worker and not loaded from our server anymore. So instead of hacking our server, not Cloudflare, respectively our account need to be hacked to manipulate it. Not sure whether this is an improvement or not 😅.

I had another idea that the file could be sourced, or the whole banner script executed as user nobody to limit the possibility it has. Generally this should work fine. Only problem is that users which are not in sudo group and don't have a sudoers entry cannot call sudo, and members of sudo group and without NOPASSWD: defined, need to enter their password interactively, and su does not work as it would require the nobody account to have a password, which would then need to be entered. So I don't know an assured non-interactive solution for this.

Best would be some kind of sandboxing or limiting the user/group permissions for a single command, deny all filesystem access or similar, but I'm not aware of a possibility for this currently. Probably chroot would work, but the environment needs to be preserved.

fidesachates commented 8 months ago

Thanks for the updates on this. I had very similar concerns when I noticed a christmas tree on my diet pi instances (nothing against the holidays, just security concerns). I appreciate that this thread is still open; I personally would have liked this highlighted more promeniently as a security setting in docs or installation so I know to turn this off, however that may not be the best solution for most people.