Skip to content

Consolidate duplicated auth logic into shared module; add multi-environment test infrastructure - #8

Open
Jacob-Strokus wants to merge 3 commits into
depthsecurity:masterfrom
Jacob-Strokus:master
Open

Consolidate duplicated auth logic into shared module; add multi-environment test infrastructure#8
Jacob-Strokus wants to merge 3 commits into
depthsecurity:masterfrom
Jacob-Strokus:master

Conversation

@Jacob-Strokus

Copy link
Copy Markdown

Consolidate duplicated auth logic into shared core/auth module

Summary

LDAP and RPC authentication logic was copy-pasted across 5 files with subtle inconsistencies. This PR extracts it into a single shared module (core/auth), cleans up dead code left behind, and adds a full test suite with multi-environment CI.

Motivation

The same authentication patterns — Kerberos login, pass-the-hash, SASL password bind, null/anonymous bind, and ldap://ldaps:// channel binding fallback — were duplicated in target_parser.py, creds_checker.py, ntlmv1_detector.py, ghost_spn.py, and ntlm_reflection.py. Bug fixes (e.g., channel binding fallback) had to be applied in multiple places and were easy to miss, as seen with ntlmv1_detector.check_gpo() which only tried ldap:// and had no ldaps:// fallback.

Changes

New files

FileDescription
core/auth.pyShared auth module — 5 functions, 169 lines
tests/conftest.pyPytest fixtures with dependency stubs for running without impacket/ldap3
tests/test_auth.py51 tests across 11 test classes
tox.iniLocal multi-env testing (Python 3.8–3.12 × stubbed/full + lint)
.github/workflows/auth-tests.ymlGitHub Actions CI matrix

Shared API (core/auth)

FunctionPurpose
get_base_dn(domain)Domain → LDAP base DN conversion
is_kerberos_error(error)Detect Kerberos-specific failures (avoid retrying unrecoverable errors)
connect_ldap(config, dc_ip=None)Full LDAP connection lifecycle: Kerberos / PTH / password / null auth with auto ldaps:// fallback
configure_rpc_auth(config, transport, target)Set credentials + Kerberos options on DCE/RPC transport
connect_dce(transport, use_kerberos, uuid)Create, connect, and bind DCE/RPC handle

Refactored consumers

FileWhat changedLines removed
core/target_parser.py_enumerate_ad() auth block → connect_ldap()~80
core/creds_checker.pyAuth loop → connect_ldap() + is_kerberos_error()~50
detectors/ntlmv1_detector.pyLDAP auth in check_gpo() + RPC auth in _get_lm_compat_level()~60
detectors/ghost_spn.py_connect_ldap() body → delegation to shared module~45
detectors/ntlm_reflection.pyRPC auth in _get_ubr_from_registry() + _check_printspooler_enabled()~50

Dead code removed

  • from impacket.smbconnection import SMBConnection — imported but never used in target_parser.py
  • from impacket.dcerpc.v5 import transport — imported but never used in auth.py
  • ldap_scheme, ldap_port, ldap_url variables — computed but unreferenced after connect_ldap() handles connections internally
  • CredentialChecker._get_base_dn() — orphaned helper method + unused get_base_dn import

Bug fixes (via consolidation)

  • ntlmv1_detector.check_gpo() now correctly falls back from ldap:// to ldaps:// on channel binding enforcement — previously it only tried ldap:// and would fail silently on hardened DCs

Test coverage

51 tests covering all auth functions and integration scenarios:

  • TestGetBaseDn — simple, multi-part, single-label, empty, None
  • TestIsKerberosError — 5 Kerberos patterns, 5 non-Kerberos, case sensitivity
  • TestConnectLdapPassword — SASL login, protocol selection, explicit DC override
  • TestConnectLdapChannelBindingFallback — auto ldaps:// retry on 80090346
  • TestConnectLdapPassTheHash — empty password + nthash, fallback
  • TestConnectLdapKerberoskerberosLogin, uppercase domain, aesKey
  • TestConnectLdapNull — ldap3 anonymous bind, LDAPS variant
  • TestConnectLdapDcResolution — config fallback, DNS resolution, empty domain
  • TestConfigureRpcAuth — NTLM/Kerberos/PTH credentials, krb_dc_only logic
  • TestConnectDce — bind lifecycle, Kerberos auth type, error propagation
  • TestAuthIntegrationScenarios — end-to-end flows mimicking real consumer patterns

Tests use mock stubs so they run without impacket/ldap3 installed — safe for any CI environment.

CI matrix

JobPython versionsOSDeps
Stubbed3.8, 3.9, 3.10, 3.11, 3.12ubuntu, windowspytest only
Full3.10, 3.11, 3.12ubuntu, windowsall requirements.txt
Lint3.12ubuntupyflakes, pycodestyle

How to test locally

# Run tests directly
pytest tests/ -v
# Run with tox (all environments)
tox
# Single environment
tox -e py312-stubbed

Stats

13 files changed, 1,051 insertions(+), 370 deletions(-)

Extract LDAP and RPC authentication logic that was duplicated across 5
files into a single core/auth module with 5 shared functions:
- get_base_dn(): domain-to-DN conversion
- is_kerberos_error(): Kerberos error detection
- connect_ldap(): LDAP connection with auto ldaps fallback
- configure_rpc_auth(): RPC transport credential setup
- connect_dce(): DCE/RPC bind helper
Refactored consumers:
- core/target_parser.py: replaced ~80-line auth block
- core/creds_checker.py: replaced ~50-line auth loop
- detectors/ntlmv1_detector.py: LDAP and RPC auth paths
- detectors/ghost_spn.py: LDAP connection logic
- detectors/ntlm_reflection.py: RPC auth in 2 methods
Also removed dead code left behind: unused SMBConnection import,
unreferenced ldap_scheme/ldap_port/ldap_url variables, and orphaned
_get_base_dn() method in CredentialChecker.
Added 51 pytest tests covering all auth functions and integration
scenarios with full dependency mocking for CI environments.
Add tox.ini with stubbed (mock-only) and full (real deps) test matrices
across Python 3.8-3.12, plus a lint environment (pyflakes + pycodestyle).
Add GitHub Actions workflow (.github/workflows/auth-tests.yml) running:
- Stubbed tests: Python 3.8-3.12 x ubuntu/windows (10 combos)
- Full dependency tests: Python 3.10-3.12 x ubuntu/windows (6 combos)
- Static analysis: pyflakes + pycodestyle on core/auth.py
Also add pytest config to pyproject.toml and remove unused transport
import from core/auth.py caught by pyflakes.
@logansdiomedilogansdiomedi self-assigned this Apr 8, 2026
@logansdiomedilogansdiomedi added the enhancement New feature or request label Apr 8, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Jacob-Strokus@logansdiomedi