diff options
4 files changed, 95 insertions, 33 deletions
diff --git a/services/java/com/android/server/MountService.java b/services/java/com/android/server/MountService.java index d7adbf7b2eff..f402f4bd3dcc 100644 --- a/services/java/com/android/server/MountService.java +++ b/services/java/com/android/server/MountService.java @@ -64,6 +64,7 @@ import com.android.internal.app.IMediaContainerService; import com.android.internal.util.Preconditions; import com.android.internal.util.XmlUtils; import com.android.server.NativeDaemonConnector.Command; +import com.android.server.NativeDaemonConnector.SensitiveArg; import com.android.server.am.ActivityManagerService; import com.android.server.pm.PackageManagerService; import com.android.server.pm.UserManagerService; @@ -1642,8 +1643,8 @@ class MountService extends IMountService.Stub int rc = StorageResultCode.OperationSucceeded; try { - mConnector.execute("asec", "create", id, sizeMb, fstype, key, ownerUid, - external ? "1" : "0"); + mConnector.execute("asec", "create", id, sizeMb, fstype, new SensitiveArg(key), + ownerUid, external ? "1" : "0"); } catch (NativeDaemonConnectorException e) { rc = StorageResultCode.OperationFailedInternalError; } @@ -1743,7 +1744,7 @@ class MountService extends IMountService.Stub int rc = StorageResultCode.OperationSucceeded; try { - mConnector.execute("asec", "mount", id, key, ownerUid); + mConnector.execute("asec", "mount", id, new SensitiveArg(key), ownerUid); } catch (NativeDaemonConnectorException e) { int code = e.getCode(); if (code != VoldResponseCode.OpFailedStorageBusy) { @@ -2019,7 +2020,7 @@ class MountService extends IMountService.Stub final NativeDaemonEvent event; try { - event = mConnector.execute("cryptfs", "checkpw", password); + event = mConnector.execute("cryptfs", "checkpw", new SensitiveArg(password)); final int code = Integer.parseInt(event.getMessage()); if (code == 0) { @@ -2058,7 +2059,7 @@ class MountService extends IMountService.Stub } try { - mConnector.execute("cryptfs", "enablecrypto", "inplace", password); + mConnector.execute("cryptfs", "enablecrypto", "inplace", new SensitiveArg(password)); } catch (NativeDaemonConnectorException e) { // Encryption failed return e.getCode(); @@ -2083,7 +2084,7 @@ class MountService extends IMountService.Stub final NativeDaemonEvent event; try { - event = mConnector.execute("cryptfs", "changepw", password); + event = mConnector.execute("cryptfs", "changepw", new SensitiveArg(password)); return Integer.parseInt(event.getMessage()); } catch (NativeDaemonConnectorException e) { // Encryption failed @@ -2116,7 +2117,7 @@ class MountService extends IMountService.Stub final NativeDaemonEvent event; try { - event = mConnector.execute("cryptfs", "verifypw", password); + event = mConnector.execute("cryptfs", "verifypw", new SensitiveArg(password)); Slog.i(TAG, "cryptfs verifypw => " + event.getMessage()); return Integer.parseInt(event.getMessage()); } catch (NativeDaemonConnectorException e) { @@ -2482,8 +2483,8 @@ class MountService extends IMountService.Stub int rc = StorageResultCode.OperationSucceeded; try { - mConnector.execute( - "obb", "mount", mObbState.voldPath, hashedKey, mObbState.ownerGid); + mConnector.execute("obb", "mount", mObbState.voldPath, new SensitiveArg(hashedKey), + mObbState.ownerGid); } catch (NativeDaemonConnectorException e) { int code = e.getCode(); if (code != VoldResponseCode.OpFailedStorageBusy) { diff --git a/services/java/com/android/server/NativeDaemonConnector.java b/services/java/com/android/server/NativeDaemonConnector.java index c3f2afa04bf7..47840e0882ca 100644 --- a/services/java/com/android/server/NativeDaemonConnector.java +++ b/services/java/com/android/server/NativeDaemonConnector.java @@ -204,9 +204,28 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo } /** + * Wrapper around argument that indicates it's sensitive and shouldn't be + * logged. + */ + public static class SensitiveArg { + private final Object mArg; + + public SensitiveArg(Object arg) { + mArg = arg; + } + + @Override + public String toString() { + return String.valueOf(mArg); + } + } + + /** * Make command for daemon, escaping arguments as needed. */ - private void makeCommand(StringBuilder builder, String cmd, Object... args) { + @VisibleForTesting + static void makeCommand(StringBuilder rawBuilder, StringBuilder logBuilder, int sequenceNumber, + String cmd, Object... args) { if (cmd.indexOf('\0') >= 0) { throw new IllegalArgumentException("Unexpected command: " + cmd); } @@ -214,16 +233,26 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo throw new IllegalArgumentException("Arguments must be separate from command"); } - builder.append(cmd); + rawBuilder.append(sequenceNumber).append(' ').append(cmd); + logBuilder.append(sequenceNumber).append(' ').append(cmd); for (Object arg : args) { final String argString = String.valueOf(arg); if (argString.indexOf('\0') >= 0) { throw new IllegalArgumentException("Unexpected argument: " + arg); } - builder.append(' '); - appendEscaped(builder, argString); + rawBuilder.append(' '); + logBuilder.append(' '); + + appendEscaped(rawBuilder, argString); + if (arg instanceof SensitiveArg) { + logBuilder.append("[scrubbed]"); + } else { + appendEscaped(logBuilder, argString); + } } + + rawBuilder.append('\0'); } /** @@ -303,27 +332,27 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo */ public NativeDaemonEvent[] execute(int timeout, String cmd, Object... args) throws NativeDaemonConnectorException { + final long startTime = SystemClock.elapsedRealtime(); + final ArrayList<NativeDaemonEvent> events = Lists.newArrayList(); + final StringBuilder rawBuilder = new StringBuilder(); + final StringBuilder logBuilder = new StringBuilder(); final int sequenceNumber = mSequenceNumber.incrementAndGet(); - final StringBuilder cmdBuilder = - new StringBuilder(Integer.toString(sequenceNumber)).append(' '); - final long startTime = SystemClock.elapsedRealtime(); - makeCommand(cmdBuilder, cmd, args); + makeCommand(rawBuilder, logBuilder, sequenceNumber, cmd, args); - final String logCmd = cmdBuilder.toString(); /* includes cmdNum, cmd, args */ - log("SND -> {" + logCmd + "}"); + final String rawCmd = rawBuilder.toString(); + final String logCmd = logBuilder.toString(); - cmdBuilder.append('\0'); - final String sentCmd = cmdBuilder.toString(); /* logCmd + \0 */ + log("SND -> {" + logCmd + "}"); synchronized (mDaemonLock) { if (mOutputStream == null) { throw new NativeDaemonConnectorException("missing output stream"); } else { try { - mOutputStream.write(sentCmd.getBytes(Charsets.UTF_8)); + mOutputStream.write(rawCmd.getBytes(Charsets.UTF_8)); } catch (IOException e) { throw new NativeDaemonConnectorException("problem sending command", e); } @@ -332,7 +361,7 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo NativeDaemonEvent event = null; do { - event = mResponseQueue.remove(sequenceNumber, timeout, sentCmd); + event = mResponseQueue.remove(sequenceNumber, timeout, logCmd); if (event == null) { loge("timed-out waiting for response to " + logCmd); throw new NativeDaemonFailureException(logCmd, event); @@ -447,10 +476,11 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo private static class ResponseQueue { private static class PendingCmd { - public int cmdNum; + public final int cmdNum; + public final String logCmd; + public BlockingQueue<NativeDaemonEvent> responses = new ArrayBlockingQueue<NativeDaemonEvent>(10); - public String request; // The availableResponseCount member is used to track when we can remove this // instance from the ResponseQueue. @@ -468,7 +498,11 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo // hold references to this instance already so it can be removed from // mPendingCmds queue. public int availableResponseCount; - public PendingCmd(int c, String r) {cmdNum = c; request = r;} + + public PendingCmd(int cmdNum, String logCmd) { + this.cmdNum = cmdNum; + this.logCmd = logCmd; + } } private final LinkedList<PendingCmd> mPendingCmds; @@ -497,7 +531,7 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo // let any waiter timeout waiting for this PendingCmd pendingCmd = mPendingCmds.remove(); Slog.e("NativeDaemonConnector.ResponseQueue", - "Removing request: " + pendingCmd.request + " (" + + "Removing request: " + pendingCmd.logCmd + " (" + pendingCmd.cmdNum + ")"); } found = new PendingCmd(cmdNum, null); @@ -515,7 +549,7 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo // note that the timeout does not count time in deep sleep. If you don't want // the device to sleep, hold a wakelock - public NativeDaemonEvent remove(int cmdNum, int timeoutMs, String origCmd) { + public NativeDaemonEvent remove(int cmdNum, int timeoutMs, String logCmd) { PendingCmd found = null; synchronized (mPendingCmds) { for (PendingCmd pendingCmd : mPendingCmds) { @@ -525,7 +559,7 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo } } if (found == null) { - found = new PendingCmd(cmdNum, origCmd); + found = new PendingCmd(cmdNum, logCmd); mPendingCmds.add(found); } found.availableResponseCount--; @@ -547,7 +581,7 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo pw.println("Pending requests:"); synchronized (mPendingCmds) { for (PendingCmd pendingCmd : mPendingCmds) { - pw.println(" Cmd " + pendingCmd.cmdNum + " - " + pendingCmd.request); + pw.println(" Cmd " + pendingCmd.cmdNum + " - " + pendingCmd.logCmd); } } } diff --git a/services/java/com/android/server/NetworkManagementService.java b/services/java/com/android/server/NetworkManagementService.java index d2acb404ada9..3b84c7323043 100644 --- a/services/java/com/android/server/NetworkManagementService.java +++ b/services/java/com/android/server/NetworkManagementService.java @@ -34,7 +34,6 @@ import static com.android.server.NetworkManagementService.NetdResponseCode.Tethe import static com.android.server.NetworkManagementService.NetdResponseCode.TtyListResult; import static com.android.server.NetworkManagementSocketTagger.PROP_QTAGUID_ENABLED; -import android.bluetooth.BluetoothTetheringDataTracker; import android.content.Context; import android.net.INetworkManagementEventObserver; import android.net.InterfaceConfiguration; @@ -59,6 +58,7 @@ import android.util.SparseBooleanArray; import com.android.internal.net.NetworkStatsFactory; import com.android.internal.util.Preconditions; import com.android.server.NativeDaemonConnector.Command; +import com.android.server.NativeDaemonConnector.SensitiveArg; import com.android.server.net.LockdownVpnTracker; import com.google.android.collect.Maps; @@ -990,7 +990,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub mConnector.execute("softap", "set", wlanIface); } else { mConnector.execute("softap", "set", wlanIface, wifiConfig.SSID, - getSecurityType(wifiConfig), wifiConfig.preSharedKey); + getSecurityType(wifiConfig), new SensitiveArg(wifiConfig.preSharedKey)); } mConnector.execute("softap", "startap"); } catch (NativeDaemonConnectorException e) { @@ -1039,7 +1039,7 @@ public class NetworkManagementService extends INetworkManagementService.Stub mConnector.execute("softap", "set", wlanIface); } else { mConnector.execute("softap", "set", wlanIface, wifiConfig.SSID, - getSecurityType(wifiConfig), wifiConfig.preSharedKey); + getSecurityType(wifiConfig), new SensitiveArg(wifiConfig.preSharedKey)); } } catch (NativeDaemonConnectorException e) { throw e.rethrowAsParcelableException(); diff --git a/services/tests/servicestests/src/com/android/server/NativeDaemonConnectorTest.java b/services/tests/servicestests/src/com/android/server/NativeDaemonConnectorTest.java index 275d80764533..e2253a2151b0 100644 --- a/services/tests/servicestests/src/com/android/server/NativeDaemonConnectorTest.java +++ b/services/tests/servicestests/src/com/android/server/NativeDaemonConnectorTest.java @@ -17,10 +17,13 @@ package com.android.server; import static com.android.server.NativeDaemonConnector.appendEscaped; +import static com.android.server.NativeDaemonConnector.makeCommand; import android.test.AndroidTestCase; import android.test.suitebuilder.annotation.MediumTest; +import com.android.server.NativeDaemonConnector.SensitiveArg; + /** * Tests for {@link NativeDaemonConnector}. */ @@ -67,4 +70,28 @@ public class NativeDaemonConnectorTest extends AndroidTestCase { appendEscaped(builder, "caf\u00E9 c\u00F6ffee"); assertEquals("\"caf\u00E9 c\u00F6ffee\"", builder.toString()); } + + public void testSensitiveArgs() throws Exception { + final StringBuilder rawBuilder = new StringBuilder(); + final StringBuilder logBuilder = new StringBuilder(); + + rawBuilder.setLength(0); + logBuilder.setLength(0); + makeCommand(rawBuilder, logBuilder, 1, "foo", "bar", "baz"); + assertEquals("1 foo bar baz\0", rawBuilder.toString()); + assertEquals("1 foo bar baz", logBuilder.toString()); + + rawBuilder.setLength(0); + logBuilder.setLength(0); + makeCommand(rawBuilder, logBuilder, 1, "foo", new SensitiveArg("bar"), "baz"); + assertEquals("1 foo bar baz\0", rawBuilder.toString()); + assertEquals("1 foo [scrubbed] baz", logBuilder.toString()); + + rawBuilder.setLength(0); + logBuilder.setLength(0); + makeCommand(rawBuilder, logBuilder, 1, "foo", new SensitiveArg("foo bar"), "baz baz", + new SensitiveArg("wat")); + assertEquals("1 foo \"foo bar\" \"baz baz\" wat\0", rawBuilder.toString()); + assertEquals("1 foo [scrubbed] \"baz baz\" [scrubbed]", logBuilder.toString()); + } } |