[Passwords, Mac] Don't always prompt to access passwords saved by other browsers

chrome/browser/password_manager/password_store_mac.cc

Index: chrome/browser/password_manager/password_store_mac.cc
diff --git a/chrome/browser/password_manager/password_store_mac.cc b/chrome/browser/password_manager/password_store_mac.cc
index 74456e0a14452e354e109009a7b5377a6f7019ef..f469db2aaab82acf3d0538240a01feb190872738 100644
--- a/chrome/browser/password_manager/password_store_mac.cc
+++ b/chrome/browser/password_manager/password_store_mac.cc
@@ -17,8 +17,10 @@
 #include "base/message_loop/message_loop.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
+#include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/mac/security_wrappers.h"
 #include "chrome/browser/password_manager/login_database.h"
 #include "chrome/browser/password_manager/password_store_change.h"
 #include "content/public/browser/notification_service.h"
@@ -231,11 +233,82 @@ PasswordForm::Scheme SchemeForAuthType(SecAuthenticationType auth_type) {
   }
 }
 
-bool FillPasswordFormFromKeychainItem(const AppleKeychain& keychain,
-                                      const SecKeychainItemRef& keychain_item,
-                                      PasswordForm* form) {
+// Wraps SecTrustedApplicationCopyData, returning NULL on error and a CFDataRef
+// owned by the caller on success.
+CFDataRef CrSTrustedApplicationCopyData(SecTrustedApplicationRef application) {
+  CFDataRef data;
+  OSStatus status = SecTrustedApplicationCopyData(application, &data);
+  if (status != errSecSuccess) {
+    OSSTATUS_LOG(ERROR, status);
+    return NULL;
+  }
+
+  return data;
+}
+
+// Returns true if accessing |keychain_item| might result in a user-visible
+// authorization prompt.  The implementation scans over the list of authorized
+// apps in the ACL, and returns false iff the current app is foudn in the list.
+// The implementation is conservative.
+bool MightDisplayAuthorizationPrompt(const SecKeychainItemRef& keychain_item) {
+  base::ScopedCFTypeRef<SecTrustedApplicationRef> chrome_app(
+      chrome::CrSTrustedApplicationCreateFromPath(NULL));
+  base::ScopedCFTypeRef<CFDataRef> chrome_data(
+      CrSTrustedApplicationCopyData(chrome_app));
+
+  base::ScopedCFTypeRef<SecAccessRef> access(
+      chrome::CrSKeychainItemCopyAccess(keychain_item));
+  if (!access)
+    return true;
+
+  base::ScopedCFTypeRef<CFArrayRef> acl_list(
+      chrome::CrSAccessCopyACLList(access));
+  if (!acl_list)
+    return true;
+
+  for (CFIndex i = 0; i < CFArrayGetCount(acl_list); ++i) {
+    SecACLRef acl = base::mac::CFCast<SecACLRef>(
+        CFArrayGetValueAtIndex(acl_list, i));
+    scoped_ptr<chrome::CrSACLSimpleContents> acl_simple_contents(
+        chrome::CrSACLCopySimpleContents(acl));
+    if (!acl_simple_contents || !acl_simple_contents->application_list)
+      continue;
+
+    for (CFIndex j = 0;
+         j < CFArrayGetCount(acl_simple_contents->application_list);
+         ++j) {
+      SecTrustedApplicationRef application =
+          base::mac::CFCast<SecTrustedApplicationRef>(
+              CFArrayGetValueAtIndex(acl_simple_contents->application_list, j));
+      base::ScopedCFTypeRef<CFDataRef> application_data(
+          CrSTrustedApplicationCopyData(application));
+
+      CFRange full_range = CFRangeMake(0, CFDataGetLength(application_data));
+      CFRange result = CFDataFind(
+          application_data, chrome_data, full_range, kCFDataSearchAnchored);
+      if (result.location == full_range.location &&
+          result.length == full_range.length) {
+        return false;
+      }
+    }
+  }
+
+  return true;
+}
+
+bool FillPasswordFormFromKeychainItem(
+    const AppleKeychain& keychain,
+    const SecKeychainItemRef& keychain_item,
+    PasswordStore::AuthorizationPromptPermission permission_to_prompt,
+    PasswordForm* form) {
   DCHECK(form);
 
+  // In silent mode, don't attempt to read any keychain items that might
+  // interrupt the user with an authorization prompt.  http://crbug.com/178358
+  if (permission_to_prompt == PasswordStore::DISALLOW_PROMPT &&
+      MightDisplayAuthorizationPrompt(keychain_item))
+    return false;
+
   SecKeychainAttributeInfo attrInfo;
   UInt32 tags[] = { kSecAccountItemAttr,
                     kSecServerItemAttr,
@@ -476,12 +549,13 @@ MacKeychainPasswordFormAdapter::MacKeychainPasswordFormAdapter(
 
 std::vector<PasswordForm*>
     MacKeychainPasswordFormAdapter::PasswordsFillingForm(
-        const PasswordForm& query_form) {
+        const PasswordForm& query_form,
+        PasswordStore::AuthorizationPromptPermission permission_to_prompt) {
   std::vector<SecKeychainItemRef> keychain_items =
       MatchingKeychainItems(query_form.signon_realm, query_form.scheme,
                             NULL, NULL);
 
-  return ConvertKeychainItemsToForms(&keychain_items);
+  return ConvertKeychainItemsToForms(&keychain_items, permission_to_prompt);
 }
 
 std::vector<PasswordForm*>
@@ -492,17 +566,22 @@ std::vector<PasswordForm*>
       MatchingKeychainItems(query_form.signon_realm, query_form.scheme,
                             NULL, username.c_str());
 
-  return ConvertKeychainItemsToForms(&keychain_items);
+  // TODO(isherman): Pass a real value.

[stuartmorgan, 2013/08/29 20:39:54]

Is this a long-term TODO, or something to fix now?

+  return ConvertKeychainItemsToForms(
+      &keychain_items, PasswordStore::ALLOW_PROMPT);
 }
 
 PasswordForm* MacKeychainPasswordFormAdapter::PasswordExactlyMatchingForm(
-    const PasswordForm& query_form) {
+    const PasswordForm& query_form,
+    PasswordStore::AuthorizationPromptPermission permission_to_prompt) {
   SecKeychainItemRef keychain_item = KeychainItemForForm(query_form);
   if (keychain_item) {
     PasswordForm* form = new PasswordForm();
-    internal_keychain_helpers::FillPasswordFormFromKeychainItem(*keychain_,
-                                                                keychain_item,
-                                                                form);
+    internal_keychain_helpers::FillPasswordFormFromKeychainItem(
+        *keychain_,
+        keychain_item,
+        permission_to_prompt,
+        form);
     keychain_->Free(keychain_item);
     return form;
   }
@@ -539,7 +618,8 @@ std::vector<PasswordForm*>
     keychain_search.FindMatchingItems(&matches);
   }
 
-  return ConvertKeychainItemsToForms(&matches);
+  // TODO(isherman): Pass a real value.
+  return ConvertKeychainItemsToForms(&matches, PasswordStore::ALLOW_PROMPT);
 }
 
 bool MacKeychainPasswordFormAdapter::AddPassword(const PasswordForm& form) {
@@ -602,13 +682,14 @@ void MacKeychainPasswordFormAdapter::SetFindsOnlyOwnedItems(
 
 std::vector<PasswordForm*>
     MacKeychainPasswordFormAdapter::ConvertKeychainItemsToForms(
-        std::vector<SecKeychainItemRef>* items) {
+        std::vector<SecKeychainItemRef>* items,
+        PasswordStore::AuthorizationPromptPermission permission_to_prompt) {
   std::vector<PasswordForm*> keychain_forms;
   for (std::vector<SecKeychainItemRef>::const_iterator i = items->begin();
        i != items->end(); ++i) {
     PasswordForm* form = new PasswordForm();
-    if (internal_keychain_helpers::FillPasswordFormFromKeychainItem(*keychain_,
-                                                                    *i, form)) {
+    if (internal_keychain_helpers::FillPasswordFormFromKeychainItem(
+            *keychain_, *i, permission_to_prompt, form)) {
       keychain_forms.push_back(form);
     }
     keychain_->Free(*i);
@@ -838,7 +919,7 @@ void PasswordStoreMac::RemoveLoginImpl(const PasswordForm& form) {
     MacKeychainPasswordFormAdapter owned_keychain_adapter(keychain_.get());
     owned_keychain_adapter.SetFindsOnlyOwnedItems(true);
     PasswordForm* owned_password_form =
-        owned_keychain_adapter.PasswordExactlyMatchingForm(form);
+        owned_keychain_adapter.PasswordExactlyMatchingForm(form, ALLOW_PROMPT);
     if (owned_password_form) {
       // If we don't have other forms using it (i.e., a form differing only by
       // the names of the form elements), delete the keychain entry.
@@ -893,10 +974,11 @@ void PasswordStoreMac::RemoveLoginsCreatedBetweenImpl(
 
 void PasswordStoreMac::GetLoginsImpl(
     const content::PasswordForm& form,
-    const ConsumerCallbackRunner& callback_runner) {
+    const ConsumerCallbackRunner& callback_runner,
+    AuthorizationPromptPermission permission_to_prompt) {
   MacKeychainPasswordFormAdapter keychain_adapter(keychain_.get());
   std::vector<PasswordForm*> keychain_forms =
-      keychain_adapter.PasswordsFillingForm(form);
+      keychain_adapter.PasswordsFillingForm(form, permission_to_prompt);
 
   std::vector<PasswordForm*> database_forms;
   login_metadata_db_->GetLogins(form, &database_forms);