Repository navigation
security: hardening pass — TLS hostname verification, async UAF fixes, secret zeroization, input validation - #7
Merged
Conversation
…ound untrusted input - Set Paho SSL 'verify' (hostname check) from new TlsOptions::verify_hostname (default true); previously never set, so any CA-signed cert impersonated the broker (CWE-297) - Heap-own Paho response contexts (AsyncOp) so late callbacks after wait_for() timeouts cannot touch destroyed stack promises (CWE-416); delete on synchronous failure - Rebind MQTT callbacks in move ctor/assign; reset shallow-copied ssl_opts_ (CWE-416) - Reuse the client handle across reconnects so in-flight publishes cannot hold a destroyed handle (CWE-367) - Copy config callbacks/caps under mutex in on_message_arrived; never invoke user callbacks under a lock (CWE-362) - Strict JSON parser for STATE online; drop oversized inbound payloads via Config::max_payload_bytes (CWE-20, CWE-400) - Exception barriers around user callback dispatch; sanitize log messages (CWE-248, CWE-117); warn on credentials over plaintext transport (CWE-319)
Reject empty/oversized group/edge-node/device ids (>128 chars), device ids on node-scoped messages, missing device ids on device-scoped messages, and trailing components (previously silently ignored). These strings become permanent host-state keys and log content (CWE-400, CWE-117).
- Wrap all publisher/payload extern "C" functions in try/catch so C++ exceptions cannot unwind through the C boundary (CWE-248) - Reject lengths > INT_MAX before static_cast<int> narrowing into protobuf parse/serialize calls (CWE-190) - Fix header docs: payload handles are not thread-safe; destroy() must not race other calls; returned string pointers are invalidated by mutation
- Remove certs/passwordfile (admin/admin hash; CI regenerates it) - Auth/TCK examples read MQTT_USERNAME/MQTT_PASSWORD env vars; warn when --password is passed via argv (visible in process listings) - Mimic examples warn that the public plaintext broker is world-readable - TLS_SETUP.md: drop --insecure from the verification snippet, fix stale API/example names; README: recommend ssl:// with trust store in production
Secrets are adopted from Config into detail::SecureBuffer members that scrub their storage on clear, move, and destruction; the source std::string copies are zeroed immediately (volatile writes, immune to dead-store elimination) so plaintext no longer lingers in the heap for the object's lifetime or after frees (CWE-316). Public API unchanged; Paho retains its own copy for the connection lifetime, which is outside the library's control.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 37912030 | Triggered | Generic Password | 75c980e | src/host_application.cpp | View secret |
| 37912029 | Triggered | Generic Password | 00e5ece | examples/test_auth_password.cpp | View secret |
| 37912028 | Triggered | Generic Password | 00e5ece | examples/tck_host_application_main.cpp | View secret |
| 37912029 | Triggered | Generic Password | 00e5ece | examples/test_auth_combined.cpp | View secret |
| 37912028 | Triggered | Generic Password | 00e5ece | examples/tck_edge_node_main.cpp | View secret |
| 37912030 | Triggered | Generic Password | 75c980e | src/edge_node.cpp | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A focused security hardening pass across five commits. No public API removals; one default flip (TLS hostname verification on) and one removed committed credential file are deliberate behavior changes, called out below.
7cb0581— TLS hostname verification, async-context UAFs, bounded inputverify(hostname check) was never set, so any CA-signed certificate could impersonate the broker. NewTlsOptions::verify_hostname, defaulttrue— deployments pointing at certs whose SAN/CN don't match the connect host will now fail the handshake until they fix the cert or opt out explicitly.AsyncOp) so late callbacks afterwait_for()timeouts cannot touch destroyed stack promises; rebind MQTT callbacks in move ctor/assign and reset shallow-copiedssl_opts_; reuse the client handle across reconnects so in-flight publishes cannot hold a destroyed handle (CWE-367).on_message_arrived; user callbacks never invoked under a lock.Config::max_payload_bytes.a51d658— topic validation inTopic::parseRejects empty/oversized group/edge-node/device ids (>128 chars), device ids on node-scoped messages, missing device ids on device-scoped messages, and trailing components that were previously silently ignored. These strings become permanent host-state keys and log content (CWE-400, CWE-117). Adds 56 lines of cases to
tests/test_topic.cpp.880cec6— C API exception barriers and narrowing guardsextern "C"functions wrapped in try/catch so C++ exceptions cannot unwind through the C boundary (CWE-248).INT_MAXrejected beforestatic_cast<int>narrowing into protobuf parse/serialize calls (CWE-190).destroy()must not race other calls; returned string pointers are invalidated by mutation.00e5ece— committed credentials removed, examples and TLS docs hardenedcerts/passwordfile(admin/adminhash) removed — CI regenerates it; anyone depending on the committed file must adjust.MQTT_USERNAME/MQTT_PASSWORDenv vars; warn when--passwordis passed via argv (visible in process listings).TLS_SETUP.md: dropped--insecurefrom the verification snippet, fixed stale API/example names;README.mdrecommendsssl://with a trust store in production.75c980e— secret zeroizationMQTT password and TLS key passphrase are adopted from
Configintodetail::SecureBuffermembers that scrub storage on clear, move, and destruction; the sourcestd::stringcopies are zeroed immediately with volatile writes (immune to dead-store elimination) so plaintext no longer lingers on the heap (CWE-316). Paho retains its own copy for the connection lifetime — outside the library's control.Breaking / behavioral changes
TlsOptions::verify_hostname = false.certs/passwordfileremoved (CI regenerates).Test plan
tests/test_topic.cpp— new validation cases