On 8/21/06, Vlad Georgescu <vgeorgescu@gmail.com> wrote:
> This patch adds Cyrus SASL support to svnserve. Security layers are
> not supported yet - I'll submit another patch for that when this one
> gets the 'stamp of approval'.
>
> [[[
> * subversion/include/svn_config.h
> (SVN_CONFIG_SECTION_SASL): New section.
> (SVN_CONFIG_OPTION_USE_SASL): New option.
>
> * subversion/libsvn_repos/repos.c
> (create_conf): Document the new option.
>
> * subversion/svnserve/sasl_auth.c: New file.
>
> * subversion/svnserve/main.c
> (main): Call sasl_init().
>
> * subversion/svnserve/serve.c:
> (server_baton_t, authn_type, authz_type, get_access): Moved to server.h.
> (simple_auth_request): Contains the code that was prieviously in auth_request.
> (auth_request): Call sasl_auth_request() or simple_auth_request().
> (must_have_access): Consider the value of b->use_sasl when
> determining whether authentication should be performed.
> (find_repos): Read the value of the use-sasl option into b->use_sasl.
> Use that value when determining whether access is allowed to the repository.
>
> * subversion/svnserve/server.h
> (server_baton_t): Moved here from serve.c. Has a new member 'use_sasl'
> (authn_type, authz_type, get_access): Moved here from serve.c.
> (sasl_init, sasl_auth_request): New declarations.
> ]]]
>
> SASL supports many password-checking methods. It can use an external
> daemon saslauthd that can authenticate against /etc/shadow, PAM, LDAP,
> etc. It only works with plaintext passwords, though, so we can't use
> it until we have SSL support in for ra_svn. Until then, we have to use
> the other method, auxprop, which fetches passwords from a database
> (which is usually stored in /etc/sasldb2, but could also be an
> external SQL server, or a LDAP database)
>
> To test this patch, create a configuration file Subversion.conf in
> /usr/lib/sasl2. This file will be read by Cyrus SASL at runtime to
> determine how authentication should be performed.
> See the SASL docs for more details. A simple example could be:
>
> pwcheck_method: auxprop
> mech_list: DIGEST-MD5 ANONYMOUS
>
> Then add users and passwords to Cyrus SASL's password database:
>
> saslpasswd2 -c user -u realm -a Subversion
>
> where 'realm' is the actual realm of your repository.
> Just like Subversion, Cyrus SASL supports the concept of
> "authentication realms". In fact, svnserve will treat the repository
> realm as a SASL realm when authenticating the user. This is necessary
> because the password database is global instead of per-repository, so
> it's the only way for Subversion to tell which users belong to which
> repository.
>
> Comments and criticism are welcome, as always.
Hey Vlad, I spent a fair amount of time looking at this today, and
it's looking pretty good, but I think I found a problem in the
existing sasl client code. I'm getting some strage deadlocks while
running the svnsync tests, specifically test number 13, "detect
non-svnsync commits in destination". The problem appears to be one of
pool destruction ordering. We end up calling sasl_done before we
destroy some of the pools that hold sasl contexts, so when sasl tries
to use the mutex wrappers the pool has already been destroyed, so it
tries to lock/unlock mutexes that are allocated in memory that's been
reused. Oops.
The following patch shows the problem:
Index: subversion/libsvn_ra_svn/sasl_auth.c
===================================================================
--- subversion/libsvn_ra_svn/sasl_auth.c (revision 21225)
+++ subversion/libsvn_ra_svn/sasl_auth.c (working copy)
@@ -40,6 +40,21 @@
#include "ra_svn.h"
#include "ra_svn_sasl.h"
+static void
+debug(const char *fmt, ...)
+{
+ FILE *f = fopen("/tmp/debug.out", "a+");
+ va_list ap;
+
+ fprintf(f, "%d ", getpid());
+
+ va_start(ap, fmt);
+ vfprintf(f, fmt, ap);
+ va_end(ap);
+
+ fclose(f);
+}
+
static volatile svn_atomic_t sasl_status;
static apr_pool_t *sasl_pool = NULL;
@@ -47,6 +62,7 @@
/* Pool cleanup called when sasl_pool is destroyed. */
static apr_status_t sasl_done_cb(void *data)
{
+ debug("sasl_done\n");
/* Reset sasl_status, in case the client calls
apr_initialize()/apr_terminate() more than once. */
sasl_status = 0;
@@ -76,6 +92,9 @@
apr_thread_mutex_t *mutex;
apr_status_t apr_err;
+ if (! sasl_status)
+ return NULL;
+
apr_err = apr_thread_mutex_lock(array_mutex);
if (apr_err != APR_SUCCESS)
return NULL;
@@ -164,6 +183,7 @@
static apr_status_t sasl_dispose_cb(void *data)
{
+ debug("sasl_dispose_cb: %d\n", (int) data);
sasl_conn_t *sasl_ctx = data;
sasl_dispose(&sasl_ctx);
return APR_SUCCESS;
Run it with svnsync_tests.py --url=svn://localhost/ 13
Then look in /tmp/debug.log, and you'll see the ordering problem. The
if (! sasl_status) bit is a hack so that we don't do the bogus stuff,
which avoids the deadlock, but obviously isn't the "right" solution.
Good enough to see the debug trace though...
I'm not entirely sure what the solution here is, need to think about
it some more.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Aug 24 22:46:52 2006