Skip to content

Commit 3145c60

Browse files
JensJItaylorotwell
JensJI
and
taylorotwell
authored
[8.x] Patch for timeless timing attack vulnerability in user login (#44069)
* Timebox * Fixed formatting * Increased delta time for tests in SupportHelpersTests.php that uses usleep as part of the test. This is necessary because usleep on Windows is unreliable, and other tests that uses CPU (for instance by using usleep) that run simultaneously are then affecting the tests in SupportHelpersTests.php that asserts based on the used time. * Timebox: ensured timebox property on SessionGuard would not be null. Added config option for validateCredentialsMinimumTime per guard * formatting * remove docblock * remove unnecessary code * fix tests Co-authored-by: Taylor Otwell <[email protected]>
1 parent 762b917 commit 3145c60

File tree

6 files changed

+198
-20
lines changed

6 files changed

+198
-20
lines changed

‎src/Illuminate/Auth/AuthManager.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,11 @@ public function createSessionDriver($name, $config)
122122
{
123123
$provider = $this->createUserProvider($config['provider'] ?? null);
124124

125-
$guard = new SessionGuard($name, $provider, $this->app['session.store']);
125+
$guard = new SessionGuard(
126+
$name,
127+
$provider,
128+
$this->app['session.store'],
129+
);
126130

127131
// When using the remember me functionality of the authentication services we
128132
// will need to be set the encryption instance of the guard, which allows

‎src/Illuminate/Auth/SessionGuard.php‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Illuminate\Support\Arr;
2121
use Illuminate\Support\Facades\Hash;
2222
use Illuminate\Support\Str;
23+
use Illuminate\Support\Timebox;
2324
use Illuminate\Support\Traits\Macroable;
2425
use InvalidArgumentException;
2526
use RuntimeException;
@@ -88,6 +89,13 @@ class SessionGuard implements StatefulGuard, SupportsBasicAuth
8889
*/
8990
protected $events;
9091

92+
/**
93+
* The timebox instance.
94+
*
95+
* @var \Illuminate\Support\Timebox
96+
*/
97+
protected $timebox;
98+
9199
/**
92100
* Indicates if the logout method has been called.
93101
*
@@ -109,17 +117,20 @@ class SessionGuard implements StatefulGuard, SupportsBasicAuth
109117
* @param \Illuminate\Contracts\Auth\UserProvider $provider
110118
* @param \Illuminate\Contracts\Session\Session $session
111119
* @param \Symfony\Component\HttpFoundation\Request|null $request
120+
* @param \Illuminate\Support\Timebox|null $timebox
112121
* @return void
113122
*/
114123
public function __construct($name,
115124
UserProvider $provider,
116125
Session $session,
117-
Request $request = null)
126+
Request $request = null,
127+
Timebox $timebox = null)
118128
{
119129
$this->name = $name;
120130
$this->session = $session;
121131
$this->request = $request;
122132
$this->provider = $provider;
133+
$this->timebox = $timebox ?: new Timebox;
123134
}
124135

125136
/**
@@ -419,13 +430,17 @@ public function attemptWhen(array $credentials = [], $callbacks = null, $remembe
419430
*/
420431
protected function hasValidCredentials($user, $credentials)
421432
{
422-
$validated = ! is_null($user) && $this->provider->validateCredentials($user, $credentials);
433+
return $this->timebox->call(function ($timebox) use ($user, $credentials) {
434+
$validated = ! is_null($user) && $this->provider->validateCredentials($user, $credentials);
423435

424-
if ($validated) {
425-
$this->fireValidatedEvent($user);
426-
}
436+
if ($validated) {
437+
$timebox->returnEarly();
438+
439+
$this->fireValidatedEvent($user);
440+
}
427441

428-
return $validated;
442+
return $validated;
443+
}, 200 * 1000);
429444
}
430445

431446
/**
@@ -953,4 +968,14 @@ public function setRequest(Request $request)
953968

954969
return $this;
955970
}
971+
972+
/**
973+
* Get the timebox instance used by the guard.
974+
*
975+
* @return \Illuminate\Support\Timebox
976+
*/
977+
public function getTimebox()
978+
{
979+
return $this->timebox;
980+
}
956981
}

‎src/Illuminate/Support/Timebox.php‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<?php
2+
3+
namespace Illuminate\Support;
4+
5+
class Timebox
6+
{
7+
/**
8+
* Indicates if the timebox is allowed to return early.
9+
*
10+
* @var bool
11+
*/
12+
public $earlyReturn = false;
13+
14+
/**
15+
* Invoke the given callback within the specified timebox minimum.
16+
*
17+
* @param callable $callback
18+
* @param int $microseconds
19+
* @return mixed
20+
*/
21+
public function call(callable $callback, int $microseconds)
22+
{
23+
$start = microtime(true);
24+
25+
$result = $callback($this);
26+
27+
$remainder = $microseconds - ((microtime(true) - $start) * 1000000);
28+
29+
if (! $this->earlyReturn && $remainder > 0) {
30+
$this->usleep($remainder);
31+
}
32+
33+
return $result;
34+
}
35+
36+
/**
37+
* Indicate that the timebox can return early.
38+
*
39+
* @return $this
40+
*/
41+
public function returnEarly()
42+
{
43+
$this->earlyReturn = true;
44+
45+
return $this;
46+
}
47+
48+
/**
49+
* Indicate that the timebox cannot return early.
50+
*
51+
* @return $this
52+
*/
53+
public function dontReturnEarly()
54+
{
55+
$this->earlyReturn = false;
56+
57+
return $this;
58+
}
59+
60+
/**
61+
* Sleep for the specified number of microseconds.
62+
*
63+
* @param $microseconds
64+
* @return void
65+
*/
66+
protected function usleep($microseconds)
67+
{
68+
usleep($microseconds);
69+
}
70+
}

‎tests/Auth/AuthGuardTest.php‎

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Illuminate\Contracts\Events\Dispatcher;
1717
use Illuminate\Contracts\Session\Session;
1818
use Illuminate\Cookie\CookieJar;
19+
use Illuminate\Support\Timebox;
1920
use Mockery as m;
2021
use PHPUnit\Framework\TestCase;
2122
use Symfony\Component\HttpFoundation\Cookie;
@@ -94,6 +95,10 @@ public function testAttemptCallsRetrieveByCredentials()
9495
{
9596
$guard = $this->getGuard();
9697
$guard->setDispatcher($events = m::mock(Dispatcher::class));
98+
$timebox = $guard->getTimebox();
99+
$timebox->shouldReceive('call')->once()->andReturnUsing(function ($callback) use ($timebox) {
100+
return $callback($timebox);
101+
});
97102
$events->shouldReceive('dispatch')->once()->with(m::type(Attempting::class));
98103
$events->shouldReceive('dispatch')->once()->with(m::type(Failed::class));
99104
$events->shouldNotReceive('dispatch')->with(m::type(Validated::class));
@@ -103,9 +108,12 @@ public function testAttemptCallsRetrieveByCredentials()
103108

104109
public function testAttemptReturnsUserInterface()
105110
{
106-
[$session, $provider, $request, $cookie] = $this->getMocks();
107-
$guard = $this->getMockBuilder(SessionGuard::class)->onlyMethods(['login'])->setConstructorArgs(['default', $provider, $session, $request])->getMock();
111+
[$session, $provider, $request, $cookie, $timebox] = $this->getMocks();
112+
$guard = $this->getMockBuilder(SessionGuard::class)->onlyMethods(['login'])->setConstructorArgs(['default', $provider, $session, $request, $timebox])->getMock();
108113
$guard->setDispatcher($events = m::mock(Dispatcher::class));
114+
$timebox->shouldReceive('call')->once()->andReturnUsing(function ($callback, $microseconds) use ($timebox) {
115+
return $callback($timebox->shouldReceive('returnEarly')->once()->getMock());
116+
});
109117
$events->shouldReceive('dispatch')->once()->with(m::type(Attempting::class));
110118
$events->shouldReceive('dispatch')->once()->with(m::type(Validated::class));
111119
$user = $this->createMock(Authenticatable::class);
@@ -119,6 +127,10 @@ public function testAttemptReturnsFalseIfUserNotGiven()
119127
{
120128
$mock = $this->getGuard();
121129
$mock->setDispatcher($events = m::mock(Dispatcher::class));
130+
$timebox = $mock->getTimebox();
131+
$timebox->shouldReceive('call')->once()->andReturnUsing(function ($callback, $microseconds) use ($timebox) {
132+
return $callback($timebox);
133+
});
122134
$events->shouldReceive('dispatch')->once()->with(m::type(Attempting::class));
123135
$events->shouldReceive('dispatch')->once()->with(m::type(Failed::class));
124136
$events->shouldNotReceive('dispatch')->with(m::type(Validated::class));
@@ -128,9 +140,12 @@ public function testAttemptReturnsFalseIfUserNotGiven()
128140

129141
public function testAttemptAndWithCallbacks()
130142
{
131-
[$session, $provider, $request, $cookie] = $this->getMocks();
132-
$mock = $this->getMockBuilder(SessionGuard::class)->onlyMethods(['getName'])->setConstructorArgs(['default', $provider, $session, $request])->getMock();
143+
[$session, $provider, $request, $cookie, $timebox] = $this->getMocks();
144+
$mock = $this->getMockBuilder(SessionGuard::class)->onlyMethods(['getName'])->setConstructorArgs(['default', $provider, $session, $request, $timebox])->getMock();
133145
$mock->setDispatcher($events = m::mock(Dispatcher::class));
146+
$timebox->shouldReceive('call')->andReturnUsing(function ($callback) use ($timebox) {
147+
return $callback($timebox->shouldReceive('returnEarly')->getMock());
148+
});
134149
$user = m::mock(Authenticatable::class);
135150
$events->shouldReceive('dispatch')->times(3)->with(m::type(Attempting::class));
136151
$events->shouldReceive('dispatch')->once()->with(m::type(Login::class));
@@ -212,6 +227,10 @@ public function testFailedAttemptFiresFailedEvent()
212227
{
213228
$guard = $this->getGuard();
214229
$guard->setDispatcher($events = m::mock(Dispatcher::class));
230+
$timebox = $guard->getTimebox();
231+
$timebox->shouldReceive('call')->once()->andReturnUsing(function ($callback, $microseconds) use ($timebox) {
232+
return $callback($timebox);
233+
});
215234
$events->shouldReceive('dispatch')->once()->with(m::type(Attempting::class));
216235
$events->shouldReceive('dispatch')->once()->with(m::type(Failed::class));
217236
$events->shouldNotReceive('dispatch')->with(m::type(Validated::class));
@@ -544,9 +563,12 @@ public function testUserUsesRememberCookieIfItExists()
544563

545564
public function testLoginOnceSetsUser()
546565
{
547-
[$session, $provider, $request, $cookie] = $this->getMocks();
548-
$guard = m::mock(SessionGuard::class, ['default', $provider, $session])->makePartial();
566+
[$session, $provider, $request, $cookie, $timebox] = $this->getMocks();
567+
$guard = m::mock(SessionGuard::class, ['default', $provider, $session, $request, $timebox])->makePartial();
549568
$user = m::mock(Authenticatable::class);
569+
$timebox->shouldReceive('call')->once()->andReturnUsing(function ($callback) use ($timebox) {
570+
return $callback($timebox->shouldReceive('returnEarly')->once()->getMock());
571+
});
550572
$guard->getProvider()->shouldReceive('retrieveByCredentials')->once()->with(['foo'])->andReturn($user);
551573
$guard->getProvider()->shouldReceive('validateCredentials')->once()->with($user, ['foo'])->andReturn(true);
552574
$guard->shouldReceive('setUser')->once()->with($user);
@@ -555,19 +577,22 @@ public function testLoginOnceSetsUser()
555577

556578
public function testLoginOnceFailure()
557579
{
558-
[$session, $provider, $request, $cookie] = $this->getMocks();
559-
$guard = m::mock(SessionGuard::class, ['default', $provider, $session])->makePartial();
580+
[$session, $provider, $request, $cookie, $timebox] = $this->getMocks();
581+
$guard = m::mock(SessionGuard::class, ['default', $provider, $session, $request, $timebox])->makePartial();
560582
$user = m::mock(Authenticatable::class);
583+
$timebox->shouldReceive('call')->once()->andReturnUsing(function ($callback) use ($timebox) {
584+
return $callback($timebox);
585+
});
561586
$guard->getProvider()->shouldReceive('retrieveByCredentials')->once()->with(['foo'])->andReturn($user);
562587
$guard->getProvider()->shouldReceive('validateCredentials')->once()->with($user, ['foo'])->andReturn(false);
563588
$this->assertFalse($guard->once(['foo']));
564589
}
565590

566591
protected function getGuard()
567592
{
568-
[$session, $provider, $request, $cookie] = $this->getMocks();
593+
[$session, $provider, $request, $cookie, $timebox] = $this->getMocks();
569594

570-
return new SessionGuard('default', $provider, $session, $request);
595+
return new SessionGuard('default', $provider, $session, $request, $timebox);
571596
}
572597

573598
protected function getMocks()
@@ -577,6 +602,7 @@ protected function getMocks()
577602
m::mock(UserProvider::class),
578603
Request::create('/', 'GET'),
579604
m::mock(CookieJar::class),
605+
m::mock(Timebox::class),
580606
];
581607
}
582608

‎tests/Support/SupportHelpersTest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -552,7 +552,7 @@ public function testRetry()
552552
$this->assertEquals(2, $attempts);
553553

554554
// Make sure we waited 100ms for the first attempt
555-
$this->assertEqualsWithDelta(0.1, microtime(true) - $startTime, 0.02);
555+
$this->assertEqualsWithDelta(0.1, microtime(true) - $startTime, 0.03);
556556
}
557557

558558
public function testRetryWithPassingSleepCallback()
@@ -573,7 +573,7 @@ public function testRetryWithPassingSleepCallback()
573573
$this->assertEquals(3, $attempts);
574574

575575
// Make sure we waited 300ms for the first two attempts
576-
$this->assertEqualsWithDelta(0.3, microtime(true) - $startTime, 0.02);
576+
$this->assertEqualsWithDelta(0.3, microtime(true) - $startTime, 0.03);
577577
}
578578

579579
public function testRetryWithPassingWhenCallback()
@@ -594,7 +594,7 @@ public function testRetryWithPassingWhenCallback()
594594
$this->assertEquals(2, $attempts);
595595

596596
// Make sure we waited 100ms for the first attempt
597-
$this->assertEqualsWithDelta(0.1, microtime(true) - $startTime, 0.02);
597+
$this->assertEqualsWithDelta(0.1, microtime(true) - $startTime, 0.03);
598598
}
599599

600600
public function testRetryWithFailingWhenCallback()

‎tests/Support/SupportTimeboxTest.php‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<?php
2+
3+
namespace Illuminate\Tests\Support;
4+
5+
use Illuminate\Support\Timebox;
6+
use Mockery as m;
7+
use PHPUnit\Framework\TestCase;
8+
9+
class SupportTimeboxTest extends TestCase
10+
{
11+
public function testMakeExecutesCallback()
12+
{
13+
$callback = function () {
14+
$this->assertTrue(true);
15+
};
16+
17+
(new Timebox)->call($callback, 0);
18+
}
19+
20+
public function testMakeWaitsForMicroseconds()
21+
{
22+
$mock = m::spy(Timebox::class)->shouldAllowMockingProtectedMethods()->makePartial();
23+
$mock->shouldReceive('usleep')->once();
24+
25+
$mock->call(function () {
26+
}, 10000);
27+
28+
$mock->shouldHaveReceived('usleep')->once();
29+
}
30+
31+
public function testMakeShouldNotSleepWhenEarlyReturnHasBeenFlagged()
32+
{
33+
$mock = m::spy(Timebox::class)->shouldAllowMockingProtectedMethods()->makePartial();
34+
$mock->call(function ($timebox) {
35+
$timebox->returnEarly();
36+
}, 10000);
37+
38+
$mock->shouldNotHaveReceived('usleep');
39+
}
40+
41+
public function testMakeShouldSleepWhenDontEarlyReturnHasBeenFlagged()
42+
{
43+
$mock = m::spy(Timebox::class)->shouldAllowMockingProtectedMethods()->makePartial();
44+
$mock->shouldReceive('usleep')->once();
45+
46+
$mock->call(function ($timebox) {
47+
$timebox->returnEarly();
48+
$timebox->dontReturnEarly();
49+
}, 10000);
50+
51+
$mock->shouldHaveReceived('usleep')->once();
52+
}
53+
}

0 commit comments

Comments
 (0)