Skip to content

Commit

Permalink
smtp: Use proper name when saying What's Up (HELO/EHLO)
Browse files Browse the repository at this point in the history
This commit addresses an issue  where outgoing email can get rejected when the
target mail server is tight-assed - requiring a valid host when saying
helo/ehlo.

We were setting the name to null which resulted in localhost being used as the
name. Now we're asking the OS to give us the proper hostname of the server -
hopefully the server admin has the correct hostname and reverse-lookup setup
correctly for better deliverability.
  • Loading branch information
protich committed Oct 9, 2022
1 parent 0564438 commit b0e5ac8
Showing 1 changed file with 35 additions and 7 deletions.
42 changes: 35 additions & 7 deletions include/class.mail.php
Expand Up @@ -617,8 +617,9 @@ private function buildOptions(AccountSetting $setting) {
case $auth instanceof NoAuthCredentials:
// No Authentication - simply return host and port
return [
'host' => $connect['host'],
'port' => $connect['port']
'host' => $connect['host'],
'port' => $connect['port'],
'name' => $connect['name'],
];
break;
case $auth instanceof BasicAuthCredentials:
Expand All @@ -642,8 +643,9 @@ private function buildOptions(AccountSetting $setting) {
}

return [
'host' => $connect['host'],
'port' => $connect['port'],
'host' => $connect['host'],
'port' => $connect['port'],
'name' => $connect['name'],
'connection_class' => $auth->getConnectionClass(),
'connection_config' => $config
];
Expand Down Expand Up @@ -819,12 +821,13 @@ public function __construct(\EmailAccount $account) {
$ssl = 'tls';
}

// Set the connection settings
$this->connection = [
'host' => $host,
'port' => (int) $port,
'ssl' => $ssl,
'protocol' => strtoupper($account->getProtocol()),
'name' => null
'name' => self::get_hostname(),
];

// Set errors to null to clear validation
Expand Down Expand Up @@ -892,15 +895,17 @@ public function describe() {
private function validate() {

if (!isset($this->errors)) {
// Set errors to an array to to make sure don't
// unneccesarily validate valid info again.
$this->errors = [];
// We're simply making sure required info are set. True
// validation will happen at the protocol connection level
$info = $this->getConnectionConfig();
foreach (['host', 'port', 'protocol'] as $p ) {
if (!isset($info[$p]) || !$info[$p])
$this->errors[$p] = sprintf('%s %s',
strtoupper($p), __('Required'));
}
// TODO: Validate hostname - for now we're punting to be
// validated at the protocol connection level
}
return !count($this->errors);
}
Expand All @@ -912,6 +917,29 @@ public function isValid() {
public function getErrors() {
return $this->errors;
}

/*
* get_hostname
*
* Please note that this is different from getHost above
*
* Here we're getting the hostname to use on HELO/EHLO when
* initiating an SMTP connection. It should be a valid hostname with
* valid reverse-lookup for better deliverability.
*
* Perhaps this can be a setting in the future but allowing users
* to set it to **anything** will results in more mail issues than just
* defaulting to what the OS tells us or localhost for that matter.
*
* For now, we're simply asking core osTicket to give us the OS hostname
* otherwise it will default to localhost which some mail servers frawns
* upon since it won't have a valid reverse-lookup.
*
*/
private static function get_hostname() {
// We're simply returning what the OS is telling us!
return php_uname('n');
}
}
}

Expand Down

0 comments on commit b0e5ac8

Please sign in to comment.