New issue
Advanced search Search tips

Issue 595468 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocking:
issue 592663



Sign in to add a comment

Stop using deprecated Keychain methods in password_store_mac.cc

Project Member Reported by erikc...@chromium.org, Mar 16 2016

Issue description

The methods have been deprecated since OS X 10.7. The replacement is SecItemCopyMatching, looks functional, and has been available since OS X 10.6. 

In the short term, I'm going to add compile flags to suppress the warning. vasilii@, could you take a look at converting chrome/browser/password_manager/password_store_mac.cc?

../../crypto/apple_keychain_mac.mm:58:10: error: 'SecKeychainSearchCreateFromAttributes' is deprecated: first deprecated in OS X 10.7 [-Werror,-Wdeprecated-declarations]
  return SecKeychainSearchCreateFromAttributes(keychainOrArray, itemClass,
         ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/Security.framework/Headers/SecKeychainSearch.h:58:10: note: 'SecKeychainSearchCreateFromAttributes' has been explicitly marked deprecated here
OSStatus SecKeychainSearchCreateFromAttributes(CFTypeRef keychainOrArray, SecItemClass itemClass, const SecKeychainAttributeList *attrList, SecKeychainSearchRef *searchRef)
         ^
../../crypto/apple_keychain_mac.mm:65:10: error: 'SecKeychainSearchCopyNext' is deprecated: first deprecated in OS X 10.7 [-Werror,-Wdeprecated-declarations]
  return SecKeychainSearchCopyNext(searchRef, itemRef);
         ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/Security.framework/Headers/SecKeychainSearch.h:69:10: note: 'SecKeychainSearchCopyNext' has been explicitly marked deprecated here
OSStatus SecKeychainSearchCopyNext(SecKeychainSearchRef searchRef, SecKeychainItemRef *itemRef)
         ^
2 errors generated.
 
Summary: Stop using deprecated Keychain methods in password_store_mac.cc (was: Stop using deprecated Keychain methods)
Alternatively, SecKeychainFindInternetPassword would likely suffice here.
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 16 2016

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

commit c519490e5d3dff205429762976cfb07065e0a307
Author: erikchen <erikchen@chromium.org>
Date: Wed Mar 16 23:55:35 2016

Suppress -Wdeprecated-declarations warnings in apple_keychain_mac.mm.

This is required to update the deployment target to OS X 10.7. Eventually, the
deprecated Keychain methods should be removed entirely.

BUG= 595468 ,  592663 

Review URL: https://codereview.chromium.org/1813523002

Cr-Commit-Position: refs/heads/master@{#381593}

[modify] https://crrev.com/c519490e5d3dff205429762976cfb07065e0a307/crypto/apple_keychain_mac.mm

Are we targeting 10.7 now?
The current Canary only runs on 10.9 or higher.
What's requiring a conversion in password_store_mac.cc? I don't see any pragma ignored there.
apple_keychain_mac.mm is pretty much just a thin wrapper around some deprecated Apple keychain methods. password_store_mac.cc is the only consumer of these deprecated methods. e.g. 
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/password_manager/password_store_mac.cc&q=SearchCreateFromAttributes&sq=package:chromium&type=cs&l=169
Components: UI>Browser>Passwords

Comment 9 by vabr@chromium.org, Jul 18 2016

Labels: Hotlist-TechnicalDebt
[17/2773] OBJCXX obj/crypto/crypto/apple_keychain_mac.o
../../crypto/apple_keychain_mac.mm:61:10: warning: 'SecKeychainSearchCreateFromAttributes' is deprecated: first deprecated in macOS 10.7 [-Wdeprecated-declarations]
  return SecKeychainSearchCreateFromAttributes(keychainOrArray, itemClass,
         ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/Security.framework/Headers/SecKeychainSearch.h:58:10: note: 'SecKeychainSearchCreateFromAttributes' has been explicitly marked deprecated here
OSStatus SecKeychainSearchCreateFromAttributes(CFTypeRef keychainOrArray, SecItemClass itemClass, const SecKeychainAttributeList *attrList, SecKeychainSearchRef *searchRef)
         ^
../../crypto/apple_keychain_mac.mm:72:10: warning: 'SecKeychainSearchCopyNext' is deprecated: first deprecated in macOS 10.7 [-Wdeprecated-declarations]
  return SecKeychainSearchCopyNext(searchRef, itemRef);
         ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/Security.framework/Headers/SecKeychainSearch.h:69:10: note: 'SecKeychainSearchCopyNext' has been explicitly marked deprecated here
OSStatus SecKeychainSearchCopyNext(SecKeychainSearchRef searchRef, SecKeychainItemRef *itemRef)
         ^
2 warnings generated.

We are finally deprecating the Keychain for passwords.  In a couple of releases I'll drop password_store_mac.cc entirely.
Erik, I see you pasted a current warning on this bug, but as far as git blame shows, both those warnings were suppressed (by you actually) in March. Am I looking in the wrong place?
Cc: kerrnel@chromium.org
Labels: OS-Mac
Owner: vasi...@chromium.org
Status: Assigned (was: Available)
Right, the plan is to keep them suppressed since they're going to be removed shortly.
Project Member

Comment 15 by bugdroid1@chromium.org, Mar 2 2017

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

commit f31a6d6b2038f1987ff4106e9c833e78ad9ae73c
Author: vasilii <vasilii@chromium.org>
Date: Thu Mar 02 17:28:46 2017

Stop password migration from the Keychain on Mac.

Chrome is changing the certificate soon because the current one expires. Thus, the passwords created by Chrome in the Keychain become inaccessible. There is no point in trying the migration anymore. We just clean the database (it has no password values, only metadata) and start from scratch.

BUG= 595468 

Review-Url: https://codereview.chromium.org/2729853005
Cr-Commit-Position: refs/heads/master@{#454293}

[modify] https://crrev.com/f31a6d6b2038f1987ff4106e9c833e78ad9ae73c/chrome/browser/password_manager/password_store_proxy_mac.cc
[modify] https://crrev.com/f31a6d6b2038f1987ff4106e9c833e78ad9ae73c/chrome/browser/password_manager/password_store_proxy_mac_unittest.cc
[modify] https://crrev.com/f31a6d6b2038f1987ff4106e9c833e78ad9ae73c/components/password_manager/core/browser/keychain_migration_status_mac.h

Project Member

Comment 16 by bugdroid1@chromium.org, Mar 6 2017

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

commit fcf82d6e5b20e104b5eaba8d802ee451092126d5
Author: vasilii <vasilii@chromium.org>
Date: Mon Mar 06 20:26:33 2017

Add a value to PasswordManager.KeychainMigration.Status histogram.

It's a follow-up to https://codereview.chromium.org/2729853005/ where MigrationStatus was extended.

BUG= 595468 

Review-Url: https://codereview.chromium.org/2731323002
Cr-Commit-Position: refs/heads/master@{#454939}

[modify] https://crrev.com/fcf82d6e5b20e104b5eaba8d802ee451092126d5/tools/metrics/histograms/histograms.xml

The Chrome certificate expires soon. Therefore I stopped the migration as the passwords won't be readable anyway.

$ codesign --display --extract-certificates '/Applications/Google Chrome.app'
$ openssl x509 -inform DER -in codesign0 -text

        Validity
            Not Before: Apr 26 14:10:10 2012 GMT
            Not After : Apr 27 14:10:10 2017 GMT
Status: Fixed (was: Assigned)

Sign in to add a comment