Password manager internals page: Improve security

components/autofill/core/common/save_password_progress_logger.cc

Index: components/autofill/core/common/save_password_progress_logger.cc
diff --git a/components/autofill/core/common/save_password_progress_logger.cc b/components/autofill/core/common/save_password_progress_logger.cc
index cdb8dd7a177e97f1542db30a714749683b3094f1..397b58762e44bb885cc06af7781a5f84560dc085 100644
--- a/components/autofill/core/common/save_password_progress_logger.cc
+++ b/components/autofill/core/common/save_password_progress_logger.cc
@@ -4,9 +4,15 @@
 
 #include "components/autofill/core/common/save_password_progress_logger.h"
 
+#include <algorithm>
+
 #include "base/json/json_writer.h"
 #include "base/logging.h"
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/numerics/safe_conversions.h"
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "components/autofill/core/common/password_form.h"
 
@@ -20,6 +26,42 @@ namespace autofill {
 
 namespace {
 
+// These messages can be used as dictionary keys. Do not use '.' in them.
+const char* const kIDsToMessages[] = {
+    "Decision: ASK the user",         // STRING_DECISION_ASK
+    "Decision: DROP the password",    // STRING_DECISION_DROP,
+    "Decision: SAVE the password",    // STRING_DECISION_SAVE,
+    "Form method",                    // STRING_METHOD,
+    "GET",                            // STRING_METHOD_GET,
+    "POST",                           // STRING_METHOD_POST,
+    "(empty)",                        // STRING_METHOD_EMPTY,
+    "(other)",                        // STRING_OTHER,
+    "HTML",                           // STRING_SCHEME_HTML,
+    "Basic",                          // STRING_SCHEME_BASIC,
+    "Digest",                         // STRING_SCHEME_DIGEST,
+    "Scheme",                         // STRING_SCHEME_MESSAGE,
+    "Signon realm",                   // STRING_SIGNON_REALM,
+    "Original signon realm",          // STRING_ORIGINAL_SIGNON_REALM,
+    "Origin",                         // STRING_ORIGIN,
+    "Action",                         // STRING_ACTION,
+    "Username element",               // STRING_USERNAME_ELEMENT,
+    "Password element",               // STRING_PASSWORD_ELEMENT,
+    "Password autocomplete set",      // STRING_PASSWORD_AUTOCOMPLETE_SET,
+    "Old password element",           // STRING_OLD_PASSWORD_ELEMENT,
+    "SSL valid",                      // STRING_SSL_VALID,
+    "Password generated",             // STRING_PASSWORD_GENERATED,
+    "Times used",                     // STRING_TIMES_USED,
+    "Use additional authentication",  // STRING_USE_ADDITIONAL_AUTHENTICATION,
+    "PSL match",                      // STRING_PSL_MATCH,
+    "Form name or ID",                // STRING_NAME_OR_ID,
+    "Message",                        // STRING_MESSAGE,
+    "INVALID",                        // STRING_INVALID,
+};
+
+COMPILE_ASSERT(SavePasswordProgressLogger::StringID::STRING_MAX + 1 ==
+                   arraysize(kIDsToMessages),
+               IDsToMessagesMappingIsOutOfDate);
+
 // Removes privacy sensitive parts of |url| (currently all but host and scheme).
 std::string ScrubURL(const GURL& url) {
   if (url.is_valid())
@@ -27,112 +69,302 @@ std::string ScrubURL(const GURL& url) {
   return std::string();
 }
 
-std::string FormSchemeToString(PasswordForm::Scheme scheme) {
+// Returns true for all characters which we don't want to see in the logged IDs
+// or names of HTML elements.
+bool IsUnwantedInElementID (char c) {
+  return !IsAsciiAlpha(c) && !IsAsciiDigit(c);
+}
+
+// Replaces all characters satisfying IsUnwantedInElementID by a ' ', and turns
+// all characters to lowercase. This damages some valid HTML element IDs or
+// names, but it is likely that it will be still possible to match the scrubbed
+// string to the original ID or name in the HTML doc. That's good enough for the
+// logging purposes, and provides some security benefits.
+std::string ScrubElementID(std::string element_id) {
+  std::replace_if(
+      element_id.begin(), element_id.end(), IsUnwantedInElementID, ' ');
+  return StringToLowerASCII(element_id);
+}
+
+SavePasswordProgressLogger::StringID FormSchemeToStringID(
+    PasswordForm::Scheme scheme) {
   switch (scheme) {
     case PasswordForm::SCHEME_HTML:
-      return "HTML";
+      return SavePasswordProgressLogger::STRING_SCHEME_HTML;
     case PasswordForm::SCHEME_BASIC:
-      return "BASIC";
+      return SavePasswordProgressLogger::STRING_SCHEME_BASIC;
     case PasswordForm::SCHEME_DIGEST:
-      return "DIGEST";
+      return SavePasswordProgressLogger::STRING_SCHEME_DIGEST;
     case PasswordForm::SCHEME_OTHER:
-      return "OTHER";
+      return SavePasswordProgressLogger::STRING_OTHER;
   }
   NOTREACHED();  // Win compilers don't believe this is unreachable.
-  return std::string();
+  return SavePasswordProgressLogger::StringID::STRING_INVALID;
+}
+
+SavePasswordProgressLogger::StringID FormMethodToStringID(
+    const std::string& method) {
+  std::string method_processed;
+  base::TrimWhitespaceASCII(
+      StringToLowerASCII(method), base::TRIM_ALL, &method_processed);
+  if (method_processed.empty())
+    return SavePasswordProgressLogger::STRING_METHOD_EMPTY;
+  else if (method_processed == "get")
+    return SavePasswordProgressLogger::STRING_METHOD_GET;
+  else if (method_processed == "post")
+    return SavePasswordProgressLogger::STRING_METHOD_POST;
+  else
+    return SavePasswordProgressLogger::STRING_OTHER;
 }
 
-StringValue DecisionToStringValue(
-    SavePasswordProgressLogger::Decision decision) {
-  switch (decision) {
-    case SavePasswordProgressLogger::DECISION_SAVE:
-      return StringValue("SAVE the password");
-    case SavePasswordProgressLogger::DECISION_ASK:
-      return StringValue("ASK the user whether to save the password");
-    case SavePasswordProgressLogger::DECISION_DROP:
-      return StringValue("DROP the password");
+// Interprets |log|, sanitizes its value if needed, and returns it as
+// base::Value. Does not work on LOG_TYPE_LABEL logs.
+scoped_ptr<Value> GetValueFromStructuredLog(
+    const SavePasswordProgressLogger::StructuredLog& log) {
+  switch (log.log_type) {
+    case SavePasswordProgressLogger::StructuredLog::LOG_TYPE_LABEL:
+      NOTREACHED();
+      return scoped_ptr<Value>();
+    case SavePasswordProgressLogger::StructuredLog::LOG_TYPE_GURL:
+      return scoped_ptr<Value>(Value::CreateStringValue(ScrubURL(log.url)));
+    case SavePasswordProgressLogger::StructuredLog::LOG_TYPE_MESSAGE:
+      return scoped_ptr<Value>(
+          Value::CreateStringValue(kIDsToMessages[log.message]));
+    case SavePasswordProgressLogger::StructuredLog::LOG_TYPE_NUMBER:
+      return scoped_ptr<Value>(Value::CreateIntegerValue(log.number));
+    case SavePasswordProgressLogger::StructuredLog::LOG_TYPE_BOOL:
+      return scoped_ptr<Value>(Value::CreateBooleanValue(log.truth_value));
+    case SavePasswordProgressLogger::StructuredLog::LOG_TYPE_ELEMENT_ID:
+      return scoped_ptr<Value>(
+          Value::CreateStringValue(ScrubElementID(log.element_id)));
   }
   NOTREACHED();  // Win compilers don't believe this is unreachable.
-  return StringValue(std::string());
+  return scoped_ptr<Value>();
 }
 
 }  // namespace
 
-SavePasswordProgressLogger::SavePasswordProgressLogger() {}
+SavePasswordProgressLogger::StructuredLog::StructuredLog()
+    : log_type(LOG_TYPE_LABEL),
+      label(STRING_INVALID),
+      message(STRING_INVALID),
+      number(0),
+      truth_value(false) {
+}
 
-SavePasswordProgressLogger::~SavePasswordProgressLogger() {}
+SavePasswordProgressLogger::StructuredLog::StructuredLog(
+    SavePasswordProgressLogger::StringID label)
+    : log_type(LOG_TYPE_LABEL),
+      label(label),
+      message(STRING_INVALID),
+      number(0),
+      truth_value(false) {
+}
 
-void SavePasswordProgressLogger::LogPasswordForm(const std::string& message,
-                                                 const PasswordForm& form) {
-  DictionaryValue log;
-  // Do not use the "<<" operator for PasswordForms, because it also prints
-  // passwords. Also, that operator is only for testing.
-  log.SetString("scheme", FormSchemeToString(form.scheme));
-  log.SetString("signon realm", ScrubURL(GURL(form.signon_realm)));
-  log.SetString("original signon realm",
-                ScrubURL(GURL(form.original_signon_realm)));
-  log.SetString("origin", ScrubURL(form.origin));
-  log.SetString("action", ScrubURL(form.action));
-  log.SetString("username element", form.username_element);
-  log.SetString("password element", form.password_element);
-  log.SetBoolean("password autocomplete set", form.password_autocomplete_set);
-  log.SetString("old password element", form.old_password_element);
-  log.SetBoolean("ssl valid", form.ssl_valid);
-  log.SetBoolean("password generated",
-                 form.type == PasswordForm::TYPE_GENERATED);
-  log.SetInteger("times used", form.times_used);
-  log.SetBoolean("use additional authentication",
-                 form.use_additional_authentication);
-  log.SetBoolean("is PSL match", form.IsPublicSuffixMatch());
-  LogValue(message, log);
+SavePasswordProgressLogger::StructuredLog::StructuredLog(StringID label,
+                                                         GURL url)
+    : log_type(LOG_TYPE_GURL),
+      label(label),
+      url(url),
+      message(STRING_INVALID),
+      number(0),
+      truth_value(false) {
 }
 
-void SavePasswordProgressLogger::LogHTMLForm(const std::string& message,
-                                             const std::string& name_or_id,
-                                             const std::string& method,
-                                             const GURL& action) {
-  DictionaryValue log;
-  log.SetString("name_or_id", name_or_id);
-  log.SetString("method", method);
-  log.SetString("action", ScrubURL(action));
-  LogValue(message, log);
+SavePasswordProgressLogger::StructuredLog::StructuredLog(StringID label,
+                                                         StringID message)
+    : log_type(LOG_TYPE_MESSAGE),
+      label(label),
+      message(message),
+      number(0),
+      truth_value(false) {
 }
 
-void SavePasswordProgressLogger::LogURL(const std::string& message,
-                                        const GURL& url) {
-  LogValue(message, StringValue(ScrubURL(url)));
+SavePasswordProgressLogger::StructuredLog::StructuredLog(StringID label,
+                                                         int number)
+    : log_type(LOG_TYPE_NUMBER),
+      label(label),
+      message(STRING_INVALID),
+      number(number),
+      truth_value(false) {
 }
 
-void SavePasswordProgressLogger::LogBoolean(const std::string& message,
-                                            bool value) {
-  LogValue(message, FundamentalValue(value));
+SavePasswordProgressLogger::StructuredLog::StructuredLog(StringID label,
+                                                         bool truth_value)
+    : log_type(LOG_TYPE_BOOL),
+      label(label),
+      message(STRING_INVALID),
+      number(0),
+      truth_value(truth_value) {
 }
 
-void SavePasswordProgressLogger::LogNumber(const std::string& message,
-                                           int value) {
-  LogValue(message, FundamentalValue(value));
+SavePasswordProgressLogger::StructuredLog::StructuredLog(
+    StringID label,
+    const base::string16& element_id)
+    : log_type(LOG_TYPE_ELEMENT_ID),
+      label(label),
+      message(STRING_INVALID),
+      number(0),
+      truth_value(false),
+      element_id(UTF16ToUTF8(element_id)) {
 }
 
-void SavePasswordProgressLogger::LogNumber(const std::string& message,
-                                           size_t value) {
-  LogValue(message, FundamentalValue(checked_cast<int, size_t>(value)));
+SavePasswordProgressLogger::StructuredLog::StructuredLog(
+    StringID label,
+    const std::string& element_id)
+    : log_type(LOG_TYPE_ELEMENT_ID),
+      label(label),
+      message(STRING_INVALID),
+      number(0),
+      truth_value(false),
+      element_id(element_id) {
 }
 
-void SavePasswordProgressLogger::LogFinalDecision(Decision decision) {
-  LogValue("Final decision taken", DecisionToStringValue(decision));
+SavePasswordProgressLogger::StructuredLog::~StructuredLog() {
 }
 
-void SavePasswordProgressLogger::LogMessage(const std::string& message) {
-  LogValue("Message", StringValue(message));
+bool SavePasswordProgressLogger::StructuredLog::operator==(
+    const SavePasswordProgressLogger::StructuredLog& log) const {
+  if (log_type != log.log_type || label != log.label)
+    return false;
+  switch (log_type) {
+    case LOG_TYPE_LABEL:
+      return true;
+    case LOG_TYPE_GURL:
+      return url == log.url;
+    case LOG_TYPE_MESSAGE:
+      return message == log.message;
+    case LOG_TYPE_NUMBER:
+      return number == log.number;
+    case LOG_TYPE_BOOL:
+      return truth_value == log.truth_value;
+    case LOG_TYPE_ELEMENT_ID:
+      return element_id == log.element_id;
+  };
+  NOTREACHED();  // Win compilers don't believe this is unreachable.
+  return false;
 }
 
-void SavePasswordProgressLogger::LogValue(const std::string& name,
-                                          const Value& log) {
+bool SavePasswordProgressLogger::StructuredLog::operator!=(
+    const SavePasswordProgressLogger::StructuredLog& log) const {
+  return !operator==(log);
+}
+
+
+SavePasswordProgressLogger::SavePasswordProgressLogger() {}
+
+SavePasswordProgressLogger::~SavePasswordProgressLogger() {}
+
+// static
+std::string SavePasswordProgressLogger::SanitizeStructuredLogs(
+    const std::vector<SavePasswordProgressLogger::StructuredLog>& logs) {
+  // First, we sanitize the logs and transform them into a base::Value.
+  DictionaryValue log_dict;  // Always holds a single item: the log.
+  if (logs.size() == 1) {
+    log_dict.Set(kIDsToMessages[logs[0].label],
+                 GetValueFromStructuredLog(logs[0]).release());
+  } else {
+    if (logs.empty() || logs[0].log_type != StructuredLog::LOG_TYPE_LABEL) {
+      NOTREACHED();  // The logs vector looks damaged.
+      return std::string();
+    }
+    scoped_ptr<DictionaryValue> inner_dict(new DictionaryValue);
+    // Transform and add all but the first log into |inner_dict|.
+    for (size_t i = 1; i < logs.size(); ++i) {
+      inner_dict->Set(kIDsToMessages[logs[i].label],
+                      GetValueFromStructuredLog(logs[i]).release());
+    }
+    log_dict.Set(kIDsToMessages[logs[0].label], inner_dict.release());
+  }
+
+  // Now we convert the base::DictionaryValue to a string.
   std::string log_string;
   bool conversion_to_string_successful = base::JSONWriter::WriteWithOptions(
-      &log, base::JSONWriter::OPTIONS_PRETTY_PRINT, &log_string);
+      &log_dict, base::JSONWriter::OPTIONS_PRETTY_PRINT, &log_string);
   DCHECK(conversion_to_string_successful);
-  SendLog(name + ": " + log_string);
+  return log_string;
+}
+
+void SavePasswordProgressLogger::LogPasswordForm(
+    SavePasswordProgressLogger::StringID label,
+    const PasswordForm& form) {
+  std::vector<SavePasswordProgressLogger::StructuredLog> logs;
+  logs.reserve(15);
+  logs.push_back(StructuredLog(label));
+  logs.push_back(
+      StructuredLog(STRING_SCHEME_MESSAGE, FormSchemeToStringID(form.scheme)));
+  logs.push_back(StructuredLog(STRING_SIGNON_REALM, GURL(form.signon_realm)));
+  logs.push_back(StructuredLog(STRING_ORIGINAL_SIGNON_REALM,
+                               GURL(form.original_signon_realm)));
+  logs.push_back(StructuredLog(STRING_ORIGIN, form.origin));
+  logs.push_back(StructuredLog(STRING_ACTION, form.action));
+  logs.push_back(StructuredLog(STRING_USERNAME_ELEMENT, form.username_element));
+  logs.push_back(StructuredLog(STRING_PASSWORD_ELEMENT, form.password_element));
+  logs.push_back(StructuredLog(STRING_PASSWORD_AUTOCOMPLETE_SET,
+                               form.password_autocomplete_set));
+  logs.push_back(
+      StructuredLog(STRING_OLD_PASSWORD_ELEMENT, form.old_password_element));
+  logs.push_back(StructuredLog(STRING_SSL_VALID, form.ssl_valid));
+  logs.push_back(StructuredLog(STRING_PASSWORD_GENERATED,
+                               form.type == PasswordForm::TYPE_GENERATED));
+  logs.push_back(StructuredLog(STRING_TIMES_USED, form.times_used));
+  logs.push_back(StructuredLog(STRING_USE_ADDITIONAL_AUTHENTICATION,
+                               form.use_additional_authentication));
+  logs.push_back(StructuredLog(STRING_PSL_MATCH, form.IsPublicSuffixMatch()));
+  SendLog(logs);
+}
+
+void SavePasswordProgressLogger::LogHTMLForm(
+    SavePasswordProgressLogger::StringID label,
+    const std::string& name_or_id,
+    const std::string& method,
+    const GURL& action) {
+  std::vector<SavePasswordProgressLogger::StructuredLog> logs;
+  logs.reserve(4);
+  logs.push_back(StructuredLog(label));
+  logs.push_back(StructuredLog(STRING_NAME_OR_ID, name_or_id));
+  logs.push_back(StructuredLog(STRING_METHOD, FormMethodToStringID(method)));
+  logs.push_back(StructuredLog(STRING_ACTION, action));
+  SendLog(logs);
+}
+
+void SavePasswordProgressLogger::LogURL(
+    SavePasswordProgressLogger::StringID label,
+    const GURL& url) {
+  std::vector<SavePasswordProgressLogger::StructuredLog> logs;
+  logs.push_back(StructuredLog(label, url));
+  SendLog(logs);
+}
+
+void SavePasswordProgressLogger::LogBoolean(
+    SavePasswordProgressLogger::StringID label,
+    bool value) {
+  std::vector<SavePasswordProgressLogger::StructuredLog> logs;
+  logs.push_back(StructuredLog(label, value));
+  SendLog(logs);
+}
+
+void SavePasswordProgressLogger::LogNumber(
+    SavePasswordProgressLogger::StringID label,
+    int value) {
+  std::vector<SavePasswordProgressLogger::StructuredLog> logs;
+  logs.push_back(StructuredLog(label, value));
+  SendLog(logs);
+}
+
+void SavePasswordProgressLogger::LogNumber(
+    SavePasswordProgressLogger::StringID label,
+    size_t value) {
+  std::vector<SavePasswordProgressLogger::StructuredLog> logs;
+  logs.push_back(StructuredLog(label, checked_cast<int, size_t>(value)));
+  SendLog(logs);
+}
+
+void SavePasswordProgressLogger::LogMessage(
+    SavePasswordProgressLogger::StringID message) {
+  std::vector<SavePasswordProgressLogger::StructuredLog> logs;
+  logs.push_back(StructuredLog(STRING_MESSAGE, message));
+  SendLog(logs);
 }
 
 }  // namespace autofill