Версия для печати темы
Нажмите сюда для просмотра этой темы в оригинальном формате
Форум программистов > PHP: Общие вопросы > Авторизация на форуме


Автор: sugee 22.5.2006, 17:52
Привет всем. Есть у меня одна небольшая просьба.
Вот начал форум писать, закончил вроде систему регистрации-авторизации,  но есть сомнения...
Насколько всё это надёжно, какие есть дыры в безопасности, что нужно исправить или улучшить?

Посмотреть и потестировать можно здесь http://restoran-kaz.ho.com.ua/forum/ 

Исходный код я присоединил, так что сами скрипты я сюда постить не буду, покажу только
структуру БД
Код

use `forum`;

CREATE TABLE `header`
(
  `parent` INT NOT NULL, 
  `author` char(25) NOT NULL,
  `title` char(50) NOT NULL,
  `child` INT DEFAULT 0 NOT NULL,
  `view` INT DEFAULT 0 NOT NULL,
  `area` INT DEFAULT 1 NOT NULL,
  `posted` INT(10) not null,
  `postid` SMALLINT NOT NULL AUTO_INCREMENT, 
   PRIMARY KEY ( `postid` )
);


CREATE TABLE `message`
(
  `postid` INT UNSIGNED NOT NULL AUTO_INCREMENT,
  `posted` INT(10) not null,
  `mess` TEXT, 
   PRIMARY KEY ( `postid` ) 
);


CREATE TABLE `members` 
(
  `id` SMALLINT NOT NULL AUTO_INCREMENT,
  `name` varchar(40) NOT NULL,
  `time` INT(14) NOT NULL,
  `securid` TINYTEXT NOT NULL,
  `password` TINYTEXT NOT NULL,
  `messages` INT DEFAULT 0 NOT NULL,   
   PRIMARY KEY ( `id` ) 
);


Для авторизации используется таблица members.

Код я подробно прокомментировал, особо меня интересует вот этот момент
Код

//хэш безопасности который формируется из логина и уникального //времени регистрации  при регистрации он записывается в БД и в //cookie, а при авторизации используется для  извлечения имени //пользователя из БД (имя присваивается переменной сессии)  
//**************************************************************************************
function getsecurid( $user_name , $reg_time )
 {  return sha1( $user_name ) . sha1( $reg_time );  }



Я основывался на алгоритме описанном вот здесь http://vingrad.ru/PHP-PHPSCRIPTS-002849, но насколько эффективно
у меня эта идея реализована?  Я не вводил ограничений на браузер или на IP, но хотел по возможности исключить
возможность подмены куки.



P.S. Естественно дальше всё будет усложняться, поскольку будут разные уровни доступа: админы,  модераторы, обычные пользователи.   

Автор: smartov 22.5.2006, 18:35
Мне кажется что те скрипты что ты выложил не отвечают тем, что реально есть.
Потому что в выложенный есть дырка в авторизации.
На сайте же она не проявляется.

Добавлено @ 18:35 
Или у тебя magic_quotes включены 

Автор: smartov 22.5.2006, 18:54
А не smile
Я просто запрос неверно составлял smile

После правильного пустило под админом smile
Создал тему "В форуме дырка" smile 

mysql_real_escape_string не забывай 

Автор: sugee 22.5.2006, 19:03
smartov,  ладно,  авторизацию мою ты сломал,  может расскажешь где та дырка,  которой ты воспользовался?

Цитата(smartov @  22.5.2006,  18:35 Найти цитируемый пост)
Мне кажется что те скрипты что ты выложил не отвечают тем, что реально есть.

Да нет,  только что самую свежую версию закачал на сервак и её же здесь
выложил.

Добавлено @ 19:04 
Цитата(smartov @  22.5.2006,  18:54 Найти цитируемый пост)
mysql_real_escape_string не забывай  
Только это?
 

Автор: smartov 22.5.2006, 19:16
Дырка в запросе на авторизацию, куда ты пихаешь $name предварительно не взяв его в mysql_real...

Цитата(sugee @  22.5.2006,  18:03 Найти цитируемый пост)
Только это?

Да. Этого достаточно чтобы тот mysql-injection (погугли эту тему если интересно) не сработал.  

Автор: sugee 22.5.2006, 20:21
Так,  это я поправил.   Про mysql_real_escape_string я конечно знал,  но 
не понимал до конца.   Буду разбираться.   

Автор: madFobos 22.5.2006, 21:14
1. Если posted - это дата, то лучше использовать тип DATETIME или TIMESTAMP вместо INTEGER
2. Все char лучше заменить на varchar (в твоем конкретном случае БД будет меньше места занимать)
3. И совет на будущее, не называй идентификаторы таблиц одинакого (postid), потом запутаешься... Лучше сделай header_id и message_id. Хотя я вобще недопонял в твоей структуре через какую переменную идет связь таблицы header и message (надеюсь не postid)  

Автор: sugee 22.5.2006, 22:19
Цитата(madFobos @  22.5.2006,  21:14 Найти цитируемый пост)
я вобще недопонял в твоей структуре через какую переменную идет связь таблицы header и message (надеюсь не postid)   

Вообще-то именно через postid.  
Вот так я выбираю сообщение из базы
Код

select  h.`parent`, h.`author`, h.`title`, h.`child`, h.`postid`, 
  h.`posted`, m.`mess`
  from `header` h, `message` m where m.`postid`=' ".$CurrentId." ' and h.`postid`=' ".$CurrentId." '

Причём хотя это поле я первоначально задумывал как AUTO_INCREMENT (оно и сейчас AUTO_INCREMENT),  но я в него вставляю предварительно
вычисленное значение,  которое не совпадает ни с одним из уже существующих postid.  Абсурд конечно,  но работает.  Я просто не придумал другого способа связать header и message.   Так что AUTO_INCREMENT у поля postid можно смело убрать, он не нужен  но вроде и не мешает.


 
Цитата(madFobos @  22.5.2006,  21:14 Найти цитируемый пост)
Если posted - это дата, то лучше использовать тип DATETIME или TIMESTAMP вместо INTEGER

Я  как-то уже привык хранить TIMESTAMP в поле типа INT. 

Автор: -=Ustas=- 22.5.2006, 23:11
Цитата(sugee @  22.5.2006,  22:19 Найти цитируемый пост)
Я  как-то уже привык хранить TIMESTAMP в поле типа INT.  

И правильно делаешь ;) 

Автор: smartov 23.5.2006, 10:04
sugee, 
p.s. Не знаю обращал ли кто твое внимание, но у тебя код практически нечитаемый из-за отсутствия форматирования.
Потом сам же не сможешь прочитать.
Очень советую почитать вот это: http://pear.php.net/manual/en/standards.php
(пройди по всем ссылкам)
Эти стандарты форматирования можно сказать классические. 

Автор: sugee 23.5.2006, 11:15
А я,  между прочим,  на этот раз старался уделять внимание форматированию.   Значит недостаточно.  Хорошо,  почитаю стандарты -  пройдусь по коду ещё раз.


Цитата(smartov @  22.5.2006,  19:16 Найти цитируемый пост)
достаточно чтобы тот mysql-injection (погугли эту тему если интересно) не сработал

Нагуглил интересный факт, оказывается mysql_escape_string и mysql_real_escape_string не экранируют '%' и '_'.   
В SQL это любая строка и любой одиночный символ.
 

Автор: smartov 23.5.2006, 12:07
Цитата(sugee @  23.5.2006,  10:15 Найти цитируемый пост)
Нагуглил интересный факт, оказывается mysql_escape_string и mysql_real_escape_string не экранируют '%' и '_'.   
В SQL это любая строка и любой одиночный символ.

На твою систему аторизации это никак не повлияет 

Автор: madFobos 24.5.2006, 21:06
Цитата(sugee @  22.5.2006,  22:19 Найти цитируемый пост)
Причём хотя это поле я первоначально задумывал как AUTO_INCREMENT (оно и сейчас AUTO_INCREMENT),  но я в него вставляю предварительно
вычисленное значение,  которое не совпадает ни с одним из уже существующих postid.  Абсурд конечно,  но работает.  Я просто не придумал другого способа связать header и message.   Так что AUTO_INCREMENT у поля postid можно смело убрать, он не нужен  но вроде и не мешает.


В таком случае нужно оставлять AUTOINCREMENT только у одной таблицы (в данном случае думаю у header). И не придется делать лишних вычислений (СУБД ведь для этого и создаются, чтобы делать работу за программистов smile.

Цитата(-=Ustas=- @  22.5.2006,  23:11 Найти цитируемый пост)
Я  как-то уже привык хранить TIMESTAMP в поле типа INT.  

И правильно делаешь ;)  


Спорный вопрос, особенно при использовании MySQL ближе к пятому. Там куча функций для работы с датами, которые к типу  INT не применить никак... К тому же При TIMESTAMP можно вобще о дате не забоиться при вставке (и апдейте при желании)...
 

Автор: sugee 24.5.2006, 22:37
Цитата(madFobos @  24.5.2006,  21:06 Найти цитируемый пост)
В таком случае нужно оставлять AUTOINCREMENT только у одной таблицы 
 smile 
Блин точно ведь,  как я сам не додумался! 
А не додумался,  потому что забыл о существовании функции mysql_insert_id().  

А я смотри что делал 
Код

function NewPostid() 
{
//выбираем postid всех уже существующих постов
$post_exists_query = mysql_query("select `postid` from `header`");
 
 //создаём массив всех существующих postid
   $post_exists_array = array();
  while($post_exist = mysql_fetch_assoc($post_exists_query)) { 

     $post_exists_array[] = $post_exist['postid'];
     
  }

  if(count($post_exists_array) !== 0)  {
//находим максимальный номер поста
     $MaxPostid = max($post_exists_array);

//задаём postid добавляемого поста 
     $NextPostid = ++$MaxPostid;
}  
  else
    {  $NextPostid = 1;  }

       return $NextPostid;
}
 smile 
  

Автор: sugee 24.5.2006, 23:10
Цитата(sugee @  24.5.2006,  22:37 Найти цитируемый пост)
А я смотри что делал 

К тому же у меня могла бы получиться коллизия, в случае если бы два
юзера одновременно добавили сообщение.
Могли бы получиться два поста с одинаковым postid.
Хотя поскольку поля postid всё-таки AUTO_INCREMENT, мускул 
не пропустил бы два одинаковых значения. Один из одновременно выполняющихся  инсертов просто бы не прошёл.

 

Автор: madFobos 25.5.2006, 12:07
Цитата(sugee @  24.5.2006,  23:10 Найти цитируемый пост)
К тому же у меня могла бы получиться коллизия, в случае если бы два
юзера одновременно добавили сообщение.
Могли бы получиться два поста с одинаковым postid.
Хотя поскольку поля postid всё-таки AUTO_INCREMENT, мускул 
не пропустил бы два одинаковых значения. Один из одновременно выполняющихся  инсертов просто бы не прошёл.


Для этого, кстати, существует такая вещь как LOCK TABLE..., а INNODB кажется вобще автоматом блокируют таблицы.
 

Автор: sugee 27.5.2006, 00:11
Цитата(madFobos @  24.5.2006,  21:06 Найти цитируемый пост)
В таком случае нужно оставлять AUTOINCREMENT только у одной таблицы

убрал AUTO_INCREMENT в таблице message.
Вставку теперь делаю так
Код

 mysql_query("insert into `header` values(
                                           '".$parent."', 
                                           '".$author."', 
                                           '".$title."', 
                                           '',
                                           '', 
                                           '".$area."', 
                                           '".$posted."',
                                           '' ) 
                                         ");

 $CurrentPostid = mysql_insert_id ();

 mysql_query("insert into `message` values(
                                            '".$CurrentPostid."',
                                            '".$posted."',
                                            '".$mess."' )
                                         ");


Функционально, конечно, ничего не изменилось, просто раньше я делал
это через ж...      ИМХО не всё равно, каким способом ты добиваешься своей цели, особенно  если выкладываешь код на всеобщее обозрение.
В общем спасибо, что открыл мне глаза на очевидную вещь 

Автор: sugee 27.5.2006, 23:07
Пора писать админку...  Админку то я напишу, но в связи с этим возникает  один очень важный вопрос. 
Как наилучшим образом (с точки зрения безопасности, естественно) организовать  доступ у ней? 
В БД, в таблице members я добавил поле status, в котором могут быть три значения:  'us' - пользователь, 'adm' - админ и 'moder' - модератор.
Любой кто зарегистрировался на форуме естественно получает статус `us`.
Вопрос в том, каким образом должен осуществляться доступ к интерфейсу администрирования для пользователя у которого в поле `status` записано 'adm'.

Можно конечно в общей для всех функции авторизации проверять статус, и если  статус админ, то выводить ссылку для входа в админку.
При этом если админка будет поключаться через index.php, то доступ будет осуществлятся  через GET, что мне явно не подходит. 
Если же это будет ссылка на отдельный скрипт,    то тот кто каким-то образом узнал его адрес, сможет зайти без всякого пароля.
На этот случай при входе в админку ещё раз проверяем имя записанное в сессии,    и смотрим действительно ли у человека с таким ником статус админа.
В общем вот что я хотел спросить,  мне остановиться на последнем варианте, или   есть идеи получше? 

Автор: madFobos 28.5.2006, 02:06
А чем тебя не устраивает доступ через GET? Вобще доступ к любой разделу сайта лучше делать через одну страницу (типа index.php). Далее ты можешь создать папку типа admin с админкой и делать запрос типа

http://site.ru?mod=a&action=some

В index.php проверяешь соответственно его на существовине как-то так

Код

if ($_GET['mod'] == 'a' && file_exists(SITE_DIR.'/admin') && $_SESSION['user']['status'] == 'adm')
{
  include(SITE_DIR.'/admin/'.$_GET['action'].'php');
}
else
{
  die('Hey, go out from here! :)))');
}


Еще более навороченный вариант, ты можешь создать таблицу типа rights и указать права на все папки, а через index.php соответсвенно грузить их только по правам.

И если тебе не нравится вариант http://site.ru?mod=a&action=some, используй mod_rewrite и можешь получить нечто вроде http://site.ru/mod/a/action/some.html 

З.Ы. Ну и самое простое ты можешь создать папку admin и тупо запаролить ее с помощью .htaccess, но это не лучший вариант, т.к. динамика при этом сильно теряется. 

Автор: sugee 28.5.2006, 10:00
Цитата(madFobos @  28.5.2006,  02:06 Найти цитируемый пост)
А чем тебя не устраивает доступ через GET?
Так ведь весь путь к закрытому разделу в адресной строке отображается.   
Авторизация на главной странице форума у меня производится по куке,
по этому в данном случае доверять только ей нельзя.   
Поэтому после перехода по ссылке админ должен будет как обычно ввести свой логин-пароль.  Вобщем, так буду делать.
 

Автор: sugee 28.5.2006, 15:59
Послушайте друзья,  а как обычно выглядит админка на форуме, я никогда не видел  smile 
Можно конечно сделать для администрирования отдельный интерфейс, где так же как обычно просматриваются темы, но напротив каждого поста стоят  кнопочки "удалить", "редактировать" и т.д.  Тогда в админке придётся продублировать все функции вывода.

Но мне представляется заманчивой такая картина.  Админ заходит на форум.   Поскольку его статус 'adm', под приветствием  наверху страницы появляется ссылка  "Администрирование".  Перейдя по этой ссылке он попадает на страницу для ввода  логина и пароля администратора, чтобы подтвердить что он действительно
администратор.

После подтверждения он получает сессию, которая даёт ему доступ к функциям  администрирования  и  перенаправляется на главную страницу.
Когда он будет просматривать тему, для него на панели инструментов будут  выведены те самые кнопочки "удалить", "редактировать" и т.д., помимо  стандартных кнопок типа цитаты или инструментов форматирования.

То есть отдельного админского интерфейса не будет, просто пользователь  с более высоким статусом будет иметь в распоряжении дополнительные кнопки.
Как вам такой вариант?  

Автор: madFobos 29.5.2006, 10:21
Цитата(sugee @  28.5.2006,  10:00 Найти цитируемый пост)
Так ведь весь путь к закрытому разделу в адресной строке отображается.   
Авторизация на главной странице форума у меня производится по куке,


В том-то и дело, что раздел закрытый и даже если ты его знаешь без авторизации все-равно не попадешь (если ты не кул-хацкер конечно smile. 

Цитата(sugee @  28.5.2006,  15:59 Найти цитируемый пост)
То есть отдельного админского интерфейса не будет, просто пользователь  с более высоким статусом будет иметь в распоряжении дополнительные кнопки.
Как вам такой вариант?  


Непонятно только зачем ссылка "Администратирование". Вошел под админом, показывается твой статус и все. Вобще непонятно, зачем повторно вводить логин и пароль? Как это безопасность-то повысит? 

Автор: sugee 29.5.2006, 21:45
Цитата(madFobos @  29.5.2006,  10:21 Найти цитируемый пост)
Вобще непонятно, зачем повторно вводить логин и пароль?
Нет,  пароль вводится один раз. Когда человек(с любым статусом) заходит на сайт его сначала узнают по куке.  
В куке содержится такая вещь
Цитата(sugee @  22.5.2006,  17:52 Найти цитируемый пост)

//хэш безопасности который формируется из логина и уникального //времени регистрации  при регистрации он записывается в БД и в //cookie, а при авторизации используется для  извлечения имени //пользователя из БД (имя присваивается переменной сессии)  


Для того чтобы простой пользователь был авторизован этого достаточно.
Но админ обязательно должен ввести пароль,  поскольку с его компа может зайти и кто-то другой.
 

Powered by Invision Power Board (http://www.invisionboard.com)
© Invision Power Services (http://www.invisionpower.com)