Форум PHP

Оцените грамотность данного скрипта, если он содержит ошибки, прошу указать…

<?php     // Если не определена константа ADMIN – завершаем работу скрипта     if(defined("ADMIN"))     {         // Начинаем сессию         session_start();         // Переменные с именем пользователя и пароля         $user = "user";         $pass = "pass";         // Проверям были ли посланы данные         if(isset($_POST["button"]))         {             // Проверяем было ли указанно имя пользователя             if(isset($_POST["text"]))             {                 // Проверяем был ли указан пароль пользователя                 if(isset($_POST["password"]))                 {                     // Присваиваем текущей сессии имя пользователя и пароль                     $_SESSION["text"] = $_POST["text"];                     $_SESSION["password"] = $_POST["password"];                     // Проверяем идентичность имени пользователя и пароля с текущей сессией                     if($_SESSION["text"] == $user && $_SESSION["password"] == $pass)                     {                         // Выводим нужное нам действие                         $default = "admin";                     }                     else                     {                         // Если данные не верны, выводим шаблон авторизации                         $default = "enter";                     }                 }             }         }         else         {             // Если ввода не было или они не верны, выводим шаблон авторизации             $default = "enter";         }     } ?>

не храните пароль в сессии

почему? лучше хранить на сервере а не у клиента (в куках)...

Авторизация на PHP

там нигде не сказано, что пароль надо хранить в сесии, да еще и в открытом виде.

а это что?

.. // Проверям были ли посланы данные   if(!empty($_POST['enter']))   {           $_SESSION['login'] = $_POST['login'];           $_SESSION['passw'] = $_POST['passw'];   } ..

В сессии это в чаще всего в файлах

Если хостинг виртуальный, чаще всего папка с временными файлами одна на все хосты
, или если кто-то вообще может по файловой системе папки tmp свободно прогуляться , то пароли в открытом виде это уязвимость.
Но если всё полностью на 100% неуязвимо, то можно хранить и в открытом

, Только на большинстве сайтов даже востановление возможно только повторной генерацией хэша пароля

и!?

..В основе механизма защиты будут лежать сессии, но в качестве альтернативы можно воспользоваться и cookie. Сессии являются более надёжным вариантом, так как в отличие от cookie хранятся на сервере и вероятность несанкционированного доступа к ним существенно снижена..

А зачем его хранить в таком виде вообще? Или так... а зачем его хранить вообще?

Если не хранить пароль в сессиях или куки, то тогда как по вашему работают почтовые и сервисы?

или куки можно? Ведь галочку которую мы часто нажимаен "запомнить" работает именно через куки

к тому же это выбор пользователя, он сам решает, хранить локально или не хранить...

По-моему автор ответа имел ввиду, что нестоит хранить пароль в сессиях в отрыктом виде, как это сделано в приведенном скрипте

с использованием md5!

<?php      // Если не определена константа ADMIN – завершаем работу скрипта     if(defined("ADMIN"))     {          // Начинаем сессию         session_start();         // Переменные с именем пользователя и пароля         $user = md5("user");         $pass = md5("pass");         // Проверям были ли посланы данные         if(isset($_POST["button"]))         {             // Проверяем было ли указанно имя пользователя             if(isset($_POST["text"]))             {                 // Проверяем был ли указан пароль пользователя                 if(isset($_POST["password"]))                 {                     // Присваиваем текущей сессии имя пользователя и пароль                     $_SESSION["text"] = md5($_POST["text"]);                     $_SESSION["password"] = md5($_POST["password"]);                     // Проверяем идентичность имени пользователя и пароля с текущей сессией                     if($_SESSION["text"] == $user && $_SESSION["password"] == $pass)                     {                         // Выводим нужное нам действие                         $default = "admin";                     }                     else                     {                         // Если данные не верны, выводим шаблон авторизации                         $default = "enter";                     }                 }             }         }         else         {             // Если ввода не было или они не верны, выводим шаблон авторизации             $default = "enter";         }     } ?>

а вот мой вар, пока еще недописанный правд:

<?php ## auth.php     $var = "&nbsp";     $hint = "Логин: demo Пароль: demo";     if(isset($_SESSION['aid'])){         unset($_SESSION['aid']);         $var = "Сессия устарела";         $hint = "Требуется переавторизация";     }         if($_SERVER['REQUEST_METHOD'] == 'POST'){         connect2DB();         $login = $_POST['login'];         $pass = $_POST['pass'];         $sql = "SELECT * FROM `users`                  WHERE `login`='$login'                    AND `pass`='$pass'";         $sql2 = "SELECT * FROM `admins`                   WHERE `aid` IN(                     SELECT `uid` FROM `users`                      WHERE `login`='$login'                        AND `pass`='$pass')";         $sqlex = mysql_query($sql) or die(mysql_error());         if(mysql_num_rows($sqlex) < 1){             $var  = "Пользователь не зарегистрирован";             $hint = "";         }else{             $sql2ex = mysql_query($sql2) or die(mysql_error());             if(mysql_num_rows($sql2ex) == 0){                 $var = "ты не админ=(";             }else{                 $_SESSION['aid'] = session_id();                 header("Location:.");             }         }     } ?>

лучше не дописывай

http://93.80.200.251/eshop/admin/auth.php - на, попробуй зайти в админку, если сможешь, а если так и не сможешь, то укажи мне на мои ошибки, типа незашифрованного пароля и так далее...

Специально для Вас написана Задача #11

Госпдя... инъекции...))) грю же, нету там пока еще никаких фильтров и проверок))) а задачу видел)

Я сделал тока одну ошибку - выкинул сюда неготовый вариант. Исправлюсь)))

А Вы полагаете, что фильтры и проверки навешивают на уже написанный код?
Отнюдь. Это код нужно писать так, чтоб никаких дополнительных фильтров и проверок он не требовал.

Нету фильтров и проверок?
А что есть-то?
Аутентификация сессионным кукисом?
Хранение пароля в открытом виде?
Два SQL-запроса (из которых один сложный) вместо простейшего LEFT JOIN?

....!

Взгляните на этот конспектик. Не все там идеально, но интересного много.

:)

Ужасный код, где фильтрация входных данных (авторизироваться тут легче простого без любых паролей)? И зачем таблица администраторов (это недостаток архитектуры)?

http://95.25.202.233/eshop/admin/auth.php. Вперед))) или ты очередной пустослов?:)

Табла админов нужна, потому что в админке есть раздел такой, администраторы называется. Так почему бы не обратиться сразу к таблице администраторов, а? К таблице юзеров-то все обращаются. Мож недостаток архитектуры?:)))

Для разграничения прав пользователей должны быть списки ACL или простой флаг is_admin в таблице пользователей.

Сразу видно, что код по ссылке http://95.25.202.233/eshop/admin/auth.php не совпадает с приведённым вами (или на сервере включена опция Magic Quotes).

Админы не одинаковы - у каждого, как сказать, своя группа... у кого-то дизайн, а кто-то, к примеру, может добавлять товарные позиции. Вы все эти флаги тоже предлагаете поместить в таблицу юзерс? или всё же нет? А админка довольно большая. Поэтому я выбрал вариант с таблицей админов, где и будут все эти флаги и подфлаги.

Ну тогда это не таблица администраторов, а список доступов.

1. Там не должны храниться пароли и логины, а только идентификаторы привязных пользователей. Выходит вы храните их в 2х местах.
2. Будет логичнее переименовать таблицу в rights.

И мне всё-ещё интересно по поводу моего предположения, код на сервере не совпадает или это мейджик квотс немного мешает? (скорее 1ое)

1. Нет! Вы не правы! В таблице администраторов хранится только id, как первиичный ключ, aid, который равен users.uid и есть поля типа enum для тех самых разделов со значениями 0,1. Ведь выше по треду я так и пытался запросить в запросе (каламбур) - есть ли в admins такая строка, где admins.aid = users.uid. Никаких паролей.
2. Переименовал)

escape_string мешает) Ну, может я где-то и идиот, но не до такой же степени))) Я же говорил что код не дописан. Признал ошибку. Впредь пишу всё и сразу)

По поводу идентификатора - сорри, я не заметил что это просто запрос кривоват =). Вот посмотрите:

$sql2 = "SELECT * FROM `admins`                    WHERE `aid` IN(                      SELECT `uid` FROM `users`                       WHERE `login`='$login'                         AND `pass`='$pass')";

На момент этого запроса у вас уже есть результат предыдущей выборки, потому его можно упростить до:

$sql2 = "SELECT * FROM `admins`                    WHERE `aid` = '{$id}' LIMIT 1;

Где $id вы получаете как результат с предыдущей выборки (те вы же выполнили запрос, так получите данные о пользователе). И рекомендую ставить лимит в 1, чтобы база при нахождении пользователя не выполняла больше никаких действий.

Разница заметна, правда? :)

конечно:)

В сессии лучше хранить isAuth = true и всё, никаких паролей.

Оцените моё творение:

<?php function login_form() { if (isset($_POST['username']) && isset($_POST['password'])) {         include($_SERVER["DOCUMENT_ROOT"].'/database.php');     $username = mysql_real_escape_string($_POST['username']);      $password = mysql_real_escape_string($_POST['password']);      $query = mysql_query("SELECT * FROM users WHERE user = '$username' and password = MD5('$password')");     $row = mysql_fetch_array($query);     if(mysql_num_rows($query) == 1)     {         session_start();         $secret_code = 'о8*Оо7n7Jik3#'.$_SERVER['HTTP_USER_AGENT'].$_SERVER['HTTP_ACCEPT_CHARSET'];          $_SESSION['fingerprint'] = md5($secret_code.session_id());         $_SESSION['admin_user'] = $username;                  //Установка флагов для разграничения приоритетов         if ($row['add'] == 1)    $_SESSION['add'] = md5($secret_code.'add_access');         if ($row['edit'] == 1)    $_SESSION['edit'] = md5($secret_code.'modify_access');         if ($row['del'] == 1)    $_SESSION['del'] = md5($secret_code.'delete_access');         if ($row['mod'] == 1)    $_SESSION['mod'] = md5($secret_code.'moderate_access');                  setcookie("cookies","1");                   if(!isset($_COOKIE['cookies']))         {             header("location:panel.php?".session_name().'='.session_id());         }         else // cookie включены         {             header("location:panel.php");         }     }      else     {         login_form();         echo '<srtong><b>Ошибка!</b><br>Неверно введен логин или пароль</strong>';     } } else {     login_form(); } } ?>

Хочется услышать отзывы по поводу корректности кода)))

Попробуйте сделать систему аутентификации следующим образом:

Пользователь вводит логин/пароль: ему ставится кука (логин) + присваивается идентификатор сессии = пользователь авторизован, доступ к секретным данным открыт.

Пользователь нажимает на кнопку выход: остается кука (логин) и остается идентификатор сессии = пользователь не авторизован, доступ к секретным данным закрыт.

Короче, системе должно быть наплевать, имеется ли кука+айди сессии, доступом к авторизации должно быть только знание пароля от акаунта.

..

Не понял, а если мне надо осуществлять переходы на другие страницы, как система узнает что пользователь авторизован? Если ей наплевать на логин + идентификатор сессии

просто куки и/или идентификатор сессии не должны выполнять роль автологина, тогда теряется смысл кражи этих данных.

тогда теряется смысл этих самых кук, зачем воровать что-то бессмысленное? зачем вообще устанавливать что-то бессмысленное?

Можно делать присваивать жизнь кукам, или менять указанный у меня в коде $secret_code раз в 3 дня и для каждого пользователя делать его уникальным - но это уже паранойя ИМХО)

>Короче, системе должно быть наплевать, имеется ли кука+айди сессии, доступом к авторизации должно быть только знание пароля от акаунта.

Знание пароля от эккаунта - это чересчур.
Одно дело - автологиниться в портале с данного браузера данной машины, и другое (например) менять данные своего профиля.
Знание, хранимое в куке должно допускать первое, но его должно быть недостаточно, чтобы можно было выполнить второе.
Знание пароля дает доступ ко всему.

У вас есть замечания по поводу моего кода?

Именно по поводу кода? Или по поводу алгоритма?
Впрочем, по поводу кода тоже есть. Его не слишком легко читать.
Кроме того всё же формально Location: следует делать на абсолютный адрес, а не относительный.

По поводу алгоритма главное замечание к тому, что authentication credential (аутентифицирующим жетоном) у Вас является опять таки сессионный ключ, а все проверочные данные легко перехватываются сторонним сайтом. Вы же понимаете, что едва ли найдется браузер, который будет подсовывать разные USER_AGENT и ACCEPT_CHARSET при заходе на разные сайты.

а смысл делать Location: абсолютным? Что делать если проект вдруг придётся перенести на другой домен?

Защита от перехвата это замена ACCEPT_CHARSET на REMOTE_ADDR и всё, этого достаточно? Да и как хакер после перехвата узнает, из чего именно состоит $_SESSION['fingerprint'] для того что-бы подделать подпись, пусть даже можно подделать агента и кодировку, он же должен знать что я в подписи использую именно эти значения а не что-то другое.

P.S. Что именно сложно читать? Как избавиться от этого?

>а смысл делать Location: абсолютным?

Есть такое слово - RFC

>Что делать если проект вдруг придётся перенести на другой домен?

И что? $_SERVER['HTTP_HOST'] уже не катит?

>Защита от перехвата это замена ACCEPT_CHARSET на REMOTE_ADDR и всё, этого достаточно? Да и как хакер после перехвата узнает, из чего именно состоит $_SESSION['fingerprint'] для того что-бы подделать подпись, пусть даже можно подделать агента и кодировку, он же должен знать что я в подписи использую именно эти значения а не что-то другое.

Он посмотрит в исходник.
При анализе аспектов безопасности предполагают, что исходные тексты являются открытой информацией. Закрыты лишь ключи.

>P.S. Что именно сложно читать? Как избавиться от этого?

Добавить комментарии, описывающие процесс, а также суть применяемых объектов.

>Он посмотрит в исходник.
>При анализе аспектов безопасности предполагают, что исходные тексты являются открытой информацией. Закрыты лишь ключи.

Дык в данном случае $secret_code как раз и является ключом, который будет закрыт.

>Есть такое слово - RFC

Спасибо, на будущее учту!

Ключом может являться лишь 'о8*Оо7n7Jik3#' , и то, после того как Вы её отправите куда-нибудь из исходного текста.
Остальное - алгоритм.
Алгоритм открыт по определению.
Я даже смотреть не стану на закрытый алгоритм.

>а смысл делать Location: абсолютным? Что делать если проект вдруг придётся перенести на другой домен?
SERVER_NAME для этого и создан.

по-моему, SERVER_NAME - это немножко другое.

1. Разграничение прав доступа сделано слишком уж примитивно
2. Лучше использовать функцию хеширования (md5) в коде, а не в запросе
3. Добавьте соль в пароль (антибрутфорс)
4. Добавьте запрет на слишком частые попытки авторизации, можно добавить капчу
5. Проверка кукисов реализована сомнительно, возможно они не проверятся так
6. Вывод ошибок корявый, не смешивайте код и представление
7. > $_SERVER['HTTP_ACCEPT_CHARSET'];
Лучше не кодировку, а айпи адрес.

Провайдер может поменять IP во время сессии ,и свой станет чужим.

Провайдер (поставщик услуг интернета) во время сессии IP не меняет.

> Антибрутфорс

это что?

brute force - метод грубой силы. Фактически - перебор по всей области. Соль против него ничего не дает.
Собственно, против него вообще методов борьбы нет, если не называть ими ограничение на частоту неудачных попыток авторизации(ну и до некоторой степени - капчу).

как проверять сколько раз пользователь пытался авторизоваться? где логичнее хранить данное значение? в сессии что-ли?

Брутфорс может применятся к хешу, а не к форме. Тогда даёт.

Не дает. Когда известен хеш, известен и init vector. Эти сущности берутся из одного источника.

init vector всего лишь осложняет процесс атаки.
Затрудняет построение радужных таблиц, к примеру.
Брут форс, по определению - метод тупого перебора попыток.
И его не блокирует никакая панацея. Может только осложнить жизнь.

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

Вот тут позвольте не согласиться.
Пароль сменится, ну и что? Кто Вам сказал, что брутофорсить нужно начинать с самого начала? Уже следующее значение может оказаться верным. Согласен, это крайнемаловероятно, но вероятность существует, и скаждым последующим значением брута она увеличивается :)

Вы не правы, с ростом длинны пароля растёт сложность его подбора. Ещё дело атакующему усложняет то, что соль может состоять с спецзнаков и область перебора станет значительно больше.

А про шанс "угадать" - совсем чушь, тогда что мешает просто угадать пароль в форме авторизации на сайте? Теорию вероятности учили? Пусть такой человек лучше пойдёт в лотерею поиграет, у него и то шанс больше будет.

У нас, с хорошей солью, длинна алфавита для перебора будет примерно 72 символа и при итоговой длине пароля в 10 символов для полного перебора потребуется примерно 462539 лет. Даже если вам повезёт и вы угадаете через 231 год (взял как пример 1/2000 всего срока), то будете ли вы достаточно живы чтобы воспользоваться паролем? А уж если добавить спецсимволы и соль сделать подлиннее...

вот как раз брутофорсу плевать на вашу соль, и перебирать он будет пароли пользователя в 10 символов и вряд ли там будут спец знаки. Не путайте с подбором пароля по известному хешу.
Брутофорс используют как крайнюю меру, когда уже больше ничего не помогает.
___
у кого где-нибудь стоит пароль больше 10 символов?

Как я уже говорил, брутфорс - название метода, который может применяться как к форме авторизации (непосредственный пароль), так и к хешу (полный перебор комбинаций). Защита от таки на форму - отдельный разговор.Читать что я писал раньше не пробовали?

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

А Вы пробовали читать, что-нибудь кроме того что пишете Вы?
И раз уж я поимел хеш из Вашей базы, то я буду иметь Вашу базу до тех пор пока не назначу своему аккаунту права администратора, как вариант. Но брутофорсить хеш... увольте.

> А Вы пробовали читать, что-нибудь кроме того что пишете Вы?

Я на ваши вопросы отвечаю чётко с учётом сказанного в этой ветке ранее, чего вы не делаете. Или я что-то упустил?

> И раз уж я поимел хеш из Вашей базы, то я буду иметь Вашу базу до тех пор пока не назначу своему аккаунту права администратора, как вариант. Но брутофорсить хеш... увольте.

Есть масса различных ситуаций, когда удаётся получить только хеш пароля, но не его целиком. Если вы будете каждый раз идеализировать ситуацию в вашу сторону (поимею всю базу, угадаю пароль с первого раза и т.п.), то спор будет вечным и, увольте, спорить о таком я не буду, это глупо.

1. Разграничение прав доступа я вообще делал впервые, потому не знаю как это делать правильно, может есть где почитать или это надо на ООП делать?
Потом открою новую тему на этот счёт

2. Какая разница где использовать ф-цию хеширования?
5. Ну работает, хотя может и сомнительно :)
6. Что значит корявый, как сделать нормально?
7. переделаю

1. Существует огромное количество способов, там только их описания достойны нехилой статьи.
2. Оптимизатор у MySQL порой ведёт себя как колдун с бубном, лучше его лишними функциями не нагружать =).
6. Аналогично 1.

любой из примеров в студию. Просто я не понимаю чем плох мой :)
или делать что-то вроде try {} catch {} ?

У меня так:
1. При ошибке выбрасывается исключение (ex.: throw new DB_Exception('SomeID')) с
2. Обработчик ошибок перехватывает исключение
3. Обработчик записывает техническую информацию в лог ошибок
4. Вызывается шаблон, который соответствует уровню ошибки и в нём выводиться красивое сообщение для пользователя (сам текст сообщения ищется по идентефикатору ошибки, SomeID).
5. Если нужно, то приложение заканчивает работу.

Так в зависимости от уровня ошибок можно или завершить работу или вывести на страничке сообщение или накопить все ошибки и вывести их списком. За вывод отвечает шаблонизатор.