bluetooth: add missing HCI events, EIR elements, and SMP packet types#4987
bluetooth: add missing HCI events, EIR elements, and SMP packet types#4987XenoKovah wants to merge 1 commit intosecdev:masterfrom
Conversation
New packet definitions covering frequently-seen Bluetooth Core 5.4 events and Generic Access Profile data types that scapy did not yet decode: HCI events - HCI_Event_Connection_Request (code=0x04) - HCI_Event_Remote_Host_Supported_Features_Notification (code=0x3d) - HCI_Event_Vendor (vendor-specific debug) (code=0xff) - HCI_LE_Meta_LE_Read_Remote_Features_Complete (LE Meta event=0x04) EIR/AD elements - EIR_RandomTargetAddress (type=0x18) - EIR_LERole (type=0x1c) - EIR_BroadcastName (type=0x30) - EIR_3DInformation (type=0x3d) SMP - SM_Keypress_Notification (sm_command=0x0e) Each packet has an entry in test/scapy/layers/bluetooth.uts with a sample drawn from a real HCI snoop log (BR/EDR Connection Request, Remote Host Supported Features Notification, LE Read Remote Features Complete, vendor-specific debug event, EIR 3D Information, EIR Broadcast Name) and synthetic round-trip build/parse tests for the remainder.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4987 +/- ##
=======================================
Coverage 80.28% 80.29%
=======================================
Files 383 383
Lines 94703 94739 +36
=======================================
+ Hits 76031 76068 +37
+ Misses 18672 18671 -1
🚀 New features to boost your workflow:
|
| name = "HCI_Remote_Host_Supported_Features_Notification" | ||
| fields_desc = [ | ||
| LEMACField('bd_addr', None), | ||
| FlagsField('host_supported_features', 0, -64, _bluetooth_features) |
There was a problem hiding this comment.
Hey, @XenoKovah can you confirm that _bluetooth_features produces the adecuate flags for this field?
There was a problem hiding this comment.
Ah, looks like no they aren't correct. Despite its name I guess HCI_Remote_Host_Supported_Features_Notification only returns LMP extended features, not the baseline. But the tricky bit is they therefore could be interpreted as either page 1 or page 2 of LMP extended features (neither of which exist as definitions in scapy currently.) And even more annoying it seems like there's no way to know which interpretation is correct without keeping track statefully of what was asked for. I defer to you for the best way to say "this field could point at one of 2 possible structs"?
There was a problem hiding this comment.
I'm not aware of the protocol, so I'm just throwing in ideas.
If you need to change the field type, you could to it in the answers function if this packet is a response to a request packet which was sent immediately before this packet.
Otherwise you could use sniff sessions to track states across streams.
There was a problem hiding this comment.
Another option is to use a generic int based type and do not parse the flags... Ideally the options from @polybassa are best :)
I've been using the following definitions on my fork for a long time, because I didn't know how to make unit tests. So @antoniovazquezblanco just upstreamed some of my stuff when he had time. Well I still don't know how to make unit tests, but Claude does if I just point it at pcaps/HCI logs. So this is now a PR of stuff I've confirmed was working long ago, but now it meets the ask for unit tests. Note: the packet definitions themselves aren't AI generated, I made them. But the unit tests are AI generated based on real pcaps/HCI logs I collected in field.
HCI events
EIR/AD elements
SMP
Each packet has an entry in test/scapy/layers/bluetooth.uts with a sample drawn from a real HCI snoop log (BR/EDR Connection Request, Remote Host Supported Features Notification, LE Read Remote Features Complete, vendor-specific debug event, EIR 3D Information, EIR Broadcast Name) and synthetic round-trip build/parse tests for the remainder (which are rare and weren't present in any of the pcap/HCI logs.)
Checklist :
tox)I understand that failing to mention the use of AI may result in a ban. (We do not forbid it, but you must play fair. Be warned !)
Adds missing Bluetooth definitions from the spec.