Skip to content

Fix NETLOGON_LOGON_QUERY parsing: Add mailslot name alignment#4952

Merged
gpotter2 merged 6 commits into
secdev:masterfrom
MatrixEditor:fix/netlogon
May 21, 2026
Merged

Fix NETLOGON_LOGON_QUERY parsing: Add mailslot name alignment#4952
gpotter2 merged 6 commits into
secdev:masterfrom
MatrixEditor:fix/netlogon

Conversation

@MatrixEditor
Copy link
Copy Markdown
Contributor

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.28%. Comparing base (9ae49f8) to head (3ae1cf5).
⚠️ Report is 32 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4952      +/-   ##
==========================================
- Coverage   80.30%   80.28%   -0.03%     
==========================================
  Files         379      383       +4     
  Lines       93164    94739    +1575     
==========================================
+ Hits        74820    76065    +1245     
- Misses      18344    18674     +330     
Files with missing lines Coverage Δ
scapy/layers/netbios.py 76.13% <ø> (ø)
scapy/layers/smb.py 77.14% <ø> (+0.35%) ⬆️

... and 46 files with indirect coverage changes

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

Comment thread scapy/layers/smb.py Outdated
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
Contributor 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
@gpotter2 gpotter2 self-assigned this Apr 20, 2026
Co-Authored-By: MatrixEditor <58256046+MatrixEditor@users.noreply.github.com>
@gpotter2 gpotter2 enabled auto-merge (squash) May 20, 2026 18:52
@gpotter2 gpotter2 disabled auto-merge May 21, 2026 21:33
@gpotter2 gpotter2 merged commit 65816b7 into secdev:master May 21, 2026
41 of 45 checks passed
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