[PHP/MySQL] sicheres Sessionsystem?

BartTheDevil89

BartTheDevil89

Devilution Media
ID: 87739
L
2 Mai 2006
3.960
103
Hallo,

ich bin gerade dabei mir ein Script zu erstellen. Allerdings komm ich jetzt beim Thema Loginscript in Probleme, bzw. eben die Frage nach der Sicherheit. Denn ich möchte ein Sessionsystem, das auch Gästen eine Session zuordnet...also jeder der kommt bekommt ne Session und wenn der Login korrekt war, wird in der Session dann die Userid gespeichert. Ich habe mir das wiefolgt gedacht:

1. eine Datenbank. id ist die id, die dann die Session identifiziert. lastactivity ist immer die Zeit als zum letzten mal überprüft wurde, ob die ID noch genutzt wird. Userid ist entweder 0 für Gast oder ne ID für einen User später. Und Gruppe gibt an in welchem Usergruppe der Besucher ist.(gibt später an, was der User darf, bzw. eben ein Gast). IP eben die IP des Users.
id/lastactivity/userid/gruppe/ip

2. Besucher betritt die Seite
der Besucher war noch nie da. Damit wird ein Eintrag in der Daten erstellt mit userid = 0 und gruppe = 0, ip = die ip und last activity eben die Zeit. diese ID des Eintrages wird dann unter $_SESSION[id] gespeichert.

3. der Besucher ruft eine zweite Seite auf
die $_SESSION[id] wird der Session in der Tabelle zugeordnet und es sind damit die Daten verfügbar. Sollte die id in der Datenbank nichtmehr vorhanden sein(weil Session nichtmehr gültig ist oder $_SESSION[id] nicht vorhanden ist; siehe Punkt 5) wird einfach ne neue erstellt.

4. bei Userlogin: Wenn sich ein User einloggt steht ja auch die id der Session fest, die ihm vorher zugeteilt wurde. Wenn dann der Login erfolgreich war, wird in der Session die Userid eben auf die des Users geändert und die Spalte Rechte auch.
- wenn sich ein User ausloggt, eben das gleiche

5. Im Hintergrund laufen dann ein paar automatische Funktionen
- alte Sessions leeren: Die Besuchersessions werden nach einer Zeit von 2 Stunden seit der letzten Aktion (ist ja über lastactivity feststellbar) gelöscht. Damit verfällt ja dann auch $_SESSION[id], weil ja kein passender Eintrag in der DB mehr ist
- automatsiches Ausloggen: Usersessions also Sessions von eingeloggten Usern, werden nach einer Zeit von 20 Minuten seit der letzten Aktion gelöscht.

6. Problem mit "Eingeloggt bleiben":
Es gibt ja die Funktion "Eingeloggt bleiben", also es darf dann die Session in der Datenbank nicht gelöscht werden(kann ich ja durch ne neue datenbankspalte machen, die dann 1 ist und somit vom automatischen Ausloggen nicht betroffen ist). Aber wie speichere ich dann die id auf dem Computer am besten und vor allem sichersten? Cookie?

Daher eben meine Frage: Was würdet ihr an diesem System als kritisch ansehen? Bzw. wo sind Möglichkeiten es sicherer zu machen?
 
Hallo

Ich weiß jetzt nicht so genau, wo dein Sicherheitsproblem ist. Es ist möglich, eine Session zu "übernehmen", allerdings ist das nicht ganz unaufwendig. Da wäre z.B. Passwortsicherheit bei den Usern ein wichtigeres Thema.

Ganz allgemein fallen mir ein paar Sachen auf:
- Gibt es einen wichtigen Grund, warum du Session_id's in der DB speichern musst? Der Webserver verwaltet die Sessions ja sowieso. Du kannst ja über "session_start();" auf jeder Seite die Session starten und dann wird automatisch eine SessionID erstellt und jedem Nutzer mitgegeben. Also wenn du nicht aus irgendeinem wichtigen Grund (z.B. Seitenstatistik) jeden Nutzer eindeutig identifizieren musst, dann finde ich das komisch.
Für registrierte Nutzer, die sich einloggen, macht das ja Sinn, aber bei allen sonstigen Besuchern ???

- Ich weiß nicht genau, wie lange Session gültig bleiben. Wahrscheinlich kann man das in irgendwelchen Konfigurations-Dateien festlegen. Damit würde deine "ich lösche alle alte Sessions aus der DB"-Sache unnötig.

- Eingeloggt bleiben
Du mußt auf dem Computer des Users ein Cookie speichern, in das schreibst du folgende Daten:
userID
md5(irgenwelchem zufälligen Text oder zahlen)

Den md5-text mußt du natürlich mit in der DB abspeichern!
Dann änderst du deine Seiten so, dass am Anfang der Seite zuerst geprüft wird ob ein Cookie vorhanden ist und valide ist. Wenn ja, dann logst du den Benuzter ein. Wenn nicht, dann wie gehabt.

Ich würde es so machen (wobei ich nicht weiß, ob es deinen Anforderungen entspricht). Bei jedem Seitenstart passiert folgendes:
- session_start();
- Cookie vorhanden? Wenn ja, dann evtl. User einloggen ($_SESSION['userID']=$userIDAusCookie)

Damit wird jeder User vom Webserver eindeutig identifiziert. In die DB schreiben mußt du eigentlich nix.

Wenn sich jetzt ein User einloggt, dann speicherst du in der Session die userID: $_SESSION['userID']=$userIDAusDatenbank;

Du kannst jederzeit prüfen, ob ein User eingeloggt ist, indem du schaust, ob $_SESSION['userID']>0. Wenn die userID in der Session größer als 0, dann ist der User eingeloggt, ansonsten eben nicht.
Wenn du wissen willst, wann sich der user zuletzt eingeloggt hat, dann kannst du hier den Login-Vorgang noch in der DB mitprotokollieren.

Wenn der user eingeloggt bleiben will, dann mußt du das Cookie setzen (siehe oben).

Wenn noch jemand weiß, wie man die Dauer einer Session bestimmten kann, dann immer her damit ;)
Du sparst damit zumindest ganz viele DB-Abfragen.

Wie kann man das sicher machen?
- User müssen sichere Passwörter nutzen!
- Kein "eingeloggt bleiben" zulassen
 
Hi,

die Sessions alle über die Datenbank laufen zu lassen bringt mir halt den Vorteil, dass ich so Sachen wie "Es sind X User und X Gäste" online oder "Wer ist wo", ... umsetzen kann.
Natürlich ist es nicht ganz sicher zu machen, aber genau deswegen wollte ich ja wissen, wie ich das System sicherer machen kann.
 
Deine Ansätze sind soweit schon ganz gut, aber ein gravierender Punkt, der mir aufgefallen ist: Wozu brauchst Du überhaupt noch $_SESSION? Mit Deinem Ansatz kannst Du doch eigentlich auf das Sessionmanagement von PHP komplett verzichten. Speicher die Session-Id im Cookie und nutz dies zur Identifikation. Ob Du sie nochmal lokal in $_SESSION stehen hast, macht ja keinen Unterschied, wenn es eh der einzige Eintrag in dem Array ist und der auch noch gleich dem Wert im Cookie ist.
 
tleilax schrieb:
Deine Ansätze sind soweit schon ganz gut, aber ein gravierender Punkt, der mir aufgefallen ist: Wozu brauchst Du überhaupt noch $_SESSION? Mit Deinem Ansatz kannst Du doch eigentlich auf das Sessionmanagement von PHP komplett verzichten. Speicher die Session-Id im Cookie und nutz dies zur Identifikation. Ob Du sie nochmal lokal in $_SESSION stehen hast, macht ja keinen Unterschied, wenn es eh der einzige Eintrag in dem Array ist und der auch noch gleich dem Wert im Cookie ist.
Zum Vergrößern anklicken....

Also mir wurde erklärt, dass Cookie recht unsicher sein soll in Vergleich zu $_SESSION und deswegen wollte ich im Prinzip nur bei der eingeloggt bleiben-Variante auf ein Cookie zurückgreifen. Denn ist es nicht recht leicht über das cookie die session-id zu bekommen?
Und was heißt "soweit schon ganz gut"? :ugly:....was würde denn gemacht werden, damit es "soweit gut" ist? :p
 
Die PHP Sessions setzen auch nen Cookie mit der Session-ID und du sollst ja auch nur deine eigene Session-ID speichern.
 
Naja, solange Du Cookies erlaubst, wird auch PHP (als Default-Einstellung) immer versuchen, Deine Session-Id im Cookie zu speichern. Ob da nun die von PHP drin steht oder Deine eigene, ist für den Fall, dass diese kompromititert ist, ja eher belanglos. Als fremder User kann man sich in jedem Fall ausgeben...

Zum "Soweit gut" würde ich es implementieren. Konzepte lesen sich meistens gut, aber sind halt nur Konzepte. Erst die Implementierung zeigt, wie gut das Konzept ist...
 
tleilax schrieb:
Naja, solange Du Cookies erlaubst, wird auch PHP (als Default-Einstellung) immer versuchen, Deine Session-Id im Cookie zu speichern. Ob da nun die von PHP drin steht oder Deine eigene, ist für den Fall, dass diese kompromititert ist, ja eher belanglos. Als fremder User kann man sich in jedem Fall ausgeben...

Zum "Soweit gut" würde ich es implementieren. Konzepte lesen sich meistens gut, aber sind halt nur Konzepte. Erst die Implementierung zeigt, wie gut das Konzept ist...
Zum Vergrößern anklicken....

OK gut...werde das ganze mal in Code umsetzen und dann wieder präsentieren. ;)
 
Hi,

also hab mich jetzt mal an den Code gesetzt und an der Tabelle die IP und die Rechte weggelassen. Ich hoffe ich hab alles gut kommentiert, dass ihr mitkommt:

Tabelle "xyz_sessions" (xyz = Tabellenpräfix)
sessionid/lastactivity/userid/locked
Zum Vergrößern anklicken....

session.php (wird auf jeder Seite als erstes aufgerufen)
PHP: 
$checksestime = time() - (60 * 60 * 2); //timestamp von jetzt - 2 Stunden erstellen
$checklogtime = time() - (60 * 20); //timestamp von jetzt - 20 Minuten erstellen
$db->query("delete from ".$dbpraefix."_sessions where lastactivity < '$checksestime' and userid='0'");//Lösche alle Sessions bei denen kein User eingeloggt ist und letzte Aktivität länger als 2 Stunden her ist
$db->query("delete from ".$dbpraefix."_sessions where lastactivity < '$checklogtime' and userid!='0' and locked = '0'");//Lösche alle Sessions bei denen ein User eingeloggt ist, letzte Aktivität länger als 20 Minuten her ist und "Eingeloggt bleiben" nicht aktiviert ist

$sessionid = mysql_real_escape_string($_COOKIE['sessionid']);//Cookie laden
if($sessionid != ""){//Daten gefunden
$sessionanzahl = $db->anzahl("Select * from ".$dbpraefix."_sessions where sessionid = '$sessionid'");//Anzahl der Datenbankeinträge für $sessionid
if($sessionanzahl == 1){//Session in der DB vorhanden
$session = $db->data("Select * from ".$dbpraefix."_sessions where sessionid = '$sessionid'");//Frage Daten der Session ab
$db->query("update ".$dbpraefix."_sessions set lastactivity = ".time()." where sessionid = '$sessionid'");//Update lastactivity-time
}else{//Session nicht vorhanden
$sessionid = "";//Lösche Session-Variable
}
}

if($sessionid == ""){//Keine Session vorhanden oder Session gelöscht
$sessionid = md5(uniq_id('sessions', true));//einzigartige ID erstellen
$db->query("insert into ".$dbpraefix."_sessions set sessionid = '$sessionid',lastactivity=".time().",user='0',locked='0'");
setCookie("sessionid", $sessionid, time() + 60 * 60 * 24 * 356 * 10);//Cookie für 10 Jahre speichern (sollte ja hoffentlich reichen)
}

beim login:
PHP: 
{//wenn der Login korrekt ist, dann

$db->query("update ".$dbpraefix."_sessions set userid='$userid' where sessionid = '$sessionid'");//Setze Userid in die session ein
if($locking == 1){//wenn "Eingeloggt bleiben" aktiviert
$db->query("update ".$dbpraefix."_sessions set locked='1' where sessionid = '$sessionid'");//Setze locked = 1 um "Eingeloggt bleiben" zu aktvieren
}

}

beim logout:
PHP: 
{//wenn der Logout korrekt ist, dann

$db->query("update ".$dbpraefix."_sessions set userid='0',locked='0' where sessionid = '$sessionid'");//Lösche Userid und Locked (egal ob "Eingeloggt bleiben" aktiviert war oder nicht

}

Hab mit meiner kleinen DB-class gearbeitet, aber steht ja immer dabei, was die Abfrage jeweils macht.

Also, wo gibts Besserungsmöglichkeiten, wie kann ich es vor allem sicherer machen?
 
Zuletzt bearbeitet:
Ich plädiere auch stark für OOP. So ist das Ganze ja nicht wirklich zu gebrauchen, sorry. Wie schreibst Du zB Daten in die Session? Hier bietet sich entweder eine statische Klasse oder auch das Singleton-Pattern an, wobei die beiden sich in diesem Fall nicht viel tun. Ich würde zur statischen Klasse tendieren.

Zum Punkte Sicherheit:

Du übergibst direkt die Werte aus dem Cookie in ein Query. Sehr schlechte Idee.

Darüber hinaus ist es echt unschön, was Du in Zeile 9 und 11 machst. Zweimal das gleiche Query, wobei Du das erste nicht wirklich auswertest? Das geht eleganter...

Und schlussendlich halte ich Deinen "Garbage Collector" in Zeile 3 und 5 für fragwürdig. Ich würde die Sessionlebensdauer von Cookie und Datenbank abgleichen. Sprich: Die Sessions in der DB sind genau so lange gültig wie das Cookie, das Du beim User setzst. Ohne Cookie würde ich nach 3 Stunden Inaktivität löschen. Die Datenbank schert's nicht, wenn da ein paar mehr Einträge sind, aber Benutzer, die vom Rechner weg waren, wieder kommen und noch den alten Zustand haben, sind froh drüber. ;)

Wieso verfällt die Session eingeloggter Benutzer ohne Autologin nach 20 Minuten, während die von Besuchern erst nach 2 Stunden verfällt?

Achja, Deine Frage aus Zeile 18:
PHP: 
$unique_id = md5(uniq_id('sessions', true));
 
tleilax schrieb:
Ich plädiere auch stark für OOP. So ist das Ganze ja nicht wirklich zu gebrauchen, sorry. Wie schreibst Du zB Daten in die Session? Hier bietet sich entweder eine statische Klasse oder auch das Singleton-Pattern an, wobei die beiden sich in diesem Fall nicht viel tun. Ich würde zur statischen Klasse tendieren.
Zum Vergrößern anklicken....
OK in ne Klasse kann ich das ja noch setzen, das sollte ja nicht so das Problem sein. Dabei dann ne Funktion fürs Schreiben in die Session, etc.

Zum Punkte Sicherheit:

Du übergibst direkt die Werte aus dem Cookie in ein Query. Sehr schlechte Idee.
Zum Vergrößern anklicken....
Ja ok, denkst du reicht addslashes()?
Darüber hinaus ist es echt unschön, was Du in Zeile 9 und 11 machst. Zweimal das gleiche Query, wobei Du das erste nicht wirklich auswertest? Das geht eleganter...
Zum Vergrößern anklicken....
Zeile 9 fragt über num_rows die Anzahl der Einträge in der DB ab und wenn das eben 1 ist, lädt er die Daten dieses Eintrages in Zeile 11.
Und schlussendlich halte ich Deinen "Garbage Collector" in Zeile 3 und 5 für fragwürdig. Ich würde die Sessionlebensdauer von Cookie und Datenbank abgleichen. Sprich: Die Sessions in der DB sind genau so lange gültig wie das Cookie, das Du beim User setzst. Ohne Cookie würde ich nach 3 Stunden Inaktivität löschen. Die Datenbank schert's nicht, wenn da ein paar mehr Einträge sind, aber Benutzer, die vom Rechner weg waren, wieder kommen und noch den alten Zustand haben, sind froh drüber. ;)

Wieso verfällt die Session eingeloggter Benutzer ohne Autologin nach 20 Minuten, während die von Besuchern erst nach 2 Stunden verfällt?
Zum Vergrößern anklicken....
Naja ich dachte halt, dass ich es durch das Löschen ja einfach bestimmen kann und über die Cookies jetzt extra nochmal die Zeit mitlaufen zu lassen, find ich fast bisschen aufwendig. Außerdem halte ich so meine session-db klein, da eben alte Einträge direkt gelöscht werden. Das mit den 2 Stunden ist so, dass ja bei Besuchern eigentlich egal ist, wie lang die Session läuft. Aber ich denke nach 2 Stunden ohne ner Aktion kommt dieser Besucher nicht wieder und damit verfällt dann eben auch seine Session und die Datenbank wird auch schön klein gehalten. Bei den Usern mit 20 Minuten gehts mir vor allem um User in Internetcafes, etc. die sich eben nicht ausloggen. Denn sonst könnte man ja am gleichen PC mit der Session des alten Users und damit ja auch im Useraccount des Users arbeiten und das soll ja nicht möglich sein. Also wird einfach nach 20 Minuten Inaktivität der User automatisch ausgeloggt.
Achja, Deine Frage aus Zeile 18:
PHP: 
$unique_id = md5(uniq_id('sessions', true));
Zum Vergrößern anklicken....

Danke
 
BartTheDevil89 schrieb:
Ja ok, denkst du reicht addslashes()?
Zum Vergrößern anklicken....
mysql_real_escape_string() ist die funktion hierfür. (in deinem letzten thread schon erwähnt)

Zeile 9 fragt über num_rows die Anzahl der Einträge in der DB ab und wenn das eben 1 ist, lädt er die Daten dieses Eintrages in Zeile 11.
Zum Vergrößern anklicken....
die sql-funktion COUNT() ist der PHP-funktion mysql_num_rows vorzuziehen. (in deinem letzten thread schon erwähnt). wenn anzahl() COUNT() nutzt, würde ich das in dem eigentlichen script so lassen, wie es jetzt ist. bei mysql_num_rows brauchst du die query nur einmal zu machen, stat wie jetzt 2 mal.
 
glowhand schrieb:
mysql_real_escape_string() ist die funktion hierfür. (in deinem letzten thread schon erwähnt)
Zum Vergrößern anklicken....
OK, danke....wobei mir auch schonmal erklärt wurde, dass der check mit besonderen Zeichen irgendwie umgangen werden kann.
die sql-funktion COUNT() ist der PHP-funktion mysql_num_rows vorzuziehen. (in deinem letzten thread schon erwähnt). wenn anzahl() COUNT() nutzt, würde ich das in dem eigentlichen script so lassen, wie es jetzt ist. bei mysql_num_rows brauchst du die query nur einmal zu machen, stat wie jetzt 2 mal.
Zum Vergrößern anklicken....

Ja, derzeit läuft das über mysql_num_rows...aber ich blick grad nicht durch wie du es jetzt meinst? Denn im ersten satz sagst du, dass count() besser ist. Dann aber schreibst du, dass ich es so lassen soll wenn es count() nutzt und dann auf einmal dann mysql_num_rows würde ich nur eine Abfrage brauchen statt wie jetzt 2...aber jetzt sind es doch 2 und die laufen über mysql_num_rows
 
PHP: 
<?php
class Sessions{
	private $_sql;
	private $_sessionid;
	
	public function __construct(&$sql){
		$this->_sql = $sql;
		if(isset($_COOKIE['sessionid']))
			$_COOKIE['sessionid'] = mysql_real_escape_string($_COOKIE['sessionid']);
		else
			$_COOKIE['sessionid'] = '';
		$this->_sessionid = $_COOKIE['sessionid'];
		
		$sql->query('DELETE FROM `sessions` WHERE `lastactivity`<'.(time()-7200).' AND `userid`=0'); // Gastsessions löschen
		$sql->query('DELETE FROM `sessions` WHERE `lastactivity`<'.(time()-1200).' AND `locked`=0 AND `userid`!=0'); // Usersessions löschen
	}
	
	public function checkSession(){
		if(empty($this->_sessionid)) // Session existiert nicht -> erstellen
			$this->createSession();
		else{
			$result = $this->_sql->query('SELECT COUNT(*) AS `count` FROM `sessions` WHERE `ip`="'.$_SERVER['REMOTE_ADDR'].'" AND `sessionid`="'.$this->_sessionid.'"');
			$row = mysql_fetch_assoc($result);
			if($row['count'] == 1){ // Session existiert, lastactivity updaten
				$this->_sql->query('UPDATE `sessions` SET `lastactivity`='.time().' WHERE `sessionid`="'.$this->_sessionid.'"'); // ich würde den Timestamp nicht speichern, sondern DATETIME in MySQL verwenden
				setcookie('sessionid', $this->_sessionid, (time()+7200)); //Cookie verlängern
			}
			else{ // Session ungültig -> erstellen
				$this->createSession();
			}
		}
	}
	
	private function createSession(){
		$sessionid = substr(md5(time()*uniqid(mt_rand())), 0, 20); // noch besserere Zufallskombinationen xD
		$this->_sql->query('INSERT INTO `sessions` (`sessionid`, `lastactivity`)  VALUES ("'.$sessionid.'", '.time().')'); //die Spalten User und locked sollten Default Werte haben, ansonsten hier noch ergänzen!
		$this->_sessionid = $sessionid;
		setcookie('sessionid', $sessionid, time()+7200);
	}
	
	public function setLogin($userid, $lock=false){
		if(is_int($userid)){
			$lock = ($lock == true) ? 1 : 0;
			$this->_sql->query('UPDATE `sessions` SET `userid`='.$userid.' AND `lock`='.$lock.' WHERE `sessionid`="'.$this->_sessionid.'"');
		}
		else
			return false;
	}
	
	public function getData(){ //müsstest noch ein Feld in der DB Struktur ändern, willst ja bestimmt auch was speichern oder?
		// müssteste mit JSON speichern, jedoch ist das ziemlich begrenzt, aonsten evt. mit serialize() - hab mich noch nie wirklich mit befasst
		// array zurückgeben
	}
	
	public function writeData($key, $value){
		// alte Elemente auslesen ($this->getData()) und eins hinzufügen, dann wieder aus nem Array nen String machen
		// am besten geänderten Array mit Daten zurückgeben
	}
	
	public function delData($key){
		// Element aus Array löschen, dann updaten
		// am besten geänderten Array mit Daten zurückgeben
	}
	
	public function __close(){
		// hier evt. erst die Änderungen am Session Array speichern, muss dann nur einmal geschehen
		// Array muss dann aber hier in der Klasse intern gespeichert werden
	}
}
?>

Das hier ist erstmal ein grober Ansatz, wie du das ganze mit OOP realisieren könntest. Bei den Funktionen wo es dann ums Daten speichern geht, hatte ich keine Lust mehr, außerdem wusste ich nicht, wie ich das jetzt am besten realisiere (noch nie mit befasst). JSON scheidet aus, weils halt ne begrenzte Menge hat (oder du machst es "von Hand" und nicht mit den PHP Funktionen).
Wenn du allerdings Daten speicherst, hat in meinen Augen das Feld mit der User-ID keinen Sinn, das könnteste dann so bei den Daten abspeichern ;)
 
Du musst dich einloggen oder registrieren, um hier zu antworten.
Zurück
Oben