Migration from HTTP to HTTPS works only for forms with empty auth realm |
||||
Issue descriptionChrome Version: 70.0.3538.0 (Developer Build) (64-bit) OS: all Let’s assume that the user has in Password Store only one form. The form origin is “http://httpbin.org/" and signon_realm is http://httpbin.org/". It can be a PasswordForm::scheme HTML or non-HTML with empty realm. Then: 1. User visit a website that has an HTTP auth form. Suppose the website is https://httpbin.org/something. The form created have origin = “https://httpbin.org/something" . Let’s suppose that auth realm is “realm”, so the signon_realm will be “https://httpbin.org/realm”. 2. FormFetcherImpl requests credentials for https. 3. None are returned. HttpPasswordStoreMigrator is created to process the http credentials. It request credentials for PasswordForm::scheme = HTML, origin = “http://httpbin.org/something”, signon_realm = “http://httpbin.org/”. However, signon_realm should be “http://httpbin.org/realm”. 4. Credentials are found and they are copied (HSTS not enabled). New credentials have : origin = “https://httpbin.org/something" signon_realm = “https://httpbin.org/”, PasswordForm::scheme the same as http credentials. 5. If http credentials from Password Store is HTML then https credentials doesn’t match because of different PasswordForm::scheme (PasswordFormManager::IsMatch). Even if the http credentials from Password Store has SCHEME_BASIC (the same as submitted form), the migrated https credentials doesn’t match because of different signon_realm (LoginModelObserver::OnAutofillDataAvailable): current signon_realm = “https://httpbin.org/realm” migrated HTTPS form has signon_realm = “https://httpbin.org/”. 6. User visit https://httpbin.org/something again, steps 2-5 repeat. Moreover, if password manager state is not inactive (ManagePasswordsState::ProcessLoginsChanged), infinite password store requests reported in crbug.com/852356 is not solved for the scenario described above. Expected results: Credentials with non-empty realm should be migrated successfully. What happens instead: Wrong credentials are migrated without matching.
,
Aug 31
,
Sep 5
,
Sep 11
TL;DR: HttpPasswordStoreMigrator::HttpPasswordStoreMigrator has no idea that what the correct realm should be and hence always passes the origin (without) path in the FormDigest to PasswordStore::GetLogins. Instead, the HttpPasswordStoreMigrator should get the signon_realm from the callsites.
,
Sep 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a6758f34c67c866c599cad37833f6162c9863b4d commit a6758f34c67c866c599cad37833f6162c9863b4d Author: Gemene Narcis <gemene@google.com> Date: Fri Sep 28 14:17:55 2018 Clean Obsolete HTTP Credentials from the Password Store This change is supposed to delete obsolete HTTP data from the password store 60 seconds after start up. This CL will delete HTTP credentials with HSTS enabled for that host and for which an equivalent (i.e. same signon_realm, PasswordForm::Scheme,username and password) HTTPS credential exists. Also it replaces HTTP credentials by an HTTPS version of that form if site has HSTS enabled and no HTTPS credential with same signon_realm, same PasswordForm::Scheme and username exists. Bug: 871140, 879244 Change-Id: I605e39f25ce1052aa67d95cf84601a9db94c33a6 Reviewed-on: https://chromium-review.googlesource.com/1178282 Reviewed-by: Vaclav Brozek <vabr@chromium.org> Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org> Commit-Queue: Narcis Gemene <gemene@google.com> Cr-Commit-Position: refs/heads/master@{#595078} [modify] https://crrev.com/a6758f34c67c866c599cad37833f6162c9863b4d/components/password_manager/core/browser/http_credentials_cleaner.cc [modify] https://crrev.com/a6758f34c67c866c599cad37833f6162c9863b4d/components/password_manager/core/browser/http_credentials_cleaner.h [modify] https://crrev.com/a6758f34c67c866c599cad37833f6162c9863b4d/components/password_manager/core/browser/http_credentials_cleaner_unittest.cc [modify] https://crrev.com/a6758f34c67c866c599cad37833f6162c9863b4d/components/password_manager/core/browser/http_password_store_migrator.cc [modify] https://crrev.com/a6758f34c67c866c599cad37833f6162c9863b4d/components/password_manager/core/browser/http_password_store_migrator.h [modify] https://crrev.com/a6758f34c67c866c599cad37833f6162c9863b4d/components/password_manager/core/browser/http_password_store_migrator_unittest.cc [modify] https://crrev.com/a6758f34c67c866c599cad37833f6162c9863b4d/components/password_manager/core/browser/password_manager.cc [modify] https://crrev.com/a6758f34c67c866c599cad37833f6162c9863b4d/components/password_manager/core/browser/password_manager_util.cc [modify] https://crrev.com/a6758f34c67c866c599cad37833f6162c9863b4d/components/password_manager/core/browser/password_manager_util.h [modify] https://crrev.com/a6758f34c67c866c599cad37833f6162c9863b4d/components/password_manager/core/common/password_manager_pref_names.cc [modify] https://crrev.com/a6758f34c67c866c599cad37833f6162c9863b4d/components/password_manager/core/common/password_manager_pref_names.h |
||||
►
Sign in to add a comment |
||||
Comment 1 by dtapu...@chromium.org
, Aug 30