From eb1d4893324fcc6270cfc8a60a63d7bdc964f695 Mon Sep 17 00:00:00 2001 From: Nathan Harold Date: Thu, 9 Nov 2017 16:49:33 -0800 Subject: [PATCH 1/2] Validate IpSecAlgorithm Length Improve the Validation of IpSecAlgorithm by explicitly checking the length in addition to the truncation length (previously an oversight). In addition, we now check the lengths during un-parceling, which will catch someone maliciously manually building a parcel and passing it, bypassing the checks in the constructor. Bug: 68780091 Test: runtest -x IpSecAlgorithmTest.java Change-Id: I8172762617264d34f47d5144336464510f07a701 --- core/java/android/net/IpSecAlgorithm.java | 72 ++++++++++++++++------- 1 file changed, 50 insertions(+), 22 deletions(-) diff --git a/core/java/android/net/IpSecAlgorithm.java b/core/java/android/net/IpSecAlgorithm.java index d6e62cf1f8..f82627b942 100644 --- a/core/java/android/net/IpSecAlgorithm.java +++ b/core/java/android/net/IpSecAlgorithm.java @@ -21,6 +21,7 @@ import android.os.Build; import android.os.Parcel; import android.os.Parcelable; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.HexDump; import java.lang.annotation.Retention; @@ -34,6 +35,8 @@ import java.util.Arrays; * Internet Protocol */ public final class IpSecAlgorithm implements Parcelable { + private static final String TAG = "IpSecAlgorithm"; + /** * AES-CBC Encryption/Ciphering Algorithm. * @@ -45,6 +48,7 @@ public final class IpSecAlgorithm implements Parcelable { * MD5 HMAC Authentication/Integrity Algorithm. This algorithm is not recommended for use in * new applications and is provided for legacy compatibility with 3gpp infrastructure. * + *

Keys for this algorithm must be 128 bits in length. *

Valid truncation lengths are multiples of 8 bits from 96 to (default) 128. */ public static final String AUTH_HMAC_MD5 = "hmac(md5)"; @@ -53,6 +57,7 @@ public final class IpSecAlgorithm implements Parcelable { * SHA1 HMAC Authentication/Integrity Algorithm. This algorithm is not recommended for use in * new applications and is provided for legacy compatibility with 3gpp infrastructure. * + *

Keys for this algorithm must be 160 bits in length. *

Valid truncation lengths are multiples of 8 bits from 96 to (default) 160. */ public static final String AUTH_HMAC_SHA1 = "hmac(sha1)"; @@ -60,6 +65,7 @@ public final class IpSecAlgorithm implements Parcelable { /** * SHA256 HMAC Authentication/Integrity Algorithm. * + *

Keys for this algorithm must be 256 bits in length. *

Valid truncation lengths are multiples of 8 bits from 96 to (default) 256. */ public static final String AUTH_HMAC_SHA256 = "hmac(sha256)"; @@ -67,6 +73,7 @@ public final class IpSecAlgorithm implements Parcelable { /** * SHA384 HMAC Authentication/Integrity Algorithm. * + *

Keys for this algorithm must be 384 bits in length. *

Valid truncation lengths are multiples of 8 bits from 192 to (default) 384. */ public static final String AUTH_HMAC_SHA384 = "hmac(sha384)"; @@ -74,6 +81,7 @@ public final class IpSecAlgorithm implements Parcelable { /** * SHA512 HMAC Authentication/Integrity Algorithm. * + *

Keys for this algorithm must be 512 bits in length. *

Valid truncation lengths are multiples of 8 bits from 256 to (default) 512. */ public static final String AUTH_HMAC_SHA512 = "hmac(sha512)"; @@ -130,12 +138,10 @@ public final class IpSecAlgorithm implements Parcelable { * @param truncLenBits number of bits of output hash to use. */ public IpSecAlgorithm(@AlgorithmName String algorithm, @NonNull byte[] key, int truncLenBits) { - if (!isTruncationLengthValid(algorithm, truncLenBits)) { - throw new IllegalArgumentException("Unknown algorithm or invalid length"); - } mName = algorithm; mKey = key.clone(); - mTruncLenBits = Math.min(truncLenBits, key.length * 8); + mTruncLenBits = truncLenBits; + checkValidOrThrow(mName, mKey.length * 8, mTruncLenBits); } /** Get the algorithm name */ @@ -169,7 +175,11 @@ public final class IpSecAlgorithm implements Parcelable { public static final Parcelable.Creator CREATOR = new Parcelable.Creator() { public IpSecAlgorithm createFromParcel(Parcel in) { - return new IpSecAlgorithm(in); + final String name = in.readString(); + final byte[] key = in.createByteArray(); + final int truncLenBits = in.readInt(); + + return new IpSecAlgorithm(name, key, truncLenBits); } public IpSecAlgorithm[] newArray(int size) { @@ -177,30 +187,47 @@ public final class IpSecAlgorithm implements Parcelable { } }; - private IpSecAlgorithm(Parcel in) { - mName = in.readString(); - mKey = in.createByteArray(); - mTruncLenBits = in.readInt(); - } + private static void checkValidOrThrow(String name, int keyLen, int truncLen) { + boolean isValidLen = true; + boolean isValidTruncLen = true; - private static boolean isTruncationLengthValid(String algo, int truncLenBits) { - switch (algo) { + switch(name) { case CRYPT_AES_CBC: - return (truncLenBits == 128 || truncLenBits == 192 || truncLenBits == 256); + isValidLen = keyLen == 128 || keyLen == 192 || keyLen == 256; + break; case AUTH_HMAC_MD5: - return (truncLenBits >= 96 && truncLenBits <= 128); + isValidLen = keyLen == 128; + isValidTruncLen = truncLen >= 96 && truncLen <= 128; + break; case AUTH_HMAC_SHA1: - return (truncLenBits >= 96 && truncLenBits <= 160); + isValidLen = keyLen == 160; + isValidTruncLen = truncLen >= 96 && truncLen <= 160; + break; case AUTH_HMAC_SHA256: - return (truncLenBits >= 96 && truncLenBits <= 256); + isValidLen = keyLen == 256; + isValidTruncLen = truncLen >= 96 && truncLen <= 256; + break; case AUTH_HMAC_SHA384: - return (truncLenBits >= 192 && truncLenBits <= 384); + isValidLen = keyLen == 384; + isValidTruncLen = truncLen >= 192 && truncLen <= 384; + break; case AUTH_HMAC_SHA512: - return (truncLenBits >= 256 && truncLenBits <= 512); + isValidLen = keyLen == 512; + isValidTruncLen = truncLen >= 256 && truncLen <= 512; + break; case AUTH_CRYPT_AES_GCM: - return (truncLenBits == 64 || truncLenBits == 96 || truncLenBits == 128); + // The keying material for GCM is a key plus a 32-bit salt + isValidLen = keyLen == 128 + 32 || keyLen == 192 + 32 || keyLen == 256 + 32; + break; default: - return false; + throw new IllegalArgumentException("Couldn't find an algorithm: " + name); + } + + if (!isValidLen) { + throw new IllegalArgumentException("Invalid key material keyLength: " + keyLen); + } + if (!isValidTruncLen) { + throw new IllegalArgumentException("Invalid truncation keyLength: " + truncLen); } } @@ -217,8 +244,9 @@ public final class IpSecAlgorithm implements Parcelable { .toString(); } - /** package */ - static boolean equals(IpSecAlgorithm lhs, IpSecAlgorithm rhs) { + /** @hide */ + @VisibleForTesting + public static boolean equals(IpSecAlgorithm lhs, IpSecAlgorithm rhs) { if (lhs == null || rhs == null) return (lhs == rhs); return (lhs.mName.equals(rhs.mName) && Arrays.equals(lhs.mKey, rhs.mKey) From 4e164f9e94e3756fa848c0e23ca0757693682a1c Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 29 Nov 2017 11:18:23 -0700 Subject: [PATCH 2/2] API for apps to tag sockets with their own UID. This enables app A to create a socket, pass it to app B, and have app B accept blame for the traffic performed on that socket. Also adds helpful public APIs for tagging raw FileDescriptor sockets instead of making developers go through shady SocketImpl wrappers. Test: cts-tradefed run commandAndExit cts-dev -m CtsAppSecurityHostTestCases -t android.appsecurity.cts.AppSecurityTests#testAppFailAccessPrivateData Bug: 63932076 Change-Id: I08925c843974675fc82e4080cec2eaab9ab7cd41 --- core/java/android/net/TrafficStats.java | 39 +++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/core/java/android/net/TrafficStats.java b/core/java/android/net/TrafficStats.java index c339856f43..954e59c2c4 100644 --- a/core/java/android/net/TrafficStats.java +++ b/core/java/android/net/TrafficStats.java @@ -17,6 +17,7 @@ package android.net; import android.annotation.RequiresPermission; +import android.annotation.SuppressLint; import android.annotation.SystemApi; import android.app.DownloadManager; import android.app.backup.BackupManager; @@ -30,6 +31,8 @@ import com.android.server.NetworkManagementSocketTagger; import dalvik.system.SocketTagger; +import java.io.FileDescriptor; +import java.io.IOException; import java.net.DatagramSocket; import java.net.Socket; import java.net.SocketException; @@ -263,15 +266,26 @@ public class TrafficStats { NetworkManagementSocketTagger.setThreadSocketStatsUid(uid); } + /** + * Set specific UID to use when accounting {@link Socket} traffic + * originating from the current thread as the calling UID. Designed for use + * when another application is performing operations on your behalf. + *

+ * Changes only take effect during subsequent calls to + * {@link #tagSocket(Socket)}. + */ + public static void setThreadStatsUidSelf() { + setThreadStatsUid(android.os.Process.myUid()); + } + /** * Clear any active UID set to account {@link Socket} traffic originating * from the current thread. * * @see #setThreadStatsUid(int) - * @hide */ @SystemApi - @RequiresPermission(android.Manifest.permission.UPDATE_DEVICE_STATS) + @SuppressLint("Doclava125") public static void clearThreadStatsUid() { NetworkManagementSocketTagger.setThreadSocketStatsUid(-1); } @@ -315,6 +329,27 @@ public class TrafficStats { SocketTagger.get().untag(socket); } + /** + * Tag the given {@link FileDescriptor} socket with any statistics + * parameters active for the current thread. Subsequent calls always replace + * any existing parameters. When finished, call + * {@link #untagFileDescriptor(FileDescriptor)} to remove statistics + * parameters. + * + * @see #setThreadStatsTag(int) + */ + public static void tagFileDescriptor(FileDescriptor fd) throws IOException { + SocketTagger.get().tag(fd); + } + + /** + * Remove any statistics parameters from the given {@link FileDescriptor} + * socket. + */ + public static void untagFileDescriptor(FileDescriptor fd) throws IOException { + SocketTagger.get().untag(fd); + } + /** * Start profiling data usage for current UID. Only one profiling session * can be active at a time.