-
Notifications
You must be signed in to change notification settings - Fork 95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for SMTP targets #21
Conversation
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 |
Okay, I just tested this. I see that you need to send a starttls command to an smtp server. |
Hi @ribbybibby, thanks for taking the time to look at the PR 👍 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 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? |
The fact that some smtp connections don't use starttls makes me wonder whether it's correct to use the scheme 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):
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:
This strikes me as more transparent to the user in terms of what's actually being done here at the level of TLS communcation. |
Thanks for the feedback and suggestions! TL;DR: After giving this some thought, I agree that using the scheme Actually, the current ssl exporter already supports SMTP servers with explicit SSL connections. This is how such a server can be checked:
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 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? |
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 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 |
Cool, I agree that implementing I can take a shot at implementing upgrade TLS functionality via a |
Any updates please? |
Thanks for the interest shown regarding this feature :) Unfortunately, I haven't had time to implement the TLS functionality using a |
I'm doing some work on the exporter at the moment and I'll hopefully be implementing this soon. |
See: #36 |
No description provided.