From 1e803d5ed0544fc9fe57199ad32d4310615fcaad Mon Sep 17 00:00:00 2001 From: Paul Lawrence Date: Tue, 11 Nov 2014 17:28:28 +0000 Subject: [PATCH 1/2] Revert "DO NOT MERGE: Don't log passwords returned from vdc" This reverts commit 201c2f73b61c220cd26c2f132522fd679c726ae4. The original change disabled all logging of RCVed messages in NativeDaemonConnector. For MR1 we want a much more surgical disabling of sensitive messages. First, though, we have to defeat the automerger. Change-Id: I712919aee2db63f7fc0b2c6d6a2a658325dce596 --- .../core/java/com/android/server/NativeDaemonConnector.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/services/core/java/com/android/server/NativeDaemonConnector.java b/services/core/java/com/android/server/NativeDaemonConnector.java index 8c3b020f57..96f9ab061c 100644 --- a/services/core/java/com/android/server/NativeDaemonConnector.java +++ b/services/core/java/com/android/server/NativeDaemonConnector.java @@ -176,6 +176,7 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo if (buffer[i] == 0) { final String rawEvent = new String( buffer, start, i - start, StandardCharsets.UTF_8); + log("RCV <- {" + rawEvent + "}"); boolean releaseWl = false; try { @@ -196,6 +197,7 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo mResponseQueue.add(event.getCmdNumber(), event); } } catch (IllegalArgumentException e) { + log("Problem parsing message: " + rawEvent + " - " + e); } finally { if (releaseWl) { mWakeLock.acquire(); @@ -207,6 +209,7 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo } if (start == 0) { final String rawEvent = new String(buffer, start, count, StandardCharsets.UTF_8); + log("RCV incomplete <- {" + rawEvent + "}"); } // We should end at the amount we read. If not, compact then From 19a1c3f7d96b59ce844b23e2478ddc29a394804c Mon Sep 17 00:00:00 2001 From: Paul Lawrence Date: Tue, 11 Nov 2014 12:23:22 -0800 Subject: [PATCH 2/2] Add mechanism for securely returning parameters though NativeDaemonConnector If the first word in the response is {{sensitive}}, don't log the body of the response. Bug: 18260068 Change-Id: Ibfb5c6abab1d04b4321cdbcf6c7cf6f18f903146 --- .../android/server/NativeDaemonConnector.java | 12 ++++++++---- .../com/android/server/NativeDaemonEvent.java | 17 ++++++++++++++--- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/NativeDaemonConnector.java b/services/core/java/com/android/server/NativeDaemonConnector.java index 96f9ab061c..d2dfc7bbdc 100644 --- a/services/core/java/com/android/server/NativeDaemonConnector.java +++ b/services/core/java/com/android/server/NativeDaemonConnector.java @@ -174,14 +174,18 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo for (int i = 0; i < count; i++) { if (buffer[i] == 0) { + // Note - do not log this raw message since it may contain + // sensitive data final String rawEvent = new String( buffer, start, i - start, StandardCharsets.UTF_8); - log("RCV <- {" + rawEvent + "}"); boolean releaseWl = false; try { final NativeDaemonEvent event = NativeDaemonEvent.parseRawEvent( rawEvent); + + log("RCV <- {" + event + "}"); + if (event.isClassUnsolicited()) { // TODO: migrate to sending NativeDaemonEvent instances if (mCallbacks.onCheckHoldWakeLock(event.getCode()) @@ -197,7 +201,7 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo mResponseQueue.add(event.getCmdNumber(), event); } } catch (IllegalArgumentException e) { - log("Problem parsing message: " + rawEvent + " - " + e); + log("Problem parsing message " + e); } finally { if (releaseWl) { mWakeLock.acquire(); @@ -207,9 +211,9 @@ final class NativeDaemonConnector implements Runnable, Handler.Callback, Watchdo start = i + 1; } } + if (start == 0) { - final String rawEvent = new String(buffer, start, count, StandardCharsets.UTF_8); - log("RCV incomplete <- {" + rawEvent + "}"); + log("RCV incomplete"); } // We should end at the amount we read. If not, compact then diff --git a/services/core/java/com/android/server/NativeDaemonEvent.java b/services/core/java/com/android/server/NativeDaemonEvent.java index 59d50bde1b..4e61c0b09d 100644 --- a/services/core/java/com/android/server/NativeDaemonEvent.java +++ b/services/core/java/com/android/server/NativeDaemonEvent.java @@ -33,16 +33,21 @@ public class NativeDaemonEvent { private final int mCode; private final String mMessage; private final String mRawEvent; + private final String mLogMessage; private String[] mParsed; - private NativeDaemonEvent(int cmdNumber, int code, String message, String rawEvent) { + private NativeDaemonEvent(int cmdNumber, int code, String message, + String rawEvent, String logMessage) { mCmdNumber = cmdNumber; mCode = code; mMessage = message; mRawEvent = rawEvent; + mLogMessage = logMessage; mParsed = null; } + static public final String SENSITIVE_MARKER = "{{sensitive}}"; + public int getCmdNumber() { return mCmdNumber; } @@ -62,7 +67,7 @@ public class NativeDaemonEvent { @Override public String toString() { - return mRawEvent; + return mLogMessage; } /** @@ -151,9 +156,15 @@ public class NativeDaemonEvent { } } + String logMessage = rawEvent; + if (parsed.length > 2 && parsed[2].equals(SENSITIVE_MARKER)) { + skiplength += parsed[2].length() + 1; + logMessage = parsed[0] + " " + parsed[1] + " {}"; + } + final String message = rawEvent.substring(skiplength); - return new NativeDaemonEvent(cmdNumber, code, message, rawEvent); + return new NativeDaemonEvent(cmdNumber, code, message, rawEvent, logMessage); } /**