[trel] limit direct peer sockaddr updates to mode 0/1 secured frames

[aussinfosec]

This PR makes a single change: the rx-based fallback that directly updates a TREL peer's socket address is restricted to frames secured with the network key (key ID mode 1) or the KEK (key ID mode 0), by replacing aFrame->GetSecurityEnabled() with aFrame->IsSecuredWith(RxFrame::kAllowKeyIdMode0 | RxFrame::kAllowKeyIdMode1) at the peer update site in Mac::HandleReceivedFrame().

Key ID mode 2 uses the well-known key and does not identify a specific sender, so mode 2 frames, like unsecured frames, now only trigger the discrepancy signal to the platform layer, matching the policy in ThreadLinkInfo::SetFrom().

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request hardens the security of TREL peer socket-address updates by tying them to verified identities. Previously, updates were keyed on unauthenticated TREL headers, creating a potential vulnerability where an attacker could rebind a peer's socket address. The changes ensure that only cryptographically verified frames can authorize these updates, while maintaining support for legitimate peer mobility.

Highlights

• MAC Security Enforcement: Restricted TREL peer socket-address updates to frames secured with network key (mode 1) or KEK (mode 0), explicitly rejecting unauthenticated mode-2 frames.
• Cross-Peer Consistency Check: Added a verification step to ensure the security-verified frame source matches the TREL header source, preventing unauthorized rebinding of peer socket addresses.
• MLE Lane Security: Enhanced CheckTrelPeerAddrOnSecureMleRx to require verified identity matching and frame-counter validation for direct socket-address updates.
• Regression Testing: Introduced three new nexus regression tests covering key-mode filtering, cross-peer consistency, and MLE lane security.

New Features

🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request enhances the security of TREL peer socket-address updates by ensuring updates are only authorized by frames secured with the network key (mode 1) or KEK (mode 0), and by adding cross-peer binding and MLE-secured source identity checks. It also introduces several regression tests to verify these changes. The review feedback highlights a few build issues: the new test file test_trel_peer_addr_update.cpp needs to be guarded with #if OPENTHREAD_CONFIG_RADIO_LINK_TREL_ENABLE to prevent compilation failures when TREL is disabled, and its CMake target requires the trel component dependency to avoid linker errors.

[github-actions]

Merging #13414 into main

name	branch	text	data	bss	total
ot-cli-ftd	803266d	481528	860	66556	548944
	2d98ae5	481528	860	66556	548944
	+/-	+0	+0	+0	+0
ot-ncp-ftd	803266d	446948	764	61792	509504
	2d98ae5	446948	764	61792	509504
	+/-	+0	+0	+0	+0
ot-cli-mtd	803266d	373736	764	50932	425432
	2d98ae5	373736	764	50932	425432
	+/-	+0	+0	+0	+0
ot-ncp-mtd	803266d	355108	764	46200	402072
	2d98ae5	355108	764	46200	402072
	+/-	+0	+0	+0	+0
ot-cli-ftd-br	803266d	592952	864	130708	724524
	2d98ae5	592952	864	130708	724524
	+/-	+0	+0	+0	+0
ot-rcp	803266d	63788	568	20828	85184
	2d98ae5	63788	568	20828	85184
	+/-	+0	+0	+0	+0

Library files

name	branch	text	data	bss	total
libopenthread-ftd.a	803266d	251301	95	40335	291731
	2d98ae5	251301	95	40335	291731
	+/-	+0	+0	+0	+0
libopenthread-cli-ftd.a	803266d	61777	0	8091	69868
	2d98ae5	61777	0	8091	69868
	+/-	+0	+0	+0	+0
libopenthread-ncp-ftd.a	803266d	33359	0	5948	39307
	2d98ae5	33359	0	5948	39307
	+/-	+0	+0	+0	+0
libopenthread-mtd.a	803266d	170847	0	24743	195590
	2d98ae5	170847	0	24743	195590
	+/-	+0	+0	+0	+0
libopenthread-cli-mtd.a	803266d	41482	0	8059	49541
	2d98ae5	41482	0	8059	49541
	+/-	+0	+0	+0	+0
libopenthread-ncp-mtd.a	803266d	25783	0	5948	31731
	2d98ae5	25783	0	5948	31731
	+/-	+0	+0	+0	+0
libopenthread-ftd-br.a	803266d	363451	99	104447	467997
	2d98ae5	363451	99	104447	467997
	+/-	+0	+0	+0	+0
libopenthread-cli-ftd-br.a	803266d	80892	0	8131	89023
	2d98ae5	80892	0	8131	89023
	+/-	+0	+0	+0	+0
libopenthread-rcp.a	803266d	9946	0	5060	15006
	2d98ae5	9946	0	5060	15006
	+/-	+0	+0	+0	+0
libopenthread-radio.a	803266d	20654	0	262	20916
	2d98ae5	20654	0	262	20916
	+/-	+0	+0	+0	+0

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.78%. Comparing base (803266d) to head (8ca6d8f).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13414      +/-   ##
==========================================
+ Coverage   69.16%   71.78%   +2.61%     
==========================================
  Files         711      711              
  Lines       97124   100292    +3168     
==========================================
+ Hits        67175    71992    +4817     
+ Misses      29949    28300    -1649     

Files with missing lines	Coverage Δ
src/core/mac/mac.cpp	84.48% <100.00%> (+3.34%)	⬆️

... and 178 files with indirect coverage changes

🚀 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.

[abtink]

Thanks for the detailed write-up and the PR.

I don't think most of these additional checks are strictly necessary. Here is some context on how TREL security and peer discovery are intended to work:

First, TREL packets are expected to be secured by the underlying TREL interface. The entire TREL UDP packet (both the TREL header and its payload) must be encrypted and authenticated by the underlying link layer (e.g., Wi-Fi link-layer security).

Background on TREL Peer Discovery

TREL peer discovery relies on mDNS, which itself provides no security beyond what the local link layer provides. Technically, any compromised or rogue device on the local network can spoof mDNS responses/announcements to claim another host's address or alter service ports. We effectively trust all devices on the local link; if a malicious device wants to cause disruptions, spoofing mDNS is by far the easiest vector.

Normally, when a device's socket address or port changes, it announces the update via mDNS. The TREL platform implementation is expected to detect this and notifies the OpenThread stack so it can update its peer table accordingly.

However, because mDNS updates aren't always delivered or detected reliably, this mechanism was added as a fallback: if we receive a valid TREL packet from a peer using a new socket address (IP or port), we update the peer info so we can communicate back using that new address.

During Thread Technical Committee discussions, I argued that since we already trust mDNS messages on the local link, we should treat TREL UDP packets on that same link with the same level of trust. In the end, we decided to restrict this fallback mechanism to secured MAC/MLE frames (the embedded frame within the received TREL packet) as a guardrail.

In practice, trying to over-protect this pathway doesn't add meaningful security when the primary discovery mechanism (mDNS) can be trivially spoofed on the local network. A local attacker would simply spoof mDNS rather than construct specialized TREL packets that pass Thread link/MLE security.

Summary / Recommendation

I think updating aFrame->GetSecurityEnabled() to aFrame->IsSecuredWith(RxFrame::kAllowKeyIdMode0 | RxFrame::kAllowKeyIdMode1) is a great improvement. However, beyond that change, I don't think the additional complexity introduced by the other changes in this PR is justified.

[aussinfosec]

@abtink Thanks a lot for the detailed background, and especially for the Technical Committee history on how the fallback and its guardrail came about. That context makes the trust model clear: the local link is trusted as a whole, mDNS discovery already carries that trust, and the TREL header is expected to be authenticated by the underlying interface. Agreed that the extra checks were guarding a boundary the design does not draw.

I have restructured the branch to contain exactly the change you endorsed: aFrame->IsSecuredWith(RxFrame::kAllowKeyIdMode0 | RxFrame::kAllowKeyIdMode1) at the peer update site in Mac::HandleReceivedFrame(), plus a single nexus regression test (a key ID mode 2 frame arriving from a new socket address must not update the peer entry; a mode 1 secured frame from a new socket address must). Since most of the previous diff was removed, I rewrote the branch history and force-pushed rather than stacking removal commits. The source-consistency and MLE receive path additions are gone, and both comment blocks you flagged went with them; you are right about the wording there, the TREL header's protection comes from the underlying interface.

One small question, mentioned only for completeness: when an entry is updated through the rx fallback, re-resolution keeps the rx-learned address unless the mDNS-advertised address list itself changes (the mSockAddrUpdatedBasedOnRx handling in PeerDiscoverer), so an rx-driven address can persist across re-resolutions, whereas an incorrect mDNS announcement would be corrected by the next genuine one. If that persistence is considered acceptable within the same local-link trust model, then the slimmed change covers everything.

[abtink]

LGTM. Thanks for making those changes, @aussinfosec!

Could you please also update the PR title and commit message to match the final changes? Thanks.

[aussinfosec]

Done in c021956 - squashed to a single commit whose message describes only the final mac.cpp change, and rewrote the PR summary above to match (the regression-test and history wording is gone). The title already reflects the final scope, so it is unchanged.

[jwhui]

@aussinfosec , I submitted #13438 to resolve the GitHub Actions Scan failure. Can you rebase?

[aussinfosec]

Done - rebased onto current main (803266d, includes #13438); the change is otherwise identical (8ca6d8f).

[jwhui]

Thanks! 👍🏼