Thread by @alvar_f

Habe mir den Datenbank-Code der Corona-Warn-App angeschaut. An den paar Zeilen kann man beispielhaft zeigen, was bei vielen Datenbank-Projekten nicht ganz optimal läuft. Ein Thread über Datenbank-Sicherheit, #SQL, die Corona-App und #PostgreSQL. #CoronaWarnApp #cwa 1/x

Kurzzusammenfassung: Die Datenbank-Berechtigungen sind viel zu weitgehend, ein erfolgreicher Angreifer könnte auf alle Daten zugreifen, löschen usw. Außerdem werden Datum und Zeit als Zahl gespeichert, unschön, denn es gibt einen TIMESTAMP-Datentyp! Und was ist mit der ID? 2/x

Bei den Berechtigungen könnte man einwenden: ein Angreifer darf nicht so weit kommen; aber das passiert leider immer wieder, und man kann sich mit einfachen Methoden schützen. Sieht auf den ersten Blick kompliziert aus, ist aber ganz simpel. Kommentare Willkommen! Also los: 3/x

Unter github.com/corona-warn-ap… sieht man die Abhängigkeiten und Default-Einstellungen der benötigten Dienste. Bei PostgreSQL fällt auf: Login per Superuser postgres (also Admin/root) ist via Netzwerk möglich. Sollte man nie machen, sondern nur via lokal erlauben. 4/x

Per Netzwerk sollten nur Nutzer mit eingeschränkten Rechten Zugriff haben, der Superuser nur vom lokalen Unix-User postgres. Passwortlos (trust auf local in der pg_hba.conf ist sinnvoll, ein Angreifer könnte das sowieso ändern), andere User nur per Passwort (scram-sha-256). 5/x

Die unter github.com/corona-warn-ap… genannten Default-Passwörter sind nicht gut. Erfahrungsgemäß werden Passwörter in Produktion oft nicht geändert. Besser: Passwörter wie „Change-me-in-Production-oder-Finger-ab“ sind eindeutig. Ja, das hilft – meistens 😱😜 6/x

Da die Anwendung einen Datenbank-Superuser erhält und keine Datenbank konfiguriert wird, legt sie diese wohl selbst an. Das ist schlecht: niemand, außer der DBA auf der Kommandozeile, sollte Superuser-Rechte haben. Man lässt heutzutage auch keinen Webserver als root laufen. 7/x

Ein erfolgreicher Angreifer kann sonst alles machen: alle Daten auslesen, löschen, verändern. Einfach alles. Stattdessen sollte jeder Teil der Anwendung je einen Datenbank Nutzer mit minimalen Rechten bekommen. Der cwa_inserter kann nur einfügen, der cwa_reader nur lesen. 8/x

Dazu später mehr. Die Datenbank sollte vom DBA mit Superuser-Rechten angelegt werden, per Default darf sich nicht jeder („public") damit verbinden:

CREATE DATABASE cwa;
REVOKE ALL ON cwa FROM PUBLIC;

9/x

Niemand (außer Superuser) kann sich nun mit der Datenbank cwa verbinden. Das ist gut, denn nun legen wir Nutzer an:

CREATE ROLE cwa_users;
CREATE USER cwa_reader IN ROLE cwa_user PASSWORD 'change-me-in-prod';
CREATE USER cwa_inserter IN ROLE cwa_user PASSWO[…];

10/x

Nun bekommen alle User mit der Role cwa_user die CONNECT-Rechte:

GRANT CONNECT ON DATABASE to cwa_users;

Die Tabelle (github.com/corona-warn-ap…) wird normal angelegt, aber …

11/x

Aber per Default hat auch wieder PUBLIC alle Rechte, also jeder kann lesen/einfügen/löschen. Weg damit:

REVOKE ALL ON diagnosis_key FROM PUBLIC;
REVOKE ALL ON diagnosis_key FROM current_user;

12/x

Nun wieder Rechte vergeben:

GRANT SELECT ON diagnosis_key TO cwa_reader;
GRANT INSERT ON diagnosis_key TO cwa_inserter;

Der _reader darf nur lesen, der _writer nur einfügen. Niemand darf etwas ändern oder löschen – der Superuser darf aber immer alles.

13/x

Das ist schon ganz OK, Verbesserung dazu später. Erst mal zur Tabellen-Definition von github.com/corona-warn-ap…: Die Spalte submission_timestamp soll wohl Datum&Uhrzeit enthalten. Als Bigint, also Zahl in Sekunden. Meist Doof. PostgreSQL kennt TIMESTAMP, das ist sinnvoller.

14/x

Denn Zeiten in Sekunden machen nur Ärger. Umrechnen ist schwer, es ist nicht menschenlesbar usw. Besser: „TIMESTAMP WITH TIMEZONE“. Damit kann man auch rechnen, z.B.:
SELECT age(submission_timestamp);
SELECT submission_timestamp - '1 day'::interval;

15/x

Außerdem: die Daten sollen nach einer gewissen Zeit gelöscht werden. Kann man machen mit:

DELETE FROM diagnosis_key WHERE age(submission_timestamp) > 30; -- alles nach 30 Tagen löschen

Das ist aber relativ langsam und zerstückelt die Datenbank, besser: Partitionierung.

16/x

Mit Partitionierung werden automatisch Partitionen (Tabellen) z.B. für jeden Tag angelegt, für den Anwendungs-Entwickler transparent. Beim z.B. täglichen Löschen müssen dann nur die einzelnen Tabellen entfernt werden. Das geht auch bei großen Datenmenken extrem schnell. 17/x

Dazu siehe die Dokumentation postgresql.org/docs/current/d…; mit der PostgreSQL-Extension pg_partman, siehe github.com/pgpartman/pg_p…, geht mehr und voll-automatisch. Da hier ja potentiell viele Daten da sind, verbessert Partitionierung die Performance wahrscheinlich sehr. 18/x

Interessant in der Tabellen-Definition: die ID ist ein BIGINT. Soll das einfach hochzählen? Macht das die Anwendung? AUTSCH! Nein, sowas muss die Datenbank selbst machen! Am besten: Datentyp SERIAL nehmen. Aber: eine hochzählende Zahl könnte ein Angreifer erraten. Hmmm … 19/x

Wenn die ID von einem User übergeben wird, dann kann u.U. ein Unberechtigter auf fremde Daten zugreifen. Schlecht. Besser: Datentyp UUID und zufällige ID gleich als DEFAULT-Wert vergeben! Sowas immer die Datenbank machen lassen, nicht die Anwendung, daher: Default setzen! 20/x

Zurück zu den Lese- und Schreibrechten. Der cwa_reader-User oben kann ALLE Daten einfach so auf ein mal lesen. Auch das ist ein Angriffspunkt. Besser: Funktionen mit minimaler Funktionalität erstellen, die unter mit passenden Rechten laufen aber nicht viel machen: 21/x

CREATE OR REPLACE FUNCTION get_key_data(in_id UUID)
RETURNS JSONB
AS 'SELECT key_data FROM diagnosis_key WHERE id = in_id;'
LANGUAGE sql SECURITY DEFINER SET search_path = :schema, pg_temp;

Mit dabei gleich noch die ID Änderungen und Daten als JSONB statt BYTEA …

22/x

Die oben bei 13 beschriebenen Berechtigungen fallen weg, dafür gibt es Zugriff auf die Funktion:

ALTER FUNCTION get_key_data(UUID) OWNER TO cwa_owner;
REVOKE ALL ON FUNCTION get_key_dataUUID) FROM PUBLIC;
GRANT EXECUTE ON FUNCTION get_key_data(UUID) TO cwa_reader;

23/x

Beim inserter sieht es dann ähnlich aus. Der User cwa_owner kommt noch dazu, der wird als Owner der Tabellen eingetragen. Durch die Definition von SECURITY DEFINER wird die Funktion als User cwa_owner aufgerufen, das darf aber nur cwa_reader. Der kann also nur einen […] 24/x

[…] einzigen Datensatz lesen. Sonst nix. Der Inserter kann nur einfügen. Prima – selbst ein erfolgreicher Angreifer kommt damit an der Stelle nicht weit. Aus 8 Zeilen CREATE TABLE wird da zwar viel mehr Code, aber es ist in der Anwendung simpel und eben deutlich sicherer. 25/x

Einziger Nachteil aus Anwendungsentwickler-Sicht: Komische Objekt-Relationale Mapper (ORM) mögen keine Funktionen, sondern wollen auf Tabellen arbeiten. Tja. ORMs machen aber in der Regel eh Mist, lieber SQL-Funktionen auf Methoden im Code mappen. Sauber, sicher, schnell. 26/x

Uuups, ist der Thread lang. Kommentare, Kritik, Ergänzungen willkommen! Ich mache dann bei Gelegenheit mal einen übersichtlicheren Blog-Beitrag draus. 27/x

Ach, noch ein Tipp: Solche CREATE-Statements wie oben am besten in mehrere Files für die psql-Shell packen; Dateinamen nummerieren: 03-variables.sql, 10-create-user.sql, 20-create-table-a.sql, usw. In install-all.sql werden alle Files per \ir included. psql ist sehr mächtig! 28/x

Hinweis für Journalisten: dies ist keine Kritik an den Entwicklern. Kein Streit. Sondern es sind in der Entwicklung von Open Source Software übliche Verbesserungsvorschläge, die auch wiederum neuen Verbesserungsvorschlägen unterliegen können. 29/ENDE

BUGFIX 1: Da bei #10

https://twitter.com/alvar_f/status/1267705330403860480

muss das natürlich „IN ROLE cwa_users“ (_users, nicht _user) heißen, denn eins drüber heißt die Role auch cwa_users).

Ergänzung am Abend: Wie man die Rechtetrennung mit Funktionen umsetzen kann, kann man auch an dem Beispiel da sehen: github.com/alvar-freude/P…