You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Vlad Georgescu <vg...@gmail.com> on 2006/10/11 10:59:04 UTC

[PATCH] Make SASL mechanism negotiation smarter

Hi,

This is a fix for a problem originally described here:
http://svn.haxx.se/dev/archive-2006-09/0994.shtml

Specifically, if GSSAPI is part of the list of mechanisms sent by the
server, the client will always choose it over the others (because it's
the most secure) even if it isn't really prepared to use it (e.g.
because there are no Kerberos credentials). Thus authentication will
always fail. The correct behavior would be to retry authentication
with the next best mechanism, which is what this patch does.

The patch isn't specific to GSSAPI, but AFAIK only GSSAPI exhibits the
problem that this patch is trying to fix.

[[[
If a SASL mechanism fails sufficiently early (i.e. before the client
sends the initial response), don't automatically fail the
authentication. Instead, fall back to the next best mechanism sent by
the server.

* subversion/libsvn_ra_svn/sasl_auth.c
  (try_auth): If sasl_client_start() fails with a non-fatal error message,
  delete the current mechanism from the list and try again.
]]]

-- 
Vlad

Re: [PATCH] Make SASL mechanism negotiation smarter

Posted by David Glasser <gl...@mit.edu>.
On 10/11/06, Malcolm Rowe <ma...@farside.org.uk> wrote:
> Okay.  So, sorry if this is a stupid question, but is it therefore not
> possible to compile a SASL-enabled svnserve and point it at a regular
> svnserve password file using svnserve.conf?

No, you have to enable sasl in an individual svnserve.conf file, I think.

--dave

-- 
David Glasser | glasser@mit.edu | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Make SASL mechanism negotiation smarter

Posted by Vlad Georgescu <vg...@gmail.com>.
On 10/11/06, Malcolm Rowe <ma...@farside.org.uk> wrote:
> On Wed, Oct 11, 2006 at 09:07:19PM +0300, Vlad Georgescu wrote:
> > - the SASL anonymous plugin doesn't have anything to do with the
> > internal anonymous/cram-md5 auth; it's just a simple implementation of
> > the SASL plugin API; in fact, the old code (located in
> > libsvn_ra_svn/simple_auth.c) isn't even compiled when SASL is
> > detected; both simple_auth.c and sasl_auth.c implement the function
> > svn_ra_svn__do_auth, but in different ways.
> >
>
> Okay.  So, sorry if this is a stupid question, but is it therefore not
> possible to compile a SASL-enabled svnserve and point it at a regular
> svnserve password file using svnserve.conf?
>
> That is, SASL takes over the entire authn process, and you either have a
> SASL-enabled server that uses SASL, or a non-SASL-enabled server that
> uses the 'passwd' setting in svnserve.conf?
>

Ah. Sorry if I wasn't clear. When you compile Subversion with SASL
support, it's only the client that uses SASL unconditionally. The
server will still authenticate users as before, unless you set
'use-sasl' to 'true' in svnserve.conf, which will make all
authentication requests go through SASL.

-- 
Vlad

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Make SASL mechanism negotiation smarter

Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Wed, Oct 11, 2006 at 08:22:39PM +0100, Malcolm Rowe wrote:
> Yes, there is an (entirely undocumented; could we fix that?) config

Man, I'm having a bad day today.  It's only undocumented if I create the
repository with a 1.4.0 svnadmin...

Sorry,
Malcolm

Re: [PATCH] Make SASL mechanism negotiation smarter

Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Wed, Oct 11, 2006 at 07:44:11PM +0100, Malcolm Rowe wrote:
> On Wed, Oct 11, 2006 at 09:07:19PM +0300, Vlad Georgescu wrote:
> > - the SASL anonymous plugin doesn't have anything to do with the
> > internal anonymous/cram-md5 auth; it's just a simple implementation of
> > the SASL plugin API; in fact, the old code (located in
> > libsvn_ra_svn/simple_auth.c) isn't even compiled when SASL is
> > detected; both simple_auth.c and sasl_auth.c implement the function
> > svn_ra_svn__do_auth, but in different ways.
> > 
> 
> Okay.  So, sorry if this is a stupid question, but is it therefore not
> possible to compile a SASL-enabled svnserve and point it at a regular
> svnserve password file using svnserve.conf?
> 

That seems to be completely incorrect.  Sorry for the noise.
Yes, there is an (entirely undocumented; could we fix that?) config
option in the repository's config file to enable SASL for the server
process.

What's happening in _my_ case is that my SASL-aware-but-not-enabled
svnserve is offering up ANONYMOUS, but my SASL-aware svn client is
unable to handle it.  The assumption, I guess, is that the SASL library
will _always_ be able to negotiate ANONYMOUS and CRAM-MD5, the
mechanisms that a non-SASL-enabled server returns.

(Incidentally, if I do set up a password file on svnserve, the client
works, because it authenticates via the CRAM-MD5 mech).

Regards,
Malcolm

Re: [PATCH] Make SASL mechanism negotiation smarter

Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Wed, Oct 11, 2006 at 09:07:19PM +0300, Vlad Georgescu wrote:
> - the SASL anonymous plugin doesn't have anything to do with the
> internal anonymous/cram-md5 auth; it's just a simple implementation of
> the SASL plugin API; in fact, the old code (located in
> libsvn_ra_svn/simple_auth.c) isn't even compiled when SASL is
> detected; both simple_auth.c and sasl_auth.c implement the function
> svn_ra_svn__do_auth, but in different ways.
> 

Okay.  So, sorry if this is a stupid question, but is it therefore not
possible to compile a SASL-enabled svnserve and point it at a regular
svnserve password file using svnserve.conf?

That is, SASL takes over the entire authn process, and you either have a
SASL-enabled server that uses SASL, or a non-SASL-enabled server that
uses the 'passwd' setting in svnserve.conf?

Regards,
Malcolm

Re: [PATCH] Make SASL mechanism negotiation smarter

Posted by Vlad Georgescu <vg...@gmail.com>.
On 10/11/06, Malcolm Rowe <ma...@farside.org.uk> wrote:
> On Wed, Oct 11, 2006 at 06:53:17PM +0300, Vlad Georgescu wrote:
> > On 10/11/06, Malcolm Rowe <ma...@farside.org.uk> wrote:
> > >$ basic_tests.py --url=svn://localhost:3691 1
> > >../svn-trunk/subversion/libsvn_ra_svn/sasl_auth.c:339: (apr_err=170001)
> > >svn: SASL(-4): no mechanism available: No worthy mechs found
> > >FAIL:  basic_tests.py 1: basic checkout of a wc
> >
> > You shouldn't have to configure anything. A SASL-enabled svn is
> > supposed to be backward compatible with old servers _if_ the ANONYMOUS
> > and CRAM-MD5 plugins are installed on the system. (SASL mechanisms are
> > implemented as plugins which are dynamically loaded at runtime by the
> > main sasl lib). In your case, it's probably caused by a missing
> > ANONYMOUS plugin (there should be a libanonymous.so in
> > /usr/lib/sasl2).
> >
>
> Ah, that'll be it.  I have no libanonymous.so, and unfortunately my
> distro in this case (Slackware) doesn't seem to provide it at all.
>
> I'll ask why not - there could be a good reason, I guess - but it would
> also be nice if we didn't just fall in a heap in this case.
>
> > >[Incidentally, I presume that these are done on a per-service basis or
> > >similar?  We're not going to ship in a mode that suddenly allows every
> > >valid account in /etc/passwd to commit to a repository, are we?]
> >
> > Yes, configuring the range of allowed mechanisms and the
> > password-checking method is done in a configuration file specific to
> > Subversion (usually /usr/lib/sasl2/subversion.conf).
> >
>
> So if I wasn't authenticating via the SASL2 anonymous plugin (which I
> guess then ends up at our internal anonymous or cram-md5 auth?), I would
> have to create a system-wide config file before I could use any of the
> other SASL mechanisms?

To clarify:

- the SASL anonymous plugin doesn't have anything to do with the
internal anonymous/cram-md5 auth; it's just a simple implementation of
the SASL plugin API; in fact, the old code (located in
libsvn_ra_svn/simple_auth.c) isn't even compiled when SASL is
detected; both simple_auth.c and sasl_auth.c implement the function
svn_ra_svn__do_auth, but in different ways.

- the configuration file is only necessary on the server side; its
format is described here:
http://www.sendmail.org/~ca/email/cyrus2/options.html

You might also want to read these first:
http://www.sendmail.org/~ca/email/cyrus2/components.html
http://www.sendmail.org/~ca/email/cyrus2/sysadmin.html

I described a simple setup in my original server-side patch:
http://svn.haxx.se/dev/archive-2006-08/0613.shtml

-- 
Vlad

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Make SASL mechanism negotiation smarter

Posted by Malcolm Rowe <ma...@farside.org.uk>.
On Wed, Oct 11, 2006 at 06:53:17PM +0300, Vlad Georgescu wrote:
> On 10/11/06, Malcolm Rowe <ma...@farside.org.uk> wrote:
> >$ basic_tests.py --url=svn://localhost:3691 1
> >../svn-trunk/subversion/libsvn_ra_svn/sasl_auth.c:339: (apr_err=170001)
> >svn: SASL(-4): no mechanism available: No worthy mechs found
> >FAIL:  basic_tests.py 1: basic checkout of a wc
> 
> You shouldn't have to configure anything. A SASL-enabled svn is
> supposed to be backward compatible with old servers _if_ the ANONYMOUS
> and CRAM-MD5 plugins are installed on the system. (SASL mechanisms are
> implemented as plugins which are dynamically loaded at runtime by the
> main sasl lib). In your case, it's probably caused by a missing
> ANONYMOUS plugin (there should be a libanonymous.so in
> /usr/lib/sasl2).
> 

Ah, that'll be it.  I have no libanonymous.so, and unfortunately my
distro in this case (Slackware) doesn't seem to provide it at all.

I'll ask why not - there could be a good reason, I guess - but it would
also be nice if we didn't just fall in a heap in this case.

> >[Incidentally, I presume that these are done on a per-service basis or
> >similar?  We're not going to ship in a mode that suddenly allows every
> >valid account in /etc/passwd to commit to a repository, are we?]
> 
> Yes, configuring the range of allowed mechanisms and the
> password-checking method is done in a configuration file specific to
> Subversion (usually /usr/lib/sasl2/subversion.conf).
> 

So if I wasn't authenticating via the SASL2 anonymous plugin (which I
guess then ends up at our internal anonymous or cram-md5 auth?), I would
have to create a system-wide config file before I could use any of the
other SASL mechanisms?

> >Can we not fall back to the previous authentication mechanism if SASL
> >doesn't have any matching authentication mechanisms?
> 
> I think it's simpler to just require users to have those plugins
> installed than to complicate the SASL implementation.
> 

I'm not sure I agree, but I'll see if I can find out why Slackware
doesn't ship with that plugin.

Regards,
Malcolm

Re: [PATCH] Make SASL mechanism negotiation smarter

Posted by Vlad Georgescu <vg...@gmail.com>.
On 10/11/06, Malcolm Rowe <ma...@farside.org.uk> wrote:
> I'm also having a problem with the SASL-enabled svnserve - and while it's
> not completely identical to the problem you're fixing here, I suspect
> it's in the same area.
>
> The problem I have is that if I link to the SASL libraries (which happens
> by default), I can no longer establish a non-SASL connection.  That is,
> there appears to be no fallback from the SASL negotiation to the non-SASL
> negotiation (of anonymous, cram-md5).
>
> Specifically, I get the following error:
>
> $ basic_tests.py --url=svn://localhost:3691 1
> ../svn-trunk/subversion/libsvn_ra_svn/sasl_auth.c:339: (apr_err=170001)
> svn: SASL(-4): no mechanism available: No worthy mechs found
> FAIL:  basic_tests.py 1: basic checkout of a wc
>
> Which, arguably, is correct, as I have no configured SASL mechanisms
> that can be used by the client to prove the user's identity to the server.

You shouldn't have to configure anything. A SASL-enabled svn is
supposed to be backward compatible with old servers _if_ the ANONYMOUS
and CRAM-MD5 plugins are installed on the system. (SASL mechanisms are
implemented as plugins which are dynamically loaded at runtime by the
main sasl lib). In your case, it's probably caused by a missing
ANONYMOUS plugin (there should be a libanonymous.so in
/usr/lib/sasl2).

Distros usually put SASL plugins in a separate package from the main
sasl lib (either all plugins in the same package or one package per
plugin). In that case, Subversion package maintainers will probably
have to add a dependency for the ANONYMOUS and CRAM-MD5 package(s), to
ensure backward compatibility.

> [Incidentally, I presume that these are done on a per-service basis or
> similar?  We're not going to ship in a mode that suddenly allows every
> valid account in /etc/passwd to commit to a repository, are we?]

Yes, configuring the range of allowed mechanisms and the
password-checking method is done in a configuration file specific to
Subversion (usually /usr/lib/sasl2/subversion.conf).

> Anyway, the current situation is a not a good user experience, especially
> as I have no reason to enable SASL myself (though I _do_ need the SASL
> libraries to be installed for some other applications).
>
> Can we not fall back to the previous authentication mechanism if SASL
> doesn't have any matching authentication mechanisms?
>

I think it's simpler to just require users to have those plugins
installed than to complicate the SASL implementation.

-- 
Vlad

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Make SASL mechanism negotiation smarter

Posted by Malcolm Rowe <ma...@farside.org.uk>.
Hi Vlad,

On Wed, Oct 11, 2006 at 01:59:04PM +0300, Vlad Georgescu wrote:
> If a SASL mechanism fails sufficiently early (i.e. before the client
> sends the initial response), don't automatically fail the
> authentication. Instead, fall back to the next best mechanism sent by
> the server.
> 

I'm also having a problem with the SASL-enabled svnserve - and while it's
not completely identical to the problem you're fixing here, I suspect
it's in the same area.

The problem I have is that if I link to the SASL libraries (which happens
by default), I can no longer establish a non-SASL connection.  That is,
there appears to be no fallback from the SASL negotiation to the non-SASL
negotiation (of anonymous, cram-md5).

Specifically, I get the following error:

$ basic_tests.py --url=svn://localhost:3691 1
../svn-trunk/subversion/libsvn_ra_svn/sasl_auth.c:339: (apr_err=170001)
svn: SASL(-4): no mechanism available: No worthy mechs found
FAIL:  basic_tests.py 1: basic checkout of a wc

Which, arguably, is correct, as I have no configured SASL mechanisms
that can be used by the client to prove the user's identity to the server.

[Incidentally, I presume that these are done on a per-service basis or
similar?  We're not going to ship in a mode that suddenly allows every
valid account in /etc/passwd to commit to a repository, are we?]

Anyway, the current situation is a not a good user experience, especially
as I have no reason to enable SASL myself (though I _do_ need the SASL
libraries to be installed for some other applications).

Can we not fall back to the previous authentication mechanism if SASL
doesn't have any matching authentication mechanisms?

Regards,
Malcolm

Re: [PATCH] Make SASL mechanism negotiation smarter

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 10/11/06, Vlad Georgescu <vg...@gmail.com> wrote:
> Hi,
>
> This is a fix for a problem originally described here:
> http://svn.haxx.se/dev/archive-2006-09/0994.shtml
>
> Specifically, if GSSAPI is part of the list of mechanisms sent by the
> server, the client will always choose it over the others (because it's
> the most secure) even if it isn't really prepared to use it (e.g.
> because there are no Kerberos credentials). Thus authentication will
> always fail. The correct behavior would be to retry authentication
> with the next best mechanism, which is what this patch does.
>
> The patch isn't specific to GSSAPI, but AFAIK only GSSAPI exhibits the
> problem that this patch is trying to fix.
>
> [[[
> If a SASL mechanism fails sufficiently early (i.e. before the client
> sends the initial response), don't automatically fail the
> authentication. Instead, fall back to the next best mechanism sent by
> the server.
>
> * subversion/libsvn_ra_svn/sasl_auth.c
>   (try_auth): If sasl_client_start() fails with a non-fatal error message,
>   delete the current mechanism from the list and try again.
> ]]]

So, I've been playing with this patch, and it seems generally fine
(still got that weird set of failures in svnsync_tests.py, but that's
not new), but I did notice one problem with the sasl code, and it's
unrelated to this patch.

When I run the svnsync tests with --enable-sasl it's crazy slow.  Like
basic_tests.py usually takes me under a minute, but when I turn sasl
on it takes like 6 minutes to finish.  This seems bad to me...

Anyway, this patch does fix a real problem, so I'll likely commit it
soon, I just wanted to ask if anyone else was seeing the crazy
slowdown with sasl turned on, and if so if they know what's causing
it...

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org