Skip to content

Commit a8c4d6a

Browse files
DaneEverittDaneEveritt
committed
Update random ID method to use str_random and not random_bytes
The use of random_bytes in combination with bin2hex was producing a lot of duplicate keys when tested in batches of 10k (anywhere from 2 to 6). The use of str_random yielded no duplicates even at scales of 100k keys that were 8 characters.
1 parent 0e518be commit a8c4d6a

14 files changed

+50
-59
lines changed

‎app/Services/Api/KeyCreationService.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@
3030

3131
class KeyCreationService
3232
{
33-
const PUB_CRYPTO_BYTES = 8;
34-
const PRIV_CRYPTO_BYTES = 32;
33+
const PUB_CRYPTO_LENGTH = 16;
34+
const PRIV_CRYPTO_LENGTH = 64;
3535

3636
/**
3737
* @var \Illuminate\Database\ConnectionInterface
@@ -86,8 +86,8 @@ public function __construct(
8686
*/
8787
public function handle(array $data, array $permissions, array $administrative = [])
8888
{
89-
$publicKey = bin2hex(random_bytes(self::PUB_CRYPTO_BYTES));
90-
$secretKey = bin2hex(random_bytes(self::PRIV_CRYPTO_BYTES));
89+
$publicKey = str_random(self::PUB_CRYPTO_LENGTH);
90+
$secretKey = str_random(self::PRIV_CRYPTO_LENGTH);
9191

9292
// Start a Transaction
9393
$this->connection->beginTransaction();

‎app/Services/Nodes/NodeCreationService.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828

2929
class NodeCreationService
3030
{
31-
const DAEMON_SECRET_LENGTH = 18;
31+
const DAEMON_SECRET_LENGTH = 36;
3232

3333
/**
3434
* @var \Pterodactyl\Contracts\Repository\NodeRepositoryInterface
@@ -55,7 +55,7 @@ public function __construct(NodeRepositoryInterface $repository)
5555
*/
5656
public function handle(array $data)
5757
{
58-
$data['daemonSecret'] = bin2hex(random_bytes(self::DAEMON_SECRET_LENGTH));
58+
$data['daemonSecret'] = str_random(self::DAEMON_SECRET_LENGTH);
5959

6060
return $this->repository->create($data);
6161
}

‎app/Services/Nodes/NodeUpdateService.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public function handle($node, array $data)
8383
}
8484

8585
if (! is_null(array_get($data, 'reset_secret'))) {
86-
$data['daemonSecret'] = bin2hex(random_bytes(NodeCreationService::DAEMON_SECRET_LENGTH));
86+
$data['daemonSecret'] = str_random(NodeCreationService::DAEMON_SECRET_LENGTH);
8787
unset($data['reset_secret']);
8888
}
8989

‎app/Services/Servers/DetailsModificationService.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
use Illuminate\Database\DatabaseManager;
3030
use GuzzleHttp\Exception\RequestException;
3131
use Pterodactyl\Exceptions\DisplayException;
32+
use Pterodactyl\Services\Nodes\NodeCreationService;
3233
use Pterodactyl\Repositories\Eloquent\ServerRepository;
3334
use Pterodactyl\Repositories\Daemon\ServerRepository as DaemonServerRepository;
3435

@@ -83,6 +84,7 @@ public function __construct(
8384
*
8485
* @throws \Pterodactyl\Exceptions\DisplayException
8586
* @throws \Pterodactyl\Exceptions\Model\DataValidationException
87+
* @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException
8688
*/
8789
public function edit($server, array $data)
8890
{
@@ -97,7 +99,7 @@ public function edit($server, array $data)
9799
(isset($data['reset_token']) && ! is_null($data['reset_token'])) ||
98100
(isset($data['owner_id']) && $data['owner_id'] != $server->owner_id)
99101
) {
100-
$data['daemonSecret'] = bin2hex(random_bytes(18));
102+
$data['daemonSecret'] = str_random(NodeCreationService::DAEMON_SECRET_LENGTH);
101103
$shouldUpdate = true;
102104
}
103105

‎app/Services/Servers/ServerCreationService.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
use Illuminate\Database\DatabaseManager;
3030
use GuzzleHttp\Exception\RequestException;
3131
use Pterodactyl\Exceptions\DisplayException;
32+
use Pterodactyl\Services\Nodes\NodeCreationService;
3233
use Pterodactyl\Contracts\Repository\NodeRepositoryInterface;
3334
use Pterodactyl\Contracts\Repository\UserRepositoryInterface;
3435
use Pterodactyl\Contracts\Repository\ServerRepositoryInterface;
@@ -134,12 +135,13 @@ public function __construct(
134135
*
135136
* @throws \Pterodactyl\Exceptions\DisplayException
136137
* @throws \Pterodactyl\Exceptions\Model\DataValidationException
138+
* @throws \Pterodactyl\Exceptions\Repository\RecordNotFoundException
137139
*/
138140
public function create(array $data)
139141
{
140142
// @todo auto-deployment
141143
$validator = $this->validatorService->isAdmin()->setFields($data['environment'])->validate($data['option_id']);
142-
$uniqueShort = bin2hex(random_bytes(4));
144+
$uniqueShort = str_random(8);
143145

144146
$this->database->beginTransaction();
145147

@@ -163,7 +165,7 @@ public function create(array $data)
163165
'option_id' => $data['option_id'],
164166
'pack_id' => (! isset($data['pack_id']) || $data['pack_id'] == 0) ? null : $data['pack_id'],
165167
'startup' => $data['startup'],
166-
'daemonSecret' => bin2hex(random_bytes(18)),
168+
'daemonSecret' => str_random(NodeCreationService::DAEMON_SECRET_LENGTH),
167169
'image' => $data['docker_image'],
168170
'username' => $this->usernameService->generate($data['name'], $uniqueShort),
169171
'sftp_password' => null,

‎app/Services/Servers/UsernameGenerationService.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class UsernameGenerationService
3737
public function generate($name, $identifier = null)
3838
{
3939
if (is_null($identifier) || ! ctype_alnum($identifier)) {
40-
$unique = bin2hex(random_bytes(4));
40+
$unique = str_random(8);
4141
} else {
4242
if (strlen($identifier) < 8) {
4343
$unique = $identifier . str_random((8 - strlen($identifier)));

‎app/Services/Subusers/SubuserCreationService.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
use GuzzleHttp\Exception\RequestException;
3030
use Illuminate\Database\ConnectionInterface;
3131
use Pterodactyl\Exceptions\DisplayException;
32+
use Pterodactyl\Services\Nodes\NodeCreationService;
3233
use Pterodactyl\Services\Users\UserCreationService;
3334
use Pterodactyl\Contracts\Repository\UserRepositoryInterface;
3435
use Pterodactyl\Exceptions\Repository\RecordNotFoundException;
@@ -40,8 +41,6 @@
4041

4142
class SubuserCreationService
4243
{
43-
const DAEMON_SECRET_BYTES = 18;
44-
4544
/**
4645
* @var \Illuminate\Database\ConnectionInterface
4746
*/
@@ -158,7 +157,7 @@ public function handle($server, $email, array $permissions)
158157
$subuser = $this->subuserRepository->create([
159158
'user_id' => $user->id,
160159
'server_id' => $server->id,
161-
'daemonSecret' => bin2hex(random_bytes(self::DAEMON_SECRET_BYTES)),
160+
'daemonSecret' => str_random(NodeCreationService::DAEMON_SECRET_LENGTH),
162161
]);
163162

164163
$daemonPermissions = $this->permissionService->handle($subuser->id, $permissions);

‎tests/Unit/Services/Api/KeyCreationServiceTest.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,15 +84,15 @@ public function setUp()
8484
*/
8585
public function testKeyIsCreated()
8686
{
87-
$this->getFunctionMock('\\Pterodactyl\\Services\\Api', 'bin2hex')
88-
->expects($this->exactly(2))->willReturn('bin2hex');
87+
$this->getFunctionMock('\\Pterodactyl\\Services\\Api', 'str_random')
88+
->expects($this->exactly(2))->willReturn('random_string');
8989

9090
$this->connection->shouldReceive('beginTransaction')->withNoArgs()->once()->andReturnNull();
91-
$this->encrypter->shouldReceive('encrypt')->with('bin2hex')->once()->andReturn('encrypted-secret');
91+
$this->encrypter->shouldReceive('encrypt')->with('random_string')->once()->andReturn('encrypted-secret');
9292

9393
$this->repository->shouldReceive('create')->with([
9494
'test-data' => 'test',
95-
'public' => 'bin2hex',
95+
'public' => 'random_string',
9696
'secret' => 'encrypted-secret',
9797
], true, true)->once()->andReturn((object) ['id' => 1]);
9898

@@ -113,6 +113,6 @@ public function testKeyIsCreated()
113113
);
114114

115115
$this->assertNotEmpty($response);
116-
$this->assertEquals('bin2hex', $response);
116+
$this->assertEquals('random_string', $response);
117117
}
118118
}

‎tests/Unit/Services/Nodes/NodeCreationServiceTest.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,12 @@ public function setUp()
6161
*/
6262
public function testNodeIsCreatedAndDaemonSecretIsGenerated()
6363
{
64-
$this->getFunctionMock('\\Pterodactyl\\Services\\Nodes', 'bin2hex')
65-
->expects($this->once())->willReturn('hexResult');
64+
$this->getFunctionMock('\\Pterodactyl\\Services\\Nodes', 'str_random')
65+
->expects($this->once())->willReturn('random_string');
6666

6767
$this->repository->shouldReceive('create')->with([
6868
'name' => 'NodeName',
69-
'daemonSecret' => 'hexResult',
69+
'daemonSecret' => 'random_string',
7070
])->once()->andReturnNull();
7171

7272
$this->assertNull($this->service->handle(['name' => 'NodeName']));

‎tests/Unit/Services/Nodes/NodeUpdateServiceTest.php‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
use GuzzleHttp\Exception\RequestException;
3434
use Pterodactyl\Exceptions\DisplayException;
3535
use Pterodactyl\Services\Nodes\NodeUpdateService;
36-
use Pterodactyl\Services\Nodes\NodeCreationService;
3736
use Pterodactyl\Contracts\Repository\NodeRepositoryInterface;
3837
use Pterodactyl\Contracts\Repository\Daemon\ConfigurationRepositoryInterface;
3938

@@ -97,20 +96,13 @@ public function setUp()
9796
*/
9897
public function testNodeIsUpdatedAndDaemonSecretIsReset()
9998
{
100-
$this->getFunctionMock('\\Pterodactyl\\Services\\Nodes', 'random_bytes')
101-
->expects($this->once())->willReturnCallback(function ($bytes) {
102-
$this->assertEquals(NodeCreationService::DAEMON_SECRET_LENGTH, $bytes);
103-
104-
return '\00';
105-
});
106-
107-
$this->getFunctionMock('\\Pterodactyl\\Services\\Nodes', 'bin2hex')
108-
->expects($this->once())->willReturn('hexResponse');
99+
$this->getFunctionMock('\\Pterodactyl\\Services\\Nodes', 'str_random')
100+
->expects($this->once())->willReturn('random_string');
109101

110102
$this->repository->shouldReceive('withoutFresh')->withNoArgs()->once()->andReturnSelf()
111103
->shouldReceive('update')->with($this->node->id, [
112104
'name' => 'NewName',
113-
'daemonSecret' => 'hexResponse',
105+
'daemonSecret' => 'random_string',
114106
])->andReturn(true);
115107

116108
$this->configRepository->shouldReceive('setNode')->with($this->node->id)->once()->andReturnSelf()

0 commit comments

Comments
 (0)