[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

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

From: Ivan Zhakov <ivan_at_visualsvn.com>
Date: Thu, 20 Jun 2013 19:27:02 +0400

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 <markphip_at_gmail.com> wrote:
> On Thu, Jun 20, 2013 at 9:52 AM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
>> On Thu, Jun 20, 2013 at 5:44 PM, Mark Phippard <markphip_at_gmail.com> wrote:
>>> On Thu, Jun 20, 2013 at 9:40 AM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
>>>> On Thu, Jun 20, 2013 at 5:30 PM, Bert Huijben <bert_at_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
Received on 2013-06-20 17:27:55 CEST

This is an archived mail posted to the Subversion Dev mailing list.