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

Auth code issues (was: svn commit: r32132 - in trunk/subversion: include include/private libsvn_auth_gnome_keyring libsvn_ra libsvn_ra_neon libsvn_subr)

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Tue, 15 Jul 2008 16:27:06 -0400

kfogel_at_tigris.org writes:
> Log:
> Cache SSL client certificate passphrases, when user indicates it's okay.
>
> Patch by: stylesen
> (Tweaked by kfogel.)

This patch (kudos to stylsen for taking it on -- it was big) raised some
issues with our auth code. Just so I don't forget any of these things,
I'm listing them below. Some are quite complex to explain; sorry about
that. The point of this mail is thoroughness, not skimmability :-).

   1. The client still sometimes prompts twice about whether or not to
      store the client cert passphrase. I've filed issue #3238 about
      this, though I'm not sure how reliable the reproduction recipe
      there is (please confirm if it happens for you too).

   2. We remember the client cert file by relative path, not absolute
      path. This causes a useability problem that will come up a lot.
      I've filed issue #3239 about it.

   3. There's now a lot of duplication in our auth code. Many of
      r32132's libsvn_subr/ssl_client_cert_pw_providers.c changes are
      made to code copied from libsvn_subr/simple_providers.c, for
      example. It would be nice to abstract some of this stuff out
      (stylesen said he was interested, but that he wanted to get this
      change in first and then work on abstraction -- I agree, FWIW).

   4. We've created another instance of an existing pool-lifetime
      issue. See issue #3236 about that, or search for "XXX: Hopefully"
      in the change.

   5. Some of the auth code uses "svn_auth__" prefixes when they're not
      necessary (i.e., on strictly file-internal symbols). Although I
      changed that in the r32132 patch, the problem still exists in the
      old code from which parts of r32132 were copied. I haven't filed
      any issue about this, just made a note.

   4. Subversion can't decide whether it's "passphrase" or "password"
      when we're talking about local stuff like client certs. Prior to
      r32132, our code said "password" (or the abbreviation "pw") in
      lots of places where it was really about passphrases. In r32132,
      we mostly used "passphrase" and "pp", which is consistent within
      that change but not consistent with all of the prior code (though
      it is consistent with some of the prior code).

      Obviously, there's no technical difference between a password and
      passphrase; the choice of term is a matter of convention. The
      issue here is just that if we want to present users with
      consistent terms (in ~/.subversion/servers, say), then we're
      probably going to get confused sometimes when we read our own code
      and those terms don't map backwards into our code in the way one
      would expect.

      I'm not sure this has to be solved; it would be a lot of work. I
      just wanted to mention the problem, and see if anyone had any
      bright ideas about terminology.

Whew. I think that's it.

Our auth code is extremely complex, but also extremely flexible -- it's
easy to criticize it while forgetting how much it does for us. I do
think there may be a bit more complexity than necessary, though.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-15 22:27:30 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.