openwall / john

John the Ripper jumbo - advanced offline password cracker, which supports hundreds of hash and cipher types, and runs on many operating systems, CPUs, GPUs, and even some FPGAs
https://www.openwall.com/john/
Other
10.32k stars 2.1k forks source link

PEM and SSH format enhancements #5145

Open solardiz opened 2 years ago

solardiz commented 2 years ago

pem_common_plug.c: pem_decrypt() decrypts the entire ciphertext, then uses a padding check for early-reject. It could instead start by decrypting only the last block (using the previous block as IV), do the padding check, and only if padding looks good (yet insufficiently large - see below) proceed to decrypt the remainder of the ciphertext (all but the already-decrypted last block) and do checks on it. The speedup will be very small, though, because most time is spent on PBKDF2, so even decrypting ~1 KB of data is relatively fast.

As a separate related enhancement, if the padding check is good and the padding is large enough, we can skip further checks. This wouldn't provide much speedup (for large paddings, those extra checks were rarely reached anyway), however it would reduce the likelihood of false negatives (in case our extra checks are too strict). For example, we can do:

         if (check_pkcs_pad(out, length, block_size) < 0)
                 return -1;

+        if (out[length - 1] >= 7)
+                return 0;

(or 6 if we aren't too afraid of false positives)

The above comments also apply to the version of pem_decrypt in run/opencl/pem_kernel.c.

While at it, in pem_common_plug.c: pem_decrypt() we could also drop the initial memset (looks unnecessary?) and replace the many uses of cur_salt->ciphertext_length with length (already initialized to the same value at the beginning of the function).

We should also move pem_common_plug.c: pem_decrypt() to pem_fmt_plug.c since that function is no longer shared with the OpenCL format.

Summarizing the above, it could be best to start with implementing the simpler changes - which means all but the first paragraph above.

solardiz commented 2 years ago

Some of the above also applies to the SSH formats (CPU and OpenCL), which have similar padding and ASN.1 checks.

solardiz commented 2 years ago

in pem_common_plug.c: pem_decrypt() we could also drop the initial memset (looks unnecessary?)

Confirmed by changing 0 to -1 - it still passes self-tests.

solardiz commented 2 years ago

This is not pretty, seen while running --test-full=1:

Testing: PEM, PKCS#8 private key (RSA/DSA/ECDSA) [PBKDF2-SHA1 32/64 3DES/AES]... (3xOMP) Warning: PEM kdf algorithm <> is not supported currently!
PASS
claudioandre-br commented 2 years ago

This is not pretty, seen while running --test-full=1:

Testing: PEM, PKCS#8 private key (RSA/DSA/ECDSA) [PBKDF2-SHA1 32/64 3DES/AES]... (3xOMP) Warning: PEM kdf algorithm <> is not supported currently!
PASS

This patch fix it:

diff --git a/src/pem_common_plug.c b/src/pem_common_plug.c
index 18c61e94b..dcde9c40e 100644
--- a/src/pem_common_plug.c
+++ b/src/pem_common_plug.c
@@ -50,7 +50,7 @@ int pem_valid(char *ciphertext, struct fmt_main *self)
        keeptr = ctcopy;

        ctcopy += TAG_LENGTH;
-       if ((p = strtokm(ctcopy, "$")) == NULL) // type
+       if ((p = strtokm(ctcopy, "$")) == NULL || *p == 0) // type
                goto err;
        if (strcmp(p, "1") != 0 && !warned) {
                if ((p = strtokm(NULL, "$")) == NULL)

But it's not clear to me if valid should be (properly) re-written.