From 72a2bf420a232911299e041a3e1f5f239989bcb5 Mon Sep 17 00:00:00 2001 From: Mark Hamstra Date: Sat, 28 Mar 2020 00:01:22 +0100 Subject: [PATCH 1/3] First stab at implementing asynchronously pushed-back task results The general idea is to not keep workers waiting for tasks that may take a while to complete. Things like backups and upgrades may take the client a minute to process - the worker does not need to sit idle during that time; it might as well process other tasks simultaniously. Flow: 1. Worker pushes the task to the client, with a uri and one-time signing key 2. Client acknowledges receipt and support for the async return push with a 202 3. Worker marks task as awaiting async response and continues to whatever other tasks are queued 4. Client processes slow task, and when done pushes the response to the sitedash endpoint from (1). Endpoint stores received data, marks related job exec as having received data to process. 5. Same worker (when free) is assigned again to job exec and finishes processing the data --- assets/components/sitedashclient/pull.php | 11 +- .../src/Communication/Pusher.php | 109 ++++++++++++++++++ .../src/Communication/Result.php | 28 +++++ .../components/sitedashclient/src/Refresh.php | 2 +- .../sitedashclient/src/Upgrade/Backup.php | 42 ++++--- 5 files changed, 174 insertions(+), 18 deletions(-) create mode 100644 core/components/sitedashclient/src/Communication/Pusher.php create mode 100644 core/components/sitedashclient/src/Communication/Result.php diff --git a/assets/components/sitedashclient/pull.php b/assets/components/sitedashclient/pull.php index 5fe1688..be17a1e 100644 --- a/assets/components/sitedashclient/pull.php +++ b/assets/components/sitedashclient/pull.php @@ -45,6 +45,15 @@ // Make sure the params are sanitized $params = $modx::sanitize($_POST); +$pusher = null; +if (array_key_exists('_return_push', $_POST) && array_key_exists('_return_push_key', $_POST)) { + $server = $modx->getOption('sitedash.server_uri', null, 'https://sitedash.app/', true); + $responseUri = (string)$_POST['_return_push']; + $signingKey = (string)$_POST['_return_push_key']; + + $pusher = new \modmore\SiteDashClient\Communication\Pusher($server, $responseUri, $signingKey); +} + switch ($params['request']) { case 'system': case 'system/refresh': @@ -96,7 +105,7 @@ case 'upgrade/backup': - $cmd = new \modmore\SiteDashClient\Upgrade\Backup($modx); + $cmd = new \modmore\SiteDashClient\Upgrade\Backup($modx, $pusher); $cmd->run(); break; diff --git a/core/components/sitedashclient/src/Communication/Pusher.php b/core/components/sitedashclient/src/Communication/Pusher.php new file mode 100644 index 0000000..4d76b99 --- /dev/null +++ b/core/components/sitedashclient/src/Communication/Pusher.php @@ -0,0 +1,109 @@ +responseUri = $server . $responseUri; + $this->signingKey = base64_decode($signingKey); + } + + public function acknowledge() + { + ob_start(); + + echo json_encode([ + 'return_push' => true, + ]); + + // Get the size of the output. + $size = ob_get_length(); + + // 202 accepted + http_response_code(202); + + // Disable compression (in case content length is compressed). + header('Content-Encoding: none'); + + // Set the content length of the response. + header("Content-Length: {$size}"); + + // Close the connection. + header('Connection: close'); + + // Flush all output. + ob_end_flush(); + ob_flush(); + flush(); + + ignore_user_abort(true); + @session_write_close(); + + if (is_callable('fastcgi_finish_request')) { + fastcgi_finish_request(); + return; + } + sleep(1); + } + + public function push(array $data) + { + $logFile = MODX_CORE_PATH . 'cache/logs/sitedash_push_' . date('Y-m-d-H-i-s') . '.log'; + + $ch = curl_init(); + + $postData = $this->prepareData($data); + curl_setopt($ch, CURLOPT_URL, $this->responseUri); + curl_setopt($ch, CURLOPT_POST, true); + curl_setopt($ch, CURLOPT_POSTFIELDS, json_encode($postData)); + curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); + curl_setopt($ch, CURLOPT_HTTPHEADER, ['Content-Type:application/json']); + + $response = curl_exec($ch); + $error = curl_error($ch); + $errno = curl_errno($ch); + curl_close($ch); + + $dataFormat = json_encode($data, JSON_PRETTY_PRINT); + $postDataFormat = json_encode($postData, JSON_PRETTY_PRINT); + $log = <<responseUri} with one-time use signing key: + +{$this->signingKey} + +Data: {$dataFormat} + +Data to post to SiteDash, incl signature: {$postDataFormat} + +Response from SiteDash: {$errno} {$error} + + {$response} +HTML; + + file_put_contents($logFile, $log); + } + + private function prepareData(array $data) + { + return [ + 'data' => $data, + 'signature' => $this->sign($data), + ]; + } + + private function sign(array $data) + { + $sigData = json_encode($data); + + $binary_signature = ''; + openssl_sign($sigData, $binary_signature, $this->signingKey, OPENSSL_ALGO_SHA1); + + // Encode it as base64 + $binary_signature = base64_encode($binary_signature); + return $binary_signature; + } +} \ No newline at end of file diff --git a/core/components/sitedashclient/src/Communication/Result.php b/core/components/sitedashclient/src/Communication/Result.php new file mode 100644 index 0000000..843f85a --- /dev/null +++ b/core/components/sitedashclient/src/Communication/Result.php @@ -0,0 +1,28 @@ +pusher = $pusher; + } + + public function __invoke($responseCode, $data) + { + if ($this->pusher) { + $this->pusher->push($data); + } + else { + http_response_code($responseCode); + echo json_encode($data, JSON_PRETTY_PRINT); + @session_write_close(); + exit(); + } + } +} \ No newline at end of file diff --git a/core/components/sitedashclient/src/Refresh.php b/core/components/sitedashclient/src/Refresh.php index d766b75..d9fd33e 100644 --- a/core/components/sitedashclient/src/Refresh.php +++ b/core/components/sitedashclient/src/Refresh.php @@ -17,7 +17,7 @@ public function run() $data = []; $data['client'] = \SiteDashClient::VERSION; $data['client_options'] = [ - 'supports_return_push' => false, + 'supports_return_push' => true, 'supports_async_execute' => false, ]; $data['modx'] = $this->getMODXData(); diff --git a/core/components/sitedashclient/src/Upgrade/Backup.php b/core/components/sitedashclient/src/Upgrade/Backup.php index d1f5208..7e430b7 100644 --- a/core/components/sitedashclient/src/Upgrade/Backup.php +++ b/core/components/sitedashclient/src/Upgrade/Backup.php @@ -3,6 +3,8 @@ namespace modmore\SiteDashClient\Upgrade; use modmore\SiteDashClient\CommandInterface; +use modmore\SiteDashClient\Communication\Pusher; +use modmore\SiteDashClient\Communication\Result; use Symfony\Component\Process\ExecutableFinder; use Symfony\Component\Process\Process; @@ -10,10 +12,15 @@ class Backup implements CommandInterface { protected $modx; protected $files = []; protected $targetDirectory; + /** + * @var Pusher|null + */ + private $pusher; - public function __construct(\modX $modx) + public function __construct(\modX $modx, $pusher = null) { $this->modx = $modx; + $this->pusher = $pusher; $this->files = [ MODX_CORE_PATH . 'config/' . MODX_CONFIG_KEY . '.inc.php', @@ -66,6 +73,13 @@ public function run() return; } + // If a push result was requested, send an ack response and continue processing + if ($this->pusher) { + $this->pusher->acknowledge(); + } + + $result = new Result($this->pusher); + /** * Include the config file to access the database information * @@ -112,51 +126,48 @@ public function run() $msg = str_replace($password_parameter, '-p\'\'', $msg); $trace = $e->getTraceAsString(); $trace = str_replace($password_parameter, '-p\'\'', $trace); - http_response_code(503); - echo json_encode([ + + $result(503, [ 'success' => false, 'message' => 'Received an error trying to run mysqlbackup: ' . $msg, 'binary' => $mysqldump, 'directory' => str_replace(MODX_CORE_PATH, '{core_path}', $this->targetDirectory), 'output' => $trace, - ], JSON_PRETTY_PRINT); + ]); return; } $output = $backupProcess->getErrorOutput() . ' ' . $backupProcess->getOutput(); $output = str_replace($password_parameter, '-p\'\'', $output); if (!$backupProcess->isSuccessful()) { - http_response_code(503); $code = $backupProcess->getExitCode(); if ($code === 127) { - echo json_encode([ + $result(503, [ 'success' => false, 'message' => 'Could not find the mysqldump program on your server; please configure the sitedashclient.mysqldump_binary system setting to point to mysqldump to create backups.', 'binary' => $mysqldump, 'directory' => str_replace(MODX_CORE_PATH, '{core_path}', $this->targetDirectory), 'output' => $output, - ], JSON_PRETTY_PRINT); + ]); return; } - echo json_encode([ + $result(503, [ 'success' => false, 'message' => 'Received exit code ' . $code . ' trying to create a database backup using ' . $mysqldump . ' with message: ' . $output, 'output' => $output, 'return' => $code, - ], JSON_PRETTY_PRINT); + ]); return; } $backupSize = filesize($targetFile); if ($backupSize < 150 * 1024) { // a clean install is ~ 200kb, so we ask for at least 150 - http_response_code(503); - - echo json_encode([ + $result(503, [ 'success' => false, 'message' => 'While the backup with ' . $mysqldump . ' did not indicate an error, the mysql backup is only ' . number_format($backupSize / 1024, 0) . 'kb in size, so it probably failed.', 'output' => $output, 'return' => $backupProcess->getExitCode(), - ], JSON_PRETTY_PRINT); + ]); return; } @@ -179,11 +190,10 @@ public function run() } } - http_response_code(200); - echo json_encode([ + $result(200, [ 'success' => true, 'directory' => str_replace(MODX_CORE_PATH, '', $this->targetDirectory), - ], JSON_PRETTY_PRINT); + ]); } private function createDirectory($target) From ebe9bb33c12c2691af5340101c27b8a7e0f7178c Mon Sep 17 00:00:00 2001 From: Mark Hamstra Date: Wed, 14 Jul 2021 13:54:49 +0200 Subject: [PATCH 2/3] Remove the signing key, as that offers very little added security for the added complexity The return push uri is now a 96-character random string that's only valid once (when SiteDash expects a response) so that's going to be incredibly difficult to cause arbitrary data to get sent back by an attacker. Lack of a signature does theoretically allow MITM, but nearly impossible to execute server-to-server and production runs on HTTPS with HSTS. --- assets/components/sitedashclient/pull.php | 5 ++-- .../src/Communication/Pusher.php | 27 +++---------------- 2 files changed, 6 insertions(+), 26 deletions(-) diff --git a/assets/components/sitedashclient/pull.php b/assets/components/sitedashclient/pull.php index be17a1e..6322d73 100644 --- a/assets/components/sitedashclient/pull.php +++ b/assets/components/sitedashclient/pull.php @@ -46,12 +46,11 @@ $params = $modx::sanitize($_POST); $pusher = null; -if (array_key_exists('_return_push', $_POST) && array_key_exists('_return_push_key', $_POST)) { +if (array_key_exists('_return_push', $_POST) && !empty($_POST['_return_push'])) { $server = $modx->getOption('sitedash.server_uri', null, 'https://sitedash.app/', true); $responseUri = (string)$_POST['_return_push']; - $signingKey = (string)$_POST['_return_push_key']; - $pusher = new \modmore\SiteDashClient\Communication\Pusher($server, $responseUri, $signingKey); + $pusher = new \modmore\SiteDashClient\Communication\Pusher($server, $responseUri); } switch ($params['request']) { diff --git a/core/components/sitedashclient/src/Communication/Pusher.php b/core/components/sitedashclient/src/Communication/Pusher.php index 4d76b99..dc81896 100644 --- a/core/components/sitedashclient/src/Communication/Pusher.php +++ b/core/components/sitedashclient/src/Communication/Pusher.php @@ -4,12 +4,10 @@ final class Pusher { private $responseUri; - private $signingKey; - public function __construct($server, $responseUri, $signingKey) + public function __construct($server, $responseUri) { $this->responseUri = $server . $responseUri; - $this->signingKey = base64_decode($signingKey); } public function acknowledge() @@ -71,9 +69,7 @@ public function push(array $data) $dataFormat = json_encode($data, JSON_PRETTY_PRINT); $postDataFormat = json_encode($postData, JSON_PRETTY_PRINT); $log = <<responseUri} with one-time use signing key: - -{$this->signingKey} +Push request to {$this->responseUri}: Data: {$dataFormat} @@ -89,21 +85,6 @@ public function push(array $data) private function prepareData(array $data) { - return [ - 'data' => $data, - 'signature' => $this->sign($data), - ]; - } - - private function sign(array $data) - { - $sigData = json_encode($data); - - $binary_signature = ''; - openssl_sign($sigData, $binary_signature, $this->signingKey, OPENSSL_ALGO_SHA1); - - // Encode it as base64 - $binary_signature = base64_encode($binary_signature); - return $binary_signature; + return $data; } -} \ No newline at end of file +} From 290b5edf92d409d84ec4b214694352c2cd5d7ca3 Mon Sep 17 00:00:00 2001 From: Mark Hamstra Date: Wed, 14 Jul 2021 13:56:05 +0200 Subject: [PATCH 3/3] Add command to run a real test of async pushes --- assets/components/sitedashclient/pull.php | 5 +++ .../src/Communication/TestAsyncPush.php | 38 +++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 core/components/sitedashclient/src/Communication/TestAsyncPush.php diff --git a/assets/components/sitedashclient/pull.php b/assets/components/sitedashclient/pull.php index 6322d73..852d5fb 100644 --- a/assets/components/sitedashclient/pull.php +++ b/assets/components/sitedashclient/pull.php @@ -122,6 +122,11 @@ $cmd->run(); break; + case 'communication/test-async-push': + $cmd = new \modmore\SiteDashClient\Communication\TestAsyncPush($pusher); + $cmd->run(); + break; + default: http_response_code(400); echo json_encode([ diff --git a/core/components/sitedashclient/src/Communication/TestAsyncPush.php b/core/components/sitedashclient/src/Communication/TestAsyncPush.php new file mode 100644 index 0000000..a01859f --- /dev/null +++ b/core/components/sitedashclient/src/Communication/TestAsyncPush.php @@ -0,0 +1,38 @@ +pusher = $pusher; + } + + public function run() + { + $this->pusher->acknowledge(); + $result = new Result($this->pusher); + + $logFile = MODX_CORE_PATH . 'cache/logs/sitedash_pushtest_' . date('Y-m-d-H-i-s') . '.log'; + $limit = 15; + $i = 0; + @set_time_limit($limit + 5); + while ($i < $limit) { + $i++; + file_put_contents($logFile, date('Y-m-d H:i:s') . "\n", FILE_APPEND); + sleep(1); + } + + $result(200, [ + 'success' => true, + 'counted_to' => $limit, + ]); + } +}