diff --git a/appinfo/info.xml b/appinfo/info.xml index 51cc1682..0f63140e 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -11,7 +11,7 @@ - 1.3.0 + 1.3.1 agpl Robin Appelman NotifyPush diff --git a/lib/Controller/TestController.php b/lib/Controller/TestController.php index 4bc12b17..aa17b8ef 100644 --- a/lib/Controller/TestController.php +++ b/lib/Controller/TestController.php @@ -10,11 +10,12 @@ use OC\AppFramework\Http\Request; use OCA\NotifyPush\Queue\IQueue; -use OCA\NotifyPush\Queue\RedisQueue; use OCP\App\IAppManager; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataDisplayResponse; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Http\Response; use OCP\IAppConfig; use OCP\IRequest; @@ -33,10 +34,17 @@ public function __construct( * @PublicPage * @NoCSRFRequired */ - public function cookie(): DataResponse { + public function cookie(): Response { // starting with 32, the app config does some internal caching // that interferes with the quick set+get from this test. $this->appConfig->clearCache(); + + $expected = $this->queue->get('test-token'); + $token = $this->request->getHeader('token'); + if ($expected !== $token) { + return new Response(Http::STATUS_FORBIDDEN); + } + return new DataResponse($this->appConfig->getValueInt('notify_push', 'cookie')); } @@ -45,12 +53,16 @@ public function cookie(): DataResponse { * @PublicPage * @NoCSRFRequired */ - public function remote(): DataDisplayResponse { - if ($this->queue instanceof RedisQueue) { - if ($this->request instanceof Request) { - $this->queue->getConnection()->set('notify_push_forwarded_header', $this->request->getHeader('x-forwarded-for')); - $this->queue->getConnection()->set('notify_push_remote', $this->request->server['REMOTE_ADDR']); - } + public function remote(): Response { + $expected = $this->queue->get('test-token'); + $token = $this->request->getHeader('token'); + if ($expected !== $token) { + return new Response(Http::STATUS_FORBIDDEN); + } + + if ($this->request instanceof Request) { + $this->queue->set('notify_push_forwarded_header', $this->request->getHeader('x-forwarded-for')); + $this->queue->set('notify_push_remote', $this->request->server['REMOTE_ADDR']); } return new DataDisplayResponse($this->request->getRemoteAddress()); } @@ -65,8 +77,6 @@ public function remote(): DataDisplayResponse { * @return void */ public function version(): void { - if ($this->queue instanceof RedisQueue) { - $this->queue->getConnection()->set('notify_push_app_version', $this->appManager->getAppVersion('notify_push')); - } + $this->queue->set('notify_push_app_version', $this->appManager->getAppVersion('notify_push')); } } diff --git a/lib/Queue/IQueue.php b/lib/Queue/IQueue.php index 3a4343e1..af932356 100644 --- a/lib/Queue/IQueue.php +++ b/lib/Queue/IQueue.php @@ -10,9 +10,17 @@ interface IQueue { /** - * @param string $channel * @param mixed $message - * @return void */ - public function push(string $channel, $message); + public function push(string $channel, $message): void; + + /** + * @param mixed $value + */ + public function set(string $key, $value): void; + + /** + * @return mixed + */ + public function get(string $key); } diff --git a/lib/Queue/NullQueue.php b/lib/Queue/NullQueue.php index a22e4408..d5c037bb 100644 --- a/lib/Queue/NullQueue.php +++ b/lib/Queue/NullQueue.php @@ -9,10 +9,19 @@ namespace OCA\NotifyPush\Queue; class NullQueue implements IQueue { - /** - * @return void - */ - public function push(string $channel, $message) { + #[\Override] + public function push(string $channel, $message): void { // noop } + + #[\Override] + public function set(string $key, $value): void { + // noop + } + + #[\Override] + public function get(string $key) { + return null; + } + } diff --git a/lib/Queue/RedisQueue.php b/lib/Queue/RedisQueue.php index 9c09d403..84f5ef79 100644 --- a/lib/Queue/RedisQueue.php +++ b/lib/Queue/RedisQueue.php @@ -18,7 +18,8 @@ public function __construct($redis) { $this->redis = $redis; } - public function push(string $channel, $message) { + #[\Override] + public function push(string $channel, $message): void { $this->redis->publish($channel, json_encode($message)); } @@ -28,4 +29,14 @@ public function push(string $channel, $message) { public function getConnection() { return $this->redis; } + + #[\Override] + public function set(string $key, $value): void { + $this->redis->set($key, $value); + } + + #[\Override] + public function get(string $key) { + return $this->redis->get($key); + } } diff --git a/lib/SelfTest.php b/lib/SelfTest.php index f6a27f27..3c3e4e7b 100644 --- a/lib/SelfTest.php +++ b/lib/SelfTest.php @@ -12,10 +12,12 @@ use OCA\NotifyPush\Queue\IQueue; use OCA\NotifyPush\Queue\RedisQueue; use OCP\App\IAppManager; +use OCP\Http\Client\IClient; use OCP\Http\Client\IClientService; use OCP\IAppConfig; use OCP\IConfig; use OCP\IDBConnection; +use OCP\Security\ISecureRandom; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\HttpFoundation\IpUtils; @@ -23,8 +25,9 @@ class SelfTest { public const ERROR_OTHER = 1; public const ERROR_TRUSTED_PROXY = 2; - private $client; - private $cookie; + private IClient $client; + private int $cookie; + private string $token; public function __construct( IClientService $clientService, @@ -33,9 +36,15 @@ public function __construct( private IQueue $queue, private IDBConnection $connection, private IAppManager $appManager, + private ISecureRandom $random, ) { $this->client = $clientService->newClient(); $this->cookie = rand(1, (int)pow(2, 30)); + $this->token = $this->random->generate(32); + } + + private function getHttpOpts(): array { + return ['nextcloud' => ['allow_local_address' => true], 'verify' => false, 'headers' => ['token' => $this->token]]; } public function test(string $server, OutputInterface $output, bool $ignoreProxyError = false): int { @@ -56,11 +65,12 @@ public function test(string $server, OutputInterface $output, bool $ignoreProxyE $output->writeln('🗴 push server URL is set to localhost, the push server will not be reachable from other machines'); } + $this->queue->getConnection()->set('test-token', $this->token); $this->queue->push('notify_test_cookie', $this->cookie); $this->appConfig->setValueInt('notify_push', 'cookie', $this->cookie); try { - $retrievedCookie = (int)$this->client->get($server . '/test/cookie', ['nextcloud' => ['allow_local_address' => true], 'verify' => false])->getBody(); + $retrievedCookie = (int)$this->client->get($server . '/test/cookie', $this->getHttpOpts())->getBody(); } catch (\Exception $e) { $msg = $e->getMessage(); $output->writeln("🗴 can't connect to push server: $msg"); @@ -80,7 +90,7 @@ public function test(string $server, OutputInterface $output, bool $ignoreProxyE // If no admin user was created during the installation, there are no oc_filecache and oc_mounts entries yet, so this check has to be skipped. if ($storageId !== null) { try { - $retrievedCount = (int)$this->client->get($server . '/test/mapping/' . $storageId, ['nextcloud' => ['allow_local_address' => true], 'verify' => false])->getBody(); + $retrievedCount = (int)$this->client->get($server . '/test/mapping/' . $storageId, $this->getHttpOpts())->getBody(); } catch (\Exception $e) { $msg = $e->getMessage(); $output->writeln("🗴 can't connect to push server: $msg"); @@ -97,7 +107,7 @@ public function test(string $server, OutputInterface $output, bool $ignoreProxyE // test if the push server can reach nextcloud by having it request the cookie try { - $response = $this->client->get($server . '/test/reverse_cookie', ['nextcloud' => ['allow_local_address' => true], 'verify' => false])->getBody(); + $response = $this->client->get($server . '/test/reverse_cookie', $this->getHttpOpts())->getBody(); $retrievedCookie = (int)$response; if ($this->cookie === $retrievedCookie) { @@ -117,7 +127,7 @@ public function test(string $server, OutputInterface $output, bool $ignoreProxyE // test that the push server is a trusted proxy try { - $resolvedRemote = $this->client->get($server . '/test/remote/1.2.3.4', ['nextcloud' => ['allow_local_address' => true], 'verify' => false])->getBody(); + $resolvedRemote = $this->client->get($server . '/test/remote/1.2.3.4', $this->getHttpOpts())->getBody(); } catch (\Exception $e) { $msg = $e->getMessage(); $output->writeln("🗴 can't connect to push server: $msg"); @@ -185,7 +195,7 @@ public function test(string $server, OutputInterface $output, bool $ignoreProxyE // test that the binary is up to date try { $this->queue->getConnection()->del('notify_push_version'); - $response = $this->client->post($server . '/test/version', ['nextcloud' => ['allow_local_address' => true], 'verify' => false]); + $response = $this->client->post($server . '/test/version', $this->getHttpOpts()); if ($response === 'error') { $output->writeln('🗴 failed to get binary version, check the push server output for more information'); return self::ERROR_OTHER; diff --git a/src/error.rs b/src/error.rs index d88ca5a0..b4364e45 100644 --- a/src/error.rs +++ b/src/error.rs @@ -2,7 +2,7 @@ * SPDX-FileCopyrightText: 2022 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ - + use flexi_logger::FlexiLoggerError; use miette::Diagnostic; use redis::RedisError; @@ -10,6 +10,7 @@ use reqwest::StatusCode; use std::net::AddrParseError; use std::num::ParseIntError; use thiserror::Error; +use warp::reject::Reject; #[derive(Debug, Error, Diagnostic)] pub enum Error { @@ -36,6 +37,8 @@ pub enum Error { SystemD(#[from] std::io::Error), } +impl Reject for Error {} + #[derive(Debug, Error, Diagnostic)] pub enum NextCloudError { #[error(transparent)] @@ -68,6 +71,8 @@ pub enum DatabaseError { #[derive(Debug, Error, Diagnostic)] pub enum SelfTestError { + #[error("Invalid token")] + Token, #[error("Failed to test database access: {0}")] Database(#[from] DatabaseError), #[error("Failed to test redis access: {0}")] diff --git a/src/lib.rs b/src/lib.rs index 24743d7b..e0a8e9bc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,7 +2,6 @@ * SPDX-FileCopyrightText: 2021 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ - use crate::config::{Bind, Config, TlsConfig}; use crate::connection::{handle_user_socket, ActiveConnections, ConnectionOptions}; pub use crate::error::Error; @@ -36,7 +35,8 @@ use tokio::sync::{broadcast, oneshot}; use tokio::time::sleep; use tokio_stream::wrappers::UnixListenerStream; use warp::filters::addr::remote; -use warp::{Filter, Reply}; +use warp::reject::custom; +use warp::{Filter, Rejection, Reply}; use warp_real_ip::get_forwarded_for; pub mod config; @@ -268,6 +268,7 @@ pub fn serve( let cookie_test = warp::path!("test" / "cookie") .and(app.clone()) + .and(test_token(app.clone())) .map(|app: Arc| { let cookie = app.test_cookie.load(Ordering::SeqCst); log::debug!("current test cookie is {cookie}"); @@ -276,8 +277,10 @@ pub fn serve( let reverse_cookie_test = warp::path!("test" / "reverse_cookie") .and(app.clone()) - .and_then(|app: Arc| async move { - let response = match app.nc_client.get_test_cookie().await { + .and(test_token(app.clone())) + .and(warp::header("token")) + .and_then(|app: Arc, token: String| async move { + let response = match app.nc_client.get_test_cookie(&token).await { Ok(cookie) => { log::debug!("got remote test cookie {cookie}"); cookie.to_string() @@ -293,6 +296,7 @@ pub fn serve( let mapping_test = warp::path!("test" / "mapping" / u32) .and(app.clone()) + .and(test_token(app.clone())) .and_then(|storage_id: u32, app: Arc| async move { let access = app .storage_mapping @@ -312,10 +316,12 @@ pub fn serve( let remote_test = warp::path!("test" / "remote" / IpAddr) .and(app.clone()) - .and_then(|remote: IpAddr, app: Arc| async move { + .and(test_token(app.clone())) + .and(warp::header("token")) + .and_then(|remote: IpAddr, app: Arc, token: String| async move { let result = app .nc_client - .test_set_remote(remote) + .test_set_remote(remote, &token) .await .map(|remote| remote.to_string()) .unwrap_or_else(|e| e.to_string()); @@ -325,7 +331,8 @@ pub fn serve( let version = warp::path!("test" / "version") .and(warp::post()) - .and(app) + .and(app.clone()) + .and(test_token(app)) .and_then(|app: Arc| async move { Result::<_, Infallible>::Ok(match app.redis.connect().await { Ok(mut client) => { @@ -451,3 +458,23 @@ pub async fn listen(app: Arc) -> Result<()> { ping_handle.abort(); Ok(()) } + +fn test_token( + app: impl Filter,), Error = Infallible> + Clone, +) -> impl Filter + Clone { + app.and(warp::header("token")) + .and_then(|app: Arc, token: String| async move { + validate_token(&app, &token).await.map_err(custom) + }) + .untuple_one() +} + +async fn validate_token(app: &App, token: &str) -> Result<()> { + let mut redis = app.redis.connect().await?; + let expected = redis.get("test-token").await?; + if !token.is_empty() && token == expected { + Ok(()) + } else { + Err(SelfTestError::Token.into()) + } +} diff --git a/src/nc.rs b/src/nc.rs index 30337d87..8d2a5b7b 100644 --- a/src/nc.rs +++ b/src/nc.rs @@ -75,13 +75,14 @@ impl Client { .map_err(NextCloudError::NextcloudConnect) } - pub async fn get_test_cookie(&self) -> Result { + pub async fn get_test_cookie(&self, token: &str) -> Result { let response = self .http .get( self.base_url .join("index.php/apps/notify_push/test/cookie")?, ) + .header(HeaderName::from_static("token"), token.to_string()) .send() .await?; let status = response.status(); @@ -101,7 +102,11 @@ impl Client { } } - pub async fn test_set_remote(&self, addr: IpAddr) -> Result { + pub async fn test_set_remote( + &self, + addr: IpAddr, + token: &str, + ) -> Result { self.http .get( self.base_url @@ -109,6 +114,7 @@ impl Client { .map_err(NextCloudError::from)?, ) .header(&X_FORWARDED_FOR, addr.to_string()) + .header(HeaderName::from_static("token"), token.to_string()) .send() .await? .text()