From 512c27da4aac1788cbbe4112283c93aa0e446a37 Mon Sep 17 00:00:00 2001
From: Daan Sprenkels <dsprenkels@gmail.com>
Date: Mon, 7 Nov 2016 17:21:02 +0100
Subject: [PATCH] Added authentication up to 2.17

---
 report/authentication/checklist.md |  66 -------
 report/report.tex                  |   1 +
 report/v2_authentication.tex       | 272 +++++++++++++++++++++++++++++
 3 files changed, 273 insertions(+), 66 deletions(-)
 delete mode 100644 report/authentication/checklist.md
 create mode 100644 report/v2_authentication.tex

diff --git a/report/authentication/checklist.md b/report/authentication/checklist.md
deleted file mode 100644
index 1f3971c..0000000
--- a/report/authentication/checklist.md
+++ /dev/null
@@ -1,66 +0,0 @@
-# Authentication
-
-## 2.1 Verify that all private pages are indeed private
-All admin pages are private. Unpublished posts are also private.
-
-However, the install submission page does not do any authentication at all,
-allowing any user to reset the database and claim an admin account.
-
-User pages can be viewed and submitted by any authenticated user.
-
-## 2.2
-## 2.3
-## 2.4
-## 2.5
-## 2.6
-## 2.7
-## 2.8
-## 2.9
-Changing a user's password does not require the original password.
-
-## 2.10
-## 2.11
-## 2.12
-
-## 2.13 Password storage security
-Password are stored in database using the PHP function `crypt`. Internally, this
-functions uses salted MD5. This is way too easy to brute-force.
-
-Instead use `argon2` or the `password_hash` function.
-
-## 2.14
-## 2.15
-
-## 2.16
-The app allows admin users to log in over HTTP. This is insecure. Force HTTPS
-for the admin_controller and for the installation script.
-
-## 2.17
-## 2.18
-## 2.19
-
-## 2.20
-No anti-automation measures are deployed.
-
-Needed for email validation, installation database check, login, comment
-submissions, etc.
-
-## 2.21
-## 2.22
-## 2.23
-## 2.24
-## 2.25
-## 2.26
-## 2.27
-## 2.28
-## 2.29
-## 2.30
-## 2.31
-## 2.32
-## 2.33
-
-## Other
-- `Users::find` is vulnerable to SQL injection.
-- The default admin password is generated using a Mersenne Twister, which is
-  not cryptographically secure.
-- Verification of hash equality in login is not done in constant time.
diff --git a/report/report.tex b/report/report.tex
index 51a35d3..55ef024 100644
--- a/report/report.tex
+++ b/report/report.tex
@@ -15,6 +15,7 @@
 
 \addtocounter{subsection}{1}
 \subsection{Authentication}
+\input{v2_authentication}
 
 \subsection{Session Management}
 
diff --git a/report/v2_authentication.tex b/report/v2_authentication.tex
new file mode 100644
index 0000000..adfc0f8
--- /dev/null
+++ b/report/v2_authentication.tex
@@ -0,0 +1,272 @@
+
+\begin{enumerate}[label={2.\arabic*}]
+
+\item
+\fail{}
+Verify all pages and resources by default require
+authentication except those specifically intended to be
+public (Principle of complete mediation).
+
+\begin{result}
+    All admin pages are private. Unpublished posts are also private.
+    However, the install submission page does not do any authentication at all,
+    allowing any user to reset the database and claim an admin account.
+    User pages can be viewed (and submitted) by any authenticated user.
+\end{result}
+
+\item
+\pass{}
+Verify that forms containing credentials are not filled in by
+the application. Pre-filling by the application implies that
+credentials are stored in plaintext or a reversible format,
+which is explicitly prohibited.
+
+\begin{result}
+No credentials (that should not be stored in plain text) are ever filled in by
+the application.
+\end{result}
+
+\setcounter{enumi}{3}
+
+\item
+\pass{}
+Verify all authentication controls are enforced on the
+server side.
+
+\begin{result}
+    All authentication controls (login credentials and client cookies) are
+    enforced by the application. Note however item~\ref{auth:6}, about the
+    security of these controls in the immplementation.
+\end{result}
+
+\setcounter{enumi}{5}
+
+\item\label{auth:6}
+\fail{}
+Verify all authentication controls fail securely to ensure
+attackers cannot log in.
+
+\begin{result}
+    The input to various forms is not sanitized at all. This makes the implementation
+    in (not only) \texttt{Users::find} vulnerable to SQL injections. The login form
+    is also vulnerable. Any user can execute arbitrary SQL code from the username field.
+    The following example code can submitted as username in the login form, which
+    will set the password of the \texttt{admin} user to \texttt{s3cret}:
+
+    \code{"; UPDATE users SET password='\$1\$OWgsBb90\$Lkko6aZwmp9XOVrFI09Ab0' WHERE \\username='admin' AND 'a' = "a}
+
+    After using this exploit, the attacker (of course) knows the admin password.
+\end{result}
+
+\item
+\pass{}
+Verify password entry fields allow, or encourage, the use
+of passphrases, and do not prevent password managers,
+long passphrases or highly complex passwords being
+entered.
+
+\begin{result}
+    The application allows the user to use any password (except ones that contain
+    SQL code).
+\end{result}
+
+\item
+\pass{}
+Verify all account identity authentication functions (such
+as update profile, forgot password, disabled / lost token,
+help desk or IVR) that might regain access to the account
+are at least as resistant to attack as the primary
+authentication mechanism.
+
+\begin{result}
+    The only alternative mechanism for authenticating (except for using
+    vulnerabilities as described in other point) is using the ``Recover
+    password'' functionality. This sends a secret token to the email
+    address owning the account.
+
+    After such a token is used, it cannot be reused, because the token is
+    derived from the password hash (and thus a random salt). Tokens do not
+    expire, however. It would be even better to require that a token be used
+    withing a day (or so) after creation.
+
+    The security of the SMTP connection is at the discretion of the web server.
+    Often these connections are not very secure, but this worry is beyond the
+    scope of this audit.
+\end{result}
+
+\item
+\fail{}
+Verify that the changing password functionality includes
+the old password, the new password, and a password
+confirmation.
+
+\begin{result}
+    Changing a user's password does not require the original password, nor does the
+    password Changing form request for a password confirmation. This makes it easy
+    to set a wrong password for yourself.
+\end{result}
+
+\setcounter{enumi}{11}
+
+\item
+\fail{}
+Verify that all authentication decisions can be logged,
+without storing sensitive session identifiers or passwords.
+This should include requests with relevant metadata
+needed for security investigations.
+
+\begin{result}
+    The application does not log any notable authentication event.
+\end{result}
+
+\item
+\fail{}
+Verify that account passwords are one way hashed with a
+salt, and there is sufficient work factor to defeat brute
+force and password hash recovery attacks.
+
+\begin{result}
+    Password are stored in database using the PHP function \texttt{crypt}. Internally, this
+    function uses salted MD5. This is way too reverse with brute-force attacks using dictionary files.
+
+    Instead it would be better to use the \texttt{argon2} password hashing algorithm
+    or the PHP \texttt{password\_hash} function (which currently uses BCRYPT).
+\end{result}
+
+\setcounter{enumi}{15}
+
+\item
+\fail{}
+Verify that credentials are transported using a suitable
+encrypted link and that all pages/functions that require a
+user to enter credentials are done so using an encrypted
+link.
+
+\begin{result}
+    The app allows admin users to log in over HTTP. This is insecure, as it allows
+    eavesdroppers to intercept password.
+    The app should force HTTPS for at least the login form, the \texttt{admin\_controller} and
+    for the installation script (because the users posts secrets like the database
+    password to this page).
+\end{result}
+
+\item
+\pass{}
+Verify that the forgotten password function and other
+recovery paths do not reveal the current password and
+that the new password is not sent in clear text to the user.
+
+\begin{result}
+    No password is stored in plain text. The password recovery path works with
+    a secret token, which is used to reset the password. Although this token
+    is derived from the password hash, it does not reveal any secret
+    information.
+\end{result}
+
+\TODO{\item
+
+Verify that information enumeration is not possible via
+login, password reset, or forgot account functionality.}
+
+\TODO{\item
+
+Verify there are no default passwords in use for the
+application framework or any components used by the
+application (such as “admin/password”).}
+
+\item
+\fail{}
+Verify that anti-automation is in place to prevent breached
+credential testing, brute forcing, and account lockout
+attacks.
+
+\begin{result}
+    No anti-automation measures are deployed.
+
+    This is needed for:
+    \begin{itemize}
+        \item Email validation, to harden brute force email address discovery
+        \item Installation database check, to prevent guessing attacks for the database password
+        \item Login, to prevent login guessing
+        \item And comment submission, to prevent spam, phishing et cetera (by using CAPTCHA).
+    \end{itemize}
+\end{result}
+
+\TODO{\item
+
+Verify that all authentication credentials for accessing
+services external to the application are encrypted and
+stored in a protected location.}
+
+\TODO{\item
+
+Verify that forgotten password and other recovery paths
+use a TOTP or other soft token, mobile push, or other
+offline recovery mechanism. Use of a random value in an
+e-mail or SMS should be a last resort and is known weak.}
+
+\TODO{\item
+
+Verify that account lockout is divided into soft and hard
+lock status, and these are not mutually exclusive. If an
+account is temporarily soft locked out due to a brute force
+attack, this should not reset the hard lock status.}
+
+\TODO{\item
+
+Verify that if shared knowledge based questions (also
+known as "secret questions") are required, the questions
+do not violate privacy laws and are sufficiently strong to
+protect accounts from malicious recovery.}
+
+\TODO{\item
+
+Verify that the system can be configured to disallow the
+use of a configurable number of previous passwords.}
+
+\TODO{\item
+
+Verify that risk based re-authentication, two factor or
+transaction signing is in place for high value transactions.}
+
+\TODO{\item
+
+Verify that measures are in place to block the use of
+commonly chosen passwords and weak passphrases.}
+
+\TODO{\item
+
+Verify that all authentication challenges, whether
+successful or failed, should respond in the same average
+response time.}
+
+\TODO{\item
+
+Verify that secrets, API keys, and passwords are not
+included in the source code, or online source code
+repositories.}
+
+\setcounter{enumi}{30}
+
+\TODO{\item
+
+Verify that if an application allows users to authenticate,
+they can authenticate using two-factor authentication or
+other strong authentication, or any similar scheme that
+provides protection against username + password
+disclosure.}
+
+\TODO{\item
+
+Verify that administrative interfaces are not accessible to
+untrusted parties.}
+
+\TODO{\item
+
+Browser autocomplete, and integration with password
+managers are permitted unless prohibited by risk based
+policy.
+
+}
+
+\end{enumerate}
-- 
2.20.1