> -----Oorspronkelijk bericht-----
> Van: 'Stefan Sperling' [mailto:stsp_at_elego.de]
> Verzonden: donderdag 25 juni 2009 15:11
> On Thu, Jun 25, 2009 at 01:54:10PM +0200, Johan Corveleyn wrote:
[snip]
> > I must say that I was quite disappointed, when searching the
> > mailinglist archives, to see that a patch to get LDAP support
> directly
> > into svnserve (without SASL) was rejected:
> > http://svn.haxx.se/dev/archive-2008-02/0081.shtml
> >
> > I can understand the dev's arguments about increased maintenance
> cost
> > etc., but this is just such an important feature. Maybe nobody at
> the
> > time realized that svn+SASL+LDAP just doesn't work ...
>
> Note that Eric is not active anymore these days (he could still
> veto the change but I guess he won't care much...)
>
> But I see the following (different set of) problems with it,
> in order of importance:
>
> - It introduces a dependency on the ldap library even though this
> should (in theory) already be supported by SASL.
Granted, adding yet another library dependency might be annoying.
> There's a better short-term solution, see patch at the end of
> this mail.
> - As far as I can tell it sends passwords in plaintext, so unless
> you're running your ldap server and svnserve on the same box,
> you're going to have problems with clear text passwords being
> exposed on your network. To work around this you could set up
> an encrypted connection via a VPN or stunnel or similar means.
> But the same workaround can be used for existing SASL support
> (again, see patch at end of this mail).
> - It introduces a dependency on the ldap library without adding
> the necessary config foo to the configure script
> - It does not follow our coding conventions consistently, some of
> the spacing looks really wrong
Ok, but those last two problems are kind of peripheral. I agree the patch could be improved after some review (follow coding conventions, fix configure script, add tests, ...), but that's a minor detail from my point of view.
[snip]
> > For us this was a deal-breaker to use svnserve. We had no choice
> but
> > to go for apache+svn in the end. This is ok, but in reality this
> > means: svnserve is not usable in a company setting.
>
> I would not generalise like that. Many companies are fine with
> authenticating svnserve via SSH.
Ok, sorry. I was only referring to plain svnserve. But svnserve+ssh has its own set of problems (need to distribute public/private keypairs, which is not fun for Windows clients).
>
> Plain svn:// over public networks is obviously meant for read-only
> access anyway, because it cannot encrypt passwords.
>
> All this would change if someone added SSL support.
>
> Also, in the mean time, we could relax the requirements of
> svnserve's
> SASL support, and stop assuming that SSL is the only means of
> securing
> the PLAIN and LOGIN mechanisms. See patch below.
I'd leave the security issue up to the responsibility of the people installing svn this way. It's their call to make if they don't set it up securely, and just trust their closed, private intranet in some way (or to secure it by other means). As long as it's clearly documented that the passwords are transmitted in clear text, it seems fine to me ("and if you don't like that, use Apache+ssl+svn"). I figure there are also shops that don't bother with SSL, and use Basic Authentication over plain http...
OTOH, SSL support in svnserve would be superb, and offer more of a real choice ...
>
> If we then also amended the documentation to tell people to set up
> a VPN or stunnel or similar to secure communication to saslauthd,
> that's fine by me.
>
> Would this make you happy?
:). I don't have any personal interest in this feature anymore, since we've switched to Apache+SVN. It took some work, but now we're happy mod_dav_svn users. I just joined the discussion because of my past experiences with svnserve, and to help out other (future) SVN admins encountering this problem. Anyway, it would certainly add some closure :).
To the point: it's a workable solution of course, but nowhere near as convenient as the patch proposed by Eugene Ivlev.
- SASL is still very difficult to setup/configure/troubleshoot for a lot of people. Compared to the 5 line configuration in svnserve.conf (with the other patch), there is a huge difference.
- Using LDAP with SASL requires running an extra daemon on the server (saslauthd). That makes it more complex, less robust, ...
- SASL doesn't give you LDAP support on Windows (see http://www.sendmail.org/~ca/email/cyrus2/windows.html - no saslauthd). So you'd exclude svnserve setups on Windows.
I'd still prefer that other patch. OTOH, any LDAP support is better than nothing.
> Would you be willing to make the necessary documentation updates?
Hm, I'm really extremely busy lately (and doubt that's gonna change soon). But then again, isn't everyone?
If you would decide to implement this patch (or even better, the other one :)), I'll see if I can make some time to make my own contribution ...
But where/how do I do this? Any pointers, where to start?
Thanks a lot for your efforts.
Regards,
Johan
>
> Stefan
>
> [[[
> Stop assuming that SSL is the only way to secure PLAIN and LOGIN
> mechanisms. Until svnserve gains SSL support, people can secure
> communication to saslauthd by other means, such as VPNs, stunnel,
> etc.
>
> This should allow LDAP authentication with svnserve, but it's the
> user's responsibility to make sure plaintext passwords aren't
> leaked
> to the network.
>
> * subversion/libsvn_ra_svn/cyrus_auth.c,
> subversion/svnserve/cyrus_auth.c
> (new_sasl_ctx, cyrus_auth_request): Don't set
> SASL_SEC_NOPLAINTEXT.
> ]]]
>
> Index: subversion/libsvn_ra_svn/cyrus_auth.c
> ===================================================================
> --- subversion/libsvn_ra_svn/cyrus_auth.c (revision 38192)
> +++ subversion/libsvn_ra_svn/cyrus_auth.c (working copy)
> @@ -351,10 +351,8 @@ static svn_error_t *new_sasl_ctx(sasl_conn_t
> **sas
> sasl_errdetail(*sasl_ctx));
> }
>
> - /* Set security properties. Don't allow PLAIN or LOGIN, since we
> - don't support TLS yet. */
> + /* Set security properties. */
> svn_ra_svn__default_secprops(&secprops);
> - secprops.security_flags = SASL_SEC_NOPLAINTEXT;
> sasl_setprop(*sasl_ctx, SASL_SEC_PROPS, &secprops);
>
> return SVN_NO_ERROR;
> Index: subversion/svnserve/cyrus_auth.c
> ===================================================================
> --- subversion/svnserve/cyrus_auth.c (revision 38192)
> +++ subversion/svnserve/cyrus_auth.c (working copy)
> @@ -282,9 +282,6 @@ svn_error_t
> *cyrus_auth_request(svn_ra_svn_conn_t
> /* Initialize security properties. */
> svn_ra_svn__default_secprops(&secprops);
>
> - /* Don't allow PLAIN or LOGIN, since we don't support TLS yet.
> */
> - secprops.security_flags = SASL_SEC_NOPLAINTEXT;
> -
> /* Don't allow ANONYMOUS if a username is required. */
> no_anonymous = needs_username || get_access(b, UNAUTHENTICATED)
> < required;
> if (no_anonymous)
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=2365506
To unsubscribe from this discussion, e-mail: [users-unsubscribe_at_subversion.tigris.org].
Received on 2009-06-26 00:14:28 CEST