On Mon, Sep 06, 2010 at 01:59:22PM +0700, Victor Sudakov wrote:
> Stefan Sperling wrote:
> > > > > Colleagues, I understand that you are expecting a patch. I am sorry, I
> > > > > am a systems administrator and not a programmer, my code writing
> > > > > ability does not go beyond scripting.
> > > >
> > > > Can you try this patch and let me know if it works?
> > >
> > > Please try this one instead, it's slightly cleaner (no functional change).
> > > Thanks.
> >
> > Ping. Did you find time to test this?
> > Should we file an issue so we don't forget about this?
>
> Sorry for the delay, I have been on vacation. Does not compile with
> your patch.
Ooops, sorry about that.
I just noticed that my svn builds didn't use SASL at all, so it compiled
for me because the code was disabled :-/
The updated diff below compiles fine for me, with SASL enabled.
Stefan
Index: subversion/libsvn_subr/config_file.c
===================================================================
--- subversion/libsvn_subr/config_file.c (revision 993034)
+++ subversion/libsvn_subr/config_file.c (working copy)
@@ -759,6 +759,11 @@ svn_config_ensure(const char *config_dir, apr_pool
NL
"### may be cached to disk." NL
"### username Specifies the default username." NL
+ "### preferred-sasl-mechanism Specifies which SASL mechanism" NL
+ "### among the ones offered by the " NL
+ "### server should be tried first." NL
+ "### See the SASL documentation for" NL
+ "### a list of mechanisms available." NL
"###" NL
"### Set store-passwords to 'no' to avoid storing passwords in the" NL
"### auth/ area of your config directory. It defaults to 'yes'," NL
Index: subversion/libsvn_ra_svn/cyrus_auth.c
===================================================================
--- subversion/libsvn_ra_svn/cyrus_auth.c (revision 993034)
+++ subversion/libsvn_ra_svn/cyrus_auth.c (working copy)
@@ -27,6 +27,7 @@
#include <apr_thread_mutex.h>
#include <apr_version.h>
+#include "svn_config.h"
#include "svn_types.h"
#include "svn_string.h"
#include "svn_error.h"
@@ -720,6 +721,67 @@ svn_error_t *svn_ra_svn__get_addresses(const char
return SVN_NO_ERROR;
}
+
+/* Return one or more SASL mechanisms from MECHLIST.
+ * SESS is the session baton.
+ * If a preferred SASL mechanism has been defined in the configuration,
+ * prefer it if it occurs within MECHLIST. Else, fall back to EXTERNAL,
+ * then ANONYMOUS, then let SASL decide.
+ * Potentially allocate the returned list of mechanisms in RESULT_POOL.
+ * Use SCRATCH_POOL for temporary allocations. */
+static const char *
+get_sasl_mechanisms(svn_ra_svn__session_baton_t *sess,
+ apr_array_header_t *mechlist,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+ const char *mechstring = "";
+ svn_config_t *cfg;
+
+ cfg = sess->config ? apr_hash_get(sess->config, SVN_CONFIG_CATEGORY_SERVERS,
+ APR_HASH_KEY_STRING) : NULL;
+ if (cfg)
+ {
+ const char *server_group;
+ const char *preferred_mech;
+
+ server_group = svn_config_find_group(cfg, sess->hostname,
+ SVN_CONFIG_SECTION_GROUPS,
+ scratch_pool);
+ if (server_group)
+ svn_config_get(cfg, &preferred_mech, server_group,
+ SVN_CONFIG_OPTION_PREFERRED_SASL_MECHANISM, NULL);
+ else
+ preferred_mech = NULL;
+
+ if (preferred_mech && svn_ra_svn__find_mech(mechlist, preferred_mech))
+ return preferred_mech;
+ }
+
+ if (svn_ra_svn__find_mech(mechlist, "EXTERNAL"))
+ return "EXTERNAL";
+ else if (svn_ra_svn__find_mech(mechlist, "ANONYMOUS"))
+ return "ANONYMOUS";
+ else
+ {
+ int i;
+
+ /* Create a string containing the list of mechanisms,
+ * separated by spaces. */
+ for (i = 0; i < mechlist->nelts; i++)
+ {
+ svn_ra_svn_item_t *elt = &APR_ARRAY_IDX(mechlist, i,
+ svn_ra_svn_item_t);
+ mechstring = apr_pstrcat(result_pool,
+ mechstring,
+ i == 0 ? "" : " ",
+ elt->u.word, NULL);
+ }
+ }
+
+ return mechstring;
+}
+
svn_error_t *
svn_ra_svn__do_cyrus_auth(svn_ra_svn__session_baton_t *sess,
apr_array_header_t *mechlist,
@@ -734,7 +796,6 @@ svn_ra_svn__do_cyrus_auth(svn_ra_svn__session_bato
array terminator). */
sasl_callback_t callbacks[3];
cred_baton_t cred_baton;
- int i;
if (!sess->is_tunneled)
{
@@ -742,24 +803,7 @@ svn_ra_svn__do_cyrus_auth(svn_ra_svn__session_bato
sess->conn, pool));
}
- /* Prefer EXTERNAL, then ANONYMOUS, then let SASL decide. */
- if (svn_ra_svn__find_mech(mechlist, "EXTERNAL"))
- mechstring = "EXTERNAL";
- else if (svn_ra_svn__find_mech(mechlist, "ANONYMOUS"))
- mechstring = "ANONYMOUS";
- else
- {
- /* Create a string containing the list of mechanisms, separated by
spaces. */
- for (i = 0; i < mechlist->nelts; i++)
- {
- svn_ra_svn_item_t *elt = &APR_ARRAY_IDX(mechlist, i,
svn_ra_svn_item_t);
- mechstring = apr_pstrcat(pool,
- mechstring,
- i == 0 ? "" : " ",
- elt->u.word, NULL);
- }
- }
-
+ mechstring = get_sasl_mechanisms(sess, mechlist, pool, pool);
realmstring = apr_psprintf(pool, "%s %s", sess->realm_prefix, realm);
/* Initialize the credential baton. */
Index: subversion/libsvn_ra_svn/client.c
===================================================================
--- subversion/libsvn_ra_svn/client.c (revision 993034)
+++ subversion/libsvn_ra_svn/client.c (working copy)
@@ -715,6 +715,7 @@ static svn_error_t *ra_svn_open(svn_ra_session_t *
reparent with a server that doesn't support reparenting. */
SVN_ERR(open_session(&sess, url, &uri, tunnel_argv,
callbacks, callback_baton, sess_pool));
+ sess->config = config;
session->priv = sess;
return SVN_NO_ERROR;
Index: subversion/libsvn_ra_svn/ra_svn.h
===================================================================
--- subversion/libsvn_ra_svn/ra_svn.h (revision 993034)
+++ subversion/libsvn_ra_svn/ra_svn.h (working copy)
@@ -97,6 +97,7 @@ struct svn_ra_svn__session_baton_t {
void *callbacks_baton;
apr_off_t bytes_read, bytes_written; /* apr_off_t's because that's what
the callback interface uses */
+ apr_hash_t *config;
};
/* Set a callback for blocked writes on conn. This handler may
Index: subversion/include/svn_config.h
===================================================================
--- subversion/include/svn_config.h (revision 993034)
+++ subversion/include/svn_config.h (working copy)
@@ -81,6 +81,7 @@ typedef struct svn_config_t svn_config_t;
#define SVN_CONFIG_OPTION_STORE_SSL_CLIENT_CERT_PP_PLAINTEXT \
"store-ssl-client-cert-pp-plaintext"
#define SVN_CONFIG_OPTION_USERNAME "username"
+#define SVN_CONFIG_OPTION_PREFERRED_SASL_MECHANISM "preferred-sasl-mechanism"
#define SVN_CONFIG_CATEGORY_CONFIG "config"
#define SVN_CONFIG_SECTION_AUTH "auth"
