[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r17619 - in trunk: subversion/include subversion/libsvn_subr

From: mark benedetto king <mbk_at_lowlatency.com>
Date: 2005-12-07 03:54:20 CET

On Tue, Dec 06, 2005 at 04:48:16PM -0800, Daniel Rall wrote:
>
> > Note: SecKeychainSetUserInteractionAllowed (FALSE) does not appear to
> > actually prevent all user interaction. Specifically, if the executable
> > changes (for example, if it is rebuilt), the system prompts the user
> > to okay the use of the new executable.
>
> Perhaps this gotcha should be documented in-line as well?

Good idea. The is a patch below; maybe I won't have to commit it if
a KeyChain Services guru materializes from the ether and fills in some
of my knowledge gap. More on the state of affairs below.

>
> Are these calls to SecKeychainSetUserInteractionAllowed() thread-local
> or something? If not, it seems like there is a race here where
> another simultaneously executing application could have its
> "interaction allowed" flag toggled. Does the KeyChain API supply some
> sort of mutex or sychronization API which can be used in conjunction
> with this setting? (I looked around for some documentation on that,
> but didn't find much other than other code doing the same thing.)
>
> This possible race is in keychain_password_get() as well.

I'd like to think that they were thread-local. It's certainly possible,
but I fear that it might actually worse than that: I think it may be
system-global.

I think that because of this paragraph from (sorry for the long URL)

http://developer.apple.com/documentation/Security/Reference/keychainservices/index.html

  Special Considerations

  If you disable user interaction before calling a Keychain Services
  function, be sure to reenable it when you are finished. Failure
  to reenable user interaction will affect other clients of the
  Keychain Services.

That's, um, scary.

Since it's unclear to me under which circumstances it even actually
works, we might want to stop calling it entirely. Except that not
calling it might make scripts less reliable. What a dilemma!

--ben

[[[
Document the state of affairs WRT SecKeychainSetUserInteractionAllowed().

* subversion/libsvn_subr/simple_providers.c:
  Add comment.
]]]

Index: subversion/libsvn_subr/simple_providers.c
===================================================================
--- subversion/libsvn_subr/simple_providers.c (revision 17659)
+++ subversion/libsvn_subr/simple_providers.c (working copy)
@@ -733,6 +733,22 @@
 #if SVN_HAVE_KEYCHAIN_SERVICES
 #include <Security/Security.h>

+/*
+ * XXX: SecKeychainSetUserInteractionAllowed (FALSE) does not appear to
+ * actually prevent all user interaction. Specifically, if the executable
+ * changes (for example, if it is rebuilt), the system prompts the user
+ * to okay the use of the new executable.
+ *
+ * Worse than that, as Daniel Rall pointed out, there is a race condition
+ * in the implementation below, presuming that the API doesn't address
+ * thread-local storage of some sort (which it may).
+ *
+ * Worse than THAT, it appears that the race might even be *system-wide*,
+ * rather than merely intra-process, since the on-line documentation warns
+ * that "failure to reenable user interaction will affect other clients
+ * of the Keychain Services".
+ */
+
 /* Implementation of password_set_t that stores the password
    in the OS X KeyChain. */
 static svn_boolean_t

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Dec 7 03:52:05 2005

This is an archived mail posted to the Subversion Dev mailing list.