From b81ec0f07310f8d0401d744ebd6b3fce18dd2388 Mon Sep 17 00:00:00 2001 From: Alexander Bakker Date: Fri, 5 Jun 2020 12:49:12 +0200 Subject: [PATCH] Fix importing andOTP backups with more than 10000 PBKDF iterations These larger numbers of iterations cause the key derivation process to take longer, so I also moved that task to the background. andOTP now also has a proper "issuer" field, so we make use of that as well. Also fixes an issue where padded base32 could not be decoded. This issue is only present for the andOTP importer as far as I know, so that's why that change is included here. --- .../aegis/encoding/Base32.java | 6 +- .../aegis/importers/AndOtpImporter.java | 158 +++++++++++++----- 2 files changed, 118 insertions(+), 46 deletions(-) diff --git a/app/src/main/java/com/beemdevelopment/aegis/encoding/Base32.java b/app/src/main/java/com/beemdevelopment/aegis/encoding/Base32.java index 29c0f89a..b23b1d1e 100644 --- a/app/src/main/java/com/beemdevelopment/aegis/encoding/Base32.java +++ b/app/src/main/java/com/beemdevelopment/aegis/encoding/Base32.java @@ -3,21 +3,19 @@ package com.beemdevelopment.aegis.encoding; import com.google.common.io.BaseEncoding; public class Base32 { - private static final BaseEncoding _encoding = BaseEncoding.base32().omitPadding(); - private Base32() { } public static byte[] decode(String s) throws EncodingException { try { - return _encoding.decode(s.toUpperCase()); + return BaseEncoding.base32().decode(s.toUpperCase()); } catch (IllegalArgumentException e) { throw new EncodingException(e); } } public static String encode(byte[] data) { - return _encoding.encode(data); + return BaseEncoding.base32().omitPadding().encode(data); } } diff --git a/app/src/main/java/com/beemdevelopment/aegis/importers/AndOtpImporter.java b/app/src/main/java/com/beemdevelopment/aegis/importers/AndOtpImporter.java index 5df1ce51..01034cc7 100644 --- a/app/src/main/java/com/beemdevelopment/aegis/importers/AndOtpImporter.java +++ b/app/src/main/java/com/beemdevelopment/aegis/importers/AndOtpImporter.java @@ -8,7 +8,6 @@ import com.beemdevelopment.aegis.R; import com.beemdevelopment.aegis.crypto.CryptParameters; import com.beemdevelopment.aegis.crypto.CryptResult; import com.beemdevelopment.aegis.crypto.CryptoUtils; -import com.beemdevelopment.aegis.vault.VaultEntry; import com.beemdevelopment.aegis.encoding.Base32; import com.beemdevelopment.aegis.encoding.EncodingException; import com.beemdevelopment.aegis.otp.HotpInfo; @@ -17,6 +16,8 @@ import com.beemdevelopment.aegis.otp.OtpInfoException; import com.beemdevelopment.aegis.otp.SteamInfo; import com.beemdevelopment.aegis.otp.TotpInfo; import com.beemdevelopment.aegis.ui.Dialogs; +import com.beemdevelopment.aegis.ui.tasks.ProgressDialogTask; +import com.beemdevelopment.aegis.vault.VaultEntry; import org.json.JSONArray; import org.json.JSONException; @@ -49,8 +50,6 @@ public class AndOtpImporter extends DatabaseImporter { private static final int SALT_SIZE = 12; private static final int KEY_SIZE = 256; // bits - private static final int MAX_ITERATIONS = 10000; - public AndOtpImporter(Context context) { super(context); } @@ -96,38 +95,12 @@ public class AndOtpImporter extends DatabaseImporter { _data = data; } - private DecryptedState decrypt(char[] password, boolean oldFormat) throws DatabaseImporterException { + private DecryptedState decryptData(SecretKey key, int offset) throws DatabaseImporterException { + byte[] nonce = Arrays.copyOfRange(_data, offset, offset + NONCE_SIZE); + byte[] tag = Arrays.copyOfRange(_data, _data.length - TAG_SIZE, _data.length); + CryptParameters params = new CryptParameters(nonce, tag); + try { - SecretKey key; - int offset = 0; - - if (oldFormat) { - // WARNING: DON'T DO THIS IN YOUR OWN CODE - // this exists solely to support the old andOTP backup format - // it is not a secure way to derive a key from a password - MessageDigest hash = MessageDigest.getInstance("SHA-256"); - byte[] keyBytes = hash.digest(CryptoUtils.toBytes(password)); - key = new SecretKeySpec(keyBytes, "AES"); - } else { - offset = INT_SIZE + SALT_SIZE; - - byte[] iterBytes = Arrays.copyOfRange(_data, 0, INT_SIZE); - int iterations = ByteBuffer.wrap(iterBytes).getInt(); - if (iterations < 1 || iterations > MAX_ITERATIONS) { - throw new DatabaseImporterException(String.format("Invalid number of iterations for PBKDF: %d", iterations)); - } - - byte[] salt = Arrays.copyOfRange(_data, INT_SIZE, offset); - SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1"); - KeySpec spec = new PBEKeySpec(password, salt, iterations, KEY_SIZE); - key = factory.generateSecret(spec); - } - - // extract nonce and tag - byte[] nonce = Arrays.copyOfRange(_data, offset, offset + NONCE_SIZE); - byte[] tag = Arrays.copyOfRange(_data, _data.length - TAG_SIZE, _data.length); - CryptParameters params = new CryptParameters(nonce, tag); - Cipher cipher = CryptoUtils.createDecryptCipher(key, nonce); int len = _data.length - offset - NONCE_SIZE - TAG_SIZE; CryptResult result = CryptoUtils.decrypt(_data, offset + NONCE_SIZE, len, cipher, params); @@ -137,13 +110,50 @@ public class AndOtpImporter extends DatabaseImporter { } catch (NoSuchAlgorithmException | InvalidAlgorithmParameterException | InvalidKeyException - | InvalidKeySpecException | NoSuchPaddingException | IllegalBlockSizeException e) { throw new RuntimeException(e); } } + private void decrypt(Context context, char[] password, boolean oldFormat, DecryptListener listener) throws DatabaseImporterException { + if (oldFormat) { + // WARNING: DON'T DO THIS IN YOUR OWN CODE + // this exists solely to support the old andOTP backup format + // it is not a secure way to derive a key from a password + MessageDigest hash; + try { + hash = MessageDigest.getInstance("SHA-256"); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException(e); + } + byte[] keyBytes = hash.digest(CryptoUtils.toBytes(password)); + SecretKey key = new SecretKeySpec(keyBytes, "AES"); + DecryptedState state = decryptData(key, 0); + listener.onStateDecrypted(state); + } else { + int offset = INT_SIZE + SALT_SIZE; + + byte[] iterBytes = Arrays.copyOfRange(_data, 0, INT_SIZE); + int iterations = ByteBuffer.wrap(iterBytes).getInt(); + if (iterations < 1) { + throw new DatabaseImporterException(String.format("Invalid number of iterations for PBKDF: %d", iterations)); + } + + byte[] salt = Arrays.copyOfRange(_data, INT_SIZE, offset); + AndOtpKeyDerivationTask.Params params = new AndOtpKeyDerivationTask.Params(password, salt, iterations); + AndOtpKeyDerivationTask task = new AndOtpKeyDerivationTask(context, key1 -> { + try { + DecryptedState state = decryptData(key1, offset); + listener.onStateDecrypted(state); + } catch (DatabaseImporterException e) { + listener.onError(e); + } + }); + task.execute(params); + } + } + @Override public void decrypt(Context context, DecryptListener listener) { String[] choices = new String[]{ @@ -158,8 +168,7 @@ public class AndOtpImporter extends DatabaseImporter { int i = ((AlertDialog) dialog).getListView().getCheckedItemPosition(); Dialogs.showPasswordInputDialog(context, password -> { try { - DecryptedState state = decrypt(password, i != 0); - listener.onStateDecrypted(state); + decrypt(context, password, i != 0, listener); } catch (DatabaseImporterException e) { listener.onError(e); } @@ -221,12 +230,17 @@ public class AndOtpImporter extends DatabaseImporter { String name; String issuer = ""; - String[] parts = obj.getString("label").split(" - "); - if (parts.length > 1) { - issuer = parts[0]; - name = parts[1]; + if (obj.has("issuer")) { + name = obj.getString("label"); + issuer = obj.getString("issuer"); } else { - name = parts[0]; + String[] parts = obj.getString("label").split(" - "); + if (parts.length > 1) { + issuer = parts[0]; + name = parts[1]; + } else { + name = parts[0]; + } } return new VaultEntry(info, name, issuer); @@ -235,4 +249,64 @@ public class AndOtpImporter extends DatabaseImporter { } } } + + private static class AndOtpKeyDerivationTask extends ProgressDialogTask { + private Callback _cb; + + public AndOtpKeyDerivationTask(Context context, Callback cb) { + super(context, context.getString(R.string.unlocking_vault)); + _cb = cb; + } + + @Override + protected SecretKey doInBackground(AndOtpKeyDerivationTask.Params... args) { + setPriority(); + + AndOtpKeyDerivationTask.Params params = args[0]; + SecretKey key; + try { + SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1"); + KeySpec spec = new PBEKeySpec(params.getPassword(), params.getSalt(), params.getIterations(), KEY_SIZE); + key = factory.generateSecret(spec); + } catch (NoSuchAlgorithmException | InvalidKeySpecException e) { + throw new RuntimeException(e); + } + + return key; + } + + @Override + protected void onPostExecute(SecretKey key) { + super.onPostExecute(key); + _cb.onTaskFinished(key); + } + + public static class Params { + private char[] _password; + private byte[] _salt; + private int _iterations; + + public Params(char[] password, byte[] salt, int iterations) { + _iterations = iterations; + _password = password; + _salt = salt; + } + + public char[] getPassword() { + return _password; + } + + public int getIterations() { + return _iterations; + } + + public byte[] getSalt() { + return _salt; + } + } + + public interface Callback { + void onTaskFinished(SecretKey key); + } + } }