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:


<?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!
Ik had het verkeerd gelezen, ik dacht dat er stond juist een )) :S
Maar in mijn vorige post (Niet de ene met procedurele code) maar object-georiënteerd, daar had ik dat toch al?
@arien, Inderdaad geeft het geen 100% garantie op exacte plaatsen,coordinaten of postcodes, maar wua landen kun je er vrijwel zeker van zijn.
Uiteraard met proxies enz kun je dit weer onzeilen, iemand in israel kan een nederlandse proxy gebruiken
dab kun je wel HTTP_X_FORWARDED_FOR gebruiken welke ook nep kan zijn, dus kun je ze geheel blokkeren indien gewenst. dan heb je ook nog vpn connecties waar je weinig tegen kan doen.

@TS, ik zou niet vermelden welke landen toegestaan zijn ;)
Maar ik sta ook alleen bepaalde hosts toe, vgm zijn zijn daarmee ook meeste VPN's weg (tenzij je een VPN hebt die van één van die hostnamen is)

[size=xsmall]Toevoeging op 18/03/2017 16:35:25:[/size]

"@TS, ik zou niet vermelden welke landen toegestaan zijn ;)"
dan doe ik deze simpele fout: Er is een onbekende fout opgetreden.
als je clienten vaste ip addressen hebt, zou ik idd hun ip's vastleggen en toelaten, maar dit kan snel veranderen, ook qua isp
hostnames en ipranges die je vaak zal moeten bijwerken enz, let er wel op :)
ipinfo.io werkt hun database heel vaak bij.
mysqli_query() is niet echt object-georiënteerd ;-)

Ik laat eens wat van mijn code zien. Het is niet alles, en de class is ook even gestript van het onnodige, en het is ook niet super OO-gebouwd (ik ben er ook geen extreme kei in), maar voor mij doet het wat het moet doen, en het is naar mijn idee ook logisch opgebouwd.


<?php
require("/classes/news.class.php");

$slug = new Slug();
$news = new News($db,$settings); // deze worden al eerder opgehaald via een include buiten dit stukje script. $db is een extend op de interne MySQLi class. Deze kent nog wat extra functies, zoals queries tellen en foutafhandeling.
	if (empty($param[0])) { // kijk in de URL of er een ID-nummer is.
	    throw new Exception('news_no_value');
	} else {
	    $data = $news->getNewsByID($param[1]);
	    if(!isset($param[2])) {
		header("HTTP/1.1 301 Moved Permanently"); 
		header('location:http://'.$_SERVER['HTTP_HOST']."/news/".$param[1]."/".$slug->create($data['title']));
		exit();
	    }
	    $tpl->assign("title",htmlentities($data['title'], ENT_COMPAT,'ISO-8859-1', true));
	    $tpl->assign("data",$data);
	    $tpl->assign("tags",$data['tags']);
	    $tpl->display("news.tpl");
		//$tpl is de instance van Smarty, de template-class waarmee ik PHP en HTML strict gescheiden houd. Als ik het opnieuw zou doen zou ik Twig gebruiken of de plain functies van PHP zelf.
	}
?>


En de class:

<?php
class News {
    
    public $db;
    private $settings;

    public function __construct($db,$settings) {
	$this->db = $db;
	$this->settings = $settings;
    } 
    
    public function getNewsByID($id) {
	$sql = "SELECT een,hoop,velden FROM nieuwstabel WHERE '".$this->db->escape_string($id)."'"; // fictieve querie, niet relevant voor voorbeeld.
	$result = $this->db->query($sql);
	if($result->num_rows==0) {
	    header("HTTP/1.0 404 Not Found");
	    throw new Exception('news_not_found'); // in de exceptionhandler vindt een vertaalslag plaats naar een goed leesbare foutmelding die iedereen snapt.
	}
	return $result->fetch_assoc();
    }
?>


Nogmaals, het is niet 100% optimaal, en er zullen vast mensen zijn die kritische puntjes hebben.
Uiteraard sta ik daar voor open (houd wel ruimte voor de vraag van de TS), hoewel de beslissing om een framework te gebruiken er nog steeds ligt voor ooit! Niet nu, tenzij ik weer met deadlines moet gaan schuiven en weer ene hoop ervaring met frameworks op moet doen. ;-)

Niets is 100% perfect, hoewel dit al wel veel beter is dan de procedurele rotzooi is die ik voorheen had.
Maar waar had ik dan mysqli_query() in mijn eerste code?

<?php
searchUserSql = mysqli_query($mysqli, "SELECT `id`, `wachtwoord` FROM `leden` WHERE `gebruikersnaam`='".mysqli_real_escape_string($username)."'");
?>


Tenzij je het al aangepast hebt.
Dat was volgensmij de verkeerde code ;D Heb het indd. al aangepast

[size=xsmall]Toevoeging op 18/03/2017 17:25:00:[/size]

Ik heb een nieuwe public functie aangemaakt, om te registreren en ik plaats hem even om te kijken of ik het allemaal goed doe ;D


<?php
public function register($username, $password, $password_repeat, $email)
    {
        $mysqli = $this->_mysqli;
        $ip = $_SERVER['REMOTE_ADDR'];
        $ipInfo = json_decode(file_get_contents("http://ipinfo.io/$ip/json"));
        $country = $ipInfo->country;

        if ($country === "NL" || $country === "BE")
        {
            $searchUserSql = $mysqli->query("SELECT * FROM `leden` WHERE `gebruikersnaam`='".$mysqli->real_escape_string($username)."'");
            $searchEmailSql = $mysqli->query("SELECT * FROM `leden` WHERE `emailadres`='".$mysqli->real_escape_string($email)."'");

            if (
                $searchUserSql
                && $searchEmailSql
                && isset($searchUserSql->num_rows)
                && isset($searchEmailSql->num_rows)
              )
            {
                if (
                    strlen($username) >= 2
                    && strlen($username) <= 15
                    && $password === $password_repeat
                    && preg_match("/^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*[$%^&!@#$*()]).*$/", $password)
                    && strlen($password) >= 8
                    && !filter_var($emailadres, FILTER_VALIDATE_EMAIL)
                  )
                  {
                      $insertUserSql = $mysqli->query("INSERT INTO `leden` (`gebruikersnaam`, `wachtwoord`, `emailadres`, `ip`, `registratie_datum`) VALUES ('".$mysqli->real_escape_string($username)."', '".$mysqli->real_escape_string(password_hash($password, PASSWORD_DEFAULT))."', '".$mysqli->real_escape_string($email)."', '".sha1($ip)."', '".time()."')");

                      if (!$insertUserSql)
                      {
                        echo '0+Er is een onbekende fout opgetreden.';
                      }
                      else
                      {
                        echo '1+Je account is aangemaakt, je kan nu inloggen.';
                      }
                  }
                  else
                  {
                    echo '0+De ingevulde gegevens zijn niet goed2.';
                  }
            }
            else
            {
              echo '0+De ingevulde gegevens zijn niet goed1.';
            }
        }
        else
        {
            echo '0+Er is een onbekende fout opgetreden.';
        }
    ?>
Om de berg aan inspringende code te vermijden kan je een Validatie-class bouwen, die ervoor zorgt dat je alle makkelijk kan valideren. Den aan herhalende wachtwoorden, invoer.

Ook de foutafhandeling kan je extraheren naar de extended MySQLi-class, en daar de query() method klonen, en foutafhandeling erin bouwen. Mogelijk dat dit beter kan, maar zo doe ik het. Het scheelt in elke method weer een extra if-else controle bij de query.

Verder horen er geen echo's in een class.... Dus laat een functie-aanroep uitsluitend iets van true of false of een integer afgeven via return.
Dan zal ik die echo wel even aanpassen ;-)

Reageren