Session-Handler Entwurf

[vls2k]

Hallo!

Ich arbeite zur Zeit an einem Session-Handler und würde gerne mal zu diesem Entwurf einige Meinungen hören bzgl. Funktionalität / Sicherheit:

<?php
final class session
{
    # Initialisierungsfunktion, muss manuell aufgerufen werden
    static public function loadsessionmanager()
    {
        # Achtung bei session.auto_start!
        ini_set('session.gc_probability', 1);
        ini_set('session.gc_divisor', 50);
        ini_set('session.save_handler', 'user');
        ini_set('session.gc_maxlifetime', 3600 * 24 * 8);
        ini_set('session.cookie_lifetime', 3600 * 24 * 7);

        session_set_save_handler(array("session", "on_session_start"), array("session", "on_session_end"),
            array("session", "on_session_read"), array("session", "on_session_write"),
            array("session", "on_session_destroy"), array("session", "on_session_gc"));
        
        session_start();
        
        session_regenerate_id(false);
        return;
    }

    static public function on_session_start()
    {
        return true;
    }

    static public function on_session_end()
    {
        return true;
    }

    static public function on_session_read($key)
    {
        $res = DB::query('SELECT session_data FROM sessions WHERE session_id = "' . mysql_real_escape_string($key) . '";');
        $obj = mysql_fetch_object($res);
        return ($obj->session_data);
    }

    static public function on_session_write($key, $val)
    {
        # Session Fixation vermeiden, eine Session-ID ist nur eine Woche
                # gültig, dieser Gültigkeitszeitraum wird nicht aktualisiert!
                # Da dann bereits eine neue ID erstellt wurde
        $query = 'INSERT INTO sessions (session_id, session_data, session_time) VALUES ('
                    . '"' .  mysql_real_escape_string($key) . '",'
                    . '"' .  mysql_real_escape_string($val) . '",'
                    . 'UNIX_TIMESTAMP() ) '
                    . 'ON DUPLICATE KEY UPDATE '
                    . 'session_data="' .  mysql_real_escape_string($val) . '";';
        mysql_query($query);
        return true;
    }

    static public function on_session_destroy($key)
    {
        return DB::query('DELETE FROM sessions WHERE session_id="'.$key.'"');
    }

    static public function on_session_gc($max_lifetime)
    {
        DB::query('DELETE FROM sessions WHERE session_time + "' . mysql_real_escape_string($max_lifetime) . '" < UNIX_TIMESTAMP()');
        return true;
    }
}
?>

# SQL-DUMP:
CREATE TABLE `sessions` (
  `session_id` char(32) collate latin1_general_ci NOT NULL,
  `session_data` text collate latin1_general_ci NOT NULL,
  `session_time` int(10) NOT NULL default '0',
  PRIMARY KEY  (`session_id`),
  KEY `t_stamp` (`session_time`)
) ENGINE=MyISAM DEFAULT CHARSET=latin1 COLLATE=latin1_general_ci;

Danke!

[PHP-Desaster]

1. Das on_session_ bei den Funktionsnamen weg!
2. Eine Instanz anstatt statischer Methoden
3. Ini-Werte nicht überschreiben, oder zumindest nicht statisch!

Was mir so spontan dazu einfällt!

[Abraxax]

1. umbrüche in den code einbinden!
2. statt INSERT ... ON DUPLICATE ... lieber auf REPLACE zurück greifen.

[Griecherus]

Ich würde der Klasse noch die Möglichkeit geben, ein Konfigurations-Array entgegen zu nehmen, damit du Änderungen an den relevanten Session Ini-Einträgen komfortabel vornehmen kannst, statt die Klasse editieren zu müssen.

Z.B.

class SessionSaveHandler
{
    private $_options = array(
        'gc_probability' => null,
        'gc_divisor'     => null,
        'gc_maxlifetime' => null,
        // ...
    );

    public function __construct(array $options = array())
    {
        foreach ($options as $option => $value)
        {
            if (array_key_exists($option, $this->_options))
            {
                ini_set('session_' . $option, $value);
            }
        }
    }
} 

Nur mal als kleine Anregung.

[bla$ter]

Hmm, sieht mit nicht besonders race safe aus, ich würde dringend den Einsatz von "SELECT ... FOR UPDATE" oder "SELECT GET_LOCK" empfehlen, um Dateninkonsistenz zu vermeiden.

[vls2k]

Original geschrieben von Abraxax
1. umbrüche in den code einbinden!
2. statt INSERT ... ON DUPLICATE ... lieber auf REPLACE zurück greifen.

zu 1.: done ;-)
zu 2.: dann ist der effekt aber weg, dass eine session sich nicht mehr verlängern lässt... bei REPLACE würde ja auch die time neu gesetzt werden...

Original geschrieben von bla$ter
Hmm, sieht mit nicht besonders race safe aus, ich würde dringend den Einsatz von "SELECT ... FOR UPDATE" oder "SELECT GET_LOCK" empfehlen, um Dateninkonsistenz zu vermeiden.

An welcher Stelle denn? Die Write-Funktion wird ja erst am Ende des Skriptes aufgerufen, so lange kann ich doch nicht alle anderen Prozesse warten lassen?

[Abraxax]

Original geschrieben von vls2k
zu 1.: done ;-)
zu 2.: dann ist der effekt aber weg, dass eine session sich nicht mehr verlängern lässt... bei REPLACE würde ja auch die time neu gesetzt werden...

1. da sind aber noch mindestens 3 zeilen recht lang ... aber schon deutlich besser als vorher ...

2. du willst doch bestimmt eh eine anzeige ala letzte aktivität haben. oder?

und das db-feld würde ich auf TIMESTAMP definieren und mich bei der query nicht mehr drum kümmern.

[vls2k]

Original geschrieben von Abraxax
1. da sind aber noch mindestens 3 zeilen recht lang ... aber schon deutlich besser als vorher ...

2. du willst doch bestimmt eh eine anzeige ala letzte aktivität haben. oder?

und das db-feld würde ich auf TIMESTAMP definieren und mich bei der query nicht mehr drum kümmern.

zu 2. Nein, der Session-Handler ist für ein Communityähnliches System... Da z.B. ein mit PHPSESSID geposteter Link (bei deaktivierten Cookies) die Übernahme der Session ermöglichen würde, suche ich nach möglichkeiten, den Schaden zu begrenzen, indem nach Session-ID-Wechsel bei einem Aufruf die alte ID icht mehr refreshbar ist. Was gibt es noch für weitere möglichkeiten? (außer IP) Nur auf Cookies setzen?

[bla$ter]

Original geschrieben von vls2k

An welcher Stelle denn? Die Write-Funktion wird ja erst am Ende des Skriptes aufgerufen, so lange kann ich doch nicht alle anderen Prozesse warten lassen?

Aus diesem Grund sollst du ja row level locks verwenden, die beeinträchtigen andere Prozesse auf anderen Sessions nicht im geringsten. PHP macht das nicht anders, der default session handler legt ein exlock auf das session file und behält das so lange bei, bis das Script beendet wird.