From 05492af180180c985590e467b7f7e97ae2909b86 Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Sun, 14 Oct 2018 20:25:07 +0800 Subject: [PATCH] AuthenticationUtility: also sign serializable intent extras Also include intent extras into the signature to improve security. Note that after this commit, all `Utility.transferIntentToProfile` MUST be called AFTER all the extras have been added to the intent --- .../typeblog/shelter/ui/DummyActivity.java | 4 +- .../shelter/util/AuthenticationUtility.java | 64 ++++++++++++++++--- 2 files changed, 57 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/net/typeblog/shelter/ui/DummyActivity.java b/app/src/main/java/net/typeblog/shelter/ui/DummyActivity.java index cd0675c..ba79722 100644 --- a/app/src/main/java/net/typeblog/shelter/ui/DummyActivity.java +++ b/app/src/main/java/net/typeblog/shelter/ui/DummyActivity.java @@ -312,7 +312,6 @@ public class DummyActivity extends Activity { if (!mIsProfileOwner) { // Forward it to work profile Intent intent = new Intent(UNFREEZE_AND_LAUNCH); - Utility.transferIntentToProfile(this, intent); String packageName = getIntent().getStringExtra("packageName"); intent.putExtra("packageName", packageName); intent.putExtra("shouldFreeze", @@ -334,6 +333,7 @@ public class DummyActivity extends Activity { intent.putExtra("linkedPackages", packages); intent.putExtra("linkedPackagesShouldFreeze", packagesShouldFreeze); } + Utility.transferIntentToProfile(this, intent); startActivity(intent); finish(); return; @@ -388,10 +388,10 @@ public class DummyActivity extends Activity { // after loading the full list to freeze if (!mIsProfileOwner) { Intent intent = new Intent(FREEZE_ALL_IN_LIST); - Utility.transferIntentToProfile(this, intent); String[] list = LocalStorageManager.getInstance() .getStringList(LocalStorageManager.PREF_AUTO_FREEZE_LIST_WORK_PROFILE); intent.putExtra("list", list); + Utility.transferIntentToProfile(this, intent); startActivity(intent); finish(); } else { diff --git a/app/src/main/java/net/typeblog/shelter/util/AuthenticationUtility.java b/app/src/main/java/net/typeblog/shelter/util/AuthenticationUtility.java index 97917d2..42cd196 100644 --- a/app/src/main/java/net/typeblog/shelter/util/AuthenticationUtility.java +++ b/app/src/main/java/net/typeblog/shelter/util/AuthenticationUtility.java @@ -1,11 +1,16 @@ package net.typeblog.shelter.util; import android.content.Intent; +import android.os.Bundle; -import java.nio.ByteBuffer; +import java.lang.reflect.Array; import java.security.InvalidKeyException; import java.security.NoSuchAlgorithmException; +import java.util.ArrayList; +import java.util.Comparator; import java.util.Date; +import java.util.List; +import java.util.stream.Collectors; import javax.crypto.KeyGenerator; import javax.crypto.Mac; @@ -44,7 +49,7 @@ public class AuthenticationUtility { } else { long timestamp = new Date().getTime(); intent.putExtra("timestamp", timestamp); - intent.putExtra("signature", sign(key, timestamp)); + intent.putExtra("signature", sign(key, intentToString(intent))); } } @@ -66,8 +71,10 @@ public class AuthenticationUtility { } else { long timestamp = new Date().getTime(); long intentTimestamp = intent.getLongExtra("timestamp", 0); + String signature = intent.getStringExtra("signature"); + intent.removeExtra("signature"); // We don't include the signature itself while checking return timestamp - intentTimestamp < 30 * 1000 && - sign(key, intentTimestamp).equals(intent.getStringExtra("signature")); + sign(key, intentToString(intent)).equals(signature); } } @@ -75,12 +82,12 @@ public class AuthenticationUtility { LocalStorageManager.getInstance().remove(LocalStorageManager.PREF_AUTH_KEY); } - private static String sign(String hexKey, long timestamp) { + private static String sign(String hexKey, String serializedString) { try { SecretKeySpec keySpec = new SecretKeySpec(hexStringToByteArray(hexKey), "HmacSHA256"); Mac mac = Mac.getInstance("HmacSHA256"); mac.init(keySpec); - return bytesToHex(mac.doFinal(longToBytes(timestamp))); + return bytesToHex(mac.doFinal(serializedString.getBytes())); } catch (NoSuchAlgorithmException | InvalidKeyException e) { throw new RuntimeException("WTF?"); } @@ -116,9 +123,48 @@ public class AuthenticationUtility { } } - private static byte[] longToBytes(long x) { - ByteBuffer buffer = ByteBuffer.allocate(Long.BYTES); - buffer.putLong(x); - return buffer.array(); + // A deterministic way to convert an Intent into a String + private static String intentToString(Intent intent) { + // Read all the extras as "key,value" first + Bundle intentExtras = intent.getExtras(); + List extras = new ArrayList<>(); + for (String key : intentExtras.keySet()) { + Object obj = intentExtras.get(key); + + // Sign all primitive-typed and primitive-array-typed extras + if (isPrimitiveType(obj.getClass())) { + extras.add(key + "," + obj); + } else if (isPrimitiveArray(obj.getClass())) { + extras.add(key + "," + primitiveArrayToString(obj)); + } + } + + // Sort all the extras alphabetically + extras.sort(Comparator.naturalOrder()); + + // Collapse all extras into one string and append it after the action + return intent.getAction() + ";" + extras.stream().collect(Collectors.joining(";")); + } + + private static boolean isPrimitiveType(Class clazz) { + return clazz == Integer.class || clazz == Long.class || clazz == Float.class || clazz == Double.class + || clazz == Short.class || clazz == Byte.class || clazz == Character.class + || clazz == String.class || clazz == Boolean.class + || clazz == int.class || clazz == long.class || clazz == float.class + || clazz == double.class || clazz == short.class || clazz == byte.class + || clazz == char.class || clazz == boolean.class; + } + + private static boolean isPrimitiveArray(Class clazz) { + return clazz.isArray() && isPrimitiveType(clazz.getComponentType()); + } + + private static String primitiveArrayToString(Object array) { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < Array.getLength(array); i++) { + sb.append(Array.get(array, i)); + sb.append("|"); + } + return sb.toString(); } }