diff options
5 files changed, 284 insertions, 1 deletions
diff --git a/core/java/android/content/Intent.java b/core/java/android/content/Intent.java index 031380dc1962..044178c4f6aa 100644 --- a/core/java/android/content/Intent.java +++ b/core/java/android/content/Intent.java @@ -20,6 +20,7 @@ import static android.app.sdksandbox.SdkSandboxManager.ACTION_START_SANDBOXED_AC import static android.content.ContentProvider.maybeAddUserId; import static android.os.Flags.FLAG_ALLOW_PRIVATE_PROFILE; import static android.security.Flags.FLAG_FRP_ENFORCEMENT; +import static android.security.Flags.preventIntentRedirect; import android.Manifest; import android.accessibilityservice.AccessibilityService; @@ -7687,9 +7688,17 @@ public class Intent implements Parcelable, Cloneable { /** @hide */ public static final int LOCAL_FLAG_FROM_SYSTEM = 1 << 5; + /** + * This flag indicates the creator token of this intent has been verified. + * + * @hide + */ + public static final int LOCAL_FLAG_CREATOR_TOKEN_VERIFIED = 1 << 6; + /** @hide */ @IntDef(flag = true, prefix = { "EXTENDED_FLAG_" }, value = { EXTENDED_FLAG_FILTER_MISMATCH, + EXTENDED_FLAG_MISSING_CREATOR_OR_INVALID_TOKEN, }) @Retention(RetentionPolicy.SOURCE) public @interface ExtendedFlags {} @@ -7703,6 +7712,13 @@ public class Intent implements Parcelable, Cloneable { @TestApi public static final int EXTENDED_FLAG_FILTER_MISMATCH = 1 << 0; + /** + * This flag indicates the creator token of this intent is either missing or invalid. + * + * @hide + */ + public static final int EXTENDED_FLAG_MISSING_CREATOR_OR_INVALID_TOKEN = 1 << 1; + // --------------------------------------------------------------------- // --------------------------------------------------------------------- // toUri() and parseUri() options. @@ -7870,6 +7886,7 @@ public class Intent implements Parcelable, Cloneable { this.mPackage = o.mPackage; this.mComponent = o.mComponent; this.mOriginalIntent = o.mOriginalIntent; + this.mCreatorTokenInfo = o.mCreatorTokenInfo; if (o.mCategories != null) { this.mCategories = new ArraySet<>(o.mCategories); @@ -12176,6 +12193,60 @@ public class Intent implements Parcelable, Cloneable { return (mExtras != null) ? mExtras.describeContents() : 0; } + private static class CreatorTokenInfo { + // Stores a creator token for an intent embedded as an extra intent in a top level intent, + private IBinder mCreatorToken; + // Stores all extra keys whose values are intents for a top level intent. + private ArraySet<String> mExtraIntentKeys; + } + + private @Nullable CreatorTokenInfo mCreatorTokenInfo; + + /** @hide */ + public void removeCreatorTokenInfo() { + mCreatorTokenInfo = null; + } + + /** @hide */ + public @Nullable IBinder getCreatorToken() { + return mCreatorTokenInfo == null ? null : mCreatorTokenInfo.mCreatorToken; + } + + /** @hide */ + public Set<String> getExtraIntentKeys() { + return mCreatorTokenInfo == null ? null : mCreatorTokenInfo.mExtraIntentKeys; + } + + /** @hide */ + public void setCreatorToken(@NonNull IBinder creatorToken) { + if (mCreatorTokenInfo == null) { + mCreatorTokenInfo = new CreatorTokenInfo(); + } + mCreatorTokenInfo.mCreatorToken = creatorToken; + } + + /** + * Collects keys in the extra bundle whose value are intents. + * @hide + */ + public void collectExtraIntentKeys() { + if (!preventIntentRedirect()) return; + + if (mExtras != null && !mExtras.isParcelled() && !mExtras.isEmpty()) { + for (String key : mExtras.keySet()) { + if (mExtras.get(key) instanceof Intent) { + if (mCreatorTokenInfo == null) { + mCreatorTokenInfo = new CreatorTokenInfo(); + } + if (mCreatorTokenInfo.mExtraIntentKeys == null) { + mCreatorTokenInfo.mExtraIntentKeys = new ArraySet<>(); + } + mCreatorTokenInfo.mExtraIntentKeys.add(key); + } + } + } + } + public void writeToParcel(Parcel out, int flags) { out.writeString8(mAction); Uri.writeToParcel(out, mData); @@ -12225,6 +12296,16 @@ public class Intent implements Parcelable, Cloneable { } else { out.writeInt(0); } + + if (preventIntentRedirect()) { + if (mCreatorTokenInfo == null) { + out.writeInt(0); + } else { + out.writeInt(1); + out.writeStrongBinder(mCreatorTokenInfo.mCreatorToken); + out.writeArraySet(mCreatorTokenInfo.mExtraIntentKeys); + } + } } public static final @android.annotation.NonNull Parcelable.Creator<Intent> CREATOR @@ -12282,6 +12363,14 @@ public class Intent implements Parcelable, Cloneable { if (in.readInt() != 0) { mOriginalIntent = new Intent(in); } + + if (preventIntentRedirect()) { + if (in.readInt() != 0) { + mCreatorTokenInfo = new CreatorTokenInfo(); + mCreatorTokenInfo.mCreatorToken = in.readStrongBinder(); + mCreatorTokenInfo.mExtraIntentKeys = (ArraySet<String>) in.readArraySet(null); + } + } } /** diff --git a/core/java/android/security/responsible_apis_flags.aconfig b/core/java/android/security/responsible_apis_flags.aconfig index 56d3669ac50c..45e9def2c15f 100644 --- a/core/java/android/security/responsible_apis_flags.aconfig +++ b/core/java/android/security/responsible_apis_flags.aconfig @@ -52,3 +52,11 @@ flag { description: "Opt the system into enforcement of BAL" bug: "339403750" } + +flag { + name: "prevent_intent_redirect" + namespace: "responsible_apis" + description: "Prevent intent redirect attacks" + bug: "361143368" + is_fixed_read_only: true +}
\ No newline at end of file diff --git a/core/tests/coretests/Android.bp b/core/tests/coretests/Android.bp index d98836f8ce20..e9ce71230a82 100644 --- a/core/tests/coretests/Android.bp +++ b/core/tests/coretests/Android.bp @@ -104,6 +104,7 @@ android_test { "mockito-target-extended-minus-junit4", "TestParameterInjector", "android.content.res.flags-aconfig-java", + "android.security.flags-aconfig-java", ], libs: [ diff --git a/core/tests/coretests/src/android/content/IntentTest.java b/core/tests/coretests/src/android/content/IntentTest.java new file mode 100644 index 000000000000..d169ce3c07d0 --- /dev/null +++ b/core/tests/coretests/src/android/content/IntentTest.java @@ -0,0 +1,99 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.content; + +import static com.google.common.truth.Truth.assertThat; + +import static org.junit.Assert.assertEquals; + +import android.net.Uri; +import android.os.Binder; +import android.os.IBinder; +import android.os.Parcel; +import android.platform.test.annotations.Presubmit; +import android.platform.test.annotations.RequiresFlagsEnabled; +import android.platform.test.flag.junit.CheckFlagsRule; +import android.platform.test.flag.junit.DeviceFlagsValueProvider; +import android.security.Flags; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import androidx.test.filters.SmallTest; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; + +/** + * Build/Install/Run: + * atest FrameworksCoreTests:IntentTest + */ +@Presubmit +@SmallTest +@RunWith(AndroidJUnit4.class) +public class IntentTest { + private static final String TEST_ACTION = "android.content.IntentTest_test"; + private static final String TEST_EXTRA_NAME = "testExtraName"; + private static final Uri TEST_URI = Uri.parse("content://com.example/people"); + + @Rule + public final CheckFlagsRule mCheckFlagsRule = DeviceFlagsValueProvider.createCheckFlagsRule(); + + @Test + @RequiresFlagsEnabled(Flags.FLAG_PREVENT_INTENT_REDIRECT) + public void testReadFromParcelWithExtraIntentKeys() { + Intent intent = new Intent("TEST_ACTION"); + intent.putExtra(TEST_EXTRA_NAME, new Intent(TEST_ACTION)); + intent.putExtra(TEST_EXTRA_NAME + "2", 1); + + intent.collectExtraIntentKeys(); + final Parcel parcel = Parcel.obtain(); + intent.writeToParcel(parcel, 0); + parcel.setDataPosition(0); + final Intent target = new Intent(); + target.readFromParcel(parcel); + + assertEquals(intent.getAction(), target.getAction()); + assertEquals(intent.getExtraIntentKeys(), target.getExtraIntentKeys()); + assertThat(intent.getExtraIntentKeys()).hasSize(1); + } + + @Test + public void testCreatorTokenInfo() { + Intent intent = new Intent(TEST_ACTION); + IBinder creatorToken = new Binder(); + + intent.setCreatorToken(creatorToken); + assertThat(intent.getCreatorToken()).isEqualTo(creatorToken); + + intent.removeCreatorTokenInfo(); + assertThat(intent.getCreatorToken()).isNull(); + } + + @Test + @RequiresFlagsEnabled(Flags.FLAG_PREVENT_INTENT_REDIRECT) + public void testCollectExtraIntentKeys() { + Intent intent = new Intent(TEST_ACTION); + Intent extraIntent = new Intent(TEST_ACTION, TEST_URI); + intent.putExtra(TEST_EXTRA_NAME, extraIntent); + + intent.collectExtraIntentKeys(); + + assertThat(intent.getExtraIntentKeys()).hasSize(1); + assertThat(intent.getExtraIntentKeys()).contains(TEST_EXTRA_NAME); + } + +} diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 3c574769eaec..7ff6832762c9 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -262,6 +262,7 @@ import android.appwidget.AppWidgetManagerInternal; import android.content.AttributionSource; import android.content.AutofillOptions; import android.content.BroadcastReceiver; +import android.content.ClipData; import android.content.ComponentCallbacks2; import android.content.ComponentName; import android.content.ContentCaptureOptions; @@ -418,7 +419,6 @@ import com.android.internal.util.FastPrintWriter; import com.android.internal.util.FrameworkStatsLog; import com.android.internal.util.MemInfoReader; import com.android.internal.util.Preconditions; -import com.android.server.crashrecovery.CrashRecoveryHelper; import com.android.server.AlarmManagerInternal; import com.android.server.BootReceiver; import com.android.server.DeviceIdleInternal; @@ -438,6 +438,7 @@ import com.android.server.am.LowMemDetector.MemFactor; import com.android.server.appop.AppOpsService; import com.android.server.compat.PlatformCompat; import com.android.server.contentcapture.ContentCaptureManagerInternal; +import com.android.server.crashrecovery.CrashRecoveryHelper; import com.android.server.criticalevents.CriticalEventLog; import com.android.server.firewall.IntentFirewall; import com.android.server.graphics.fonts.FontManagerInternal; @@ -482,6 +483,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStreamReader; import java.io.PrintWriter; +import java.lang.ref.WeakReference; import java.time.Instant; import java.time.ZoneId; import java.time.ZonedDateTime; @@ -499,6 +501,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.UUID; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executor; @@ -19078,4 +19081,87 @@ public class ActivityManagerService extends IActivityManager.Stub Freezer getFreezer() { return mFreezer; } + + // Set of IntentCreatorToken objects that are currently active. + private static final Map<IntentCreatorToken.Key, WeakReference<IntentCreatorToken>> + sIntentCreatorTokenCache = new ConcurrentHashMap<>(); + + /** + * A binder token used to keep track of which app created the intent. This token can be used to + * defend against intent redirect attacks. It stores uid of the intent creator and key fields of + * the intent to make it impossible for attacker to fake uid with a malicious intent. + * + * @hide + */ + public static final class IntentCreatorToken extends Binder { + @NonNull + private final Key mKeyFields; + + public IntentCreatorToken(int creatorUid, Intent intent) { + super(); + this.mKeyFields = new Key(creatorUid, intent); + } + + public int getCreatorUid() { + return mKeyFields.mCreatorUid; + } + + /** {@hide} */ + public static boolean isValid(@NonNull Intent intent) { + IBinder binder = intent.getCreatorToken(); + IntentCreatorToken token = null; + if (binder instanceof IntentCreatorToken) { + token = (IntentCreatorToken) binder; + } + return token != null && token.mKeyFields.equals( + new Key(token.mKeyFields.mCreatorUid, intent)); + } + + private static class Key { + private Key(int creatorUid, Intent intent) { + this.mCreatorUid = creatorUid; + this.mAction = intent.getAction(); + this.mData = intent.getData(); + this.mType = intent.getType(); + this.mPackage = intent.getPackage(); + this.mComponent = intent.getComponent(); + this.mFlags = intent.getFlags() & Intent.IMMUTABLE_FLAGS; + ClipData clipData = intent.getClipData(); + if (clipData != null) { + this.mClipDataUris = new ArrayList<>(clipData.getItemCount()); + for (int i = 0; i < clipData.getItemCount(); i++) { + this.mClipDataUris.add(clipData.getItemAt(i).getUri()); + } + } + } + + private final int mCreatorUid; + private final String mAction; + private final Uri mData; + private final String mType; + private final String mPackage; + private final ComponentName mComponent; + private final int mFlags; + private List<Uri> mClipDataUris; + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Key key = (Key) o; + return mCreatorUid == key.mCreatorUid && mFlags == key.mFlags && Objects.equals( + mAction, key.mAction) && Objects.equals(mData, key.mData) + && Objects.equals(mType, key.mType) && Objects.equals(mPackage, + key.mPackage) && Objects.equals(mComponent, key.mComponent) + && Objects.equals(mClipDataUris, key.mClipDataUris); + } + + @Override + public int hashCode() { + return Objects.hash(mCreatorUid, mAction, mData, mType, mPackage, mComponent, + mFlags, + mClipDataUris); + } + } + } } |