Skip to content

fix(camera): restore legacy TLS ciphers for old Reolink firmware, drop python 3.9 pin - #414

Open
MateoLostanlen wants to merge 5 commits into
developfrom
fix/reolink-legacy-tls
Open

fix(camera): restore legacy TLS ciphers for old Reolink firmware, drop python 3.9 pin#414
MateoLostanlen wants to merge 5 commits into
developfrom
fix/reolink-legacy-tls

Conversation

@MateoLostanlen

@MateoLostanlen MateoLostanlen commented Sep 8, 2026

Copy link
Copy Markdown
Member

Context

pyro-camera-api was stuck on Python 3.9 (#237) because one station runs Reolink cameras on old firmware whose HTTPS handshake fails with Python 3.10+.

Diagnosis on the station (RLC-810A, firmware v3.1.0.764_22022803): the camera negotiates TLS 1.2 with secure renegotiation, but only offers RSA key exchange ciphers (AES128-GCM-SHA256, no ECDHE). Python 3.10 removed all non PFS ciphers from its default list, so the handshake fails. Restoring OpenSSL's own cipher list fixes it, verified live against the camera with Python 3.11 on the Pi.

Changes

  • Add a _LegacyTLSAdapter (requests HTTPAdapter that restores OpenSSL's default cipher list, so RSA key exchange is offered again) in the Reolink adapter and in pyroengine/sensors.py; all camera calls now go through a session mounted with it. Same approach as reolink_aio used by Home Assistant. No security regression: these calls already used verify=False, min protocol stays TLS 1.2, and a camera that offers ECDHE still gets ECDHE.
  • The session sets trust_env = False (camera IPs are local, no proxy) and Connection: close so the pool never hands back a socket the camera dropped while idle, keeping the per-call behaviour of requests.get.
  • Bump pyro-camera-api Docker image from python:3.9.16-slim to python:3.11-slim-bookworm, aligned with the engine image, and requires-python to >=3.11; regenerate the camera-api lock and requirements for 3.11.
  • Move both Docker base images off EOL bullseye to bookworm (bullseye's security Release file expired for good in August 2026, which was breaking every image build).
  • Retarget tests/test_sensors.py mocks to the new session; add a test asserting the HTTPS session offers the RSA cipher these firmwares need.

@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.46%. Comparing base (968284f) to head (657240b).
⚠️ Report is 3 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #414      +/-   ##
===========================================
+ Coverage    78.83%   79.46%   +0.62%     
===========================================
  Files            6        6              
  Lines          841      852      +11     
===========================================
+ Hits           663      677      +14     
+ Misses         178      175       -3     
Flag Coverage Δ
unittests 79.46% <100.00%> (+0.62%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bullseye-security's Release file expired for good when bullseye LTS
ended in August 2026, so apt-get update fails in every image build.
@MateoLostanlen
MateoLostanlen requested a review from fe51 September 8, 2026 08:16
Keep only the proven need (OpenSSL default ciphers for RSA key exchange),
drop SECLEVEL=1 and legacy renegotiation, ignore proxy env vars so the
adapter cannot be bypassed, cover the session in CI-run tests, and restore
zip(strict=True) now that the runtime is Python 3.11.

@fe51 fe51 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @MateoLostanlen , thanks for this important PR

Cipher part looks fine to me. Checked it on 3.11: set_ciphers("DEFAULT") goes from 17 to 60 ciphers,
min protocol stays TLS 1.2, no RC4/3DES/NULL, and we were already on verify=False. A camera that offers
ECDHE still gets ECDHE. So no worry for the other stations on that point.

Four questions though:

  1. The shared _session worries me more than the ciphers. We went from a fresh connection per call
    to a pooled one with keep-alive, for every station. Reolink CGI drops idle connections and
    HTTPAdapter defaults to max_retries=0 — won't we start seeing random ConnectionError in the
    patrol loops? Did you test a station over a few hours, or should we just add
    _session.headers["Connection"] = "close" and keep the old behaviour?

  2. Same session is shared by the patrol threads and the FastAPI handlers. requests.Session isn't
    thread-safe (cookie jar). Probably harmless with Reolink, but is it intentional?

  3. 3.9 → 3.11 changes more than the TLS. requirements.txt has python_full_version markers, so
    the image now pulls requests 2.33.1, protobuf 7, websockets 16... plus bookworm (ffmpeg 4.3 → 5.1).
    None of that has ever run on a station. Can we soak it on one non-legacy site before the fleet?
    (I checked the ffmpeg flags, -vsync 1 still works on 5.1, so that part should be ok.)

  4. pyro_camera_api/pyproject.toml still says requires-python = ">=3.9" but pose_azimuths.py now
    uses zip(..., strict=True) (3.10+). Bump it to >=3.11?

Minor: the PR description still mentions SECLEVEL=1 and legacy renegotiation, which 0511a4e removed —
worth fixing, ops will read that. And pyro_camera_api/tests/ isn't in make test / CI (pytest tests/),
so the new test_reolink_tls.py never actually runs.

…3.11

Reolink CGI drops idle connections, so the pooled session could hand back
a stale socket (max_retries=0). Send Connection: close to restore the
per-call behaviour of requests.get.

Bump requires-python to >=3.11 (zip strict, 3.11 image) and regenerate the
camera-api lock and requirements for 3.11; resolved versions are unchanged
from what the universal lock already selected under 3.11.
@MateoLostanlen

Copy link
Copy Markdown
Member Author

Thanks @fe51 , I did the updates ! I'll deploy to Brison and Saint-peray and test, especially the live stream, before deploying all fleet

The 7 files in pyro_camera_api/tests were never executed: CI and make test
only ran pytest on tests/ from the repo root. Adds a pytest dependency group
for the camera API and wires the suite into tests.yml and make test.

Closes #418
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants