SitePoint Sponsor

User Tag List

Results 1 to 21 of 21
  1. #1
    SitePoint Member
    Join Date
    Mar 2008
    Location
    Norway
    Posts
    20
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Looking for input on authentication class

    I'm trying to build an all-around good authentication class that can be used on most servers, especially shared-hosting that offers low security.

    Short description:
    Authenticate is meant to be called on every page behind for example "/admin". Sanitize isn't really complete yet because I haven't decided on what characters are going to be allowed for username/password, input here would be appreciated. The class was inspired by another thread here regarding login security where different methods were discussed.

    The secret key is suppose to be generated automatically by a php script and written to a config file upon app install. I'm interested in input about where to store this config file and how to best protect it in a shared-hosting environment.

    Meant for php 5.2+


    PHP Code:
    <?php
    /**
     * Description of adminAuthentication
     *
     * @author Lars Boldt (01/2009)
     */
    class adminAuthentication {
        private 
    $db$secretKey;

        
    /**
         * @param object $pdo PDO object
         */
        
    public function __construct($pdo$secretKey) {
            try {
                if (
    $pdo instanceof PDO) {
                    
    $this->db $pdo;
                } else {
                    throw new 
    Exception('Param $pdo must be a PDO object');
                }
                
    $this->secretKey $secretKey;
            } catch (
    Exception $e) {
                die(
    $e->getMessage());
            }
        }

        
    /**
         * Sets a $_SESSION with plain-text username and hashed password, then calls authenticate().
         * @param string $username Add directly from i.e $_POST
         * @param string $password Add directly from i.e $_POST
         * @return bool Returns true if username/password authenticates, false if not.
         */
        
    public function login($username$password) {
            
    session_regenerate_id(true);
            
    $_SESSION[get_class($this)] = serialize(array('username' => $this->sanitize($username), 'password' => hash('sha256'$this->sanitize($password).$this->secretKey)));
            return 
    $this->authenticate();
        }

        
    /**
         * Destroys the $_SESSION with username/password
         * @return bool Returns true if $_SESSION is set and unset, false if $_SESSION is not set.
         */
        
    public function logout() {
            if (isset(
    $_SESSION[get_class($this)])) {
                unset(
    $_SESSION[get_class($this)]);
                return 
    true;
            } else {
                return 
    false;
            }
        }

        
    /**
         * Compares username/password in $_SESSION against username/password in database.
         * @return bool Returns true if they match, false if $_SESSION is not set or there's a mismatch.
         */
        
    public function authenticate() {
            if (isset(
    $_SESSION[get_class($this)])) {
                
    $sessionData unserialize($_SESSION[get_class($this)]);
                
    $username hash('sha256'$sessionData['username'].$this->secretKey);
                
    $password $sessionData['password'];

                
    $rVal $this->db->prepare('SELECT COUNT(*) FROM adminAuthentication WHERE username = :usr AND password = :pwd');
                
    $rVal->execute(array('usr' => $username'pwd' => $password));
                if (
    $rVal->fetchColumn() > 0) {
                    return 
    true;
                } else {
                    return 
    false;
                }
            } else {
                return 
    false;
            }
        }

        
    /**
         * Creates a cookie with the username and password for a remember-me function.
         * @param string $domain
         * @return bool Cookie may still not be created even if true is returned, false means cookie is not created. Check for cookie manually after page reload to verify.
         */
        
    public function setCookie($username$password$domain '') {
            
    $sessionData unserialize($_SESSION[get_class($this)]);

            
    // expires in 30days
            
    $expires time()+60*60*24*30;
            
    // store username/password/expiry date in cookie
            
    $cookieData serialize(array('username' => $this->sanitize($username), 'password' => $this->sanitize($password), 'expires' => $expires));

            
    // encrypt $cookieData using RIJNDAEL_256
            
    $iv_size mcrypt_get_iv_size(MCRYPT_RIJNDAEL_256MCRYPT_MODE_ECB);
            
    $iv mcrypt_create_iv($iv_sizeMCRYPT_RAND);
            
    $encrypted_data mcrypt_encrypt(MCRYPT_RIJNDAEL_256$this->secretKey$cookieDataMCRYPT_MODE_ECB$iv);

            return 
    setcookie(get_class($this), $encrypted_data$expires'/'$domainfalsetrue);
        }

        
    /**
         * Decrypt, validate and retrieve the cookie data.
         * @return bool|array Returns false if cookie is not set or fails expiry check. Returns array if cookie is set and validates.
         */
        
    public function getCookieData() {
            if (isset(
    $_COOKIE[get_class($this)])) {
                
    $cookieData $_COOKIE[get_class($this)];

                
    // Decrypt cookie data      
                
    $iv_size mcrypt_get_iv_size(MCRYPT_RIJNDAEL_256MCRYPT_MODE_ECB);
                
    $iv mcrypt_create_iv($iv_sizeMCRYPT_RAND);
                
    $decrypted_data mcrypt_decrypt(MCRYPT_RIJNDAEL_256$this->secretKey$cookieDataMCRYPT_MODE_ECB$iv);

                return 
    $this->verifyCookieExpires(unserialize($decrypted_data));
            } else {
                return 
    false;
            }
        }

        
    /**
         * Destroys the cookie.
         * @param string $domain
         * @return bool Cookie may still be present even if true is returned, false means cookie is still present. Check for cookie manually after page reload to verify.
         */
        
    public function removeCookie($domain '') {
            if (isset(
    $_COOKIE[get_class($this)])) {
                return 
    setcookie(get_class($this), ''time() - 3600'/'$domain);
            } else {
                return 
    false;
            }
        }

        
    /**
         * Verify expiry date of cookie.
         * @return bool|array - see getCookieData().
         */
        
    private function verifyCookieExpires($cookieData) {
            if (isset(
    $cookieData['expires']) && time() <= $cookieData['expires']) {
                return 
    $cookieData;
            } else {
                return 
    false;
            }
        }
        
        
    /**
         * Sanitize input
         * @return string
         */
        
    private function sanitize($data) {
            return 
    strip_tags(trim($data));
        }
    }
    ?>
    Lars

  2. #2
    SitePoint Wizard silver trophy kyberfabrikken's Avatar
    Join Date
    Jun 2004
    Location
    Copenhagen, Denmark
    Posts
    6,157
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    To start with, you don't need to explicitly serialize/unserialize data in $_SESSION. That is done automatically by PHPs session management system. Also, you're currently storing 'expires' in the cookie, which is sort of pointless, since cookies have this as a property (Unless I missed something obvious here?).
    How about http authentication. And how about openid-integration? Those would be nice additions.

  3. #3
    SitePoint Member
    Join Date
    Mar 2008
    Location
    Norway
    Posts
    20
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Indeed, will remove the serialize/unserialize for sessions. As for storing expiry date, it's quite easy to change the expiry date of a cookie using for instance FireBug in Firefox, so if you want to enforce the expiry date you set for the cookie the best way is to store it in the value and check it.

    As for open-id, I remember reading an article not long ago that pretty much said open-id wasn't ready or had many flaws, don't remember where though, will look for it. What's your take on open-id as of now?

    Will look into implementing http auth, that seems like a good idea.

  4. #4
    SitePoint Enthusiast
    Join Date
    Mar 2005
    Posts
    94
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    in authenticate() the username is hashed, instead of the password.
    use hash_hmac() for keyed hashes.

    i'd suggest to make the class independent of the data layer (pdo).

    storing the secret key:
    at first you should make sure that the config file has only read access
    for the webserver.
    then it shouldn't be placed in htdocs. if that isn't possible, use a php
    configuration file.

  5. #5
    SitePoint Member
    Join Date
    Mar 2008
    Location
    Norway
    Posts
    20
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Only the username is hashed in authenticate() because the password is already hashed in the session storing it. Reason is I didn't want plain-text username AND password in the session because of security issues with shared hosting...hashing both username and password in the session would allow someone who got access to a session to use it directly against the database...with this setup he has to also get access to where the key is stored and hash the username as well...I'm not sure how much security this adds in a shared environment however, but it seems more secure to me?

    Will look into hash_hmac.

    Location of secret key; In a shared environment all user's "webserver" run with the same name (noname etc - unless suPHP is active), meaning that the webserver can read files in your home directory even if the script is executed in another... How "big" this threat is i'm not sure of, but i've seen hosts where this definatly is possible. Would really like some solid input on this.

  6. #6
    SitePoint Wizard silver trophy kyberfabrikken's Avatar
    Join Date
    Jun 2004
    Location
    Copenhagen, Denmark
    Posts
    6,157
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Lars Boldt View Post
    As for open-id, I remember reading an article not long ago that pretty much said open-id wasn't ready or had many flaws, don't remember where though, will look for it. What's your take on open-id as of now?
    Obviously I can't comment on the article, but the biggest flaw with OpenId is that it isn't implemented in enough places yet. That's something you could help with by supporting it.

  7. #7
    SitePoint Member
    Join Date
    Mar 2008
    Location
    Norway
    Posts
    20
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I don't know openid well enough to comment it, but one ring to rule them all seems abit scary if it's compromised somehow. Will definatly read more about it however and make a more educated decision later.

  8. #8
    SitePoint Wizard silver trophy kyberfabrikken's Avatar
    Join Date
    Jun 2004
    Location
    Copenhagen, Denmark
    Posts
    6,157
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Lars Boldt View Post
    I don't know openid well enough to comment it, but one ring to rule them all seems abit scary if it's compromised somehow. Will definatly read more about it however and make a more educated decision later.
    OpenId is more an infrastructure thing/protocol, than a system. As a user you pick your own OpenId provider. The good thing about it (security wise), is that most people reuse the same password for a lot of logins. Thus if just one of these places have weak security, you are compromised all around. In this scenario it's much better to just have your password in one place. Of course you still have the problem if this one place is compromised, but that's less likely because you'll generally pick some place with a good grasp on security.

  9. #9
    SitePoint Enthusiast
    Join Date
    Mar 2005
    Posts
    94
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Lars Boldt View Post
    Only the username is hashed in authenticate() because the password is already hashed in the session storing it. Reason is I didn't want plain-text username AND password in the session because of security issues with shared hosting...hashing both username and password in the session would allow someone who got access to a session to use it directly against the database...with this setup he has to also get access to where the key is stored and hash the username as well...I'm not sure how much security this adds in a shared environment however, but it seems more secure to me?
    i'd only keep a userid or (unique) username in the session and authenticate
    against the database only once on login(). reauthentication on each request
    adds no security because the session remains valid.

  10. #10
    SitePoint Member
    Join Date
    Mar 2008
    Location
    Norway
    Posts
    20
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    It adds a feature which I think can be useful if compromised. Lets say a user know his/her account has been compromised and the attacker might be using it right now...all the user has to do then is request a new password, once the database is updated, the attacker will be locked out of the system because the passwords no longer match and the original user doesn't have to deal with a banned account.

    If the session only stores a userid or similar the only option is to block the account, which might not be desireable.

  11. #11
    SitePoint Enthusiast
    Join Date
    Mar 2005
    Posts
    94
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    if the session is compromised you only need to logout.

  12. #12
    SitePoint Member
    Join Date
    Mar 2008
    Location
    Norway
    Posts
    20
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    true, if session hijacking or fixation was the issue. But if the attacker knew both the username / password and logged in to the system, account banning/blocking would be the only way to prevent him using the system.

    If I accidentaly gave away my credentials, I would feel better knowing that once i've changed password there was no way noone could continue with the old credentials.

  13. #13
    SitePoint Enthusiast
    Join Date
    Mar 2005
    Posts
    94
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    if username and password of an account are leaked the account is lost
    anyway. a sane attacker would set a new password himself, asap.

  14. #14
    SitePoint Member
    Join Date
    Mar 2008
    Location
    Norway
    Posts
    20
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    that depends entirely on how you implement a change of password.

    You could require an answer to a question if you want to change password/email directly from the account panel, something like mothers maiden name etc.

    This secret question wouldn't be changeable through the account panel, but would require you to click the link in an email that was sent to the registered email.

    An attacker would then need email access to be able to "take over" the account.

    Obviously this raises the issue of what happens if the original email is inaccessible and the user forgot the answer to the secret question, but thats another issue.

  15. #15
    Use The Cloud
    Join Date
    Jan 2006
    Location
    Boise, ID
    Posts
    556
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    The first thing I noticed is that you can type hint PDO in your constructor instead of checking inside of it. This saves code + makes testing easier.
    Brad Hanson, Web Applications & Scalability Specialist
    ► Is your website outgrowing its current hosting solution?
    ► PM me for a FREE scalability consult!
    ► USA Based: Available by Phone, Skype, AIM, and E-mail.

  16. #16
    . shoooo... silver trophy logic_earth's Avatar
    Join Date
    Oct 2005
    Location
    CA
    Posts
    9,013
    Mentioned
    8 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by bhanson View Post
    The first thing I noticed is that you can type hint PDO in your constructor instead of checking inside of it. This saves code + makes testing easier.
    Also to add this this, remove the try/catch it is pointless and is the wrong usage. No error checking should be done in a try block and most certainly you never throw an exception in a try block.

    ...haven't decided on what characters are going to be allowed for username/password, input here would be appreciated
    For the password, do not limit the allowed characters, let them enter anything they like because you should be hashing the passwords once you get them. And a hash normally is hex (0-9 a-f). Also do not set a maximum length for the password.
    Logic without the fatal effects.
    All code snippets are licensed under WTFPL.


  17. #17
    SitePoint Member
    Join Date
    Mar 2008
    Location
    Norway
    Posts
    20
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    and most certainly you never throw an exception in a try block.
    http://www.php.net/try - Isn't that what they are doing on this page?

    Anyways, changing to type hinting, i've missed that option entirely for some reason.

  18. #18
    SitePoint Member
    Join Date
    Mar 2008
    Location
    Norway
    Posts
    20
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Here's an interesting read on shared-hosting and security btw. It's from 2004, but I don't think this has changed since, would love to be proven wrong however.

    http://shiflett.org/articles/shared-hosting

    So where to hide that secret key and DB credentials?

  19. #19
    . shoooo... silver trophy logic_earth's Avatar
    Join Date
    Oct 2005
    Location
    CA
    Posts
    9,013
    Mentioned
    8 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Lars Boldt View Post
    http://www.php.net/try - Isn't that what they are doing on this page?
    No i mean...
    PHP Code:
    try {
        throw 
    Exception( ... );
    } catch ( 
    Exception $e ) {} 
    You don't want to do that. The whole point of a try/catch block is to execute code the functions/methods throw exceptions and the catch stops the code and does whatever. Zero error checking in the try block itself.
    Logic without the fatal effects.
    All code snippets are licensed under WTFPL.


  20. #20
    SitePoint Member
    Join Date
    Mar 2008
    Location
    Norway
    Posts
    20
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Point taken.

  21. #21
    SitePoint Addict SirAdrian's Avatar
    Join Date
    Jul 2005
    Location
    Kelowna, BC
    Posts
    289
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Usually you try executing code from another class, and catch its errors; if it's the same class there isn't much point. Use type hinting instead, in this case.

    You might want to split it up into a few classes. For example, you could make...

    -database access class encapsulating any database access (queries)
    -a session management class
    -an encryption class


    Keep the database verification on every page load. I'm all for that.
    Adrian Schneider - Web Developer


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •