Improved error handling on PGP and S/MIME decrypt

dev/External/ko.js

@@ -66,8 +66,7 @@ Object.assign(ko.bindingHandlers, {
 		},
 		update: (element, fValueAccessor) => {
 			let value = ko.unwrap(fValueAccessor());
-			value = isFunction(value) ? value() : value;
+			errorTip(element, isFunction(value) ? value() : value);
-			errorTip(element, value);
 		}
 	},
 

dev/Stores/User/GnuPG.js

@@ -204,7 +204,7 @@ export const GnuPGUserStore = new class {
 	}
 
 	async verify(message) {
-		let data = message.pgpSigned(); // { bodyPartId: "1", sigPartId: "2", micAlg: "pgp-sha256" }
+		let data = message.pgpSigned(); // { partId: "1", sigPartId: "2", micAlg: "pgp-sha256" }
 		if (data) {
 			data = { ...data }; // clone
 //			const sender = message.from[0].email;

dev/Stores/User/OpenPGP.js

@@ -229,7 +229,7 @@ export const OpenPGPUserStore = new class {
 	 * https://docs.openpgpjs.org/#sign-and-verify-cleartext-messages
 	 */
 	async verify(message) {
-		const data = message.pgpSigned(), // { bodyPartId: "1", sigPartId: "2", micAlg: "pgp-sha256" }
+		const data = message.pgpSigned(), // { partId: "1", sigPartId: "2", micAlg: "pgp-sha256" }
 			publicKey = this.publicKeys().find(key => key.emails.includes(message.from[0].email));
 		if (data && publicKey) {
 			data.folder = message.folder;

dev/Stores/User/Pgp.js

@@ -108,19 +108,14 @@ export const
 		}
 
 		async verify(message) {
-			const signed = message.pgpSigned();
+			const signed = message.pgpSigned(),
+				sender = message.from[0].email;
 			if (signed) {
-				const sender = message.from[0].email,
+				// OpenPGP only when inline, else we must download the whole message
-					gnupg = GnuPGUserStore.hasPublicKeyForEmails([sender]),
+				if (!signed.sigPartId && OpenPGPUserStore.hasPublicKeyForEmails([sender])) {
-					openpgp = OpenPGPUserStore.hasPublicKeyForEmails([sender]);
-				// Detached signature use GnuPG first, else we must download whole message
-				if (gnupg && signed.sigPartId) {
-					return GnuPGUserStore.verify(message);
-				}
-				if (openpgp) {
 					return OpenPGPUserStore.verify(message);
 				}
-				if (gnupg) {
+				if (GnuPGUserStore.hasPublicKeyForEmails([sender])) {
 					return GnuPGUserStore.verify(message);
 				}
 				// Mailvelope can't

dev/View/User/MailBox/MessageView.js

@@ -570,9 +570,14 @@ export class MailMessageView extends AbstractViewRight {
 	}
 
 	pgpDecrypt() {
-		const oMessage = currentMessage();
+		const oMessage = currentMessage(),
+			data = oMessage.pgpEncrypted();
+		delete data.error;
 		PgpUserStore.decrypt(oMessage).then(result => {
-			if (result) {
+			if (!result) {
+				// TODO: translate
+				throw Error('Decryption failed, canceled or not possible');
+			}
 			oMessage.pgpDecrypted(true);
 			if (result.data) {
 				MimeToMessage(result.data, oMessage);
@@ -585,14 +590,12 @@ export class MailMessageView extends AbstractViewRight {
 					});
 				}
 			}
-			} else {
-				// TODO: translate
-				throw Error('Decryption failed, canceled or not possible');
-			}
 		})
 		.catch(e => {
-			console.error(e)
+			data.error = e.message;
-			alert(e.message);
+		})
+		.finally(() => {
+			oMessage.pgpEncrypted(data);
 		});
 	}
 
@@ -627,16 +630,17 @@ export class MailMessageView extends AbstractViewRight {
 
 	async smimeDecrypt() {
 		const message = currentMessage();
-		let pass, data = message.smimeEncrypted(); // { partId: "1" }
 		const addresses = message.from.concat(message.to, message.cc, message.bcc).map(item => item.email),
-			identity = IdentityUserStore.find(item => addresses.includes(item.email()));
+			identity = IdentityUserStore.find(item => addresses.includes(item.email())),
+			data = message.smimeEncrypted(); // { partId: "1" }
 		if (data && identity) {
-			data = { ...data }; // clone
+			delete data.error;
-			data.folder = message.folder;
+			let pass, params = { ...data }; // clone
-			data.uid = message.uid;
+			params.folder = message.folder;
-//			data.bodyPart = data.bodyPart?.raw;
+			params.uid = message.uid;
-			data.certificate = identity.smimeCertificate();
+//			params.bodyPart = params.bodyPart?.raw;
-			data.privateKey = identity.smimeKey();
+			params.certificate = identity.smimeCertificate();
+			params.privateKey = identity.smimeKey();
 			if (identity.smimeKeyEncrypted()) {
 				pass = await Passphrases.ask(identity,
 					i18n('SMIME/PRIVATE_KEY_OF', {EMAIL: identity.email()}),
@@ -645,15 +649,20 @@ export class MailMessageView extends AbstractViewRight {
 				if (!pass) {
 					return;
 				}
-				data.passphrase = pass?.password;
+				params.passphrase = pass?.password;
 			}
-			Remote.post('SMimeDecryptMessage', null, data).then(response => {
+			Remote.post('SMimeDecryptMessage', null, params).then(response => {
-				if (response?.Result) {
+				if (response?.Result?.data) {
 					message.smimeDecrypted(true);
-					MimeToMessage(response.Result, message);
+					MimeToMessage(response.Result.data, message);
 					message.html() ? message.viewHtml() : message.viewPlain();
 					pass && pass.remember && Passphrases.set(identity, pass.password);
 				}
+			}).catch(e => {
+				data.error = e.message
+			})
+			.finally(() => {
+				message.smimeEncrypted(data);
 			});
 		}
 	}

snappymail/v/0.0.0/app/libraries/MailSo/Mail/Message.php

@@ -341,7 +341,7 @@ class Message implements \JsonSerializable
 					$oMessage->pgpSigned = [
 						// /?/Raw/&q[]=/0/Download/&q[]=/...
 						// /?/Raw/&q[]=/0/View/&q[]=/...
-						'bodyPartId' => $oPart->SubParts()[0]->PartID(),
+						'partId' => $oPart->SubParts()[0]->PartID(),
 						'sigPartId' => $oPart->SubParts()[1]->PartID(),
 						'micAlg' => $oHeaders ? (string) $oHeaders->ParameterValue(MimeHeader::CONTENT_TYPE, 'micalg') : ''
 					];
@@ -357,9 +357,9 @@ class Message implements \JsonSerializable
 				// An empty section specification refers to the entire message, including the header.
 				// But Dovecot does not return it with BODY.PEEK[1], so we also use BODY.PEEK[1.MIME].
 				$sPgpText = \trim(
-					\trim($oFetchResponse->GetFetchValue(FetchType::BODY.'['.$oMessage->pgpSigned['bodyPartId'].'.MIME]'))
+					\trim($oFetchResponse->GetFetchValue(FetchType::BODY.'['.$oMessage->pgpSigned['partId'].'.MIME]'))
 					. "\r\n\r\n"
-					. \trim($oFetchResponse->GetFetchValue(FetchType::BODY.'['.$oMessage->pgpSigned['bodyPartId'].']'))
+					. \trim($oFetchResponse->GetFetchValue(FetchType::BODY.'['.$oMessage->pgpSigned['partId'].']'))
 				);
 				if ($sPgpText) {
 					$oMessage->pgpSigned['body'] = $sPgpText;
@@ -398,7 +398,7 @@ class Message implements \JsonSerializable
 						// Cleartext Signature
 						if (!$oMessage->pgpSigned && \str_contains($sText, '-----BEGIN PGP SIGNED MESSAGE-----')) {
 							$oMessage->pgpSigned = [
-								'bodyPartId' => $oPart->PartID()
+								'partId' => $oPart->PartID()
 							];
 						}
 

snappymail/v/0.0.0/app/libraries/RainLoop/Actions/Pgp.php

@@ -303,7 +303,7 @@ trait Pgp
 		} else {
 			$sFolderName = $this->GetActionParam('folder', '');
 			$iUid = (int) $this->GetActionParam('uid', 0);
-			$sBodyPartId = $this->GetActionParam('bodyPartId', '');
+			$sPartId = $this->GetActionParam('partId', '');
 			$sSigPartId = $this->GetActionParam('sigPartId', '');
 //			$sMicAlg = $this->GetActionParam('micAlg', '');
 
@@ -312,10 +312,10 @@ trait Pgp
 			$oImapClient->FolderExamine($sFolderName);
 
 			$aParts = [
-				FetchType::BODY_PEEK.'['.$sBodyPartId.']',
+				FetchType::BODY_PEEK.'['.$sPartId.']',
 				// An empty section specification refers to the entire message, including the header.
 				// But Dovecot does not return it with BODY.PEEK[1], so we also use BODY.PEEK[1.MIME].
-				FetchType::BODY_PEEK.'['.$sBodyPartId.'.MIME]'
+				FetchType::BODY_PEEK.'['.$sPartId.'.MIME]'
 			];
 			if ($sSigPartId) {
 				$aParts[] = FetchType::BODY_PEEK.'['.$sSigPartId.']';
@@ -323,11 +323,11 @@ trait Pgp
 
 			$oFetchResponse = $oImapClient->Fetch($aParts, $iUid, true)[0];
 
-			$sBodyMime = $oFetchResponse->GetFetchValue(FetchType::BODY.'['.$sBodyPartId.'.MIME]');
+			$sBodyMime = $oFetchResponse->GetFetchValue(FetchType::BODY.'['.$sPartId.'.MIME]');
 			if ($sSigPartId) {
 				$result = [
 					'text' => \preg_replace('/\\r?\\n/su', "\r\n",
-						$sBodyMime . $oFetchResponse->GetFetchValue(FetchType::BODY.'['.$sBodyPartId.']')
+						$sBodyMime . $oFetchResponse->GetFetchValue(FetchType::BODY.'['.$sPartId.']')
 					),
 					'signature' => preg_replace('/[^\x00-\x7F]/', '',
 						$oFetchResponse->GetFetchValue(FetchType::BODY.'['.$sSigPartId.']')
@@ -336,7 +336,7 @@ trait Pgp
 			} else {
 				// clearsigned text
 				$result = [
-					'text' => $oFetchResponse->GetFetchValue(FetchType::BODY.'['.$sBodyPartId.']'),
+					'text' => $oFetchResponse->GetFetchValue(FetchType::BODY.'['.$sPartId.']'),
 					'signature' => ''
 				];
 				$decode = (new \MailSo\Mime\HeaderCollection($sBodyMime))->ValueByName(MimeEnumHeader::CONTENT_TRANSFER_ENCODING);

snappymail/v/0.0.0/app/libraries/RainLoop/Actions/SMime.php

@@ -105,7 +105,7 @@ trait SMime
 		$SMIME->setPrivateKey($sPrivateKey, $oPassphrase);
 		$result = $SMIME->decrypt($sBody);
 
-		return $this->DefaultResponse($result ?: false);
+		return $this->DefaultResponse($result ? ['data' => $result] : false);
 	}
 
 	public function DoSMimeVerifyMessage() : array

snappymail/v/0.0.0/app/libraries/snappymail/gpg/pgp.php

@@ -333,9 +333,6 @@ class PGP extends Base implements \SnappyMail\PGP\PGPInterface
 			}
 		}
 		if (!$fingerprint) {
-			if (!empty($result['errors'])) {
-				\SnappyMail\Log::error('GPG', \implode("\n\t", $result['errors']));
-			}
 			return false;
 		}
 
@@ -394,10 +391,6 @@ class PGP extends Base implements \SnappyMail\PGP\PGPInterface
 			}
 		}
 
-		if (!empty($result['errors'][0])) {
-			\SnappyMail\Log::warning('GPG', $result['errors'][0]);
-		}
-
 		return false;
 	}
 
@@ -809,12 +802,11 @@ class PGP extends Base implements \SnappyMail\PGP\PGPInterface
 		$this->_debug('BEGIN DETECT MESSAGE KEY IDs');
 		$this->setInput($data);
 //		$_ENV['PINENTRY_USER_DATA'] = null;
-		$result = $this->exec(['--decrypt','--skip-verify']);
+		$result = $this->exec(['--decrypt','--skip-verify'], false);
 		$info = [
 			'ENC_TO' => [],
 //			'KEY_CONSIDERED' => [],
 //			'NO_SECKEY' => [],
-//			'errors' => $result['errors']
 		];
 		foreach ($result['status'] as $line) {
 			$tokens = \explode(' ', $line);
@@ -826,7 +818,7 @@ class PGP extends Base implements \SnappyMail\PGP\PGPInterface
 		return $info['ENC_TO'];
 	}
 
-	private function exec(array $arguments) /*: array|false*/
+	private function exec(array $arguments, bool $throw = true) /*: array|false*/
 	{
 		if (\version_compare($this->version, '2.2.5', '<')) {
 			\SnappyMail\Log::error('GPG', "{$this->version} too old");
@@ -948,12 +940,7 @@ class PGP extends Base implements \SnappyMail\PGP\PGPInterface
 			// Timeout after 5 seconds
 			if (5 < \microtime(1) - $start) {
 				$errors[] = 'timeout';
-				return [
+				throw new \RuntimeException(\implode("\n", $errors));
-					'output' => '',
-					'status' => $status,
-					'errors' => $errors
-				];
-				exit;
 			}
 
 			$inputStreams     = [];
@@ -1076,7 +1063,6 @@ class PGP extends Base implements \SnappyMail\PGP\PGPInterface
 			// to use too much memory
 			if (\in_array($this->_input, $inputStreams, true) && \strlen($inputBuffer) < self::CHUNK_SIZE) {
 				$this->_debug('input stream is ready for reading');
-				$this->_debug('=> about to read ' . self::CHUNK_SIZE . ' bytes from input stream');
 				$chunk        = \fread($this->_input, self::CHUNK_SIZE);
 				$length       = \strlen($chunk);
 				$inputBuffer .= $chunk;
@@ -1099,7 +1085,6 @@ class PGP extends Base implements \SnappyMail\PGP\PGPInterface
 			// read message (from PHP stream)
 			if (\in_array($this->_message, $inputStreams, true)) {
 				$this->_debug('message stream is ready for reading');
-				$this->_debug('=> about to read ' . self::CHUNK_SIZE . ' bytes from message stream');
 				$chunk          = \fread($this->_message, self::CHUNK_SIZE);
 				$length         = \strlen($chunk);
 				$messageBuffer .= $chunk;
@@ -1109,7 +1094,6 @@ class PGP extends Base implements \SnappyMail\PGP\PGPInterface
 			// read output (from GPG)
 			if (\in_array($fdOutput, $inputStreams, true)) {
 				$this->_debug('output stream ready for reading');
-				$this->_debug('=> about to read ' . self::CHUNK_SIZE . ' bytes from output');
 				$chunk         = \fread($fdOutput, self::CHUNK_SIZE);
 				$length        = \strlen($chunk);
 				$outputBuffer .= $chunk;
@@ -1136,7 +1120,6 @@ class PGP extends Base implements \SnappyMail\PGP\PGPInterface
 			// read error (from GPG)
 			if (\in_array($fdError, $inputStreams, true)) {
 				$this->_debug('error stream ready for reading');
-				$this->_debug('=> about to read ' . self::CHUNK_SIZE . ' bytes from error');
 				foreach ($this->_openPipes->readPipeLines(self::FD_ERROR) as $line) {
 					$errors[] = $line;
 					$this->_debug("\t{$line}");
@@ -1146,7 +1129,6 @@ class PGP extends Base implements \SnappyMail\PGP\PGPInterface
 			// read status (from GPG)
 			if (\in_array($fdStatus, $inputStreams, true)) {
 				$this->_debug('status stream ready for reading');
-				$this->_debug('=> about to read ' . self::CHUNK_SIZE . ' bytes from status');
 				// pass lines to status handlers
 				foreach ($this->_openPipes->readPipeLines(self::FD_STATUS) as $line) {
 					// only pass lines beginning with magic prefix
@@ -1199,12 +1181,16 @@ class PGP extends Base implements \SnappyMail\PGP\PGPInterface
 
 		$this->_debug('END PROCESSING');
 
-		$this->proc_close();
+		$exitCode = $this->proc_close();
 
 		$this->_message = null;
 		$this->_input   = null;
 		$this->_output  = null;
 
+		if ($throw && $exitCode && $errors) {
+			throw new \RuntimeException(\implode("\n", $errors), $exitCode);
+		}
+
 		return [
 			'output' => $outputBuffer,
 			'status' => $status,

snappymail/v/0.0.0/app/templates/Views/User/MailMessageView.html

@@ -296,10 +296,11 @@
 							<div class="crypto-control encrypted" data-bind="css: {success: message().pgpDecrypted()}">
 								<span data-icon="🔒" data-i18n="OPENPGP/ENCRYPTED_MESSAGE"></span>
 								<button class="btn" data-bind="visible: pgpSupported, click: pgpDecrypt" data-i18n="CRYPTO/DECRYPT"></button>
+								<div class="alert-error" data-icon="⚠" data-bind="visible: message().pgpEncrypted()?.error, text: message().pgpEncrypted()?.error"></div>
 							</div>
 						</div>
 						<div data-bind="visible: message().pgpSigned()">
-							<div class="crypto-control signed" data-bind="css: {success: message().pgpVerified() && message().pgpVerified().success, error: message().pgpVerified() && !message().pgpVerified().success}">
+							<div class="crypto-control signed" data-bind="css: {success: message().pgpVerified()?.success, error: message().pgpVerified() && !message().pgpVerified().success}">
 								<span data-icon="✍" data-i18n="OPENPGP/SIGNED_MESSAGE"></span>
 								<button class="btn" data-bind="visible: pgpSupported, click: pgpVerify" data-i18n="CRYPTO/VERIFY"></button>
 								<div data-icon="⚠" data-bind="visible: message().pgpVerified()?.error, text: message().pgpVerified()?.error"></div>
@@ -310,6 +311,7 @@
 							<div class="crypto-control encrypted" data-bind="css: {success: message().smimeDecrypted()}">
 								<span data-icon="🔒" data-i18n="SMIME/ENCRYPTED_MESSAGE"></span>
 								<button class="btn" data-bind="click: smimeDecrypt" data-i18n="CRYPTO/DECRYPT"></button>
+								<div class="alert-error" data-icon="⚠" data-bind="visible: message().smimeEncrypted()?.error, text: message().smimeEncrypted()?.error"></div>
 							</div>
 						</div>
 						<div data-bind="visible: message().smimeSigned()">