I don’t see why TortoiseSVN and other Windows clients would be affected, given that we don't enable the gpg support on Windows. +1 on adding a prefix, as the current code might also break other gpg users.
Do we make sure that we only send the password to an exact match of the realm? Otherwise somebody might be able to theoretically steal passwords by using a special realm string on a completely different server.
Bert
Sent from Windows Mail
From: Ben Reser
Sent: Friday, June 6, 2014 12:31 AM
To: dev_at_subversion.apache.org
On 6/2/14, 6:59 PM, Ben Reser wrote:
> 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.
> ]]]
Committed in r1600781 with some minor differences due to other changes I made
to gpg-agent support prior to that revision and some typo fixes.
There is still one concern I have over our gpg-agent support that I haven't
mentioned or made changes related to. At current we use the MD5 hash rof the
realm string (hex representation, 32 characters) as the cache id key. There is
no prefix. At current I don't believe that this can conflict with GPG's usage
of 40 character hex keygrips (i.e. the fingerprint for the key just without the
extra spaces). However, the manual for gpg-preset-passphrase suggests that
other applications should include a prefix[1]:
[[[
cacheid is either a 40 character keygrip of hexadecimal characters identifying
the key for which the passphrase should be set or cleared. The keygrip is
listed along with the key when running the command: gpgsm --dump-secret-keys.
Alternatively an arbitrary string may be used to identify a passphrase; it is
suggested that such a string is prefixed with the name of the application (e.g
foo:12346).
]]]
We can of course change this since the cache is only temporary. The only
problem I see with doing that is clients with differing cache-ids for gpg-agent
would not be able to use the credentials stored by the other. E.G. if you're
using a TortoiseSVN based on 1.8.x with a 1.9.x command line client and we
changed this with 1.9.0 then they'd have separate caches.
I have mixed feelings on if we should do anything about this. One one hand
there's no conflict right now. On the other hand if there is a conflict in the
future I'd prefer to have solved this now, especially since I don't think very
many people are using gpg-agent support given that there haven't been a lot of
complaints about the usability issues.
So on that basis I'm leaning towards making a compatibility breaking change in
1.9.0 that shifts to using a prefix of "svn:".
[1]
https://www.gnupg.org/documentation/manuals/gnupg-devel/Invoking-gpg_002dpreset_002dpassphrase.html#Invoking-gpg_002dpreset_002dpassphrase
Received on 2014-06-06 03:23:17 CEST