You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <be...@qqmail.nl> on 2013/06/20 15:30:12 UTC

[Patch] Adding NTLM Support to Serf - Work in progress / Subversion regression

When Subversion used neon until 1.8 was released it supported the Windows
Integrated Security Scheme called 'NTLM'. This same scheme was also
supported in the ra_serf authentication framework we used before requiring
Serf 0.7. (Read: Before Subversion 1.7). But now after releasing Subversion
1.8 we found out that this scheme is broken.

Many Subversion repositories on Windows now use a configuration like:


<Location /svn>
  AuthName "My Subversion Repositories"
  AuthType SSPI

  SSPIAuth On
  SSPIAuthoritative On
  SSPIDomain MyDomain
  SSPIOmitDomain On
  SSPIOfferBasic On
  SSPIUsernameCase lower

  Require valid-user
  SSLRequireSSL

  DAV svn
  SVNListParentPath on
  SVNParentPath D:/Databases/Subversion/repos
  AuthzSVNAccessFile D:/Databases/Subversion/etc/subversion.xs
</Location>

But this configuration that worked fine in Subversion 1.5, 1.6 and 1.7
doesn't provide automatic login any more in 1.8.

(See the 'Automatic windows authentication using serf/svn 1.8' thread on
users@s.a.o).


Subversion 1.8 just asks for a username and password with this configuration
and after that works correctly... but that is not an automatic login.


Ivan suggested adding the flag 'SSPIPackage Negotiate' in the config as that
would fix Subversion 1.8.

I can confirm that adding this flag fixes the configuration above in at
least 1 setup, but it also completely breaks all Subversion 1.7 and older
clients that depend on neon.


While this may be caused by bugs in mod_auth_sspi (as Ivan) suggests I don't
think we should just break this existing scenario.
(The 'Buy Visual SVN Enterprise option' or 'develop a new mod_auth_sspi'
options are not really viable to me)


The patch to serf 1.2.1 attached to this mail is a (tiny bit cleaned up)
hack based on the old code in ra_serf, some code from an old serf branch and
the new in serf auth_kerb code, which re-enables the NTLM authentication
scheme in serf.


I think we should work to get this code cleaned up for releasing in a new
serf, to resolve this regression in Subversion.

	Bert




Re: [serf-dev] [Patch] Adding NTLM Support to Serf - Work in progress / Subversion regression

Posted by Mark Phippard <ma...@gmail.com>.
On Thu, Jun 20, 2013 at 11:27 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> Mark,
>
> Wearing my VisualSVN CTO hat:
> VisualSVN is a commercial company. And CollabNET is a commercial
> company. That's not a surprise.
>
> And it's not a surprise that VisualSVN Server authentication works
> fine after upgrade to Subversion 1.8. We have spent months with
> testing and debugging. It will be surprise if you haven't do the same
> for all configurations that you support. There were huge changes in
> area of authentication. Neon was removed, at least!

Honestly, I am glad to see you finally put your hat on.  I have been
intentionally refraining from accusing of you vetoing this for
VisualSVN reasons.

CollabNet has no factor in this at all.  We do not support SSPI in any
of our products.  Though we have customers that would like to have
automatic authentication, none of them do as we do not provide or
support it.  The only hat I am wearing here is as a longtime community
member that knows from forum activity there are a lot of people that
use this.  In my past life, where we used Windows on our servers, we
used this.  This would have been back in SVN 1.3 days.

> Also please believe that all my technical thoughts are fair and
> related to technical issues only. My veto above is a technical veto.

That is not at all how it sounded.  Your reply below is much better
but the original veto sounded like were simply opposed to supporting
NTLM.  Again, your reply below in contrast only makes that more clear.

> I may agree that this problem should be fixed. But it should be fixed
> in the proper place and in the proper way.

No objections from me on that.

>
> Did you read my email above? Let me explain again. Server advertises
> supported authentication schemes, for example Microsoft IIS:
> [[[
> C: GET / HTTP/1.1
> S: HTTP/1.1 401 Authorization Required
> S: WWW-Authenticate: NTLM
> S: WWW-Authenticate: Negotiate
> S: WWW-Authenticate: Basic
> ]]]
>
> Then client decide which authentication scheme to use. Serf tries to
> Negotiate, then Basic. But with proposed patch serf will try
> Negotiate, then Basic and then NTLM again, which is wrong because NTLM
> authentication already handled inside Negotiate protocol. This is one
> of the reasons why I vetoed this patch.

OK, sounds like a valid objection.

>
> My point is that serf should use NTLM *only* if server doesn't
> advertise support Negotiate.
>
> mod_auth_sspi is too simple and supports only one auth scheme which is
> controlled by SSPIPackage configuration directive. For example with
> "SSPIPackage NTLM" it offer only NTLM:
> [[[
> C: GET / HTTP/1.1
> S: HTTP/1.1 401 Authorization Required
> S: WWW-Authenticate: NTLM
> S: WWW-Authenticate: Basic
> ]]]
>
> Proper solution is to reconfigure mod_auth_sspi to use Negotiate auth
> scheme using "SSPIPackage Negotiate" directive. It this case
> mod_auth_sspi response will be:
> [[[
> C: GET / HTTP/1.1
> S: HTTP/1.1 401 Authorization Required
> S: WWW-Authenticate: Negotiate
> S: WWW-Authenticate: Basic
> ]]]
>
> And serf will handle it. But neon doesn't work in this case :) Why
> you're not fixing neon?

Now you are being ridiculous again.  Maybe Neon ought to be fixed but
that would not change the fact that clients which are working
perfectly well would suddenly stop working.

> You didn't understand properly my second point about duplicating
> authentication code that already there in auth/auth_kerb.c (it's
> actually should be auth_rfc4559.c). It's not aesthetic issue, because
> auth/auth_kerb.c handles a lot of different cases that could happen
> during rfc4559 style authentications. This code is platform
> independent and tested with various servers like mod_auth_kerb, Apache
> TomCat, IIS, VisualSVN Server and others. And NTLM implementation in
> serf should reuse this tested and complicated code in auth_kerb.c.
>
> I'm going to try to plug NTLM properly in the current serf auth
> architecture on the next week.

That sounds great.  If Serf "prefers" negotiate over NTLM that sounds
fine to me.  But if the server is only offering NTLM it ought to work
if it is possible.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

RE: [serf-dev] [Patch] Adding NTLM Support to Serf - Work in progress / Subversion regression

Posted by Bert Huijben <be...@qqmail.nl>.
From: kmradke@rockwellcollins.com [mailto:kmradke@rockwellcollins.com] 
> > Did you read my email above? Let me explain again. Server advertises
> > supported authentication schemes, for example Microsoft IIS:
> > [[[
> > C: GET / HTTP/1.1
> > S: HTTP/1.1 401 Authorization Required
> > S: WWW-Authenticate: NTLM
> > S: WWW-Authenticate: Negotiate
> > S: WWW-Authenticate: Basic
> > ]]]
> > 
> > Then client decide which authentication scheme to use. Serf tries to
> > Negotiate, then Basic. But with proposed patch serf will try
> > Negotiate, then Basic and then NTLM again, which is wrong because NTLM
> > authentication already handled inside Negotiate protocol. This is one
> > of the reasons why I vetoed this patch.
> > 
> > My point is that serf should use NTLM *only* if server doesn't
> > advertise support Negotiate.
> 
> The problem is that most clients will choose Kerberos instead of 
> NTLM during the Negotiate phase (which they should because it is 
> considered more secure), but there may be things that cause 
> Kerberos to fail, such as mis-configure SPNs in active directory 
> or a mis-configured Java VM options on the client.  In most of 
> these cases NTLM "just works", but the serf client never tries it 
> like neon did.  This is why most installations will be set to 
> offer NTLM only and not Negotiate. 
> 
> As stated, the correct action is to have Serf use NTLM if that 
> is the only thing the server offers.  Not just fail like 
> it does today. 

+1

[Quoting Ivan]
> > I'm going to try to plug NTLM properly in the current serf auth
> > architecture on the next week.

+10

I have a slightly cleaned up version of the original patch on
http://b.qqn.nl/f/serf-ntlm.patch
for if you would like to use that patch as a starting point.
(I plugged some memory and handle leaks in the old ra_serf code that I
copied and wrapped errors to apr errors instead of the generic APR_EGENERAL)

	Bert
> 
> Kevin R.
>


Re: [serf-dev] [Patch] Adding NTLM Support to Serf - Work in progress / Subversion regression

Posted by km...@rockwellcollins.com.
> Did you read my email above? Let me explain again. Server advertises
> supported authentication schemes, for example Microsoft IIS:
> [[[
> C: GET / HTTP/1.1
> S: HTTP/1.1 401 Authorization Required
> S: WWW-Authenticate: NTLM
> S: WWW-Authenticate: Negotiate
> S: WWW-Authenticate: Basic
> ]]]
> 
> Then client decide which authentication scheme to use. Serf tries to
> Negotiate, then Basic. But with proposed patch serf will try
> Negotiate, then Basic and then NTLM again, which is wrong because NTLM
> authentication already handled inside Negotiate protocol. This is one
> of the reasons why I vetoed this patch.
> 
> My point is that serf should use NTLM *only* if server doesn't
> advertise support Negotiate.

The problem is that most clients will choose Kerberos instead of
NTLM during the Negotiate phase (which they should because it is
considered more secure), but there may be things that cause
Kerberos to fail, such as mis-configure SPNs in active directory
or a mis-configured Java VM options on the client.  In most of
these cases NTLM "just works", but the serf client never tries it
like neon did.  This is why most installations will be set to
offer NTLM only and not Negotiate.

As stated, the correct action is to have Serf use NTLM if that
is the only thing the server offers.  Not just fail like
it does today.

Kevin R.

Re: [serf-dev] [Patch] Adding NTLM Support to Serf - Work in progress / Subversion regression

Posted by Lieven Govaerts <li...@gmail.com>.
On Thu, Jun 20, 2013 at 5:27 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> Mark,
>
..
> Also please believe that all my technical thoughts are fair and
> related to technical issues only. My veto above is a technical veto.

I personally never doubted this based on your first mail, but that's
just because I know serf auth's framework so I recognized what you
were trying to see.

> I may agree that this problem should be fixed. But it should be fixed
> in the proper place and in the proper way.
>
..

> Did you read my email above? Let me explain again. Server advertises
> supported authentication schemes, for example Microsoft IIS:
> [[[
> C: GET / HTTP/1.1
> S: HTTP/1.1 401 Authorization Required
> S: WWW-Authenticate: NTLM
> S: WWW-Authenticate: Negotiate
> S: WWW-Authenticate: Basic
> ]]]
>
> Then client decide which authentication scheme to use. Serf tries to
> Negotiate, then Basic. But with proposed patch serf will try
> Negotiate, then Basic and then NTLM again, which is wrong because NTLM
> authentication already handled inside Negotiate protocol. This is one
> of the reasons why I vetoed this patch.


>From RFC 2617:
"The user agent MUST choose to use one of the challenges with the
strongest auth-scheme it understands and request credentials from the
user based upon that
 challenge."

Serf now parses the WWW-Authenticate headers in order of appearance,
so we rely on the server being configured to add the most secure
scheme's header first, which from your example is clearly not
guaranteed.

We should make the order in which serf matches authentication schemes
explicit in both code and the documentation: negotiate, ntlm, digest,
basic.

> My point is that serf should use NTLM *only* if server doesn't
> advertise support Negotiate.

Hm, this statement should be more precise: per the ordering rules I've
noted above for your example, serf should try Negotiate first if it's
compiled in. If Negotiate fails to negotiate an authn scheme (kerb5,
ntlm or...), serf should try NTLM. If that fails fall back basic.

> mod_auth_sspi is too simple and supports only one auth scheme which is
> controlled by SSPIPackage configuration directive. For example with
> "SSPIPackage NTLM" it offer only NTLM:
> [[[
> C: GET / HTTP/1.1
> S: HTTP/1.1 401 Authorization Required
> S: WWW-Authenticate: NTLM
> S: WWW-Authenticate: Basic
> ]]]
>
> Proper solution is to reconfigure mod_auth_sspi to use Negotiate auth
> scheme using "SSPIPackage Negotiate" directive. It this case
> mod_auth_sspi response will be:
> [[[
> C: GET / HTTP/1.1
> S: HTTP/1.1 401 Authorization Required
> S: WWW-Authenticate: Negotiate
> S: WWW-Authenticate: Basic
> ]]]
>
> And serf will handle it. But neon doesn't work in this case :) Why
> you're not fixing neon?

This actually surprises me, neon has been supporting negotiate and
ntlm for years.

> You didn't understand properly my second point about duplicating
> authentication code that already there in auth/auth_kerb.c (it's
> actually should be auth_rfc4559.c). It's not aesthetic issue, because
> auth/auth_kerb.c handles a lot of different cases that could happen
> during rfc4559 style authentications. This code is platform
> independent and tested with various servers like mod_auth_kerb, Apache
> TomCat, IIS, VisualSVN Server and others. And NTLM implementation in
> serf should reuse this tested and complicated code in auth_kerb.c.

Agreed. We can't blame Bert for not recognising that most of the
auth_kerb_sspi .c code can be reused for NTLM, at the minimum the
filename doesn't match with the code it contains.

> I'm going to try to plug NTLM properly in the current serf auth
> architecture on the next week.
>

Great. Good luck. :)

For the record, I'm personally not planning to add NTLM to the unix
builds, so Ivan's work will be Windows only.

Lieven

> --
> Ivan Zhakov
>
> --
> You received this message because you are subscribed to the Google Groups "Serf Development List" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to serf-dev+unsubscribe@googlegroups.com.
> To post to this group, send email to serf-dev@googlegroups.com.
> Visit this group at http://groups.google.com/group/serf-dev.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

Re: [serf-dev] [Patch] Adding NTLM Support to Serf - Work in progress / Subversion regression

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Fri, Jun 21, 2013 at 1:51 PM, Greg Stein <gs...@gmail.com> wrote:
> On Fri, Jun 21, 2013 at 5:34 AM, Branko Čibej <br...@wandisco.com> wrote:
>> On 20.06.2013 17:27, Ivan Zhakov wrote:
>>...
>>> Also please believe that all my technical thoughts are fair and
>>> related to technical issues only. My veto above is a technical veto.
>>
>> Yep, it was. And I still maintain it's invalid, or at least, too naïve.
>
> Invariably, it is *always* a poor choice to debate whether a veto is
> valid or not. The veto exists as a unilateral lever against
> introducing problems into a codebase. The community doesn't get to
> debate the *validity*; it should work to find a solution instead.
>
> If one/more people truly feel that the veto process is being abused by
> an individual, then the conversation should move to the private@ list
> and discuss the removal of that person from the PMC (and, thus, their
> binding vote/veto). That is the kind of bar you must meet.
>
> The above is the meta discussion. In short: second-guess yourself if
> you ever want to debate the validity of a veto.
>
> Now on to the concrete situation. "Is this technical?" Sure is. Ivan
> has some considerations about code duplication, about standards
> conformance, etc. So, done and done: it's a technical veto. Deal.
>
> Second: it isn't even related to Subversion. We're talking about the
> serf codebase, and (frankly) this community doesn't govern that
> codebase. Further, it doesn't *have* to follow the ASF [voting]
> guidelines (tho we've had some minor discussion about moving to the
> ASF).
>
> Third: regardless of what Ivan has stated, we'll fix the NTLM issue.
> (IMO) we should not require server configuration changes simply
> because a client upgraded to 1.8. And from my reading of this thread,
> it also seems that "fixing" the config might break 1.7/neon users.
>
I've implemented plain NTLM authentication in r1963. While change
itself is not big, it required some refactoring/changing internal API.
So it's better to release it as serf 1.3.0, not 1.2.x.


-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com

Re: [serf-dev] [Patch] Adding NTLM Support to Serf - Work in progress / Subversion regression

Posted by Greg Stein <gs...@gmail.com>.
On Fri, Jun 21, 2013 at 5:34 AM, Branko Čibej <br...@wandisco.com> wrote:
> On 20.06.2013 17:27, Ivan Zhakov wrote:
>...
>> Also please believe that all my technical thoughts are fair and
>> related to technical issues only. My veto above is a technical veto.
>
> Yep, it was. And I still maintain it's invalid, or at least, too naïve.

Invariably, it is *always* a poor choice to debate whether a veto is
valid or not. The veto exists as a unilateral lever against
introducing problems into a codebase. The community doesn't get to
debate the *validity*; it should work to find a solution instead.

If one/more people truly feel that the veto process is being abused by
an individual, then the conversation should move to the private@ list
and discuss the removal of that person from the PMC (and, thus, their
binding vote/veto). That is the kind of bar you must meet.

The above is the meta discussion. In short: second-guess yourself if
you ever want to debate the validity of a veto.

Now on to the concrete situation. "Is this technical?" Sure is. Ivan
has some considerations about code duplication, about standards
conformance, etc. So, done and done: it's a technical veto. Deal.

Second: it isn't even related to Subversion. We're talking about the
serf codebase, and (frankly) this community doesn't govern that
codebase. Further, it doesn't *have* to follow the ASF [voting]
guidelines (tho we've had some minor discussion about moving to the
ASF).

Third: regardless of what Ivan has stated, we'll fix the NTLM issue.
(IMO) we should not require server configuration changes simply
because a client upgraded to 1.8. And from my reading of this thread,
it also seems that "fixing" the config might break 1.7/neon users.

Lieven has suggested a fix/approach, and we'll work through that over
the weekend. He's got some time this weekend to work on the issue, and
I'll try to support his time/work. Unfortunately, the proper change
may require a bump to serf 1.3.

Given this, and other breakages in 1.8.0, it looks like we should be
prepping for a patch release next week.

Cheers,
-g

Re: [serf-dev] [Patch] Adding NTLM Support to Serf - Work in progress / Subversion regression

Posted by Branko Čibej <br...@wandisco.com>.
On 20.06.2013 17:27, Ivan Zhakov wrote:
> Mark,
>
> Wearing my VisualSVN CTO hat:
> VisualSVN is a commercial company. And CollabNET is a commercial
> company. That's not a surprise.
>
> And it's not a surprise that VisualSVN Server authentication works
> fine after upgrade to Subversion 1.8. We have spent months with
> testing and debugging.

You just dropped a big pile of some very bad-smelling stuff here. Do you
really think what you wrote above has /any/ place on this list? Please
think twice before replying.

> Also please believe that all my technical thoughts are fair and
> related to technical issues only. My veto above is a technical veto.

Yep, it was. And I still maintain it's invalid, or at least, too naïve.

-- Brane


-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com

Re: [serf-dev] [Patch] Adding NTLM Support to Serf - Work in progress / Subversion regression

Posted by Ivan Zhakov <iv...@visualsvn.com>.
Mark,

Wearing my VisualSVN CTO hat:
VisualSVN is a commercial company. And CollabNET is a commercial
company. That's not a surprise.

And it's not a surprise that VisualSVN Server authentication works
fine after upgrade to Subversion 1.8. We have spent months with
testing and debugging. It will be surprise if you haven't do the same
for all configurations that you support. There were huge changes in
area of authentication. Neon was removed, at least!

Also please believe that all my technical thoughts are fair and
related to technical issues only. My veto above is a technical veto.

I may agree that this problem should be fixed. But it should be fixed
in the proper place and in the proper way.

Switching back to serf commiter hat and technical details.

On Thu, Jun 20, 2013 at 6:00 PM, Mark Phippard <ma...@gmail.com> wrote:
> On Thu, Jun 20, 2013 at 9:52 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>> On Thu, Jun 20, 2013 at 5:44 PM, Mark Phippard <ma...@gmail.com> wrote:
>>> On Thu, Jun 20, 2013 at 9:40 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>>> On Thu, Jun 20, 2013 at 5:30 PM, Bert Huijben <be...@qqmail.nl> wrote:
>>>> [...]
>>>>
>>>>> The patch to serf 1.2.1 attached to this mail is a (tiny bit cleaned up)
>>>>> hack based on the old code in ra_serf, some code from an old serf branch and
>>>>> the new in serf auth_kerb code, which re-enables the NTLM authentication
>>>>> scheme in serf.
>>>>>
>>>>>
>>>> I'm -1 for such patch:
>>>> * It duplicates auth_kerb.c which intended to have the same auth code
>>>> on different platforms with plugable platforms specific code
>>>>
>>>> * serf should not try use NTLM authentication if server supports Negotiate.
>>>
>>> So you are saying you do not think Serf should support mod_auth_sspi
>>> and do not consider this a regression?  Could you explain that
>>> position with more detail?
>> Mark,
>>
>> You didn't understand me. There are two HTTP authentication schemes
>> for automatic authentication:
>> * NTLM
>>   Uses Windows NTLM authentication
>>
>> * Negotiate (SPNEGO)
>>   Uses NTLM or Kerberos depending of what is supported by server and client.
>>
>> NTLM is not documented AFAIK, while Negotiate (SPNEGO) is documented
>> by RFC 4559 [1]
>>
>> Serf supports only Negotiate authentication schemes. Which
>> automatically provides you NTLM or Kerberos.
>>
>> mod_auth_sspi can be configured to use Negotiate protocol using
>> "SSPIPackage Negotiate" server side directive. Bert reported that with
>> "SSPIPackage Negotiate" is working fine, but neon doesn't.
>>
>> My position is that serf should use only Negotiate authentication
>> scheme if server supports both NTLM and Negotiate authentication
>> schemes.
>
> If existing 1.7, 1.6 etc clients do not support this, then your
> position is untenable, one might even say ludicrous.  That is why I am
> asking for more explanation.  Surely this cannot be what you are
> saying?
>
> We can all agree we have a significant number of existing users using
> an automatic authentication method with Windows.  I am calling that
> mod_auth_sspi.  I guess to use your terms, that means NTLM.  Are any
> of these users using the SSPI negotiate option?  If our pre-1.8
> clients do not support that option then I would have to say No.
>
> I fail to see how you can justify a veto here.
>

Did you read my email above? Let me explain again. Server advertises
supported authentication schemes, for example Microsoft IIS:
[[[
C: GET / HTTP/1.1
S: HTTP/1.1 401 Authorization Required
S: WWW-Authenticate: NTLM
S: WWW-Authenticate: Negotiate
S: WWW-Authenticate: Basic
]]]

Then client decide which authentication scheme to use. Serf tries to
Negotiate, then Basic. But with proposed patch serf will try
Negotiate, then Basic and then NTLM again, which is wrong because NTLM
authentication already handled inside Negotiate protocol. This is one
of the reasons why I vetoed this patch.

My point is that serf should use NTLM *only* if server doesn't
advertise support Negotiate.

mod_auth_sspi is too simple and supports only one auth scheme which is
controlled by SSPIPackage configuration directive. For example with
"SSPIPackage NTLM" it offer only NTLM:
[[[
C: GET / HTTP/1.1
S: HTTP/1.1 401 Authorization Required
S: WWW-Authenticate: NTLM
S: WWW-Authenticate: Basic
]]]

Proper solution is to reconfigure mod_auth_sspi to use Negotiate auth
scheme using "SSPIPackage Negotiate" directive. It this case
mod_auth_sspi response will be:
[[[
C: GET / HTTP/1.1
S: HTTP/1.1 401 Authorization Required
S: WWW-Authenticate: Negotiate
S: WWW-Authenticate: Basic
]]]

And serf will handle it. But neon doesn't work in this case :) Why
you're not fixing neon?

You didn't understand properly my second point about duplicating
authentication code that already there in auth/auth_kerb.c (it's
actually should be auth_rfc4559.c). It's not aesthetic issue, because
auth/auth_kerb.c handles a lot of different cases that could happen
during rfc4559 style authentications. This code is platform
independent and tested with various servers like mod_auth_kerb, Apache
TomCat, IIS, VisualSVN Server and others. And NTLM implementation in
serf should reuse this tested and complicated code in auth_kerb.c.

I'm going to try to plug NTLM properly in the current serf auth
architecture on the next week.

-- 
Ivan Zhakov

Re: [serf-dev] [Patch] Adding NTLM Support to Serf - Work in progress / Subversion regression

Posted by Stefan Sperling <st...@elego.de>.
On Thu, Jun 20, 2013 at 04:24:14PM +0200, Branko Čibej wrote:
> On 20.06.2013 16:00, Mark Phippard wrote:
> > On Thu, Jun 20, 2013 at 9:52 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> > I fail to see how you can justify a veto here.
> 
> I have to agree. The veto is fine on aesthetic grounds but kind of fails
> to take account of reality.

+1. I'm under the same impression. I don't really understand the
protocols involved, and don't know anything about various
implementations which might be buggy or what not... but I see a
serious problem if an authentication scheme that used to work just
fine with 1.7 clients suddently stops working with 1.8 for no reason
other than "serf only supports a newer protocol".

Re: [serf-dev] [Patch] Adding NTLM Support to Serf - Work in progress / Subversion regression

Posted by Branko Čibej <br...@wandisco.com>.
On 20.06.2013 16:00, Mark Phippard wrote:
> On Thu, Jun 20, 2013 at 9:52 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>> On Thu, Jun 20, 2013 at 5:44 PM, Mark Phippard <ma...@gmail.com> wrote:
>>> On Thu, Jun 20, 2013 at 9:40 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>>> On Thu, Jun 20, 2013 at 5:30 PM, Bert Huijben <be...@qqmail.nl> wrote:
>>>> [...]
>>>>
>>>>> The patch to serf 1.2.1 attached to this mail is a (tiny bit cleaned up)
>>>>> hack based on the old code in ra_serf, some code from an old serf branch and
>>>>> the new in serf auth_kerb code, which re-enables the NTLM authentication
>>>>> scheme in serf.
>>>>>
>>>>>
>>>> I'm -1 for such patch:
>>>> * It duplicates auth_kerb.c which intended to have the same auth code
>>>> on different platforms with plugable platforms specific code
>>>>
>>>> * serf should not try use NTLM authentication if server supports Negotiate.
>>> So you are saying you do not think Serf should support mod_auth_sspi
>>> and do not consider this a regression?  Could you explain that
>>> position with more detail?
>> Mark,
>>
>> You didn't understand me. There are two HTTP authentication schemes
>> for automatic authentication:
>> * NTLM
>>   Uses Windows NTLM authentication
>>
>> * Negotiate (SPNEGO)
>>   Uses NTLM or Kerberos depending of what is supported by server and client.
>>
>> NTLM is not documented AFAIK, while Negotiate (SPNEGO) is documented
>> by RFC 4559 [1]
>>
>> Serf supports only Negotiate authentication schemes. Which
>> automatically provides you NTLM or Kerberos.
>>
>> mod_auth_sspi can be configured to use Negotiate protocol using
>> "SSPIPackage Negotiate" server side directive. Bert reported that with
>> "SSPIPackage Negotiate" is working fine, but neon doesn't.
>>
>> My position is that serf should use only Negotiate authentication
>> scheme if server supports both NTLM and Negotiate authentication
>> schemes.
> If existing 1.7, 1.6 etc clients do not support this, then your
> position is untenable, one might even say ludicrous.  That is why I am
> asking for more explanation.  Surely this cannot be what you are
> saying?
>
> We can all agree we have a significant number of existing users using
> an automatic authentication method with Windows.  I am calling that
> mod_auth_sspi.  I guess to use your terms, that means NTLM.  Are any
> of these users using the SSPI negotiate option?  If our pre-1.8
> clients do not support that option then I would have to say No.
>
> I fail to see how you can justify a veto here.

I have to agree. The veto is fine on aesthetic grounds but kind of fails
to take account of reality.

-- Brane

-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com

Re: [serf-dev] [Patch] Adding NTLM Support to Serf - Work in progress / Subversion regression

Posted by Mark Phippard <ma...@gmail.com>.
On Thu, Jun 20, 2013 at 9:52 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On Thu, Jun 20, 2013 at 5:44 PM, Mark Phippard <ma...@gmail.com> wrote:
>> On Thu, Jun 20, 2013 at 9:40 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>> On Thu, Jun 20, 2013 at 5:30 PM, Bert Huijben <be...@qqmail.nl> wrote:
>>> [...]
>>>
>>>> The patch to serf 1.2.1 attached to this mail is a (tiny bit cleaned up)
>>>> hack based on the old code in ra_serf, some code from an old serf branch and
>>>> the new in serf auth_kerb code, which re-enables the NTLM authentication
>>>> scheme in serf.
>>>>
>>>>
>>> I'm -1 for such patch:
>>> * It duplicates auth_kerb.c which intended to have the same auth code
>>> on different platforms with plugable platforms specific code
>>>
>>> * serf should not try use NTLM authentication if server supports Negotiate.
>>
>> So you are saying you do not think Serf should support mod_auth_sspi
>> and do not consider this a regression?  Could you explain that
>> position with more detail?
> Mark,
>
> You didn't understand me. There are two HTTP authentication schemes
> for automatic authentication:
> * NTLM
>   Uses Windows NTLM authentication
>
> * Negotiate (SPNEGO)
>   Uses NTLM or Kerberos depending of what is supported by server and client.
>
> NTLM is not documented AFAIK, while Negotiate (SPNEGO) is documented
> by RFC 4559 [1]
>
> Serf supports only Negotiate authentication schemes. Which
> automatically provides you NTLM or Kerberos.
>
> mod_auth_sspi can be configured to use Negotiate protocol using
> "SSPIPackage Negotiate" server side directive. Bert reported that with
> "SSPIPackage Negotiate" is working fine, but neon doesn't.
>
> My position is that serf should use only Negotiate authentication
> scheme if server supports both NTLM and Negotiate authentication
> schemes.

If existing 1.7, 1.6 etc clients do not support this, then your
position is untenable, one might even say ludicrous.  That is why I am
asking for more explanation.  Surely this cannot be what you are
saying?

We can all agree we have a significant number of existing users using
an automatic authentication method with Windows.  I am calling that
mod_auth_sspi.  I guess to use your terms, that means NTLM.  Are any
of these users using the SSPI negotiate option?  If our pre-1.8
clients do not support that option then I would have to say No.

I fail to see how you can justify a veto here.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

Re: [serf-dev] [Patch] Adding NTLM Support to Serf - Work in progress / Subversion regression

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Thu, Jun 20, 2013 at 5:44 PM, Mark Phippard <ma...@gmail.com> wrote:
> On Thu, Jun 20, 2013 at 9:40 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>> On Thu, Jun 20, 2013 at 5:30 PM, Bert Huijben <be...@qqmail.nl> wrote:
>> [...]
>>
>>> The patch to serf 1.2.1 attached to this mail is a (tiny bit cleaned up)
>>> hack based on the old code in ra_serf, some code from an old serf branch and
>>> the new in serf auth_kerb code, which re-enables the NTLM authentication
>>> scheme in serf.
>>>
>>>
>> I'm -1 for such patch:
>> * It duplicates auth_kerb.c which intended to have the same auth code
>> on different platforms with plugable platforms specific code
>>
>> * serf should not try use NTLM authentication if server supports Negotiate.
>
> So you are saying you do not think Serf should support mod_auth_sspi
> and do not consider this a regression?  Could you explain that
> position with more detail?
Mark,

You didn't understand me. There are two HTTP authentication schemes
for automatic authentication:
* NTLM
  Uses Windows NTLM authentication

* Negotiate (SPNEGO)
  Uses NTLM or Kerberos depending of what is supported by server and client.

NTLM is not documented AFAIK, while Negotiate (SPNEGO) is documented
by RFC 4559 [1]

Serf supports only Negotiate authentication schemes. Which
automatically provides you NTLM or Kerberos.

mod_auth_sspi can be configured to use Negotiate protocol using
"SSPIPackage Negotiate" server side directive. Bert reported that with
"SSPIPackage Negotiate" is working fine, but neon doesn't.

My position is that serf should use only Negotiate authentication
scheme if server supports both NTLM and Negotiate authentication
schemes.

[1] http://tools.ietf.org/html/rfc4559

-- 
Ivan Zhakov

Re: [serf-dev] [Patch] Adding NTLM Support to Serf - Work in progress / Subversion regression

Posted by Mark Phippard <ma...@gmail.com>.
On Thu, Jun 20, 2013 at 9:40 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On Thu, Jun 20, 2013 at 5:30 PM, Bert Huijben <be...@qqmail.nl> wrote:
> [...]
>
>> The patch to serf 1.2.1 attached to this mail is a (tiny bit cleaned up)
>> hack based on the old code in ra_serf, some code from an old serf branch and
>> the new in serf auth_kerb code, which re-enables the NTLM authentication
>> scheme in serf.
>>
>>
> I'm -1 for such patch:
> * It duplicates auth_kerb.c which intended to have the same auth code
> on different platforms with plugable platforms specific code
>
> * serf should not try use NTLM authentication if server supports Negotiate.

So you are saying you do not think Serf should support mod_auth_sspi
and do not consider this a regression?  Could you explain that
position with more detail?

AFAIK, that is the primary way users have configured their servers to
do this kind of automatic authentication via Windows.

-- 
Thanks

Mark Phippard
http://markphip.blogspot.com/

RE: [serf-dev] [Patch] Adding NTLM Support to Serf - Work in progress / Subversion regression

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
> Sent: donderdag 20 juni 2013 15:41
> To: serf-dev@googlegroups.com; Bert Huijben
> Cc: dev@subversion.apache.org; Gert Kello
> Subject: Re: [serf-dev] [Patch] Adding NTLM Support to Serf - Work in
> progress / Subversion regression
> 
> On Thu, Jun 20, 2013 at 5:30 PM, Bert Huijben <be...@qqmail.nl> wrote:
> [...]
> 
> > The patch to serf 1.2.1 attached to this mail is a (tiny bit cleaned up)
> > hack based on the old code in ra_serf, some code from an old serf branch
> and
> > the new in serf auth_kerb code, which re-enables the NTLM authentication
> > scheme in serf.
> >
> >
> I'm -1 for such patch:
> * It duplicates auth_kerb.c which intended to have the same auth code
> on different platforms with plugable platforms specific code
> 
> * serf should not try use NTLM authentication if server supports Negotiate.

Do you have other code that works for these existing use cases?


I don't see that much duplication. This uses the existing plugin mechanism of serf, which still had defines for NTLM from before the NTLM stubs were removed on your Kerberos support branch.


That is the major reason why I didn't have to do anything but plugin an implementation.

If serf needs a better plugin mechanism we (or maybe you) should fix serf...
(It looks like the support is already negotiated... You can even disable schemes in the config)


Calling every user scenario except using VisualSVN server broken is not an option. 

We can't just call Kerberos supported and drop NTLM support if many users still use this. 


Using Subversion 1.7 with neon is 100% supported and we can't just break all 1.7 users to enable support for 1.8. That is 100% against our compatibility guarantees.


	Bert


Re: [serf-dev] [Patch] Adding NTLM Support to Serf - Work in progress / Subversion regression

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Thu, Jun 20, 2013 at 5:30 PM, Bert Huijben <be...@qqmail.nl> wrote:
[...]

> The patch to serf 1.2.1 attached to this mail is a (tiny bit cleaned up)
> hack based on the old code in ra_serf, some code from an old serf branch and
> the new in serf auth_kerb code, which re-enables the NTLM authentication
> scheme in serf.
>
>
I'm -1 for such patch:
* It duplicates auth_kerb.c which intended to have the same auth code
on different platforms with plugable platforms specific code

* serf should not try use NTLM authentication if server supports Negotiate.

-- 
Ivan Zhakov