5
\$\begingroup\$

Can someone look over this? This is the entire authentication file. It's referenced at the beginning of all my catalog editing files to make sure only the specified user is logged in. I want to make sure it's secure.

<?php
//      pageauth.php
//      Asks for a username and a password and checks it.

if($_SESSION['logged_in'] != 1){
    if(!isset($_SERVER['PHP_AUTH_USER'])){
        header('WWW-Authenticate: Basic realm="Catalog Administration"');
        header('HTTP/1.0 401 Unauthorized');
        echo '<h1>Hey! You can\'t be here!</h1>
        <p>Try logging in first!</p>';
        exit;
    }elseif(md5($_SERVER['PHP_AUTH_USER']) != "04b2f0a4ad7772ca864aa569917b2d2d"){
        echo '<h1>Wrong Username!</h1>
        <p>Only the admin username and password are accepted.</p>';
    exit;
    }elseif(md5($_SERVER['PHP_AUTH_PW']) != "ed972411dfcca5313ab151694af01da8"){
        echo '<h1>Wrong Password!</h1>
        <p>For obvious reasons, we need a correct password!</p>';
        exit;
    }else{
        session_start();
        $_SESSION['logged_in'] = 1;
    }
}
?>

I just need to authenticate for one user. I don't need to go into databases or anything, so I thought PHP_AUTH would be a good solution.

\$\endgroup\$
  • \$\begingroup\$ If you're using Basic authentication, you shouldn't need to deal with sessions; the browser will re-send whatever credentials it last used. \$\endgroup\$ – icktoofay Jan 26 '14 at 2:17
  • \$\begingroup\$ Thanks! Other than that, anything else? \$\endgroup\$ – ndm13 Jan 26 '14 at 2:21
3
\$\begingroup\$

Basic authentication is very basic. It is not really designed to allow responses, such as Wrong Username/Wrong Password.

If a login attempt fails it will prompt you for the username/password again and again, (usually the browsers allow 3 attempts before failing) Once the login fails you get 1 error message. This could be caused by wrong username/password from cancel option.

If you want to show bad username/password messages I suggest you implement your own authentication form (which is not difficult).

It is also not a good idea to show wrong username, and wrong password explicitly as once I guess the correct username, I will be able to tell from your response saying bad password only. Then i can work on cracking your password knowing I have the username correct.

I have altered your code substantially and tried to explain why in the comments

If you are using apache websserver, all of what you have done can also be achieved using a a simple .htaccess and .htpasswd file Here is a website that can generate those files for you http://www.htaccesstools.com/htpasswd-generator/

<?php
//      pageauth.php
//      Asks for a username and a password and checks it.

// lets define username/password first, so it is easier to change later without digging through the code.
$auth_hash_algorithm = "sha256";
$user_hash = '8c6976e5b5410415bde908bd4dee15dfb167a9c873fc4bb8a81f6f2ab448a918'; // admin
$pass_hash = '5e884898da28047151d0e56f8dc6292773603d0d6aabbdd62a11ef721d1542d8'; // password

// If i run your code up under E_STRICT then i get a warning
// Notice: Undefined variable: _SESSION in test.php
// We should really start the session before we use it, not in the conditional logic below

session_start();

// Next I get
// Notice: Undefined index: logged_in in test.php
// the first time through we should test the index logged_in exists before referencing it.
// Assuming logged_in could only ever be set to 1, then we could just test to see if logged_in is not set, rather then != 1

// if($_SESSION['logged_in'] != 1){
if(!isset($_SESSION['logged_in'])){

    // First time through it prompts me for a user/name password
    // if i leave the username/password blank it says Wrong Username!
    // $_SERVER['PHP_AUTH_USER'] is now set to blank and i can't even attempt to login again unless i restart the browser
    // I am assuming it was not the intention to only allow 1 login attempt?

    $user = isset($_SERVER['PHP_AUTH_USER']) ? $_SERVER['PHP_AUTH_USER'] : null;
    $pass = isset($_SERVER['PHP_AUTH_PW']) ? $_SERVER['PHP_AUTH_PW'] : null;

    if (hash($auth_hash_algorithm, $user) == $user_hash && hash($auth_hash_algorithm, $pass) == $pass_hash) {
        $_SESSION['logged_in'] = 1;
    } else {
        header('WWW-Authenticate: Basic realm="Catalog Administration"');
        header('HTTP/1.0 401 Unauthorized');
        echo '<h1>Hey! You can\'t be here!</h1>
        <p>Try logging in first!</p>';
        exit;
    }
}

echo "Success You must be logged in";
\$\endgroup\$
  • \$\begingroup\$ Thank you so much! This is an amazing improvement over my code! \$\endgroup\$ – ndm13 Jan 26 '14 at 20:29
5
\$\begingroup\$

md5 usage may be okay on small projects, but it's generally considered weak and shouldn't be used. As of the MD5 wikipedia page:

The security of the MD5 hash function is severely compromised.

and http://us1.php.net/md5

It is not recommended to use this function to secure passwords, due to the fast nature of this hashing algorithm.

So perhaps you should look into PHP's other hashing methods, say hash() itself. You can read more here: http://us1.php.net/manual/en/function.hash.php

Important Edit:

I read a little bit more and I am incorrect! The hash method is okay. It's not great like bcrypt! I highly suggest you read both of these and understand them, I'm sure it will come in handy later on! https://security.stackexchange.com/questions/17421/how-to-store-salt and https://security.stackexchange.com/questions/211/how-to-securely-hash-passwords. And directly relating to PHP, http://us1.php.net/manual/en/function.password-hash.php

\$\endgroup\$
  • \$\begingroup\$ Thanks for the feedback! I'll use hash() instead! \$\endgroup\$ – ndm13 Jan 26 '14 at 3:50
  • \$\begingroup\$ I have updated my answer, please read the edit! \$\endgroup\$ – Alex L Jan 29 '14 at 6:26
  • \$\begingroup\$ Thanks for the update! I feel comfortable using hash(SHA256,$password), but if I think I need more security (for something like an entire user system in a database) I'll look into bcrypt! \$\endgroup\$ – ndm13 Jan 30 '14 at 23:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.