[PHP] session gebruiker en session admin - Softwareontwikkeling

vrijdag 18 september 2015 13:30

Acties:

0 Henk 'm!

Verwijderd

Topicstarter

In een CMS'je geschreven in PHP doe ik om te kijken of iemand ingelogd is het volgende:

code:

$result = mysqli_query($db, " SELECT * FROM tbluser WHERE naam = '$user' AND wachtwoord = '$pass' ");
$rowCheck = mysqli_num_rows($result);
if ( $rowCheck > 0 ) {
    while ( $row = mysqli_fetch_array($result) ) { // dit zit in een loop nu, maar er is sowieso altijd maar 1 resultaat, want unieke naam user
        //start the session and register a variable
        session_start();
        $_SESSION['user_id'] = $row['user_id'];
        $_SESSION['user_beheerder'] = $row['beheerder'];
        header("Location: welkom.php");
    }
}

Ik maak dus altijd 2 sessies aan als er een inlogcombinatie gevonden wordt:
1. met het user_id (en dat is een auto-nummering id)
2. met de uitkomst van user_beheerder (en dat is een ja/nee veld)

Op een andere pagina wil ik daarna een menu tonen, afhankelijk van welke user is ingelod. Dit doe ik als volgt:

code:

if (!isset($_SESSION['user_id'])) header( "Location: index.php" ); // er is niemand ingelogd, dus stuur weg

if ( $_SESSION['user_beheerder'] == "ja" ) {
    echo "Er is een beheerder ingelogd, laat het admin-menu zien.";
    // verdere code hier
} else {
    echo "Er is een normale user ingelogd, laat een normaal menu zien.";
    // verdere code hier
}

Is dit een goede manier, of zijn er nog verbeterpunten?
Ik vind het wel handig om 2 sessies aan te maken. Dus meteen eentje voor de beheerder erbij.
(Dan hoef ik niet op elke pagina een query te doen of de user uit de sessie wel/niet een beheerder is).

vrijdag 18 september 2015 13:35

Acties:

0 Henk 'm!

n2theb

Tweakers Abonnee

Persoonlijk zou ik wel gewoon een query doen om te checken wie er ingelogd is, stel jij verandert iets aan een user, zoals hij is beheerder en dan niet meer dan check je via een query of hij dat is. Als je op basis van sessie doe kan die gemanipuleerd worden en/of als hij inlogt als beheerder maar jij verandert hem terug naar gewone user, dan blijft die op dat moment beheerder.

Daarbij het volgende stukje:

PHP:

1	if (!isset($_SESSION['user_id'])) header( "Location: index.php" );

Zou ik doen als:

PHP:

if (!isset($_SESSION['user_id'])) {
header( "Location: index.php" );
exit; //zodat de pagina hier ook bij stopt en niet door blijft lopen!
}

In je query om te checken welke user en welk wachtwoord moet je wel gebruik maken van escapen ( mysqli_real_escape_string ) of gebruik van prepare statements met mysqli. Misschien doe je dat al er boven, maar dat zie ik hier niet in terug.

vrijdag 18 september 2015 13:36

Acties:

0 Henk 'm!

jbdeiman

@Slingeraap2
Nee is geen ECHT goede manier, er zijn methoden om je sessie te "manipuleren" mocht je dat willen. Beter is een (hash) op te slaan van een gebruiker in je sessie en in je DB. (Bijvoorbeeld een unieke hash op basis van tijdstip en gebruikersnaam oid...).
Deze gebruik je om bij verversen informatie van die gebruiker op te halen. Dit gaat (als je het goed opzet) zo snel, dat je het niet merkt verder, maar is qua veiligheid wel een stuk bruikbaarder.

ntheweb heeft geljik waar hij zegt dat je beter met een query kan controleren wie is ingelogd, dat zorgt ervoor dat je naast het kunnen manipuleren van de sessie ook nog eens "live" je gebruikersrechten kan aanpassen.

Je moet daarnaast nooit zomaar variabelen in een query plaatsen, omdat men daarmee zelf ook query's kan manipuleren/ aanpassen. (zelfs volledig nieuwe query's draaien, als DROP DATABASE, waarna je database weg is)

@ntheweb
Op zich is die exit wel netjes, maar (normaliter) niet nodig. Waar je met een redirect wel op moet letten is dat je pas na de redirect mag beginnen met het sturen van output. (dus niet eerst "<html><header>...</header>" waarna je PHP opent en daar deze code neerzet)

[ Voor 43% gewijzigd door jbdeiman op 18-09-2015 13:40 ]

vrijdag 18 september 2015 13:40

Acties:

0 Henk 'm!

Verwijderd

Topicstarter

n2theb schreef op vrijdag 18 september 2015 @ 13:35:
Persoonlijk zou ik wel gewoon een query doen om te checken wie er ingelogd is, stel jij verandert iets aan een user, zoals hij is beheerder en dan niet meer dan check je via een query of hij dat is. Als je op basis van sessie doe kan die gemanipuleerd worden en/of als hij inlogt als beheerder maar jij verandert hem terug naar gewone user, dan blijft die op dat moment beheerder.

Daarbij het volgende stukje:
PHP:
1
if (!isset($_SESSION['user_id'])) header( "Location: index.php" );
Zou ik doen als:
PHP:
1
2
3
4
if (!isset($_SESSION['user_id'])) {
header( "Location: index.php" );
exit; //zodat de pagina hier ook bij stopt en niet door blijft lopen!
}
In je query om te checken welke user en welk wachtwoord moet je wel gebruik maken van escapen ( mysqli_real_escape_string ). Misschien doe je dat al er boven, maar dat zie ik hier niet in terug.

Aha okee, dat is zo ja.

Ja, die exit staat er normaal wel, ik had het scriptje iets vereenvoudigd.

Escapen wordt ook gedaan. Via mysqli_real_escape_string dat klopt, maar ik zit eraan te denken meteen over te stappen naar PDO. Dan gelijk een extra vraag daarover: Stel dat ik in mijn MySQL database geen data wil met \ escapetekens toegevoegd voor mijn quotes etc., dan zit ik eigenlijk wel aan PDO vast, toch?

vrijdag 18 september 2015 13:43

Acties:

0 Henk 'm!

jbdeiman

Verwijderd schreef op vrijdag 18 september 2015 @ 13:40:
[...]

Aha okee, dat is zo ja.

Ja, die exit staat er normaal wel, ik had het scriptje iets vereenvoudigd.

Escapen wordt ook gedaan. Via mysqli_real_escape_string dat klopt, maar ik zit eraan te denken meteen over te stappen naar PDO. Dan gelijk een extra vraag daarover: Stel dat ik in mijn MySQL database geen data wil met \ escapetekens toegevoegd voor mijn quotes etc., dan zit ik eigenlijk wel aan PDO vast, toch?

Als het goed is krijg je niet daadwerkelijk de ge-escapete quotes via mysqli_real_escape_string, maar zorgt dat ervoor dat je tekst op de juiste manier in de database komt.
Slashes ervoor gebeurt eigelijk alleen wanneer je "addslashes" gebruikt, dat zou ik eigenlijk gewoon afraden.

vrijdag 18 september 2015 13:54

Acties:

0 Henk 'm!

Verwijderd

Topicstarter

Okee, dank je voor je berichten jbdeiman.

Op mijn hostingserver staan nu nog magic quotes aan en die wil ik niet gebruiken dus ik ben meteen maar alles aan het dubbelchecken nu. Ik had het laatst gedaan via mysqli_real_escape_string en ik dacht dat ik ook de ge-escapete quotes toen in mijn db kreeg. Maar misschien liepen de magic quotes er ook nog bij te klooien toen. Goed om te weten allemaal, ik ga dit nog even dubbelchecken.

Beter is een (hash) op te slaan van een gebruiker in je sessie en in je DB. (Bijvoorbeeld een unieke hash op basis van tijdstip en gebruikersnaam oid...).

Okee, ik maak dan dus een hash aan van een gebruiker en die gooi ik in een sessie en in een nieuwe tabel in mijn database. Even kijken... hoe check ik dan die hash aan het begin van mijn pagina? Zoiets als: IF sessie[hash] == row.nieuwrecorduitdenieuwetabel[hash] AND dan moet ik nog kijken in de usertabel of het een beheerder is ja/nee. Klopt dat een beetje?

vrijdag 18 september 2015 13:55

Acties:

0 Henk 'm!

Patriot

Fulltime #whatpulsert

jbdeiman schreef op vrijdag 18 september 2015 @ 13:36:
@ntheweb
Op zich is die exit wel netjes, maar (normaliter) niet nodig. Waar je met een redirect wel op moet letten is dat je pas na de redirect mag beginnen met het sturen van output. (dus niet eerst "<html><header>...</header>" waarna je PHP opent en daar deze code neerzet)

Die exit is een absolute vereiste bij deze constructie. De uitvoer van het script stopt anders niet, en een kwaadwillende gebruiker kan op die manier alles inzien en doen wat een ingelogde gebruiker (of beheerder) kan.

vrijdag 18 september 2015 14:01

Acties:

0 Henk 'm!

Voutloos

M.b.t. magic quotes: De enige juiste configuratie is off. Correct escapen moet je hoe dan ook zelf doen cq. zorgen dat het - voor de juiste context! - voor je gedaan wordt.

Als magic quotes aan staat: De beste optie is een die('Magie moet uit staan');. Als dat niet kan, dan kan je het beste de magic quotes eerst z.s.m. ongedaan maken, dan weet je daarna tenminste waar je aan toe bent en hoef je niet in al je code wakker te liggen van die setting of dubbele encoding.

Ontopic (maar wellicht gerelateerd aan oudewetsche magic quotes): Ga eens op zoek naar betere, up to date, tutorial over sessions en authentication. Elk puntje waar je nu mee zit zou, tenzij tutorial/kennis uit 2005 is, niet meer voor mogen komen namelijk.

[ Voor 24% gewijzigd door Voutloos op 18-09-2015 14:05 ]

{signature}

vrijdag 18 september 2015 14:04

Acties:

0 Henk 'm!

Verwijderd

Topicstarter

Yep, de magic quotes staan in ieder geval uit nu!

Maar dat van die hash begrijp ik nog niet helemaal. Dus als iemand daar ook mee werkt en nog een tipje van die sluier kan oplichten, graag!

vrijdag 18 september 2015 14:19

Acties:

0 Henk 'm!

BlueZero

Een sessie is niet zo heel ingewikkeld te manipuleren, oftewel heel makkelijk te manipuleren zelfs.

Wat kan je daar tegen doen:
Maak een database tabel met de volgende kolommen:

session_id (INT 11) | hash (VARCHAR 255) | data ( TEXT ) | last_updated (INT 11)

Als iemand inlogt maak je een nieuwe sessie aan:

PHP:

//Genereer random hash (Dit kan stukken beter als onderstaand voorbeeld)
$hash = sha(mt_rand().time())
$_SESSION['hash'] = $hash

$data['user_id'] = 11;
$data['logged_in'] = 1;
$data['extra velden'] = 'blablba'

$insert['hash'] = $hash;
$insert['data'] = serialize($data);
$insert['last_updated'] = time();

//Doe een database insert

Als je vervolgens checkt of iemand ingelogd is.

PHP:

if(!check_hash()){
 die('Je was niet ingelogd schurkje!');
}

function check_hash(){

if($_SESSION['hash'] && !empty($_SESSION['hash'])){

   $sql = 'SELECT * FROM sessions WHERE hash = ....';
   //Doe query
   $session_data = unserialize($result['data']);
   
   if($session_data[''logged_in'] == 1){
        return true;
   }
   

}
   return false;
}

vrijdag 18 september 2015 14:24

Acties:

0 Henk 'm!

Patriot

Fulltime #whatpulsert

BlueZero schreef op vrijdag 18 september 2015 @ 14:19:
Een sessie is niet zo heel ingewikkeld te manipuleren, oftewel heel makkelijk te manipuleren zelfs.

Onzin. Een gebruiker kan niet zomaar bij de inhoud van een sessie. Het enige wat de gebruiker kan, is zijn sessie id manipuleren. Daar doet jouw methode helemaal niets tegen.

vrijdag 18 september 2015 14:25

Acties:

0 Henk 'm!

Verwijderd

Topicstarter

Hoe kan je dan je sessie id manipuleren?

vrijdag 18 september 2015 14:26

Acties:

0 Henk 'm!

Voutloos

BlueZero schreef op vrijdag 18 september 2015 @ 14:19:
Een sessie is niet zo heel ingewikkeld te manipuleren, oftewel heel makkelijk te manipuleren zelfs.

Hoe dan? User id en beheerder vlag staan nu niet in het cookie @ client side hoor. Zowel met native sessions als met jouw suggestie is enkel de session identifier cookie te manipuleren.

Alternatieve uitleg: wederom up to date tutorial.

{signature}

vrijdag 18 september 2015 14:31

Acties:

0 Henk 'm!

Patriot

Fulltime #whatpulsert

Verwijderd schreef op vrijdag 18 september 2015 @ 14:25:
Hoe kan je dan je sessie id manipuleren?

Door de cookie waarin deze is opgeslagen te wijzigen. In theorie is het mogelijk om de sessie van een andere gebruiker te kapen, maar daar kun je je op zich redelijk goed tegen beschermen.

EDIT: Voor de duidelijkheid: Je kunt het dus nooit voorkomen, maar je kunt er wel voor zorgen dat bruteforcen de enige manier is om een sessie te kapen, en dat is simpelweg niet praktisch uitvoerbaar.

[ Voor 21% gewijzigd door Patriot op 18-09-2015 14:33 ]

vrijdag 18 september 2015 14:36

Acties:

0 Henk 'm!

Verwijderd

Topicstarter

Okee, ik heb weer wat geleerd, bedankt allemaal voor al deze tips!

[ Voor 83% gewijzigd door Verwijderd op 18-09-2015 15:00 ]

vrijdag 18 september 2015 15:30

Acties:

0 Henk 'm!

azerty

Een kleinere tip verder: waarom store je het al dan niet admin zijn als "ja" of "nee" in je database in plaats van een bit/int/bool? Behalve leesbaarheid als je je tabel rechtstreeks aanpast in de databank ben je daar verder niet zo veel mee, en het maakt je code ook weer wat vervelender denk ik zo

vrijdag 18 september 2015 15:34

Acties:

0 Henk 'm!

Verwijderd

Topicstarter

Jazeker Azerty, dat was ook nog een puntje! Ik zal er een bool (0/1) van maken!

[ Voor 4% gewijzigd door Verwijderd op 18-09-2015 15:35 ]

maandag 21 september 2015 11:58

Acties:

0 Henk 'm!

BlueZero

Patriot schreef op vrijdag 18 september 2015 @ 14:24:
[...]

Onzin. Een gebruiker kan niet zomaar bij de inhoud van een sessie. Het enige wat de gebruiker kan, is zijn sessie id manipuleren. Daar doet jouw methode helemaal niets tegen.

Excuus ik keek iets te snel door de FIPO heen en dacht in mijn hoofd aan Cookie in plaats van native PHP Sessie. Dus correct mijn methode veranderd dan vrij weinig.

Ook is het veiliger om bij mijn methode ip check en user agent check toe te passen. Dus twee kolommen extra in de database user_agent en ip_address

Vervolgens als je de Sessie uit de database tegen de hash checkt gelijk even een where op remote_ip en user_agent.

maandag 21 september 2015 12:34

Acties:

0 Henk 'm!

azerty

BlueZero schreef op maandag 21 september 2015 @ 11:58:
[...]

Excuus ik keek iets te snel door de FIPO heen en dacht in mijn hoofd aan Cookie in plaats van native PHP Sessie. Dus correct mijn methode veranderd dan vrij weinig.

Ook is het veiliger om bij mijn methode ip check en user agent check toe te passen. Dus twee kolommen extra in de database user_agent en ip_address

Vervolgens als je de Sessie uit de database tegen de hash checkt gelijk even een where op remote_ip en user_agent.

Een ip change kan altijd gebeuren, dus ik zou daar geen heel strenge check op doen. Het kan een indicatie zijn, maar is niet noodzakelijk een goed bewijs dat er iets niet klopt.

maandag 21 september 2015 16:59

Acties:

0 Henk 'm!

Patriot

Fulltime #whatpulsert

BlueZero schreef op maandag 21 september 2015 @ 11:58:
[...]

Excuus ik keek iets te snel door de FIPO heen en dacht in mijn hoofd aan Cookie in plaats van native PHP Sessie. Dus correct mijn methode veranderd dan vrij weinig.

Ook is het veiliger om bij mijn methode ip check en user agent check toe te passen. Dus twee kolommen extra in de database user_agent en ip_address

Vervolgens als je de Sessie uit de database tegen de hash checkt gelijk even een where op remote_ip en user_agent.

Dat is verspilde moeite, en leidt tot een mindere gebruikerservaring. Ik heb er al eerder naar gelinkt, maar ik zal het nog eens doen: Hier staat wat je moet doen om de kans op session hijacking zo te verkleinen dat het niet interessant meer is als aanval.

dinsdag 22 september 2015 09:13

Acties:

0 Henk 'm!

BlueZero

Eens, zowiezo is het belangrijkste dat je zorgt voor SSL anders kan altijd de Session Id opgepikt worden en overgenomen.

dinsdag 22 september 2015 14:23

Acties:

0 Henk 'm!

Guillome

test

Tldr:

1. Je hebt het niet over 2 sessies maar sessievariablen
2. Gebruik geen varchar ja/nee voor beheerder, maar tinyint 1/0
3. Gebruik goede kolomnamen. Wat is Beheerder? Maak ervan: IsBeheerder met waarde 1 of 0
4. Doe geen while-loop maar if (mysql_num_rows > 0
5. Vast al genoemd, maar lees even over SQL-injectie

If then else matters! - I5 12600KF, Asus Tuf GT501, Asus Tuf OC 3080, Asus Tuf Gaming H670 Pro, 48GB, Corsair RM850X PSU, SN850 1TB, Arctic Liquid Freezer 280, ASUS RT-AX1800U router