Skip to content

Commit

Permalink
Merge pull request #2782 from nextcloud/fix/tls-peer-verification
Browse files Browse the repository at this point in the history
Add TLS host verification
  • Loading branch information
ChristophWurst authored Mar 23, 2020
2 parents 9e28a58 + 0cc7f6f commit c6ed589
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 12 deletions.
3 changes: 3 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 6 additions & 0 deletions lib/Account.php
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ public function getImapConnection() {
'port' => $port,
'secure' => $ssl_mode,
'timeout' => (int) $this->config->getSystemValue('app.mail.imap.timeout', 20),
'context' => [
'ssl' => [
'verify_peer' => $this->config->getSystemValueBool('app.mail.verify-tls-peer', true),
'verify_peer_name' => $this->config->getSystemValueBool('app.mail.verify-tls-peer', true),
],
],
];
if ($this->config->getSystemValue('debug', false)) {
$params['debug'] = $this->config->getSystemValue('datadirectory') . '/horde_imap.log';
Expand Down
6 changes: 6 additions & 0 deletions lib/IMAP/IMAPClientFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,12 @@ public function getClient(Account $account): Horde_Imap_Client_Socket {
'port' => $port,
'secure' => $sslMode,
'timeout' => (int)$this->config->getSystemValue('app.mail.imap.timeout', 5),
'context' => [
'ssl' => [
'verify_peer' => $this->config->getSystemValueBool('app.mail.verify-tls-peer', true),
'verify_peer_name' => $this->config->getSystemValueBool('app.mail.verify-tls-peer', true),
],
],
];
if ($this->cacheFactory->isAvailable()) {
$params['cache'] = [
Expand Down
8 changes: 7 additions & 1 deletion lib/SMTP/SmtpClientFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,13 @@ public function create(Account $account): Horde_Mail_Transport {
'port' => $mailAccount->getOutboundPort(),
'username' => $mailAccount->getOutboundUser(),
'secure' => $security === 'none' ? false : $security,
'timeout' => (int)$this->config->getSystemValue('app.mail.smtp.timeout', 5)
'timeout' => (int)$this->config->getSystemValue('app.mail.smtp.timeout', 5),
'context' => [
'ssl' => [
'verify_peer' => $this->config->getSystemValueBool('app.mail.verify-tls-peer', true),
'verify_peer_name' => $this->config->getSystemValueBool('app.mail.verify-tls-peer', true),
],
],
];
if ($this->config->getSystemValue('debug', false)) {
$params['debug'] = $this->config->getSystemValue('datadirectory') . '/horde_smtp.log';
Expand Down
27 changes: 16 additions & 11 deletions tests/Unit/SMTP/SmtpClientFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,18 +83,17 @@ public function testSmtpTransport() {
'smtpPassword' => 'obenc',
]);
$account = new Account($mailAccount);
$this->config->expects($this->at(0))
$this->config->expects($this->any())
->method('getSystemValue')
->with('app.mail.transport', 'smtp')
->willReturn('smtp');
$this->config->expects($this->at(2))
->method('getSystemValue')
->with('debug', false)
->willReturn(false);
$this->config->expects($this->at(1))
->method('getSystemValue')
->with('app.mail.smtp.timeout', 5)
->willReturn(2);
->willReturnMap([
['app.mail.transport', 'smtp', 'smtp'],
['debug', false, false],
['app.mail.smtp.timeout', 5, 2],
]);
$this->config->expects($this->any())
->method('getSystemValueBool')
->with('app.mail.verify-tls-peer', true)
->willReturn(true);
$this->crypto->expects($this->once())
->method('decrypt')
->with('obenc')
Expand All @@ -110,6 +109,12 @@ public function testSmtpTransport() {
'secure' => false,
'timeout' => 2,
'localhost' => 'cloud.example.com',
'context' => [
'ssl' => [
'verify_peer' => true,
'verify_peer_name' => true,
],
],
]);

$transport = $this->factory->create($account);
Expand Down

0 comments on commit c6ed589

Please sign in to comment.