You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@wandisco.com> on 2012/02/02 17:58:56 UTC

--non-interactive and keyrings

If I have passwords stored in the GNOME keyring and the keyring is
unlocked Subversion will read the passwords even when --non-interactive
is passed.  However if I have passwords stored in the KDE wallet and the
wallet is open Subversion will not read the stored passwords when
--non-interactive is passed.

http://subversion.tigris.org/issues/show_bug.cgi?id=4110

I can fix kwallet with this patch:

Index: subversion/libsvn_auth_kwallet/kwallet.cpp
===================================================================
--- subversion/libsvn_auth_kwallet/kwallet.cpp	(revision 1239705)
+++ subversion/libsvn_auth_kwallet/kwallet.cpp	(working copy)
@@ -192,7 +192,12 @@ kwallet_password_get(const char **password,
 {
   if (non_interactive)
     {
-      return FALSE;
+      QString wallet_name = get_wallet_name(parameters);
+      if (!KWallet::Wallet::isOpen(wallet_name))
+        return FALSE;
+
+      /* Is there a race here?  The wallet is open right now, but will
+         it still be open when we come to use it below? */
     }
 
   if (! dbus_bus_get(DBUS_BUS_SESSION, NULL))


However I am worried about the TOCTOU race.  There is a small window
between isOpen and the subsequent readPassword during which the wallet
may get closed.  This might cause --non-interactive to attempt to prompt
the user for a password.

The GNOME keyring code suffers from the same race, there is a small
window between checking whether the keyring is locked and getting the
passwords from the keyring.  The GNOME code also does the check earlier
in the call stack, so the window is potentially longer than in the KDE
case.

Do these small races matter?  Should we abandon attempts to use the
keyrings with --non-interactive?

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com

Re: --non-interactive and keyrings

Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Fri, Feb 03, 2012 at 12:59:47 +0000:
> Daniel Shahaf <da...@elego.de> writes:
> 
> > *shrug*.  All I'm saying is that it's done, and it's defensible.
> > I don't want to get into an argument about which behaviour is more
> > correct or more secure.
> 
> There are so many switches that interact it's hard to know what
> constitutes correct behaviour.
> 
> Suppose I have a password stored on disk.  If kwallet is enabled
> Subversion will use the disk password, I won't get prompted and the
> password will remain stored on disk.  If gnome-keyring is enabled I get
> the prompt to unlock the keyring but the password itself gets read from
> the disk and doesn't get stored in the keyring.  So despite the user
> interaction with the keyring the password remains stored on disk, that's
> a bit odd.
> 
> Worse, if I add --non-iteractive to command then kwallet will still use
> the password cached on disk without prompting.  However gnome-keyring
> will error out because it is unable to prompt for the keyring.  The
> error message is "Could not authenticate to server" with neon and "GNOME
> Keyring is locked" with serf and svnserve.  That's despite the fact that
> the passowrd is on disk.  I suppose we could argue that this is a
> configuration error on the part of the user.

Well, we could, had gnome-keyring not been enabled by default.

(For example, on Debian I'd run into the 'Password for (null) keyring'
issue when using the system's svn, despite never having enabled the
keyring or otherwise interacted with it; so I resorted to using my
self-built svn.  I suppose that proactively disabling the keyring daemon
from starting would have worked too.)

> 
> -- 
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com

Re: --non-interactive and keyrings

Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:

> *shrug*.  All I'm saying is that it's done, and it's defensible.
> I don't want to get into an argument about which behaviour is more
> correct or more secure.

There are so many switches that interact it's hard to know what
constitutes correct behaviour.

Suppose I have a password stored on disk.  If kwallet is enabled
Subversion will use the disk password, I won't get prompted and the
password will remain stored on disk.  If gnome-keyring is enabled I get
the prompt to unlock the keyring but the password itself gets read from
the disk and doesn't get stored in the keyring.  So despite the user
interaction with the keyring the password remains stored on disk, that's
a bit odd.

Worse, if I add --non-iteractive to command then kwallet will still use
the password cached on disk without prompting.  However gnome-keyring
will error out because it is unable to prompt for the keyring.  The
error message is "Could not authenticate to server" with neon and "GNOME
Keyring is locked" with serf and svnserve.  That's despite the fact that
the passowrd is on disk.  I suppose we could argue that this is a
configuration error on the part of the user.

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com

Re: --non-interactive and keyrings

Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:

>> Subversion does send the top-level names of trees that are excluded.
>
> Mike patched mod_dav_svn recently not to do that.  See for example
> https://svn.apache.org/repos/infra/
> (which has an 'infrastructure' child that isn't disclosed)

That's only for some requests. In general the names are visible:

$ svn co https://svn.apache.org/repos/infra/ --depth immediates wc
$ sqlite3 wc/.svn/wc.db "select local_relpath,presence from nodes"
infrastructure|absent
apachecon|normal
websites|normal
|normal


-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com

Re: --non-interactive and keyrings

Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Fri, Feb 03, 2012 at 10:38:49 +0000:
> Daniel Shahaf <da...@elego.de> writes:
> 
> > Philip Martin wrote on Fri, Feb 03, 2012 at 10:02:06 +0000:
> >> Julian Foad <ju...@btopenworld.com> writes:
> >> 
> >> >   * Brings kwallet to the same behaviour as Gnome keyring.
> >> 
> >> I've realised there is another difference in the current behaviour.  The
> >> way auth works is that Subversion records whether a particular provider
> >> was used to store a particular password.  The KDE provider will only
> >> prompt to open the wallet when the auth data indicates that KDE was used
> >> to store a particular password.  The GNOME provider prompts to unlock the
> >> keyring whenever any password is requested, before checking the auth
> >> data to see if this particular password was stored in the keyring.
> >> 
> >> I don't see any advantage to the GNOME behaviour, it looks more like a
> >> bug than a feature.
> >
> > That behaviour is defensible.  "Why should any random app I run know
> > what passwords my keyring stores?"
> >
> > Compare how Subversion does not disclose the names of directories one
> > doesn't have read access to.
> 
> Subversion does send the top-level names of trees that are excluded.
> 

Mike patched mod_dav_svn recently not to do that.  See for example
https://svn.apache.org/repos/infra/
(which has an 'infrastructure' child that isn't disclosed)

> Even without that I'm not sure I understand. The user can also try
> arbitrary names and get "access denied"; that would be similar to the
> user trying arbitrary URLs to see whether it caused the keyring to be
> unlocked.

Or to someone trying many usernames and seeing which of them cause the
server to prompt for a password, instead of an immediate "No such
username" error.  (The equivalent here would be to issue a password
prompt even for nonexistent usernames tried.)

*shrug*.  All I'm saying is that it's done, and it's defensible.
I don't want to get into an argument about which behaviour is more
correct or more secure.

> 
> -- 
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com

Re: --non-interactive and keyrings

Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:

> Philip Martin wrote on Fri, Feb 03, 2012 at 10:02:06 +0000:
>> Julian Foad <ju...@btopenworld.com> writes:
>> 
>> >   * Brings kwallet to the same behaviour as Gnome keyring.
>> 
>> I've realised there is another difference in the current behaviour.  The
>> way auth works is that Subversion records whether a particular provider
>> was used to store a particular password.  The KDE provider will only
>> prompt to open the wallet when the auth data indicates that KDE was used
>> to store a particular password.  The GNOME provider prompts to unlock the
>> keyring whenever any password is requested, before checking the auth
>> data to see if this particular password was stored in the keyring.
>> 
>> I don't see any advantage to the GNOME behaviour, it looks more like a
>> bug than a feature.
>
> That behaviour is defensible.  "Why should any random app I run know
> what passwords my keyring stores?"
>
> Compare how Subversion does not disclose the names of directories one
> doesn't have read access to.

Subversion does send the top-level names of trees that are excluded.

Even without that I'm not sure I understand. The user can also try
arbitrary names and get "access denied"; that would be similar to the
user trying arbitrary URLs to see whether it caused the keyring to be
unlocked.

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com

Re: --non-interactive and keyrings

Posted by Philip Martin <ph...@wandisco.com>.
Daniel Shahaf <da...@elego.de> writes:

> We're left still with the original problem --- that kwallet
> prompts for unlock even if it doesn't contain the password, but gkeyring
> prompts for unlock regardless of whether it contains the password?

Yes.  I can 'fix' gkeyring by moving the unlock prompt into
password_get_gnome_keyring.  At present that function has no way to
return the "GNOME Keyring is locked and we are non-interactive" error
so we would either have to drop that error, or change the prototypes of
svn_auth__password_get_t/svn_auth__password_set_t.

The kwallet doesn't have an equivalent of that gkeyring error, in
non-interactive it simply fails to get a password but doesn't report an
error (hence my original patch).  OSX switches of prompting but doesn't
report an error if that cause password retrieval to fail.  Windows
appears to ignore the non-interactive flag.  I don't know if that means
Windows never prompts or whether it means Windows will prompt in
non-interactive mode.

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com

Re: --non-interactive and keyrings

Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Fri, Feb 03, 2012 at 17:43:53 +0000:
> Philip Martin <ph...@wandisco.com> writes:
> 
> > The KDE behaviour is a potential information leak.  A random app can use
> > the Subversion libraries to query a repo, if it can monitor whether
> > such a query causes the KDE prompt to appear then it can determine
> > whether or not the password for the repo is in the wallet.  Since GNOME
> > always prompts no such leak is possible.
> 
> Thinking about this a bit further, it's not really a leak at all.

Agreed.  We're left still with the original problem --- that kwallet
prompts for unlock even if it doesn't contain the password, but gkeyring
prompts for unlock regardless of whether it contains the password?

> The information that is leaking is whether or not 'kwallet' is stored
> in the .subversion/auth directory for a given repository.  But any
> application that is capable of triggering the leak would also be
> capable of simply reading the .subversion/auth files.
> 
> -- 
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com

Re: --non-interactive and keyrings

Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:

> The KDE behaviour is a potential information leak.  A random app can use
> the Subversion libraries to query a repo, if it can monitor whether
> such a query causes the KDE prompt to appear then it can determine
> whether or not the password for the repo is in the wallet.  Since GNOME
> always prompts no such leak is possible.

Thinking about this a bit further, it's not really a leak at all.  The
information that is leaking is whether or not 'kwallet' is stored in the
.subversion/auth directory for a given repository.  But any application
that is capable of triggering the leak would also be capable of simply
reading the .subversion/auth files.

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com

Re: --non-interactive and keyrings

Posted by Philip Martin <ph...@wandisco.com>.
Julian Foad <ju...@btopenworld.com> writes:

> Daniel Shahaf wrote:
>
>> Philip Martin wrote:
>>> [...] Subversion records whether a particular provider
>>>  was used to store a particular password.  The KDE provider will only
>>>  prompt to open the wallet when the auth data indicates that KDE was used
>>>  to store a particular password.  The GNOME provider prompts to unlock the
>>>  keyring whenever any password is requested, before checking the auth
>>>  data to see if this particular password was stored in the keyring.
>>> 
>>>  I don't see any advantage to the GNOME behaviour, it looks more like a
>>>  bug than a feature.
>> 
>> That behaviour is defensible.  "Why should any random app I run know
>> what passwords my keyring stores?"
>
> Hi Daniel.  I don't follow what you mean.  The 'providers' that Philip
> refers to are bits of Subversion code, not the KDE/Gnome APIs
> themselves.

The KDE behaviour is a potential information leak.  A random app can use
the Subversion libraries to query a repo, if it can monitor whether
such a query causes the KDE prompt to appear then it can determine
whether or not the password for the repo is in the wallet.  Since GNOME
always prompts no such leak is possible.

It's not much of a leak, I'm not sure what one would do with the
information.

-- 
Philip

Re: --non-interactive and keyrings

Posted by Julian Foad <ju...@btopenworld.com>.
Daniel Shahaf wrote:

> Philip Martin wrote:
>> [...] Subversion records whether a particular provider
>>  was used to store a particular password.  The KDE provider will only
>>  prompt to open the wallet when the auth data indicates that KDE was used
>>  to store a particular password.  The GNOME provider prompts to unlock the
>>  keyring whenever any password is requested, before checking the auth
>>  data to see if this particular password was stored in the keyring.
>> 
>>  I don't see any advantage to the GNOME behaviour, it looks more like a
>>  bug than a feature.
> 
> That behaviour is defensible.  "Why should any random app I run know
> what passwords my keyring stores?"

Hi Daniel.  I don't follow what you mean.  The 'providers' that Philip refers to are bits of Subversion code, not the KDE/Gnome APIs themselves.

- Julian

Re: --non-interactive and keyrings

Posted by Daniel Shahaf <da...@elego.de>.
Philip Martin wrote on Fri, Feb 03, 2012 at 10:02:06 +0000:
> Julian Foad <ju...@btopenworld.com> writes:
> 
> >   * Brings kwallet to the same behaviour as Gnome keyring.
> 
> I've realised there is another difference in the current behaviour.  The
> way auth works is that Subversion records whether a particular provider
> was used to store a particular password.  The KDE provider will only
> prompt to open the wallet when the auth data indicates that KDE was used
> to store a particular password.  The GNOME provider prompts to unlock the
> keyring whenever any password is requested, before checking the auth
> data to see if this particular password was stored in the keyring.
> 
> I don't see any advantage to the GNOME behaviour, it looks more like a
> bug than a feature.

That behaviour is defensible.  "Why should any random app I run know
what passwords my keyring stores?"

Compare how Subversion does not disclose the names of directories one
doesn't have read access to.

> 
> -- 
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com

Re: --non-interactive and keyrings

Posted by Philip Martin <ph...@wandisco.com>.
Julian Foad <ju...@btopenworld.com> writes:

>   * Brings kwallet to the same behaviour as Gnome keyring.

I've realised there is another difference in the current behaviour.  The
way auth works is that Subversion records whether a particular provider
was used to store a particular password.  The KDE provider will only
prompt to open the wallet when the auth data indicates that KDE was used
to store a particular password.  The GNOME provider prompts to unlock the
keyring whenever any password is requested, before checking the auth
data to see if this particular password was stored in the keyring.

I don't see any advantage to the GNOME behaviour, it looks more like a
bug than a feature.

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com

Re: --non-interactive and keyrings

Posted by Daniel Shahaf <da...@elego.de>.
Julian Foad wrote on Thu, Feb 02, 2012 at 18:46:23 +0000:
> Seems like this fix is a big improvement already, and even better if
> we can make it more consistent with the Gnome-keyring behaviour.  If
> we care about the races, we can see about fixing them later, if there
> is any feasible way to do so.
> 

Ideally kwallet/gkeyring will grow a "may_prompt = FALSE" API parameter.
(We should presumably relay this to them.)

> - Julian

Re: --non-interactive and keyrings

Posted by Julian Foad <ju...@btopenworld.com>.
Philip Martin wrote:

> I can fix kwallet with this patch:
[...]

Great.

Without the patch:

  * Non-interactive commands will not try to authenticate from the 
kwallet, and instead attempt the next authentication method which will 
typically fail, leaving the user puzzled because the same command works 
fine without the '--non-interactive' option.

With the patch:

  * Non-interactive commands will now simply work as they should, if the Kwallet is unlocked.

  * Non-interactive commands will throw an error, if locked.

  * Brings kwallet to the same behaviour as Gnome keyring.

  * But there are still some inconsistencies between neon/serf, and 
gnome/kde, w.r.t. whether it returns an error if locked or quietly goes 
on to try the next authn method.


> However I am worried about the TOCTOU race.  [...]

So, there's a chance that a "Please unlock the keyring" GUI window will pop up, if the kwallet is configured to time out and happens to time out during the window of opportunity, or if some other process explicitly locks it during that same window.

But that doesn't sound like a grave problem to me.  Sure there are some scenarios where a hanging window is a problem, and if and when it happens it will be a surprise which is not nice.  But if someone uses this on an unattended machine they need to ensure 
the keyring won't time out, or they won't get very far anyway.  The same is already true for Gnome keyring.

Seems like this fix is a big improvement already, and even better if we can make it more consistent with the Gnome-keyring behaviour.  If we care about the races, we can see about fixing them later, if there is any feasible way to do so.

- Julian

Re: --non-interactive and keyrings

Posted by Branko Čibej <br...@apache.org>.
On 02.02.2012 17:58, Philip Martin wrote:
> However I am worried about the TOCTOU race.  There is a small window
> between isOpen and the subsequent readPassword during which the wallet
> may get closed.  This might cause --non-interactive to attempt to prompt
> the user for a password.

So ... neither KWallet nor Gnome Keyring have the option to return an
error when trying to read a locked keyring? Brilliant API design. :)

-- Brane