Skip to content

Verify inbound E2EE media HMACs#196

Open
Adri11334 wants to merge 1 commit into
beeper:mainfrom
Adri11334:line-media-hmac
Open

Verify inbound E2EE media HMACs#196
Adri11334 wants to merge 1 commit into
beeper:mainfrom
Adri11334:line-media-hmac

Conversation

@Adri11334

Copy link
Copy Markdown
Contributor

Summary

  • verify inbound E2EE media HMACs before AES-CTR decryption
  • keep file-like media on the existing HMAC(ciphertext) convention
  • add a separate video decrypt path for the existing HMAC(generateChunkHashes(ciphertext)) convention
  • fail closed on ENC_KM fallback decrypt errors instead of uploading raw encrypted bytes
  • add unit coverage for valid media, tampered ciphertext, tampered tags, thumbnails, video chunk-hash HMACs, and HMAC mode mismatches

Fixes #191

Testing

  • GOWORK=off go test ./pkg/connector ./pkg/connector/handlers
  • GOWORK=off go vet ./pkg/connector ./pkg/connector/handlers
  • GOWORK=off go run honnef.co/go/tools/cmd/staticcheck@latest ./pkg/connector ./pkg/connector/handlers
  • GOWORK=off go run golang.org/x/tools/cmd/goimports@latest -local github.com/highesttt/matrix-line-messenger -l ...
  • git diff --check
  • docker build -t matrix-line-media-hmac .
  • local bridge E2E: sent a photo and video from LINE, both bridged to Beeper/Matrix successfully

Notes

The live E2E test covers valid media still bridging correctly. The HMAC failure behavior is covered at unit level by tampering encrypted media bytes and tags directly, which is the deterministic security property fixed here.

Copilot AI review requested due to automatic review settings June 29, 2026 19:11
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7d0f8643-357a-43ac-860c-a860ba000f07

📥 Commits

Reviewing files that changed from the base of the PR and between be98960 and a6d06b0.

📒 Files selected for processing (6)
  • pkg/connector/handle_message.go
  • pkg/connector/handlers/audio.go
  • pkg/connector/handlers/handler.go
  • pkg/connector/handlers/video.go
  • pkg/connector/media.go
  • pkg/connector/media_test.go
📜 Recent review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: Lint with 1.25
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Use go fmt for code formatting across all Go files
Use goimports with -local "github.com/highesttt/matrix-line-messenger" flag to group project-local imports correctly
Use zerolog for logging throughout the codebase
Do not use Msgf in logging; use Msg with structured fields instead
Use Stringer interface where applicable in Go code

Files:

  • pkg/connector/handlers/video.go
  • pkg/connector/handle_message.go
  • pkg/connector/handlers/handler.go
  • pkg/connector/media_test.go
  • pkg/connector/handlers/audio.go
  • pkg/connector/media.go
**/!(ltsm)/**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/!(ltsm)/**/*.go: Run staticcheck on all Go files excluding pkg/ltsm package (transpiled WASM code)
Run go vet on all Go files excluding pkg/ltsm package (transpiled WASM code)

Files:

  • pkg/connector/handlers/video.go
  • pkg/connector/handle_message.go
  • pkg/connector/handlers/handler.go
  • pkg/connector/media_test.go
  • pkg/connector/handlers/audio.go
  • pkg/connector/media.go
🔇 Additional comments (6)
pkg/connector/media_test.go (1)

1-111: LGTM!

pkg/connector/media.go (1)

28-35: LGTM!

Also applies to: 52-80

pkg/connector/handle_message.go (1)

35-35: LGTM!

pkg/connector/handlers/video.go (1)

91-91: LGTM!

Also applies to: 111-117

pkg/connector/handlers/audio.go (1)

100-103: LGTM!

pkg/connector/handlers/handler.go (1)

27-28: 🩺 Stability & Availability

No change needed; DecryptVideoMedia is set in the only handlers.Handler constructor.


📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Video attachments now use a dedicated decryption path for improved handling of encrypted media.
  • Bug Fixes

    • Audio and video decryption now fail clearly when encrypted media cannot be decoded, instead of continuing with unreadable content.
    • Media integrity checks are stricter, helping prevent playback of tampered or mismatched encrypted files.
  • Tests

    • Added coverage for image, thumbnail, and video decryption integrity checks.

Walkthrough

decryptMediaData now derives and uses macKey to verify the trailing 32-byte HMAC before decrypting. decryptVideoData passes generateChunkHashes as the MAC input while decryptImageData uses the raw ciphertext. A new DecryptVideoMedia field is added to Handler and wired to decryptVideoData. Audio and video ENC_KM fallback paths now return errors on decryption failure instead of continuing with raw bytes. Three unit tests verify HMAC enforcement.

Changes

E2EE Media HMAC Verification

Layer / File(s) Summary
decryptMediaData HMAC verification and video split
pkg/connector/media.go
decryptMediaData now assigns macKey, slices encryptedData into ciphertext and receivedMAC, computes HMAC-SHA256(macKey, macInput(ciphertext)), and rejects mismatches. decryptImageData and decryptVideoData delegate with identity vs. generateChunkHashes as the macInput function. AES-CTR decryption XORs into a fresh decrypted buffer over ciphertext.
DecryptVideoMedia field, wiring, and handler call-sites
pkg/connector/handlers/handler.go, pkg/connector/handle_message.go, pkg/connector/handlers/video.go, pkg/connector/handlers/audio.go
Handler gains DecryptVideoMedia func(data []byte, keyMaterial string) ([]byte, error). newMessageHandler sets it to lc.decryptVideoData. Both video decrypt branches call h.DecryptVideoMedia and return errors on failure. Audio ENC_KM fallback also returns an error instead of continuing with raw bytes.
HMAC verification unit tests
pkg/connector/media_test.go
Adds TestDecryptImageDataVerifiesHMAC, TestDecryptThumbnailDataVerifiesHMAC, and TestDecryptVideoDataVerifiesChunkHashHMAC covering successful round-trips and rejection of tampered ciphertext, tampered MAC bytes, and wrong HMAC mode.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: verifying inbound E2EE media HMACs.
Description check ✅ Passed The description matches the changes and summarizes the HMAC verification, video path, fail-closed behavior, and tests.
Linked Issues check ✅ Passed The diff implements the issue's requirements: MAC verification, video chunk-hash handling, fail-closed errors, and tests.
Out of Scope Changes check ✅ Passed The changes stay focused on media HMAC verification, video handling, fail-closed decryption, and test coverage.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the inbound E2EE media pipeline by authenticating encrypted media with HMAC-SHA256 before AES-CTR decryption, aligning receive-side behavior with the existing send-side HMAC conventions (including the special video chunk-hash mode).

Changes:

  • Add a shared decrypt helper that verifies the trailing 32-byte HMAC prior to AES-CTR decryption, with separate HMAC input modes for file-like media vs. video.
  • Route video decryption through a dedicated DecryptVideoMedia hook to match the HMAC(generateChunkHashes(ciphertext)) convention.
  • Fail closed on ENC_KM fallback decrypt failures (audio/video) and add unit tests covering valid/tampered media and HMAC mode mismatches.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pkg/connector/media.go Adds HMAC verification on inbound decrypt and introduces a video-specific decrypt path using chunk-hash MAC input.
pkg/connector/media_test.go Adds unit coverage for valid decrypts, tampering, and HMAC-mode mismatch behavior (file-like vs video).
pkg/connector/handlers/video.go Uses the new video decrypt hook and fails closed on ENC_KM fallback decrypt errors.
pkg/connector/handlers/handler.go Extends handler dependencies with DecryptVideoMedia for video-specific HMAC mode.
pkg/connector/handlers/audio.go Fails closed on ENC_KM fallback decrypt errors instead of uploading raw encrypted bytes.
pkg/connector/handle_message.go Wires DecryptVideoMedia to the LineClient’s video decrypt implementation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

bug: inbound E2EE media HMAC is not verified before decrypting

2 participants