Skip to content
This repository was archived by the owner on Sep 19, 2022. It is now read-only.

Commit f04012e

Browse files
BaranekDBaranekD
authored
Security improvement of the call of updateUes.php script (#139)
1 parent 27503e5 commit f04012e

File tree

7 files changed

+399
-13
lines changed

7 files changed

+399
-13
lines changed

‎composer.json‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@
3030
"symfony/var-exporter": "^5.0",
3131
"phpseclib/phpseclib": "~3.0",
3232
"ext-curl": "*",
33-
"ext-json": "*"
33+
"ext-json": "*",
34+
"ext-openssl": "*",
35+
"web-token/jwt-key-mgmt": "^2.2",
36+
"web-token/jwt-signature": "^2.2",
37+
"web-token/jwt-signature-algorithm-rsa": "^2.2",
38+
"web-token/jwt-checker": "^2.2"
3439
}
3540
}

‎config-templates/tables.sql‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,14 @@ CREATE TABLE greyList (
1313
reason VARCHAR(255),
1414
INDEX (entityId),
1515
PRIMARY KEY (entityId)
16-
);
16+
);
17+
18+
CREATE TABLE scriptChallenges (
19+
id VARCHAR(255) NOT NULL,
20+
challenge VARCHAR(255) NOT NULL,
21+
script VARCHAR(255) NOT NULL,
22+
date DATETIME NOT NULL DEFAULT current_timestamp,
23+
PRIMARY KEY (id)
24+
);
25+
26+
CREATE INDEX idx_date on scriptChallenges (date);

‎hooks/hook_cron.php‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<?php
2+
3+
use SimpleSAML\Logger;
4+
use SimpleSAML\Module\perun\DatabaseConnector;
5+
6+
const TABLE_NAME = 'scriptChallenges';
7+
const DATE_COLUMN = 'date';
8+
9+
/**
10+
* Hook to run a cron job.
11+
*
12+
* @param array &$croninfo Output
13+
* @author Dominik Baranek <[email protected]>
14+
*/
15+
function challenges_hook_cron(&$croninfo)
16+
{
17+
if ($croninfo['tag'] !== 'hourly') {
18+
Logger::debug('cron [perun]: Skipping cron in cron tag [' . $croninfo['tag'] . '] ');
19+
return;
20+
}
21+
22+
Logger::info('cron [perun]: Running cron in cron tag [' . $croninfo['tag'] . '] ');
23+
24+
try {
25+
$databaseConnector = new DatabaseConnector();
26+
$conn = $databaseConnector->getConnection();
27+
28+
if ($conn !== null) {
29+
$stmt = $conn->prepare(
30+
'DELETE FROM ' . TABLE_NAME . ' WHERE ' . DATE_COLUMN . ' < (NOW() - INTERVAL 5 MINUTE)'
31+
);
32+
33+
if (!$stmt) {
34+
$conn->close();
35+
Logger::error('cron [perun]: Error during preparing statement');
36+
return;
37+
}
38+
39+
$ex = $stmt->execute();
40+
41+
if ($ex === false) {
42+
Logger::error('cron [perun]: Error while deleting old challenges from the database.');
43+
}
44+
45+
$stmt->close();
46+
$conn->close();
47+
}
48+
} catch (\Exception $e) {
49+
$croninfo['summary'][] = 'Error while deleting old challenges from the database: ' . $e->getMessage();
50+
}
51+
}

‎lib/Auth/Process/UpdateUserExtSource.php‎

Lines changed: 61 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,16 @@
22

33
namespace SimpleSAML\Module\perun\Auth\Process;
44

5+
use Jose\Component\KeyManagement\JWKFactory;
6+
use Jose\Component\Core\AlgorithmManager;
7+
use Jose\Component\Signature\JWSBuilder;
8+
use Jose\Component\Signature\Serializer\CompactSerializer;
9+
use Jose\Component\Signature\Algorithm\RS512;
510
use SimpleSAML\Auth\ProcessingFilter;
611
use SimpleSAML\Error\Exception;
712
use SimpleSAML\Logger;
813
use SimpleSAML\Module;
9-
use SimpleSAML\Module\perun\AttributeUtils;
1014
use SimpleSAML\Module\perun\UpdateUESThread;
11-
use SimpleSAML\Configuration;
12-
use SimpleSAML\Module\perun;
1315

1416
/**
1517
* Class sspmod_perun_Auth_Process_UpdateUserExtSource
@@ -23,6 +25,9 @@ class UpdateUserExtSource extends ProcessingFilter
2325
{
2426
private $attrMap;
2527
private $attrsToConversion;
28+
private $pathToKey;
29+
30+
const SCRIPT_NAME = 'updateUes';
2631

2732
public function __construct($config, $reserved)
2833
{
@@ -36,26 +41,77 @@ public function __construct($config, $reserved)
3641
);
3742
}
3843

44+
if (!isset($config['pathToKey'])) {
45+
throw new Exception(
46+
'perun:UpdateUserExtSource: missing mandatory configuration option \'pathToKey\'.'
47+
);
48+
}
49+
3950
if (isset($config['arrayToStringConversion'])) {
4051
$this->attrsToConversion = (array)$config['arrayToStringConversion'];
4152
} else {
4253
$this->attrsToConversion = [];
4354
}
4455

4556
$this->attrMap = (array)$config['attrMap'];
57+
$this->pathToKey = $config['pathToKey'];
4658
}
4759

4860
public function process(&$request)
4961
{
62+
$id = uniqid("", true);
63+
64+
$dataChallenge = [
65+
'id' => $id,
66+
'scriptName' => self::SCRIPT_NAME
67+
];
68+
69+
$json = json_encode($dataChallenge);
70+
71+
$curlChallenge = curl_init();
72+
curl_setopt($curlChallenge, CURLOPT_POSTFIELDS, $json);
73+
curl_setopt($curlChallenge, CURLOPT_URL, Module::getModuleURL('perun/getChallenge.php'));
74+
curl_setopt($curlChallenge, CURLOPT_RETURNTRANSFER, true);
75+
76+
$challenge = curl_exec($curlChallenge);
77+
78+
if (empty($challenge)) {
79+
Logger::error('Retrieving the challenge was not successful.');
80+
return;
81+
}
82+
83+
$jwk = JWKFactory::createFromKeyFile($this->pathToKey);
84+
$algorithmManager = new AlgorithmManager([new RS512()]);
85+
$jwsBuilder = new JWSBuilder($algorithmManager);
86+
5087
$data = [
5188
'attributes' => $request['Attributes'],
5289
'attrMap' => $this->attrMap,
5390
'attrsToConversion' => $this->attrsToConversion,
5491
'perunUserId' => $request['perun']['user']->getId()
5592
];
5693

57-
$cmd = 'curl -X POST -H "Content-Type: application/json" -d \'' . json_encode($data) . '\' ' .
58-
Module::getModuleURL('perun/updateUes.php') . ' > /dev/null &';
94+
$payload = json_encode([
95+
'iat' => time(),
96+
'nbf' => time(),
97+
'exp' => time() + 300,
98+
'challenge' => $challenge,
99+
'id' => $id,
100+
'data' => $data
101+
]);
102+
103+
$jws = $jwsBuilder
104+
->create()
105+
->withPayload($payload)
106+
->addSignature($jwk, ['alg' => 'RS512'])
107+
->build();
108+
109+
$serializer = new CompactSerializer();
110+
$token = $serializer->serialize($jws, 0);
111+
112+
$cmd = 'curl -X POST -H "Content-Type: application/json" -d ' . escapeshellarg($token) . ' ' .
113+
escapeshellarg(Module::getModuleURL('perun/updateUes.php')) . ' > /dev/null &';
114+
59115
exec($cmd);
60116
}
61117
}

‎lib/ScriptsUtils.php‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
<?php
2+
3+
4+
namespace SimpleSAML\Module\perun;
5+
6+
7+
class ScriptsUtils
8+
{
9+
const CHALLENGES_TABLE_NAME = 'scriptChallenges';
10+
const CHALLENGE = 'challenge';
11+
12+
public static function generateChallenge($connection, $challenge, $id, $scriptName): bool
13+
{
14+
if ($connection === null || empty($challenge)) {
15+
Logger::error('Perun:ScriptsUtils: Error while creating a challenge');
16+
http_response_code(500);
17+
return false;
18+
}
19+
20+
$stmt = $connection->prepare(
21+
'INSERT INTO ' . self::CHALLENGES_TABLE_NAME . ' (id, challenge, script) VALUES (?, ?, ?)'
22+
);
23+
24+
if ($stmt) {
25+
$stmt->bind_param('sss', $id, $challenge, $scriptName);
26+
$ex = $stmt->execute();
27+
28+
if ($ex === false) {
29+
Logger::error('Perun:ScriptsUtils: Error while creating a challenge');
30+
http_response_code(500);
31+
return false;
32+
}
33+
34+
$stmt->close();
35+
} else {
36+
Logger::error('Perun:ScriptsUtils: Error during preparing statement');
37+
http_response_code(500);
38+
return false;
39+
}
40+
41+
return true;
42+
}
43+
44+
public static function readChallengeFromDb($connection, $id)
45+
{
46+
if ($connection === null) {
47+
http_response_code(500);
48+
return null;
49+
}
50+
51+
if (empty($id)) {
52+
http_response_code(400);
53+
return null;
54+
}
55+
56+
$stmt = $connection->prepare('SELECT challenge FROM ' . self::CHALLENGES_TABLE_NAME . ' WHERE id=?');
57+
58+
if (!$stmt) {
59+
Logger::error('Perun:ScriptsUtils: Error during preparing statement');
60+
http_response_code(500);
61+
return null;
62+
}
63+
64+
$stmt->bind_param('s', $id);
65+
$ex = $stmt->execute();
66+
67+
if ($ex === false) {
68+
Logger::error('Perun:ScriptsUtils: Error while getting the challenge from the database.');
69+
http_response_code(500);
70+
return null;
71+
}
72+
73+
$challengeDb = $stmt->get_result()->fetch_assoc()[self::CHALLENGE];
74+
$stmt->close();
75+
76+
return $challengeDb;
77+
}
78+
79+
public static function checkAccess($connection, $challenge, $challengeDb): bool
80+
{
81+
if ($connection === null) {
82+
http_response_code(500);
83+
return false;
84+
}
85+
86+
if (empty($challenge) || empty($challengeDb)) {
87+
http_response_code(400);
88+
return false;
89+
}
90+
91+
if (!hash_equals($challengeDb, $challenge)) {
92+
Logger::error('Perun:ScriptsUtils: Hashes are not equal.');
93+
http_response_code(401);
94+
return false;
95+
}
96+
97+
return true;
98+
}
99+
100+
public static function deleteChallengeFromDb($connection, $id): bool
101+
{
102+
if ($connection === null) {
103+
http_response_code(500);
104+
return false;
105+
}
106+
107+
if (empty($id)) {
108+
http_response_code(400);
109+
return false;
110+
}
111+
112+
$stmt = $connection->prepare('DELETE FROM ' . self::CHALLENGES_TABLE_NAME . ' WHERE id=?');
113+
114+
if ($stmt) {
115+
$stmt->bind_param('s', $id);
116+
$ex = $stmt->execute();
117+
118+
if ($ex === false) {
119+
Logger::error('Perun:ScriptsUtils: Error while deleting the challenge from the database.');
120+
http_response_code(500);
121+
return false;
122+
}
123+
124+
$stmt->close();
125+
} else {
126+
Logger::error('Perun:ScriptsUtils: Error during preparing statement');
127+
http_response_code(500);
128+
return false;
129+
}
130+
131+
return true;
132+
}
133+
}

‎www/getChallenge.php‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<?php
2+
3+
use SimpleSAML\Module\perun\DatabaseConnector;
4+
use SimpleSAML\Logger;
5+
use SimpleSAML\Module\perun\ScriptsUtils;
6+
7+
$entityBody = file_get_contents('php://input');
8+
$body = json_decode($entityBody, true);
9+
10+
if ($body === false) {
11+
Logger::error('Perun.getChallenge: Received invalid json.');
12+
http_response_code(400);
13+
exit;
14+
}
15+
16+
if (empty($body['id'] || strlen($body['id']) > 30 || !ctype_print($body['id']))) {
17+
Logger::error('Perun.getChallenge: Invalid id');
18+
http_response_code(400);
19+
exit;
20+
}
21+
22+
if (empty($body['scriptName']) || strlen($body['scriptName']) > 255 || !ctype_print($body['scriptName'])) {
23+
http_response_code(400);
24+
Logger::error('Perun.getChallenge: Invalid scriptName');
25+
exit;
26+
}
27+
28+
$id = $body['id'];
29+
$scriptName = $body['scriptName'];
30+
31+
const RANDOM_BYTES_LENGTH = 32;
32+
const TABLE_NAME = 'scriptChallenges';
33+
34+
try {
35+
$challenge = hash('sha256', random_bytes(RANDOM_BYTES_LENGTH));
36+
} catch (Exception $ex) {
37+
Logger::error('Perun.getChallenge: Error while generating a challenge');
38+
http_response_code(500);
39+
exit;
40+
}
41+
42+
$databaseConnector = new DatabaseConnector();
43+
$conn = $databaseConnector->getConnection();
44+
$generateChallengeSucceeded = ScriptsUtils::generateChallenge($conn, $challenge, $id, $scriptName);
45+
$conn->close();
46+
47+
if (!$generateChallengeSucceeded) {
48+
exit;
49+
}
50+
51+
echo $challenge;

0 commit comments

Comments
 (0)