Add support for SMTP targets

[ribbybibby]

Hi @algestam, thanks for the PR!

I'm not particularly familiar with TLS over SMTP. Is there any advantage to using an smtp client, as you do here, over targetting the endpoint with tcp at <host>:25 or <host>:587?

[ribbybibby]

Okay, I just tested this. I see that you need to send a starttls command to an smtp server.

[algestam]

Hi @ribbybibby, thanks for taking the time to look at the PR :+1:

Yes, when using STARTTLS the connection is initally unencrypted and is changed to use encryption once the STARTTLS command is issued.

I believe that another option for SMTP is to use an explicit SSL connection (usually on port 465). For targeting such services host:465 should work but I haven't tested that out since the mail server I'm targeting is configured to use STARTTLS.

Also, regarding unit tests, I haven't figured out how to set up a mock smtp server in a similar way as http is tested. Perhaps you have any ideas on how to do that?

[ribbybibby]

I believe that another option for SMTP is to use an explicit SSL connection (usually on port 465).

The fact that some smtp connections don't use starttls makes me wonder whether it's correct to use the scheme smtp:// to indicate it's use. If you try to hit a mail server that uses an explicit SSL connection, like smtp://email-smtp.eu-central-1.amazonaws.com:465, the exporter will fail.

openssl s_client includes an option that allows you to specify the protocol you want to send a starttls request for (it seems it isn't just smtp that supports this):

-starttls protocol

    send the protocol-specific message(s) to switch to TLS for communication. protocol is a keyword for the intended protocol. Currently, the only supported keywords are "smtp", "pop3", "imap", "ftp", "xmpp", "xmpp-server", and "irc."

I think it might be better if we took the same approach in the exporter and added a parameter for sending a protocol specific starttls message. Something along the lines of:

localhost:9219/probe?target=email-smtp.eu-central-1.amazonaws.com:25&starttls=smtp

This strikes me as more transparent to the user in terms of what's actually being done here at the level of TLS communcation.

[algestam]

Thanks for the feedback and suggestions!

TL;DR: After giving this some thought, I agree that using the scheme smtp:// and assuming that the connection uses STARTTLS can be confusing and the functionality to check starttls enabled targets should be implemented another way. I think this PR should be closed and another way to specify that the connection shall be upgraded (via starttls) will be better and more future proof.

Actually, the current ssl exporter already supports SMTP servers with explicit SSL connections. This is how such a server can be checked:

http://localhost:9219/probe?target=email-smtp.eu-central-1.amazonaws.com:465

The openssl client can definitely be one way of implementing support for starttls, I will investigate whether the starttls option is exposed via the golang library.

I looked a bit into how this is handled in the blackbox exporter and the tcp prober allows configuration of a query_response where requests and expected responses can be configured. It's also possible to upgrade the connection to use TLS within a query response using the starttls option. By using this mechanism it's possible to support different protocols supporting starttls connection upgrades. There are examples of this in the blackbox exporter configuration example.

Implementing something similar to what the blackbox exporter offers with a user configurable protocol definition makes it easy to support other protocols. Would you consider a PR that implements such functionality?

[ribbybibby]

My initial instinct is that it's not appropriate for the ssl_exporter to follow the blackbox_exporter here. Upgrading to TLS is a common use of query_response, but it isn't the only one.query_response is a generically applicable way of sending queries with applications beyond starttls. For that reason it feels out of scope to add this functionality to an exporter which is only interested in TLS.

It seems to me that there's a relatively small number of known protocols that upgrade TLS, and how that's done in each specific case is consistent, so I think it's reasonable to keep all that logic within the exporter for the sake of providing a single argument like starttls=smtp or starttls=ftp.

[algestam]

Cool, I agree that implementing query_response-like functionality may be inappropriate as it isn't actually needed and specific functionality can instead be implemented for each protocol that upgrade the connection to TLS.

I can take a shot at implementing upgrade TLS functionality via a starttls argument. I will investigate the possibility to use the openssl client in a clean way, otherwise I'll resort to either defining the required protocol commands to upgrade the connection to TLS for each protocol (starting with smtp) or let each starttls protocol implement their own solution (i.e. use net/smtp like this PR does for smtp).

[cownoise]

Any updates please?

[algestam]

Thanks for the interest shown regarding this feature :) Unfortunately, I haven't had time to implement the TLS functionality using a starttls argument as discussed above yet. Hoping to find the time and motivation soon but otherwise please feel free to implement the feature.

[ribbybibby]

I'm doing some work on the exporter at the moment and I'll hopefully be implementing this soon.

[ribbybibby]

See: #36