secdev / scapy

Scapy: the Python-based interactive packet manipulation program & library.
https://scapy.net
GNU General Public License v2.0
10.29k stars 1.99k forks source link

EDNS0 OWNER Option isn't parsed correctly (because draft-cheshire-edns0-owner-option-01 isn't accurate) #4429

Open evverx opened 2 weeks ago

evverx commented 2 weeks ago

Brief description

https://datatracker.ietf.org/doc/html/draft-cheshire-edns0-owner-option-01 refers to the Len field in https://datatracker.ietf.org/doc/html/draft-cheshire-edns0-owner-option-01#section-3.2 but it's off by 4 there because the Len field should be the size of the option data (without the "Opt" and "Len" fields: https://datatracker.ietf.org/doc/html/rfc6891#section-6.1.2)

Scapy version

6b26acebc177cc439c3df682ba3400414f1ae677

Python version

3.12.3

Operating system

6.8.11-300.fc40.x86_64

Additional environment information

No response

How to reproduce

DNSRROPT(b'\x00\x00)\x05\xa0\x00\x00\x11\x94\x00\x12\x00\x04\x00\x0e\x00F\x8f_\xef\xc6_|p\x9f\x03\x1e$s')

Actual result

###[ DNS OPT Resource Record ]###
  rrname    = b'.'
  type      = OPT
  rclass    = 1440
  extrcode  = 0
  version   = 0
  z         = 4500
  rdlen     = 18
  \rdata     \
   |###[ EDNS0 Owner (OWN) ]###
   |  optcode   = Owner
   |  optlen    = 14
   |  v         = 0
   |  s         = 70
   |  primary_mac= 8f:5f:ef:c6:5f:7c
   |###[ DNS EDNS0 TLV ]###
   |  optcode   = 28831
   |  optlen    = 798
   |  optdata   = b'$s'

Expected result

###[ DNS OPT Resource Record ]###
  rrname    = b'.'
  type      = OPT
  rclass    = 1440
  extrcode  = 0
  version   = 0
  z         = 4500
  rdlen     = 18
  \rdata     \
   |###[ EDNS0 Owner (OWN) ]###
   |  optcode   = Owner
   |  optlen    = 14
   |  v         = 0
   |  s         = 70
   |  primary_mac= 8f:5f:ef:c6:5f:7c
   |  wakeup_mac= 70:9f:03:1e:24:73

Related resources

Here's how mDNSResponder parses it: https://github.com/apple-oss-distributions/mDNSResponder/blob/d5029b5dff8aa59d1fc07ed796e994106ef58dee/mDNSCore/mDNSEmbeddedAPI.h#L914-L917

#define DNSOpt_OwnerData_ID_Space          (4 + 2 + 6)
#define DNSOpt_OwnerData_ID_Wake_Space     (4 + 2 + 6 + 6)
#define DNSOpt_OwnerData_ID_Wake_PW4_Space (4 + 2 + 6 + 6 + 4)
#define DNSOpt_OwnerData_ID_Wake_PW6_Space (4 + 2 + 6 + 6 + 6)
...
#define ValidOwnerLength(X) (   (X) == DNSOpt_OwnerData_ID_Space          - 4 || \
                                (X) == DNSOpt_OwnerData_ID_Wake_Space     - 4 || \
                                (X) == DNSOpt_OwnerData_ID_Wake_PW4_Space - 4 || \
                                (X) == DNSOpt_OwnerData_ID_Wake_PW6_Space - 4    )

https://github.com/apple-oss-distributions/mDNSResponder/blob/d5029b5dff8aa59d1fc07ed796e994106ef58dee/mDNSCore/DNSCommon.c#L3601-L3617

evverx commented 2 weeks ago

It can be fixed by replacing 18 with 14 and so on but there is another issue because it isn't possible to build the option with wakeup_mac without having to specify the length explicitly. I'm not sure how to fix it yet.