Merge branch 'master' of gitlab.science.ru.nl:mlubbers/ssproject1617

[ssproject1617.git] / report / v2_authentication.tex

diff --git a/report/v2_authentication.tex b/report/v2_authentication.tex
index 57a210e..eb512cc 100644 (file)
--- 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
 \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.
 
 \begin{result}
     All admin pages are private. Unpublished posts are also private.


@@ -15,15 +15,19 @@ public (Principle of complete mediation).
 \end{result}
 
 \item
 \end{result}
 
 \item

-\pass{}
+\fail{}

 Verify that forms containing credentials are not filled in by
 the application. Pre-filling by the application implies that
 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}
 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}
 \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
 \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}
 \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
 
 \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}:
 
     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
 \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}
 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
 \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}
 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}
 \end{result}
 
 \setcounter{enumi}{15}


@@ -143,11 +150,12 @@ user to enter credentials are done so using an encrypted
 link.
 
 \begin{result}
 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
     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
 \end{result}
 
 \item


@@ -200,7 +208,7 @@ attacks.
         \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
         \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 some CAPTCHA software).
+        using some CAPTCHA software.

     \end{itemize}
 \end{result}
 
     \end{itemize}
 \end{result}
 


@@ -211,14 +219,14 @@ services external to the application are encrypted and
 stored in a protected location.
 
 \begin{result}
 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}
     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
     \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


@@ -237,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}
 
     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.
 
 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
 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{}
 
 \item
 \fail{}


@@ -262,53 +278,65 @@ use of a configurable number of previous passwords.
     require variation in the use of different passwords.
 \end{result}
 
     require variation in the use of different passwords.
 \end{result}
 

-\notapplicable{\item
+\item
+\pass{}

 Verify that risk based re-authentication, two factor or
 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
 
 \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 \texttt{zxcvbn}\footnote{\url{https://github.com/dropbox/zxcvbn}}.
 \end{result}
 
 
 \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
 \item

-\fail{}
+% \fail{}

 Verify that all authentication challenges, whether
 successful or failed, should respond in the same average
 response time.
 
 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
 \item

-\fail{}
+% \fail{}

 Verify that secrets, \API{} keys, and passwords are not
 included in the source code, or online source code
 repositories.
 
 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}
 
 
 \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
 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{}
 
 \item
 \fail{}


@@ -323,12 +351,12 @@ untrusted parties.
 
 \item
 \pass{}
 
 \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}
 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}
 \end{result}
 
 \end{enumerate}