[Password Generation] Crowdsource noisified password length to adjust password...

[Password Generation] Crowdsource noisified password length to adjust password generator settings to sites' requirements.

This CL implements crowdsourcing noisified password length. The length is distorted in the following way:
- do report the true length L in 20% of cases.
- otherwise, do report a random value from the range [1, L-1]

Bug: 849243
Change-Id: I77d622bf44b6e4ffe60e5cc2fb5bf2b5c9164532
Reviewed-on: https://chromium-review.googlesource.com/1113443
Commit-Queue: Maxim Kolosovskiy <kolos@chromium.org>
Reviewed-by: Vaclav Brozek <vabr@chromium.org>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570363}

components/autofill/core/browser/form_structure.cc

@@ -307,26 +307,30 @@ bool AllTypesCaptured(const FormStructure& form,
   return true;
 }
 
-// Encode password attributes |vote| into |upload|.
+// Encode password attributes and length into |upload|.
 void EncodePasswordAttributesVote(
-    const std::pair<PasswordAttribute, bool>& vote,
+    const std::pair<PasswordAttribute, bool>& password_attributes_vote,
+    const size_t password_length_vote,
     AutofillUploadContents* upload) {
-  switch (vote.first) {
+  switch (password_attributes_vote.first) {
     case PasswordAttribute::kHasLowercaseLetter:
-      upload->set_password_has_lowercase_letter(vote.second);
+      upload->set_password_has_lowercase_letter(
+          password_attributes_vote.second);
       break;
     case PasswordAttribute::kHasUppercaseLetter:
-      upload->set_password_has_uppercase_letter(vote.second);
+      upload->set_password_has_uppercase_letter(
+          password_attributes_vote.second);
       break;
     case PasswordAttribute::kHasNumeric:
-      upload->set_password_has_numeric(vote.second);
+      upload->set_password_has_numeric(password_attributes_vote.second);
       break;
     case PasswordAttribute::kHasSpecialSymbol:
-      upload->set_password_has_special_symbol(vote.second);
+      upload->set_password_has_special_symbol(password_attributes_vote.second);
       break;
     case PasswordAttribute::kPasswordAttributesCount:
       NOTREACHED();
   }
+  upload->set_password_length(password_length_vote);
 }
 
 }  // namespace
@@ -448,8 +452,10 @@ bool FormStructure::EncodeUploadRequest(
   upload->set_autofill_used(form_was_autofilled);
   upload->set_data_present(EncodeFieldTypes(available_field_types));
   upload->set_passwords_revealed(passwords_were_revealed_);
-  if (password_attributes_vote_)
-    EncodePasswordAttributesVote(*password_attributes_vote_, upload);
+  if (password_attributes_vote_) {
+    EncodePasswordAttributesVote(*password_attributes_vote_,
+                                 password_length_vote_, upload);
+  }
 
   if (IsAutofillFieldMetadataEnabled()) {
     upload->set_action_signature(StrToHash64Bit(target_url_.host()));

components/autofill/core/browser/form_structure.h

@@ -271,6 +271,21 @@ class FormStructure {
   }
 #endif
 
+  void set_password_length_vote(const size_t noisified_password_length) {
+    DCHECK(password_attributes_vote_.has_value())
+        << "|password_length_vote_| doesn't make sense if "
+           "|password_attributes_vote_| has no value.";
+    password_length_vote_ = noisified_password_length;
+  }
+#if defined(UNIT_TEST)
+  size_t get_password_length_vote_for_testing() const {
+    DCHECK(password_attributes_vote_.has_value())
+        << "|password_length_vote_| doesn't make sense if "
+           "|password_attributes_vote_| has no value.";
+    return password_length_vote_;
+  }
+#endif
+
   bool operator==(const FormData& form) const;
   bool operator!=(const FormData& form) const;
 
@@ -417,6 +432,10 @@ class FormStructure {
   // character).
   base::Optional<std::pair<PasswordAttribute, bool>> password_attributes_vote_;
 
+  // Noisified password length for crowdsourcing. If |password_attributes_vote_|
+  // has no value, |password_length_vote_| should be ignored.
+  size_t password_length_vote_;
+
   DISALLOW_COPY_AND_ASSIGN(FormStructure);
 };
 

components/autofill/core/browser/form_structure_unittest.cc

@@ -2429,6 +2429,7 @@ TEST_F(FormStructureTest, EncodeUploadRequest) {
   form_structure.reset(new FormStructure(form));
   form_structure->set_password_attributes_vote(
       std::make_pair(PasswordAttribute::kHasNumeric, true));
+  form_structure->set_password_length_vote(10u);
 
   ASSERT_EQ(form_structure->field_count(), possible_field_types.size());
   for (size_t i = 0; i < form_structure->field_count(); ++i)
@@ -2454,6 +2455,7 @@ TEST_F(FormStructureTest, EncodeUploadRequest) {
   upload.set_data_present("144200030e");
   upload.set_passwords_revealed(false);
   upload.set_password_has_numeric(true);
+  upload.set_password_length(10u);
   upload.set_action_signature(15724779818122431245U);
 
   test::FillUploadField(upload.add_field(), 3763331450U, "firstname", "text",
@@ -2505,6 +2507,7 @@ TEST_F(FormStructureTest, EncodeUploadRequest) {
   form_structure.reset(new FormStructure(form));
   form_structure->set_password_attributes_vote(
       std::make_pair(PasswordAttribute::kHasNumeric, true));
+  form_structure->set_password_length_vote(10u);
   ASSERT_EQ(form_structure->field_count(), possible_field_types.size());
   for (size_t i = 0; i < form_structure->field_count(); ++i)
     form_structure->field(i)->set_possible_types(possible_field_types[i]);

components/autofill/core/browser/proto/server.proto

@@ -52,7 +52,7 @@ message AutofillQueryResponseContents {
 
 // This message contains information about the field types in a single form.
 // It is sent by the toolbar to contribute to the field type statistics.
-// Next available id: 29
+// Next available id: 30
 message AutofillUploadContents {
   required string client_version = 1;
   required fixed64 form_signature = 2;
@@ -168,8 +168,10 @@ message AutofillUploadContents {
   // True iff the the non-obfuscated password values were shown to the user.
   optional bool passwords_revealed = 24;
 
-  // The section of noisified data about password attributes.
-  // Upload only one attribute and only when a password is saved first time.
+  // The section of noisified data about password.
+  // Upload only one of character class attributes (|password_has_*|). Noisified
+  // length is always uploaded.
+  // Upload only when a password is saved.
   // Used to adjust the password generator's settings to site's requirements.
 
   // Whether the password has any lowercase letter.
@@ -183,5 +185,8 @@ message AutofillUploadContents {
 
   // Whether the password has any special symbol.
   optional bool password_has_special_symbol = 28;
+
+  // Noisifed password length.
+  optional uint32 password_length = 29;
   // The end of the section of password attributes.
 }

components/password_manager/core/browser/password_form_manager_unittest.cc

@@ -164,6 +164,18 @@ MATCHER_P(PasswordsWereRevealed, revealed, "") {
   return arg.passwords_were_revealed() == revealed;
 }
 
+MATCHER_P(HasPasswordAttributesVote, is_vote_expected, "") {
+  base::Optional<std::pair<autofill::PasswordAttribute, bool>> vote =
+      arg.get_password_attributes_vote_for_testing();
+  EXPECT_EQ(is_vote_expected, vote.has_value());
+  if (vote.has_value()) {
+    size_t reported_length = arg.get_password_length_vote_for_testing();
+    EXPECT_LT(0u, reported_length);
+    EXPECT_LE(reported_length, 5u /* actual password length */);
+  }
+  return true;
+}
+
 // Matches iff the masks in |expected_field_properties| match the mask in the
 // uploaded form exactly.
 MATCHER_P(UploadedFieldPropertiesMasksAre, expected_field_properties, "") {

components/password_manager/core/browser/vote_uploads_test_matchers.h

@@ -163,11 +163,4 @@ MATCHER_P2(UploadedFormClassifierVoteIs,
   return true;
 }
 
-MATCHER_P(HasPasswordAttributesVote, is_vote_expected, "") {
-  base::Optional<std::pair<autofill::PasswordAttribute, bool>> vote =
-      arg.get_password_attributes_vote_for_testing();
-  EXPECT_EQ(is_vote_expected, vote.has_value());
-  return true;
-}
-
 #endif  // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_VOTE_UPLOADS_TEST_MATCHERS_H_

components/password_manager/core/browser/votes_uploader.cc

@@ -498,35 +498,44 @@ bool VotesUploader::FindCorrectedUsernameElement(
 void VotesUploader::GeneratePasswordAttributesVote(
     const base::string16& password_value,
     FormStructure* form_structure) {
-  // Select a password attribute to upload. Do upload symbols more often as
-  // 2/3rd of issues are because of missing special symbols.
+  // Select a character class attribute to upload.
   int bucket = base::RandGenerator(9);
   int (*predicate)(int c) = nullptr;
-  autofill::PasswordAttribute attribute =
+  autofill::PasswordAttribute character_class_attribute =
       autofill::PasswordAttribute::kHasSpecialSymbol;
   if (bucket == 0) {
     predicate = &islower;
-    attribute = autofill::PasswordAttribute::kHasLowercaseLetter;
+    character_class_attribute =
+        autofill::PasswordAttribute::kHasLowercaseLetter;
   } else if (bucket == 1) {
     predicate = &isupper;
-    attribute = autofill::PasswordAttribute::kHasUppercaseLetter;
+    character_class_attribute =
+        autofill::PasswordAttribute::kHasUppercaseLetter;
   } else if (bucket == 2) {
     predicate = &isdigit;
-    attribute = autofill::PasswordAttribute::kHasNumeric;
+    character_class_attribute = autofill::PasswordAttribute::kHasNumeric;
   } else {  //  3 <= bucket < 9
+    // Upload symbols more often as 2/3rd of issues are because of missing
+    // special symbols.
     predicate = &ispunct;
-    attribute = autofill::PasswordAttribute::kHasSpecialSymbol;
+    character_class_attribute = autofill::PasswordAttribute::kHasSpecialSymbol;
   }
-  bool actual_value =
+  bool actual_value_for_character_class =
       std::any_of(password_value.begin(), password_value.end(), predicate);
 
   // Apply the randomized response technique to noisify the actual value
   // (https://en.wikipedia.org/wiki/Randomized_response).
-  bool randomized_value =
-      base::RandGenerator(2) ? actual_value : base::RandGenerator(2);
-
-  form_structure->set_password_attributes_vote(
-      std::make_pair(attribute, randomized_value));
+  bool randomized_value_for_character_class =
+      base::RandGenerator(2) ? actual_value_for_character_class
+                             : base::RandGenerator(2);
+  form_structure->set_password_attributes_vote(std::make_pair(
+      character_class_attribute, randomized_value_for_character_class));
+
+  size_t actual_length = password_value.size();
+  size_t randomized_length = base::RandGenerator(5) == 0
+                                 ? actual_length
+                                 : base::RandGenerator(actual_length - 1) + 1;
+  form_structure->set_password_length_vote(randomized_length);
 }
 
 }  // namespace password_manager

components/password_manager/core/browser/votes_uploader_unittest.cc

@@ -199,6 +199,9 @@ TEST_F(VotesUploaderTest, GeneratePasswordAttributesVote) {
     int reported_false[kNumberOfPasswordAttributes] = {0, 0, 0, 0};
     int reported_true[kNumberOfPasswordAttributes] = {0, 0, 0, 0};
 
+    int reported_actual_length = 0;
+    int reported_wrong_length = 0;
+
     for (int i = 0; i < 1000; ++i) {
       votes_uploader.GeneratePasswordAttributesVote(password_value,
                                                     &form_structure);
@@ -209,6 +212,15 @@ TEST_F(VotesUploaderTest, GeneratePasswordAttributesVote) {
         reported_true[attribute_index]++;
       else
         reported_false[attribute_index]++;
+      size_t reported_length =
+          form_structure.get_password_length_vote_for_testing();
+      if (reported_length == password_value.size()) {
+        reported_actual_length++;
+      } else {
+        reported_wrong_length++;
+        EXPECT_LT(0u, reported_length);
+        EXPECT_LT(reported_length, password_value.size());
+      }
     }
     for (int i = 0; i < kNumberOfPasswordAttributes; i++) {
       EXPECT_LT(0, reported_false[i]);
@@ -226,6 +238,9 @@ TEST_F(VotesUploaderTest, GeneratePasswordAttributesVote) {
             << ". password_value = " << password_value;
       }
     }
+    EXPECT_LT(0, reported_actual_length);
+    EXPECT_LT(0, reported_wrong_length);
+    EXPECT_LT(reported_actual_length, reported_wrong_length);
   }
 }