Skip to content

Commit b346f0b

Browse files
ChristophWurstChristophWurst
committed
fix: use HMAC for proxied HTML iframe content
AI-Assisted: OpenCode (Claude Haiku 4.5) Signed-off-by: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com>
1 parent cc00d75 commit b346f0b

10 files changed

Lines changed: 413 additions & 74 deletions

File tree

‎lib/Controller/ProxyController.php‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,15 @@
1111
namespace OCA\Mail\Controller;
1212

1313
use Exception;
14+
use OCA\Mail\Html\ProxyHmacGenerator;
1415
use OCA\Mail\Http\ProxyDownloadResponse;
16+
use OCA\Mail\Service\MailManager;
1517
use OCP\AppFramework\Controller;
18+
use OCP\AppFramework\Db\DoesNotExistException;
19+
use OCP\AppFramework\Http;
1620
use OCP\AppFramework\Http\Attribute\OpenAPI;
1721
use OCP\AppFramework\Http\Attribute\UserRateLimit;
22+
use OCP\AppFramework\Http\Response;
1823
use OCP\AppFramework\Http\TemplateResponse;
1924
use OCP\Http\Client\IClientService;
2025
use OCP\Http\Client\LocalServerException;
@@ -24,26 +29,36 @@
2429
use Psr\Http\Client\ClientExceptionInterface;
2530
use Psr\Log\LoggerInterface;
2631
use function file_get_contents;
32+
use function hash_equals;
2733

2834
#[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)]
2935
class ProxyController extends Controller {
3036
private IURLGenerator $urlGenerator;
3137
private ISession $session;
3238
private IClientService $clientService;
3339
private LoggerInterface $logger;
40+
private ProxyHmacGenerator $hmacGenerator;
41+
private MailManager $mailManager;
42+
private ?string $userId;
3443

3544
public function __construct(string $appName,
3645
IRequest $request,
3746
IURLGenerator $urlGenerator,
3847
ISession $session,
3948
IClientService $clientService,
40-
LoggerInterface $logger) {
49+
ProxyHmacGenerator $hmacGenerator,
50+
LoggerInterface $logger,
51+
MailManager $mailManager,
52+
?string $userId) {
4153
parent::__construct($appName, $request);
4254
$this->request = $request;
4355
$this->urlGenerator = $urlGenerator;
4456
$this->session = $session;
4557
$this->clientService = $clientService;
4658
$this->logger = $logger;
59+
$this->hmacGenerator = $hmacGenerator;
60+
$this->mailManager = $mailManager;
61+
$this->userId = $userId;
4762
}
4863

4964
/**
@@ -89,10 +104,10 @@ public function redirect(string $src): TemplateResponse {
89104
* The caching should also already happen in a cronjob so that the sender of the
90105
* mail does not know whether the mail has been opened.
91106
*
92-
* @return ProxyDownloadResponse
107+
* @return Response|ProxyDownloadResponse
93108
*/
94109
#[UserRateLimit(limit: 50, period: 60)]
95-
public function proxy(string $src): ProxyDownloadResponse {
110+
public function proxy(string $src, ?int $id, ?string $hmac): Response {
96111
// close the session to allow parallel downloads
97112
$this->session->close();
98113

@@ -102,6 +117,20 @@ public function proxy(string $src): ProxyDownloadResponse {
102117
return new ProxyDownloadResponse($content, $src, 'application/octet-stream');
103118
}
104119

120+
// HMAC check
121+
if ($this->userId === null || $id === null || $hmac === null) {
122+
return new Response(Http::STATUS_BAD_REQUEST);
123+
}
124+
try {
125+
$this->mailManager->getMessage($this->userId, $id);
126+
} catch (DoesNotExistException $e) {
127+
return new Response(Http::STATUS_BAD_REQUEST);
128+
}
129+
if (!hash_equals($this->hmacGenerator->generate($id, $src), $hmac)) {
130+
$this->logger->info('Proxied email content blocked due to invalid HMAC');
131+
return new Response(Http::STATUS_UNAUTHORIZED);
132+
}
133+
105134
$client = $this->clientService->newClient();
106135
try {
107136
$response = $client->get($src);

‎lib/Html/ProxyHmacGenerator.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\Mail\Html;
11+
12+
use OCP\IConfig;
13+
use OCP\Security\ICrypto;
14+
15+
class ProxyHmacGenerator {
16+
17+
public function __construct(
18+
private IConfig $config,
19+
private ICrypto $crypto,
20+
) {
21+
}
22+
23+
public function generate(int $id, string $src): string {
24+
return bin2hex($this->crypto->calculateHMAC(
25+
$src,
26+
implode('|', [
27+
$this->config->getSystemValueString('secret'),
28+
$id,
29+
]),
30+
));
31+
}
32+
33+
}

‎lib/Model/IMAPMessage.php‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -349,9 +349,7 @@ public function jsonSerialize() {
349349
* @return string
350350
*/
351351
public function getHtmlBody(int $id): string {
352-
return $this->htmlService->sanitizeHtmlMailBody($this->htmlMessage, [
353-
'id' => $id,
354-
], function ($cid) {
352+
return $this->htmlService->sanitizeHtmlMailBody($this->htmlMessage, $id, function ($cid) {
355353
$match = array_filter($this->inlineAttachments,
356354
static fn ($a) => $a['cid'] === $cid);
357355
$match = array_shift($match);

‎lib/Service/Html.php‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use HTMLPurifier_HTMLDefinition;
1717
use HTMLPurifier_URIDefinition;
1818
use HTMLPurifier_URISchemeRegistry;
19+
use OCA\Mail\Html\ProxyHmacGenerator;
1920
use OCA\Mail\Service\HtmlPurify\CidURIScheme;
2021
use OCA\Mail\Service\HtmlPurify\TransformHTMLLinks;
2122
use OCA\Mail\Service\HtmlPurify\TransformImageSrc;
@@ -39,10 +40,14 @@ class Html {
3940

4041
/** @var IRequest */
4142
private $request;
43+
private ProxyHmacGenerator $hmacGenerator;
4244

43-
public function __construct(IURLGenerator $urlGenerator, IRequest $request) {
45+
public function __construct(IURLGenerator $urlGenerator,
46+
IRequest $request,
47+
ProxyHmacGenerator $hmacGenerator) {
4448
$this->urlGenerator = $urlGenerator;
4549
$this->request = $request;
50+
$this->hmacGenerator = $hmacGenerator;
4651
}
4752

4853
/**
@@ -102,7 +107,7 @@ public function parseMailBody(string $body): array {
102107
];
103108
}
104109

105-
public function sanitizeHtmlMailBody(string $mailBody, array $messageParameters, Closure $mapCidToAttachmentId): string {
110+
public function sanitizeHtmlMailBody(string $mailBody, int $id, Closure $mapCidToAttachmentId): string {
106111
$config = HTMLPurifier_Config::createDefault();
107112

108113
// Append target="_blank" to all link (a) elements
@@ -130,7 +135,16 @@ public function sanitizeHtmlMailBody(string $mailBody, array $messageParameters,
130135

131136
/** @var HTMLPurifier_URIDefinition $uri */
132137
$uri = $config->getDefinition('URI');
133-
$uri->addFilter(new TransformURLScheme($messageParameters, $mapCidToAttachmentId, $this->urlGenerator, $this->request), $config);
138+
$uri->addFilter(
139+
new TransformURLScheme(
140+
$id,
141+
$mapCidToAttachmentId,
142+
$this->urlGenerator,
143+
$this->request,
144+
$this->hmacGenerator,
145+
),
146+
$config
147+
);
134148

135149
$uriSchemeRegistry = HTMLPurifier_URISchemeRegistry::instance();
136150
$uriSchemeRegistry->register('cid', new CidURIScheme());

‎lib/Service/HtmlPurify/TransformURLScheme.php‎

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use HTMLPurifier_URI;
1616
use HTMLPurifier_URIFilter;
1717
use HTMLPurifier_URIParser;
18+
use OCA\Mail\Html\ProxyHmacGenerator;
1819
use OCP\IRequest;
1920
use OCP\IURLGenerator;
2021

@@ -33,17 +34,20 @@ class TransformURLScheme extends HTMLPurifier_URIFilter {
3334
*/
3435
private $mapCidToAttachmentId;
3536

36-
/** @var array */
37-
private $messageParameters;
37+
private int $id;
3838

39-
public function __construct(array $messageParameters,
39+
private ProxyHmacGenerator $hmacGenerator;
40+
41+
public function __construct(int $id,
4042
Closure $mapCidToAttachmentId,
4143
IURLGenerator $urlGenerator,
42-
IRequest $request) {
43-
$this->messageParameters = $messageParameters;
44+
IRequest $request,
45+
ProxyHmacGenerator $hmacGenerator) {
46+
$this->id = $id;
4447
$this->mapCidToAttachmentId = $mapCidToAttachmentId;
4548
$this->urlGenerator = $urlGenerator;
4649
$this->request = $request;
50+
$this->hmacGenerator = $hmacGenerator;
4751
}
4852

4953
/**
@@ -56,7 +60,6 @@ public function __construct(array $messageParameters,
5660
*/
5761
#[\Override]
5862
public function filter(&$uri, $config, $context) {
59-
6063
if ($uri->scheme === null) {
6164
$uri->scheme = 'https';
6265
}
@@ -71,10 +74,14 @@ public function filter(&$uri, $config, $context) {
7174
if (is_null($attachmentId)) {
7275
return true;
7376
}
74-
$this->messageParameters['attachmentId'] = $attachmentId;
7577

76-
$imgUrl = $this->urlGenerator->linkToRouteAbsolute('mail.messages.downloadAttachment',
77-
$this->messageParameters);
78+
$imgUrl = $this->urlGenerator->linkToRouteAbsolute(
79+
'mail.messages.downloadAttachment',
80+
[
81+
'id' => $this->id,
82+
'attachmentId' => $attachmentId,
83+
],
84+
);
7885
$parser = new HTMLPurifier_URIParser();
7986
$uri = $parser->parse($imgUrl);
8087
}
@@ -88,23 +95,23 @@ public function filter(&$uri, $config, $context) {
8895
* @return HTMLPurifier_URI
8996
*/
9097
private function filterHttpFtp(&$uri, $context) {
91-
$originalURL = urlencode($uri->scheme . '://' . $uri->host);
98+
$originalURL = $uri->scheme . '://' . $uri->host;
9299

93100
// Add the port if it's not a default port
94101
if ($uri->port !== null
95102
&& !($uri->scheme === 'http' && $uri->port === 80)
96103
&& !($uri->scheme === 'https' && $uri->port === 443)
97104
&& !($uri->scheme === 'ftp' && $uri->port === 21)) {
98-
$originalURL = $originalURL . urlencode(':' . $uri->port);
105+
$originalURL = $originalURL . ':' . $uri->port;
99106
}
100107

101-
$originalURL = $originalURL . urlencode($uri->path);
108+
$originalURL = $originalURL . $uri->path;
102109

103110
if ($uri->query !== null) {
104-
$originalURL = $originalURL . urlencode('?' . $uri->query);
111+
$originalURL = $originalURL . '?' . $uri->query;
105112
}
106113
if ($uri->fragment !== null) {
107-
$originalURL = $originalURL . urlencode('#' . $uri->fragment);
114+
$originalURL = $originalURL . '#' . $uri->fragment;
108115
}
109116

110117
// Get the HTML attribute
@@ -116,12 +123,19 @@ private function filterHttpFtp(&$uri, $context) {
116123
return $uri;
117124
}
118125

126+
$proxyUrl = $this->urlGenerator->linkToRoute('mail.proxy.proxy', [
127+
'id' => $this->id,
128+
'hmac' => $this->hmacGenerator->generate($this->id, $originalURL),
129+
'src' => $originalURL
130+
]);
131+
$parsedProxyUrl = parse_url($proxyUrl);
132+
/** @var array{path: string, query: string} $parsedProxyUrl */
119133
return new \HTMLPurifier_URI(
120134
$this->request->getServerProtocol(),
121135
null, $this->request->getServerHost(),
122136
null,
123-
$this->urlGenerator->linkToRoute('mail.proxy.proxy'),
124-
'src=' . $originalURL . '&requesttoken=' . \OCP\Server::get(\OCP\ISession::class)->get('requesttoken'),
137+
$parsedProxyUrl['path'],
138+
$parsedProxyUrl['query'],
125139
null
126140
);
127141
}

0 commit comments

Comments
 (0)