Best way to do a login? [Advice Needed, Not Help]

[Seraphino]

Hi, I have been working on one of my more harder PHP applications for the last few weeks and I am nearing the finality of it and it is almost ready to be handed over to the people I am developing it for. However, I wanted to make sure that I was doing my login script to the best of security and speed.

Any advice would be nice as I do want this to make great turnout for me.

(PS: The MYSQL_SERVER caps is not a bleep, those are defines in the config file)

<?
include('includes/config.php');
$r_username = $_POST['username'];
$r_password = $_POST['password'];

if (!mysql_connect(MYSQL_SERVER, MYSQL_USERNAME, MYSQL_PASSWORD))
{
    echo "Error connecting to mysql server!";
}
else
{
    mysql_select_db(MYSQL_DATABASE);
}

$query = sprintf("SELECT * FROM Users WHERE Username='$r_username' AND ASCII(Password='$r_password')", mysql_real_escape_string($r_username),
mysql_real_escape_string($r_password));

$result = mysql_query($query);
if (!$result)
{
    $num = "This Username or Password is Invalid";
}
else
{
    $num = mysql_num_rows($result);
}


if ($num == "This Username or Password is Invalid")
{
    echo "Error! You can not proceed with an invalid account!";
}
elseif($num != 0)
{
    if (MAINTENANCE == true)
    {
        if ($r_username == ROOT_USERNAME and $r_password == ROOT_PASSWORD)
        {
            setcookie("admin_MMM", "true");
            setcookie("account", $r_username);
            setcookie("account_password", $r_password);
            setcookie("MECHNET_LOGGED_IN", "true");

            echo "Welcome back Administrator $r_username! Redirecting...";
            ?>
            <script type="text/javascript">
            <!--
            setTimeout("location.href='<? echo MODEL_URL; ?>'",5000)
            //-->
            </script>
            <?
        }
        else
        {
            echo "Error, the application is currently in MAINTENANCE MODE, non-admins are not allowed to login at this time";
        }
    }
    else
    {
        if ($r_username == ROOT_USERNAME and $r_password == ROOT_PASSWORD)
        {
            setcookie("admin_MMM", "false");
            setcookie("account", $r_username);
            setcookie("account_password", $r_password);
            setcookie("MECHNET_LOGGED_IN", "true");

            echo "Welcome back Administrator $r_username! Redirecting...";
            ?>
            <script type="text/javascript">
            <!--
            setTimeout("location.href='<? echo MODEL_URL; ?>'",5000)
            //-->
            </script>
            <?
        }
        else
        {
            setcookie("admin", false);
            setcookie("account", $r_username);
            setcookie("account_password", $r_password);
    
            echo "Welcome back $r_username, redirecting...";
            ?>
            <script type="text/javascript">
            <!--
            setTimeout("location.href='<? echo MODEL_URL; ?>'",5000);
            //-->
            </script>
            <?
        }
    }
}
else
{
    echo "There was an error in accessing account information";
}


?>

[penagate]

A couple of points:

• I wouldn't use echo() in a production environment. I prefer to write a function to halt execution gracefully as well as showing the message in a user-friendly format.
• Don't use JavaScript redirects. Just send a 303 response and Location header pointing to the intended page. Of course, this does require that you don't send any output before this point.
• Never store raw passwords in cookies. If you want to store an auto-login cookie, at least store the MD5 hash, and preferably use some other encryption algorithm.

Other than that, it looks fine.

[visualAd]

I am glad I am not your client

• It is not very secure as you are not escaping the variables. Well, you are but after you have put them inside the string.
• If you receive and error while connecting, you should end the script not echo a message and try to continue.
• Why use Javascript to redirect the page, why not just use:
  

  header('Refresh: 3; url=' . $path_to_new_script); 

• Your script should when it fails redirect the user to an error page rather than displaying a single line, unfriendly error message.

[Seraphino]

Thanks for helping. I asked because I wanted to make it the best before I shipped it and I do appreciate that you have helped me make it better.

However, i do not quite understand why it seems so insecure becuase I few people on my team have tried to crack it and they have not quite been able to do it yet. I also asked my clients to do the same and they did not know how to do it either.

I was just curious...

[PS] I don't know why I am not using headers, I guess I just never really knew that javascript made a difference since the application after the login requires it for some of it's components.

[PPS] In reply to penegate, the passwords are not raw when stored in cookies. They are stored in SHA1 encryption format so they are not just floating out there. Trust me, I know that much.

[kows]

[PS] I don't know why I am not using headers, I guess I just never really knew that javascript made a difference since the application after the login requires it for some of it's components.

if it can be avoided, I would drop using JavaScript altogether unless you're using it in an environment where you know that all users WILL have it enabled and supported by their browsers. otherwise, you're cutting off potential customers and users.

[PPS] In reply to penegate, the passwords are not raw when stored in cookies. They are stored in SHA1 encryption format so they are not just floating out there. Trust me, I know that much.

are you sure about that? I don't see you encrypting anything at all in the piece of code that you posted. you don't encrypt the POST variables when querying the database, and you don't encrypt anything before storing it in a cookie. although it is unlikely that your database could be compromised, it is always a possibility. it's always good to encrypt things at the database level; that way, no one but the customer themselves knows their password. that is, of course, unless you just removed it for the original post.

as a quick note of my own, you might want to rebuild the way you're checking whether or not there's an error while logging in. relying on strings is a bit sloppy, and instead you might want to use on a boolean value and then print the text depending on whether or not that user has been logged in. this way, it can be modified relatively easily (you won't need to modify multiple pieces of the same code just to change the message. a few months from now, you might [or someone else might] need to change it and you will be wondering why your login script doesn't output an error anymore). reworking some of your code myself, this might work better:

  $result = mysql_query($query);
  $n = mysql_num_rows($result);
  $loginfail = ($n == 0) ? true : false;
  if(!$loginfail){
    //check whether or not there is maintenance going on
    //log the user in if they need to be.
  }else{
    //login failed, so print something out letting them know
    // -- OR, send a header to redirect to the login error page
    /*
     * header("Location: http://domain.com/login-error.php");
     *
     * echo "There was a problem while trying to log you in.";
     */
  }