Add support for non-ASCII passwords

[kholia]

I have tested this with Android 4.4.4, Android 6.0, and Android 8.0 ADB backups.

This patch is somewhat hacky but hopefully the general idea is reusable.

[kholia]

I got this idea, of only checking the PCKS5 padding to verify the password, while working on https://github.com/magnumripper/JohnTheRipper/pull/3261 (OpenCL support for cracking Android Backups) and related issues.

[nelenkov]

Hacky indeed :) The proper way to handle non-ascii password is to fix the key derivation, why do you think that checking the padding is enough? Also, why if (pad_byte < 8)? Technically, you could have 1,2, etc. bytes of padding.

[kholia]

I have answered this question at https://github.com/magnumripper/JohnTheRipper/pull/3261#discussion_r191063447.

In short,

// The structure of masteykey_blob is fixed:
//   + masteykey_blob -> length_1 (byte) + IV (16 bytes) + length_2 + masteykey (32 bytes) + length_3 + checksum (32 bytes) => total of 83 bytes
//   + padding -> this data blob of 83 bytes gets 13 bytes of padding making the masteykey_blob_length = 96 bytes always

This implies that padding is always 13 bytes in practice.

Good question!

[nelenkov]

Hi,

I get the fixed padding part. However, this format has checksum that allows you to verify if derived key is correct. Why not just use that (aside from the slight performance optimization)? If buildPasswordKey() is called with utf8=true, non-ASCII passwords should work fine.

[kholia]

Yep, the format does have a checksum. However, the process of calculating a checksum is problematic.

The program does calculatedCk = makeKeyChecksum(mk, ckSalt, rounds, useUtf) followed by calculatedCk = makeKeyChecksum(mk, ckSalt, rounds, !useUtf). This is hacky and doesn't look robust. Wouldn't it be nice if we can avoid this hack entirely? :)

Also, I could not find a way to port PBEParametersGenerator.PKCS5PasswordToBytes (when useUtf is True) to C easily (due to wide-chars vs unsigned chars conversion).

[nelenkov]

Hm, checking the checksum is still better than only checking padding. Is the current code broken for non-ASCII passwords? AFAIK, that code was added to handle both v1 and v2 backups (with UTF-8 support)

As for porting to C, you'd need to use iconv or similar to get proper unicode support.

[kholia]

Is the current code broken for non-ASCII passwords?

Yes! This is the reason behind this PR.

Hm, checking the checksum is still better than only checking padding.

Checking proper padding of length 8 is quite safe. To make it even safer, I can check for proper padding of length 13. In JtR project, we rely on these padding checks for cracking passwords. We are able to do this without triggering false positives.

By removing the hacky checksum checking code, I think we will be able to decrypt a wider variety of encrypted backup files. I am not 100% sure about this last statement though.