script werkt niet voor bruteforce

Overzicht Reageren

Sponsored by: Vacatures door Monsterboard

Pagina: 1 2 3 volgende »

- Rob -

- Rob -

16/03/2017 19:06:44
Quote Anchor link
Hallo!

Ik wou graag enige bruteforce protection op mijn website maken waarbij als je 3 keer foute login hebt gedaan, je 50 minuten niet kan inloggen etc., en ik had het volgende:

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
<?php
function checkWrongAttempts($ip, $plaats)
{

  $settings = parse_ini_file($_SERVER['DOCUMENT_ROOT'].'/../datafile.ini');
  include($_SERVER['DOCUMENT_ROOT'].$settings['path'].'paneel/includes/init.php');
  $datum = strtotime('UTC');

  $searchWrongAttemptsSql = $link->query("SELECT * FROM `".$settings['path']."foutepogingen` WHERE `ip`='".sha1($ip)."' AND `plaats`='".$link->real_escape_string($plaats)."' ORDER BY `datum` DESC");

  if ($searchWrongAttemptsSql && $searchWrongAttemptsSql->num_rows >= 3)
  {

    $wrongAttempt = $searchWrongAttemptsSql->fetch_assoc();

    if ($datum <= strtotime('+5 minutes', $wrongAttempt['datum']))
    {

      return false;
    }
  }
}

function
insertWrongAttempt($ip, $plaats)
{

  $settings = parse_ini_file($_SERVER['DOCUMENT_ROOT'].'/../datafile.ini');
  include($_SERVER['DOCUMENT_ROOT'].$settings['path'].'paneel/includes/init.php');
  $datum = strtotime('UTC');

  $insertWrongAttempt = $link->query("INSERT INTO `".$settings['prefix']."foutepogingen` (`ip`, `plaats`, `datum`) VALUES ('".sha1($ip)."', '".$link->real_escape_string($plaats)."', '".$link->real_escape_string($datum)."')");

  if ($insertWrongAttempt)
  {

    return true;
  }
}

function
login($gebruikersnaam, $wachtwoord)
{

  $settings = parse_ini_file($_SERVER['DOCUMENT_ROOT'].'/../datafile.ini');
  include($_SERVER['DOCUMENT_ROOT'].$settings['path'].'paneel/includes/init.php');
  $datum = strtotime('UTC');

  if (checkWrongAttempts(getIp(), 'index') != true)
  {

    $searchUserSql = $link->query("SELECT `id`, `wachtwoord` FROM `leden` WHERE `gebruikersnaam`='".$link->real_escape_string($gebruikersnaam)."'");

    if ($searchUserSql && $searchUserSql->num_rows === 1)
    {

      $userDetails = $searchUserSql->fetch_assoc();

      if (!password_verify($wachtwoord, $userDetails['wachtwoord']))
      {

        if (insertWrongAttempt(getIp(), 'index') === true)
        {

          return false;
        }
      }
    }

    else
    {
      if (insertWrongAttempt(getIp(), 'index') === true)
      {

        return false;
      }
    }
  }

  else
  {
    return false;
  }
}

?>

Maar hij blijft hierbij het gehele script uitvoeren, terwijl ik al 25 keer foute login heb gedaan.. Zelf denk ik dat het probleem bij het checken ligt, maar ik dit lukt mij niet om te debuggen Alvast bedankt voor de hulp!
Gewijzigd op 16/03/2017 19:07:22 door - Rob -
 
PHP hulp

PHP hulp

29/03/2024 00:29:28
 
Joakim Broden

Joakim Broden

16/03/2017 19:28:20
Quote Anchor link
Op regel 8 in de functie "checkWrongAttempts" $settings['path'] als tabel prefix.. En op regel 27 in de functie "insertWrongAttempt" gebruik je $settings['prefix'] als tabel prefix.. Zijn dus 2 verschillende tabellen en waarschijnlijk geeft dat ook een SQL error.


PS ik vind je de settings wel een beetje raar gebruikt. Die settings moet je eigenlijk ergens setten waar je er makkelijk bij kunt. Nu moet je ze in elke functie weer apart aanroepen. Wat als het datafile.ini bestand verplaats word, dan moet je heel veel functies aanpassen.
Gewijzigd op 16/03/2017 19:28:59 door Joakim Broden
 
Ward van der Put
Moderator

Ward van der Put

16/03/2017 20:13:36
Quote Anchor link
De logica in deze opzet is kapot. De functie checkWrongAttempts() retourneert altijd null of false, maar je begint de controle van een login met if (checkWrongAttempts(getIp(), 'index') != true): die eerste controle wordt dus altijd genegeerd, want null of false is nooit gelijk aan true.
 
- Rob -

- Rob -

16/03/2017 20:38:49
Quote Anchor link
Hartstikke bedankt allemaal :D, 't blijven toch die kleine foutjes die je over het hoofd ziet :D

Toevoeging op 16/03/2017 21:32:33:

Nu blijf ik toch met het probleem zitten als ik de volgende code heb:

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
function checkWrongAttempts($ip, $plaats)
{
  $settings = parse_ini_file($_SERVER['DOCUMENT_ROOT'].'/../datafile.ini');
  include($_SERVER['DOCUMENT_ROOT'].$settings['path'].'paneel/includes/init.php');
  $datum = strtotime('UTC');

  $searchWrongAttemptsSql = $link->query("SELECT * FROM `".$settings['prefix']."foutepogingen` WHERE `ip`='".sha1($ip)."' AND `plaats`='".$link->real_escape_string($plaats)."' ORDER BY `datum` DESC");

  if ($searchWrongAttemptsSql && $searchWrongAttemptsSql->num_rows >= 3)
  {
    $wrongAttempt = $searchWrongAttemptsSql->fetch_assoc();

    if ($datum <= strtotime('+5 minutes', $wrongAttempt['datum']))
    {
      return true;
    }
    else
    {
      if (createLog('Un geblokkeerd na 5 minuten wachten door teveel foute pogingen', 'index', $ip) && $link->query("DELETE FROM `".$settings['prefix']."foutpogingen` WHERE `ip`='".sha1($ip)."'"))
      {
        return true;
      }
    }
  }
}

function insertWrongAttempt($ip, $plaats)
{
  $settings = parse_ini_file($_SERVER['DOCUMENT_ROOT'].'/../datafile.ini');
  include($_SERVER['DOCUMENT_ROOT'].$settings['path'].'paneel/includes/init.php');
  $datum = strtotime('UTC');

  $insertWrongAttempt = $link->query("INSERT INTO `".$settings['prefix']."foutepogingen` (`ip`, `plaats`, `datum`) VALUES ('".sha1($ip)."', '".$link->real_escape_string($plaats)."', '".$link->real_escape_string($datum)."')");

  if ($insertWrongAttempt)
  {
    return true;
  }
}

function login($gebruikersnaam, $wachtwoord)
{
  $settings = parse_ini_file($_SERVER['DOCUMENT_ROOT'].'/../datafile.ini');
  include($_SERVER['DOCUMENT_ROOT'].$settings['path'].'paneel/includes/init.php');
  $datum = strtotime('UTC');

  if (checkWrongAttempts(getIp(), 'index') != true)
  {
    $searchUserSql = $link->query("SELECT `id`, `wachtwoord` FROM `leden` WHERE `gebruikersnaam`='".$link->real_escape_string($gebruikersnaam)."'");

    if ($searchUserSql && $searchUserSql->num_rows === 1)
    {
      $userDetails = $searchUserSql->fetch_assoc();

      if (!password_verify($wachtwoord, $userDetails['wachtwoord']))
      {
        if (insertWrongAttempt(getIp(), 'index') === true)
        {
          return false;
        }
      }
    }
    else
    {
      if (insertWrongAttempt(getIp(), 'index') === true)
      {
        return false;
      }
    }
  }
  else
  {
    return false;
  }
}


Als het 5 minuten na die tijd is, hoort het script gewoon verder te gaan, maar dat doet het het dus niet. Heb ik wel de laatste row van de table gepakt? Als 't goed is wel door ORDER BY datum DESC toch? En hij zou in preciepe toch gewoon alles moeten verwijderen van foutpogingen waar ip $ip is? Wat doe ik hier fout?
 
Ben van Velzen

Ben van Velzen

16/03/2017 21:59:33
Quote Anchor link
Begin hier eens (alweer):
$datum = strtotime('UTC'); wordt
$datum = time();

Anders heb je een afwijking van minstens een uur.
Daarnaast zou ik gewoon een query draaien om specifiek te zoeken naar een poging in plaats van een resultset van foute pogingen af te lopen. Nog sterker: zo te zien heb je helemaal geen tijdsberekeningen in PHP nodig maar kun je veel eenvoudiger met wat DATETIME velden en de juiste queries krijgen wat je wilt. Voor het geval je nu al DATETIME gebruikt: verrassend dat het zou werken, op zijn minst.
Gewijzigd op 16/03/2017 22:01:14 door Ben van Velzen
 
Thomas van den Heuvel

Thomas van den Heuvel

17/03/2017 01:23:10
Quote Anchor link
SELECT * FROM whatever is tevens mogelijk een goede manier om je database (sneller) op de knieën te krijgen bij een bruteforce? Dit worden dan steeds meer rijen. Indien "ip" en "plaats" niet geïndexeerd zijn is dit ook nog eens een trage query. Waarom geen COUNT? En waarom is het aantal pogingen hardcoded?

En als er echt sprake zou zijn van bruteforce dan halen bovenstaande queries niet zoveel uit omdat ze niet wachten op een antwoord en mogelijk vuurt het antwoord "minder dan 3" voordat de fout gelogd kan worden omdat dit hele zootje niet in een transactie staat. Bruteforce: honderden zoniet duizenden request hameren tegelijkertijd op je database met de vraag "heb ik al ten minste 3 inlogpogingen gedaan". Nog voordat door je loggingfunctionaliteit deze telling waarschijnlijk zijn bijgewerkt zijn vele bruteforce pogingen dit poortje inmiddels gepasseerd...

Daarnaast is er waarschijnlijk maar één plaats waar de functie checkWrongAttempts() wordt gebruikt: een check als onderdeel van inloggen. Heeft een functie die maar op één plaats wordt gebruik echt bestaansrecht?

Ik denk dat het enige wat bovenstaande functies bereiken is dat het moeilijker is om uiteindelijk te begrijpen wat de code nu concreet doet.

Maak geen functies om het functies maken. Overweeg op het moment dat je meerdere keren hetzelfde doet dit in een functie te gieten, maar doe dat niet op voorhand.
 
Ben van Velzen

Ben van Velzen

17/03/2017 01:38:15
Quote Anchor link
+1. Ik zie sowieso een hoop ruimte om de database effectiever te gebruiken, maar dat heb ik al aangegeven. Ook het gebruik van functies kan beter; gebruik een functie wanneer nodig, anders niet. En uiteraard wil je tegen brute forcing een harde blokkade hebben, ook terwijl een poging tot inloggen ondernomen wordt.

Mijn advies (en het is de tweede keer dat ik het zeg) is vanaf nul opnieuw beginnen, stel op wat je precies wilt bereiken, maak desnoods een flowdiagram. Het is voor ons makkelijker om je door het proces heen te loodsen dan deze code te corrigeren, zoals mogelijk al opgevallen is.

Een paar pointers voor de code op voorhand:
1. strtotime() is niet bedoeld om de huidige tijd op te vragen, het is enkel bedoeld om ermee te rekenen. time() is bedoeld om de tijd op te vragen
2. Je database kan veel meer met de data die je hebt als je deze vraagt om er wat mee te doen. Zelf de interne functionaliteit van je database nabouwen klinkt leuk, maar is zinloos. Zie wat je doet met je foute logins als het nabouwen van nested loop functionaliteit die je database al heeft. Schrijf queries die opvragen wat je wil hebben, probeer het verwerken binnen PHP te vermijden.
3. Functies zijn leuk en handig, maar alleen als ze relevant zijn. Binnen procedurele code geldt dat je eenvoudig tot een resultaat wilt komen. Als je bezig gaat met OOP ga je bepaalde zaken wel in methodes gieten, maar dat is een heel ander verhaal
En uiteindelijk 4: begrijpend lezen. Er is meerdere malen op bepaalde zaken gewezen waar je opvolgend niets mee doet, of niet gebruikt zoals letterlijk voorgespiegeld.

Ik begrijp dat je een beginner bent, maar juist dan zou ik verwachten dat je zaken zoals ze zijn uitgelegd ook oppakt en niet vanuit het niets er heel iets anders van maakt. De zaken die ik hier noem noem ik immers niet voor het eerst.
 
Thomas van den Heuvel

Thomas van den Heuvel

17/03/2017 01:52:48
Quote Anchor link
Iets wat ik niet zeker weet maar wellicht kan uitmaken: op het moment dat je werkt met datums en tijden is het wellicht verstandig om één autoriteit te hebben die code / de applicatie oplegt "hoe laat het is". Het is namelijk niet gegarandeerd (klopt dit? zou dit graag weerlegd willen zien indien verkeerd) dat de klokken in PHP en je database helemaal gelijk lopen. Heel kort door de bocht: een huwelijk waarbij $_SERVER['REQUEST_TIME'] (in PHP) in een query gecombineerd wordt met NOW() (in MySQL), ik noem maar wat, lijkt mij niet verstandig - reken met één maat.

En dan hebben we het nog niet eens over tijdszones :P. Het beste lijkt mij om alles in UTC op te slaan en alles in UTC te verrekenen en dan in een hogere laag (bij weergave) hier een tijdszone verrekening overheen te gooien ofzo.

Niet direct hier aan gerelateerd maar ik heb het meegemaakt dat door een soortgelijke verwarring over tijden het niet mogelijk was dat mensen 's-middags in konden loggen in een systeem...
 
Ben van Velzen

Ben van Velzen

17/03/2017 03:09:46
Quote Anchor link
Klopt helemaal. Normaliter wordt dit pas relevant wanneer je je database op een andere server draait dan je webserver, maar better safe than sorry.

Op het punt van meten met 2 maten: ik kan me een zekere marslander herinneren die crashte doordat er door verschillende teams met verschillende maten werd gemeten.
 
Dennis WhoCares

Dennis WhoCares

17/03/2017 08:30:00
Quote Anchor link
Ik was een heel verhaal aan het schrijven, maar ik maak het kort en krachtig.
Bruteforce attacks dat uitgevoerd wordt door een 'hacker' of 'scriptkiddie' zijn krachtig, en veelzijdig en voornamelijk 'snel' (of langzaam, kan allemaal bepaald worden door de gebruiker)
Meerdere ip adressen, verschillende user-agents.

-controlleer user-agent
-controlleer ip (op basis van land, er zijn zat ip adressen die je bij voorbaat al zou willen blokkeren)
-eenmaal een bruteforcer gedetecteerd, laat gewoon een neppe 'ingelogd' pagina zien, kan gewoon een simpele html pagina zijn, zodat je ook de impact op je database verzacht
-de rest van je script
-gebruik een captcha na een mislukte login poging

Wellicht zou je naar andere beveiligingsmaatregelen willen kijken op server niveau.
 
Ivo P

Ivo P

17/03/2017 09:45:41
Quote Anchor link
@Thomas

Dat heb ik inderdaad ooit eens aan de hand gehad: de database bleek uiteindelijk op een andere server te staan dan 'localhost' en had een klok die 2 minuten achter liep.

Om middernacht startte een cronjob op de webserver die iets deed met query's waarin de data van de vorige dag geselecteerd werd.
Maar aangezien het op de klok van de database pas 23:58 was, was "gisteren" nog een dag eerder.
Geeft verrassende resulaten.


--
verder lijkt het me niet handig om in elke functie elke keer opnieuw een settings file te parsen.
Als ik dat af zou moeten zetten tegen het gebruik van "global", dan zou ik dat nog liever doen.

Als je straks ergens een function maakt die 25 keer aangeroepen wordt op een pagina, dan zou je alleen al in die functie 25 keer de ini-file parsen en init.php ook nog een 25 keer includen.


Je zou iets kunnen doen als

$settings = settings::get_settings();

en dan

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
<?php
class settings
{
  protected static $settings = array();

  public static function get_settings()
  {

    if(!self::$settings) {
       self::$settings = parse_ini_file($_SERVER['DOCUMENT_ROOT'].'/../datafile.ini');
    }


    return self::$settings;

  }

}

?>


Dan parse je die file maar 1x

vraag is ook of je init.php niet ook op zo'n soort manier gedaan kan worden
 
Ward van der Put
Moderator

Ward van der Put

17/03/2017 10:25:18
Quote Anchor link
Voor global settings gebruik ik liever constanten omdat die efficiënter met geheugen omgaan en overal beschikbaar zijn. Dan ben je van global af en heb je er verder geen omkijken meer naar: de configuratieconstanten zijn er gewoon overal en altijd. Dat kun je uiteraard ook implementeren met een parse_ini_file().
 
Remco nvt

Remco nvt

17/03/2017 11:24:28
Quote Anchor link
Zou dit vooral verder bouwen omdat je er veel van kan leren!

Maar praktisch is het niet en je zal eerder de normale gebruiker er mee tot last zijn dan dat je 'hackers' er mee buiten sluit.
Paar redenen:
1) De check is op IP. Dus als meerdere mensen in een bedrijf een foute inlogpogingen doen zijn ze samen de sjaak, daarnaast zal een 'hacker' verkeer vanuit verschillende ip's sturen. Denk aan een botnet of iets van Amazon waar je snel veel kleine scripts kan draaien, en als we naar ipv6 gaan kijken dan zijn de mogelijkheden nog groter. Daarbij komt bij ipv6 dat verschillende notaties heeft voor 1 ipadres.

2) Je script is Timing attack gevoelig. Want als een gebruiker niet wordt gevonden dan krijg je meteen een return, terwijl hij anders wel een password_verify doet. In het verschil van de response tijden weet de 'hacker' in elk geval of een gebruikersnaam wel of niet bestaat.
De password_verify functis is wel timing attack proof.

En vraag aan je hoster of ze een NTP sync willen aanzetten op je server ;)
 
- Rob -

- Rob -

17/03/2017 19:32:32
Quote Anchor link
Bedankt voor de reacties! Ik ben nu van plan alle dingen toe te passen, en te beginnen met OOP. Dan gelijk daar een vraag over, ik had met OOP een mysqli connect gemaakt maar nu ik wil ik buiten de Class om, de variable $mysqli aanroepen. Ik heb het opgezocht op google maar ik (wat ik zocht) heb het niet kunnen vinden.

mijn code:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
<?php
class database extends mysqli {
    public $mysqli;

    public function __construct() {
        $this->mysqli = new mysqli('localhost', '***', '***', '***');

        if (!$this->mysqli) {
            die('Fout met verbinding');
        }

        else
        {
          return $this->mysqli;
        }
    }
}


// ANDER BESTAND
include($_SERVER['DOCUMENT_ROOT'].'/paneel/includes/classes/database.class.php');

$mysqli = new database();

echo $mysqli->real_escape_string('`test`');
?>


Ik hoop dat iemand even mij hieruit kan helpen! (real_escape_string() gebruikte ik even om te testen)
 
- Ariën  -
Beheerder

- Ariën -

17/03/2017 19:37:32
Quote Anchor link
Extenden is wel hartstikke leuk, maar je vindt nu wel het wiel opnieuw uit.
Normaal kan je met
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
<?php $mysqli = new mysqli("localhost", "user", "password", "mijnwebsite"); ?>
connecten met je database via MySQLi.
 
- Rob -

- Rob -

17/03/2017 19:43:21
Quote Anchor link
Harstikke bedankt! ik heb nu dit om het te echoen:
echo $database->mysqli->real_escape_string('`d`');
en heb extends mysqli weg gehaald en return etc. ook.
 
- Ariën  -
Beheerder

- Ariën -

17/03/2017 19:46:12
Quote Anchor link
$database bestaat niet eens, tenzij je die variabele waar je de class initieert hebt hernoemd.
 
- Rob -

- Rob -

17/03/2017 19:46:35
Quote Anchor link
Nu stuit ik alleen op het volgende probleem, hij showt nu geen error als de gegevens fout zijn.. Waar zit nu het probleem?

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
<?php
class database
{
    public $mysqli;

    public function __construct()
    {

        $this->mysqli = new mysqli("localhost", "****", "*****", "******");

        if (!$this->mysqli)
        {

             die('Fout met verbinding');
        }
    }
}

?>







include($_SERVER['DOCUMENT_ROOT'].'/paneel/includes/classes/database.class.php');

$database = new database();


Toevoeging op 17/03/2017 19:47:08:

Update^^
Gewijzigd op 17/03/2017 21:25:22 door - Ariën -
 
Bart V B

Bart V B

17/03/2017 21:15:58
Quote Anchor link
Waarom maak je het jezelf zo moeilijk...
Recht uit de manual:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
<?php

class database  extends mysqli {
    public function __construct($host, $user, $pass, $db) {
        parent::__construct($host, $user, $pass, $db);

        if (mysqli_connect_error()) {
            die('Connect Error (' . mysqli_connect_errno() . ') '
                    . mysqli_connect_error());
        }
    }
}


$db = new database('localhost', 'my_user', 'my_password', 'my_db');

echo 'Success... ' . $db->host_info . "\n";

$db->close();
?>
 
Ben van Velzen

Ben van Velzen

17/03/2017 23:31:16
Quote Anchor link
Ik zou hier alleen wel de OO varianten gebruiken en binnen de class ook niet terugvallen op de procedurele varianten. Dit kan tot onvoorspelbaar gedrag en/of incompatibiliteit tussen PHP versies leiden:

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
<?php

class database  extends mysqli {
    public function __construct($host, $user, $pass, $db) {
        parent::__construct($host, $user, $pass, $db);

        if ($this->connect_error()) {
            die('Connect Error (' . $this->connect_errno() . ') '
                    . $this->connect_error());
        }
    }
}


$db = new database('localhost', 'my_user', 'my_password', 'my_db');

echo 'Success... ' . $db->host_info . "\n";

$db->close();
?>
 
- Rob -

- Rob -

18/03/2017 07:15:12
Quote Anchor link
Maar met deze class kan ik toch niet met een andere class $db gebruiken?

Toevoeging op 18/03/2017 07:18:47:

Of moet ik dan $db meegeven met die class?

$class = new class($db);
 

Pagina: 1 2 3 volgende »



Overzicht Reageren

 
 

Om de gebruiksvriendelijkheid van onze website en diensten te optimaliseren maken wij gebruik van cookies. Deze cookies gebruiken wij voor functionaliteiten, analytische gegevens en marketing doeleinden. U vindt meer informatie in onze privacy statement.