[mac] reject unsecured Enh-Ack carrying IEs in response to a secured transmission

[aussinfosec]

Per the Thread specification, in response to a transmitted MAC-secured Frame Version 0b10 frame a device MUST accept either an Enh-Ack frame that is MAC-secured, or an Enh-Ack frame that is not MAC-secured and does not carry any IEs (thanks @abtink for the precise reference).

Mac::ProcessEnhAckSecurity() currently accepts ANY unsecured ack for a secured version-2015 transmission via the early kErrorNone return, including an unsecured Enh-Ack carrying IEs, which falls outside both acceptable categories. Such an ack then reaches ProcessEnhAckProbing(), which parses its Link Metrics probing IE with no security gate (unlike ProcessCsl(), which requires key ID mode 1 security).

This change rejects an unsecured Enh-Ack that carries IEs in response to a secured version-2015 transmission, and continues to accept all other unsecured acks: unsecured transmissions, IE-less unsecured Enh-Acks (as the specification requires), and Imm-Acks (which cannot carry IEs).

[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 strengthens the MAC layer security in OpenThread by ensuring that secured IEEE 802.15.4-2015 frames are only acknowledged by secured Enhanced Acks. Previously, unsecured Acks were incorrectly accepted, which could lead to security vulnerabilities and incorrect neighbor link information. The changes include a validation check in the MAC processing logic and a comprehensive regression test to ensure future compliance.

Highlights

• Security Enforcement: Updated Mac::ProcessEnhAckSecurity to reject unsecured Enhanced Acks when responding to secured IEEE 802.15.4-2015 transmissions, preventing potential silent data loss and unverified radio metadata injection.
• Regression Testing: Added a new nexus regression test that uses an ack-intercept hook to verify that unsecured Acks for secured transmissions are correctly rejected, triggering retransmissions.

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 ensures that unsecured Enhanced Acknowledgments (Enh-Acks) are rejected for secured IEEE 802.15.4-2015 transmissions in Mac::ProcessEnhAckSecurity(). This prevents silent data loss and unverified radio metadata from affecting neighbor link info. It also introduces a test-only ACK interception hook in the Nexus simulation platform and adds a regression test to verify this behavior. There are no review comments, so I have no feedback to provide.

[github-actions]

Merging #13417 into main

name	branch	text	data	bss	total
ot-cli-ftd	2fea118	481616	860	66540	549016
	15cc3c2	481648	860	66540	549048
	+/-	+32	+0	+0	+32
ot-ncp-ftd	2fea118	447036	764	61776	509576
	15cc3c2	447068	764	61776	509608
	+/-	+32	+0	+0	+32
ot-cli-mtd	2fea118	373768	764	50932	425464
	15cc3c2	373832	764	50932	425528
	+/-	+64	+0	+0	+64
ot-ncp-mtd	2fea118	355140	764	46200	402104
	15cc3c2	355204	764	46200	402168
	+/-	+64	+0	+0	+64
ot-cli-ftd-br	2fea118	592992	864	130684	724540
	15cc3c2	593040	864	130684	724588
	+/-	+48	+0	+0	+48
ot-rcp	2fea118	63788	568	20828	85184
	15cc3c2	63788	568	20828	85184
	+/-	+0	+0	+0	+0

Library files

name	branch	text	data	bss	total
libopenthread-ftd.a	2fea118	251560	95	40319	291974
	15cc3c2	251616	95	40319	292030
	+/-	+56	+0	+0	+56
libopenthread-cli-ftd.a	2fea118	61777	0	8091	69868
	15cc3c2	61777	0	8091	69868
	+/-	+0	+0	+0	+0
libopenthread-ncp-ftd.a	2fea118	33359	0	5948	39307
	15cc3c2	33359	0	5948	39307
	+/-	+0	+0	+0	+0
libopenthread-mtd.a	2fea118	171009	0	24743	195752
	15cc3c2	171065	0	24743	195808
	+/-	+56	+0	+0	+56
libopenthread-cli-mtd.a	2fea118	41482	0	8059	49541
	15cc3c2	41482	0	8059	49541
	+/-	+0	+0	+0	+0
libopenthread-ncp-mtd.a	2fea118	25783	0	5948	31731
	15cc3c2	25783	0	5948	31731
	+/-	+0	+0	+0	+0
libopenthread-ftd-br.a	2fea118	363654	99	104423	468176
	15cc3c2	363710	99	104423	468232
	+/-	+56	+0	+0	+56
libopenthread-cli-ftd-br.a	2fea118	80892	0	8131	89023
	15cc3c2	80892	0	8131	89023
	+/-	+0	+0	+0	+0
libopenthread-rcp.a	2fea118	9946	0	5060	15006
	15cc3c2	9946	0	5060	15006
	+/-	+0	+0	+0	+0
libopenthread-radio.a	2fea118	20689	0	262	20951
	15cc3c2	20689	0	262	20951
	+/-	+0	+0	+0	+0

[codecov]

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 74.05%. Comparing base (c7a3a19) to head (c38d356).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
src/core/mac/mac.cpp	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #13417      +/-   ##
==========================================
+ Coverage   70.26%   74.05%   +3.78%     
==========================================
  Files         711      710       -1     
  Lines       99339    96814    -2525     
==========================================
+ Hits        69804    71694    +1890     
+ Misses      29535    25120    -4415     

Files with missing lines	Coverage Δ
src/core/mac/mac.cpp	83.16% <75.00%> (+1.04%)	⬆️

... and 168 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]

The change LGTM. Thanks @aussinfosec
One suggestion about the test.

[abtink]

Thanks for all the changes. LGTM. 👍

[jwhui]

Thanks! 👍🏼