From b54007176cc6802f154fdb11d1daf00b066ea210 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=BCrgen=20Kellerer?= Date: Sat, 17 Aug 2019 22:33:42 +0200 Subject: [PATCH] AcmeClient: Fixed not detecting remote side closing connection. --- .../mvc/app/library/OPNsense/AcmeClient/SftpClient.php | 4 ++-- .../app/library/OPNsense/AcmeClient/SftpUploader.php | 10 +++++++--- .../scripts/OPNsense/AcmeClient/upload_sftp.php | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpClient.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpClient.php index e3ba387ec..0292fa166 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpClient.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpClient.php @@ -62,7 +62,7 @@ class SftpClient public function connected(): ?array { - return $this->process + return $this->process && $this->process->isRunning() ? $this->connection_info : null; } @@ -109,7 +109,7 @@ class SftpClient // Creating the sftp process if ($this->process = Process::open($cmd)) { $this->processAvailableInput(self::CONNECT_REPLY_TIMEOUT, 1, null, 0.75); - if ($error = $this->lastError()) { + if (($error = $this->lastError()) || !$this->process->isRunning()) { Utils::log()->error("Failed connecting to '$host' (user: '$username')", $error); return false; } diff --git a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpUploader.php b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpUploader.php index 83d072cd4..b56bc2411 100644 --- a/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpUploader.php +++ b/security/acme-client/src/opnsense/mvc/app/library/OPNsense/AcmeClient/SftpUploader.php @@ -140,9 +140,6 @@ class SftpUploader */ public function upload(): int { - $connection = Utils::requireThat($this->sftp->connected(), "The sftp client must be connected"); - $username = $connection["user"]; - // Correct state when we are restarted after an error if ($this->current_file) { // Restore the remote path to where we originally have been. @@ -182,6 +179,12 @@ class SftpUploader $this->sftp->clearError(); try { + $connection = $this->sftp->connected(); + if (!$connection) { + Utils::log()->error("The sftp client is not connected, upload stopped."); + return self::UPLOAD_ERROR; + } + // Changing remote directory if required. if (($target_dir = dirname($file["target"])) !== $remote_path) { @@ -229,6 +232,7 @@ class SftpUploader } // Preparing upload + $username = $connection["user"]; $remote_filename = basename((empty($file["target"]) ? $local_file : $file["target"])); $remote_file = $remote_files[$remote_filename] ?: ["type" => "-", "owner" => $username]; $remote_is_file = $remote_file["type"] === "-"; diff --git a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/upload_sftp.php b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/upload_sftp.php index 7e205490a..aa86faa3c 100644 --- a/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/upload_sftp.php +++ b/security/acme-client/src/opnsense/scripts/OPNsense/AcmeClient/upload_sftp.php @@ -179,7 +179,7 @@ function commandTestConnection(array &$options): int $result["actions"][] = "connected"; $result["remote"] = array_merge($sftp->connected(), ["path" => $sftp->pwd()]); } else { - $result = array_merge($result, $error); + $result = array_merge($result, ($error ?: [])); } // Testing file upload