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

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

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

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

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

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