Monitor considered up whenever TCP succeeds yet TLS fails

[770grappenmaker]

📑 I have found these related issues/pull requests

I have been unable to find related issues and/or pull requests.

🛡️ Security Policy

• I have read and agree to Uptime Kuma's Security Policy.

📝 Description

As the title says. I have a setup where I am proxying a service that uses TLS. The TCP connection to the proxy succeeds, and I have then configured uptime-kuma to attempt a TLS handshake. The handshake fails, because the remote immediately disconnects, since the backend of the proxy is down. However, the monitor is still up:

👟 Reproduction steps

1. Start some sort of TCP server (netcat?) that immediately disconnects after the first data packet is sent. Alternatively, use some sort of TCP proxy service that points to nothing
2. Add a monitor to uptime-kuma for this faulty TCP server
3. Observe the message "TLS connection failed", yet the monitor is up

👀 Expected behavior

The monitor is reported as DOWN and the appropriate notifications get sent out

😓 Actual Behavior

Monitor stays up even though no successful connection was made whatsoever

🐻 Uptime-Kuma Version

2.2.1

💻 Operating System and Arch

Alpine Linux v3.22 (Linux 6.12.74-0-virt x86_64)

🌐 Browser

Firefox

🖥️ Deployment Environment

• Runtime Environment:
  • Docker: Version 28.3.3 (Build 980b85681696fbd95927fd8ded8f6d91bdca95b0)
  • Docker Compose: Version 2.36.2
• Database:
  • SQLite: Embedded
• Database Storage:
  • Filesystem:
    • Linux: ext4
  • Storage Medium: NVMe
• Uptime Kuma Setup:
  • Number of monitors: 10

📝 Relevant log output

There is no relevant log output :(

[CommanderStorm]

Not sure if that is a bug or working as intended. Feel free to look into the code to see how we handle this.

I think TLS under our current architecture is a message notifciation, so might not be able to trigger DOWN.

[770grappenmaker]

That is a good question, I'll investigate this later. Still, i'd probably like an option to have this trigger DOWN (especially since you can set "expected TLS notification" to "none (successful connection). If I have the time I might make a pull request

[770grappenmaker]

Interesting sidenote: the warning "TLS connection failed" does not even show up when "Certificate expiry notification" is disabled.

[770grappenmaker]

I now realize this is a duplicate of #2860 and #2557

[770grappenmaker]

Can reproduce with

#include <stdio.h>
#include <unistd.h>
#include <sys/socket.h>
#include <errno.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <netinet/tcp.h>

int main(void) {
	int family = AF_INET;
	int fd = socket(family, SOCK_STREAM, 0);
	if (fd == -1) {
		fprintf(stderr, "ERROR: couldn't open socket\n");
		perror("socket");
		return 1;
	}

	struct sockaddr_in addr = {0};

	char * addr_str = "0.0.0.0";
	addr.sin_family = family;
	addr.sin_port = htons(18932);

	if (inet_pton(family, addr_str, &addr.sin_addr) == 0) {
		fprintf(stderr, "ERROR: invalid address\n");
		perror("inet_pton");
		return 1;
	}

	if (bind(fd, (struct sockaddr *) &addr, sizeof(addr))) {
		fprintf(stderr, "ERROR: failed to bind to socket address\n");
		perror("bind");
		return 1;
	}

	if (listen(fd, 0)) {
		fprintf(stderr, "ERROR: could not listen to TCP socket\n");
		perror("listen");
		return 1;
	}

	for (;;) {
		int remote = accept(fd, NULL, NULL);
		if (remote == -1) {
			fprintf(stderr, "ERROR: failed to accept socket connection\n");
			perror("accept");
			continue;
		}

		printf("Got connection, killing it again\n");
		if (close(remote)) {
			fprintf(stderr, "ERROR: failed to close connection\n");
			perror("close");
		}
	}
}

[770grappenmaker]

The following patch would solve my problem

diff --git a/server/monitor-types/tcp.js b/server/monitor-types/tcp.js
index afb4867c..09695066 100644
--- a/server/monitor-types/tcp.js
+++ b/server/monitor-types/tcp.js
@@ -107,7 +107,7 @@ class TCPMonitorType extends MonitorType {
         const expectedTlsAlert = monitor.expected_tls_alert;
 
         // If expecting a TLS alert, use TLS connection with alert detection
-        if (expectedTlsAlert && expectedTlsAlert !== "none") {
+        if (expectedTlsAlert && expectedTlsAlert !== "any") {
             await this.checkTlsAlert(monitor, heartbeat, expectedTlsAlert);
             return;
         }
diff --git a/src/lang/en.json b/src/lang/en.json
index 976a3f76..fda98be2 100644
--- a/src/lang/en.json
+++ b/src/lang/en.json
@@ -1500,6 +1500,7 @@
     "passwordTooWeak": "Password is too weak. It should contain alphabetic and numeric characters. It must be at least 6 characters in length.",
     "TLS Alerts": "TLS Alerts",
     "Expected TLS Alert": "Expected TLS Alert",
+    "Unset": "Unset",
     "None (Successful Connection)": "None (Successful Connection)",
     "expectedTlsAlertDescription": "Select the TLS alert you expect the server to return. Use {code} to verify mTLS endpoints reject connections without client certificates. See {link} for details.",
     "TLS Alert Spec": "RFC 8446",
diff --git a/src/pages/EditMonitor.vue b/src/pages/EditMonitor.vue
index ac152a87..7562acdf 100644
--- a/src/pages/EditMonitor.vue
+++ b/src/pages/EditMonitor.vue
@@ -792,6 +792,7 @@
                                         v-model="monitor.expectedTlsAlert"
                                         class="form-select"
                                     >
+                                        <option value="any">{{ $t("Unset") }}</option>
                                         <option value="none">{{ $t("None (Successful Connection)") }}</option>
                                         <!-- TLS alert names are from RFC 8446 spec and should NOT be translated -->
                                         <optgroup :label="$t('TLS Alerts')">

as this gives me enough control to decide whether to care about a successful TLS connection or not. This is not a polished solution, though.

[CommanderStorm]

This is not a polished solution, though.

What is beteen your poroposal and a polished solution?

[770grappenmaker]

Polish :p
I imagined this proposal is too hacky / unclear, and this still doesn't work properly I believe when certificate expiry notification is disabled, which is already the case for the current version of uptime-kuma