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

TLS Expiry not checking intermediate redirect targets #3921

Open
2 tasks done
nneul opened this issue Oct 20, 2023 · 7 comments · May be fixed by #3942
Open
2 tasks done

TLS Expiry not checking intermediate redirect targets #3921

nneul opened this issue Oct 20, 2023 · 7 comments · May be fixed by #3942
Labels
area:cert-expiry related to certificate expiry notifications bug Something isn't working

Comments

@nneul
Copy link

nneul commented Oct 20, 2023

⚠️ Please verify that this bug has NOT been raised before.

  • I checked and didn't find similar issue

🛡️ Security Policy

Description

Had a service's cert expire today, haven't touched monitors for this for several weeks at least, and have received no notices until UTC rollover today that the cert fully expired. Monitor is configured for default 21/14/7.

👟 Reproduction steps

Not sure yet other than just having a cert that is pending expiration. Going to try to see if I can recreate scenario.

👀 Expected behavior

Some sort of alert at the defined thresholds.

😓 Actual Behavior

Only notified when it actually fully expired.

🐻 Uptime-Kuma Version

1.23.2

💻 Operating System and Arch

Ubuntu 20.04 x86

🌐 Browser

Chrome

🐋 Docker Version

24.0.6

🟩 NodeJS Version

No response

📝 Relevant log output

No response

@nneul nneul added the bug Something isn't working label Oct 20, 2023
@nneul
Copy link
Author

nneul commented Oct 20, 2023

One possibility that was discussed internally - is the expiration check handled in a separate https operation from the service check itself? Is there any chance that check is not sending SNI part of the interaction?

This particular cert was being delivered from a proxy server that hands out the certs via SNI, and the "default" cert (with no SNI element) is not expired.

@nneul
Copy link
Author

nneul commented Oct 20, 2023

image

@nneul
Copy link
Author

nneul commented Oct 20, 2023

Realized that easiest way to test this would just be to define a 90 day expiration warning on a server with a standard letsencrypt issued cert - that should fail almost immediately if the check is working properly.

@nneul
Copy link
Author

nneul commented Oct 20, 2023

So I set a 95 day expiration warning - and immediately received alerts from a FEW of my services, but NOT all that I would have expected.

Also noticed something additional - on ONE of the servers - the certificate error reported:

[home.domain.com][https://home.domain.com/] server certificate accounts.google.com will be expired in 52 days

That particular server kicks over to google for OIDC authentication. It really shouldn't be reporting the certificate of the target server - especially when the ACTUAL certificate on that box is set to expire in about 34 days. It should be reporting the certificate status before redirections are applied.

Hmm... That's what caused my initial problem, the servers that had the issue with all do redirects to OIDC/SAML auth providers, who's certificates are NOT expiring.

@nneul
Copy link
Author

nneul commented Oct 20, 2023

I think to do this and have it actually give the correct results, the code needs to do something leveraging the axios "beforeRedirect" callback function.

@nneul nneul changed the title TLS Expiry monitoring did not work - fired only when fully expired TLS Expiry monitoring fails - it's checking on final target of redirects when accessing URL, not the URL itself Oct 20, 2023
@nneul
Copy link
Author

nneul commented Oct 20, 2023

Confirmed - just set up an explicit monitor with redirects disabled, and immediately started getting the expiry alert. So at least I've got a workaround at the moment - I can define an extra "0 redirects" monitor for every https monitor - however, ideally, this should probably perform the tls cert check on every hop.

@CommanderStorm
Copy link
Collaborator

Could you edit the title to something shorter like TLS Expiry not checking intermediate redirect targets

See https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md if you are able to provide a fix for this behaviour ^^

@nneul nneul changed the title TLS Expiry monitoring fails - it's checking on final target of redirects when accessing URL, not the URL itself TLS Expiry not checking intermediate redirect targets Oct 21, 2023
@chakflying chakflying linked a pull request Oct 26, 2023 that will close this issue
7 tasks
@CommanderStorm CommanderStorm added the area:cert-expiry related to certificate expiry notifications label Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:cert-expiry related to certificate expiry notifications bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants