Merge pull request #11118 from nextcloud/fix/smime/encoding

st3iny · web-flow · commit 27e1ae5319a4 · 2025-05-08T16:48:44.000+02:00
fix(smime): use proper binary encoding when signing messages
diff --git a/REUSE.toml b/REUSE.toml
@@ -108,7 +108,7 @@ SPDX-FileCopyrightText = "2024 Nextcloud GmbH and Nextcloud contributors"
 SPDX-License-Identifier = "AGPL-3.0-or-later"
 
 [[annotations]]
-path = ["tests/data/imip/*"]
+path = ["tests/data/imip/*", "tests/data/html-with-signature.txt"]
 precedence = "aggregate"
 SPDX-FileCopyrightText = "2025 Nextcloud GmbH and Nextcloud contributors"
 SPDX-License-Identifier = "AGPL-3.0-or-later"
diff --git a/lib/Service/SmimeService.php b/lib/Service/SmimeService.php
@@ -371,15 +371,14 @@ public function signMimePart(Horde_Mime_Part $part,
 		file_put_contents($inPath, $part->toString([
 			'canonical' => true,
 			'headers' => true,
+			'encode' => Horde_Mime_Part::ENCODE_8BIT,
 		]));
-		if (!openssl_pkcs7_sign($inPath, $outPath, $decryptedCertificate, $decryptedKey, null, PKCS7_DETACHED, $decryptedCertificateFile)) {
+		if (!openssl_pkcs7_sign($inPath, $outPath, $decryptedCertificate, $decryptedKey, null, PKCS7_DETACHED | PKCS7_BINARY, $decryptedCertificateFile)) {
 			throw new SmimeSignException('Failed to sign MIME part');
 		}
 
 		try {
-			$parsedPart = Horde_Mime_Part::parseMessage(file_get_contents($outPath), [
-				'forcemime' => true,
-			]);
+			$parsedPart = Horde_Mime_Part::parseMessage(file_get_contents($outPath));
 		} catch (Horde_Mime_Exception $e) {
 			throw new SmimeSignException(
 				'Failed to parse signed MIME part: ' . $e->getMessage(),
diff --git a/tests/Unit/Service/SmimeServiceTest.php b/tests/Unit/Service/SmimeServiceTest.php
@@ -457,4 +457,34 @@ public function testParseCertificate(SmimeCertificate $certificate,
 			$this->smimeService->parseCertificate($certificate->getCertificate()),
 		);
 	}
+
+	public function testSignMimePart(): void {
+		$plainPart = Horde_Mime_Part::parseMessage(file_get_contents(__DIR__ . '/../../data/html-with-signature.txt'));
+		$certificate = $this->getTestCertificate('debug@imap.localhost');
+
+		$this->crypto
+			->method('decrypt')
+			->will($this->returnArgument(0));
+		$this->tempManager
+			->method('getTemporaryFile')
+			->willReturnCallback(function () {
+				return $this->createTempFile();
+			});
+
+		// Can't compare to serialized part as the boundaries inside the signed MIME message are
+		// generated randomly each time. => Verify the signed part instead.
+		$actualPart = $this->smimeService->signMimePart($plainPart, $certificate);
+		$messageTemp = $this->createTempFile();
+		file_put_contents($messageTemp, $actualPart->toString([
+			'canonical' => true,
+			'headers' => true,
+			'encode' => Horde_Mime_Part::ENCODE_8BIT,
+		]));
+		$this->assertTrue(openssl_pkcs7_verify(
+			$messageTemp,
+			0,
+			null,
+			[__DIR__ . '/../../data/smime-certs/imap.localhost.ca.crt'],
+		));
+	}
 }
diff --git a/tests/data/html-with-signature.txt b/tests/data/html-with-signature.txt
@@ -0,0 +1,27 @@
+Content-Type: multipart/alternative; boundary="=_OCjivB0eKOELIVVHw6KUaJG"
+MIME-Version: 1.0
+Content-Transfer-Encoding: 8bit
+
+This message is in MIME format.
+
+--=_OCjivB0eKOELIVVHw6KUaJG
+Content-Type: text/plain; charset=utf-8
+Content-Transfer-Encoding: 8bit
+
+This is a test message ...
+ 
+... with empty lines.
+--  
+This is a signature.
+ 
+With rich text.
+--=_OCjivB0eKOELIVVHw6KUaJG
+Content-Type: text/html; charset=utf-8
+Content-Transfer-Encoding: 8bit
+
+<!DOCTYPE html>
+<html><meta http-equiv="content-type" content="text/html; charset=UTF-8"><body>
+<p style="margin:0;">This is a test message ...</p><p style="margin:0;"> </p><p style="margin:0;">... with empty lines.</p><div class="signature">-- <p style="margin:0;"> </p><p style="margin:0;">This is a signature.</p><p style="margin:0;"> </p><p style="margin:0;"><strong>With</strong> <i>rich</i> <span style="color:hsl(0,75%,60%);">text</span>.</p></div>
+</body></html>
+
+--=_OCjivB0eKOELIVVHw6KUaJG--
