Thanks for the report. Unfortunately, without a concrete reproduction case, it's difficult to know exactly what's going on. +some password/sync folks

Assigning to tschumann@ for triage, +battre@ FYI.

I imagine there's detail you could pull out of chrome://sync-internals to help diagnose the issue, but I don't know enough about that mechanism to point you to anything specific.

Do you use Android? Was any of those passwords related to the Android apps you use?

Marking this security_severity-low since it doesn't appear data is leaked outside the browser. There could be security implications on a shared device, though the user normally wouldn't be syncing in that case.

assigning to zea@ -- can you add somebody from the old sync team we can partner up with here? 

Adding our current fixer Pavel.

konklone@: Could you give me permission to look at server data associated with your account along with approximate timeframe when you deleted the passwords. I'll try to track down which client created them. Also let me know which account you are signed-in with? Is it konklone@?

Pavel - Yes, you have my permission. 

I unfortunately don't have good estimates of timing of when I noticed previous passwords had been restored. 18f.slack.com can be used as a good example of one that I am confident has been restored before.

I did conduct a delete operation during the authoring of this bug, for that and a few other slack.com subdomains, and I'm checking back on a regular cadence to see whether they reappear (nothing yet).

This issue is readily reproducible on my Gmail, and I'd be happy to volunteer my account so you can look at logs or do some troubleshooting.  Let me know how I can help move this forward.

Here's what happens. Half a year ago there was a bug ( https://crbug.com/739101 ) discovered in Android. It just released Android Autofill and the credentials were saved internally in the following format:

origin_url: android://u0A-O7Ivuokjnqmf1ciagyiwkxqsLSrXA9ZzyIb4yGEVu5tPRfhJbn8REHGmDAUkUCGf71TqwUcwSfwFObRssA==@com.twitter.android
signon_realm: android://u0A-O7Ivuokjnqmf1ciagyiwkxqsLSrXA9ZzyIb4yGEVu5tPRfhJbn8REHGmDAUkUCGf71TqwUcwSfwFObRssA==@com.twitter.android

The correct one is
origin_url: android://u0A-O7Ivuokjnqmf1ciagyiwkxqsLSrXA9ZzyIb4yGEVu5tPRfhJbn8REHGmDAUkUCGf71TqwUcwSfwFObRssA==@com.twitter.android/
signon_realm: android://u0A-O7Ivuokjnqmf1ciagyiwkxqsLSrXA9ZzyIb4yGEVu5tPRfhJbn8REHGmDAUkUCGf71TqwUcwSfwFObRssA==@com.twitter.android/

As the result the former credentials were not autofilled in Chrome. I fixed it in Chrome Password Sync in a way that Chrome creates a copy of the wrong credentials and saves it locally in the correct format only. The server now has two copies but the users see only one.

Back to nowadays. This is how the current bug is reproduced.
- There is a Chrome instance and Android instance (e.g. Google pixel) running simultaneously.
- Android Autofill (GmsCore v15) saves a credential like
 origin_url: android://u0A-O7Ivuokjnqmf1ciagyiwkxqsLSrXA9ZzyIb4yGEVu5tPRfhJbn8REHGmDAUkUCGf71TqwUcwSfwFObRssA==@com.twitter.android
 signon_realm: android://u0A-O7Ivuokjnqmf1ciagyiwkxqsLSrXA9ZzyIb4yGEVu5tPRfhJbn8REHGmDAUkUCGf71TqwUcwSfwFObRssA==@com.twitter.android/
 I don't know how/why but it's a bug. We reproduced it in different apps.
- PasswordSyncableService::ProcessSyncChanges in Chrome gets the new password and corrects it to android://u0A-O7Ivuokjnqmf1ciagyiwkxqsLSrXA9ZzyIb4yGEVu5tPRfhJbn8REHGmDAUkUCGf71TqwUcwSfwFObRssA==@com.twitter.android/ for both fields and saves locally (unexpected but correct).
- Chrome restarts. PasswordSyncableService::MergeSyncDataWithLocalData doesn't expect the wrong format (origin without '/') and saves it locally. Now Chrome has 2 credentials like the server side data.
- At some point a deletion on p.g.c. happens and 2 deletions are dispatched to Chrome. ProcessSyncChanges knows that there is only one correct format and deletes only it. The wrong credential stays in Chrome.
- Chrome restarts again. PasswordSyncableService::MergeSyncDataWithLocalData picks the wrong credential and uploads it to the server. The credential is back available to the app :(

If the credential is deleted via Chrome then it's removed correctly from all the devices. Basically, it's a workaround for now.

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

commit e2e467d19e84f8acd20a730203a1e495a7e2a012
Author: Vasilii Sukhanov <vasilii@chromium.org>
Date: Fri Feb 02 16:49:41 2018

Revert special handling of Android credentials in Chrome Sync.

It was introduced in r488421 to handle the credentials without trailing '/' in signon_realm and origin.
Now however, the signon_realm is created correctly by GmsCore. The origin has no '/'. As a result
- r488421 is useless with a lot of complexity.
- r488421 is harmful because it prevent the credential from being deleted via passwords.google.com. See b/69245513

R=battre@chromium.org

Bug:  739101 , 801918 
Change-Id: I20e64481b8dd972e3b43f18a00b0b3078f58c4fe
Reviewed-on: https://chromium-review.googlesource.com/897610
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534073}
[modify] https://crrev.com/e2e467d19e84f8acd20a730203a1e495a7e2a012/components/password_manager/core/browser/password_syncable_service.cc
[modify] https://crrev.com/e2e467d19e84f8acd20a730203a1e495a7e2a012/components/password_manager/core/browser/password_syncable_service.h
[modify] https://crrev.com/e2e467d19e84f8acd20a730203a1e495a7e2a012/components/password_manager/core/browser/password_syncable_service_unittest.cc

Vasilii, looking at comment #21, it appears that the patch is not needed anymore because the issue is fixed in GMSCore. Do we have numbers about how many of the buggy GMSCore clients are still out there? [I'm not familiar with the update process of GMSCore].

There is a dashboard (go/gmscore-board) but I don't have access to it. One needs to be a member of the Android team. I think this week you can easily catch someone in the office in person.

I want to merge r534073 back. It removes a lot of complexity and fixes a privacy issue.

This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

+ awhalley@ for M65 merge review. 

From a security point of view, this is a nice to have. I imagine this shouldn't be too risky given it's reverting to previous shipping behaviour.

Thank you  awhalley@. 
vasilii@, how safe is to merge r534073 to M65?

It's the implementation that we had for years for the passwords sync. Assuming that the actual revert is correctly applied it's safe. We can wait for a couple of days more of course to see if any bug is reported.

Ok, pls update on Thursday, 02/08/18 with canary result. Thank you.

The NextAction date has arrived: 2018-02-08

Let's merge it. I think it's safe.

Approving merge to M65 branch 3325 based on comments #27, #30 and #33. Please merge ASAP. Thank you.

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

commit 7a576874a67509c0e15707408807efc213cb3e13
Author: Vasilii Sukhanov <vasilii@chromium.org>
Date: Thu Feb 08 19:22:01 2018

Revert special handling of Android credentials in Chrome Sync.

It was introduced in r488421 to handle the credentials without trailing '/' in signon_realm and origin.
Now however, the signon_realm is created correctly by GmsCore. The origin has no '/'. As a result
- r488421 is useless with a lot of complexity.
- r488421 is harmful because it prevent the credential from being deleted via passwords.google.com. See b/69245513

R=battre@chromium.org
TBR=vasilii@chromium.org

(cherry picked from commit e2e467d19e84f8acd20a730203a1e495a7e2a012)

Bug:  739101 , 801918 
Change-Id: I20e64481b8dd972e3b43f18a00b0b3078f58c4fe
Reviewed-on: https://chromium-review.googlesource.com/897610
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#534073}
Reviewed-on: https://chromium-review.googlesource.com/909391
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#383}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/7a576874a67509c0e15707408807efc213cb3e13/components/password_manager/core/browser/password_syncable_service.cc
[modify] https://crrev.com/7a576874a67509c0e15707408807efc213cb3e13/components/password_manager/core/browser/password_syncable_service.h
[modify] https://crrev.com/7a576874a67509c0e15707408807efc213cb3e13/components/password_manager/core/browser/password_syncable_service_unittest.cc

The Chrome VRP panel looked at this and decided to track this as a functional bug. Thanks for the report!

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot