subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ben Reser <...@reser.org>
Subject Improving gpg-agent support
Date Tue, 03 Jun 2014 01:59:01 GMT
Since 1.8 we've supported using gpg-agent to store passwords in memory.
http://subversion.apache.org/docs/release-notes/1.8.html#gpg-agent

Today I was getting asked about some odd behavior that a customer was seeing so
I took some time to investigate.

The behavior of this setup is very wonky and not what a user would expect.
When gpg-agent is setup the first time you connect to a realm you'll be
prompted by the Subversion client for the password.  It'll then store in our
auth cache that the password is stored in gpg-agent.  However, the agent
doesn't get this password from this action.  The next time the user connects to
this realm they are prompted by the pinentry program that is configured with
gpg-agent to get the password.  After that there is a confirmation prompt
requiring they re-enter the password.  Subsequent connections to the realm then
use the password if it is still cached in gpg-agent.  If the agent's cache has
been cleared (timeout or other actions that would clear it) then the pinentry
prompt (confirmation entry included) is repeated.

This seems like a very poor implementation from an end user perspective.  None
of the above behavior is documented in our release notes.  So the average user
is probably going to believe that things are not working properly.  There are
of course some very good reasons for this behavior.

gpg-agent and Subversion's auth system has somewhat contradictory designs.
gpg-agent takes on the responsibility of not just storing the password but also
of the user interface for the user to input the password if it is not stored.
Subversion on the other hand presumes that the cache systems are just dumb
storage and that it has to handle the UI.  Subversion independently stores that
it has a password cached in a given cache system (in a file under
~/.subversion/auth/svn.simple with the file being the MD5 hash of the realm).
It only contacts that system for the cached password if the appropriate
svn.simple entry is present.

With every other Subversion password caching mechanism Subversion would prompt
you for your password the first time you connected to the realm and then if the
password is correct it would store it in the appropriate cache and create the
svn.simple entry.  However, with gpg-agent it does not store the password with
that first use, but does store the svn.simple entry causing subsequent requests
to go to gpg-agent.  Since gpg-agent does not have the password cached it then
prompts the user.

Subversion currently tells gpg-agent when prompting the user for a password to
confirm the password by asking the user to re-enter it.  This choice appears to
have been made because gpg-agent will cache a password without knowing for sure
that the password is correct.  If an incorrect password were to be cached then
the user would have to take steps to clear the gpg-agent cache on their own
(kill -HUP, gpg-connect-agent CLEAR_PASSPHARSE).  Until they did so they would
be prompted for the password by Subversion (since the cached password failed)
on every single connection.  Since the cached password in gpg-agent would be
tried on every connection, the user may run into systems that lock them out as
well.  The choice of a confirmation entry doesn't really seem very good in my
opinion.  First, it only protects against typos, a user entering the wrong
password (though entering the same password) twice will still experience the
problem described above.  Second, the default timeout for passwords in
gpg-agent is 10 minutes.  So now a user using this setup will have to enter
their password twice every 10 minutes.  The caching feature is a convenience
feature, but having to enter the password twice seems not very convenient.

Fortunately there are some thing we can do to resolve these issues.  First
let's talk about the double prompt from pinentry.  This is entirely
unnecessary.  Subversion's API provides two functions to retrieve credentials
from the cache.  A first credential function and a next credential function.
The purpose being to allow multiple credentials to be stored for a realm and
then walked until you find the ones that work.  In practice we've never used
this for passwords because there's no point in storing more than one credential
for a password since only one will work.  However, we can use this to allow us
to drive gpg-agent as it intends.  We do a GET_PASSPHRASE in first and then if
we receive a next call we do a CLEAR_PASSPHRASE followed by a GET_PASSPHRASE
with an error text telling the user the authentication failed (if we're being
run in interactive mode).  I've attached a patch that implements this bit.  But
I haven't committed it because I'm not sure this is necessarily the best
solution.  I'm also abusing the parameters hash to avoid duplicating the
svn_auth__simple_creds_cache_get() function.

The failure to cache on the first connection to the realm issue is a little bit
harder to solve.  There is actually a PRESET_PASSPHARSE call in gpg-agent's
API.  But it only works when gpg-agent is started with
--allow-preset-passphrase.  I think we should make the save function of the
gpg-agent provider implement the PRESET_PASSPHRASE call.  We can document to
users they will have a better experience if they provide the
--allow-preset-passphrase option to gpg-agent when they start it.  We can
ignore errors if it doesn't.

There is another option and that is to use gpg-agent as a dumb store like we do
other caches by combining PRESET_PASSPHRASE in the save function and
GET_PASSPHARSE --no-ask in the first function.  This would allow us to retain
the behavior that the svn client asks for the password and thus not have to
jump through hoops to support the cache system being responsible for the UI to
request the password.  Doing this of course would rquire that
--allow-preset-passphrase be passed.  So I think it'd probably be best to have
a setting in our Subversion config that enables this mode but that then fails
if --allow-preset-passphrase is not enabled on gpg-agent.  This behavior would
give the best experience to our users, but since it's intrusive on gpg-agent's
configuration I don't think it should be default.

Because of these things I believe the patch should be applied and the later two
things should be iterated on top of that.

Thoughts?

Commit message for the patch:
[[[
Make the gpg-agent pinentry not ask for confirmation of password entries and
make it re-prompt if the password is incorrect.

* subversion/libsvn_subr/gpg_agent.c:
  (ATTEMPT_PARAMETER): New macro.
  (send_options, get_cache_id): New functions with code taken out of
    password_get_gpg_agent() so it can be reused.
  (password_get_gpg_agent): Use send_options() and get_cache_id(),
    retrieve the attempt from the parameters and use it to determine
    if we should set an error message that will be displayed in pinentry.
  (simple_gpg_agent_first_creds): Set a iter_baton so we can limit the
    retries, put the iter_baton in the parameters so password_get_gpg_agent()
    can access it.
  (simple_gpg_agent_next_creds): New function, removes the cached password
    and prompts the user again.
  (gpg_agent_simple_provider): Add simple_gpg_agent_next_creds callback.
]]]

Mime
View raw message