fixup! Turn on TLS peer verification

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

.travis.yml

@@ -82,6 +82,9 @@ before_script:
   - php -f core/occ app:enable mail
   # Enable app twice to check occ errors of registered commands
   - php -f core/occ app:enable mail
+  # Turn off TLS verification here as the test server is not trusted
+  - php -f core/occ config:system:set app.mail.verify-tls-peer --type bool --value false
+
   - cd core/apps/mail
   - sh -c "if [ '$TEST_SUITE' = 'TEST-JS' ]; then npm install -g npm@latest; fi"
   - sh -c "if [ '$TEST_SUITE' = 'TEST-JS' ]; then make dev-setup; fi"

composer.json

@@ -50,8 +50,8 @@
 		"lint": "find . -name \\*.php -not -path './vendor/*' -exec php -l \"{}\" \\;",
 		"phan": "phan --allow-polyfill-parser -k .phan/config.php",
 		"test:integration": "phpunit -c tests/phpunit.integration.xml tests/Integration --fail-on-warning",
-		"test:integration:dev": "phpunit -c tests/phpunit.integration.xml tests/Integration --no-coverage --fail-on-warning",
+		"test:integration:dev": "phpunit -c tests/phpunit.integration.xml tests/Integration --no-coverage --fail-on-warning --stop-on-error --stop-on-failure",
 		"test:unit": "phpunit -c tests/phpunit.unit.xml tests/Unit --fail-on-warning",
-		"test:unit:dev": "phpunit -c tests/phpunit.unit.xml tests/Unit --no-coverage --fail-on-warning"
+		"test:unit:dev": "phpunit -c tests/phpunit.unit.xml tests/Unit --no-coverage --fail-on-warning --stop-on-error --stop-on-failure"
 	}
 }

lib/Account.php

@@ -33,6 +33,7 @@
 
 namespace OCA\Mail;
 
+use Horde_Imap_Client_Exception;
 use Horde_Imap_Client_Mailbox;
 use Horde_Imap_Client_Socket;
 use Horde_Mail_Rfc822_List;
@@ -43,6 +44,7 @@
 use OCA\Mail\Cache\Cache;
 use OCA\Mail\Db\Alias;
 use OCA\Mail\Db\MailAccount;
+use OCA\Mail\Exception\ServiceException;
 use OCA\Mail\Model\IMessage;
 use OCA\Mail\Model\Message;
 use OCP\ICacheFactory;
@@ -115,9 +117,11 @@ public function getEMailAddress() {
 	/**
 	 * @deprecated use \OCA\Mail\IMAP\IMAPClientFactory instead
 	 * @return Horde_Imap_Client_Socket
+	 *
+	 * @throws ServiceException
 	 */
 	public function getImapConnection() {
-		if (is_null($this->client)) {
+		if ($this->client === null) {
 			$host = $this->account->getInboundHost();
 			$user = $this->account->getInboundUser();
 			$password = $this->account->getInboundPassword();
@@ -134,8 +138,8 @@ public function getImapConnection() {
 				'timeout' => (int) $this->config->getSystemValue('app.mail.imap.timeout', 20),
 				'context' => [
 					'ssl' => [
-						'verify_peer' => true,
-						'verify_peer_name' => false
+						'verify_peer' => $this->config->getSystemValueBool('app.mail.verify-tls-peer', true),
+						'verify_peer_name' => $this->config->getSystemValueBool('app.mail.verify-tls-peer', true),
 					],
 				],
 			];
@@ -152,7 +156,11 @@ public function getImapConnection() {
 				}
 			}
 			$this->client = new \Horde_Imap_Client_Socket($params);
-			$this->client->login();
+			try {
+				$this->client->login();
+			} catch (Horde_Imap_Client_Exception $e) {
+				throw new ServiceException("Could not connect to IMAP: " . $e->getMessage(), $e->getCode(), $e);
+			}
 		}
 		return $this->client;
 	}
@@ -161,6 +169,8 @@ public function getImapConnection() {
 	 * @deprecated
 	 * @param string $folderId
 	 * @return Mailbox
+	 *
+	 * @throws ServiceException
 	 */
 	public function getMailbox($folderId) {
 		return new Mailbox(
@@ -202,6 +212,8 @@ public function getEmail() {
 	 * @deprecated
 	 *
 	 * @return void
+	 *
+	 * @throws ServiceException
 	 */
 	public function testConnectivity(Horde_Mail_Transport $transport): void {
 		// connect to imap

lib/Contracts/IMailManager.php

@@ -38,6 +38,8 @@ interface IMailManager {
 	 * @param Account $account
 	 *
 	 * @return Folder[]
+	 *
+	 * @throws ServiceException
 	 */
 	public function getFolders(Account $account): array;
 
@@ -46,6 +48,8 @@ public function getFolders(Account $account): array;
 	 * @param string $name
 	 *
 	 * @return Folder
+	 *
+	 * @throws ServiceException
 	 */
 	public function createFolder(Account $account, string $name): Folder;
 
@@ -54,6 +58,8 @@ public function createFolder(Account $account, string $name): Folder;
 	 * @param string $folderId
 	 *
 	 * @return FolderStats
+	 *
+	 * @throws ServiceException
 	 */
 	public function getFolderStats(Account $account, string $folderId): FolderStats;
 
@@ -64,6 +70,7 @@ public function getFolderStats(Account $account, string $folderId): FolderStats;
 	 * @param bool $loadBody
 	 *
 	 * @return IMAPMessage
+	 *
 	 * @throws ServiceException
 	 */
 	public function getMessage(Account $account, string $mailbox, int $id, bool $loadBody = false): IMAPMessage;
@@ -94,6 +101,8 @@ public function deleteMessage(Account $account, string $mailboxId, int $messageI
 	 *
 	 * @param Account $account
 	 * @param string $folderId
+	 *
+	 * @throws ServiceException
 	 */
 	public function markFolderAsRead(Account $account, string $folderId): void;
 

lib/Controller/AccountsController.php

@@ -110,7 +110,7 @@ public function index(): JSONResponse {
 	 * @param int $accountId
 	 *
 	 * @return JSONResponse
-	 * @throws Exception
+	 * @throws ClientException
 	 */
 	public function show($accountId): JSONResponse {
 		return new JSONResponse($this->accountService->find($this->currentUserId, $accountId));
@@ -187,6 +187,8 @@ public function update(int $id,
 	 * @param int|null $order
 	 *
 	 * @return JSONResponse
+	 *
+	 * @throws ClientException
 	 */
 	public function patchAccount(int $accountId,
 								 string $editorMode = null,
@@ -217,6 +219,9 @@ public function patchAccount(int $accountId,
 	 * @param string|null $signature
 	 *
 	 * @return JSONResponse
+	 *
+	 * @throws ClientException
+	 * @throws ServiceException
 	 */
 	public function updateSignature(int $accountId, string $signature = null): JSONResponse {
 		$this->accountService->updateSignature($accountId, $this->currentUserId, $signature);
@@ -301,6 +306,7 @@ public function create(string $accountName, string $emailAddress, string $passwo
 	 *
 	 * @return JSONResponse
 	 *
+	 * @throws ClientException
 	 * @throws ServiceException
 	 */
 	public function send(int $accountId,
@@ -350,6 +356,8 @@ public function send(int $accountId,
 	 * @param int $uid
 	 *
 	 * @return JSONResponse
+	 *
+	 * @throws ClientException
 	 */
 	public function draft(int $accountId,
 						  string $subject = null,
@@ -359,7 +367,7 @@ public function draft(int $accountId,
 						  string $bcc,
 						  bool $isHtml = true,
 						  int $draftUID = null): JSONResponse {
-		if (is_null($draftUID)) {
+		if ($draftUID === null) {
 			$this->logger->info("Saving a new draft in account <$accountId>");
 		} else {
 			$this->logger->info("Updating draft <$draftUID> in account <$accountId>");

lib/Controller/FoldersController.php

@@ -83,6 +83,9 @@ public function __construct(string $appName,
 	 *
 	 * @param int $accountId
 	 * @return JSONResponse
+	 *
+	 * @throws ClientException
+	 * @throws ServiceException
 	 */
 	public function index(int $accountId): JSONResponse {
 		$account = $this->accountService->find($this->currentUserId, $accountId);
@@ -143,6 +146,8 @@ public function sync(int $accountId, string $folderId, array $uids = [], bool $i
 	 * @param int $accountId
 	 * @param string $folderId
 	 * @return JSONResponse
+	 *
+	 * @throws ClientException
 	 */
 	public function markAllAsRead(int $accountId, string $folderId): JSONResponse {
 		$account = $this->accountService->find($this->currentUserId, $accountId);
@@ -164,6 +169,8 @@ public function markAllAsRead(int $accountId, string $folderId): JSONResponse {
 	 * @param string $folderId
 	 *
 	 * @return JSONResponse
+	 *
+	 * @throws ClientException
 	 */
 	public function stats(int $accountId, string $folderId): JSONResponse {
 		$account = $this->accountService->find($this->currentUserId, $accountId);
@@ -196,6 +203,9 @@ public function update() {
 	/**
 	 * @NoAdminRequired
 	 * @TrapError
+	 *
+	 * @throws ClientException
+	 * @throws ServiceException
 	 */
 	public function create(int $accountId, string $name) {
 		$account = $this->accountService->find($this->currentUserId, $accountId);

lib/Controller/MessagesController.php

@@ -137,6 +137,7 @@ public function __construct(string $appName,
 	 * @param string $filter
 	 *
 	 * @return JSONResponse
+	 *
 	 * @throws ClientException
 	 * @throws ServiceException
 	 */
@@ -168,6 +169,8 @@ public function index(int $accountId, string $folderId, int $cursor = null, stri
 	 * @param int $id
 	 *
 	 * @return JSONResponse
+	 *
+	 * @throws ClientException
 	 * @throws ServiceException
 	 */
 	public function show(int $accountId, string $folderId, int $id): JSONResponse {
@@ -197,6 +200,8 @@ public function show(int $accountId, string $folderId, int $id): JSONResponse {
 	 * @param int $messageId
 	 *
 	 * @return JSONResponse
+	 *
+	 * @throws ClientException
 	 * @throws ServiceException
 	 */
 	public function getBody(int $accountId, string $folderId, int $messageId): JSONResponse {
@@ -239,7 +244,9 @@ public function getBody(int $accountId, string $folderId, int $messageId): JSONR
 	 * @param string $destFolderId
 	 *
 	 * @return JSONResponse
-	 * @throws Exception
+	 *
+	 * @throws ClientException
+	 * @throws ServiceException
 	 */
 	public function move($accountId, $folderId, $id, $destAccountId, $destFolderId): JSONResponse {
 		$srcAccount = $this->accountService->find($this->currentUserId, $accountId);
@@ -260,6 +267,8 @@ public function move($accountId, $folderId, $id, $destAccountId, $destFolderId):
 	 * @param int $messageId
 	 *
 	 * @return HtmlResponse|TemplateResponse
+	 *
+	 * @throws ClientException
 	 */
 	public function getHtmlBody(int $accountId, string $folderId, int $messageId): Response {
 		try {
@@ -317,6 +326,9 @@ public function getHtmlBody(int $accountId, string $folderId, int $messageId): R
 	 * @param int $attachmentId
 	 *
 	 * @return AttachmentDownloadResponse
+	 *
+	 * @throws ClientException
+	 * @throws ServiceException
 	 */
 	public function downloadAttachment(int $accountId, string $folderId, int $messageId,
 									   string $attachmentId) {
@@ -352,6 +364,9 @@ public function downloadAttachment(int $accountId, string $folderId, int $messag
 	 * @param string $targetPath
 	 *
 	 * @return JSONResponse
+	 *
+	 * @throws ClientException
+	 * @throws ServiceException
 	 */
 	public function saveAttachment(int $accountId, string $folderId, int $messageId,
 								   string $attachmentId, string $targetPath) {
@@ -400,6 +415,9 @@ public function saveAttachment(int $accountId, string $folderId, int $messageId,
 	 * @param array $flags
 	 *
 	 * @return JSONResponse
+	 *
+	 * @throws ClientException
+	 * @throws ServiceException
 	 */
 	public function setFlags(int $accountId, string $folderId, int $messageId, array $flags): JSONResponse {
 		$mailBox = $this->getFolder($accountId, $folderId);
@@ -424,6 +442,8 @@ public function setFlags(int $accountId, string $folderId, int $messageId, array
 	 * @param int $id
 	 *
 	 * @return JSONResponse
+	 *
+	 * @throws ClientException
 	 * @throws ServiceException
 	 */
 	public function destroy(int $accountId, string $folderId, int $id): JSONResponse {
@@ -444,6 +464,9 @@ public function destroy(int $accountId, string $folderId, int $id): JSONResponse
 	 * @param string $folderId
 	 *
 	 * @return IMailBox
+	 *
+	 * @throws ClientException
+	 * @throws ServiceException
 	 */
 	private function getFolder(int $accountId, string $folderId): IMailBox {
 		$account = $this->accountService->find($this->currentUserId, $accountId);

lib/Controller/PageController.php

@@ -28,10 +28,10 @@
 
 use Exception;
 use OCA\Mail\AppInfo\Application;
+use OCA\Mail\Contracts\IMailManager;
 use OCA\Mail\Contracts\IUserPreferences;
 use OCA\Mail\Service\AccountService;
 use OCA\Mail\Service\AliasesService;
-use OCA\Mail\Service\MailManager;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\ContentSecurityPolicy;
 use OCP\AppFramework\Http\RedirectResponse;
@@ -66,7 +66,7 @@ class PageController extends Controller {
 	/** @var IUserPreferences */
 	private $preferences;
 
-	/** @var MailManager */
+	/** @var IMailManager */
 	private $mailManager;
 
 	/** @var IInitialStateService */
@@ -84,7 +84,7 @@ public function __construct(string $appName,
 								?string $UserId,
 								IUserSession $userSession,
 								IUserPreferences $preferences,
-								MailManager $mailManager,
+								IMailManager $mailManager,
 								IInitialStateService $initialStateService,
 								ILogger $logger) {
 		parent::__construct($appName, $request);

lib/IMAP/IMAPClientFactory.php

@@ -79,8 +79,8 @@ public function getClient(Account $account): Horde_Imap_Client_Socket {
 				'timeout' => (int)$this->config->getSystemValue('app.mail.imap.timeout', 5),
 				'context' => [
 					'ssl' => [
-						'verify_peer' => true,
-						'verify_peer_name' => true
+						'verify_peer' => $this->config->getSystemValueBool('app.mail.verify-tls-peer', true),
+						'verify_peer_name' => $this->config->getSystemValueBool('app.mail.verify-tls-peer', true),
 					],
 				],
 			];

lib/IMAP/MailboxSync.php

@@ -23,6 +23,8 @@
 
 namespace OCA\Mail\IMAP;
 
+use Horde_Imap_Client_Exception;
+use OCA\Mail\Exception\ServiceException;
 use function json_encode;
 use OCA\Mail\Account;
 use OCA\Mail\Db\MailAccountMapper;
@@ -66,6 +68,9 @@ public function __construct(MailboxMapper $mailboxMapper,
 		$this->logger = $logger;
 	}
 
+	/**
+	 * @throws ServiceException
+	 */
 	public function sync(Account $account, bool $force = false): void {
 		if (!$force && $account->getMailAccount()->getLastMailboxSync() >= ($this->timeFactory->getTime() - 7200)) {
 			$this->logger->debug("account is up to date, skipping mailbox sync");
@@ -74,8 +79,12 @@ public function sync(Account $account, bool $force = false): void {
 
 		$client = $this->imapClientFactory->getClient($account);
 
-		$folders = $this->folderMapper->getFolders($account, $client);
-		$this->folderMapper->getFoldersStatus($folders, $client);
+		try {
+			$folders = $this->folderMapper->getFolders($account, $client);
+			$this->folderMapper->getFoldersStatus($folders, $client);
+		} catch (Horde_Imap_Client_Exception $e) {
+			throw new ServiceException("IMAP error" . $e->getMessage(), $e->getCode(), $e);
+		}
 		$this->folderMapper->detectFolderSpecialUse($folders);
 
 		$old = $this->mailboxMapper->findAll($account);