From 351eb01bfb1ae47a0414875383cf080d42a06e19 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dominik=20Bar=C3=A1nek?= <0Baranek.dominik0@gmail.com>
Date: Fri, 5 Mar 2021 13:54:54 +0100
Subject: [PATCH] Security improvement of the call of updateUes.php script
---
composer.json | 7 +-
config-templates/tables.sql | 12 +-
hooks/hook_cron.php | 51 +++++++++
lib/Auth/Process/UpdateUserExtSource.php | 66 ++++++++++-
lib/ScriptsUtils.php | 133 +++++++++++++++++++++++
www/getChallenge.php | 51 +++++++++
www/updateUes.php | 92 +++++++++++++++-
7 files changed, 399 insertions(+), 13 deletions(-)
create mode 100644 hooks/hook_cron.php
create mode 100644 lib/ScriptsUtils.php
create mode 100644 www/getChallenge.php
diff --git a/composer.json b/composer.json
index e99e612c..14bab4e2 100644
--- a/composer.json
+++ b/composer.json
@@ -29,6 +29,11 @@
"cesnet/simplesamlphp-module-chartjs": "~2.8.0",
"symfony/var-exporter": "^5.0",
"ext-curl": "*",
- "ext-json": "*"
+ "ext-json": "*",
+ "ext-openssl": "*",
+ "web-token/jwt-key-mgmt": "^2.2",
+ "web-token/jwt-signature": "^2.2",
+ "web-token/jwt-signature-algorithm-rsa": "^2.2",
+ "web-token/jwt-checker": "^2.2"
}
}
diff --git a/config-templates/tables.sql b/config-templates/tables.sql
index a0af0461..a5a19188 100644
--- a/config-templates/tables.sql
+++ b/config-templates/tables.sql
@@ -13,4 +13,14 @@ CREATE TABLE greyList (
reason VARCHAR(255),
INDEX (entityId),
PRIMARY KEY (entityId)
-);
\ No newline at end of file
+);
+
+CREATE TABLE scriptChallenges (
+ id VARCHAR(255) NOT NULL,
+ challenge VARCHAR(255) NOT NULL,
+ script VARCHAR(255) NOT NULL,
+ date DATETIME NOT NULL DEFAULT current_timestamp,
+ PRIMARY KEY (id)
+);
+
+CREATE INDEX idx_date on scriptChallenges (date);
diff --git a/hooks/hook_cron.php b/hooks/hook_cron.php
new file mode 100644
index 00000000..081159cc
--- /dev/null
+++ b/hooks/hook_cron.php
@@ -0,0 +1,51 @@
+<?php
+
+use SimpleSAML\Logger;
+use SimpleSAML\Module\perun\DatabaseConnector;
+
+const TABLE_NAME = 'scriptChallenges';
+const DATE_COLUMN = 'date';
+
+/**
+ * Hook to run a cron job.
+ *
+ * @param array &$croninfo Output
+ * @author Dominik Baranek <baranek@ics.muni.cz>
+ */
+function challenges_hook_cron(&$croninfo)
+{
+ if ($croninfo['tag'] !== 'hourly') {
+ Logger::debug('cron [perun]: Skipping cron in cron tag [' . $croninfo['tag'] . '] ');
+ return;
+ }
+
+ Logger::info('cron [perun]: Running cron in cron tag [' . $croninfo['tag'] . '] ');
+
+ try {
+ $databaseConnector = new DatabaseConnector();
+ $conn = $databaseConnector->getConnection();
+
+ if ($conn !== null) {
+ $stmt = $conn->prepare(
+ 'DELETE FROM ' . TABLE_NAME . ' WHERE ' . DATE_COLUMN . ' < (NOW() - INTERVAL 5 MINUTE)'
+ );
+
+ if (!$stmt) {
+ $conn->close();
+ Logger::error('cron [perun]: Error during preparing statement');
+ return;
+ }
+
+ $ex = $stmt->execute();
+
+ if ($ex === false) {
+ Logger::error('cron [perun]: Error while deleting old challenges from the database.');
+ }
+
+ $stmt->close();
+ $conn->close();
+ }
+ } catch (\Exception $e) {
+ $croninfo['summary'][] = 'Error while deleting old challenges from the database: ' . $e->getMessage();
+ }
+}
diff --git a/lib/Auth/Process/UpdateUserExtSource.php b/lib/Auth/Process/UpdateUserExtSource.php
index b99b22b0..9cf124a5 100644
--- a/lib/Auth/Process/UpdateUserExtSource.php
+++ b/lib/Auth/Process/UpdateUserExtSource.php
@@ -2,14 +2,16 @@
namespace SimpleSAML\Module\perun\Auth\Process;
+use Jose\Component\KeyManagement\JWKFactory;
+use Jose\Component\Core\AlgorithmManager;
+use Jose\Component\Signature\JWSBuilder;
+use Jose\Component\Signature\Serializer\CompactSerializer;
+use Jose\Component\Signature\Algorithm\RS512;
use SimpleSAML\Auth\ProcessingFilter;
use SimpleSAML\Error\Exception;
use SimpleSAML\Logger;
use SimpleSAML\Module;
-use SimpleSAML\Module\perun\AttributeUtils;
use SimpleSAML\Module\perun\UpdateUESThread;
-use SimpleSAML\Configuration;
-use SimpleSAML\Module\perun;
/**
* Class sspmod_perun_Auth_Process_UpdateUserExtSource
@@ -23,6 +25,9 @@ class UpdateUserExtSource extends ProcessingFilter
{
private $attrMap;
private $attrsToConversion;
+ private $pathToKey;
+
+ const SCRIPT_NAME = 'updateUes';
public function __construct($config, $reserved)
{
@@ -36,6 +41,12 @@ class UpdateUserExtSource extends ProcessingFilter
);
}
+ if (!isset($config['pathToKey'])) {
+ throw new Exception(
+ 'perun:UpdateUserExtSource: missing mandatory configuration option \'pathToKey\'.'
+ );
+ }
+
if (isset($config['arrayToStringConversion'])) {
$this->attrsToConversion = (array)$config['arrayToStringConversion'];
} else {
@@ -43,10 +54,36 @@ class UpdateUserExtSource extends ProcessingFilter
}
$this->attrMap = (array)$config['attrMap'];
+ $this->pathToKey = $config['pathToKey'];
}
public function process(&$request)
{
+ $id = uniqid("", true);
+
+ $dataChallenge = [
+ 'id' => $id,
+ 'scriptName' => self::SCRIPT_NAME
+ ];
+
+ $json = json_encode($dataChallenge);
+
+ $curlChallenge = curl_init();
+ curl_setopt($curlChallenge, CURLOPT_POSTFIELDS, $json);
+ curl_setopt($curlChallenge, CURLOPT_URL, Module::getModuleURL('perun/getChallenge.php'));
+ curl_setopt($curlChallenge, CURLOPT_RETURNTRANSFER, true);
+
+ $challenge = curl_exec($curlChallenge);
+
+ if (empty($challenge)) {
+ Logger::error('Retrieving the challenge was not successful.');
+ return;
+ }
+
+ $jwk = JWKFactory::createFromKeyFile($this->pathToKey);
+ $algorithmManager = new AlgorithmManager([new RS512()]);
+ $jwsBuilder = new JWSBuilder($algorithmManager);
+
$data = [
'attributes' => $request['Attributes'],
'attrMap' => $this->attrMap,
@@ -54,8 +91,27 @@ class UpdateUserExtSource extends ProcessingFilter
'perunUserId' => $request['perun']['user']->getId()
];
- $cmd = 'curl -X POST -H "Content-Type: application/json" -d \'' . json_encode($data) . '\' ' .
- Module::getModuleURL('perun/updateUes.php') . ' > /dev/null &';
+ $payload = json_encode([
+ 'iat' => time(),
+ 'nbf' => time(),
+ 'exp' => time() + 300,
+ 'challenge' => $challenge,
+ 'id' => $id,
+ 'data' => $data
+ ]);
+
+ $jws = $jwsBuilder
+ ->create()
+ ->withPayload($payload)
+ ->addSignature($jwk, ['alg' => 'RS512'])
+ ->build();
+
+ $serializer = new CompactSerializer();
+ $token = $serializer->serialize($jws, 0);
+
+ $cmd = 'curl -X POST -H "Content-Type: application/json" -d ' . escapeshellarg($token) . ' ' .
+ escapeshellarg(Module::getModuleURL('perun/updateUes.php')) . ' > /dev/null &';
+
exec($cmd);
}
}
diff --git a/lib/ScriptsUtils.php b/lib/ScriptsUtils.php
new file mode 100644
index 00000000..4bc1547e
--- /dev/null
+++ b/lib/ScriptsUtils.php
@@ -0,0 +1,133 @@
+<?php
+
+
+namespace SimpleSAML\Module\perun;
+
+
+class ScriptsUtils
+{
+ const CHALLENGES_TABLE_NAME = 'scriptChallenges';
+ const CHALLENGE = 'challenge';
+
+ public static function generateChallenge($connection, $challenge, $id, $scriptName): bool
+ {
+ if ($connection === null || empty($challenge)) {
+ Logger::error('Perun:ScriptsUtils: Error while creating a challenge');
+ http_response_code(500);
+ return false;
+ }
+
+ $stmt = $connection->prepare(
+ 'INSERT INTO ' . self::CHALLENGES_TABLE_NAME . ' (id, challenge, script) VALUES (?, ?, ?)'
+ );
+
+ if ($stmt) {
+ $stmt->bind_param('sss', $id, $challenge, $scriptName);
+ $ex = $stmt->execute();
+
+ if ($ex === false) {
+ Logger::error('Perun:ScriptsUtils: Error while creating a challenge');
+ http_response_code(500);
+ return false;
+ }
+
+ $stmt->close();
+ } else {
+ Logger::error('Perun:ScriptsUtils: Error during preparing statement');
+ http_response_code(500);
+ return false;
+ }
+
+ return true;
+ }
+
+ public static function readChallengeFromDb($connection, $id)
+ {
+ if ($connection === null) {
+ http_response_code(500);
+ return null;
+ }
+
+ if (empty($id)) {
+ http_response_code(400);
+ return null;
+ }
+
+ $stmt = $connection->prepare('SELECT challenge FROM ' . self::CHALLENGES_TABLE_NAME . ' WHERE id=?');
+
+ if (!$stmt) {
+ Logger::error('Perun:ScriptsUtils: Error during preparing statement');
+ http_response_code(500);
+ return null;
+ }
+
+ $stmt->bind_param('s', $id);
+ $ex = $stmt->execute();
+
+ if ($ex === false) {
+ Logger::error('Perun:ScriptsUtils: Error while getting the challenge from the database.');
+ http_response_code(500);
+ return null;
+ }
+
+ $challengeDb = $stmt->get_result()->fetch_assoc()[self::CHALLENGE];
+ $stmt->close();
+
+ return $challengeDb;
+ }
+
+ public static function checkAccess($connection, $challenge, $challengeDb): bool
+ {
+ if ($connection === null) {
+ http_response_code(500);
+ return false;
+ }
+
+ if (empty($challenge) || empty($challengeDb)) {
+ http_response_code(400);
+ return false;
+ }
+
+ if (!hash_equals($challengeDb, $challenge)) {
+ Logger::error('Perun:ScriptsUtils: Hashes are not equal.');
+ http_response_code(401);
+ return false;
+ }
+
+ return true;
+ }
+
+ public static function deleteChallengeFromDb($connection, $id): bool
+ {
+ if ($connection === null) {
+ http_response_code(500);
+ return false;
+ }
+
+ if (empty($id)) {
+ http_response_code(400);
+ return false;
+ }
+
+ $stmt = $connection->prepare('DELETE FROM ' . self::CHALLENGES_TABLE_NAME . ' WHERE id=?');
+
+ if ($stmt) {
+ $stmt->bind_param('s', $id);
+ $ex = $stmt->execute();
+
+ if ($ex === false) {
+ Logger::error('Perun:ScriptsUtils: Error while deleting the challenge from the database.');
+ http_response_code(500);
+ return false;
+ }
+
+ $stmt->close();
+ } else {
+ Logger::error('Perun:ScriptsUtils: Error during preparing statement');
+ http_response_code(500);
+ return false;
+ }
+
+ return true;
+ }
+}
diff --git a/www/getChallenge.php b/www/getChallenge.php
new file mode 100644
index 00000000..881c290d
--- /dev/null
+++ b/www/getChallenge.php
@@ -0,0 +1,51 @@
+<?php
+
+use SimpleSAML\Module\perun\DatabaseConnector;
+use SimpleSAML\Logger;
+use SimpleSAML\Module\perun\ScriptsUtils;
+
+$entityBody = file_get_contents('php://input');
+$body = json_decode($entityBody, true);
+
+if ($body === false) {
+ Logger::error('Perun.getChallenge: Received invalid json.');
+ http_response_code(400);
+ exit;
+}
+
+if (empty($body['id'] || strlen($body['id']) > 30 || !ctype_print($body['id']))) {
+ Logger::error('Perun.getChallenge: Invalid id');
+ http_response_code(400);
+ exit;
+}
+
+if (empty($body['scriptName']) || strlen($body['scriptName']) > 255 || !ctype_print($body['scriptName'])) {
+ http_response_code(400);
+ Logger::error('Perun.getChallenge: Invalid scriptName');
+ exit;
+}
+
+$id = $body['id'];
+$scriptName = $body['scriptName'];
+
+const RANDOM_BYTES_LENGTH = 32;
+const TABLE_NAME = 'scriptChallenges';
+
+try {
+ $challenge = hash('sha256', random_bytes(RANDOM_BYTES_LENGTH));
+} catch (Exception $ex) {
+ Logger::error('Perun.getChallenge: Error while generating a challenge');
+ http_response_code(500);
+ exit;
+}
+
+$databaseConnector = new DatabaseConnector();
+$conn = $databaseConnector->getConnection();
+$generateChallengeSucceeded = ScriptsUtils::generateChallenge($conn, $challenge, $id, $scriptName);
+$conn->close();
+
+if (!$generateChallengeSucceeded) {
+ exit;
+}
+
+echo $challenge;
diff --git a/www/updateUes.php b/www/updateUes.php
index ec8af706..6ba041f1 100644
--- a/www/updateUes.php
+++ b/www/updateUes.php
@@ -4,22 +4,102 @@
* Script for updating UES in separate thread
*
* @author Pavel VyskoÄŤil <vyskocilpavel@muni.cz>
+ * @author Dominik Baranek <baranek@ics.muni.cz>
*/
+use Jose\Component\Checker\ClaimCheckerManager;
+use Jose\Component\Checker;
+use Jose\Component\Core\AlgorithmManager;
+use Jose\Component\KeyManagement\JWKFactory;
+use Jose\Component\Signature\Algorithm\RS512;
+use Jose\Component\Signature\JWSVerifier;
+use Jose\Component\Checker\HeaderCheckerManager;
+use Jose\Component\Checker\AlgorithmChecker;
+use Jose\Component\Signature\JWSTokenSupport;
+use Jose\Component\Signature\Serializer\CompactSerializer;
+use Jose\Component\Signature\Serializer\JWSSerializerManager;
+use SimpleSAML\Configuration;
use SimpleSAML\Logger;
use SimpleSAML\Module\perun\Adapter;
+use SimpleSAML\Module\perun\DatabaseConnector;
+use SimpleSAML\Module\perun\ScriptsUtils;
$adapter = Adapter::getInstance(Adapter::RPC);
+$token = file_get_contents('php://input');
-$entityBody = file_get_contents('php://input');
-$body = json_decode($entityBody, true);
+if (empty($token)) {
+ http_response_code(400);
+ exit('The entity body is empty');
+}
-$attributesFromIdP = $body['attributes'];
-$attrMap = $body['attrMap'];
-$attrsToConversion = $body['attrsToConversion'];
-$perunUserId = $body['perunUserId'];
+$attributesFromIdP = null;
+$attrMap = null;
+$attrsToConversion = null;
+$perunUserId = null;
+$id = null;
const UES_ATTR_NMS = 'urn:perun:ues:attribute-def:def';
+const CONFIG_FILE_NAME = 'keys.php';
+
+try {
+ $config = Configuration::getConfig(CONFIG_FILE_NAME);
+ $keyPub = $config->getString('updateUes');
+
+ $algorithmManager = new AlgorithmManager([new RS512()]);
+ $jwsVerifier = new JWSVerifier($algorithmManager);
+ $jwk = JWKFactory::createFromKeyFile($keyPub);
+
+ $serializerManager = new JWSSerializerManager([new CompactSerializer()]);
+ $jws = $serializerManager->unserialize($token);
+
+ $headerCheckerManager = new HeaderCheckerManager([new AlgorithmChecker(['RS512'])], [new JWSTokenSupport()]);
+ $headerCheckerManager->check($jws, 0);
+
+ $isVerified = $jwsVerifier->verifyWithKey($jws, $jwk, 0);
+
+ if (!$isVerified) {
+ Logger::error('Perun.updateUes: The token signature is invalid!');
+ http_response_code(401);
+ exit;
+ }
+
+ $claimCheckerManager = new ClaimCheckerManager(
+ [
+ new Checker\IssuedAtChecker(),
+ new Checker\NotBeforeChecker(),
+ new Checker\ExpirationTimeChecker(),
+ ]
+ );
+
+ $claims = json_decode($jws->getPayload(), true);
+ $claimCheckerManager->check($claims);
+
+ $challenge = $claims['challenge'];
+
+ $attributesFromIdP = $claims['data']['attributes'];
+ $attrMap = $claims['data']['attrMap'];
+ $attrsToConversion = $claims['data']['attrsToConversion'];
+ $perunUserId = $claims['data']['perunUserId'];
+ $id = $claims['id'];
+
+ $databaseConnector = new DatabaseConnector();
+
+ $conn = $databaseConnector->getConnection();
+
+ $challengeDb = ScriptsUtils::readChallengeFromDb($conn, $id);
+ $checkAccessSucceeded = ScriptsUtils::checkAccess($conn, $challenge, $challengeDb);
+ $challengeSuccessfullyDeleted = ScriptsUtils::deleteChallengeFromDb($conn, $id);
+
+ $conn->close();
+
+ if (!$checkAccessSucceeded || !$challengeSuccessfullyDeleted) {
+ exit;
+ }
+} catch (Checker\InvalidClaimException | Checker\MissingMandatoryClaimException $ex) {
+ Logger::error('Perun.updateUes: An error occurred when the token was verifying.');
+ http_response_code(400);
+ exit;
+}
try {
$userExtSource = $adapter->getUserExtSource(
--
GitLab