helo_hostname is not fully realized

[dallaslu]

Describe the bug

Config.dns.helo_hostname, commented with 'The hostname to use in HELO/EHLO when connecting to external SMTP servers'.

But in app/lib/smtp_server/client.rb:

new_io.print("220 #{Postal::Config.postal.smtp_hostname} ESMTP Postal/#{client.trace_id}")

And a few other places.

[willpower232]

The endpoint seems to evaluate a few options before deciding which to be but that isn't used in numerous other places so I agree it is a little confusing.

postal/app/lib/smtp_client/endpoint.rb

Lines 142 to 146 in da90e75

	def default_helo_hostname
	Postal::Config.dns.helo_hostname ||
	Postal::Config.postal.smtp_hostname ||
	"localhost"
	end

[schueffi]

As I understand it,
helo_hostname is the name it uses for outgoing connections (i.e. acting as a client when sending emails), and defaults to smtp_hostname if not explicitly set. This helo_hostname also needs to be available with a proper PTR record pointing back to your server in order to avoid being considered as spam.

smtp_hosname is used both as a default for the helo_hostname, and, more important, as the name for incoming connections (i.e. when receiving emails). This name is the one you would use in your TLS certificate if you enable TLS/StartTLS on the server side.

As I understand the code, this is implemented correctly in the current main branch already.

[dallaslu]

As I understand the code, this is implemented correctly in the current main branch already.

I agree with what you said. Also, the sending process is independent and does not need to be aligned with the server used to receive the message. Within Postal, if we use the default settings, i.e. only one domain name, there is no problem.

The problem is that when we specify helo_hostname in the config file, the code uses smtp_hostname in a lot of places where helo_hostname should be used.

new_io.print("220 #{Postal::Config.postal.smtp_hostname} ESMTP Postal/#{client.trace_id}")

Should be something like:

new_io.print("220 #{Postal::Config.postal.helo_hostname} ESMTP Postal/#{client.trace_id}")

https://mxtoolbox.com/problem/smtp/smtp-banner-check:

It is best practice to put the name of your server in your SMTP banner so that anybody who connects via your IP Address has a clue as to who they are talking to. You will get this warning if the name you present yourself as is not in the same domain as the hostname we get when we perform a PTR lookup on your IP Address.

[schueffi]

The code in "app/lib/smtp_client/..." is used for outbound connections, and properly uses the helo_hostname (if set), and falls back to smtp_hostname (otherwise).

The code in "app/lib//smtp_server/..." is used for inbound connections. This code still uses smtp_hostname instead of the helo_hostname, in order to enable to have two different helo_hostnames for outgoing and inbound connections.

If the server code would be refactored to also use the helo_hostname (so, using helo_hostname always in all places where currently smtp_hostname is used), there seems to be no need to set up two different variables anymore (as the smtp_hostname will never be used, then?) At least, I did not really found a remaining place where the "smtp_hostname" would be used regardless of the helo_hostname setting.

Maybe a better name for the current helo_hostname could be "helo_hostname_for_outgoing_connections_(if_set_and_if_should_be_different_than_the_smtp/helo_hostname_for_inbound_connections)?

Don't know whether this makes sense, and I don't want to argue or judge what is right here. Just giving my thoughts on this topic to help finding the right-thing-to-do.

[dallaslu]

helo_hostname is more important when posting emails outbound. This is because external servers may rely on this to check certificates, PTRs.

I can't think of a situation where you need to deliberately specify different helo_hostname for incoming and outgoing. They can be the same or different.

The helo_hostname config is optional, and its default value is ${smtp_hostname}.

My concern now is that helo_hostname isn't working as expected.