X-Git-Url: https://git.martlubbers.net/?a=blobdiff_plain;f=report%2Fv2_authentication.tex;h=eb512cc872228886cdf9bcf4da8096244a9433e1;hb=bf85c6f569c297c733227d532cbaec0dd663e985;hp=adfc0f81ac31c461c68d58a717337ff78cc19ec0;hpb=512c27da4aac1788cbbe4112283c93aa0e446a37;p=ssproject1617.git diff --git a/report/v2_authentication.tex b/report/v2_authentication.tex index adfc0f8..eb512cc 100644 --- a/report/v2_authentication.tex +++ b/report/v2_authentication.tex @@ -1,11 +1,11 @@ -\begin{enumerate}[label={2.\arabic*}] +\begin{enumerate}[label={V2.\arabic*}] \item \fail{} Verify all pages and resources by default require authentication except those specifically intended to be -public (Principle of complete mediation). +public (principle of complete mediation). \begin{result} All admin pages are private. Unpublished posts are also private. @@ -15,15 +15,19 @@ public (Principle of complete mediation). \end{result} \item -\pass{} +\fail{} 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, +credentials are stored in plain-text 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. +No credentials that come from the database are pre-filled by the application. +However, in some forms, the application pre-fills password fields from the +request's POST data. This is not necesarry.\footnote{This issue was actually +overlooked when auditing manually, and was found when running the Fortify tool. +In the initial audit, we only ensured that no internal information (from the +database) was leaked in this way.} \end{result} \setcounter{enumi}{3} @@ -36,7 +40,7 @@ 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. + security of these controls in the implementation. \end{result} \setcounter{enumi}{5} @@ -48,10 +52,10 @@ 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. + in (not only) \code{Users::find} vulnerable to \SQL{} injections. Through this vulnerability, + an attacker can execute arbitrary \SQL{} code from the username field in the login form. 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}: + will set the password of the \code{admin} user to \code{s3cret}: \code{"; UPDATE users SET password='\$1\$OWgsBb90\$Lkko6aZwmp9XOVrFI09Ab0' WHERE \\username='admin' AND 'a' = "a} @@ -61,13 +65,13 @@ attackers cannot log in. \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 +of pass-phrases, and do not prevent password managers, +long pass-phrases or highly complex passwords being entered. \begin{result} - The application allows the user to use any password (except ones that contain - SQL code). + The application allows the user to use any password (except ones that trigger + errors in injected \SQL{} code). \end{result} \item @@ -89,7 +93,7 @@ authentication mechanism. 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. + 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} @@ -126,11 +130,14 @@ 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). + Password are stored in database using the \PHP{} function \code{crypt}. Internally, this + function uses salted MD5 on modern UNIX systems. On legacy systems, this uses an old DES + based algorithm, which uses only the first eight characters of the supplied password. + This is way too easy to reverse with brute-force attacks using dictionary files. + + Instead the \CMS{} should use the \code{argon2} password hashing algorithm + or the \PHP{} \code{password\_hash} function (which currently uses BCRYPT) which are + (at this moment) considered safe to use. \end{result} \setcounter{enumi}{15} @@ -143,11 +150,12 @@ 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 + The app allows admin users to log in over \HTTP{}. This is insecure, as it allows + eavesdroppers to intercept any password. + The app should force \HTTPS{} for at least the login form, the \code{admin\_controller} and for the installation script (because the users posts secrets like the database - password to this page). + password to this page and the \CMS{} supplies the user with the credentials of the \code{admin} + account). \end{result} \item @@ -163,16 +171,27 @@ that the new password is not sent in clear text to the user. information. \end{result} -\TODO{\item - +\item +\fail{} Verify that information enumeration is not possible via -login, password reset, or forgot account functionality.} +login, password reset, or forgot account functionality. -\TODO{\item +\begin{result} + All these forms are vulnerable to \SQL{} injection attacks. So any information + can leak any information from the database. +\end{result} +\item +\pass{} Verify there are no default passwords in use for the application framework or any components used by the -application (such as “admin/password”).} +application (such as “admin/password”). + +\begin{result} + No secrets are initialized by predefined values. The admin user will have + username \code{admin} by default. This is no secret and therefore not + considered unsafe. +\end{result} \item \fail{} @@ -188,85 +207,156 @@ attacks. \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). + \item And comment submission, to prevent spam, phishing et cetera (by + using some CAPTCHA software. \end{itemize} \end{result} -\TODO{\item - +\item +\fail{} Verify that all authentication credentials for accessing services external to the application are encrypted and -stored in a protected location.} +stored in a protected location. -\TODO{\item +\begin{result} + The database credentials are hard-coded in \code{config.php}. While it + would be better to pass secrets as environment variables, this is not + really bad practice. + + However, the installation instructions state the following: + \begin{verbatim} +Change the file permissions to allow all users write access to the +folder you extracted TestCMS to. + \end{verbatim} + This implies making the configuration file readable for all users on the + system. This information should not be accessible for any user other than + running the \PHP{} script. +\end{result} +\item +\pass{} Verify that forgotten password and other recovery paths -use a TOTP or other soft token, mobile push, or other +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.} +e-mail or SMS should be a last resort and is known weak. -\TODO{\item +\begin{result} + The password recovery path uses a random looking token. It is sent over + e-mail, which is considered weak (but not unsafe). +\end{result} +\item +\fail{} 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.} +attack, this should not reset the hard lock status. -\TODO{\item +\begin{result} + The application has not implemented any lockout mechanisms. +\end{result} +\item +\pass{} Verify that if shared knowledge based questions (also -known as "secret questions") are required, the questions +known as ``secret questions'') are required, the questions do not violate privacy laws and are sufficiently strong to -protect accounts from malicious recovery.} +protect accounts from malicious recovery. -\TODO{\item +\begin{result} + The application uses no shared knowledge based questions, and thus not + violate any privacy laws. +\end{result} +\item +\fail{} Verify that the system can be configured to disallow the -use of a configurable number of previous passwords.} +use of a configurable number of previous passwords. -\TODO{\item +\begin{result} + The system does not remember any previously used passwords and does not + require variation in the use of different passwords. +\end{result} +\item +\pass{} Verify that risk based re-authentication, two factor or -transaction signing is in place for high value transactions.} +transaction signing is in place for high value transactions. -\TODO{\item +\begin{result} + There are no (really) risk based action for which re-authentication would be + fit. +\end{result} +\item +\fail{} Verify that measures are in place to block the use of -commonly chosen passwords and weak passphrases.} +commonly chosen passwords and weak pass-phrases. -\TODO{\item +\begin{result} + No password strengthening measures are implemented. The app should + use some password strength estimator like \texttt{zxcvbn}\footnote{\url{https://github.com/dropbox/zxcvbn}}. +\end{result} +\notapplicable{ +\item +% \fail{} Verify that all authentication challenges, whether successful or failed, should respond in the same average -response time.} +response time. -\TODO{\item +% \begin{result} +% String comparisation for checking password hashes and password reset +% tokens are not in constant time. +% \end{result} +} -Verify that secrets, API keys, and passwords are not +\notapplicable{ +\item +% \fail{} +Verify that secrets, \API{} keys, and passwords are not included in the source code, or online source code -repositories.} +repositories. -\setcounter{enumi}{30} +% \begin{result} +% The database credentials are hard coded in \code{config.php}. These +% credentials should ideally be passed using environment variables. +% \end{result} +} -\TODO{\item +\setcounter{enumi}{30} +\item +\fail{} 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.} +disclosure. -\TODO{\item +\begin{result} + No such features are implemented. +\end{result} +\item +\fail{} Verify that administrative interfaces are not accessible to -untrusted parties.} +untrusted parties. -\TODO{\item +\begin{result} + Any authenticated user is allowed to view and use the administrative + interface. A separation should be made between administrators and normal + users. +\end{result} -Browser autocomplete, and integration with password +\item +\pass{} +Browser auto-complete, and integration with password managers are permitted unless prohibited by risk based policy. -} +\begin{result} + Browser auto-complete functionality is not restricted in any way. +\end{result} \end{enumerate}