Skip to content

fix: verify TLS in poller clients#4210

Merged
Scottcjn merged 1 commit intoScottcjn:mainfrom
bolasse1234:codex/verify-poller-tls
May 10, 2026
Merged

fix: verify TLS in poller clients#4210
Scottcjn merged 1 commit intoScottcjn:mainfrom
bolasse1234:codex/verify-poller-tls

Conversation

@bolasse1234
Copy link
Copy Markdown
Contributor

Summary:

  • add get_ssl_context() to the shared TLS helper for urllib/websocket clients
  • switch the agent reputation, websocket feed, and explorer websocket pollers away from unconditional unverified TLS contexts
  • keep an explicit local-development escape hatch via RUSTCHAIN_TLS_VERIFY=false
  • add tests for the verified-by-default behavior and explicit opt-out behavior

Security note:

  • Severity: Low
  • Scope: three background pollers plus the shared TLS helper
  • Previously these clients disabled certificate verification for every HTTPS node request. The new default verifies certificates or a pinned ~/.rustchain/node_cert.pem certificate.

Verification:

  • python -m pytest tests\test_tls_config.py
  • python -m pytest explorer\test_explorer_websocket.py::TestWebSocketConfiguration::test_default_configuration
  • python -m py_compile agent_reputation.py websocket_feed.py explorer\explorer_websocket_server.py node\tls_config.py

Bounty note:
If this is accepted for the bounty loop, please use my GitHub-login miner_id for payout. I am not posting payment details publicly.

@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related tests Test suite changes size/M PR: 51-200 lines labels May 10, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

  • Your PR has a BCOS-L1 or BCOS-L2 label
  • New code files include an SPDX license header
  • You've tested your changes against the live node

Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150)

A maintainer will review your PR soon. Thanks for contributing!

Copy link
Copy Markdown
Contributor

@cerredz cerredz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TLS verification direction is good, and the targeted tests pass locally, but this currently regresses the default poller configuration.

All three touched poller modules still default to the raw IP node URL (https://50.28.86.131). With the new verified SSLContext, that default endpoint fails hostname verification because the certificate is not valid for the IP address. The failure is then swallowed by the poller fetch helpers, so default deployments just stop receiving node data unless users already know to override RUSTCHAIN_NODE_URL.

Repro on this PR branch using get_ssl_context() directly:

https://50.28.86.131/health URLError <urlopen error [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: IP address mismatch, certificate is not valid for '50.28.86.131'. (_ssl.c:992)>
https://rustchain.org/health 200

And through one touched poller:

NODE_URL https://50.28.86.131
health None

If I set RUSTCHAIN_NODE_URL=https://rustchain.org before importing websocket_feed, the same _fetch('/health') returns a healthy JSON response. I would either switch these modules' default NODE_URL values to the certificate-valid hostname, or otherwise ensure the verified context is paired with a default URL whose certificate can pass hostname verification. A regression test around the default node URL scheme/host would help keep the TLS-hardening patch from silently disabling the pollers.

Validation I ran:

  • python -m pytest tests\\test_tls_config.py -q => 2 passed
  • python -m pytest explorer\\test_explorer_websocket.py::TestWebSocketConfiguration::test_default_configuration -q => 1 passed
  • python -m py_compile agent_reputation.py websocket_feed.py explorer\\explorer_websocket_server.py node\\tls_config.py tests\\test_tls_config.py => passed
  • git diff --check origin/main...HEAD => passed

@bolasse1234 bolasse1234 force-pushed the codex/verify-poller-tls branch from 94871aa to 6e67db9 Compare May 10, 2026 21:36
@bolasse1234
Copy link
Copy Markdown
Contributor Author

Thanks for catching that. I updated the three verified-TLS clients to default to https://rustchain.org instead of the raw node IP, so hostname verification matches the certificate by default.

I also added a regression check for those defaults and re-ran:

  • python -m pytest tests\test_tls_config.py -q => 3 passed
  • python -m pytest explorer\test_explorer_websocket.py::TestWebSocketConfiguration::test_default_configuration -q => 1 passed
  • python -m py_compile agent_reputation.py websocket_feed.py explorer\explorer_websocket_server.py node\tls_config.py tests\test_tls_config.py
  • live verified-context probe to https://rustchain.org/health => HTTP 200

@Scottcjn
Copy link
Copy Markdown
Owner

💰 PAID — 15 RTC pending, will confirm in 24h.

  • tx hash: e58f0ebdef617ba8a068bd2642562d42
  • Pending ID: 1409

Why this rate

TLS verification across 5 files is Medium-tier security hardening — the kind of cross-cutting fix that closes a class of vulnerability rather than a single instance. Each file change is small and scoped; the test coverage for tls_config validates the actual behavior.

Third clean PR today. Consistency is what builds contributor trust here — keep going.

— auto-triage 2026-05-10

@Scottcjn Scottcjn merged commit 0c42879 into Scottcjn:main May 10, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related size/M PR: 51-200 lines tests Test suite changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants