refactor: address CommanderStorm's review feedback
- Use i18n-t for description with code tag and RFC 8446 spec link - Add comment that TLS alert names are from spec (not translatable) - Refactor TCP monitor into smaller functions: - checkTcp() for standard TCP connectivity check - performStartTls() for STARTTLS handshake - checkTlsCertificate() for TLS certificate validation - attemptTlsConnection() for TLS connection with alert capture - Improve error messages with more context
This commit is contained in:
mkdev11
parent
327b51f304
commit
dc1e96f7d1
@ -113,6 +113,16 @@ class TCPMonitorType extends MonitorType {
|
||||
}
|
||||
|
||||
// Standard TCP check
|
||||
await this.checkTcp(monitor, heartbeat);
|
||||
}
|
||||
|
||||
/**
|
||||
* Standard TCP connectivity check
|
||||
* @param {object} monitor Monitor object
|
||||
* @param {object} heartbeat Heartbeat object
|
||||
* @returns {Promise<void>}
|
||||
*/
|
||||
async checkTcp(monitor, heartbeat) {
|
||||
try {
|
||||
const resp = await tcping(monitor.hostname, monitor.port);
|
||||
heartbeat.ping = resp;
|
||||
@ -124,11 +134,28 @@ class TCPMonitorType extends MonitorType {
|
||||
|
||||
let socket_;
|
||||
|
||||
const preTLS = () =>
|
||||
new Promise((resolve, reject) => {
|
||||
// Handle TLS certificate checking for secure/starttls connections
|
||||
if ([ "secure", "starttls" ].includes(monitor.smtpSecurity) && monitor.isEnabledExpiryNotification()) {
|
||||
const reuseSocket = monitor.smtpSecurity === "starttls" ? await this.performStartTls(monitor) : {};
|
||||
socket_ = reuseSocket.socket;
|
||||
await this.checkTlsCertificate(monitor, reuseSocket);
|
||||
}
|
||||
|
||||
if (socket_ && !socket_.destroyed) {
|
||||
socket_.end();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Perform STARTTLS handshake for various protocols (SMTP, IMAP, XMPP)
|
||||
* @param {object} monitor Monitor object
|
||||
* @returns {Promise<{socket: net.Socket}>} Object containing the socket
|
||||
*/
|
||||
performStartTls(monitor) {
|
||||
return new Promise((resolve, reject) => {
|
||||
let dialogTimeout;
|
||||
let bannerTimeout;
|
||||
socket_ = net.connect(monitor.port, monitor.hostname);
|
||||
const socket_ = net.connect(monitor.port, monitor.hostname);
|
||||
|
||||
const onTimeout = () => {
|
||||
log.debug(this.name, `[${monitor.name}] Pre-TLS connection timed out`);
|
||||
@ -196,10 +223,15 @@ class TCPMonitorType extends MonitorType {
|
||||
dialogTimeout = setTimeout(onTimeout, 1000 * TIMEOUT);
|
||||
bannerTimeout = setTimeout(onBannerTimeout, 1000 * 1.5);
|
||||
});
|
||||
}
|
||||
|
||||
const reuseSocket = monitor.smtpSecurity === "starttls" ? await preTLS() : {};
|
||||
|
||||
if ([ "secure", "starttls" ].includes(monitor.smtpSecurity) && monitor.isEnabledExpiryNotification()) {
|
||||
/**
|
||||
* Check TLS certificate validity
|
||||
* @param {object} monitor Monitor object
|
||||
* @param {object} reuseSocket Socket to reuse for STARTTLS
|
||||
* @returns {Promise<void>}
|
||||
*/
|
||||
async checkTlsCertificate(monitor, reuseSocket) {
|
||||
let socket = null;
|
||||
try {
|
||||
const options = {
|
||||
@ -244,11 +276,6 @@ class TCPMonitorType extends MonitorType {
|
||||
}
|
||||
}
|
||||
|
||||
if (socket_ && !socket_.destroyed) {
|
||||
socket_.end();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check for expected TLS alert (for mTLS verification)
|
||||
* @param {object} monitor Monitor object
|
||||
@ -277,12 +304,43 @@ class TCPMonitorType extends MonitorType {
|
||||
}
|
||||
}
|
||||
|
||||
const result = await new Promise((resolve, reject) => {
|
||||
const result = await this.attemptTlsConnection(monitor, options, startTime, timeout);
|
||||
|
||||
heartbeat.ping = result.responseTime;
|
||||
|
||||
// Handle TLS info for certificate expiry monitoring
|
||||
if (result.tlsInfo && monitor.isEnabledExpiryNotification()) {
|
||||
await monitor.handleTlsInfo(result.tlsInfo);
|
||||
}
|
||||
|
||||
// Check if we got the expected alert
|
||||
if (result.alertName === expectedTlsAlert) {
|
||||
heartbeat.status = UP;
|
||||
heartbeat.msg = `TLS alert received as expected: ${result.alertName} (${result.alertNumber})`;
|
||||
} else if (result.success) {
|
||||
throw new Error(`Expected TLS alert '${expectedTlsAlert}' but connection succeeded. The server accepted the connection without requiring a client certificate.`);
|
||||
} else if (result.alertNumber !== null) {
|
||||
throw new Error(`Expected TLS alert '${expectedTlsAlert}' but received '${result.alertName}' (${result.alertNumber})`);
|
||||
} else {
|
||||
throw new Error(`Expected TLS alert '${expectedTlsAlert}' but got unexpected error: ${result.errorMessage}`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Attempt TLS connection and capture result/alert
|
||||
* @param {object} monitor Monitor object
|
||||
* @param {object} options TLS connection options
|
||||
* @param {number} startTime Connection start timestamp
|
||||
* @param {number} timeout Connection timeout in ms
|
||||
* @returns {Promise<object>} Connection result with success, responseTime, tlsInfo, alertNumber, alertName, errorMessage
|
||||
*/
|
||||
attemptTlsConnection(monitor, options, startTime, timeout) {
|
||||
return new Promise((resolve, reject) => {
|
||||
const socket = tls.connect(options);
|
||||
|
||||
const timeoutId = setTimeout(() => {
|
||||
socket.destroy();
|
||||
reject(new Error("Connection timed out"));
|
||||
reject(new Error("TLS connection timed out"));
|
||||
}, timeout);
|
||||
|
||||
socket.on("secureConnect", () => {
|
||||
@ -331,28 +389,9 @@ class TCPMonitorType extends MonitorType {
|
||||
socket.on("timeout", () => {
|
||||
clearTimeout(timeoutId);
|
||||
socket.destroy();
|
||||
reject(new Error("Connection timed out"));
|
||||
reject(new Error("TLS connection timed out"));
|
||||
});
|
||||
});
|
||||
|
||||
heartbeat.ping = result.responseTime;
|
||||
|
||||
// Handle TLS info for certificate expiry monitoring
|
||||
if (result.tlsInfo && monitor.isEnabledExpiryNotification()) {
|
||||
await monitor.handleTlsInfo(result.tlsInfo);
|
||||
}
|
||||
|
||||
// Check if we got the expected alert
|
||||
if (result.alertName === expectedTlsAlert) {
|
||||
heartbeat.status = UP;
|
||||
heartbeat.msg = `TLS alert received as expected: ${result.alertName} (${result.alertNumber})`;
|
||||
} else if (result.success) {
|
||||
throw new Error(`Expected TLS alert '${expectedTlsAlert}' but connection succeeded`);
|
||||
} else if (result.alertNumber !== null) {
|
||||
throw new Error(`Expected TLS alert '${expectedTlsAlert}' but got '${result.alertName}' (${result.alertNumber})`);
|
||||
} else {
|
||||
throw new Error(`Expected TLS alert '${expectedTlsAlert}' but got error: ${result.errorMessage}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -406,6 +406,7 @@
|
||||
<label for="expected_tls_alert" class="form-label">{{ $t("Expected TLS Alert") }}</label>
|
||||
<select id="expected_tls_alert" v-model="monitor.expectedTlsAlert" class="form-select">
|
||||
<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')">
|
||||
<option value="certificate_required">certificate_required (116)</option>
|
||||
<option value="bad_certificate">bad_certificate (42)</option>
|
||||
@ -417,9 +418,14 @@
|
||||
<option value="certificate_revoked">certificate_revoked (44)</option>
|
||||
</optgroup>
|
||||
</select>
|
||||
<div class="form-text">
|
||||
{{ $t("expectedTlsAlertDescription") }}
|
||||
</div>
|
||||
<i18n-t tag="div" class="form-text" keypath="expectedTlsAlertDescription">
|
||||
<template #code>
|
||||
<code>certificate_required</code>
|
||||
</template>
|
||||
<template #link>
|
||||
<a href="https://www.rfc-editor.org/rfc/rfc8446#section-6.2" target="_blank" rel="noopener noreferrer">{{ $t("TLS Alert Spec") }}</a>
|
||||
</template>
|
||||
</i18n-t>
|
||||
</div>
|
||||
</template>
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user