fix: document macos keychain issues (#4309)

Author: adamspofford-dfinity

CHANGELOG.md

@@ -40,7 +40,7 @@ the `/http_gateway` endpoint, you should instead use `dfx info pocketic-config-p
 
 ### feat: add dfx native support for aarch64-Darwin
 
-Add dfx native support for aarch64-Darwin.
+Add dfx native support for aarch64-Darwin. Using it may require editing your identities. See the [migration guide](./docs/migration/dfx-0.28.0-migration-guide.md) for more information.
 
 ## Dependencies
 

Cargo.lock

@@ -0,0 +1,25 @@
+# dfx 0.28.0 migration guide
+
+## Keychain issues on macOS arm64
+
+This release brings the first arm64 macOS version of `dfx`. On arm64 macOS, there is an issue that prevents `dfx` from accessing identities stored in the keychain if an x64 version of `dfx` also has access to it. In order to restore arm64 `dfx`'s access to identities, you will need to remove x64 `dfx`'s access.
+
+You may want to export identities first using the previous version of dfx in case you make a mistake:
+
+```
+dfx +<older-version> identity export <identity-name>
+```
+
+First, open the "Keychain Access" application. You may see a popup telling you to try a different "Passwords" app instead, ignore it and open "Keychain Access".
+
+Next, scroll through the "Passwords" section of the "login" keychain. If it is blank, try switching to a different keychain, and then switch back to "login". Find the two or more entries labeled `internet_computer_identities`.
+
+![The Keychain Access app, on the passwords screen of the login keychain with an IC identity highlighted](images/dfx-0.28.0-keychain_access.png)
+
+For each `internet_computer_identities` entry you see, double-click it and switch to the "Access Control" tab. If you see `dfx` on that list, click it and then click the 'minus' button to remove it. If `dfx` shows up multiple times, remove each such entry. Finally, click "Save Changes".
+
+![A properties window for an individual password, showing the dfx entry on the Access Control screen.](images/dfx-0.28.0-keychain_access_focused.png)
+
+After these steps, `dfx` should no longer give you this error.
+
+If your workflow requires using multiple versions of dfx, note that this error will be re-created each time you "Always Allow" an x64 or pre-0.28 version of dfx to access your identities. You may want to copy (via export/import) identities so post-0.28 versions use different identities from pre-0.28 versions; that way they will not step on each other's toes.

docs/migration/dfx-0.28.0-migration-guide.md

@@ -52,6 +52,9 @@ tiny-bip39 = "1.0.0"
 time = { workspace = true, features = ["serde", "serde-human-readable"] }
 url.workspace = true
 
+[target.'cfg(target_os = "macos")'.dependencies]
+security-framework = "3"
+
 [dev-dependencies]
 futures.workspace = true
 proptest = "1.0"

docs/migration/images/dfx-0.28.0-keychain_access.png

@@ -3,6 +3,7 @@ use crate::error::fs::{
     ReadFileError, ReadPermissionsError, RemoveDirectoryAndContentsError, RemoveDirectoryError,
     RemoveFileError, RenameError, SetPermissionsError, WriteFileError,
 };
+use crate::error::keyring::KeyringMaintenanceError;
 use crate::error::{
     config::ConfigError,
     encryption::EncryptionError,
@@ -43,6 +44,9 @@ pub enum ConvertMnemonicToKeyError {
 pub enum CreateIdentityConfigError {
     #[error("Failed to generate a fresh encryption configuration")]
     GenerateFreshEncryptionConfigurationFailed(#[source] EncryptionError),
+
+    #[error("Failed to check for keyring availability")]
+    KeyringAvailabilityCheckFailed(#[source] KeyringMaintenanceError),
 }
 
 #[derive(Error, Debug)]

docs/migration/images/dfx-0.28.0-keychain_access_focused.png

@@ -29,4 +29,17 @@ pub enum KeyringError {
 
     #[error("Failed to set password for keyring")]
     SetPasswordFailed(#[source] keyring::Error),
+
+    #[error(transparent)]
+    MaintenanceRequired(#[from] KeyringMaintenanceError),
 }
+
+#[derive(Error, Debug)]
+#[error("\
+A macOS issue prevents arm64 versions of dfx from accessing your identities while an x64 version of dfx also has access.
+
+You will need to go into Keychain Access and remove dfx from the 'Access Control' tab of all 'internet_computer_identities' keys.
+
+For more information, see the dfx 0.28.0 migration guide: https://github.com/dfinity/sdk/blob/0.28.1/docs/migration/dfx-0.28.0-migration-guide.md
+")]
+pub struct KeyringMaintenanceError;

src/dfx-core/Cargo.toml

@@ -8,7 +8,9 @@ use crate::error::identity::{
     ConvertMnemonicToKeyError,
     ConvertMnemonicToKeyError::DeriveExtendedKeyFromPathFailed,
     CreateIdentityConfigError,
-    CreateIdentityConfigError::GenerateFreshEncryptionConfigurationFailed,
+    CreateIdentityConfigError::{
+        GenerateFreshEncryptionConfigurationFailed, KeyringAvailabilityCheckFailed,
+    },
     CreateNewIdentityError,
     CreateNewIdentityError::{
         ConvertSecretKeyToSec1PemFailed, CreateMnemonicFromPhraseFailed,
@@ -340,7 +342,9 @@ impl IdentityManager {
             } else {
                 match mode {
                     IdentityStorageMode::Keyring => {
-                        if keyring_mock::keyring_available(log) {
+                        if keyring_mock::keyring_available(log)
+                            .map_err(KeyringAvailabilityCheckFailed)?
+                        {
                             Ok(IdentityConfiguration {
                                 keyring_identity_suffix: Some(String::from(name)),
                                 ..Default::default()

src/dfx-core/src/error/identity.rs

@@ -1,9 +1,9 @@
 use super::TEMP_IDENTITY_PREFIX;
-use crate::error::keyring::KeyringError;
 use crate::error::keyring::KeyringError::{
     DecodePemFailed, DeletePasswordFailed, GetPasswordFailed, LoadMockKeyringFailed,
     MockKeyNotFound, MockUnavailable, NewEntryFailed, SaveMockKeyringFailed, SetPasswordFailed,
 };
+use crate::error::keyring::{KeyringError, KeyringMaintenanceError};
 use crate::json::{load_json_file, save_json_file};
 use keyring;
 use serde::{Deserialize, Serialize};
@@ -75,7 +75,10 @@ pub fn load_pem_from_keyring(identity_name_suffix: &str) -> Result<Vec<u8>, Keyr
         KeyringMockMode::NoMock => {
             let entry = keyring::Entry::new(KEYRING_SERVICE_NAME, &keyring_identity_name)
                 .map_err(NewEntryFailed)?;
-            let encoded_pem = entry.get_password().map_err(GetPasswordFailed)?;
+            let encoded_pem = entry
+                .get_password()
+                .handle_macos_acl_error()?
+                .map_err(GetPasswordFailed)?;
             let pem = hex::decode(encoded_pem).map_err(DecodePemFailed)?;
             Ok(pem)
         }
@@ -104,6 +107,7 @@ pub fn write_pem_to_keyring(
                 .map_err(NewEntryFailed)?;
             entry
                 .set_password(&encoded_pem)
+                .handle_macos_acl_error()?
                 .map_err(SetPasswordFailed)?;
             Ok(())
         }
@@ -118,7 +122,7 @@ pub fn write_pem_to_keyring(
 }
 
 /// Determines if keyring is available by trying to write a dummy entry.
-pub fn keyring_available(log: &Logger) -> bool {
+pub fn keyring_available(log: &Logger) -> Result<bool, KeyringMaintenanceError> {
     match KeyringMockMode::current_mode() {
         KeyringMockMode::NoMock => {
             trace!(log, "Checking for keyring availability.");
@@ -128,13 +132,16 @@ pub fn keyring_available(log: &Logger) -> bool {
                 KEYRING_IDENTITY_PREFIX, TEMP_IDENTITY_PREFIX, "dummy"
             );
             if let Ok(entry) = keyring::Entry::new(KEYRING_SERVICE_NAME, &dummy_entry_name) {
-                entry.set_password("dummy entry").is_ok()
+                Ok(entry
+                    .set_password("dummy entry")
+                    .handle_macos_acl_error()?
+                    .is_ok())
             } else {
-                false
+                Ok(false)
             }
         }
-        KeyringMockMode::MockReject => false,
-        KeyringMockMode::MockAvailable => true,
+        KeyringMockMode::MockReject => Ok(false),
+        KeyringMockMode::MockAvailable => Ok(true),
     }
 }
 
@@ -144,7 +151,7 @@ pub fn delete_pem_from_keyring(identity_name_suffix: &str) -> Result<(), Keyring
         KeyringMockMode::NoMock => {
             let entry = keyring::Entry::new(KEYRING_SERVICE_NAME, &keyring_identity_name)
                 .map_err(NewEntryFailed)?;
-            if entry.get_password().is_ok() {
+            if entry.get_password().handle_macos_acl_error()?.is_ok() {
                 entry.delete_credential().map_err(DeletePasswordFailed)?;
             }
         }
@@ -157,3 +164,24 @@ pub fn delete_pem_from_keyring(identity_name_suffix: &str) -> Result<(), Keyring
     }
     Ok(())
 }
+
+trait KeyringResultExt: Sized {
+    fn handle_macos_acl_error(self) -> Result<Self, KeyringMaintenanceError>;
+}
+
+impl<T> KeyringResultExt for Result<T, keyring::Error> {
+    fn handle_macos_acl_error(self) -> Result<Self, KeyringMaintenanceError> {
+        match self {
+            Ok(value) => Ok(Ok(value)),
+            #[cfg(target_os = "macos")]
+            Err(keyring::Error::PlatformFailure(err))
+                if err
+                    .downcast_ref::<security_framework::base::Error>()
+                    .is_some_and(|err| err.code() == -67671) =>
+            {
+                Err(KeyringMaintenanceError)
+            }
+            Err(e) => Ok(Err(e)),
+        }
+    }
+}