Fix based on Coverity and Sonar audits (part 12) - #22340
Conversation
Sonar c:S5527 (CRITICAL vulnerability): mqtt_wss_connect() configured the peer certificate chain check and sent SNI via SSL_set_tlsext_host_name(), but it never asked OpenSSL to verify that the certificate's CN/SAN matched the requested hostname. A certificate issued by a trusted CA for any other host would pass the existing checks, allowing a man-in-the-middle to impersonate the target. Add SSL_set1_host(client->ssl, client->target_host) before SSL_connect() so OpenSSL's certificate verification also matches the peer's identity against the requested hostname. The call is guarded by the existing MQTT_WSS_SSL_DONT_CHECK_CERTS opt-out so callers that explicitly want to skip cert checks are unchanged. Failure to configure the hostname is reported and returns the same TLS-setup error code path as the SNI failure above; silently leaving the verification disabled would re-introduce the vulnerability.
…P mismatch The previous commit added SSL_set1_host() to enable TLS hostname verification. This catches a real MITM gap on production Cloud connections, but it tightens the rules for on-prem deployments where the cert's CN/SAN may legitimately not match the hostname the agent connects to: - the cert was issued for a different DNS name and reached via an alias / CNAME / load-balancer entry, - the cert was issued for a hostname but the agent is configured with the IP literal, - the cert is self-signed without proper SAN entries (CN-only). These were already the kind of setups MQTT_WSS_SSL_ALLOW_SELF_SIGNED was meant for. Extend cert_verify_callback so this flag also accepts X509_V_ERR_HOSTNAME_MISMATCH and X509_V_ERR_IP_ADDRESS_MISMATCH (in addition to the existing self-signed-leaf override). Default deployments (no flag) keep strict hostname verification; on-prem operators who set MQTT_WSS_SSL_ALLOW_SELF_SIGNED keep working without having to fall back to the much blunter MQTT_WSS_SSL_DONT_CHECK_CERTS.
Use X509_VERIFY_PARAM_set1_host through SSL_get0_param for OpenSSL 1.0.2 compatibility while preserving hostname verification behavior.
X509_VERIFY_PARAM_set1_host() only matches against the certificate's dNSName SAN. When the agent connects to an IP literal (e.g. an on-prem deployment configured with 10.0.0.5 instead of a hostname), the verification fails even if the certificate has a valid iPAddress SAN because the IP literal is interpreted as a DNS name. Try X509_VERIFY_PARAM_set1_ip_asc() first -- it parses the input as an IP and matches against the iPAddress SAN, returning 0 when the input is not a valid IP. If that fails (the typical DNS hostname case), fall back to X509_VERIFY_PARAM_set1_host(). Both code paths are guarded by the existing MQTT_WSS_SSL_DONT_CHECK_CERTS opt-out, and the MQTT_WSS_SSL_ALLOW_SELF_SIGNED override already covers X509_V_ERR_IP_ADDRESS_MISMATCH for setups whose certs do not match.
|
|
@cubic-dev-ai review this PR |
@stelfrag I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
Pull request overview
Updates the MQTT-over-WebSockets (WSS) client TLS verification behavior to better align with audit findings and common on-prem deployment needs.
Changes:
- Extends the certificate verify callback to optionally accept additional TLS verification failures (hostname/IP mismatch) when
MQTT_WSS_SSL_ALLOW_SELF_SIGNEDis enabled. - Adds explicit TLS peer identity verification configuration for
client->target_host, supporting both DNS hostnames and IP literals via OpenSSL verify parameters.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@cubic-dev-ai review this PR |
@stelfrag I have started the AI code review. It will take a few minutes to complete. |
thiagoftsm
left a comment
There was a problem hiding this comment.
Connection with cloud and the chart visualization are working as expected. LGTM!
* mqtt_wss: enable TLS hostname verification via SSL_set1_host Sonar c:S5527 (CRITICAL vulnerability): mqtt_wss_connect() configured the peer certificate chain check and sent SNI via SSL_set_tlsext_host_name(), but it never asked OpenSSL to verify that the certificate's CN/SAN matched the requested hostname. A certificate issued by a trusted CA for any other host would pass the existing checks, allowing a man-in-the-middle to impersonate the target. Add SSL_set1_host(client->ssl, client->target_host) before SSL_connect() so OpenSSL's certificate verification also matches the peer's identity against the requested hostname. The call is guarded by the existing MQTT_WSS_SSL_DONT_CHECK_CERTS opt-out so callers that explicitly want to skip cert checks are unchanged. Failure to configure the hostname is reported and returns the same TLS-setup error code path as the SNI failure above; silently leaving the verification disabled would re-introduce the vulnerability. * mqtt_wss: extend MQTT_WSS_SSL_ALLOW_SELF_SIGNED to cover hostname / IP mismatch The previous commit added SSL_set1_host() to enable TLS hostname verification. This catches a real MITM gap on production Cloud connections, but it tightens the rules for on-prem deployments where the cert's CN/SAN may legitimately not match the hostname the agent connects to: - the cert was issued for a different DNS name and reached via an alias / CNAME / load-balancer entry, - the cert was issued for a hostname but the agent is configured with the IP literal, - the cert is self-signed without proper SAN entries (CN-only). These were already the kind of setups MQTT_WSS_SSL_ALLOW_SELF_SIGNED was meant for. Extend cert_verify_callback so this flag also accepts X509_V_ERR_HOSTNAME_MISMATCH and X509_V_ERR_IP_ADDRESS_MISMATCH (in addition to the existing self-signed-leaf override). Default deployments (no flag) keep strict hostname verification; on-prem operators who set MQTT_WSS_SSL_ALLOW_SELF_SIGNED keep working without having to fall back to the much blunter MQTT_WSS_SSL_DONT_CHECK_CERTS. * mqtt_wss: address PR-review findings Use X509_VERIFY_PARAM_set1_host through SSL_get0_param for OpenSSL 1.0.2 compatibility while preserving hostname verification behavior. * mqtt_wss: support IP literals in TLS hostname verification X509_VERIFY_PARAM_set1_host() only matches against the certificate's dNSName SAN. When the agent connects to an IP literal (e.g. an on-prem deployment configured with 10.0.0.5 instead of a hostname), the verification fails even if the certificate has a valid iPAddress SAN because the IP literal is interpreted as a DNS name. Try X509_VERIFY_PARAM_set1_ip_asc() first -- it parses the input as an IP and matches against the iPAddress SAN, returning 0 when the input is not a valid IP. If that fails (the typical DNS hostname case), fall back to X509_VERIFY_PARAM_set1_host(). Both code paths are guarded by the existing MQTT_WSS_SSL_DONT_CHECK_CERTS opt-out, and the MQTT_WSS_SSL_ALLOW_SELF_SIGNED override already covers X509_V_ERR_IP_ADDRESS_MISMATCH for setups whose certs do not match. --------- Co-authored-by: Costa Tsaousis <costa@netdata.cloud> (cherry picked from commit c655511)



Summary