louislam / uptime-kuma

A fancy self-hosted monitoring tool
https://uptime.kuma.pet
MIT License
59.63k stars 5.33k forks source link

Database is stored without encryption at rest #4784

Closed Tchebychev closed 5 months ago

Tchebychev commented 5 months ago

DO NOT PROVIDE ANY DETAILS HERE. Please privately report to https://github.com/louislam/uptime-kuma/security/advisories/new.

Why need this issue? It is because GitHub Advisory do not send a notification to @louislam, it is a workaround to do so.

Your GitHub Advisory URL: https://github.com/louislam/uptime-kuma/security/advisories/GHSA-5hjg-3v4v-3cx7

louislam commented 5 months ago

The advisory is closed, but I think it is a good question, I make it public to see what other people think.

@Tchebychev is concerning plaintext storage of sensitive information inside the SQLite database like PostgreSQL monitor's password.

But I didn't encrypt them, because I think encrypting them is meaningless, as the encryption key will be in the same data directory. If an attacker can read your SQLite database, they can probably get the encryption key to decrypt the information too.

Unlike user password hashing (one-way hashing), passwords can still be verified after being hashed. But for a PostgreSQL password, Uptime Kuma would have to decrypt it to connect to the PostgreSQL database. That is the difference.

louislam commented 5 months ago

From the advisory:

PoC I carried out this test in a Pod running on Openshift: Connection to the uptime kuma Pod terminal sqlite3 /app/data/kuma.db select * from monitor; This command displays this:

From another perspective, a lot of deployment methods like .env, docker-compose.yaml or hardcoded password inside a source code program are also in plain text.

So, I think we need a strong reason why storing a plain text password in a .env file is considered acceptable, but Uptime Kuma is not ok.

CommanderStorm commented 5 months ago

I have writtten my take on this one here: https://github.com/louislam/uptime-kuma/issues/4778#issuecomment-2124913962

In v1, the performance budget is too tight to include this. In v2, you can use MariaDB and https://mariadb.com/kb/en/data-at-rest-encryption-overview/ to achieve this. Please see #4500 for the status of said release and what needs to happen before we can publish a beta.

Enabling the SQLite crypto extension https://www.sqlite.org/see/doc/trunk/www/readme.wiki does not seem worth dev-time as I don't think this would help with actual security that much.

Think this resolves your request. If you have insights of how SQLITE-SEE works and if this is simple to configure/maintain and performant enough (provide a benchmark ^^), we can reopen the issue.

=> lets continue any discussion there

thielj commented 5 months ago

@louislam

But I didn't encrypt them, because I think encrypting them is meaningless, as the encryption key will be in the same data directory. If an attacker can read your SQLite database, they can probably get the encryption key to decrypt the information too.

That's not how you do it, and also not why you do it. Unless you have a hardware key storage, an encryption key or seed is provided through some other means (secrets, environment) and never persisted to disk and ideally stored in memory that isn't swapped out.

The encryption protects plaintext or weakly hashed passwords, connections strings, authorization headers, etc. A docker volume mounted to store the DB for example gets detached and lives on until it is destroyed. Even then the data is probably still on it and can be scanned for valuable information.

Or consider the case of an exploit enabling an attacker to download the database. It doesn't even have to be a vulnerability in Uptime Kuma; an attacker might get access to storage volumes through other means, without actually having the key information.

And yes, someone who has compromised your running container will have access to it. At this point, you have a much bigger problem already.

Unlike user password hashing (one-way hashing), passwords can still be verified after being hashed. But for a PostgreSQL password, Uptime Kuma would have to decrypt it to connect to the PostgreSQL database. That is the difference.

Symmetric decryption is extremely fast, and it needs to be done only once when the monitor configuration is loaded.

CommanderStorm commented 5 months ago

Plese respect my comment from above, lets continue the discussion in https://github.com/louislam/uptime-kuma/issues/4778

thielj commented 5 months ago

You have closed both this issue and the other as "completed" ¯_(ツ)_/¯

And louislam explicitly wanted to know what other people think.

The advisory is closed, but I think it is a good question, I make it public to see what other people think.

CommanderStorm commented 5 months ago

You have closed both this issue and the other as completed

I think I have made my point that

Discussing stuff in two issues simultaneously does not really have a point => lets continue the discussion in https://github.com/louislam/uptime-kuma/issues/4778 If you really want we can make that into a discussion issue..