Merge pull request #9456 from nextcloud/techdebt/noid/make-checksum-verification-generic

techdebt(api): Make the checksum verification generic so it can be re…

2 files changed, 18 insertions, 15 deletions

diff --git a/lib/Controller/RoomController.php b/lib/Controller/RoomController.php
index 920624aa6..dae1542ce 100644
--- a/lib/Controller/RoomController.php
+++ b/lib/Controller/RoomController.php
@@ -50,11 +50,11 @@ use OCA\Talk\Model\Session;
 use OCA\Talk\Participant;
 use OCA\Talk\Room;
 use OCA\Talk\Service\BreakoutRoomService;
+use OCA\Talk\Service\ChecksumVerificationService;
 use OCA\Talk\Service\ParticipantService;
 use OCA\Talk\Service\RoomFormatter;
 use OCA\Talk\Service\RoomService;
 use OCA\Talk\Service\SessionService;
-use OCA\Talk\Service\SIPBridgeService;
 use OCA\Talk\TalkSession;
 use OCA\Talk\Webinary;
 use OCP\App\IAppManager;
@@ -97,7 +97,7 @@ class RoomController extends AEnvironmentAwareController {
 	protected IUserStatusManager $statusManager;
 	protected IEventDispatcher $dispatcher;
 	protected ITimeFactory $timeFactory;
-	protected SIPBridgeService $SIPBridgeService;
+	protected ChecksumVerificationService $checksumVerificationService;
 	protected RoomFormatter $roomFormatter;
 	protected IConfig $config;
 	protected Config $talkConfig;
@@ -123,7 +123,7 @@ class RoomController extends AEnvironmentAwareController {
 		IUserStatusManager $statusManager,
 		IEventDispatcher $dispatcher,
 		ITimeFactory $timeFactory,
-		SIPBridgeService $SIPBridgeService,
+		ChecksumVerificationService $checksumVerificationService,
 		RoomFormatter $roomFormatter,
 		IConfig $config,
 		Config $talkConfig,
@@ -146,7 +146,7 @@ class RoomController extends AEnvironmentAwareController {
 		$this->statusManager = $statusManager;
 		$this->dispatcher = $dispatcher;
 		$this->timeFactory = $timeFactory;
-		$this->SIPBridgeService = $SIPBridgeService;
+		$this->checksumVerificationService = $checksumVerificationService;
 		$this->config = $config;
 		$this->talkConfig = $talkConfig;
 		$this->cloudIdManager = $cloudIdManager;
@@ -385,7 +385,7 @@ class RoomController extends AEnvironmentAwareController {
 		$random = $this->request->getHeader('TALK_SIPBRIDGE_RANDOM');
 		$checksum = $this->request->getHeader('TALK_SIPBRIDGE_CHECKSUM');
 		$secret = $this->talkConfig->getSIPSharedSecret();
-		return $this->SIPBridgeService->validateSIPBridgeRequest($random, $checksum, $secret, $token);
+		return $this->checksumVerificationService->validateRequest($random, $checksum, $secret, $token);
 	}
 
 	protected function formatRoom(Room $room, ?Participant $currentParticipant, ?array $statuses = null, bool $isSIPBridgeRequest = false, bool $isListingBreakoutRooms = false): array {
diff --git a/lib/Service/SIPBridgeService.php b/lib/Service/ChecksumVerificationService.php
index 3695606ea..78902fbbb 100644
--- a/lib/Service/SIPBridgeService.php
+++ b/lib/Service/ChecksumVerificationService.php
@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /*
  * @copyright Copyright (c) 2022 Vitor Mattos <vitor@php.rio>
  *
@@ -24,24 +26,24 @@ namespace OCA\Talk\Service;
 
 use OCA\Talk\Exceptions\UnauthorizedException;
 
-class SIPBridgeService {
+class ChecksumVerificationService {
 	/**
 	 * Check if the current request is coming from an allowed backend.
 	 *
-	 * The SIP bridge is sending the custom header "Talk-SIPBridge-Random"
+	 * The backend servers are sending custom headers "Talk-{{FEATURE}}-Random"
 	 * containing at least 32 bytes random data, and the header
-	 * "Talk-SIPBridge-Checksum", which is the SHA256-HMAC of the random data
+	 * "Talk-{{FEATURE}}-Checksum", which is the SHA256-HMAC of the random data
 	 * and the body of the request, calculated with the shared secret from the
 	 * configuration.
 	 *
 	 * @param string $random
 	 * @param string $checksum
 	 * @param string $secret
-	 * @param string $token
-	 * @return bool True if the request is from the SIP bridge and valid, false if not from SIP bridge
-	 * @throws UnauthorizedException when the request tried to sign as SIP bridge but is not valid
+	 * @param string $data
+	 * @return bool True if the request is from the backend and valid, false if not from SIP bridge
+	 * @throws UnauthorizedException when the request tried to authenticate as backend but is not valid
 	 */
-	public function validateSIPBridgeRequest(string $random, string $checksum, string $secret, string $token): bool {
+	public function validateRequest(string $random, string $checksum, string $secret, string $data): bool {
 		if ($random === '' && $checksum === '') {
 			return false;
 		}
@@ -50,14 +52,15 @@ class SIPBridgeService {
 			throw new UnauthorizedException('Invalid random provided');
 		}
 
-		if (empty($checksum)) {
+		if ($checksum === '') {
 			throw new UnauthorizedException('Invalid checksum provided');
 		}
 
-		if (empty($secret)) {
+		if ($secret === '') {
 			throw new UnauthorizedException('No shared SIP secret provided');
 		}
-		$hash = hash_hmac('sha256', $random . $token, $secret);
+
+		$hash = hash_hmac('sha256', $random . $data, $secret);
 
 		if (hash_equals($hash, strtolower($checksum))) {
 			return true;