New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SSH key stealing #138

Merged
merged 9 commits into from Jun 5, 2018

Conversation

Projects
None yet
2 participants
@VakarisZ
Contributor

VakarisZ commented May 24, 2018

Feature / Fixes

Gets all SSH keys available and sends them to island, where they are encrypted and stored in format:
{ 'private_key' : '', 'public_key': ''}

SSH key stealing tested locally on windows(does not crash) and ubuntu(root, user, unpriviledged user)

Changes

  • Changed db shema and default monkey config
  • Changed credential encryption methods to also encrypt ssh keys
@@ -251,6 +251,7 @@ def get_exploit_user_password_or_hash_product(self):
exploit_password_list = ["Password1!", "1234", "password", "12345678"]
exploit_lm_hash_list = []
exploit_ntlm_hash_list = []
exploit_ssh_keys = []

This comment has been minimized.

@danielguardicore

danielguardicore May 27, 2018

Contributor

Where is the equivalent change in example.conf

for user in creds:
for field in ['password', 'lm_hash', 'ntlm_hash']:
if field in creds[user]:
# this encoding is because we might run into passwords which are not pure ASCII
creds[user][field] = encryptor.enc(creds[user][field].encode('utf-8'))
if ssh_info:

This comment has been minimized.

@danielguardicore

danielguardicore May 27, 2018

Contributor

If the two pieces of code are foreign, don't reuse the same function name. Rename to say, encrypt_system_info_credentials and encrypt_system_info_sshkeys.

@@ -203,6 +219,15 @@ def add_system_info_creds_to_config(creds):
ConfigService.creds_add_lm_hash(creds[user]['lm_hash'])
if 'ntlm_hash' in creds[user]:
ConfigService.creds_add_ntlm_hash(creds[user]['ntlm_hash'])
if ssh_info:

This comment has been minimized.

@danielguardicore

danielguardicore May 27, 2018

Contributor

See note for encrypt_system_info_creds, split this up

@danielguardicore danielguardicore referenced this pull request May 27, 2018

Closed

Steal SSH keys #91

exploited = False
for user, ssh_key_pair in user_ssh_key_pairs:

This comment has been minimized.

@danielguardicore

danielguardicore May 29, 2018

Contributor

These pieces of code are now big enough I'd rather split into functions for brute force by user or password. Would also (depends on style) fix the bug later ahead

Generally, we can start splitting this up into smaller functions, such as a function to check machine type, etc.

self.report_login_attempt(False, user, ssh_key=ssh_string)
continue
user_password_pairs = self._config.get_exploit_user_password_pairs()

This comment has been minimized.

@danielguardicore

danielguardicore May 29, 2018

Contributor

Bug: Why do we try this if exploited = True?

ssh_info = telemetry_json['data']['ssh_info']
Telemetry.encrypt_system_info_ssh_keys(ssh_info)
if telemetry_json['data']['network_info']['networks']:
Telemetry.add_ip_to_ssh_keys(telemetry_json['data']['network_info']['networks'][0], ssh_info)

This comment has been minimized.

@danielguardicore

danielguardicore May 29, 2018

Contributor

Why only for the first network? Too complex to keep for all the networks?

@@ -888,6 +896,19 @@ def creds_add_lm_hash(lm_hash):
def creds_add_ntlm_hash(ntlm_hash):
ConfigService.add_item_to_config_set('internal.exploits.exploit_ntlm_hash_list', ntlm_hash)
@staticmethod
def ssh_add_keys(public_key, private_key, user, ip):
if not ConfigService.ssh_key_exists(ConfigService.get_config_value(['internal'], False, False)

This comment has been minimized.

@danielguardicore

danielguardicore May 29, 2018

Contributor

Why not pass in ['exploits']['exploit_ssh_keys'] to get_config_value like we do elsewhere? For example, get_config_exploits

@@ -414,7 +417,7 @@ class ReportPageComponent extends AuthComponent {
<ScannedServers data={this.state.report.glance.scanned}/>
</div>
<div>
<StolenPasswords data={this.state.report.glance.stolen_creds}/>
<StolenPasswords data={this.state.report.glance.stolen_creds, this.state.report.glance.ssh_keys}/>

This comment has been minimized.

@danielguardicore

danielguardicore May 29, 2018

Contributor

Pretty sure that's a bug and should be [this.state.report.glance.stolen_creds, this.state.report.glance.ssh_keys]

This comment has been minimized.

@VakarisZ

VakarisZ Jun 2, 2018

Contributor

It works and from my understanding it shouldn't be. It's probably best to ask someone who understands jsx

"exploit_ssh_keys": {
"title": "SSH key pairs list",
"type": "array",
"uniqueItems": True,

This comment has been minimized.

@danielguardicore

danielguardicore May 29, 2018

Contributor

Missing
"items": { "type": "string" },

@@ -504,6 +504,13 @@
},
"default": [],
"description": "List of NTLM hashes to use on exploits using credentials"
},
"exploit_ssh_keys": {

This comment has been minimized.

@danielguardicore

danielguardicore May 30, 2018

Contributor

Roping in @itaymmguardicore
The data is saved as a dictionary. This is fine/not fine internally?
When displayed on the UI, it's unusable but does it matter at this point?
image

This comment has been minimized.

@VakarisZ

VakarisZ Jun 2, 2018

Contributor

Preloading ssh keys is of no use. Should I figure a way to remove it from ui?

This comment has been minimized.

@danielguardicore

danielguardicore Jun 5, 2018

Contributor

No, if @itaymmguardicore has an idea it'll be nice to fix but really not important

@danielguardicore danielguardicore merged commit 5e7a218 into guardicore:develop Jun 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment