Skip to content
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

New feature: Monitor TLS ports #1626

Closed
wants to merge 1 commit into from

Conversation

mrubli
Copy link

@mrubli mrubli commented May 9, 2022

Description

This adds a new monitor type for TLS ports, similar to the "TCP Port" monitor but ensuring that a valid TLS connection can be established and checking for an expected keyword. This allows monitoring of services like IMAP or POP3.

I haven't spent much time on style or documentation but will gladly do that if there is interest in merging this feature.

My current code is on the feature/tls-monitor-dev branch (needs rebasing): mrubli@6d92a60

I've deployed it on my own server and it works well so far for monitoring IMAP and POP3. A Docker image can be found here:
https://github.com/mrubli/uptime-kuma/pkgs/container/uptime-kuma

Fixes #1079.

Type of change

Please delete any options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas
    (including JSDoc for methods)
  • My changes generate no new warnings
  • My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

image

@louislam
Copy link
Owner

For me, I always thought TLS = SSL = HTTPS for some reason. Maybe call it TCP Port (TLS)?

@shaneshort
Copy link

is there anything holding this feature up? it looks very useful

@hoerup
Copy link

hoerup commented Feb 10, 2023

I would very much like to see this feature get implemented.

2 question for @mrubli

  1. would it be possible to have it work for both implicit TLS and explicit/STARTTLS types ?
  2. do you check for cert expiration as well

@mrubli
Copy link
Author

mrubli commented Feb 20, 2023

@louislam

For me, I always thought TLS = SSL = HTTPS for some reason. Maybe call it TCP Port (TLS)?

I personally wouldn't call that equation accurate but renaming is easy and I don't mind. :-)

If the name is the only thing holding up this feature I'll spend some time rebasing it and cleaning it up where necessary.

@hoerup

  1. would it be possible to have it work for both implicit TLS and explicit/STARTTLS types ?

STARTTLS is quite a different beast and apparently takes some work. I don't have time to implement that at this point but if someone wants to help integrating that would be welcome.

  1. do you check for cert expiration as well

The way I understand the API an expired server certificate should make the connection test fail. You could easily verify that by pointing it at https://expired.badssl.com.

@hoerup
Copy link

hoerup commented Feb 22, 2023

  1. do you check for cert expiration as well

The way I understand the API an expired server certificate should make the connection test fail. You could easily verify that by pointing it at https://expired.badssl.com.

I might have been a bit unclear - what I meant was something like the certificate Expiry from https monitor, so we get warnings BEFORE the cert expires and everything breaks :)

@mrubli
Copy link
Author

mrubli commented Feb 23, 2023

I might have been a bit unclear - what I meant was something like the certificate Expiry from https monitor, so we get warnings BEFORE the cert expires and everything breaks :)

Ah yes, that sounds like a useful addition to have. I'll look into it if there's interest in merging this feature to begin with.

@tobhv
Copy link

tobhv commented Apr 8, 2023

hi. yes, the certificate expiration is also something i am looking for. using openssl client on the commandline the info can be found like this:
echo | openssl s_client -connect fqdn:port 2>/dev/null | openssl x509 -noout -dates

and also a javascript reference that may help:
https://medium.com/@keithwan.programming/doing-ssl-certificate-expiry-alert-in-node-js-1714ef15621

not sure though what is best in terms of splitting the work up (e.g. first release the feature without the expiration check) or combine but even without the expiration check there is value in this feature.

@louislam what are your thoughts ?

@parallelo3301
Copy link

parallelo3301 commented Apr 15, 2023

This is exactly what I'm looking for right now and have to use custom script to check the validity with the openssl client. Is there anything to do I could help with to get this merged in the future?

@hoerup
Copy link

hoerup commented May 7, 2023

@mrubli any news on this one ?

@chakflying chakflying added Stale area:monitor Everything related to monitors labels Dec 2, 2023
@github-actions github-actions bot closed this Dec 3, 2023
@ErnieB44
Copy link

Can someone please explain if this is already merged in to a release? And if not, are there any plans to do this in the near future? Can't wait to bring this feature to use..

Thanks

@CommanderStorm
Copy link
Collaborator

Can someone please explain if this is already merged in to a release?

No, the closest thing that exists is the https monitor. (you can configure the HOST Header)
This PR was closed due to being stale and without progress for quie a while.

And if not, are there any plans to do this in the near future?

Depends what you mean by near future.
All milestones with the PRs we want to review in said releases is written here: https://github.com/louislam/uptime-kuma/milestones

Currently, the core team is quite heavily focused on bringing v2.0 on the road. (see #4171)
Here is our contribution guide: https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md

@ErnieB44
Copy link

Can someone please explain if this is already merged in to a release?

No, the closest thing that exists is the https monitor. (you can configure the HOST Header) This PR was closed due to being stale and without progress for quie a while.

And if not, are there any plans to do this in the near future?

Depends what you mean by near future. All milestones with the PRs we want to review in said releases is written here: https://github.com/louislam/uptime-kuma/milestones

Currently, the core team is quite heavily focused on bringing v2.0 on the road. (see #4171) Here is our contribution guide: https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md

Hi @CommanderStorm , thanks for the quick repsonse.
It's a very nice to hear that the core team is working hard on v2. I've tried to use the HOST header but it fails.. would you be so kind to let me know if it is even possible what i'm trying to achieve? I want to monitor de SSL certificate on a Windows RDS server. So it should check the certificate at an endpoint on port 3389.

Thanks

@CommanderStorm
Copy link
Collaborator

I don't know what Windows RDS Server is and how it works.
If not supported, and you know how it should work, I would suggest you to use the push monitor instead or propose a monitor via the contribution guide above.

@d3xt3r01
Copy link

I don't think the app protocol is of any importance in this case as the expected check is at a different layer. TLS is TLS. It's the layer that handles the encryption. It just wraps the protocol in encryption. The monitor many of us want is just the tls part. To make sure the certificate is installed correctly and not expired. We're not trying to validate the application/protocol in any way.
That's why openssl s_client -connect fqdn:port should work with anything, no matter the app protocol.
In any case, a tcp check in this case usually causes tls errors on the other end since the connection is started on a tls port but tls negotiation doesn't finish and we get tons of SSL_accept(): Internal OpenSSL error or protocol error: : unexpected eof while reading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:monitor Everything related to monitors Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add SSL Certificate checks for General TCP ports
10 participants