Skip to content
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

Added support for EC SSH keys encrypted with AES-256-CBC #5642

Open
wants to merge 2 commits into
base: bleeding-jumbo
Choose a base branch
from

Conversation

peshev
Copy link

@peshev peshev commented Dec 29, 2024

No description provided.

Copy link
Member

@magnumripper magnumripper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we enable some of the commented-out test vectors (nicked from hashcat in #5633) with these changes? If so, I suggest you do so as well. Thanks!

@solardiz
Copy link
Member

Thank you for the contribution @peshev!

@Nothing4You This PR reworks your addition from #5641 (why does it, @peshev?), you could want to test whether the "hash" produced by the script as revised here is still also usable with hashcat or not - and let us all know. Thank you!

@peshev
Copy link
Author

peshev commented Dec 30, 2024

@solardiz The fact that both I and @Nothing4You submitted similar PRs in a short timespan is probably not a coincidence. I assume both of us were doing the same thing: a CTF-style challenge, part of 38C3 - TELNET-KLARTEXT-REDEN.

His PR allows the ssh2john.py script to produce hash files that work in hashcat - unfortunately they do not work in john.
My version introduces a new cipher type (7), and its implementation in the john code - unfortunately, the hasfile produced by the script doesn't work in hashcat, since hashcat doesn't have cipher type 7.

I'll do some further work on this, in order to allow the ssh2john.py script to produce files that work in both john and hashcat, and fix #5634

@solardiz
Copy link
Member

I'll do some further work on this, in order to allow the ssh2john.py script to produce files that work in both john and hashcat, and fix #5634

Thank you! Please also add a doc/NEWS entry summarizing these improvements.

@Nothing4You
Copy link
Contributor

@solardiz The fact that both I and @Nothing4You submitted similar PRs in a short timespan is probably not a coincidence. I assume both of us were doing the same thing: a CTF-style challenge, part of 38C3 - TELNET-KLARTEXT-REDEN.

This was indeed what prompted me to PR this :)

I'm not sure though why this would need to introduce a new hash type rather than making $5$ work with John?
Especially as that is already working with Hashcat, that seems to be the more sensible change, unless there is some fundamental difference in implementations that would prevent this?

@magnumripper
Copy link
Member

I'm not sure though why this would need to introduce a new hash type rather than making $5$ work with John?
Especially as that is already working with Hashcat, that seems to be the more sensible change, unless there is some fundamental difference in implementations that would prevent this?

Right, we definitely want hashcat and john use the same input "hash" format if at all possible. For brand new formats to both, we normally seek consensus on the format beforehand nowadays.

@solardiz
Copy link
Member

Per the discussion here, it sounds like this PR isn't ready yet. Should we expect an update? Thanks!

@peshev
Copy link
Author

peshev commented Jan 22, 2025

That is correct, you should expect an update. I'll do the work and push to my branch as soon as I'm able to

@claudioandre-br
Copy link
Member

After reviewing the PR, I came up with the following proposal [1]:

0001-Added-support-for-EC-SSH-keys-encrypted-with-AES-256.PATCH

Then, I create a script to automate the test:

touch solucao.txt

for i in {1..32}; do
    echo "-- Iteration: $i"
    string=$(tr -dc A-Za-z0-9 </dev/urandom | head -c $i)
    echo $string >> solucao.txt
    echo $string > johnpass
    openssl ecparam -name secp521r1 -genkey | openssl ec -aes256 -out "$i".key -passout file:johnpass
done

# Extracting hashes
python3 ../run/ssh2john.py *.key > ../ssh-hashes.txt

# Testing john
rm -f ../run/john.pot; ../run/john --wordlist=solucao.txt ../ssh-hashes.txt

[1]

From 3030a1fb7c26958e69f3296899e8fbeeb22600cb Mon Sep 17 00:00:00 2001
From: Peter Peshev <[email protected]>
Date: Sun, 29 Dec 2024 23:35:16 +0200
Subject: [PATCH] Added support for EC SSH keys encrypted with AES-256-CBC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Claudio André <[email protected]>
---
 src/ssh_fmt_plug.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/ssh_fmt_plug.c b/src/ssh_fmt_plug.c
index 50013a72e..e3766581a 100644
--- a/src/ssh_fmt_plug.c
+++ b/src/ssh_fmt_plug.c
@@ -69,6 +69,7 @@ john_register_one(&fmt_ssh);
 #define SALT_ALIGN          sizeof(int)
 #define MIN_KEYS_PER_CRYPT  1
 #define MAX_KEYS_PER_CRYPT  8
+#define EC_KEY_WITH_AES256  (cur_salt->cipher == 5 && cur_salt->ctl == 224 && cur_salt->sl == 16)
 
 /*
  * For cost 1 using core i7, MKPC=8 and OMP_SCALE 128 works fine but that
@@ -380,6 +381,16 @@ static void common_crypt_code(char *password, unsigned char *out, int full_decry
 		AES_set_decrypt_key(key, 128, &akey);
 		// full decrypt
 		AES_cbc_encrypt(cur_salt->ct, out, cur_salt->ctl, &akey, iv, AES_DECRYPT);
+	} else if (EC_KEY_WITH_AES256) { // EC keys with AES-256
+		unsigned char key[32];
+		AES_KEY akey;
+		unsigned char iv[16];
+
+		memcpy(iv, cur_salt->salt, 16);
+		generate_key_bytes(32, (unsigned char*)password, key);
+		AES_set_decrypt_key(key, 256, &akey);
+		// full decrypt
+		AES_cbc_encrypt(cur_salt->ct, out, cur_salt->ctl, &akey, iv, AES_DECRYPT);
 	} else if (cur_salt->cipher == 4) { // RSA/DSA keys with AES-192
 		unsigned char key[24];
 		AES_KEY akey;
@@ -438,7 +449,7 @@ static int crypt_all(int *pcount, struct db_salt *salt)
 		} else if (cur_salt->cipher == 2 || cur_salt->cipher == 6) {  // new ssh key format handling
 			cracked[index] =
 				!check_structure_bcrypt(out, cur_salt->ctl);
-		} else if (cur_salt->cipher == 3) { // EC keys
+		} else if (cur_salt->cipher == 3 || EC_KEY_WITH_AES256) { // EC keys
 			cracked[index] =
 				!check_padding_and_structure_EC(out, cur_salt->ctl, 0);
 		} else if (cur_salt->cipher == 4) {  // AES-192
@@ -481,7 +492,7 @@ static int cmp_exact(char *source, int index)
 		return !check_padding_and_structure(out, cur_salt->ctl, 1, 16);
 	} else if (cur_salt->cipher == 2 || cur_salt->cipher == 6) {  /* new ssh key format handling */
 		return 1; // XXX add more checks!
-	} else if (cur_salt->cipher == 3) { // EC keys
+	} else if (cur_salt->cipher == 3 || EC_KEY_WITH_AES256) { // EC keys
 		return 1;
 	} else if (cur_salt->cipher == 4) {
 		return !check_padding_and_structure(out, cur_salt->ctl, 1, 16);
-- 
2.43.0

Note:

  • I've kept the original author;
  • It succeeds in a random sequence of tests.

@peshev
Copy link
Author

peshev commented Jan 22, 2025

This is pretty much what I was planning to do - go back to cipher == 5, and figure out some other way of checking whether the key is EC or not.
Can you give me a bit of detail on what the ctl and sl fields of the custom_salt cur_salt structure represent, and what the values you're comparing them to actually mean? Is 224 the key length? What about EC keys of different length?

Edit: I'm guessing that ctl is CipherText Length and sl is Salt Length - if that's the case, I don't think there's any point in including sl in the EC key check, since that would be 16 for all AES-256 encrypted keys, EC or not. Is the fact that the ciphertext length is 224 (bytes) and the EC key length is 224 (bits) a coincidence?

@claudioandre-br
Copy link
Member

claudioandre-br commented Jan 22, 2025

Can you give me a bit of detail on ...

I'm afraid I'm not the right person to do this. I did a quick reverse engineering on the subject and that's it.
@kholia is probably the best person here to comment.


Anyway, I got the impression that the way the format was designed originaly is fragile and problems will occur in the future because of this. The hashcat has the -m codes, we don't.

This is one solution to this problem; others issues are bound to arise in the future.

@peshev
Copy link
Author

peshev commented Jan 22, 2025

I did a quick reverse engineering on the subject

Can you point me to where you did that? Specifically where you got those values from?

@claudioandre-br
Copy link
Member

sl is Salt Length - if that's the case, I don't think there's any point in including sl in the EC key check, since that would be 16 for all AES-256 encrypted keys

It is indeed len(saltstr).

@Nothing4You
Copy link
Contributor

The hashcat has the -m codes, we don't.

hashcat uses the same mode regardless of the key type, only the encryption is different:

22911 | RSA/DSA/EC/OpenSSH Private Keys ($0$)                      | Private Key
22921 | RSA/DSA/EC/OpenSSH Private Keys ($6$)                      | Private Key
22931 | RSA/DSA/EC/OpenSSH Private Keys ($1, $3$)                  | Private Key
22941 | RSA/DSA/EC/OpenSSH Private Keys ($4$)                      | Private Key
22951 | RSA/DSA/EC/OpenSSH Private Keys ($5$)                      | Private Key

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants