Skip to content

Commit

Permalink
Merge branch 'grabowski/last-changes-to-documents'
Browse files Browse the repository at this point in the history
* grabowski/last-changes-to-documents:
  submissions: add all docs for the third iteration deliverable
  fix a few document errors
  fix typos
  docs: fix external document references
  fix a few typos
  • Loading branch information
eloquenza committed Dec 9, 2018
2 parents 838dc1a + 5d10f1b commit 4b494db
Show file tree
Hide file tree
Showing 14 changed files with 43 additions and 55 deletions.
10 changes: 5 additions & 5 deletions docs/architecture.tex
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ \section{CSRF-Tokens}

\section{Play Cookies}
\label{play:cookies}
Das Play Framework nutzt zur Persistierung eines Cookies in dem Client-Browser das \enquote{\gls{jwt}}-Format.
Ein \gls{jwt} besteht aus 3 Teilen: einem Header, einer Payload sowie einer Signatur, siehe \ref{jwtformat}.
Das Play Framework nutzt zur Persistierung eines Cookies in dem Client-Browser das \enquote{JSON Web Token}}-Format.
Ein JWT besteht aus 3 Teilen: einem Header, einer Payload sowie einer Signatur, siehe \ref{jwtformat}.
Jeder dieser Teile ist ein namenloses JSON-Objekt. \\
Innerhalb des Headers ist gelistet, welcher Algorithmus genutzt wurde, um das JWT zu signieren.
Die Payload ist ein einfaches JSON-Objekt, in welchem Daten gespeichert werden können, die zu signieren sind.
Expand Down Expand Up @@ -217,7 +217,7 @@ \section{Session-Konzept}
Die Annotation bewirkt, dass der korrospondierende Request \underline{vor} Erreichen der dazugehörigen Controller-Methode abgefangen wird und einer Prüfung unterzogen wird, ob der Absender authentifiziert ist. Ist dies nicht der Fall, wird die weitere Ausführung abgebrochen und der Nutzer auf die Login-Seite weitergeleitet. Ansonsten wird der Request an den entsprechenden Controller weitergereicht.

Zur Umsetzung dieser Funktionalität setzen wir auf dem Session-Konzept von Play auf.
Initialisiert ein Nutzer eine neue Session, so persistieren wir seine IP-Adresse, seine User-ID, das Initialisierungsdatum und eine kryptografisch sicheren \enquote{\gls{uuid}} als Identifikator dieser neuen Session in unserer Datenbank. \linebreak
Initialisiert ein Nutzer eine neue Session, so persistieren wir seine IP-Adresse, seine User-ID, das Initialisierungsdatum und eine kryptografisch sicheren \enquote{Universally unique identifier (UUID)} als Identifikator dieser neuen Session in unserer Datenbank. \linebreak
Um diese Session sicher beim Client speichern zu können, verwenden wir das bereits genannte Cookie \textit{PLAY\_SESSION}.
Innerhalb des Payloads dieses JWT befindet sich ein JSON-Objekt namens \enquote{data} hinterlegt, zu welchem man weitere Applikationsdaten als Schlüssel-Wert-Paare hinzufügen kann.
Das machen wir uns zu eigen und persistieren das Schlüssel-Wert-Paar (\enquote{HsHSession}: \enquote{UUID}), durch welches wir den Nutzer authentisieren können.
Expand Down Expand Up @@ -249,7 +249,7 @@ \section{Verschlüsselung von Netzwerkdienst-Zugangsdaten}

Als Chiffre wird AES-256 im CBC Modus mit PKCS5 Padding verwendet. Dazu gibt es eine native Java Implementierung.

Um aus dem Passwort des Benutzers einen kryptografisch sicheren Schlüssel zum verschlüsseln des CredentialKey zu generieren wird PBKDF2 benutzt. Die Rundenzahl beträgt 65536. Der Salt ist 8 byte Lang und wird mittels SecureRandom generiert. Auch dazu gibt es eine native Implementeriung in Java.
Um aus dem Passwort des Benutzers einen kryptografisch sicheren Schlüssel zum verschlüsseln des CredentialKey zu generieren wird PBKDF2 benutzt. Die Rundenzahl beträgt 65536. Der Salt ist 8 byte Lang und wird mittels SecureRandom generiert. Auch dazu gibt es eine native Implementeriung in Java.

\section{Eingabevalidierung}
Für einige Parameter gibt es festgelegte Constraints, die in allen entsprechenden Dtos verwendet werden. Das sind:
Expand Down Expand Up @@ -403,7 +403,7 @@ \section{Passwort-Zurücksetzen durch unmittelbares setzen eines neuen temporär
In Hinblick auf die von uns gewählte Verschlüsselungsmethode für Netzdienst-Zugangsdaten haben wir diesen Ansatz verworfen: Das Secret hierfür ist faktisch das Klartext-Passwort des Benutzers. Eine Funktionalität, die jedem offen steht und das Passwort des Benutzerkontos ändert, führt in diesem Kontext zu Problemen: Ein Nutzer kann sich zwar noch einloggen, jedoch nicht mehr seine Zugangsdaten entschlüsseln.

Aus diesem Grund haben wir uns dazu entschlossen, die andere Variante der Anforderungsbeschreibung zu implementieren.


\printbibliography

Expand Down
14 changes: 7 additions & 7 deletions docs/quality-assurance.tex
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ \subsection{Funktionale Tests}
Hierbei haben wir darauf geachtet, dass es uns durch \enquote{Dependency Injection} möglich ist, die Abhängigkeiten einer Klasse durch \enquote{Mocks} zu ersetzen.
Dadurch wird garantiert, dass lediglich das Verhalten einer Klasse getestet wird, und dass dieses Verhalten nur auf dem durchs Interface vorgeschriebene Verhalten seiner Abhängigkeiten basiert.

\externaldocument[Arch1-]{architecture}
Für die in dem Klassendiagramm (siehe Architekturbeschreibung \ref{Arch1-architecture:class_diagram} \& \ref{Arch1-architecture:class_diagram2}) aufgezeigten \enquote{Pakete} werden folgende Voraussetzungen sowie Nachbedingungen getestet:
\externaldocument{architecture}
Für die in dem Klassendiagramm (siehe Architekturbeschreibung \ref{architecture:class_diagram} \& \ref{architecture:class_diagram2}) aufgezeigten \enquote{Pakete} werden folgende Voraussetzungen sowie Nachbedingungen getestet:

\begin{itemize}
\item Controller
Expand Down Expand Up @@ -133,9 +133,9 @@ \subsubsection{Race Condition bei Einzigartigkeit der Dateinamen}

Mit diesem Skript wurde es offensichtlich, das unsere Maßnahmen nicht ausreichend waren, um zu garantieren, dass der Dateiname für jeden Nutzer nur einmal vorkommen kann. Es konnte sogar passieren, dass alle 4 parallelen Anfragen erfolgreich waren.

Das Prüfen ob eine Datei für einen Benutzer mit einem bestimmten Namen bereits existiert und das Anlegen der Datei, falls das nicht so ist findet in einer Datenbank-Transaktion statt. Als Isolationslevel wurde \texttt{serializable} gewählt, was genau so einen Fall verhindern sollte. Die Prüfung ob eine Datei bereits existiert ist ein \texttt{SELCET} Query. Existiert eine Datei nicht, liefer es keine Daten zurück. Um sicherzustellen, dass nebenläufig ausgeführte Transaktionen wirklich \texttt{serializable} ablaufen, müsste H2 bereits bei diesem \texttt{SELECT} eine exklusive Sperre auf die Tabelle vergeben oder ähnliche Maßnahmen ergreifen. Offensichtlich ist das nicht der Fall.
Das Prüfen ob eine Datei für einen Benutzer mit einem bestimmten Namen bereits existiert und das Anlegen der Datei, falls das nicht so ist findet in einer Datenbank-Transaktion statt. Als Isolationslevel wurde \texttt{serializable} gewählt, was genau so einen Fall verhindern sollte. Die Prüfung ob eine Datei bereits existiert ist ein \texttt{SELECT} Query. Existiert eine Datei nicht, liefert es keine Daten zurück. Um sicherzustellen, dass nebenläufig ausgeführte Transaktionen wirklich \texttt{serializable} ablaufen, müsste H2 bereits bei diesem \texttt{SELECT} eine exklusive Sperre auf die Tabelle vergeben oder ähnliche Maßnahmen ergreifen. Offensichtlich ist das nicht der Fall.

Es stellte sich heraus, das H2 das Setzen von Isolationsleveln für Transaktionen ignoriert, wenn \texttt{MVCC} aktiviert ist\footnote{Seit H2 Version 1.4.x ist das die Standardeinstellung. Siehe http://www.h2database.com/html/advanced.html\#mvcc}. Im \texttt{MVCC} Modus vergiebt H2 exklusive Sperren nur in Ausnahmefällen\footnote{Einer dieser Fälle ist \texttt{SELECT ... FOR UPDATE}. In diesem Fall hilft \texttt{FOR UPDATE} aber nicht, da das \texttt{SELECT} Query keine Zeile selektiert.}. Durch das Deaktivieren von MVCC verhalten sich die Transaktionen wie erwartet und des Erstellen von Dateien mit dem gleichem Namen ist nicht mehr möglich.
Es stellte sich heraus, das H2 das Setzen von Isolationsleveln für Transaktionen ignoriert, wenn \texttt{MVCC} aktiviert ist\footnote{Seit H2 Version 1.4.x ist das die Standardeinstellung. Siehe http://www.h2database.com/html/advanced.html\#mvcc}. Im \texttt{MVCC} Modus vergibt H2 exklusive Sperren nur in Ausnahmefällen\footnote{Einer dieser Fälle ist \texttt{SELECT ... FOR UPDATE}. In diesem Fall hilft \texttt{FOR UPDATE} aber nicht, da das \texttt{SELECT}-Query keine Zeile selektieren kann, da sie nicht existiert.}. Durch das Deaktivieren von MVCC verhalten sich die Transaktionen wie erwartet und des Erstellen von Dateien mit dem gleichem Namen ist nicht mehr möglich.


\subsubsection{HTTP Response Splitting mittels Dateinamen}
Expand All @@ -149,12 +149,12 @@ \subsubsection{HTTP Response Splitting mittels Dateinamen}
\subsubsection{Aufrufen von Seiten ohne nötige Berechtigung}
Einige Seiten in HshHelper sind für normale Benutzer nicht zugänglich. Diese sollen nur von Administratoren verwendet werden. Dazu zählen zum Beispiel die Seiten zum Anlegen und Bearbeiten von Benutzern und Netzdiensten. Mit diesem Penetrationstests sollte geprüft werden, ob ein normaler Benutzer ohne Adminstrator-Rechte Zugriff auf diese Seiten erlangen kann. Da die Anzahl der zu prüfenden Seiten nicht sehr groß war wurde diese Prüfung manuell durchgeführt. Dazu wurde jede URL der betroffenen Seiten\footnote{Alle betroffenen \texttt{GET} Routen aus der \texttt{routes} Datei.} in einem Browser aufgerufen, während ein Benutzer ohne Administrator-Rechte angemeldet war. Das erwartete Verhalten war eine Weiterleitung auf eine Seite mit einer entsprechenden Fehlermeldung.

Ein ähnlicher Test wurde für alle URLs in denen URL-Parameter verwendet werden durchgeführt. Hierbei wurden für die Parameter in den URLs Werte gewählt für die der Test-Benutzer keine Berechtigung hat\footnote{Alle Parameter in URLs sind IDs.}. Zum Beispiel eine Gruppen-Id einer Gruppe in der er nicht Mitglied ist oder eine Datei-Id auf die er keine Berechtigung hat.
Ein ähnlicher Test wurde für alle URLs in denen URL-Parameter verwendet werden durchgeführt. Hierbei wurden für die Parameter in den URLs Werte gewählt für die der Test-Benutzer keine Berechtigung hat\footnote{Alle Parameter in URLs sind IDs.}. Zum Beispiel eine Gruppen-Id einer Gruppe in der er nicht Mitglied ist oder eine Datei-Id auf die er keine Berechtigung hat.

Ergebnis dieses Penetrationstests ist, dass die Seiten zum Anlegen und Bearbeiten von Netzdiensten für nicht-Administratoren zugänglichen waren. Zwar konnte ein Benutzer keine Aktionen auf diesen Seiten ausführen, Zugriff sollte aber dennoch nicht möglich sein.

\subsubsection{Cross-Site Requests}
\label{csrf:analyse}
\label{csrf:analyse}
Im Rahmen eines Penetrationstest sollte überprüft werden, ob eine Controller-Methode, die JSON zurückliefert über ein script-Tag auf einer Drittseite eingebunden werden konnte. Während dieses Tests wurde eine schwere Sicherheitslücke entdeckt, die sich aus der Play Standard-Konfiguration in Verbindung mit unserem Session-Konzept ergab.

Play nutzt für gesetzte Cookies die SameSite-Option \enquote{LAX} \footnote{\url{https://www.playframework.com/documentation/2.6.x/SettingsSession}}. Diese Option bewirkt, dass Requests, die von einer Drittseite stammen (wie z.B. ein script-Tag, dessen src-Attribut auf unsere Anwendung zeigt), das dazugehörige Cookie nicht an unsere Anwendung übertragen. Der Request selbst erreicht jedoch unsere Anwendung. Gleichzeitig generiert Play ein neues Session-Cookie, wenn eine Seite mit einem Formular aufgerufen wird, das über den integrierten CSRF-Schutz verfügt.
Expand Down Expand Up @@ -207,7 +207,7 @@ \subsection{Eingabevalidierung der HTML-Formulare}

Im Zuge des Reviews wurde festgestellt, dass das vorhandene Design Race Condition begünstigt. Die Validierungen im DTO waren nicht ausdrücklich auf \enquote{statische} Tests begrenzt. Es wurden teilweise Validierungen vorgenommen, die Datenbankzugriffe beinhaltet haben, beispielsweise die Prüfung der Existenz eines Benutzernames. Wird im Controller die Formular-Validierung\footnote{boundForm.hasErrors()} aufgerufen, ist die im DTO vorgenommene Validierung sowie der Controller-Code nicht Teil einer Transaktion. D.h. wenn die Formular-Validation meldet, dass der Nutzername noch nicht existiert, ist dieser Zustand nicht mehr garantiert, wenn der Benutzer tatsächlich angelegt wird.

Auch wenn dies im vorliegenden Fall aufgrund der Unique-Constraints in der Datenbank kein Sicherheitsproblem darstellt, haben wir uns dazu entschlossen, von einem solchen Design abzusehen. Ein Design, was in derartiger Weise Race Conditions begünstigt, ist objektiv schlecht. In den DTOs finden inzwischen nur noch \enquote{statische} Tests statt. Diese Entscheidung hat zur Entstehung der Manager-Klassen geführt (siehe Architekturdokument \ref{Arch1-architecture:manager-class-creation}). Diese bieten Methoden, die transaktionssicher sind und werfen beispielsweise eine Exception, wenn der Nutzer bereits existiert.
Auch wenn dies im vorliegenden Fall aufgrund der Unique-Constraints in der Datenbank kein Sicherheitsproblem darstellt, haben wir uns dazu entschlossen, von einem solchen Design abzusehen. Ein Design, was in derartiger Weise Race Conditions begünstigt, ist objektiv schlecht. In den DTOs finden inzwischen nur noch \enquote{statische} Tests statt. Diese Entscheidung hat zur Entstehung der Manager-Klassen geführt (siehe Architekturdokument \ref{architecture:manager-class-creation}). Diese bieten Methoden, die transaktionssicher sind und werfen beispielsweise eine Exception, wenn der Nutzer bereits existiert.

\subsection{Bcrypt Password Hashing}
Der Code zum sicheren Abspeichern eines Nutzerpassworts in der Datenbank wurde durch eine Person einem Review unterzogen. Die dabei aufgekommene Anregung wurde mit dem ganzen Team diskutiert. Das Passwort wird im Code von lediglich zwei Stellen aus gesetzt. Der \enquote{\texttt{UserManager}} verwendet sie beim Zurücksetzen des Passworts und der \enquote{\texttt{LoginManager}} beim Setzen eines neuen Passworts. In beiden Fällen wird direkt beim Aufruf des Setters der Hash durch die Verwendung des \enquote{\texttt{PasswordSecurityModule}} erzeugt. Somit ist sichergestellt, dass beim aktuellen Codestand keine Klartextpasswörter in der Datenbank abgespeichert werden. BCrypt wird verwendet um den Hash zu generieren und es wurde kurz diskutiert ob man nicht doch auf PBKDF2 wechselt. Da beide aber nicht ohne eine Third-Party-Library auskommen, wurde davon abgesehen. Dem Bcrypt-Code \autocite{BcryptCode} kann entnommen werden, dass das automatisch generierte Salt SecureRandom verwendet und somit ausreichend zufällige Salts für unterschiedliche Passwörter generiert und auch die Rundenanzahl, die explizit festgelegt wurde, ist ausreichend sicher.
Expand Down
4 changes: 2 additions & 2 deletions docs/security-policies.tex
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ \chapter{Annahmen}
\item Die von Google-Servern eingebundenen reCAPTCHA JavaScript-Dateien werden niemals zur Code-Injection verwendet / Google's reCAPTCHA Server sind grund\-sätzlich vertrauenswürdig.
\item Bibliotheksfunktion \texttt{java.security.SecureRandom} \autocite{JavaDocs.SecureRandom} erstellt Zufallszahlen, die kryptografisch sicher sind.
\item Einstellung \texttt{ALLOW\_LITERALS=NONE} der Datenbank \enquote{h2} verhindert SQL Injections.
\item \enquote{\gls{jwt}}-Format ist kryptografisch sicher zur Speicherung von Session- sowie Cookie-Daten.
\item \enquote{JSON Web Token}-Format ist kryptografisch sicher zur Speicherung von Session- sowie Cookie-Daten.
\item Die von JWT verwendete Signatur (HMAC-SHA256) verhindert, dass eine Manipulation von JWT-Cookies erfolgen kann.
\item Applikation kann nur über die definierten Eintrittspunkte verwendet werden.
\item Benutzer des HsH-Helpers verraten nicht ihr Passwort an andere.
Expand Down Expand Up @@ -224,7 +224,7 @@ \chapter{Richtlinien}
\begin{itemize}
\item Nutzer [U+] können nur mit dem System interagieren, wenn sie authentisiert sind (AK2-4, sowie AK6-21).
\item Ein Nutzer [U-] kann sich nur einloggen, wenn er ein eigenes Passwort vergeben hat.
\item Ein Nutzer [U-] kann sein Passwort nur dann zurücksetzen, wenn er ein Authentifizierungsmerkmal angibt, dass er über einen vertrauenswürdigen Zweit-Kanal erhalten hat.
\item Ein Nutzer [U-] kann sein Passwort nur dann zurücksetzen, wenn er ein Authentifizierungsmerkmal angibt, dass er über einen vertrauenswürdigen Zweit-Kanal erhalten hat.
\item Nutzer [GM, GO, A] können nur Gruppen sehen, dessen Mitglied sie sind (AK14).
\item Ausschließlich Administratoren [A] können alle Gruppen sehen (AK14).
\item Nutzer [GO] dürfen nur Mitglieder zu einer Gruppe hinzufügen, wenn sie der Besitzer dieser Gruppe sind (AK11).
Expand Down
Loading

0 comments on commit 4b494db

Please sign in to comment.