Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

am 56cd646a: Avoid logging sensitive data.

Browse Source 
* commit '56cd646abeae51e806791f82ab0995fe047b1fe4':
  Avoid logging sensitive data.
This commit is contained in:
Jeff Sharkey
2013-06-09 19:07:48 -07:00
committed by Android Git Automerger
parent 87c79d64ab 56cd646abe
commit e96a820771
4 changed files with 97 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
services/java/com/android/server/MountService.java
View File

@@ -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) {

80
services/java/com/android/server/NativeDaemonConnector.java
View File

@@ -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 ArrayList<NativeDaemonEvent> events = Lists.newArrayList();
final int sequenceNumber = mSequenceNumber.incrementAndGet();
final StringBuilder cmdBuilder =
new StringBuilder(Integer.toString(sequenceNumber)).append(' ');
final long startTime = SystemClock.elapsedRealtime();
makeCommand(cmdBuilder, cmd, args);
final ArrayList<NativeDaemonEvent> events = Lists.newArrayList();
final StringBuilder rawBuilder = new StringBuilder();
final StringBuilder logBuilder = new StringBuilder();
final int sequenceNumber = mSequenceNumber.incrementAndGet();
makeCommand(rawBuilder, logBuilder, sequenceNumber, cmd, args);
final String rawCmd = rawBuilder.toString();
final String logCmd = logBuilder.toString();
final String logCmd = cmdBuilder.toString(); /* includes cmdNum, cmd, args */
log("SND -> {" + logCmd + "}");
cmdBuilder.append('\0');
final String sentCmd = cmdBuilder.toString(); /* logCmd + \0 */
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);
}
}
}

6
services/java/com/android/server/NetworkManagementService.java
View File

@@ -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();

27
services/tests/servicestests/src/com/android/server/NativeDaemonConnectorTest.java
View File

@@ -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());
}
}