mail app does not send fqdn

[pclerie]

The Mail App fails when sending to a server configured to reject hosts with no FQDN. Below are some notes from trying to solve that problem:

NextCloud uses the Horde libraries for the mail app. While I don’t know zilch about PHP I thought I would at least look at the code.

The interesting part is in apps/mail/vendor/pear-pear.horde.org/Horde_Smtp/Horde/Smtp.php.

First there is a function _hello that does the EHLO exchange:

protected function _hello()
{
    $ehlo = $host = $this->_getHostname();
    if ($host === false) {
        $ehlo = $_SERVER['SERVER_ADDR'];
        $host = 'localhost';
    }
...

We want the function _getHostname:

protected function _getHostname()
{
    return ($localhost = $this->getParam('localhost'))
        ? $localhost
        : gethostname();
}

It appears that the PHP builtin gethostname() can only return the non-qualified server host name. So what’s left is the function getParam.

getParam does things to a _params array. That takes us back to the top of the file where there is a constructor filling up a params array. And there is provision for a parameter named localhost. I figured I could play with the array too. So I added a line to force params(‘localhost’) to contain my fqdn:

public function __construct(array $params = array())
{
    // Default values.
    $params = array_merge(array(
        'chunk_size' => self::CHUNK_DEFAULT,
        'context' => array(),
        'host' => 'localhost',
        'port' => 587,
        'secure' => true,
        'timeout' => 30,
+        'localhost' => 'myhostname.example.com'
    ), array_filter($params));

Save. Try sending. Bingo.

At best that’s a hack, not a fix; I’ll have to edit that file for every upgrade.

---

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[ChristophWurst]

Hi,

thanks for reporting this. We should try to pass this parameter to the horde libs somehow, since patching it with a hard-coded value doesn't really help much here.

[ChristophWurst]

We should try to pass this parameter to the horde libs somehow

Can be done easily: https://github.com/nextcloud/mail/blob/4c6fd6e3bcc9bd6fa36954df3f2f6d1681bc95fa/lib/Account.php#L372-L379. The question is, how can we determine the correct FQDN reliably?

[pclerie]

For what it's worth, I am tempted to recommend that you make it a configured value, and use the trusted_domains array in the root config.php as source for a default. Otherwise, you may find yourself reinventing gethostbyname.

[pclerie]

:-)
And maybe pass the buck to Horde and to PHP.
:-)

[monojp]

Hi,
I stumbled upon the same problem. Just wanted to let you know that according to http://www.postfix.org/postconf.5.html#reject_non_fqdn_helo_hostname this is required by the RFC and according to http://www.netarky.com/programming/arch_linux/Arch_Linux_mail_server_setup_1.html it's a simple way to prevent spam

[drajar]

My resolution to this important problem:

```
lib/SMTP/SmtpClientFactory.php | 2 ++
1 file changed, 2 insertions(+)

diff --git a/lib/SMTP/SmtpClientFactory.php b/lib/SMTP/SmtpClientFactory.php
index 9a9de193..9393cba1 100644
--- a/lib/SMTP/SmtpClientFactory.php
+++ b/lib/SMTP/SmtpClientFactory.php
@@ -30,6 +30,7 @@ use Horde_Mail_Transport_Smtphorde;
use OCA\Mail\Account;
use OCP\IConfig;
use OCP\Security\ICrypto;
+use OCP\Util;

class SmtpClientFactory {

@@ -63,6 +64,7 @@ class SmtpClientFactory {
$password = $this->crypto->decrypt($password);
$security = $mailAccount->getOutboundSslMode();
$params = [

• 'localhost' => Util::getServerHostName(),
  'host' => $mailAccount->getOutboundHost(),
  'password' => $password,
  'port' => $mailAccount->getOutboundPort(),
  ```

[ChristophWurst]

@drajar awesome to see a solution for this. Would you mind submitting this as pull request so that we can discuss the code changes further and get this integrated?
We should probably look for an injectable class that abstracts this static method call to make it testable, but I will happily take over this refactoring if that's too much of an effort for you :)

Thanks! :rocket:

[drajar]

Maybe I must clarify param localhost added because the tests fail with:

1) OCA\Mail\Tests\Smtp\SmtpClientFactoryTest::testSmtpTransport
Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
         'password' => 'pass123'
-        'port' => '25'
+        'port' => 25
         'username' => '[email protected]'
         'secure' => false
         'timeout' => 2
+        'localhost' => 'localhost'
     )
 )

The port error is clear, the test must be done without quote.
The localhost error: I use 'localhost' => Util::getServerHostName(). that means that I sent the domain name of nextcloud server. For example if nextcloud use nextcloud.domain.tld, then that will be set. So the verification must be done that way.

[ChristophWurst]

Final PR can be found at https://github.com/nextcloud/mail/pull/1021. Thanks for you contribution, @drajar :rocket:

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and questions.