1695 Mimikatz credential parsing #1730
Conversation
* Leftover from broken info gathering package
No system info telemetries need to be processed anymore
| from enum import Enum | ||
|
|
||
|
|
||
| class CredentialsType(Enum): |
There was a problem hiding this comment.
I'm not sure about the change from singular to plural here. In my understanding, credentials (plural) are made up of components, i.e. identities and secrets. A single identity or a single secret would be singular, not plural. Example: "What type of credential is this? It's a password."
We could consider changing this to CredentialComponentType to avoid ambiguity.
| @@ -17,6 +17,9 @@ def __init__(self, credentials: Iterable[Credentials]): | |||
| """ | |||
| self._credentials = credentials | |||
|
|
|||
| def send(self, log_data=True): | |||
There was a problem hiding this comment.
| def send(self, log_data=True): | |
| def send(self, _=None): |
There was a problem hiding this comment.
Complains, because the method signature contains a parameter named log_data, not named _
There was a problem hiding this comment.
Either way, we're implementing an interface method, we shouldn't change the interface even if we could
| if is_ssh_keypair(credential): | ||
| SECRET_PROCESSORS[CredentialsType.SSH_KEYPAIR.value](credential, credentials["monkey_guid"]) | ||
| else: | ||
| for identity in credential["identities"]: | ||
| IDENTITY_PROCESSORS[identity["credential_type"]](identity) | ||
| for secret in credential["secrets"]: | ||
| SECRET_PROCESSORS[secret["credential_type"]](secret) |
There was a problem hiding this comment.
Credentials can contain multiple secrets. This logic assumes that if the Credentials contain an ssh keypair, they do not contain any other type of secrets, which would lead to those secrets never being processed.
| ) | ||
|
|
||
|
|
||
| def encrypt_system_info_ssh_keys(ssh_key: dict): |
There was a problem hiding this comment.
I'd recommend making this a pure function and returning a tuple instead of modifying the dict. It could help avoid bugs down the road.
There was a problem hiding this comment.
Yes, I'm still working on it, this code is not even tested. I'm removing it for this PR and will implement in another
| } | ||
|
|
||
|
|
||
| def parse_credentials(credentials: dict): |
There was a problem hiding this comment.
A general comment for all new code: Prefer Mapping to dict or Dict.
| parse_credentials, | ||
| ) | ||
|
|
||
| MIMIKATZ_TELEM_TEMPLATE = { |
There was a problem hiding this comment.
Can we remove the references to mimikatz? Does anything related to this processing care whether or not credentials were collected by mimikatz or some other mechanism?
What does this PR do?
Fixes part of #1695
Add any further explanations here.
PR Checklist
Testing Checklist
Explain Changes
Are the commit messages enough? If not, elaborate.