Skip to content

Fix NETLOGON_LOGON_QUERY parsing: Add mailslot name alignment#4952

Open
MatrixEditor wants to merge 5 commits intosecdev:masterfrom
MatrixEditor:fix/netlogon
Open

Fix NETLOGON_LOGON_QUERY parsing: Add mailslot name alignment#4952
MatrixEditor wants to merge 5 commits intosecdev:masterfrom
MatrixEditor:fix/netlogon

Conversation

@MatrixEditor
Copy link
Copy Markdown

As described in MS-ADTS "6.3.1.4 NETLOGON_LOGON_QUERY", the "MailslotName" field must be "aligned to an even byte boundary, with padding (bytes of value 0) to the next even byte boundary as necessary.".

This pull request fixes parsing NETLOGON_LOGON_QUERY messages by adding an optional padding byte.

  • Before:
###[ NETLOGON_LOGON_QUERY ]###
  OpCode    = LOGON_PRIMARY_QUERY
  ComputerName= b'WIN-SO93T7CNMNN'
  MailslotName= b'\\MAILSLOT\\NET\\GETDC465'
  UnicodeComputerName= 圀䤀一ⴀ匀伀㤀㌀吀㜀䌀一䴀一一
  NtVersion = bit_8+bit_9+bit_11
  LmNtToken = 0xff20
  Lm20Token = 0xffff
###[ Raw ]###
     load      = b'\xff'
  • After
###[ NETLOGON_LOGON_QUERY ]###
  OpCode    = LOGON_PRIMARY_QUERY
  ComputerName= b'WIN-SO93T7CNMNN'
  MailslotName= b'\\MAILSLOT\\NET\\GETDC943'
  MailslotPad= 0
  UnicodeComputerName= WIN-SO93T7CNMNN
  NtVersion = V1+V5+V5EX_WITH_IP+IP
  LmNtToken = 0xffff
  Lm20Token = 0xffff

@gpotter2
Copy link
Copy Markdown
Member

Hi and thanks for the PR !

Could you add the example packet you provided to the unit tests, at the end of this file:

= Dissect NETLOGON_SAM_LOGON_RESPONSE_NT40 - V1

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.30%. Comparing base (9ae49f8) to head (e305200).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4952      +/-   ##
==========================================
- Coverage   80.30%   80.30%   -0.01%     
==========================================
  Files         379      379              
  Lines       93164    93164              
==========================================
- Hits        74820    74816       -4     
- Misses      18344    18348       +4     
Files with missing lines Coverage Δ
scapy/layers/smb.py 77.14% <ø> (+0.35%) ⬆️

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-authored-by: Gabriel <10530980+gpotter2@users.noreply.github.com>
- revert change when parsing NETLOGON_LOGON_QUERY: always use length of MailslotName as a reference
@MatrixEditor
Copy link
Copy Markdown
Author

@gpotter2

I did some research and WIndows clients behave correctly (according to the spec). However, using your propsed PadField solution does not work properly - the padding is set even when on a correct byte boundary. I added two tests that include the padded variant and default PDC query. The current solution is to check whether the computer name's length is uneven, resulting in an uneven byte boundary.

If you have any suggestions on a cleaner approach, I am open for it.

@MatrixEditor MatrixEditor requested a review from gpotter2 April 6, 2026 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants