[PHP] Funkcja generująca jednorazowy klucz oraz funkcja sprawdzająca., Czy jet wystarczająco dobrze napisana? Opcje

[andrzejt17]

Witam. No więc wymyśliłem sobie taki bajerek:

Funkcja generująca klucz:

[PHP] pobierz, plaintext 
function generateKey() { 
	$_SESSION['secureKey'] = md5(uniqid());
}
[PHP] pobierz, plaintext 

Funkcja sprawdzająca klucz:

[PHP] pobierz, plaintext 
function checkSecureKey() { 
	if(isset($_GET['key']) && isset($_SESSION['secureKey']) && $_GET['key'] == $_SESSION['secureKey']) {
		unset($_SESSION['secureKey']);
		return true;
	}
	unset($_SESSION['secureKey']);
	return false;
}
[PHP] pobierz, plaintext 

Przykład użycia:

[PHP] pobierz, plaintext 
generateKey();
print ' <form name="testForm" method="post" action="test.php?key='.$_SESSION['secureKey'].'">
			<input type="submit" name="" value="testuj">
			Testowe pole: <input type="text" name="testowe">
		</form>';
[PHP] pobierz, plaintext 

i sprawdzanie:

[PHP] pobierz, plaintext 
if(checkSecureKey()) {
	// poleci z formularzem
} else {
	header('Location test.php');
}
[PHP] pobierz, plaintext 

Czy to wystarczające zabezpieczenie przed np. wysłaniem formularza z innego serwera albo przed ingerencją użytkownika w formularz i przekazywane dane?

Ten post edytował andrzejt17 14.05.2011, 20:48:40

[lobopol]

Nie

[andrzejt17]

Widzę, że idzie opornie. Wiem, że to sens na pewno jakiś ma. Pytanie więc, czy można to bardziej ulepszyć.

[lobopol]

Można np. dodając captche. Zadałeś proste pytanie to dostałeś prostą odpowiedź. To co zrobiłeś od biedy można nazwać csrf, zabezpiecza to tylko przed tym, że użytkownik nie zostanie przez inną witrynę zmuszony do wysłania formularza. Nie zablokuje bota.

[andrzejt17]

Nie no, nie chodziło mi o bota Captcha oczywiście tradycyjnie będzie. Zabezpiecza też przed tym, że jak użytkownik da 'wstecz' w przeglądarce i będzie chciał ponownie wysłać formularz to już mu go nie wyśle. Zabezpiecza też skrypt przed ponownym jego wykonaniem (f5) ;p

[lobopol]

I nie dawaj tego w get tylko jako pole hidden w formularzu, na pewno wizualnie będzie lepsze. Dodatkowo ma kolejną wadę mając otwarty 2 razy ten formularz tylko drugi będzie można wysłać, pierwszy będzie miał już podmienionego tokena

[PHP] pobierz, plaintext 
$_SESSION['secureKey'][md5(uniqid()] = true);
[PHP] pobierz, plaintext 

i sprawdzaj czy w tablicy $_SESSION['secureKey'] jest pole o tym kluczu

[andrzejt17]

Robię to w GET dlatego, że klucz nie będzie mi potrzebny tylko w formularzach. Będą takie miejsca, gdzie nie będzie można odebrać klucza postem więc musi być get. Chyba, że ciastko?

Podaną wadę widzę jako zaletę Bo po kiego grzyba userowi 2 razy otwarty formularz np rejestracji? Żeby mi zrobić nalot z kontami na serwis? O nie

Po co robić tablice asocjacyjne dla klucza? oO

[lobopol]

Po pierwsze ktoś może otworzyć stronę w zakładce i kliknąć przez przypadek 2 razy. Zamknie 2 okno uzupełni cały formularz i nagle się okaże, że formularz gucio zrobił. Większość userów powie p... nie chcę mi się drugi raz i oleje sprawę.

Po drugie o ile w rejestracji spoko (pomijając pierwszy punkt) to np. przy komentarzach na wielu stronach po otworzeniu zakładek będzie to działać dokładnie tak samo jak wyżej.

Po trzecie po to:
-szybsze sprawdzenie czy jest ustawiony token
-bezproblemowe usuwanie tokena
-zamiast true można ustawić np. czas życia tokena i przy sprawdzaniu patrzeć czy nie upłynął

[andrzejt17]

Jak user nie umie klikać to nie mój problem Formularz będzie na tyle prosty, że nie będzie wielkim wyczynem uzupełnić go raz jeszcze (6 pól, 3 comboxy i captcha) ;p Ponadto to w jego interesie będzie się zarejestrować (strona o dość specyficznej tematyce)

W wypadku komentarzy to w ogóle by nie zdało egzaminu. Wyobraź sobie, że nawet jak będzie captcha to mając 2 zakładki to captcha będzie aktualna tylko w tej drugiej (używam własnej captchy i kod jest przechowywany w $_SESSION.

Ten twój magiczny sposobik musiałbym też do tokena wdymać.

Może będę to robił parami? Coś w stylu klucz i podklucz. Według mnie to zda egzamin. Dzięki temu każda otwarta strona będzie mogła mieć własny key. Coś w stylu lol.php?lubie=packi&key=blablabla&subkey=tratata A przy sprawdzaniu trzepanie $_SESSION['secureKey'] w poszukiwaniu klucza tablicy o nazwie 'tratata' i sprawdzi czy wartość zmiennej z takim kluczem tablicy równa się z 'blablabla'. $_SESSION['secureKey']['tratata'] bedzie miał wartość 'blablabla'

Dobrze rozumuję?

== EDIT ==

O jaa.. dopiero po kilku przeczytaniach twojego "$_SESSION['secureKey'][md5(uniqid()] = true);" zrozumiałem idee jegzo działania. Coś w stylu tym co ja napisałem, tylko, że klucz w GET będzie kluczem w tablicy $_SESSION['secureKey'] a potem skrypt sprawdzający obczai, czy jest taki klucz i czy ma wartość true Czyli to co wcześniej wyskrobałem wymięka przy tym ;p Bo tu o jeden GET mniej i w ogóle mniej kodu

Jest tylko jeden mankament. Jeśli jakiś user wejdzie sobie w formularz i przemieli mój skrypt np 20 razy przez F5 to utworzy się 20 tablic w $_SESSION['secureKey'] no i generalnie kaszanka trochę będzie. Może np. zrobić, że może tam być tylko 5 tablic? Jeśli przekroczy 5 tablic to wszystkie zostaną usunięte a najnowszy klucz zostanie wygenerowany i zapisany raz jeszcze ;>

A tak btw. to wkradł Ci się tam błąd:

[PHP] pobierz, plaintext 
$_SESSION['secureKey'][md5(uniqid())] = true;
[PHP] pobierz, plaintext 

== EDIT2 ==

Wykombinowałem sobie takie cacko:

[PHP] pobierz, plaintext 
function generateKey() { 
	$count = count($_SESSION['secureKey']);
	if($count > 5) {
		unset($_SESSION['secureKey']);
	}
	$key = substr(md5(uniqid()), 0, 10)
	$_SESSION['secureKey'][$key] = true;
}
 
function checkSecureKey() { 
	if(isset($_GET['key']) && isset($_SESSION['secureKey']) && $_SESSION['secureKey'][$_GET['key']] == 'true') {			
		unset($_SESSION['secureKey']);
		return true;
	}
	unset($_SESSION['secureKey']);
	return false;
}
[PHP] pobierz, plaintext 

Nie wiem czy będzie śmigało ale na logikę powinno hulać

== EDIT3 ==

Powyższy przykład miałby kłopot z wygenerowaniem klucza i przekazaniem go przez GET bo przecież są tam tablice. Posiedziałem jednak i wymyśliłem sbie coś takiego:

[PHP] pobierz, plaintext 
function generateKey($mode = 0) { 
	$count = count($_SESSION['secureKey']);
	$lastID = $_SESSION['secureKey']['lastKeyID'];
	if($count = 5) {
		$unsetID = $lastID - 4;
		unset($_SESSION['secureKey'][$unsetID]);
	}
	$key = substr(md5(uniqid()), 0, 10);
	$lastID++;
	$keyID = $lastID;
	$_SESSION['secureKey']['lastKeyID'] = $keyID;
	$_SESSION['secureKey'][$keyID] = $key;
	if($mode == 0) {
		return $_SESSION['secureKey'][$keyID];
	} else {
		print $_SESSION['secureKey'][$keyID];
	}
}
 
function checkSecureKey() { 
	if(isset($_GET['key']) && isset($_SESSION['secureKey'])) {
		$keyID = $_SESSION['secureKey']['lastKeyID'];
		$count = count($_SESSION['secureKey']);
		for($i = 0; $i < $count; $i++) {
			if($_SESSION['secureKey'][$keyID] == $_GET['key']) {
				$_SESSION['secureKey'][$keyID] = 'empty';
				return true;
			}
			$keyID--;
		}
		return false;
	}
}
[PHP] pobierz, plaintext 

Śmiga aż miło. Keneruje tokeny to max 5 a potem tworzy następny i usuwa najstarszy. Kod chyba najprościej napisany jak się tylko da Więc albo przyznaje każdemu userowi jeden token i mam małe funkcje, albo przyznaje mu pięć i mam je nieco większe.

Jak by było coś nie zrozumiałe: $_SESSION['secureKey']['lastKeyID'] przechowuje ID najnowszego klucza z tokenem. Pętla przetrzepuje każde 5 tablic z kluczami począwszy od najnowszej. Jeśli się zgadza z podanym przez usera to nadaje mu wartość 'empty' i zwraca true. Nie mogę niestety użyć unset() dla tablicy, która trzymała klucz bo potem skrypcik mógłby działać nieprawidłowo bo mógłby mieć dziury w ID tablic.

Swoją drogą chyba jednak zostanę przy tym, by userowi dać tylko jeden token. Przy metodzie z pięcioma może sobie wygenerować kilka tokenów, wkleić je do swojego formularza na swojej stronie i próbować się wbić do mnie z wcześniej wygenerowanymi tokenami.

Wiem, ze sporo tego ale jak już tu dobrnąłeś to co o tym myślisz? ;>

Ten post edytował andrzejt17 15.05.2011, 00:36:58

[lobopol]

Po to możesz sobie dać żywotność klucza na np. 10-15 minut po których klucz będzie usuwany np.
wersja z kluczami o tym samym czasie wygasania

[PHP] pobierz, plaintext 
$_SESSION['klucz'][md5(time())]=time();
 
przy odbiorze
if(isset($_SESSION['klucz']['token'])){
if((time()-$_SESSION['klucz']['token']<600){
$ok = true;
}
else{
$ok=false;
}
unset($_SESSION['klucz']['token']);
i
}
//kasowanie starych kluczy
foreach($_SESSION['klucz'] as $key=>$value){
if((time()-$value>=600){
unset(unset($_SESSION['klucz'][$key])
}
}
[PHP] pobierz, plaintext 

Wersja z kluczami o różnym czasie życia

[PHP] pobierz, plaintext 
$_SESSION['klucz'][md5(time())]=time()+czas_zycia;
 
przy odbiorze
if(isset($_SESSION['klucz']['token'])){
if((time()>$_SESSION['klucz']['token']){
$ok = true;
}
else{
$ok=false;
}
unset($_SESSION['klucz']['token']);
i
}
//kasowanie starych kluczy
foreach($_SESSION['klucz'] as $key=>$value){
if((time()>=$value){
unset(unset($_SESSION['klucz'][$key])
}
}
[PHP] pobierz, plaintext 

A czy w tablicy będzie 5 czy 1000 tokenów to nie ma jakiejś gigantycznej różnicy.

Jak user nie umie klikać to nie mój problem

Właśnie oto chodzi, że to twój problem . Userzy są przeważnie mało inteligentni i bardzo łątwo rezygnują ze strony jeżeli czegoś nie będą w stanie za pierwszym razem zrobić.

A i nie stworzy się 20 tablic tylko jedna tablica 20 elementowa.

Ps. jeżeli już lecieć twoim sposobem to:

[PHP] pobierz, plaintext 
function generateKey($mode = 0) {
    $lastID=0;
    if(isset ($_SESSION['secureKey'])){
        $count = count($_SESSION['secureKey']);
        $lastID = $_SESSION['secureKey']['lastKeyID'];
        if($count >= 5) {
            $unsetID = $lastID - 4;
            unset($_SESSION['secureKey'][$unsetID]);
        }
    }
        $key = substr(md5(uniqid()), 0, 10);
        $lastID++;
        $keyID = $lastID;
        $_SESSION['secureKey']['lastKeyID'] = $keyID;
        $_SESSION['secureKey'][$keyID] = $key;
        if($mode == 0) {
            return $_SESSION['secureKey'][$keyID];
        } else {
            print $_SESSION['secureKey'][$keyID];
        }	
}
[PHP] pobierz, plaintext 

i
i możesz teraz używać albo przy przesyłąniu $_GET['key'] albo $_POST['key'] przy obu równocześnie post będzie w $_REQUEST['key']

[PHP] pobierz, plaintext 
function checkSecureKey() { 
	if(isset($_REQUEST['key']) && isset($_SESSION['secureKey'])) {
		$keyID = $_SESSION['secureKey']['lastKeyID'];
		$count = count($_SESSION['secureKey']);
		for($i = 0; $i < $count; $i++) {
			if($_SESSION['secureKey'][$keyID] == $_REQUEST['key']) {
				$_SESSION['secureKey'][$keyID] = 'empty';
				return true;
			}
			$keyID--;
		}
		return false;
	}
}
[PHP] pobierz, plaintext 

Choć jak patrzę to sobie po prostu utrudniasz życie ograniczając się do 5 tokenów np. na forum php jest użyta bardzo podobna metoda co podałem razem z postem przesyłany jest auth_key

[andrzejt17]

No dobrze Tylko po co mam dać userowi możliwość wygenerowania no 200 tokenów? Jak bym pisał skrypt forum to ok, bo sam mam manie otwierania 3872648564 tematów i pisania w nich więc wygaśnięty klucz by mnie tylko irytował. Z tym, że ja pisze tylko skrypcik rejestracji no i potem będzie skrypt zmian ustawień w swoim profilu Wiec czy jest sens używania takiego rozwiazania? ;>

[lobopol]

Sam piszesz, że będziesz to wykorzystywał w różnych miejscach, to znacznie lepiej mieć jeden stały kod do tego niż się rozbijać na niewiadomo ile. Myśl przyszłościowo, a nie na dzień dzisiejszy. Z resztą co to szkodzi, aby user miał 200 tokenów? Cały kilobajt czy 2 danych w sesji co to jest? Raz, że niby będziesz wygodniej operował (w drugiej metodzie masz możliwość ustawiania czasu życia dla każdego tokena osobno).
zalety :
-większa kontrola nad tokenami
-szybkość ich sprawdzania
-niemal nieograniczona ilość otwartych formularzy
-znacznie prostszy kod w utrzymaniu
wady:
-po każdym odświeżeniu strony nowy token bez usuwania starego (który zostanie usunięty dopiero po upłynięciu jego czasu życia)

[andrzejt17]

Dobra, dzięki bardzo za rady ;p Tak czy inaczej rozważę to jeszcze. Póki co nie wybiorę jeszcze sposobu, pomyśle nad tym jak przygotuję cały serwis graficznie i przyjdzie mi to wszystko oskryptować

Dziękówka i pozdro