Reimplement the macOS Keychain auth provider. The SecKeychain* API has been
deprecated for more than 10 years, causing deprecation warnings; so use the
"new" SecItem* API for accessing the Keychain (except for bits and pieces
that don't actually work as advertised).
The previous implementation remains and will be used on systems that don't
have the SecItem* API (anything older than macOS 10.6, I believe), and can
still be tested by uncommenting an #undef at the top of the file.
* subversion/libsvn_subr/macos_keychain.c:
Include svn_ctype.h for svn_ctype_isspace().
Remove unused headers svn_config.h, svn_user.h and svn_utf.h.
Update the top-level docstring in the file.
(keychain_password_set, keychain_password_get): Forward declare prototypes.
(allow_user_interaction, forbid_user_interaction): New helper functions;
these are wrapped in diagnostic pragmas that silence deprecation warnings.
[#if !SVN_HAVE_KEYCHAIN_SECITEM_API]:
(keychain_password_set, keychain_password_get): Use these new helpers
instead of calling SecKeychainSetUserInteractionAllowed directly.
[#if SVN_HAVE_KEYCHAIN_SECITEM_API]:
(query_entry, build_dict, make_label, cfstr, cfdata, safe_CFRelease):
(keychain_password_set, keychain_password_get): Implement using the
SecItem* API. Except for the non-interactive parts, which, while they
apparently have SecItem* equivalents, these are either deprecated or
don't work or both.
git-svn-id: https://svn.apache.org/repos/asf/subversion/trunk@1926564 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/subversion/libsvn_subr/macos_keychain.c b/subversion/libsvn_subr/macos_keychain.c
index 8ef0c29..1ece1d8 100644
--- a/subversion/libsvn_subr/macos_keychain.c
+++ b/subversion/libsvn_subr/macos_keychain.c
@@ -27,10 +27,8 @@
#include <apr_pools.h>
#include "svn_auth.h"
+#include "svn_ctype.h"
#include "svn_error.h"
-#include "svn_utf.h"
-#include "svn_config.h"
-#include "svn_user.h"
#include "auth.h"
#include "private/svn_auth_private.h"
@@ -39,10 +37,13 @@
#ifdef SVN_HAVE_KEYCHAIN_SERVICES
+/* Uncomment for testing the old SecKeychain*-based implementation. */
+/* #undef SVN_HAVE_KEYCHAIN_SECITEM_API */
+
#include <Security/Security.h>
/*-----------------------------------------------------------------------*/
-/* keychain simple provider, puts passwords in the KeyChain */
+/* keychain simple provider, puts passwords in the macOS Keychain */
/*-----------------------------------------------------------------------*/
/*
@@ -62,10 +63,77 @@
* implementation below between calls to
* SecKeychainSetUserInteractionAllowed() when multiple instances of
* the same Subversion auth provider-based app run concurrently.
+ *
+ *
+ * XXX (2025-06-18): All of the SecKeychain* API was deprecated in macOS 10.10
+ * (Yosemite, released in October 2014). The new SecItem* API was inherited
+ * from iOS, but it sometimes behaves strangely on macOS, see:
+ *
+ * https://developer.apple.com/forums/thread/724013
+ *
+ * Still, it's the way forward and the best way to avoid the deprecation
+ * warnings that pollute maintainer-mode builds. It also gives us some nice
+ * goodies, such as having a different label on the keychain item than the
+ * realm name, and a description to see the difference between a repository
+ * password and a client certificate passphrase.
*/
+/* Forward declarations for the SecItem-based implementation. */
+
/* Implementation of svn_auth__password_set_t that stores
- the password in the OS X KeyChain. */
+ the password in the OS X Keychain. */
+static svn_error_t *
+keychain_password_set(svn_boolean_t *done,
+ apr_hash_t *creds,
+ const char *realmstring,
+ const char *username,
+ const char *password,
+ apr_hash_t *parameters,
+ svn_boolean_t non_interactive,
+ apr_pool_t *pool);
+
+/* Implementation of svn_auth__password_get_t that retrieves
+ the password from the OS X Keychain. */
+static svn_error_t *
+keychain_password_get(svn_boolean_t *done,
+ const char **password,
+ apr_hash_t *creds,
+ const char *realmstring,
+ const char *username,
+ apr_hash_t *parameters,
+ svn_boolean_t non_interactive,
+ apr_pool_t *pool);
+
+
+/* It turns out that it's really not possible to do this without using any
+ deprecated symbols at all, at least not in command-line code. The
+ SecItem* variants kSecUseNoAuthenticationUI, kSecUseAuthenticationUI
+ either don't work on macOS, are also deprecated or both. */
+#ifdef __APPLE__
+# if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 2)
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+# endif
+#endif /* __APPLE__ */
+static void allow_user_interaction(svn_boolean_t non_interactive)
+{
+ if (non_interactive)
+ SecKeychainSetUserInteractionAllowed(TRUE);
+}
+static void forbid_user_interaction(svn_boolean_t non_interactive)
+{
+ if (non_interactive)
+ SecKeychainSetUserInteractionAllowed(FALSE);
+}
+#ifdef __APPLE__
+# if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 2)
+# pragma GCC diagnostic pop
+# endif
+#endif /* __APPLE__ */
+
+
+#if !SVN_HAVE_KEYCHAIN_SECITEM_API
+
static svn_error_t *
keychain_password_set(svn_boolean_t *done,
apr_hash_t *creds,
@@ -79,8 +147,7 @@
OSStatus status;
SecKeychainItemRef item;
- if (non_interactive)
- SecKeychainSetUserInteractionAllowed(FALSE);
+ forbid_user_interaction(non_interactive);
status = SecKeychainFindGenericPassword(NULL, (int) strlen(realmstring),
realmstring, username == NULL
@@ -105,16 +172,13 @@
CFRelease(item);
}
- if (non_interactive)
- SecKeychainSetUserInteractionAllowed(TRUE);
+ allow_user_interaction(non_interactive);
*done = (status == 0);
return SVN_NO_ERROR;
}
-/* Implementation of svn_auth__password_get_t that retrieves
- the password from the OS X KeyChain. */
static svn_error_t *
keychain_password_get(svn_boolean_t *done,
const char **password,
@@ -131,8 +195,7 @@
*done = FALSE;
- if (non_interactive)
- SecKeychainSetUserInteractionAllowed(FALSE);
+ forbid_user_interaction(non_interactive);
status = SecKeychainFindGenericPassword(NULL, (int) strlen(realmstring),
realmstring, username == NULL
@@ -140,8 +203,7 @@
: (int) strlen(username),
username, &length, &data, NULL);
- if (non_interactive)
- SecKeychainSetUserInteractionAllowed(TRUE);
+ allow_user_interaction(non_interactive);
if (status != 0)
return SVN_NO_ERROR;
@@ -152,6 +214,8 @@
return SVN_NO_ERROR;
}
+#endif /* !SVN_HAVE_KEYCHAIN_SECITEM_API */
+
/* Get cached encrypted credentials from the simple provider's cache. */
static svn_error_t *
keychain_simple_first_creds(void **credentials,
@@ -261,4 +325,260 @@
po->vtable = &keychain_ssl_client_cert_pw_provider;
*provider = po;
}
-#endif /* SVN_HAVE_KEYCHAIN_SERVICES */
+
+
+/* ==================================================================== */
+/* Using the SecItem API. */
+#if SVN_HAVE_KEYCHAIN_SECITEM_API
+
+/* Helpers for dealing with query dictionaries. */
+struct query_entry {
+ CFStringRef key;
+ CFTypeRef value;
+};
+
+/* Compensate for the horrible Core Foundation API that builds dictionaries
+ from two separate arrays of keys and values, which is ... less than
+ readable and interestingly error prone. */
+static CFDictionaryRef build_dict(const struct query_entry items[],
+ const CFIndex length)
+{
+ /* Use fixed buffer sizes to avoid allocation. */
+ static const CFIndex max_length = 12;
+
+ if (length <= max_length)
+ {
+ const void *keys[max_length];
+ const void *values[max_length];
+ CFIndex i;
+
+ /* Copy the keys and values into separate arrays. */
+ for (i = 0; i < length; ++i)
+ {
+ keys[i] = items[i].key;
+ values[i] = items[i].value;
+ }
+
+ return CFDictionaryCreate(kCFAllocatorDefault,
+ keys, values, length,
+ &kCFTypeDictionaryKeyCallBacks,
+ &kCFTypeDictionaryValueCallBacks);
+ }
+
+ /* abort(); (for debugging) */
+ return NULL;
+}
+
+/* Convert the generated REALMSTRING to a user-friendly label.
+ Assume REALMSTRING looks like "<uri:something> Realm" and skip
+ the <uri...> part. If that's not the case, or if the Realm bit is
+ empty, then return the whole REALMSTRING.
+*/
+static const char *make_label(const char *realmstring, apr_pool_t *pool)
+{
+ static const char prefix[] = "Subversion: ";
+ const char *p;
+
+ if (!realmstring || realmstring[0] != '<')
+ return realmstring;
+
+ /* Find the end of the <uri...> and skip whitespace after the closing >. */
+ p = strchr(realmstring, '>');
+ if (!p)
+ return realmstring;
+
+ do { ++p; } while (*p && svn_ctype_isspace(*p));
+ if (!*p)
+ return realmstring;
+
+ return apr_pstrcat(pool, prefix, p, NULL);
+}
+
+/* Wrap an immutable C string CSTR in a CFString.
+ The returned reference must be disposed with CFRelease. */
+static CFStringRef cfstr(const char *cstr)
+{
+ return CFStringCreateWithCStringNoCopy(kCFAllocatorDefault,
+ cstr ? cstr : "",
+ kCFStringEncodingUTF8,
+ kCFAllocatorNull);
+}
+
+/* Wrap an immutable C string PASSWD in a CFData. Only good for passwords.
+ The returned reference must be disposed with CFRelease. */
+static CFDataRef cfdata(const char *passwd)
+{
+ const UInt8 *data = (UInt8*)(passwd ? passwd : "");
+ const CFIndex length = strlen((char*)data);
+
+ return CFDataCreateWithBytesNoCopy(kCFAllocatorDefault,
+ data, length,
+ kCFAllocatorNull);
+}
+
+/* Wraps CFRelease so that ITEM can be NULL. */
+static void safe_CFRelease(CFTypeRef item)
+{
+ if (item)
+ CFRelease(item);
+}
+
+
+static svn_error_t *
+keychain_password_set(svn_boolean_t *done,
+ apr_hash_t *creds,
+ const char *realmstring,
+ const char *username,
+ const char *password,
+ apr_hash_t *parameters,
+ svn_boolean_t non_interactive,
+ apr_pool_t *pool)
+{
+ /* This is the user-visible item description in the Keychain. Defaults to
+ "application password", which isn't too user-friendly because the
+ realmstring is the same for both the client-cert passphrase and the
+ repository password.
+
+ Certificate passphrases get an empty username, see DUMMY_USERNAME
+ in subversion/libsvn_subr/ssl_client_cert_pw_providers.c. */
+ CFStringRef description = (username && *username
+ ? cfstr(_("repository password"))
+ : cfstr(_("certificate passphrase")));
+
+ CFStringRef service = cfstr(realmstring);
+ CFStringRef account = cfstr(username);
+ CFStringRef label = cfstr(make_label(realmstring, pool));
+ CFDataRef value = cfdata(password);
+
+ CFDictionaryRef add_query = NULL;
+ CFDictionaryRef update_query = NULL;
+ CFDictionaryRef update_attrs = NULL;
+ OSStatus status = errSecSuccess;
+
+ const struct query_entry query_items[] = {
+ { kSecClass, kSecClassGenericPassword },
+ { kSecAttrService, service },
+ { kSecAttrAccount, account },
+ { kSecMatchLimit, kSecMatchLimitOne },
+ /* These three entries are used only to add a new password. */
+ { kSecAttrDescription, description },
+ { kSecAttrLabel, label },
+ { kSecValueData, value }
+ };
+ const CFIndex add_length = sizeof(query_items) / sizeof(*query_items);
+ const CFIndex update_length = add_length - 3;
+
+ forbid_user_interaction(non_interactive);
+ *done = FALSE;
+
+ /* Memory allocation gone wrong? */
+ if (!service || !account || !label || !value)
+ goto cleanup;
+
+ add_query = build_dict(query_items, add_length);
+ if (!add_query)
+ goto cleanup;
+
+ status = SecItemAdd(add_query, NULL);
+ if (status == errSecSuccess)
+ {
+ *done = TRUE;
+ }
+ else if (status == errSecDuplicateItem)
+ {
+ const struct query_entry attr_items[] = {
+ { kSecAttrDescription, description }, /* Update the description. */
+ { kSecAttrLabel, label }, /* Update the label to the new format. */
+ { kSecValueData, value } /* Update the password. */
+ };
+ const CFIndex attr_length = sizeof(attr_items) / sizeof(*attr_items);
+
+ update_query = build_dict(query_items, update_length);
+ update_attrs = build_dict(attr_items, attr_length);
+ if (update_query && update_attrs)
+ {
+ status = SecItemUpdate(update_query, update_attrs);
+ if (status == errSecSuccess)
+ *done = TRUE;
+ }
+ }
+
+ cleanup:
+ allow_user_interaction(non_interactive);
+
+ /* Release in reverse order of allocation. */
+ safe_CFRelease(update_attrs);
+ safe_CFRelease(update_query);
+ safe_CFRelease(add_query);
+ safe_CFRelease(value);
+ safe_CFRelease(label);
+ safe_CFRelease(account);
+ safe_CFRelease(service);
+ safe_CFRelease(description);
+
+ return SVN_NO_ERROR;
+}
+
+
+static svn_error_t *
+keychain_password_get(svn_boolean_t *done,
+ const char **password,
+ apr_hash_t *creds,
+ const char *realmstring,
+ const char *username,
+ apr_hash_t *parameters,
+ svn_boolean_t non_interactive,
+ apr_pool_t *pool)
+{
+ CFStringRef service = cfstr(realmstring);
+ CFStringRef account = cfstr(username);
+ CFDictionaryRef query = NULL;
+ CFTypeRef item = NULL;
+ OSStatus status = errSecSuccess;
+
+ const struct query_entry query_items[] = {
+ { kSecClass, kSecClassGenericPassword },
+ { kSecAttrService, service },
+ { kSecAttrAccount, account },
+ { kSecMatchLimit, kSecMatchLimitOne },
+ /* Return only the data (password), which is actually a CFData object. */
+ { kSecReturnData, kCFBooleanTrue }
+ };
+ const CFIndex query_length = sizeof(query_items) / sizeof(*query_items);
+
+ forbid_user_interaction(non_interactive);
+ *done = FALSE;
+
+ /* Memory allocation gone wrong? */
+ if (!service || !account)
+ goto cleanup;
+
+ query = build_dict(query_items, query_length);
+ if (!query)
+ goto cleanup;
+
+ status = SecItemCopyMatching(query, &item);
+ if (status == errSecSuccess)
+ {
+ const CFIndex length = CFDataGetLength(item);
+ char *pwd = calloc(1, length + 1);
+
+ memcpy(pwd, CFDataGetBytePtr(item), length);
+ *password = pwd;
+ *done = TRUE;
+ }
+
+ cleanup:
+ allow_user_interaction(non_interactive);
+
+ /* Release in reverse order of allocation. */
+ safe_CFRelease(item);
+ safe_CFRelease(query);
+ safe_CFRelease(account);
+ safe_CFRelease(service);
+
+ return SVN_NO_ERROR;
+}
+
+#endif /* SVN_HAVE_KEYCHAIN_SECITEM_API */
+#endif /* SVN_HAVE_KEYCHAIN_SERVICES */
