From 512c27da4aac1788cbbe4112283c93aa0e446a37 Mon Sep 17 00:00:00 2001 From: Daan Sprenkels 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