ENG-8835 fix(network-ops): redact credentials from tool logs and traces

capabilities/network-ops/tests/test_tool_fixes.py

@@ -124,6 +124,7 @@ def _load_module(name: str, filename: str):
 krbrelayx_mod = _load_module("krbrelayx", "krbrelayx.py")
 netexec_mod = _load_module("netexec", "netexec.py")
 coercion_mod = _load_module("coercion", "coercion.py")
+smbclient_mod = _load_module("smbclient", "smbclient.py")
 # The tool modules above import ``._relay`` relatively, so it is already loaded
 # under the package; reuse that instance rather than loading a second copy.
 relay_mod = sys.modules.get(f"{_PKG}._relay") or _load_module("_relay", "_relay.py")
@@ -1709,9 +1710,7 @@ def test_no_legacy_auth_param_names_in_any_method(self):
                     params = inspect.signature(attr).parameters
                 except (TypeError, ValueError):
                     continue
-                offenders.extend(
-                    f"{cls_name}.{name}({banned})" for banned in self._BANNED if banned in params
-                )
+                offenders.extend(f"{cls_name}.{name}({banned})" for banned in self._BANNED if banned in params)
         assert not offenders, f"legacy auth param names remain: {offenders}"
 
     def test_netexec_kerberos_is_bool_with_aes_key_and_ccache(self):
@@ -1724,3 +1723,91 @@ def test_certipy_uses_dc_ip_not_target(self):
         params = inspect.signature(certipy_mod.Certipy.certipy).parameters
         assert "dc_ip" in params
         assert "target" not in params
+
+
+# ===================================================================
+# Credential redaction: secrets= threaded into execute()
+# ===================================================================
+
+
+def _secrets_of(mock_exec):
+    """The secrets= list passed to a patched execute()."""
+    return mock_exec.call_args.kwargs.get("secrets")
+
+
+class TestCredentialRedaction:
+    """Spot-checks that each threading style carries the credential into
+    execute(secrets=...): named auth params, a list-valued param, an opaque
+    arg list (flag recovery), and embedded operation secrets."""
+
+    @pytest.mark.asyncio
+    async def test_bloodyad_password_and_nt_hash_threaded(self):
+        bad = bloodyad_mod.BloodyAD()
+        with patch.object(bloodyad_mod, "execute", new_callable=AsyncMock) as mock_exec:
+            await bad.bloodyad_get_object(
+                "CN=alice", domain="corp.local", username="alice", dc_ip="dc01", password="hunter2"
+            )
+            assert "hunter2" in _secrets_of(mock_exec)
+        with patch.object(bloodyad_mod, "execute", new_callable=AsyncMock) as mock_exec:
+            await bad.bloodyad_get_object(
+                "CN=alice", domain="corp.local", username="alice", dc_ip="dc01", nt_hash="aad3b:5f4"
+            )
+            assert "aad3b:5f4" in _secrets_of(mock_exec)
+
+    @pytest.mark.asyncio
+    async def test_netexec_named_and_args_embedded_secrets(self):
+        nxc = netexec_mod.Netexec()
+        with patch.object(netexec_mod, "execute", new_callable=AsyncMock) as mock_exec:
+            await nxc.netexec(
+                protocol="smb",
+                targets=["10.0.0.1"],
+                args=["-M", "spider", "-p", "argpw"],
+                username="alice",
+                password="hunter2",
+            )
+            secrets = _secrets_of(mock_exec)
+            assert "hunter2" in secrets
+            assert "argpw" in secrets
+
+    @pytest.mark.asyncio
+    async def test_certipy_named_and_opaque_args_secrets(self):
+        c = certipy_mod.Certipy()
+        c.certipy_cmd = "certipy"
+        c.timeout = 10
+        with patch.object(certipy_mod, "execute", new_callable=AsyncMock) as mock_exec:
+            await c.certipy(action="find", args=[], dc_ip="10.0.0.1", username="alice", password="hunter2")
+            assert "hunter2" in _secrets_of(mock_exec)
+        with patch.object(certipy_mod, "execute", new_callable=AsyncMock) as mock_exec:
+            await c.certipy_auth(args=["-aes", "deadbeefkey", "-pfx-password", "pfxpw"])
+            secrets = _secrets_of(mock_exec)
+            assert "deadbeefkey" in secrets
+            assert "pfxpw" in secrets
+
+    @pytest.mark.asyncio
+    async def test_impacket_secretsdump_threads_password_and_aes_key(self):
+        imp = impacket_mod.Impacket()
+        imp.timeout = 120
+        with (
+            patch.object(impacket_mod, "execute", new_callable=AsyncMock) as mock_exec,
+            patch.object(imp, "_build_script_command", return_value=["secretsdump.py"]),
+        ):
+            await imp.impacket_secretsdump(target="10.0.0.1", username="alice", password="hunter2", aes_key="deadbeef")
+            secrets = _secrets_of(mock_exec)
+            assert "hunter2" in secrets
+            assert "deadbeef" in secrets
+
+    @pytest.mark.asyncio
+    async def test_smbclient_password_threaded(self):
+        smb = smbclient_mod.SmbClient()
+        with (
+            patch.object(smbclient_mod, "execute", new_callable=AsyncMock) as mock_exec,
+            patch.object(smbclient_mod, "_require_smbclient", return_value="smbclient"),
+        ):
+            await smb.smb_download_file(
+                target="10.0.0.1",
+                share_name="C$",
+                remote_path="f.txt",
+                username="alice",
+                password="hunter2",
+            )
+            assert "hunter2" in _secrets_of(mock_exec)

capabilities/network-ops/tools/_relay.py

@@ -36,7 +36,20 @@
 ]
 
 # argv flags whose following value is a credential and must not be logged.
-_SECRET_FLAGS = {"-p", "--password", "-hashes", "--hashes", "-aeskey", "--aeskey", "-aesKey"}
+# ``--add-computer`` carries a "NAME PASSWORD" value (ntlmrelayx); the others
+# cover the identity/operation secrets relay tools accept.
+_SECRET_FLAGS = {
+    "-p",
+    "--password",
+    "-hashes",
+    "--hashes",
+    "-aeskey",
+    "--aeskey",
+    "-aesKey",
+    "--add-computer",
+    "-pfx-password",
+    "--computer-pass",
+}
 
 # How long to wait after SIGTERM before escalating to SIGKILL.
 _SIGKILL_TIMEOUT = 5.0

capabilities/network-ops/tools/bloodyad.py

@@ -157,6 +157,7 @@ async def bloodyad_get_object(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -224,6 +225,7 @@ async def bloodyad_get_children(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -285,6 +287,7 @@ async def bloodyad_get_membership(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -341,6 +344,7 @@ async def bloodyad_get_writable(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -395,6 +399,7 @@ async def bloodyad_get_dnsdump(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -449,6 +454,7 @@ async def bloodyad_get_trusts(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -516,6 +522,7 @@ async def bloodyad_get_search(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -574,6 +581,7 @@ async def bloodyad_add_groupmember(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -635,6 +643,7 @@ async def bloodyad_add_computer(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash, computer_password) if s],
         )
 
     @tool_method(catch=True)
@@ -696,6 +705,7 @@ async def bloodyad_add_user(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash, user_password) if s],
         )
 
     @tool_method(catch=True)
@@ -752,6 +762,7 @@ async def bloodyad_add_dcsync(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -810,6 +821,7 @@ async def bloodyad_add_rbcd(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -871,6 +883,7 @@ async def bloodyad_add_shadowcredentials(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash, pfx_password) if s],
         )
 
     @tool_method(catch=True)
@@ -929,6 +942,7 @@ async def bloodyad_add_genericall(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -987,6 +1001,7 @@ async def bloodyad_add_dnsrecord(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -1045,6 +1060,7 @@ async def bloodyad_set_password(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash, new_password) if s],
         )
 
     @tool_method(catch=True)
@@ -1106,6 +1122,7 @@ async def bloodyad_set_object(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -1164,6 +1181,7 @@ async def bloodyad_remove_groupmember(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -1220,6 +1238,7 @@ async def bloodyad_remove_object(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -1276,6 +1295,7 @@ async def bloodyad_remove_dcsync(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -1334,6 +1354,7 @@ async def bloodyad_remove_rbcd(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -1390,6 +1411,7 @@ async def bloodyad_remove_shadowcredentials(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -1448,6 +1470,7 @@ async def bloodyad_remove_dnsrecord(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )
 
     @tool_method(catch=True)
@@ -1506,4 +1529,5 @@ async def bloodyad_remove_genericall(
             timeout=self.timeout,
             input=input,
             env=env,
+            secrets=[s for s in (password, nt_hash) if s],
         )