wir haben gestern den Alarmiator in unserer Organisation ausgerollt. Vor dem Ausrollen hatten wir eine vierwöche Erprobungsphase mit ca. 20 Benutzern.
Bei der gestrigen Einführungsveranstaltung ist was fürchterliches passiert:
Alle Anwender haben mehr oder weniger gleichzeitig in der App (egal ob iOS aoder Android) das s.g. Profil bzw Dienstbuch geöffnet und plötzlich die Daten anderer Mitglieder erhalten. Ein absolutes Datenschutzdesaster und Peinlichkeit bei einer Produktivnahme…
Wir verwenden den Alarmiator in einem Dockercontainer und NGinx als Reverse-Proxy. Es findet auf dem Weg zum Alarmiator-Container keinerlei Caching o. ä. statt. Das haben wir nochmal genauer untersucht.
Mir blieb nicht anderes übrig, als den Pfad zum „Profile“-Endpoint im Reverse-Proxy heraus zu filtern / zu sperren.
Noch eine Sache: Die Request-URL für die Profildatenabfrage erhält über Query-Paramter die E-Mail-Adresse und den Passwort-Hash… IMHO hat Letzteres in der App nichts verloren. Da gibt es sicherlich bessere Lösungen für ein Auth-Token… z.B. eine UUID als API-Key im Header.
es wäre super, wenn du auch mal ohne Proxy testen könntest, ob das Problem mit den Profilen reproduzierbar ist (das werde ich auch mal tun). Das darf nämlich wie du sagst auf keinen Fall sein und wir wollen definitiv schauen wo das passiert. Welchen Reverse Proxy hast du denn? Du sagst ja, dass du Caching überprüft hast aber am „Sichersten“ wäre es das nochmal zu überprüfen ohne Proxy. Dann eben per http mit einem Skript oder mehreren Anwendern wie gestern.
Bezüglich des Basic Auth. Ja da haben wir bereits eine Änderung in der Pipeline. Die App kann mittlerweile Public Key und übermittelt dem aktuellen Dev Server den Public Key einmal (mit diesem Hash wie aktuell) und nutzt fortan nur noch signierte Tokens, um zu zeigen, dass es wirklich die App ist.
Das haben wir also auf dem Schirm. Serverseitig liegen die Passwörter mit Argon2id dem aktuell empfohlenen Passworthashing von OWASP in der DB gespeichert und mit Public Key soll zukünftig auch die Handyapp schöner gelöst sein.
Man muss hier aber auch sagen, dass jemand dein Handy klauen müsste und den Hash rückrechnen muss um ans Plaintext Passwort zu kommen oder du musst http nutzen und jemandem im Netz haben der Snifft und so den Hash sieht. Es ist also auch wenn wir das aktuell umstellen, weil wie du selber gesehen hast, es nicht optimal ist, noch einiger Aufwand damit verbunden da ran zu kommen und ich würde behaupten im aktuellen Threat Model von meiner Organisation klaut keiner das Handy und versucht da aus der App meinen Hash zu finden (zumal das Handy gerootet sein müsste).
Wie du aber sagst ist das nicht optimal und deswegen haben wir das bereits angefangen umzubauen und es ist so schon im Dev Stand.
Die Problematik ist aber natürlich bei uns auch, dass wir nicht zentral einfach ein paar Feature Switches aktivieren und alle haben nun eine neue Version. Deshalb wird da auch die Umstellung für alle ein wenig dauern. Du kannst das aber dennoch natürlich dann direkt bei dir im THW nutzen, sobald es released ist oder bereits jetzt mit dem Dev Container (latest-beta auf Dockerhub) und App Version 1.6
ja, ein Skript zum Reproduzieren will ich auf jeden Fall noch machen und euch gerne bereitstellen. Ich kann mir beim besten Willen nicht vorstellen, wie sowas passieren kann.
Was wir noch rausgefunden hatten war, dass die Rückmeldungen zu einem Alarm nicht von dem Problem betroffen waren. Also man hatte nicht plötzlich eine Rückmeldung für einen Benutzer abgegeben, sondern weiterhin für seinen eigenen.
ich hab mal mit folgendem Python-Skript versucht sowohl direkt gegen den Container, als auch über den Reverse-Proxy das Verhalten zu reproduzieren. Hatte damit bisher aber keinen Erfolg…
import asyncio
import aiohttp
async def send_request(session, url):
try:
async with session.get(url["url"]) as response:
status = response.status
data = await response.text()
if url["name"] in data:
print(f"ok: {url['name']}")
else:
print("peng!")
return status, data
except Exception as e:
return None, str(e)
async def send_requests_concurrently(urls, num_requests_per_url):
async with aiohttp.ClientSession() as session:
tasks = []
for url in urls:
tasks.extend([send_request(session, url) for _ in range(num_requests_per_url)])
results = await asyncio.gather(*tasks)
return results
async def main():
#baseUrl="https://foo.bar"
baseUrl="http://localhost:5010"
urls = [{"name": "Musterfrau", "url": f"{baseUrl}/inapp/dashboard/profile?username=maxi.musterfrau&passwordHash=eb7e3f3401a30b5bfe167ca1d9ee5085"},
{"name": Mustermann", "url": f"{baseUrl}/inapp/dashboard/profile?username=max.mustermann&passwordHash=8aaa942d30289936eb4afd9429295cf4"},
{"name": "Dingsbums", "url": f"{baseUrl}/inapp/dashboard/profile?username=dana.dingsbums&passwordHash=efacb874547e16bbade3db47950b14a3"}]
num_requests = 20
repetitions = 5
for i in range(repetitions):
print(f"Durchgang {i + 1}:")
results = await send_requests_concurrently(urls, num_requests)
for idx, (status, data) in enumerate(results):
if not status:
print(f"Anfrage {idx + 1}: Fehler {data}")
print("\n")
if __name__ == "__main__":
asyncio.run(main())
Cloudware etc haben / nutzen wir nicht.
Alle Anwender waren im gleichen Raum und im Handynetz eingebucht. Könnte eine Shared-IP so ein Verhalten erklären?
Ich hab mir den Code dazu noch nicht wkrlich tief angesehen. Wie ‚stateless‘ ist denn der Endpoint? Ich sah irgendwo eine sqlite db Namens „sessions.db“. Kommt die nur für die Webseite zum Einsatz?
ich habe gleichzeit das Skript von einem weiteren Server, dem Server auf dem der Container läuft und meinem Rechner ausm Heimnetz laufen lassen. Keine Reproduktion…
Ich vermute nicht, dass Caching das Problem ist.
Vllt müssen wir die Situation von gestern wieder nachstellen. Also mit 10 oder mehr Smartphones gleichzeitig mehrmals die Profile abrufen und das irgendwie mitschneiden…
Nein auf die IP wird nicht geschaut. Jeder Nutzer authentifiziert sich ja mit seinen Basic Auth Credentials und diese werden benutzt, um den User zu verifizieren und seine Nutzerdaten zu holen. Die sessions.db ist für die Sessions im Admin UI auf Port 5000. Hier kriegst du ja nach Login einen Cookie. Hat also nichts mit der Api hier zu tun.
Deshalb ist mir auch unklar, wie es passieren sollte (ohne caching auf einer Ebene zwischen Alarmiator und den Handys, dass jemand die generierte Seite für jemand anderen erhält.
Ich möchte mich hier auch entschuldigen, dass ich euch nicht erst per E-Mail kontaktiert, sondern das alles hier direkt offen ins Forum geschrieben habe. Erst jetzt kam mir die Erkenntnis, dass euch das unnötigt unter Druck setzt und auch das Vertrauen in die Anwendung beschädigen kann. Das war nicht meine Absicht und ich hoffe ihr seht es mir nach.
ok, wir können es nun reproduzieren. Meine Kamerad Christian gab den entscheidenen Hinweis.
Ich habe den if block in ‚send_request‘ geändert in
if f"https://foo.bar/inapp/dashboard/profile?username={url['name']}" not in data:
print(f"peng! >> {data}")
und in der main-Methode im urls-Dict anstatt den Nachnamen den Username eingetragen.
Es passiert folgendes:Wenn ich das Profile aufrufe,erhalte ich die korrekten Daten.Allerdings sind die Links falsch!Sie enthalten den Username und den Hash eines anderen Users.Klickt man auf diese Links,erhält man entsprechend die Daten des anderen Users.Das ist auch ziemlich random:immer wieder ein anderer User.Das gilt für den Link zum Dienstbuch,Alarmeinstellung und Webhooks.
Dank dem Skript wurde der Fehler schnell gefunden. Es handelt sich hier um eine Race Condition in der Template Engine die wir nutzen. Dieses Verhalten ist als Entwickler schwer zu finden, da alles funktioniert wie es soll, bis 40 Leute auf einmal die gleiche Seite aufrufen und die Template Engine dann Nutzernamen mischt.
Wir sind deshalb sehr froh darüber, dass wir schon seit einiger Zeit daran arbeiten ein moderneres Frontend und Backend mittels Rest-API und Angular bereitzustellen, welches deutlich schöner getrennt ist und wartbarer ist.
Danke an Michael für die Detektivarbeit und das Nachstellen. Das Problem ist bereits gefixt für die Inapp Renderings und wird in der nächsten Version dementsprechend behoben sein.
Außerdem werden wir die nächsten Tage einen neuen Beta Stand auf Dockerhub releasen für alle, die diese Änderung bereits vor dem offiziellen Release haben wollen.