am 56cd646a: Avoid logging sensitive data. (e96a8207) · Commits · e / os / android_frameworks_base

services/java/com/android/server/MountService.java

+10 −9

Original line number	Diff line number	Diff line
		@@ -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) {

services/java/com/android/server/NativeDaemonConnector.java

+55 −21

Original line number	Diff line number	Diff line
		@@ -203,10 +203,29 @@ 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);
		}
		}
		}

services/java/com/android/server/NetworkManagementService.java

+3 −3

Original line number	Diff line number	Diff line
		@@ -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();

services/tests/servicestests/src/com/android/server/NativeDaemonConnectorTest.java

+27 −0

Original line number	Diff line number	Diff line
		@@ -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());
		}
		}