X-Git-Url: https://git.martlubbers.net/?a=blobdiff_plain;f=report%2Fv2_authentication.tex;h=eb512cc872228886cdf9bcf4da8096244a9433e1;hb=bf85c6f569c297c733227d532cbaec0dd663e985;hp=74875af7df93965a4d954e854ee94b09c993f31e;hpb=db9adf4baa5dabb6cb7df7a57fc0aa22881e785e;p=ssproject1617.git diff --git a/report/v2_authentication.tex b/report/v2_authentication.tex index 74875af..eb512cc 100644 --- a/report/v2_authentication.tex +++ b/report/v2_authentication.tex @@ -5,7 +5,7 @@ \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,8 +52,8 @@ attackers cannot log in. \begin{result} The input to various forms is not sanitized at all. This makes the implementation - in (not only) \code{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 \code{admin} user to \code{s3cret}: @@ -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 @@ -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 \code{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 \code{argon2} password hashing algorithm - or the \PHP{} \code{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 \code{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 @@ -199,7 +207,8 @@ 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} @@ -210,14 +219,14 @@ services external to the application are encrypted and stored in a protected location. \begin{result} - The database credentials are hardcoded in \code{config.php}. While it + 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. +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 @@ -236,20 +245,28 @@ e-mail or SMS should be a last resort and is known weak. e-mail, which is considered weak (but not unsafe). \end{result} -\notapplicable{\item +\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. -% The application has not implemented any lockout mechanisms. -} +\begin{result} + The application has not implemented any lockout mechanisms. +\end{result} -\notapplicable{\item +\item +\pass{} 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.} +protect accounts from malicious recovery. + +\begin{result} + The application uses no shared knowledge based questions, and thus not + violate any privacy laws. +\end{result} \item \fail{} @@ -261,53 +278,65 @@ use of a configurable number of previous passwords. require variation in the use of different passwords. \end{result} -\notapplicable{\item +\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. -% There are no (really) risk based action or which re-authentication would be -% fit +\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. \begin{result} No password strengthening measures are implemented. The app should - use some password strength estimator like \code{zxcvbn}. + use some password strength estimator like \texttt{zxcvbn}\footnote{\url{https://github.com/dropbox/zxcvbn}}. \end{result} +\notapplicable{ \item -\fail{} +% \fail{} Verify that all authentication challenges, whether successful or failed, should respond in the same average response time. -\begin{result} - String comparisation for checking password hases and password reset tokens - are not in constant time. -\end{result} +% \begin{result} +% String comparisation for checking password hashes and password reset +% tokens are not in constant time. +% \end{result} +} +\notapplicable{ \item -\fail{} +% \fail{} Verify that secrets, \API{} keys, and passwords are not included in the source code, or online source code repositories. -\begin{result} - The database credentials are hard coded in \code{config.php}. These - credentials should ideally be passed using environment variables. -\end{result} +% \begin{result} +% The database credentials are hard coded in \code{config.php}. These +% credentials should ideally be passed using environment variables. +% \end{result} +} \setcounter{enumi}{30} -\notapplicable{\item +\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. + +\begin{result} + No such features are implemented. +\end{result} \item \fail{} @@ -322,12 +351,12 @@ untrusted parties. \item \pass{} -Browser autocomplete, and integration with password +Browser auto-complete, and integration with password managers are permitted unless prohibited by risk based policy. \begin{result} - Browser autocomplete functionality is not restricted in any way. + Browser auto-complete functionality is not restricted in any way. \end{result} \end{enumerate}