Security Enhancement, Keys For Actions

[David]

This is designed to be used as a toolkit to create security mods.  What it allows you todo is instead of like resetting a password instead create a key and e-mail the user a confirmation if they really want to reset their password.  The code has not fully been tested but should work.  I have also provided the schema for the table required for this.  If you make updates to this code please post them in this thread.  Hope someone can make this useful.  This code can only be used in YaBBSE or PfaBB.

Code Select Expand

CREATE TABLE `yabbse_action_keys` (

`ID_MEMBER` int ( 25 ) NOT NULL UNSIGNED,
`action` VARCHAR( 10 ) NOT NULL ,
`key` VARCHAR( 32 ) NOT NULL ,
`expires` BIGINT( 20 ) NOT NULL ,
`datetime_submitted` BIGINT( 20 ) NOT NULL ,
`ip_submiter` VARCHAR( 15 ) NOT NULL ,
INDEX ( `username` , `action` , `key` )
)

Code Select Expand

function new_action_key($memid, $action, $expires)
{
   global $db_prefix, $REMOTE_ADDR, $db_passwd, $cleaned_action_keys;

   if($cleaned_action_keys != 1)
      clean_action_keys();
   
   $result = mysql_query('SELECT expires FROM '.$db_prefix.'action_keys WHERE ID_MEMBER='.$memid.' && action=\''.$action.'\' && key=\''.$key.'\'') or database_error(__FILE__, __LINE__);
   if(mysql_num_rows($result) > 0) //Key exists
      return false;
   else
   {
      $key = md5(time().$username.$db_passwd);
      
      mysql_query('INSERT INTO '.$db_prefix.'action_keys VALUES \''.$memid.'\', \''.$action.'\', \''.$key.'\', '.$expires.', '.time().', \''.$REMOTE_ADDR.'\'') or database_error(__FILE__, __LINE__);

      return $key;
   }
}

function check_action_key($memid, $action, $key)
{
   global $db_prefix, $cleaned_action_keys;
   
   if($cleaned_action_keys != 1)
      clean_action_keys();
   
   $result = mysql_query('SELECT expires FROM '.$db_prefix.'action_keys WHERE ID_MEMBER='.$memid.' && action=\''.$action.'\' && key=\''.$key.'\'') or database_error(__FILE__, __LINE__);
   if(mysql_num_rows($result) > 0)
   {
      return true;
   }
   else
      return false;
}

function clean_action_keys()
{
   global $db_prefix, $cleaned_action_keys;
   
   mysql_query('DELETE FROM '.$db_prefix.'action_keys WHERE expires<='.time()) or database_error(__FILE__, __LINE__);
   $cleaned_action_keys = 1;
}

[Chris Cromer]

I don't think the keys need an expire time do they?

After the key expires the user would be able to send another e-mail using the new key system. They just have to wait for it to expire before they could do it again.

[David]

I don't think the keys need an expire time do they?

After the key expires the user would be able to send another e-mail using the new key system. They just have to wait for it to expire before they could do it again.

Obviously a few more funtionc need to be written.  My thinking was that if a key existed then a new one could not be added, thus the purpose of the expire time.  Reasoning is to stop someone from sending 100s of password reset requests.

[Chris Cromer]

Yeah, I see the point in having a expiretime now... although after it expires the same problem exists.

So basically the guy causing the problems would be able to still cause problems, he would just have to wait longer between sending each e-mail. So it does pretty much no good... because I could make it send out the e-mails in intervals using a simple php script that refreshed after the expiretime.

[David]

So basically the guy causing the problems would be able to still cause problems, he would just have to wait longer between sending each e-mail. So it does pretty much no good... because I could make it send out the e-mails in intervals using a simple php script that refreshed after the expiretime.

But the expire time can be tweaked depending on the call to the new key function.  Thus an admin could choose their own expire time and not tell people what it is.

[Chris Cromer]

Ok well I guess that could be a good temp fix though, although I wouldn't rely on it because most likely the person you built this to stop, would just keep trying till it did go through.

[David]

Ok well I guess that could be a good temp fix though, although I wouldn't rely on it because most likely the person you built this to stop, would just keep trying till it did go through.

This is something I have been meaning to write for a while.  I will try and finish off the functions tommorrow.

[Jack.R.Abbit]

So maybe I'm missing something or its not there but how does it work so that you can't create a key if ther is already one?

Maybe I just need a bit more of a walk through on how it would be used.

Sounds like a really nice idea though

-Jack

[Omar Bazavilvazo]

Hmm... I always wondered why we didn't had a mod like this...

Sounds very great, i'll be waiting for it to be finished.
I'll give it a try after i finish a mod i'm doing right now.

[David]

So maybe I'm missing something or its not there but how does it work so that you can't create a key if ther is already one?

Maybe I just need a bit more of a walk through on how it would be used.

Sounds like a really nice idea though

-Jack

Walkthrough for forgotten password.

User requests a password reminder.

Reminder.php calls new_action_key which either returns a new key or false.  If it returns a new key then reminder.php sends out an e-mail containing this new key.  If it returns false, shows an error message that this action was already requested and the previous key has not expired.

User recieves key in e-mail and a link to the second part of the action.

Reminder.php calls check_action_key, if it returns true then it lets the user choose a new password.  If it returns false then the key is wrong.



A key is bound to an action which allows multiple actions to implement these functions when they are done and they will not conflict with each other.

One more idea I had for the expire time is to make it each time be somewhere between a low number of minutes and high number set by the admin.  Like a range of 30 to 90 minutes then the key could expire anytime during that window.  And since the expire time can be set in the call to new_action_key you could have a shorter time, 15 minutes for a forgotten password, while having a time like a week for activating a new account.

[Jack.R.Abbit]

I see.  So currently the new_action_key function does not have anything to check if should return a key or false.  Thats where I was tripping up.  I understand that it is a work in progress.

And the expire range would mean that each time a new key was generates it would randomly select a time within that range, unless you had explicitly specified an expire time.

One more thing, would it log somewhere the failed attempts to get a new key?

-Jack

[David]

Ok, updated it.  new_action_key now checks if there is an existing key for this username and for this action.

new_action_key also now requires an expire time be passed to it.  Thus the range idea would be implemented in the call to that function, not in the function.

The function clean_action_keys was added.  It deletes all expired keys.  It can either be run on every click on the board from like index.php or both new_action_key and check_action_key now check if it has been run on this click and if not then they run it.

I am planning to work on logging next.  How would you want logging to work, what should be logged?  Should any action be taken if there are a lot of errors?

[[Unknown]]

Code Select Expand

CREATE TABLE `yabbse_action_keys` (

`username` VARCHAR( 25 ) NOT NULL ,
`action` VARCHAR( 10 ) NOT NULL ,
`key` VARCHAR( 32 ) NOT NULL ,
`expires` BIGINT( 20 ) NOT NULL ,
`datetime_submitted` BIGINT( 20 ) NOT NULL ,
`ip_submiter` VARCHAR( 15 ) NOT NULL ,
INDEX ( `username` , `action` , `key` )
)


Why do it by username?  I would do it by ID_MEMBER.

-[Unknown]

[David]

Code Select Expand

CREATE TABLE `yabbse_action_keys` (

`username` VARCHAR( 25 ) NOT NULL ,
`action` VARCHAR( 10 ) NOT NULL ,
`key` VARCHAR( 32 ) NOT NULL ,
`expires` BIGINT( 20 ) NOT NULL ,
`datetime_submitted` BIGINT( 20 ) NOT NULL ,
`ip_submiter` VARCHAR( 15 ) NOT NULL ,
INDEX ( `username` , `action` , `key` )
)


Why do it by username?  I would do it by ID_MEMBER.

-[Unknown]

Good idea.  What is the vairable that ID_MEMBER is stored in, like username is $username.

Also is there a function to look up ID_MEMBER given a username?  Like the forgotten password the user inputs the username.

[Jack.R.Abbit]

I am planning to work on logging next.  How would you want logging to work, what should be logged?  Should any action be taken if there are a lot of errors?

I guess since you won't always have a username for the person requesting the action (maybe never) you can't really do much.  So if the "Key already exists" error gets logged into the normal board error log not much else you can do.  Only thing maybe is to ban that IP after like 10 failed attempts or something but history shows that the bastards doing bad things usually have ways around IP ban so it would not solve anything.

-Jack