client: allow sending a second EHLO after STARTTLS

[emersion]

Use-case: provide a different hostname over the TLS connection. Ref https://github.com/emersion/go-smtp/pull/184#discussion_r863123368

[Spiral90210]

Just ran into this after updating! Ideally we would be able to not send the first EHLO localhost either, perfering to use at least a fully qualified hostname. It's ok for testing, but not against a hardened server rejecting non fully qualified hostnames on helo.

Would it be enough to change the default local name to mail.example.com rather than localhost, then change the hello function to something like the following?

// hello runs a hello exchange if needed.
func (c *Client) hello(preStartTLS bool) error {
	if !c.didHello {
	        if !preStartTLS {
		     c.didHello = true
		}
		if err := c.greet(); err != nil {
			c.helloError = err
		} else if err := c.ehlo(); err != nil {
			c.helloError = c.helo()
		}
	}
	return c.helloError
}

[emersion]

Hm, so in your case do you care about the EHLO sent before STARTTLS? That one can't be trusted by servers because it's transmitted over an insecure connection.

RFC 3207 section 4.2 says:

Upon completion of the TLS handshake, the SMTP protocol is reset to the initial state (the state in SMTP after a server issues a 220 service ready greeting). The server MUST discard any knowledge obtained from the client, such as the argument to the EHLO command, which was not obtained from the TLS negotiation itself.

[emersion]

Maybe we should reset c.didHello after STARTTLS. However we need to adjust c.greet calls (to not wait for an additional greeting after STARTTLS).

[torikki-tou]

@emersion I also run into hostname problem. Looks like we:

• need custom hostname if we dont plan STARTTLS
• does not need custom hostname if we only plan STARTTLS (connection established but STARTTLS not called yet)
• need custom hostname in ehlo method called after STARTTLS succeded (last line in StartTLS method)

Might submit a PR later but wanna hear your troughts anyway

[emersion]

OK, so sounds like a good way to solve your issue would be to update the library to allow Hello after STARTTLS. That is, you'd want to use Dial + Hello for non-STARTTLS connections, and DialStartTLS + Hello for STARTTLS connections.

Dial + Hello should work already, but the library doesn't allow DialStartTLS + Hello currently, so that would need a patch.

Is that correct?

[torikki-tou]

Sounds correct! I can do PR this evening if you wont do it before me. It`s a one line fix

[emersion]

I think it's slightly more than a one-line fix, see https://github.com/emersion/go-smtp/issues/245#issuecomment-2066234103. But yeah, shouldn't be too involved. Patches welcome!