You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@subversion.apache.org by Alec Kloss <al...@SetFilePointer.com> on 2009/03/21 01:06:44 UTC
[Patch] Subversion 1.5 SASL doesn't work correctly with Kerberos
cross-realm authentication
I once pined:
On 2008-08-12 14:00, Alec Kloss wrote:
> First off, Subversion's code is lovely to read.
>
> The subject pretty much says it all. The SASL support in
> Subversion 1.5 blindly removes realm specifiers from the user's
> authentication.
>
> There's a seatbelt early in cyrus_auth.c which protects against
> security problems associated with removing the realm as described
> by this comment:
>
> /* The only valid realm is user_realm (i.e. the repository's realm).
> If the user gave us another realm, complain. */
>
> Later, at the end of cyrus_auth_request() the realm is yanked off
> of the authenticated user, which creates the potential security
> issue that the seatbelt in cyrus_auth.c is protecting against:
>
> if ((p = strchr(user, '@')) != NULL)
> /* Drop the realm part. */
> b->user = apr_pstrndup(b->pool, user, p - (char *)user);
> else
>
> I guess I'd propose changing the default behavior to allow
> cross-realm and strip the realm part off in cyrus_auth_request()
> if-and-only-if it matches the configured "user_realm". I'd like to
> see a flag to disable the stripping of the realm entirely, as
> people with lots of cross-realm will almost certainly prefer that.
>
> (And while I'm commenting, has no one looked into logging in
> svnserve since 2005? It's kinda a big thing to be missing, at
> least among the paranoid.)
Please see attached patch. It works against 1.6.0 and trunk r36738.
--
Alec Kloss alec@SetFilePointer.com IM: daemonalec@gmail.com
PGP key at http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xA241980E
"No Bunny!" -- Simon, http://wiki.adultswim.com/xwiki/bin/Frisky+Dingo/Simon
Re: [Patch] Subversion 1.5 SASL doesn't work correctly with
Kerberos cross-realm authentication
Posted by Gavin Baumanis <ga...@thespidernet.com>.
I have created Issue #3394 and attached the proposed patch.
Gavin.
On 21/03/2009, at 11:07 PM, Alec Kloss wrote:
> I once pined:
>
> On 2008-08-12 14:00, Alec Kloss wrote:
>> First off, Subversion's code is lovely to read.
>>
>> The subject pretty much says it all. The SASL support in
>> Subversion 1.5 blindly removes realm specifiers from the user's
>> authentication.
>>
>> There's a seatbelt early in cyrus_auth.c which protects against
>> security problems associated with removing the realm as described
>> by this comment:
>>
>> /* The only valid realm is user_realm (i.e. the repository's
>> realm).
>> If the user gave us another realm, complain. */
>>
>> Later, at the end of cyrus_auth_request() the realm is yanked off
>> of the authenticated user, which creates the potential security
>> issue that the seatbelt in cyrus_auth.c is protecting against:
>>
>> if ((p = strchr(user, '@')) != NULL)
>> /* Drop the realm part. */
>> b->user = apr_pstrndup(b->pool, user, p - (char *)user);
>> else
>>
>> I guess I'd propose changing the default behavior to allow
>> cross-realm and strip the realm part off in cyrus_auth_request()
>> if-and-only-if it matches the configured "user_realm". I'd like to
>> see a flag to disable the stripping of the realm entirely, as
>> people with lots of cross-realm will almost certainly prefer that.
>>
>> (And while I'm commenting, has no one looked into logging in
>> svnserve since 2005? It's kinda a big thing to be missing, at
>> least among the paranoid.)
>
> Please see attached patch. It works against 1.6.0 and trunk r36738.
>
> --
> Alec Kloss alec@SetFilePointer.com IM: daemonalec@gmail.com
> PGP key at http://pgp.mit.edu:11371/pks/lookup?
> op=get&search=0xA241980E
> "No Bunny!" -- Simon, http://wiki.adultswim.com/xwiki/bin/Frisky+Dingo/Simon
> <sasl-patch.txt>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1605348
Re: bump [Patch] Subversion 1.5 SASL doesn't work correctly with
Kerberos cross-realm authentication
Posted by Alec Kloss <al...@SetFilePointer.com>.
On 2009-03-21 07:07, Alec Kloss wrote:
>
> Please see attached patch. It works against 1.6.0 and trunk r36738.
>
Does anyone have any comments about this cross-realm patch? All I
hear are crickets...
--
Alec Kloss alec@SetFilePointer.com IM: daemonalec@gmail.com
PGP key at http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xA241980E
"No Bunny!" -- Simon, http://wiki.adultswim.com/xwiki/bin/Frisky+Dingo/Simon
Re: [Patch] Subversion 1.5 SASL doesn't work correctly with
Kerberos cross-realm authentication
Posted by Alec Kloss <al...@setfilepointer.com>.
I once pined:
On 2008-08-12 14:00, Alec Kloss wrote:
> First off, Subversion's code is lovely to read.
>
> The subject pretty much says it all. The SASL support in
> Subversion 1.5 blindly removes realm specifiers from the user's
> authentication.
>
> There's a seatbelt early in cyrus_auth.c which protects against
> security problems associated with removing the realm as described
> by this comment:
>
> /* The only valid realm is user_realm (i.e. the repository's realm).
> If the user gave us another realm, complain. */
>
> Later, at the end of cyrus_auth_request() the realm is yanked off
> of the authenticated user, which creates the potential security
> issue that the seatbelt in cyrus_auth.c is protecting against:
>
> if ((p = strchr(user, '@')) != NULL)
> /* Drop the realm part. */
> b->user = apr_pstrndup(b->pool, user, p - (char *)user);
> else
>
> I guess I'd propose changing the default behavior to allow
> cross-realm and strip the realm part off in cyrus_auth_request()
> if-and-only-if it matches the configured "user_realm". I'd like to
> see a flag to disable the stripping of the realm entirely, as
> people with lots of cross-realm will almost certainly prefer that.
>
> (And while I'm commenting, has no one looked into logging in
> svnserve since 2005? It's kinda a big thing to be missing, at
> least among the paranoid.)
Please see attached patch. It works against 1.6.0 and trunk r36738.
--
Alec Kloss alec@SetFilePointer.com IM: daemonalec@gmail.com
PGP key at http://pgp.mit.edu:11371/pks/lookup?op=get&search=0xA241980E
"No Bunny!" -- Simon, http://wiki.adultswim.com/xwiki/bin/Frisky+Dingo/Simon
Re: [Patch] Subversion 1.5 SASL doesn't work correctly with
Kerberos cross-realm authentication
Posted by Gavin Baumanis <ga...@thespidernet.com>.
Hi Lieven,
Thanks - I did check the issue tracker but that was posted in April.
Since this email thread was more recent I thought I would keep it
alive here on the mailing list.
Gavin.
On 06/08/2009, at 03:54 , Lieven Govaerts wrote:
> On Mon, Aug 3, 2009 at 2:07 AM, Gavin 'Beau'
> Baumanis<ga...@thespidernet.com> wrote:
>> Hi Everyone,
>>
>> Ping. This submission has received no further comments in a week.
>>
>> (I have included the DEV mailing list in the reply since it seems
>> more
>> appropriate to discuss a patch submission there than on the USERS
>> list.)
>>
>
> Gavin,
>
> note that this patch isn't new, you already logged it in the issue
> tracker last year. (#3394).
>
> I think it's mostly good, although not yet convinced we really need al
> those options. Won't be able to look at it into more detail before
> September though.
>
> Lieven
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=2380568
>
> To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org
> ].
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2380656
Re: [Patch] Subversion 1.5 SASL doesn't work correctly with
Kerberos cross-realm authentication
Posted by Alec Kloss <al...@oracle.com>.
On 2009-08-05 19:54, Lieven Govaerts wrote:
[chop]
>
> I think it's mostly good, although not yet convinced we really need al
> those options. Won't be able to look at it into more detail before
> September though.
>
> Lieven
Thanks for taking the time to look at the patch. If you've got any
suggestions about changes or quetions about what happens without
one of the options currently in the patch, I'd be happy to adjust
the patch or answer your questions. I'm fine waiting until
September to discuss too.
--
Alec.Kloss@oracle.com Oracle Middleware
PGP key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x432B9956
Re: [Patch] Subversion 1.5 SASL doesn't work correctly with
Kerberos cross-realm authentication
Posted by Lieven Govaerts <sv...@mobsol.be>.
On Mon, Aug 3, 2009 at 2:07 AM, Gavin 'Beau'
Baumanis<ga...@thespidernet.com> wrote:
> Hi Everyone,
>
> Ping. This submission has received no further comments in a week.
>
> (I have included the DEV mailing list in the reply since it seems more
> appropriate to discuss a patch submission there than on the USERS list.)
>
Gavin,
note that this patch isn't new, you already logged it in the issue
tracker last year. (#3394).
I think it's mostly good, although not yet convinced we really need al
those options. Won't be able to look at it into more detail before
September though.
Lieven
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2380567
Re: [Patch] Subversion 1.5 SASL doesn't work correctly with
Kerberos cross-realm authentication
Posted by Gavin Baumanis <ga...@thespidernet.com>.
Hi Everyone,
Ping. This submission has received no further comments in a week.
(I have included the DEV mailing list in the reply since it seems more
appropriate to discuss a patch submission there than on the USERS list.)
Gavin.
On 24/07/2009, at 04:04 , Alec Kloss wrote:
> On 2009-07-23 15:30, Lieven Govaerts wrote:
>> On Sat, Mar 21, 2009 at 3:06 AM, Alec
>> Kloss<al...@setfilepointer.com> wrote:
> [chop]
>>>
>>> Please see attached patch. It works against 1.6.0 and trunk r36738.
>>
>>> [[[
>>>
>>> Add option cross-realm support to cyrus_auth.c. Adds three
>>> boolean configuration entries in the [sasl] section:
>>>
>>> enable-cross-realm: set to enable cross-realm support
>>> remove-local-realm: set to true to remove the local realm
>>> from a user name, false to keep the realm on a local user.
>>> Defaults to !enable-cross-realm
>>> remove-remote-realm: set to true to remove the realm of a remote
>>> user. This is a potential security hazard and should not
>>> be enabled unless you're confident all realms are trustworthy.
>>>
>>> ]]]
>>
>> Why do we need those three flags exactly? Can we not select a safe
>> default?
>>
>> We don't seem to need them for kerberos over http (ra_neon)?
>>
>> Lieven
>>
>
> Short story:
>
> I think that these options are necessary for svnserve to stay
> consistent with its current behavior *and* allow it to be
> configured the way an administrator in a cross-realm environment
> would want it.
>
> Long story:
>
> The safe default is to enable cross realm and never strip the realm
> name. Unfortunately, the current implementation disables cross
> realm and always strips the realm. If you enable cross-realm by
> default and start including the realm, you break everyone's
> existing authz files. If you enable cross-realm and still strip
> the realm, you're created a potential security problem. If you
> stop stripping the realm, everyone's authz files will break when
> users start showing up with @REALM on them. I felt the flags and
> their defaults were closest to what an administrator would want for
> the following reasons:
>
> 1. Administrators of simple configurations don't have to worry
> about any of this cross-realm stuff.
>
> 2. Administrators who need cross-realm can flip a single knob,
> enable-cross-realm to make cross-realm work the way they'd expect.
>
> 3. Administrators who are migrating from a single realm to multiple
> realms can set enable-cross-realm and remove-local-realm. This
> keeps the existing authz file working and they can grant access to
> users from remote realms easily.
>
> 4. Administrators who are extremly trusting or have full control
> over the administration of all realms they have cross-realm trust
> with can set remove-local-realm and remove-remote-realm to use only
> usernames and never realms for authenticating users.
>
>
> Even longer story:
>
> You're almost talking apples and oranges. Nothing in Subversion
> uses plain Kerberos. Over HTTP, Subversion uses SPNEGO (aka
> Negotiate) which happens to normally be backed with Kerberos, and
> svn protocol uses SASL, which supports GSSAPI backed by Kerberos.
>
> In the end though, it is all Kerberos doing the heavy lifting, even
> though Subversion can't really tell that because the Kerberos is
> wrapped up in SPNEGO on http:// or SASL on svn://. Also note that
> ra_neon is the client half of the connection to a server which is
> using mod_auth_kerb, and svnserve is the server half of the
> connection from a subversion client. The more analogous part of
> the connection is mod_auth_kerb, not ra_neon.
>
> Here's the config info for modauthkerb:
>
> http://modauthkerb.sourceforge.net/configure.html
>
> This documentation is a little out of date; in 5.4 there is an
> option, KrbLocalUserMapping, which when turned on (not recommended)
> calls krb5_aname_to_localname. svnserve, right now, always
> disables cross-realm and always does roughly what
> krb5_aname_to_localname does, which is to strip the realm off the
> authenticated name. So, mod_auth_kerb always enables cross-realm,
> so it doesn't have a knob for it. It has a knob like
> remove-local-realm, and then it doesn't seem to have a knob like
> remove-remote-realm, probably because it seems like a bad idea,
> although people do ask for it. If you'd like to simplify the patch
> and get rid of remoe-remote-realm, I'm all for it. If you'd like
> to simplify the patch more and remove the remove-local-realm option
> and change the code to never remove the local realm, I'm okay with
> it, but you'll annoy lots of people when them upgrade and have to
> re-write their authz files. Removing the local-realm by default
> and not providing a config entry to change it is probably forcing
> administrators into a potentially unwise security situation.
>
>
> Hope that helps. Let me know if you have more questions or other
> ideas.
>
> --
> Alec.Kloss@oracle.com Oracle Middleware
> PGP key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x432B9956
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2379363
Re: [Patch] Subversion 1.5 SASL doesn't work correctly with
Kerberos cross-realm authentication
Posted by Alec Kloss <al...@oracle.com>.
On 2009-07-23 15:30, Lieven Govaerts wrote:
> On Sat, Mar 21, 2009 at 3:06 AM, Alec
> Kloss<al...@setfilepointer.com> wrote:
[chop]
> >
> > Please see attached patch. It works against 1.6.0 and trunk r36738.
>
> > [[[
> >
> > Add option cross-realm support to cyrus_auth.c. Adds three
> > boolean configuration entries in the [sasl] section:
> >
> > enable-cross-realm: set to enable cross-realm support
> > remove-local-realm: set to true to remove the local realm
> > from a user name, false to keep the realm on a local user.
> > Defaults to !enable-cross-realm
> > remove-remote-realm: set to true to remove the realm of a remote
> > user. This is a potential security hazard and should not
> > be enabled unless you're confident all realms are trustworthy.
> >
> > ]]]
>
> Why do we need those three flags exactly? Can we not select a safe default?
>
> We don't seem to need them for kerberos over http (ra_neon)?
>
> Lieven
>
Short story:
I think that these options are necessary for svnserve to stay
consistent with its current behavior *and* allow it to be
configured the way an administrator in a cross-realm environment
would want it.
Long story:
The safe default is to enable cross realm and never strip the realm
name. Unfortunately, the current implementation disables cross
realm and always strips the realm. If you enable cross-realm by
default and start including the realm, you break everyone's
existing authz files. If you enable cross-realm and still strip
the realm, you're created a potential security problem. If you
stop stripping the realm, everyone's authz files will break when
users start showing up with @REALM on them. I felt the flags and
their defaults were closest to what an administrator would want for
the following reasons:
1. Administrators of simple configurations don't have to worry
about any of this cross-realm stuff.
2. Administrators who need cross-realm can flip a single knob,
enable-cross-realm to make cross-realm work the way they'd expect.
3. Administrators who are migrating from a single realm to multiple
realms can set enable-cross-realm and remove-local-realm. This
keeps the existing authz file working and they can grant access to
users from remote realms easily.
4. Administrators who are extremly trusting or have full control
over the administration of all realms they have cross-realm trust
with can set remove-local-realm and remove-remote-realm to use only
usernames and never realms for authenticating users.
Even longer story:
You're almost talking apples and oranges. Nothing in Subversion
uses plain Kerberos. Over HTTP, Subversion uses SPNEGO (aka
Negotiate) which happens to normally be backed with Kerberos, and
svn protocol uses SASL, which supports GSSAPI backed by Kerberos.
In the end though, it is all Kerberos doing the heavy lifting, even
though Subversion can't really tell that because the Kerberos is
wrapped up in SPNEGO on http:// or SASL on svn://. Also note that
ra_neon is the client half of the connection to a server which is
using mod_auth_kerb, and svnserve is the server half of the
connection from a subversion client. The more analogous part of
the connection is mod_auth_kerb, not ra_neon.
Here's the config info for modauthkerb:
http://modauthkerb.sourceforge.net/configure.html
This documentation is a little out of date; in 5.4 there is an
option, KrbLocalUserMapping, which when turned on (not recommended)
calls krb5_aname_to_localname. svnserve, right now, always
disables cross-realm and always does roughly what
krb5_aname_to_localname does, which is to strip the realm off the
authenticated name. So, mod_auth_kerb always enables cross-realm,
so it doesn't have a knob for it. It has a knob like
remove-local-realm, and then it doesn't seem to have a knob like
remove-remote-realm, probably because it seems like a bad idea,
although people do ask for it. If you'd like to simplify the patch
and get rid of remoe-remote-realm, I'm all for it. If you'd like
to simplify the patch more and remove the remove-local-realm option
and change the code to never remove the local realm, I'm okay with
it, but you'll annoy lots of people when them upgrade and have to
re-write their authz files. Removing the local-realm by default
and not providing a config entry to change it is probably forcing
administrators into a potentially unwise security situation.
Hope that helps. Let me know if you have more questions or other
ideas.
--
Alec.Kloss@oracle.com Oracle Middleware
PGP key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x432B9956
Re: [Patch] Subversion 1.5 SASL doesn't work correctly with
Kerberos cross-realm authentication
Posted by Lieven Govaerts <sv...@mobsol.be>.
On Sat, Mar 21, 2009 at 3:06 AM, Alec
Kloss<al...@setfilepointer.com> wrote:
> I once pined:
>
> On 2008-08-12 14:00, Alec Kloss wrote:
>> First off, Subversion's code is lovely to read.
>>
>> The subject pretty much says it all. The SASL support in
>> Subversion 1.5 blindly removes realm specifiers from the user's
>> authentication.
>>
>> There's a seatbelt early in cyrus_auth.c which protects against
>> security problems associated with removing the realm as described
>> by this comment:
>>
>> /* The only valid realm is user_realm (i.e. the repository's realm).
>> If the user gave us another realm, complain. */
>>
>> Later, at the end of cyrus_auth_request() the realm is yanked off
>> of the authenticated user, which creates the potential security
>> issue that the seatbelt in cyrus_auth.c is protecting against:
>>
>> if ((p = strchr(user, '@')) != NULL)
>> /* Drop the realm part. */
>> b->user = apr_pstrndup(b->pool, user, p - (char *)user);
>> else
>>
>> I guess I'd propose changing the default behavior to allow
>> cross-realm and strip the realm part off in cyrus_auth_request()
>> if-and-only-if it matches the configured "user_realm". I'd like to
>> see a flag to disable the stripping of the realm entirely, as
>> people with lots of cross-realm will almost certainly prefer that.
>>
>> (And while I'm commenting, has no one looked into logging in
>> svnserve since 2005? It's kinda a big thing to be missing, at
>> least among the paranoid.)
>
> Please see attached patch. It works against 1.6.0 and trunk r36738.
> [[[
>
> Add option cross-realm support to cyrus_auth.c. Adds three
> boolean configuration entries in the [sasl] section:
>
> enable-cross-realm: set to enable cross-realm support
> remove-local-realm: set to true to remove the local realm
> from a user name, false to keep the realm on a local user.
> Defaults to !enable-cross-realm
> remove-remote-realm: set to true to remove the realm of a remote
> user. This is a potential security hazard and should not
> be enabled unless you're confident all realms are trustworthy.
>
> ]]]
Why do we need those three flags exactly? Can we not select a safe default?
We don't seem to need them for kerberos over http (ra_neon)?
Lieven
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=1065&dsMessageId=2374818
To unsubscribe from this discussion, e-mail: [users-unsubscribe@subversion.tigris.org].