Skip to content

Commit b6b149a

Browse files
committed
wall of text
1 parent a3fdd78 commit b6b149a

File tree

2 files changed

+73
-15
lines changed

2 files changed

+73
-15
lines changed

README.md

Lines changed: 71 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ session_set_save_handler(new \UMA\RedisSessionHandler(), true);
2323
```
2424

2525
Note that calling `session_set_save_handler` overwrites any value you might have set in the `session.save_handler` option
26-
of the php.ini file, so you don't need to change it. However, RedisSessionHandler still uses `session.save_path` to find
27-
the Redis server, just like the phpredis handle.
26+
of the php.ini file, so you don't need to change that. However, RedisSessionHandler still uses `session.save_path` to find
27+
the Redis server, just like the vanilla phpredis session handler.
2828

2929
```ini
3030
session.save_path = "tcp://1.2.3.4:6379"
@@ -33,16 +33,26 @@ session.save_path = "tcp://1.2.3.4:6379"
3333

3434
## Motivation
3535

36-
For years, the C session handler bundled with [phpredis](https://github.com/phpredis/phpredis) has been suffering
37-
from a couple of bugs that I care about, namely the [lack of per-session locking](https://github.com/phpredis/phpredis/issues/37) and the [impossibility to run it in "strict" mode](https://github.com/phpredis/phpredis/issues/37).
36+
The Redis session handler bundled with [phpredis](https://github.com/phpredis/phpredis) has had a couple of rather serious
37+
bugs for years, namely the [lack of per-session locking](https://github.com/phpredis/phpredis/issues/37) and the [impossibility to protect against session fixation attacks](https://github.com/phpredis/phpredis/issues/37).
38+
39+
This package provides a session handler built on top of the Redis extension that is not affected by these issues.
3840

3941

4042
### Session Locking explained
4143

44+
In the context of PHP, "session locking" means that when multiple requests with the same session ID hit the server roughly
45+
at the same time, only one gets to run while the others get stuck waiting inside `session_start()`. Only when that first request
46+
calls [`session_write_close()`](http://php.net/manual/en/function.session-write-close.php), one of the others can move on.
47+
48+
When a session handler does not implement session locking concurrency bugs might start to surface under
49+
heavy traffic. I'll demonstrate the problem using the default phpredis handler and this simple script:
50+
4251
```
4352
<?php
4453
45-
// test-project/web/visit-counter.php
54+
// a script that returns the total number of
55+
// requests made during a given session's lifecycle.
4656
4757
session_start();
4858
@@ -55,7 +65,10 @@ $_SESSION['visits']++;
5565
echo $_SESSION['visits'];
5666
```
5767

58-
```
68+
First, we send a single request that will setup a new session. Then we use the session ID returned in
69+
the `Set-Cookie` header to send a burst of 200 concurrent, authenticated requests.
70+
71+
```bash
5972
1ma@werkbox:~$ http localhost/visit-counter.php
6073
HTTP/1.1 200 OK
6174
Cache-Control: no-store, no-cache, must-revalidate
@@ -70,7 +83,7 @@ Transfer-Encoding: chunked
7083

7184
1
7285

73-
1ma@werkbox:~$ hey -H "Cookie: PHPSESSID=9mcjmlsh9gp0conq7i5rci7is8gfn6s0gh8r3eub3qpac09gnh21;" http://localhost/visit-counter.php
86+
1ma@werkbox:~$ hey -n 200 -H "Cookie: PHPSESSID=9mcjmlsh9gp0conq7i5rci7is8gfn6s0gh8r3eub3qpac09gnh21;" http://localhost/visit-counter.php
7487
All requests done.
7588

7689
Summary:
@@ -84,7 +97,8 @@ Status code distribution:
8497
[200] 200 responses
8598
```
8699

87-
BUT:
100+
Everything looks fine from the outside, we got the expected two hundred OK responses, but if we peek inside the Redis
101+
database, we see that the counter is way off. Instead of 201 visits we see a random number that is way lower than that:
88102

89103
```
90104
127.0.0.1:6379> KEYS *
@@ -94,6 +108,11 @@ BUT:
94108
"visits|i:134;"
95109
```
96110

111+
Looking at Redis' `MONITOR` output we can see that under heavy load, Redis often executes two or more `GET` commands
112+
one after the other, thus returning the same number of visits to two or more different requests. When that happens, all
113+
those unlucky requests pass the same number of visits back to Redis, so some of them are ultimately lost. For instance, in this excerpt
114+
you can see how the 130th request is not accounted for.
115+
97116
```
98117
1485174643.241711 [0 172.21.0.2:49780] "GET" "PHPREDIS_SESSION:9mcjmlsh9gp0conq7i5rci7is8gfn6s0gh8r3eub3qpac09gnh21"
99118
1485174643.241891 [0 172.21.0.2:49782] "GET" "PHPREDIS_SESSION:9mcjmlsh9gp0conq7i5rci7is8gfn6s0gh8r3eub3qpac09gnh21"
@@ -103,10 +122,20 @@ BUT:
103122
1485174643.245385 [0 172.21.0.2:49784] "SETEX" "PHPREDIS_SESSION:9mcjmlsh9gp0conq7i5rci7is8gfn6s0gh8r3eub3qpac09gnh21" "900" "visits|i:130;"
104123
```
105124

125+
RedisSessionHandler solves this problem with a "lock" entry for every session that only one thread of execution can create at a time.
106126

107-
### Session Strict Mode explained
108127

109-
```
128+
### Session fixation explained
129+
130+
[Session fixation](https://www.owasp.org/index.php/Session_fixation) is the ability to choose your own session ID as an HTTP client. When clients are allowed to choose their
131+
session IDs, a malicious attacker might be able to trick other users into using an ID already known to him, then let them log in and hijack their session.
132+
133+
Starting from PHP 5.5.2, there's an INI directive called [`session.use_strict_mode`](http://php.net/manual/en/session.configuration.php#ini.session.use-strict-mode) to protect
134+
PHP applications against such attacks. When "strict mode" is enabled and a random session ID is received, PHP should ignore it and generate a new
135+
one, just as if it was not there at all. Unfortunately the phpredis handler ignores that directive and always trust whatever session ID is received from
136+
the HTTP request.
137+
138+
```bash
110139
1ma@werkbox:~$ http -v http://localhost/visit-counter.php Cookie:PHPSESSID=madeupkey
111140
GET / HTTP/1.1
112141
Accept: */*
@@ -125,15 +154,44 @@ Host: 127.0.0.1
125154
Pragma: no-cache
126155

127156
1
128-
```
129157

130-
```
158+
1ma@werkbox:~$ redis-cli
159+
131160
127.0.0.1:6379> keys *
132161
1) "PHPREDIS_SESSION:madeupkey"
133162

134-
127.0.0.1:6379> GET PHPREDIS_SESSION:9mcjmlsh9gp0conq7i5rci7is8gfn6s0gh8r3eub3qpac09gnh21
163+
127.0.0.1:6379> GET PHPREDIS_SESSION:madeupkey
135164
"visits|i:1;"
136165
```
137166

167+
RedisSessionHandler also ignores the `session.use_strict_mode` directive but to do the opposite, i.e. make the above behaviour impossible.
168+
138169

139170
## Running the tests
171+
172+
To run the tests you need Docker >=1.10 and docker-compose >=1.8. The next steps are spinning up the testing
173+
environment, downloading the dev dependencies and launching the test sutie.
174+
175+
```bash
176+
1ma@werkbox:~/RedisSessionHandler$ docker-compose -f tests/docker-compose.yml up -d
177+
Creating network "tests_default" with the default driver
178+
Creating tests_fpm_1
179+
Creating tests_redis_1
180+
Creating tests_nginx_1
181+
Creating tests_testrunner_1
182+
183+
1ma@werkbox:~/RedisSessionHandler$ docker exec -it tests_testrunner_1 composer install
184+
Loading composer repositories with package information
185+
Installing dependencies (including require-dev) from lock file
186+
Nothing to install or update
187+
Generating autoload files
188+
189+
1ma@werkbox:~/RedisSessionHandler$ docker exec -it tests_testrunner_1 php vendor/bin/phpunit
190+
PHPUnit 4.8.31 by Sebastian Bergmann and contributors.
191+
192+
.......
193+
194+
Time: 22.74 seconds, Memory: 8.00MB
195+
196+
OK (7 tests, 27 assertions)
197+
```

tests/e2e/EndToEndTestCase.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ class EndToEndTestCase extends \PHPUnit_Framework_TestCase
1818
protected $redis;
1919

2020
/**
21-
* e2e test cases start creating a connection against the
22-
* web server container and flushing the Redis database.
21+
* e2e test cases start creating an HTTP client pointing
22+
* to the testing web server and flushing the Redis database.
2323
*/
2424
public function setUp()
2525
{

0 commit comments

Comments
 (0)