Kan ikke finde fejl.

I SELECTen bruger du feltet nøgle, og i UPDATEn bruger du feltet key - det kan da ikke være rigtigt...

Jeg er til gengæld lidt nysgerrig efter at vide hvorfor OP mener at kombinationen af en session og en selvstændig cookie skulle kunne øge sikkerheden?

Argumenterne er de samme som for http://php.net/session_regenerate_id - som så faktisk vil være lidt nemmere at bruge.

Prøv at benytte
  if(mysql_num_rows($q)>=1)
i stedet for
  if(mysql_num_rows($q)==1)

Det ser ud til, at brugeren opretter en online entry hver gang der logges på, men de bliver ikke slettet. Derfor kan du ikke regne med, der kun er 1 entry.

Hvis det ikke virker, er det altid en god ide at debugge ved at udskrive dine variable i stedet for at teste om det hele virker på en gang:

<?php
session_start();
include "connect.php";
print_r($_SESSION);
print_r($_COOKIE);
if(isset($_SESSION['brugernavn']) && isset($_COOKIE['key']))
{
$q=mysql_query("SELECT * FROM online WHERE brugernavn='".$_SESSION['brugernavn']."' AND nøgle='".$_COOKIE['key']."' ");
if(mysql_num_rows($q)==1)
{
$key=time().rand();
mysql_query("UPDATE online WHERE brugernavn='".$_SESSION['brugernavn']."' SET key='".$key."'");
setcookie("key", $key);
echo "Du er stadig online"; die;
}
else
{
echo "Fejl";
}
}
?>

sqrt(1): Brugernavn OG random nøgle skulle gerne være så unikt, at der kun er een række, der passer.

sqrt(1): Næh, ikke nødvendigvis ... fordi den sidste UPDATE (hvis den virkede) sætter nøglen på alle indgange for brugeren. Så du har ret.

Men det kan som sagt laves simplere.

Hvis der kun skal være en entry for online, ville jeg måske lægge den i bruger tabellen. Medmindre det skal bruges som aktivitetslog.

#1 Det er rigtigt, der er en fejl der.

#2 Læs evt. http://www.eksperten.dk/guide/857

#3 session_regenerate_id() er en rigtigt god ide, for at gøre det nemmere. Men det ændre ikke på problemet.

#4 Du skal bare gå ud fra der er 1 i min lille test.

#5 Har testet det på en helt masse måder for at finde fejlen.

#6 Brugernavnet skal gerne være helt unikt.

#7 Jo brugernavet skal gerne være unikt ellers giver det problemer her "if(mysql_num_rows($q)== 1)".

#8 Det er i min lille test ikke vigtigt i hvorfor en tabel jeg lægger min data i.

Jeg har nu rette lidt og det ser ud som her under. Det virker lidt bedre. Problemet er nu at hvis jeg opdaterer (trykker F5) en helt masse så skriver den stadig "Fejl". Det virker dog hvis jeg skriver "if(mysql_num_rows($q)>=1)" i stedet for "if(mysql_num_rows($q)==1)". Men det vil jo sige at der kan være mere end en bruger med disse oplysninger? og sådan skal det ikke være. Håber i kan finde fejlen.

//1.php
<?php
session_start(); //Starter session
$_SESSION['brugernavn'] = "lalalllala";
include "connect.php";
$key=time()+rand();
setcookie("key", $key);
mysql_query("INSERT INTO online (id,brugernavn,nøgle)VALUES(NULL,'".$_SESSION['brugernavn']."','".$key."')");
?>
<a href="2.php">Gå nu til 2.php</a>



//2.php
<?php
session_start();
include "connect.php";
if(isset($_SESSION['brugernavn']) && isset($_COOKIE['key']))
{
$q=mysql_query("SELECT * FROM online WHERE brugernavn='".$_SESSION['brugernavn']."' AND nøgle='".$_COOKIE['key']."' ");
if(mysql_num_rows($q) == 1)
{
$key=time()+rand();
mysql_query("UPDATE online SET nøgle = '".$key."' WHERE brugernavn = '".$_SESSION['brugernavn']."'");
setcookie("key", $key);
echo "Du er stadig online";
}
else
{
echo "Fejl";
}
}
?>

Kig hvad der står i databasen: hver gang du kører 1.php indsætter du en ny række.

#10 Ja det er jeg godt klar over. Men dette er en test så jeg kører kun 1.php 1 gang hvor efter jeg kører 2.php. Og jeg tømme tabellen efter hvert forsøg. Du skal se 1.php som en login-side og 2.php som en side der tjekker om du er logget ind.

Så 'session hijacking' er simpelthen årsagen til det finurlige påfund? Jo, jo...

#12 Jeg synes da ikke det er et "finurligt påfund". Men hvis du har et bedre forslag til en løsning, må du meget gerne skrive den.

#11 har du dobbelttjekket, at der kun er en? Når kun mysql_num_rows >= 1 tyder på, at der er mere end 1!!!!!!

#14 Ja, jeg har testet det så meget jeg kan. Ved ikke lige hvordan jeg skulle kunne teste det mere. Min eneste ide til at der kan fejlen skulle være at mysql er for langsomt. Men det tror jeg ikke.

*Min eneste ide til hvad fejlen skyldes skulle være at mysql er for langsomt. Men det tror jeg ikke.

#13
Det var nu ikke fordi det var et problem jeg havde tænkt mig at imødekomme; men hvis det endelig er synes jeg da at #3 kom med et fremragende alternativ som simpelt kunne løse dit problem - endda på en pæn måde...

#17 Jeg har også tænkt mig at benytte session_regenerate_id() men det løser bare ikke mit problem.

#15 Du kunne jo udskrive de fundne værdier sammen med session og cookie og paste det ind her. Benyt dette script og paste det fra kildekoden (I IE: højreklik -> view source)

//1.php
<?php
session_start(); //Starter session
$_SESSION['brugernavn'] = "lalalllala";
include "connect.php";
$key=time()+rand();
setcookie("key", $key);
mysql_query("INSERT INTO online (id,brugernavn,nøgle)VALUES(NULL,'".$_SESSION['brugernavn']."','".$key."')");
?>
<a href="2.php">Gå nu til 2.php</a>



//2.php
<?php
session_start();
include "connect.php";

print_r($_SESSION);
print_r($_COOKIE['key']);

if(isset($_SESSION['brugernavn']) && isset($_COOKIE['key']))
{
$q=mysql_query("SELECT * FROM online WHERE brugernavn='".$_SESSION['brugernavn']."' AND nøgle='".$_COOKIE['key']."' ");

while($data = mysql_fetch_assoc($q)){
  print_r($data);
}

if(mysql_num_rows($q) == 1)
{
$key=time()+rand();
mysql_query("UPDATE online SET nøgle = '".$key."' WHERE brugernavn = '".$_SESSION['brugernavn']."'");
setcookie("key", $key);
echo "Du er stadig online";
}
else
{
echo "Fejl";
}
}
?>

#18
Problemet ligger nok i din database.
Det eneste jeg har gjort er at oprette en tabel der hedder online og navngive felterne anderledes.
Og så brugte jeg de oprindelige scripts fra #0 og rettede kun feltnavnene til i dine SQL sætninger.
Det virker fint.
SQL:


1.php: http://php.pastebin.com/m379b7d10
2.php: http://php.pastebin.com/m9f633c6

#19 Ved ikke helt hvad du mener. Men har lavet så man kan se siderne online nu.

//1.php
http://www.je1020.wep.dk/login/1.php
//2.php
http://www.je1020.wep.dk/login/1.php
//Til at se data fra tabellen "online" og se session/cookie
http://www.je1020.wep.dk/login/test.php
//Til at slette session og cookie
http://www.je1020.wep.dk/login/slet.php

#20 Tror altså ikke det er databasen. Du kan selv prøve at gå ind på 1.php og så 2.php og så trykke F5. Du kan se linke her over.

#21
Mit forslag har jeg testet og konstateret at det virker; har du selv prøvet at teste det jeg har givet dig? Eller konstaterer du blot at det ikke er databasen der fejler noget?

#22 Har prøvet det du sagde. Men det virker ikke. Når jeg trykker F5, så siger den fejl.

#22 Det du har givet mig ligger på http://www.je1020.wep.dk/login2/ og det virker ikke.. Jeg får også fejlen Warning: mysql_num_rows(): supplied argument is not a valid MySQL result resource in F:\HJEMMESIDER\JE1020\JE1020.WEP.DK\login2\2.php on line 7 ved din udgave.

Det er lidt svært at bruge de links til noget, fordi du nu ingen chance har for at vide hvor mange der klikker på 1.php

#24: Eksemplet fra repox bruger vist nogle andre navne i tabellerne, derfor SQL-fejlen - formentlig.

#24 Nej ups. Den fejl kommer ikke. Men den skriver stadig fejl når man trykker F5.

#25 Har lige lavet så det ikke betyder noget hvor mange der trykker. Du får et unikt brugernavn når du går ind i filen 1.php.

Hvis du opdaterer meget hurtigt, kan du sikkert godt komme ud af sync med værdierne. Men hvad er der i databasen? Der er kun een linie for brugernavnet, ikke?

#29 men det skal jeg jo bare ikke kunne komme ud fra.. Der er de her to tabeller, de ligger ikke i samme db. 
//login

CREATE TABLE IF NOT EXISTS `online` (
  `id` int(11) NOT NULL auto_increment,
  `brugernavn` varchar(255) NOT NULL,
  `nøgle` varchar(255) NOT NULL,
  PRIMARY KEY  (`id`)
) ENGINE=MyISAM  DEFAULT CHARSET=latin1 AUTO_INCREMENT=3 ;


//login2

CREATE TABLE IF NOT EXISTS `online` (
  `oId` int(11) NOT NULL auto_increment,
  `oBrugernavn` varchar(255) NOT NULL,
  `oKey` varchar(255) NOT NULL,
  PRIMARY KEY  (`oId`)
) ENGINE=MyISAM  DEFAULT CHARSET=utf8 AUTO_INCREMENT=1 ;

Er du sikker på det ikke er fordi at der sidder et par stykker af os - udover dig selv - og tester på 1.php og 2.php samtidig? For så skal det jo fejle (det var vel hensigten).

Udover det, så er der i 2.php, linie 7 også lige det krav at mysql resultatet SKAL returnere 1 række. Og i 1.php indsætter du faktisk bare en ny række for hver gang (ihvertfald inden du valgte at generere et unikt brugernavn for hver gang man besøgte 1.php).

Nej, den er god nok - at den fejler, altså. Du kan få 2 opdateringer af databasen, men kun een cookie værdi tilbage - og måske den forkerte, afhængig af hvordan det kører på serveren.

Men nu for tredie gang: Hvad står der i databasen? Er der tilfælde med mere end een række for samme brugernavn - i hvert fald efter det er blevet tilfældigt?

#31 Nej det er ikke derfor det fejler for når alle har et unikt brugernavn er det ikke et problem.

#32 Er ikke helt med på hvad du mener med "Du kan få 2 opdateringer af databasen, men kun een cookie værdi tilbage - og måske den forkerte, afhængig af hvordan det kører på serveren."

Men nej det er usandsynligt man får det samme brugernavn.. :) men du kan se alt der ligger i "online" ved at gå til http://www.je1020.wep.dk/login/test.php

Er der ikke en der kan komme med en fornuftigt forklaring på det virker med "if(mysql_num_rows($q)>=1)" og ikke med "if(mysql_num_rows($q)==1)" ?

Nej, jeg kan ikke forklare ">="-problemet ud fra de givne data. Før du lavede tilfældige brugernavne, kunne der godt være et problem.

Når du laver 2 hurtige forespørgsler til webserveren, udføres begge i "tilfældig" rækkefølge. Dvs at du får begge opdateringer til databasen, men kun een af cookie-værdierne.

#36 Ok. Men hvad gør man normalt i et login-system med hensyn til det her?

Normalt: ikke noget.

Skal det være sikrere kan man bruge en https://-forbindelse.

Som jeg også skrev tidligere var det ikke et problem jeg gad at imødekomme. Og som nævnt i #38, så ville en fin løsning være SSL, for at undgå packet sniffing, som nok er den eneste 'reelle' trussel i denne sammenhæng.

#38, #39 Så hvis jeg bare lave det sådan jeg gemmer en cookie eller bruger session_id(), som jeg som jeg gemmer værdien af i en tabel og brugens ip. Så kan jeg tjekke om bruger sidder på den samme ip hele tiden ellers skal han ikke have lov at være logget mere. så skal en hacker både lave sin ip om og og skaffe det session_id().  Er det ikke det mange andre gør og er det er god løsning? så kan jeg evt. bruge SSL hvis det skal være mere sikkert.. Men synes det er trælst jeg ikke kan skifte den cookie ud hele tiden.. :( Det kunne være dejligt at gøre det meget svært at få fat i den.

Lige en ting til. Ville det være dumt at bruge kode med if(mysql_num_rows($q)>=1)?

2.php virker ikke mere.

Ja det vil være dumt at bruge løsningen if(mysql_num_rows($q)>=1), når du ikke ved, hvorfor det er nødvendigt.

#40
Jeg bliver nød til at vide, hvorfor du mener at du bliver nød til tackle problemet med session hijacking? Er det et problem du er stødt på? Og hvorfor kan du ikke løse problemet med teknikken nævnt i #3, hvis det er så vigtigt?

Det med IP-nummeret skal du nok tænke over en ekstra gang.

Der findes brugere som skifter IP hele tiden - I USA er/var AOL kendt for det. Det er nok ikke synderligt udbredt i Danmark. Måske vil mobilt Internet også kunne skifte IP-nummer. Det vil give dig utilfredse besøgende, hvis de bliver smidt af hele tiden.

Og på den anden side, så kan der jo sagtens sidde mange personer bag det samme IP-nummer.

Ganske overordnet skal du bestemme dig til hvor meget "sikkerhed" du vil have. Jo mere sikkerhed, jo mere bøvl.

#43 Det gør den nu.. Ja, det er rigtigt nok.

#42 Jeg synes generelt alt sikkerhed er meget vigtigt. Derfor synes jeg også man skal lukke alle huller så godt som muligt som udvikler. Hvis du kan lave et eksempel med teknikken nævnt i #3 som virker. Så synes jeg det er meget flot og så vil jeg gerne have du skriver hvad ud gør. Om jeg bruger session_regenerate_id eller min løsning, så får jeg da det samme problem?

#43 Det er rigtigt nok at nogle skifter ip-nummer tit. Men hvis ikke jeg gør andet end at sige $_SESSION['loggetind'] = ja; så kan en hacker da let komme ind?

#45


Ret simpelt egentlig - den cookie som er ansvarlig for at medsende session id'et skifter hver eneste gang session_regenerate_id() anvendes. parameteren true anvender jeg så for at slette den gamle session.
Ved at give brugeren et nyt session id, overføre den aktuelle sessions data over i den nye og samtidig slette indholdet i den gamle, vil det blive tæt på umuligt for en 'hacker' at hijacke din session.
Så slipper du for at lave det gøjl som du har gang i, når nu PHP egentlig basalt set selv gør hvad det er du forsøger.

#47 Det ser godt ud. :) Hvad skal det her til for "$_SESSION["tal"]++;"?



Du vil bare have jeg skal bruge det uden en database så?

$_SESSION["tal"]++ var blot for at illustrere at du bibeholdte din session, selvom du - nemt - skiftede session id.
Men ja, du behøver ikke en database for at opretholde en sikker session på dette.

#49 Så hvis jeg gør som her under. Så undgår (gør et forsøg for at undgå)jeg session hijacking. Eller skal jeg give $_SESSION["logget_ind"] en ny værdi? så det ikke er "ja" hele tiden hvis brugeren er logget ind? 

//login side
<?php
session_start(); //Starter session
$_SESSION["logget_ind"] = ja;

der mangler en masse med kode tjek og sådan noget med lige meget.
?>

//tjek side
<?php
session_start(); //Starter session
session_regenerate_id(true);

if(!isset($_SESSION["logget_ind"]))
    $_SESSION["logget_ind"] = ja;
else
{
exit;
}

?>

Altså, at have session variabler kaldet logget_ind med indhold som 1 eller true eller bare en statisk streng er - i min verden - ret tåbeligt.
Anvend kun ting du rent faktisk kan bruge.

Session variablen "logget_ind" kan jo kun bruges til en ting - og det er at konstatere at en given bruger er logget ind.

Personligt foretrækker jeg at anvende variablen til noget praktisk.

Eksempelvis:


Og når du så skal tjekke om folk er logget ind:

Det ser rigtigt godt ude og nu er jeg noget mere sikker på hvordan jeg skal gøre det. Mange tak. Har bare lige et spørgsmål mere hvad sker der her " $password = md5($salt.$_POST["password"]);". hvad bruger du $salt til?

Men ellers bare smid et svar. :)

$salt er en lang kompliceret streng bestående af en statisk tekst som jeg 'salter' et kodeord med.
Det gør det (nærmest) umuligt at decrypte et kodeord f.eks vha af rainbow tables.
F.eks. vil kodeordet 'test' få md5 værdien 098f6bcd4621d373cade4e832627b4f6
Et hurtigt lookup i en rainbow table fortæller at md5 strengen 098f6bcd4621d373cade4e832627b4f6 svarer til ordet test.
Men hvis jeg nu salter den med en x antal tegn lang sætning med tilfældigt udvalgte tegn:

så vil du opnå en en anden md5 værdi som ikke bare kan slås op i en rainbow table.

Årsagen til jeg havde det med er at jeg bruger den teknik til alt hvad der har med mine brugere at gøre; det er god etik at værne om sine brugeres privatliv.

#53 Okay. :) Det er ikke en helt dum ide. Tak for hjælpen.

#51 Hvad mener du med "Kontrollen kan være mere omfattende.". Skal der tjekkes om brugeren findes eller hvad havde du tænkt.

#55
Det er en mulighed.
Jeg kontrollerer - i et system hvor kunden mener brugerkontrollen er kritisk - for hvert sidekald om brugeren er logget ind,herefter om brugeren stadig eksister, dernæst om brugeren er aktiv og slutteligt om brugeren har rettigheder til at anvende siden (såfremt siden er omfattet af restriktioner).

Men du skal altid have i tankerne om din kode står mål til formålet; kan det forsvares at lave så omfattende brugerkontroller eller er det nok blot at kontrollere at det er en bruger der er logget ind? Eller er det nødvendigt for en administrator at kunne lukke for brugeren realtime?

Ok. Lige en ting, hvordan vil du tjekke om en bruger er "aktiv", hvis der bliver lavet et sidekald må brugeren da være det?

Brugeren som sådan behøver ikke være en aktiv bruger i systemet. 'Aktiv' betyder ikke om brugeren foretager sig noget i denne sammenhæng - det skal forstås som om at en bruger kan have en aktiveret eller deaktiveret brugerkonto på sitet.

Nå på den måde. :) Tak.