Is this code secure enough

[eluser]veledrom[/eluser]
Hi,

I use code below to authenticate user login. I have questions though.

Thanks in advance

1. Is it good and/or secure approach?
2. How can I make it harder to break into?
3. Should I store any other dynamic or static data in database to make it more secure?


<b>DATABASE</b>

Code:

CREATE TABLE `users` (
  `id` mediumint(8) unsigned NOT NULL AUTO_INCREMENT,
  `username` varchar(20) NOT NULL,
  `password` varchar(40) NOT NULL COMMENT 'SHA1 encrypted password',
  PRIMARY KEY (`id`)
);

<b>CONFIG.PHP</b>

Code:

$config['encryption_key'] = "A1.b2,C3?D4_E5?";

<b>LOGIN PAGE</b>

Code:

<form action="http://localhost/index.php/loginout/do_login" method="post">
Username : <input type="text" name="text_username" value="" />
<br />
Password : &lt;input type="password" name="text_password" value="" /&gt;
<br />
&lt;input type="submit" name="submit_button" value="Login" /&gt;
&lt;/form&gt;

<b>CONTROLLER</b>

Code:

class Loginout extends CI_Controller {

public function __construct()
{
  parent::__construct();
}

public function hash_password($password)
{
  $salt  = $this->config->item('encryption_key');
  $hash  = sha1($salt . $password . $salt);
}

public function do_login()
{
  $username = $this->input->post('text_username', true);
  $password = $this->input->post('text_password', true);
  
  $this->db->where('username', $username);
  $this->db->where('password', this->hash_password($password));

  $query = $this->db->get('users', 1);  
  
  echo ($query->num_rows() == 1) ? 'SUCCESS' : 'FAIL';
}
}

[eluser]kr1pt[/eluser]
You should proccess user data in the model, not controller.

[eluser]porquero[/eluser]
Also you must validate form data. Use http://ellislab.com/codeigniter/user-gui...ation.html

[eluser]veledrom[/eluser]
Should I store any other dynamic or static data in database to make it more secure? Some people store session id etc..

[eluser]achilleusrage[/eluser]
I agree that you should validate the input. Also, you should check this out:

http://www.openwall.com/articles/PHP-Users-Passwords

You may want to store a unique salt for each user in the database (along side the hash). This makes it harder to crack should your user table ever be compromised. Lots of other tips at the link above. Particularly the section on password strecthing.

[eluser]veledrom[/eluser]
I'm sending plain PHP code without CI bits to reduce lines.

How do I validate user login if I use script below? I mean, since salt is dynamic how do I use in SELECT statement in VALIDATE USER LOGIN section below?

Thanks

Code:

<?php
/*
**
** DATABASE STRUCTURE **************************************************************
**
** CREATE TABLE `users` (
** `id` mediumint(8) unsigned NOT NULL AUTO_INCREMENT,
** `username` varchar(20) NOT NULL,
** `password` varchar(40) NOT NULL COMMENT 'encrypted password',
** `salt` varchar(20) NOT NULL COMMENT 'random key',
** PRIMARY KEY (`id`)
** );
**
*/


/*
** CREATE AN ACCOUNT IN DATABASE ***************************************************
*/

$username  = $_POST['username'];     //Get username from form
$password  = $_POST['password'];     //Get password from form

$ci_encryption_key = $this->config->item('encryption_key');  //Get CI's static key
$salt   = mt_rand();                 //Generate dynamic salt value
$hash   = sha1($ci_encryption_key . $password. $salt);  //Generate hash password

$sql = "INSERT INTO login (username, password, salt) VALUES ('" . $username . "', '" . $hash . "', '" . $salt . "')";


/*
** VALIDATE USER LOGIN *************************************************************
*/

$username  = $_POST['username'];     //Get username from form
$password  = $_POST['password'];     //Get password from form

$ci_encryption_key = $this->config->item('encryption_key');  //Get CI's static key

$sql = "SELECT username FROM login WHERE username = '" . $username . "' AND ?????????????????";

[eluser]veledrom[/eluser]
I guess the SELECT statement should be this one: Good approach????

Code:

SELECT
`username`
FROM `login`
WHERE
`username` = '$username' AND
`password` = (SELECT SHA1(CONCAT('$ci_encryption_key', '$password', `salt`)) FROM `login` WHERE `username` = '$username')

[eluser]porquero[/eluser]
Also I recommend you read:

http://ellislab.com/codeigniter/user-gui...urity.html

[eluser]Mhabub[/eluser]
Hello,

When you encrypt password while creating user account, you can add salt in to the password and generate the password. If you put salt in SHA1 method its hard to break the password.


Mhabub
http://www.developmentwall.com/

[eluser]skunkbad[/eluser]
[quote author="porquero" date="1330965731"]Also you must validate form data. Use http://ellislab.com/codeigniter/user-gui...ation.html[/quote]

How do you propose validating a username or password? Especially in regards to the password, which may contain special chars if it is a good password, it's difficult to use the validation that CodeIgniter provides. Since Active Record is properly escaping queries, then I just wonder if there are any examples of how OPs code is truly insecure. I'm truly interested in OP's questions, so please don't interpret my comments as saying I am right.