2-Stap autorisatie (in PHP)

[jinvanthee]

Hallo,
Ik ben bezig met een 2-Stap autorisatie, waarbij een gebruiker die probeert in te loggen een email ontvangt, met daarin een 6-cijferige code die de gebruiker dan weer moet invoeren om uiteindelijk in te kunnen loggen. En om de gebruiker het wat makkelijker te maken, wil ik ook dat de gebruiker een "onthoud mij" knop heeft, waarmee de gebruiker maar 1x het 2-Stap autorisatie proces hoeft uit te voeren.

Hoe zou ik dit het best kunnen aanpakken?
Ik heb al een MYSQLI database gemaakt met 4 kolommen:

• ID
• CookieID
• Token
• verval-datum

De token wordt gegenereerd door: $token = bin2hex(random_bytes(6)); en de verval-datum wordt gedaan door: $vervaltoken= date("U") + 300;
Ik wil niet dat na 5 minuten, de gebruiker weer wordt "vergeten" en dat de gebruiker weer de 2-Staps autorisatie moet doorlopen. Met de $vervaltoken, wil ik dat de code die de gebruiker moet invoeren, na 5 minuten weer vervalt. De gebruiker moet na 3 maanden, weer opnieuw de 2-staps autorisatie doen.

Zijn er nog lekken in qua beveiliging, of zijn er nog suggesties voor een beter / handiger systeem?
Alvast bedankt,

[418O2]

https://github.com/Dolondro/google-authenticator

Daar zijn toch tig libs voor? Dit zou ik niet zelf gaan uitvogelen

[johnkeates]

Gebruik een JWT voor persistente cryptografische controle van iemands sessie. Enige wat je veilig moet houden is de private key server-side.

[RobIII]

418O2 schreef op vrijdag 8 maart 2019 @ 20:38:
https://github.com/Dolondro/google-authenticator

Die lib doet echter nul van waar TS het over heeft Daarbij: deze heeft, zoals veel andere concurrenten, een mogelijkheid tot een timing sidechannel attack omdat er een gewone string-compare gebruikt wordt i.p.v. een veilige compare. _{(En er zijn betere Disclaimer: ik ben de auteur van die lib).}

Die libs zorgen voor 't hele TOTP afhandelen (genereren en controleren van de TOTP code's, QR codes genereren, dat soort shizzle), niets van de afhandeling van de businesslogica waar TS het over heeft zoals 'aangemeld blijven' en het vervallen van tokens enz.

[ Voor 75% gewijzigd door RobIII op 08-03-2019 21:49 ]

[Jimbolino]

Daarnaast vraagt de TS over authenticatie via email, niet via een google authenticator.

Wat een beetje veilig login/registratiesysteem nodig heeft is:
- Registratiepagina die random token opslaat in DB("temp" user tabel) en mailt naar gebruiker
- Validatiepagina waar gebruiker op terugkomt met token uit email, waarna de "echte" user wordt gemaakt.
- Loginpagina (evt met remember-me checkbox), valideert op de "echte" user tabel, en maakt sessie aan.
- Logoutknop die de sessie correct verwijderd.
- Wachtwoordvergeet pagina voor als de user zijn wachtwoord is vergeten
- Sessies die verlopen na een x aantal uur
- Minimale wachtwoord eisen
- Correct gehashte en gesalte wachtwoord hashes
- Beveiliging tegen robots die wachtwoorden brute forcen (dmv rate limiting of captchas)
- CSRF en XSS beveiliging, zodat login sessies niet makkelijk gekaapt kunnen worden
- etc etc

Als het voor een hobby website is, is het natuurlijk hartstikke leuk en leerzaam om dit allemaal zelf uit te zoeken en zelf te schrijven. Bouw je dit voor een klant of is het een systeem waar je meer mee wilt gaan doen dan alleen voor de fun, dan zou ik kijken naar een framework zoals laravel:
https://laravel.com/docs/...authentication-quickstart

Het voordeel van een framework als laravel is dat:
- Het meest gebruikt, dus ook veel vraag/antwoord op forums
- Goed getest, dus geen veiligheidsproblemen
- Goed gedocumenteerd.

Nadelen:
- Wellicht moeilijk om aan te sluiten op je bestaande codebase / applicatie.

[Anoniem: 80910]

RobIII schreef op vrijdag 8 maart 2019 @ 21:38:
[...]

Die lib doet echter nul van waar TS het over heeft Daarbij: deze heeft, zoals veel andere concurrenten, een mogelijkheid tot een timing sidechannel attack omdat er een gewone string-compare gebruikt wordt i.p.v. een veilige compare. _{(En er zijn betere Disclaimer: ik ben de auteur van die lib).}

Die libs zorgen voor 't hele TOTP afhandelen (genereren en controleren van de TOTP code's, QR codes genereren, dat soort shizzle), niets van de afhandeling van de businesslogica waar TS het over heeft zoals 'aangemeld blijven' en het vervallen van tokens enz.

Oef: je moet de functie goed bekijken om te zien dat de timing attack hier niet geld. Immers de eerste waarde is volgens mij een hexadecimale string die met voorloop 0 wordt aangevult. Hierdoor vergelijk je altijd met een 32 bit hexadecimale waarde. Hierdoor kun je er alleen achterkomen dat het een 32 bit hexadecimale waarde moet zijn middels timing attack.
Ik weet helaas niet hoe php / c, hexadecimale waardes vergelijkt... Indien het een string was zou ik je gelijkgeven...

[RobIII]

Anoniem: 80910 schreef op zondag 10 maart 2019 @ 22:05:
Oef: je moet de functie goed bekijken om te zien dat de timing attack hier niet geld. Immers de eerste waarde is volgens mij een hexadecimale string die met voorloop 0 wordt aangevult.

offtopic:
Aangevuld of niet; het is/blijft een string compare; calculateCode() returned immers (vrij expliciet) een string. De rest van je post begrijp ik dan ook niet helemaal, sorry. Of je begrijpt niet helemaal waar 't issue zit; de timing attack zit 'm namelijk in de string compare zélf. Je kunt meten dat er een verschil in tijd is tussen een "foobar" == "foebar" en een "foobar" == "foober" en dus bepalen hoeveel karakters (if any) je goed hebt.

[ Voor 46% gewijzigd door RobIII op 10-03-2019 23:44 ]

[Anoniem: 80910]

RobIII schreef op zondag 10 maart 2019 @ 23:33:
[...]

offtopic:
Aangevuld of niet; het is/blijft een string compare; calculateCode() returned immers (vrij expliciet) een string. De rest van je post begrijp ik dan ook niet helemaal, sorry. Of je begrijpt niet helemaal waar 't issue zit; de timing attack zit 'm namelijk in de string compare zélf. Je kunt meten dat er een verschil in tijd is tussen een "foobar" == "foebar" en een "foobar" == "foober" en dus bepalen hoeveel karakters (if any) je goed hebt.

Ok ben nog ff verder gaan lezen: wat jij dus zegt is dat bij een string comparison de == en === met een split op 1 karakter, op het moment dat ie de 'foute karakter' tegenkomt een break doet waardoor er verschil in tijd zit? Waarom splitten ze niet op de lengte wat links zit van de vergelijking dan, dan zou je dit probleem niet hebben behalve dat je wellicht de lengte kunt bepalen

[RobIII]

Anoniem: 80910 schreef op maandag 11 maart 2019 @ 00:19:
Ok ben nog ff verder gaan lezen: wat jij dus zegt is dat bij een string comparison de == en === met een split op 1 karakter, op het moment dat ie de 'foute karakter' tegenkomt een break doet waardoor er verschil in tijd zit?

offtopic:
Juist; zie ook de 2 links die ik in die post verstopt heb waarin 't uitgelegd wordt

Anoniem: 80910 schreef op maandag 11 maart 2019 @ 00:19:
Waarom splitten ze niet op de lengte wat links zit van de vergelijking dan, dan zou je dit probleem niet hebben behalve dat je wellicht de lengte kunt bepalen

offtopic:
Hier raak ik je weer kwijt...

Stel dat ik je vraag om "foobar" met "foobar" te vergelijken; jij gaat dat karakter voor karakter doen. Elke keer kijk je of de karakters gelijk zijn todat je een karakter tegenkomt dat niet gelijk is. In dit geval kijk je dus: f == f? Jep. o == o? jep. o == o? jep. b == b? jep. a == a? jep. r == r? jep. Strings gelijk! Maar nu doe je datzelfde voor "foobar" en "foebar". f == f? jep. o == o? jep. o == e? nope. Stop maar; want de rest doet er niet meer toe. Zo werkt een string compare nou eenmaal; waarom CPU cycles verspillen aan de overige karakters terwijl je al wéét dat wat de uitkomst gaat worden (de strings zijn niet gelijk)? In een normale situatie wil je dit dus. Zeker als je twee versies van Hamlet gaat vergelijken en het derde karakter verschilt al; waarom dan de rest van Hamlet nog doorworstelen? In een security context wil je dit juist niet omdat, door het tijdverschil dat er tussen de antwoorden zit kunt afleiden hoeveel karakters je goed hebt. Dus wil je een 'constant time' functie die de string compare doet (dus die gewoon 'dom' de rest van de string afwerkt, ondanks dat al lang bekend is dat de strings niet gelijk zijn). Dat is wat hash_equals doet in PHP maar er zijn zat manieren om dat zelf te doen.

Iets anders wat je zult moeten weten/beseffen is dat de strings die compared worden een fixed length hebben (net zoals, bijvoorbeeld, een sha256 hash altijd een bepaalde vaste lengte heeft). Dus er valt niets te 'verraden' qua string lengte, dat wéét de attacker al in dit geval; de lengte van de TOTP code is publiek bekend (meestal is dat 6 tekens, maar de standaard laat verschillende lengtes toe; maar ook die zal gewoon bekend zijn bij de attacker, die kan immers zelf een account aanmaken en 2FA inschakelen en kijken wat voor lengte code gebruikt wordt; en zelfs al kon dat niet dan is die informatie nog steeds geen 'top secret'). Echter juist omdat de lengte van een TOTP code maar 6 tekens is, en dan ook nog eens beperkt tot alleen cijfers, betekent dat dat er maar maximaal 1.000.000 combinaties mogelijk zijn. Daarbij zijn TOTP codes vaak enkele 'timeslices' lang geldig om verschillen in klokken op te vangen (codes in de apps die je ziet zijn vaak 30 seconden "geldig" maar bij validatie wordt er vaak 1 code terug en 1 vooruit gekeken om die tijdsverschillen op te vangen; soms zelfs meer). Je zult dan, met een beetje mazzel, gemiddeld maar hooguit 500.000 pogingen in anderhalve minuut hoeven doen in een situatie waarbij er nog niet eens sprake is van zo'n timing attack. Dat, an sich, is al vrij haalbare kaart (misschien niet in alle gevallen, zeker niet als er netjes ge-rate-limit wordt) maar gooi er dan nog eens zo'n timing vulnerability overheen en opeens kun je 't brute-forcen in een paar honderd of misschien paar duizend pogingen; je maakt je systeem aanzienlijk zwakker zo.

ps in dit verhaal maakt == vs === niet veel, of waarschijnlijk zelfs helemaal geen, verschil omdat in alle gevallen de de linker en rechter operands strings zijn en dus, ongeacht == of === een constante tijd zullen innemen wanneer een attacker pogingen aan 't doen is

[ Voor 32% gewijzigd door RobIII op 11-03-2019 02:13 ]

[DJMaze]

RobIII schreef op vrijdag 8 maart 2019 @ 21:38:
_{(En er zijn betere Disclaimer: ik ben de auteur van die lib).}

offtopic:
Ook al is hash_equals tegenwoordig ingebakken.
Je kon ook het volgende doen in private function codeEquals.

PHP:

        $i = strlen($user);
        if (strlen($safe) === $i) {
                $user ^= $safe;
                $result = 0;
                while ($i--) {
                    $result |= ord($user[$i]);
                }
                return 0 === $result;
        }

offtopic:
je script is namelijk niet zo snel i.v.m. for loop en je kan bitwise doen op de string.
Al met al is "betere" dus altijd een relatief begrip

[ Voor 7% gewijzigd door DJMaze op 12-03-2019 13:36 ]

[RobIII]

DJMaze schreef op dinsdag 12 maart 2019 @ 13:34:

offtopic:
Al met al is "betere" dus altijd een relatief begrip

offtopic:
Je hebt 't hier over een belachelijke micro-optimalisatie die misschien sneller is (heb je 't gebenchmarked?). Het punt was dat er geen timing attack mogelijk was (want: constant-time); als mijn code een paar nanosecondes langer loopt dan de jouwe neem ik dat graag voor lief (leesbaarheid doet ook wat). Nogmaals: we hoeven hier niet over heel Hamlet te itereren karakter bij karakter maar over een constante(!) 6 karakters en dus iteraties (misschien 8 ... hell, laten we 12 zeggen als je van de standaard af durft te wijken en daarmee compatibiliteit met allerlei TOTP apps te breken).

DJMaze schreef op dinsdag 12 maart 2019 @ 13:34:

offtopic:
je script is namelijk niet zo snel i.v.m. for loop

offtopic:
Dus jij beweert dat een while per definitie sneller is dan een for? Nu verrast me bij PHP nog maar weinig, daar niet van, maar heb je een betrouwbare bron? Sowieso zou ik graag eens je benchmark resultaten zien (en hoe je je benchmark doet) voor je beweringen in je post. Niet dat die paar nanoseconden verschil überhaupt relevant zijn als er al verschil in zit. Daarbij geeft jouw code een undefined result als de lengtes van $user en $safe verschillen

Edit: een héle quick-'n'-dirty benchmark (bij een string lengte van, de gangbare, 6) geeft dat 't verschil 0,0000007196093 seconde is, ofwel 0.7 µs op een server van 5+ jaar oud En daarbij; de 'default' (waar mogelijk) waar hash_equals gebruikt wordt out-performed jouw code nog steeds met een factor 6,5 dus waar hebben we 't nou helemaal over? Die hash_equals zal in veruit de meeste gevallen gebruikt worden; als die functie niet bestaat heb je wel grotere problemen (lees: straatoude versie van PHP < 5.6)

[ Voor 70% gewijzigd door RobIII op 12-03-2019 14:58 ]

[jinvanthee]

Dit is mijn code:

PHP: login.inc.php

<?php
session_start();
require 'connection.inc.php';
// controleren of ze de cookie hebben geaccepteerd
  if(!isset($_COOKIE['acceptcookie'])) {
    header('Location: ../login.php?error=nocookie');
    exit();
  }
  else {
      if(isset($_POST['Login'])) {
     // variabelen ophalen
          htmlspecialchars($userName = $_POST['UName']);
          htmlspecialchars($password = $_POST['pwd']);
         if(empty($_POST['UName']) || empty($_POST['pwd'])) {
              header("location:../login.php?Empty= Vul alle velden in");
         }
         else {
          // myslqi statement voor het selecteren van gebruikersnaam
             $sql= "SELECT * FROM gebruikers WHERE UName=?";
             $stmt = mysqli_stmt_init($conn);
             if(!mysqli_stmt_prepare($stmt, $sql)) {
                 header('Location: ../login.php?MYSQLI_ERROR');
                 exit();
             }
             else {
                 mysqli_stmt_bind_param($stmt, "s", $userName);
                 mysqli_stmt_execute($stmt);
                 $result = mysqli_stmt_get_result($stmt);
                 if($row = mysqli_fetch_assoc($result)) {
               // wachtwoord decoderen
                     $pwdCheck = password_verify($password, $row['Pass']);
          // als het wachtwoord goed is, moet je de gebruiker inloggen
                     if($pwdCheck == true) {
               // kijken of de gebruiker 2-staps verificatie aan / uit heeft staan
                         if($row['2FAStatus'] == 0) {
                              $_SESSION['User'] =$row['UName'];
                              $_SESSION['ID'] =$row['ID'];
                              header("Location:../wellcome.php?uid=".$row['UName']);
                              exit();
                         }
                         else{
                    // kijken of de gebruiker op "onthoud mij' heeft geklikt
                             if(isset($_COOKIE['expire'])) {
                                  $_SESSION['User'] =$row['UName'];
                                  $_SESSION['ID'] =$row['ID'];
                                  header("Location:../wellcome.php?uid=".$row['UName']);
                                  exit();
                             }
                             else {
                    // als dat niet zo is, dan moet je de gebruiker naar het 2-staps autorisatie pagina leiden.
                                  header('Location: ../2FALogin.php?request=valid');
                                  exit();
                             }
                         }
                     }
                     else if($pwdCheck == false){
                         header("location:../login.php?Invalid= Verkeerd wachtwoord/gebruikersnaam combinatie");
                         exit();
                     }
                     else {
                         header("location:../login.php?Invalid= Verkeerd wachtwoord/gebruikersnaam combinatie");
                         exit();
                     }
                 }
                  header("location:../login.php?Invalid= Verkeerd wachtwoord/gebruikersnaam combinatie");
                  exit();
             }
         }
      }
      else {
          header('Location: ../login.php?error=invalidrequest');exit();
  } 
}
?>

Mijn code voor het genereren van de 2-Factor Autorisatie code:

PHP: Login2FA.inc.php

<?php
use PHPMailer\PHPMailer\PHPMailer;
use PHPMailer\PHPMailer\Exception;
require 'PHPMailer-master/src/Exception.php';
require 'PHPMailer-master/src/PHPMailer.php';
require 'PHPMailer-master/src/SMTP.php';
require 'connection.inc.php';
// variableen ophalen       
htmlspecialchars($email2FA = $_POST['email2FA']);     
// klikvalidatie
if(!isset($_POST['2FAEnable'])) {
    header('Location: ../login.php?error=invalidrequest');
    exit();
}
else if (isset($_SESSION['User'])) {
    header('Location: ../wellcome.php?error=alreadyloggedin');
    exit();
}
else {
    // code genereren
    function randomtoken($size) {
        $size = intval($size);
        if($size == 0) {
            return NULL;
        }
        $charSet = '0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefhijklmnopqrstuvwxyz!';
        $len = strlen($charSet);
        $str = '';
        $i = 0;
        while(strlen($str) < $size) {
            $num = rand(0, ($len-1));
            $tmp = substr($charSet, $num, 1);
            $str = $str . $tmp;
            $i++;
        }
        return $str;
    }
    $token = randomtoken(6);
    date_default_timezone_set('Europe/Amsterdam');
    // controleren of email leeg is gelaten
    if(empty($_POST['email2FA'])) { 
        header('Location: ../2FALogin.php?error=emptyfields&request=valid');
        exit(); 
    }
    else {
        // controleren of de email geldig is
        if(!filter_var($_POST['email2FA'], FILTER_VALIDATE_EMAIL)) {
            header('Location: ../2FALogin.php?request=valid&error=invalidemail');
            exit();
        }   // controleren of de email geregistreerd is
        else {
            $sqlMail = 'SELECT * FROM gebruikers WHERE Mail=?;';
            $stmtMail = mysqli_stmt_init($conn);
            if(!mysqli_stmt_prepare($stmtMail, $sqlMail)) {
                echo 'MYSQLI Mail statement is mislukt';
                exit();
            }
            else {
                mysqli_stmt_bind_param($stmtMail, "s", $email2FA);
                mysqli_stmt_execute($stmtMail);
                mysqli_stmt_store_result($stmtMail);
                $resultMail = mysqli_stmt_num_rows($stmtMail);
                if($resultMail > 0) {
                    $sqlCheck = 'SELECT * FROM autorisatie WHERE uid=?';
                    $stmtCheck = mysqli_stmt_init($conn);
                    if(!mysqli_stmt_prepare($stmtCheck, $sqlCheck)) {
                        echo error_get_last();
                        exit();
                    }
                    else {
                        mysqli_stmt_bind_param($stmtCheck, "s", $email2FA);
                        mysqli_stmt_execute($stmtCheck);
                        mysqli_stmt_store_result($stmtCheck);
                        $resultCheck = mysqli_stmt_num_rows($stmtCheck);
                        if($resultCheck > 0) {
                            header('Location: ../2FALogin.php?error=tokenregistered&email='.$email2FA.'&request=valid');
                            exit();
                        }
                        else {
                             // prepared statement voor het invoeren van token gegevens
                            $sql = "INSERT INTO autorisatie (cookieID, token, vervaldatum, uid) VALUES (?, ?, NOW(), ?)";
                            $stmt = mysqli_stmt_init($conn);
                            if(!mysqli_stmt_prepare($stmt, $sql)) {
                                echo mysqli_stmt_error($stmt);
                                exit();
                            }            
                            else {
                                $cookieID = setcookie('expire', $email2FA, time() + (86400 * 90), "/");
                                $hashedToken = password_hash($token, PASSWORD_DEFAULT);                
                                mysqli_stmt_bind_param($stmt, "sss", $cookieID, $hashedToken, $email2FA);
                                mysqli_execute($stmt);
                                $result = mysqli_stmt_get_result($stmt);
                                mysqli_stmt_close($stmt);
                                mysqli_close($conn);                 
                                // mail verzenden met de code
                                $mail = new PHPMailer(true);
                                try {
                                    $mail->SMTPDebug = 3;
                                    $mail->isSMTP();                                      // Set mailer to use SMTP
                                    $mail->SMTPAuth = true;                               // Enable SMTP authentication
                                    $mail->SMTPSecure = 'ssl';                            // Enable TLS encryption, `ssl` also accepted            
                                    $mail->Host = 'mail.axc.nl';  // Specify main and backup SMTP servers
                                    $mail->Port = 465;                                    // TCP port to connect to
                                    $mail->SMTPOptions = array(
                                        'ssl' => array(
                                            'verify_peer' => false,
                                            'verify_peer_name' => false,
                                            'allow_self_signed' => true
                                        )
                                    );                
                                    $mail->isHTML(true);                                   // Set email format to HTML            
                                    $mail->Username = '[e-mail]';                 // SMTP username
                                    $mail->Password = 'mijnwachtwoord';                           // SMTP password
                                    //Recipients
                                    $mail->setFrom('[e-mail]');
                                    $mail->addAddress($email2FA);     // Add a recipient
                                    $mail->addReplyTo('[e-mail]', 'Ondersteuning');
                                    //Content
                                    $mail->Subject = '2-Staps Autorisatie *****.nl';
                                    $mail->Body    = "<p>Er is een verzoek binnengekomen om 2-Staps Autorisatie code te verzenden voor uw account. \n\r Als u dit niet was, wordt u aangeraden om uw account te <a href='jinvantongeren.nl/adjust.php'>Beveiligen</a>\n\r Uw code is: ".$token."</p><br>Met vriendelijke groeten, ";
                                    $mail->AltBody = 'Er is een verzoek binnengekomen om 2-Staps Autorisatie code te verzenden voor uw account. \r\n. De code is: '.$token.' \r\n Als u dit niet was, wordt u aangeraden om uw account te <a href="jinvantongeren.nl/adjust.php">Beveiligen</a> \r\n Met vriendelijke groeten,';    
                                    $mail->send(); 
                                    if(isset($_POST['remember'])) {
                                        setcookie('remember', $email2FA, time() + (86400 * 90), "/");
                                        echo "<script>window.location.assign('../2FAVerify.php?sent=success&request=valid&email=".$email2FA."')</script>";
                                        exit();                
                                    }
                                    else {
                                        echo "<script>window.location.assign('../2FAVerify.php?sent=success&request=valid&email=".$email2FA."')</script>";
                                        exit();  
                                    }
                                }                  
                                catch (Exception $e)  {
                                    echo error_get_last();
                                    exit();            
                                }                               
                            }                             
                        }
                    }
                }
                else {
                    header('Location: ../2FAlogin.php?error=mailnotregistered&request=valid');
                    exit();
                }
            }
        }
    }
}

En dit is de code voor het inloggen via 2-Staps Autorisatie

PHP: 2FALoginVerify.inc.php

<?php
session_start();
require 'connection.inc.php';
if(!isset($_GET['request'])) {
    if($_GET['request'] !== 'valid') {
        header('../2FAVerify.php?error=invalidrequest');
        exit();
    }
}
else if(!isset($_POST['2FAVerifyCode'])) {
    header('Location: ../2FAVerify.php?error=invalidrequest');
    exit();
}
else if(empty($_POST['2FACode'])) {
    header('Location: ../2FAVerify.php?request=valid&error=emptyfields&email='.$email);
    exit();
}
else {
    htmlspecialchars($code = $_POST['2FACode']);
    htmlspecialchars($email = $_GET['email']);
    $sql = 'SELECT * FROM autorisatie WHERE uid=?';
    $stmt = mysqli_stmt_init($conn);
    if(!mysqli_stmt_prepare($stmt, $sql)) {
        echo 'mysqli check statement voor uid is mislukt';
        exit();
    }
    else {
        mysqli_stmt_bind_param($stmt, "s", $email);
        mysqli_stmt_execute($stmt);
        $result = mysqli_stmt_get_result($stmt);
        if($row = mysqli_fetch_assoc($result)) {
            // code dehashen
            $codeCheck = password_verify($code, $row['token']);
            // controleren wel / niet goed
            if($codeCheck == false) {
                header('Location: ../2FAVerify.php?error=wrongcode&request=valid&email='.$email);
                exit();
            }
            // wel goed
            else if ($codeCheck == true) {
                $sqlDelete = 'DELETE FROM autorisatie WHERE uid=?';
                $stmtDelete = mysqli_stmt_init($conn);
                if(!mysqli_stmt_prepare($stmtDelete, $sqlDelete)) {
                    echo 'MYSQLI delete statement is mislukt';
                    exit();
                }
                else {
                    mysqli_stmt_bind_param($stmtDelete, "s", $email);
                    mysqli_stmt_execute($stmtDelete);
                    $result = mysqli_stmt_get_result($stmtDelete);
                    if(!$result) {
                        $sqlLogin = 'SELECT * FROM gebruikers WHERE Mail=?';
                        $stmtLogin = mysqli_stmt_init($conn);
                        if(!mysqli_stmt_prepare($stmtLogin, $sqlLogin)) {
                            echo 'MYSQLI login statement is mislukt';
                            exit();
                        }
                        else {
                            mysqli_stmt_bind_param($stmtLogin, "s", $email);
                            mysqli_stmt_execute($stmtLogin);
                            $resultLogin = mysqli_stmt_get_result($stmtLogin);
                            if($rowLogin = mysqli_fetch_assoc($resultLogin)) {
                                $_SESSION['User'] =$rowLogin['UName'];
                                $_SESSION['ID'] = $rowLogin['ID'];
                                header("location:../wellcome.php?uid=".$row['UName']);
                                exit();
                            }
                            else {
                                echo error_get_last();
                                exit();
                            }
                        }
                    }
                    else {
                        header('Location: ../login.php?error=unknown');
                        exit();
                    }
                }
            }
            else if($codeCheck == false) {
                header('Location: ../2FAVerify.php?error=wrongcode&request=valid&email='.$email);
                exit();
            }            
        }   
        else {
            header('Location: ../2FAVerify.php?error=nouser&request=valid&email='.$email);
            exit();
        }
    }
}

Ik wilde geen API zoals Google Authenticator gebruiken, want ik heb dit gemaakt, omdat ik momenteel PHP zit te leren.
Ik vraag u om nu kritisch te kijken naar mijn code en mij feedback te geven.
(Ik gebruik procedural style en geen OOP, maar dat ga ik nog zeker doen en PDO vind ik voor nu nog iets te moeilijk)

Met vriendelijke groeten,
Jin van Tongeren

[incaz]

@jinvanthee, ik ga die code niet allemaal lezen, maar als het puur om te leren is, zou ik zeker aanraden om er tests bij te schrijven. Niets helpt je zo goed te begrijpen wat je wilt doen en welke edge cases je af wilt vangen dan door tests te schrijven. (En je gaat dan ook automatisch je code opdelen in testbare functies, wat je begrip en de opbouw ten goede komt.)

[Barryvdh]

http://php.net/manual/en/function.rand.php

This function does not generate cryptographically secure values, and should not be used for cryptographic purposes. If you need a cryptographically secure value, consider using random_int(), random_bytes(), or openssl_random_pseudo_bytes() instead.

Gebruik random_bytes of random_int als je iets veiliger wil doen.

Afgezien van de rest van de code, is e-mail GEEN 2 factor als je ook de optie hebt tot wachtwoord reset. Want dan is je wachtwoord namelijk via hetzelfde medium te verkrijgen als de 2FA code. Daarom wordt de methode die je nu gebruikt (token generen/controleren en mailen) voornamelijk gebruikt als wachtwoord reset methode. Dus wachtwoord vergeten? -> Link met code -> Als code klopt kan je wachtwoord instellen.

2FA is een ander device, vandaar dat er verwezen wordt naar de 'Google Authenticator', wat eigenlijk niks van Google is, maar toevallig een populaire app die dat principe ondersteund. Het idee is dat je een 'secret' genereert voor de gebruiker, waarmee zowel jij (de website) als de gebruiker (met Goole Authenticator/Authy etc) een code kan genereren/verifieren, op basis van de huidige tijd. In principe kan je zo'n token dus ook zelf genereren en e-mail (zonder op te slaan, want dat is op basis van je secret dat niet veranderd). Maar zoals gezegd, als je wachtwoord reset hebt dan is dit geen goede 2de factor.

[RobIII]

jinvanthee schreef op zaterdag 23 maart 2019 @ 21:23:
Ik vraag u om nu kritisch te kijken naar mijn code en mij feedback te geven.

Nee, dat is niet de bedoeling van dit forum. Zie o.a. Kan iemand even...? en wat vinden jullie van... (iets ander onderwerp, zelfde idee). Sorry Zit je er echt om verlegen, lees dan Devschuurder werven? Gebruik Vraag & Aanbod! Of bekijk deze site van onze conculega's eens: https://codereview.stackexchange.com/

Overigens; Wij tweakers doen elkaar permanent de groeten dus dat mag je ook weg laten

En ondanks dat ik 't niet zou moeten doen: leer functies gebruiken (houdt) je code behapbaar, nest je if-statements niet tig levels diep, gebruik engelse woorden/termen waar mogelijk (je comments zou IMHO beter zijn, maar dat is nog tot daar aan toe, maar "SELECT * FROM gebruikers" is zo... ), zoek op wat DRY betekent en, last but not least; helemaal super dat je nog aan 't leren bent enzo en tof dat je 't allemaal zelf wil doen Maar besef, zéker als beginner, dat alles wat met beveiliging te maken heeft niet iets is om "zomaar even opnieuw het wiel uit te gaan vinden". Leuk voor een testprojectje voor jezelf maar slinger het, alsjeblieft, niet het web op. Niet "in productie", ook niet "voor even" of "intern gebruik". Just. Don't. Je ziet in de offtopic discussie hierboven al hoe subtiel een beveiligingsprobleem kán zijn. Gebruik waar mogelijk tried & tested methodes/libraries.

[ Voor 51% gewijzigd door RobIII op 24-03-2019 00:02 ]