The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59868dfdfdcb9d3c0373717c3eb91c27c7694f95

commit 59868dfdfdcb9d3c0373717c3eb91c27c7694f95
Author: jdoerrie <jdoerrie@chromium.org>
Date: Fri Jul 20 08:04:47 2018

[Pwd Settings] Improve Testing Coverage

This change improves the testing coverage of the password section
browsertest by adding more test cases and increasing the number of
checked expectations.

Bug:  865458 
Change-Id: Ibcfad5d5a8556bddc3b928b3a6898585cf20031f
Reviewed-on: https://chromium-review.googlesource.com/1143468
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576807}
[modify] https://crrev.com/59868dfdfdcb9d3c0373717c3eb91c27c7694f95/chrome/test/data/webui/settings/settings_passwords_section_browsertest.js

Here are some thoughts on how we can improve the robustness of using an identifier to refer to specific passwords when communicating deletion and show password events between the backend and frontend:

In the current implementation the backend partitions the list of credentials into equivalence classes and sends one member of each class to the frontend for display to the user. Two credentials are deemed equivalent when they have the same key determined by CreateSortKey [1]. While most equivalence classes should be of size exactly one, it is possible that multiple credentials have the same sort key due to legacy data in the password store. Thus there is a bijection between sort keys and list elements in the UI and I believe it makes sense to derive the identifier from this key. In the most simple implementation the complete sort key could be the identifier, but there are a few problems with this approach:

- The length of the sort key is unbounded, and duplicates a lot of data that is sent to the frontend already.
- More severely, the sort key contains the actual user password. For security reasons we currently avoid sending all the passwords to the UI upfront, and only send passwords on a on-demand basis after a successful OS level authentication. Having the password part of the sent identifier would circumvent this protection.

A second option is to send a hash of the sort key instead, but this has problems as well:
- There is a non-zero chance of hash collisions, which can result in deleting the wrong password and thus unintended data loss.
- While a bit harder, a determined attacker could still recover the password from the hashed sort key, by performing an offline brute-force attack.

Because of the flaws in the just mentioned approaches, I suggest using an ever increasing counter to identify a given sort key. This requires additional bookkeeping in the backend, but avoids the problem with leaking information about the password. It would work in the following way: On initial page-load each of the N sort-keys gets assigned a distinct number between 0 and N-1. Note that this part is quite similar to the currently used index approach. However, a given sort key's identifier is stable, as long as there is at least one credential with this entry. This means if a password and the corresponding sort key are deleted, the identifiers of the other sort keys remain unaffected. If a new sort key is added due to adding a credential to the password store, this sort key will be assigned a new number, which will be larger than any other used number before. In the most simple implementation this is a counter that will be incremented whenever a new sort key appears. Using a new number avoids the currently existing id re-use, and guards against the case when backend and frontend become out of sync. As identifiers are never re-used, there is no risk of unintentional data-loss if the frontend sends a stale identifier.

However, I'm also curious to hear what other people think about this. dpapad@ mentioned to me that he has given this problem some thought as well.

[1] https://codesearch.chromium.org/chromium/src/components/password_manager/core/browser/password_list_sorter.h?l=30&rcl=d8a60e779be20114b5de192bdf9685f5db2e5d9d

+vabr who I just chatted with regarding the proposal in #c2.

vabr going hobby only -> reducing involvement.
Please contact me directly in urgent matters.

jdoerrie@, it looks like this is already done. Should it be merged with  https://crbug.com/892260 ?

I should probably link where this was done. https://chromium-review.googlesource.com/c/chromium/src/+/1273717/8/chrome/browser/extensions/api/passwords_private/passwords_private_utils.cc

Ah yes, the CLs in  https://crbug.com/892260  implemented the logic suggested in #c2, so this can be closed. Thanks for pointing that out!