SASL + pre-revprop-change == badness?

So, I've been trying to figure out why the svnsync tests
fail over
ra_svn when you turn on the server side sasl support. 
Here's what
I've come up with so far:

1) svnsync tests work fine over ra_svn without sasl

2) with sasl and forking enabled we get errors like this:

['subversion/libsvn_repos/hooks.c:111: (apr_err=165001)n',
"svnsync:
Failed to start
'/home/rooneg/code/svn-trunk/subversion/tests/cmdline/svn-te
st-work/repositories/svnsync_tests-1-1/hooks/pre-revprop-cha
nge'
hookn", 'subversion/libsvn_subr/utf.c:621:
(apr_err=22)n', 'svnsync:
Valid UTF-8 datan', '(hex: 48 6b 0f 08 41 41 41 41 40)n',
'followed
by invalid UTF-8 sequencen', '(hex: 9b 0f 08 a1)n']

3) turn off forking and that particular symptom goes away

4) running svnserve under valgrind with either forking or
threading
and sasl enabled results in dozens of errors, basically
every time we
do anything with the connection pool it complains:

==5067== Invalid read of size 1
==5067==    at 0x401E3AB: memcpy (mac_replace_strmem.c:394)
==5067==    by 0x415F5DF: apr_pstrdup (apr_strings.c:79)
==5067==    by 0x8054421: svn_repos_get_commit_editor4
(commit.c:818)
==5067==    by 0x804F17C: commit (serve.c:962)
==5067==    by 0x80A5299: svn_ra_svn_handle_commands
(marshal.c:838)
==5067==    by 0x805299B: serve (serve.c:2305)
==5067==    by 0x804C237: serve_thread (main.c:253)
==5067==    by 0x4169991: dummy_worker (thread.c:105)
==5067==    by 0x41E6340: start_thread (in
/lib/tls/i686/cmov/libpthread-2.3.6.so)

Turning sasl support off makes all valgrind complaints go
away.

As far as I can tell we don't actually have any problems in
the
svnserve pool handling.  It's only when the --enable-sasl
flag is
passed to the test (i.e. sasl is turned on in svnserve.conf)
that
problems appear, and while they aren't always user visible,
valgrind
seems to see them in all combinations that involve server
side sasl.

And just to throw another weird thing into the mix, any test
that
doesn't do any revprop changes (and thus has no
pre-revprop-change
hook script to call) doesn't show any problems, test passes
fine, and
not a peep from valgrind.

So I'm officially confused.  Anyone got any ideas?

-garrett

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org

On 9/26/06, Garrett Rooney <roonegelectricjellyfish.net>
wrote:

> And just to throw another weird thing into the mix, any
test that
> doesn't do any revprop changes (and thus has no
pre-revprop-change
> hook script to call) doesn't show any problems, test
passes fine, and
> not a peep from valgrind.

Ok, it's weirder.  As suggested by David Glasser on IRC I
tried doing
some manual revprop changes, and those work just fine with
sasl on,
valgrind doesn't say a thing.  So it seems to be just the
sequence of
operations used by svnsync init that causes the problem.

-garrett

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org

On 9/26/06, Garrett Rooney <roonegelectricjellyfish.net>
wrote:
> So, I've been trying to figure out why the svnsync
tests fail over
> ra_svn when you turn on the server side sasl support. 
Here's what
> I've come up with so far:

In trying to track this down, I managed to make a bunch of
dumb
configuration mistakes.  The biggest one was putting the
Subversion.conf file in the wrong place for a pretty long
time.  Is
there a way that we can detect that there isn't actually a
Subversion.conf file?  It doesn't seem like SASL works at
all if there
isn't one, so if use-sasl is true but there's no
configuration file, I
think svnserve shouldn't run.

--dave

-- 
David Glasser | glassermit.edu | http://www.davidglasser.
net/

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org

On 9/28/06, David Glasser <glassermit.edu> wrote:
> On 9/26/06, Garrett Rooney <roonegelectricjellyfish.net> wrote:
> > So, I've been trying to figure out why the svnsync
tests fail over
> > ra_svn when you turn on the server side sasl
support.  Here's what
> > I've come up with so far:
>
> In trying to track this down, I managed to make a bunch
of dumb
> configuration mistakes.  The biggest one was putting
the
> Subversion.conf file in the wrong place for a pretty
long time.  Is
> there a way that we can detect that there isn't
actually a
> Subversion.conf file?  It doesn't seem like SASL works
at all if there
> isn't one, so if use-sasl is true but there's no
configuration file, I
> think svnserve shouldn't run.

Interesting. I would've expected SASL initialization to fail
if there
is no subversion.conf file but apparently, this is not the
case.
Instead I get errors such as this when I try to connect to
the server:
svn: SASL(-1): generic failure: GSSAPI Error: Miscellaneous
failure
(No credentials cache found)

This means that the server is sending a list of all possible
mechanisms, and the client selects GSSAPI because it is the
most
secure. However, the client isn't really configured to use
GSSAPI/Kerberos, so it fails during sasl_client_start()
because there
is no credential cache. The correct behavior in this case
would be to
delete GSSAPI from the list of mechanisms and try again.
I'll start
working on a patch for this right away.

To answer your question, there is no (reliable) way to
detect that
subversion.conf is missing, and, in any case, this is not
what we want
to do, for at least two reasons:
1. People who are using a SASL-enabled svnserve, but aren't
setting
'use-sasl' to 'true' in their repos would be awfully
surprised to see
svnserve fail because subversion.conf could not be found
(subversion.conf? what's that?)
2. Even if the admin set use-sasl to true, but there is no
subversion.conf file, authentication could still succeed if
a) the
admin added the appropriate users to the database and b) we
filtered
out failing mechanisms on the client-side, as described
above. So, for
this use case (authenticating against sasl's own database),
subversion.conf would effectively become optional.

OTOH, I'm not able to reproduce the problem Garrett
described (the
svnsync failures, or the valgrind errors). Everything seems
to work
fine. I'm using Cyrus SASL 2.1.22 on a FC5 box.

-- 
Vlad

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org

On 9/28/06, Vlad Georgescu <vgeorgescugmail.com> wrote:
> On 9/28/06, David Glasser <glassermit.edu> wrote:
> > On 9/26/06, Garrett Rooney <roonegelectricjellyfish.net> wrote:
> > > So, I've been trying to figure out why the
svnsync tests fail over
> > > ra_svn when you turn on the server side sasl
support.  Here's what
> > > I've come up with so far:
> >
> > In trying to track this down, I managed to make a
bunch of dumb
> > configuration mistakes.  The biggest one was
putting the
> > Subversion.conf file in the wrong place for a
pretty long time.  Is
> > there a way that we can detect that there isn't
actually a
> > Subversion.conf file?  It doesn't seem like SASL
works at all if there
> > isn't one, so if use-sasl is true but there's no
configuration file, I
> > think svnserve shouldn't run.
>
> Interesting. I would've expected SASL initialization to
fail if there
> is no subversion.conf file but apparently, this is not
the case.
> Instead I get errors such as this when I try to connect
to the server:
> svn: SASL(-1): generic failure: GSSAPI Error:
Miscellaneous failure
> (No credentials cache found)
>
> This means that the server is sending a list of all
possible
> mechanisms, and the client selects GSSAPI because it is
the most
> secure. However, the client isn't really configured to
use
> GSSAPI/Kerberos, so it fails during sasl_client_start()
because there
> is no credential cache. The correct behavior in this
case would be to
> delete GSSAPI from the list of mechanisms and try
again. I'll start
> working on a patch for this right away.

That seems reasonable.  We shouldn't just fail on the first
mechanism
that doesn't work.

> To answer your question, there is no (reliable) way to
detect that
> subversion.conf is missing, and, in any case, this is
not what we want
> to do, for at least two reasons:
> 1. People who are using a SASL-enabled svnserve, but
aren't setting
> 'use-sasl' to 'true' in their repos would be awfully
surprised to see
> svnserve fail because subversion.conf could not be
found
> (subversion.conf? what's that?)

Agreed that failing based on lack of subversion.conf when
use-sasl is
off would be silly.

> 2. Even if the admin set use-sasl to true, but there is
no
> subversion.conf file, authentication could still
succeed if a) the
> admin added the appropriate users to the database and
b) we filtered
> out failing mechanisms on the client-side, as described
above. So, for
> this use case (authenticating against sasl's own
database),
> subversion.conf would effectively become optional.

I'm still waiting for a justification for why a user that
turned on
sasl in svnserve.conf but didn't condfigure subversion.conf
should do
anything other than deny all access to the system.  Just
defaulting to
all the SASL mechanisms (does that include ANONYMOUS?  That
would be
bad) seems incredibly bad to me.

> OTOH, I'm not able to reproduce the problem Garrett
described (the
> svnsync failures, or the valgrind errors). Everything
seems to work
> fine. I'm using Cyrus SASL 2.1.22 on a FC5 box.

FWIW, David appears to be able to make this problem happen
on Mac OS
X, so it's not just me 

Could you try compiling your APR with --enable-pool-debug,
and then
run svnserve under valgrind for the tests and see if you get
any weird
errors?  I use something like:

valgrind subversion/svnserve/svnserve -d --foreground -r
`pwd`/subversion/tests/cmdline

-garrett

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org

On 9/28/06, Garrett Rooney <roonegelectricjellyfish.net>
wrote:
> On 9/28/06, Vlad Georgescu <vgeorgescugmail.com> wrote:
> > On 9/28/06, David Glasser <glassermit.edu> wrote:
> > > On 9/26/06, Garrett Rooney <roonegelectricjellyfish.net> wrote:
> > > > So, I've been trying to figure out why
the svnsync tests fail over
> > > > ra_svn when you turn on the server side
sasl support.  Here's what
> > > > I've come up with so far:
> > >
> > > In trying to track this down, I managed to
make a bunch of dumb
> > > configuration mistakes.  The biggest one was
putting the
> > > Subversion.conf file in the wrong place for a
pretty long time.  Is
> > > there a way that we can detect that there
isn't actually a
> > > Subversion.conf file?  It doesn't seem like
SASL works at all if there
> > > isn't one, so if use-sasl is true but there's
no configuration file, I
> > > think svnserve shouldn't run.
> >
> > Interesting. I would've expected SASL
initialization to fail if there
> > is no subversion.conf file but apparently, this is
not the case.
> > Instead I get errors such as this when I try to
connect to the server:
> > svn: SASL(-1): generic failure: GSSAPI Error:
Miscellaneous failure
> > (No credentials cache found)
> >
> > This means that the server is sending a list of
all possible
> > mechanisms, and the client selects GSSAPI because
it is the most
> > secure. However, the client isn't really
configured to use
> > GSSAPI/Kerberos, so it fails during
sasl_client_start() because there
> > is no credential cache. The correct behavior in
this case would be to
> > delete GSSAPI from the list of mechanisms and try
again. I'll start
> > working on a patch for this right away.
>
> That seems reasonable.  We shouldn't just fail on the
first mechanism
> that doesn't work.
>
> > To answer your question, there is no (reliable)
way to detect that
> > subversion.conf is missing, and, in any case, this
is not what we want
> > to do, for at least two reasons:
> > 1. People who are using a SASL-enabled svnserve,
but aren't setting
> > 'use-sasl' to 'true' in their repos would be
awfully surprised to see
> > svnserve fail because subversion.conf could not be
found
> > (subversion.conf? what's that?)
>
> Agreed that failing based on lack of subversion.conf
when use-sasl is
> off would be silly.
>
> > 2. Even if the admin set use-sasl to true, but
there is no
> > subversion.conf file, authentication could still
succeed if a) the
> > admin added the appropriate users to the database
and b) we filtered
> > out failing mechanisms on the client-side, as
described above. So, for
> > this use case (authenticating against sasl's own
database),
> > subversion.conf would effectively become optional.
>
> I'm still waiting for a justification for why a user
that turned on
> sasl in svnserve.conf but didn't condfigure
subversion.conf should do
> anything other than deny all access to the system. 
Just defaulting to
> all the SASL mechanisms (does that include ANONYMOUS? 
That would be
> bad) seems incredibly bad to me.

All our restrictions would still be enforced (this means no
ANONYMOUS
if auth-access is none etc.). Besides, this situation (SASL
sending
all mechanisms) also happens if there is no mech_list option
in
subversion.conf, which is perfectly OK. And there is no way
we can
reliably detect the absence of subversion.conf, so we don't
really
have a choice.

The fix I was proposing (falling back to other mechanisms if
GSSAPI
fails) seems like a reasonable way to deal with these cases.
The
jabber guys seem to agree with me:

http://j2.openaether.org/mediawiki/i
ndex.php/CyrusSASL#Client_considerations

> > OTOH, I'm not able to reproduce the problem
Garrett described (the
> > svnsync failures, or the valgrind errors).
Everything seems to work
> > fine. I'm using Cyrus SASL 2.1.22 on a FC5 box.
>
> FWIW, David appears to be able to make this problem
happen on Mac OS
> X, so it's not just me 
>
> Could you try compiling your APR with
--enable-pool-debug, and then
> run svnserve under valgrind for the tests and see if
you get any weird
> errors?  I use something like:
>
> valgrind subversion/svnserve/svnserve -d --foreground
-r
> `pwd`/subversion/tests/cmdline
>

OK, I'll try again with pool debugging enabled. Although
it's weird
that I didn't get any test failures, either.

-- 
Vlad

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org

On 9/28/06, Vlad Georgescu <vgeorgescugmail.com> wrote:

> All our restrictions would still be enforced (this
means no ANONYMOUS
> if auth-access is none etc.). Besides, this situation
(SASL sending
> all mechanisms) also happens if there is no mech_list
option in
> subversion.conf, which is perfectly OK. And there is no
way we can
> reliably detect the absence of subversion.conf, so we
don't really
> have a choice.
>
> The fix I was proposing (falling back to other
mechanisms if GSSAPI
> fails) seems like a reasonable way to deal with these
cases. The
> jabber guys seem to agree with me:
>
> http://j2.openaether.org/mediawiki/i
ndex.php/CyrusSASL#Client_considerations

Ok, works for me.

> > > OTOH, I'm not able to reproduce the problem
Garrett described (the
> > > svnsync failures, or the valgrind errors).
Everything seems to work
> > > fine. I'm using Cyrus SASL 2.1.22 on a FC5
box.
> >
> > FWIW, David appears to be able to make this
problem happen on Mac OS
> > X, so it's not just me 
> >
> > Could you try compiling your APR with
--enable-pool-debug, and then
> > run svnserve under valgrind for the tests and see
if you get any weird
> > errors?  I use something like:
> >
> > valgrind subversion/svnserve/svnserve -d
--foreground -r
> > `pwd`/subversion/tests/cmdline
> >
>
> OK, I'll try again with pool debugging enabled.
Although it's weird
> that I didn't get any test failures, either.

Not all that weird.  If it's a memory corruption problem
it's bound to
be intermitent.

-garrett

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org