[jcifs] Version 1.3.1 bug (and fix)

Travaglini, Joseph Joseph.Travaglini at FMR.com
Fri Dec 5 16:32:30 GMT 2008


Yoda,

What might this alternative be?

I agree with John that removing the filter and support for it is not a
good idea.

-Joe

-----Original Message-----
From: jcifs-bounces+joseph.travaglini=fmr.com at lists.samba.org
[mailto:jcifs-bounces+joseph.travaglini=fmr.com at lists.samba.org] On
Behalf Of Michael B Allen
Sent: Friday, December 05, 2008 11:22 AM
To: John.Baker at barclayscapital.com
Cc: jcifs at lists.samba.org
Subject: Re: [jcifs] Version 1.3.1 bug (and fix)

John,

First, as I explained before, all of the properties that are static
are properties that should never need to be changed and therefore
there should be no problem with leaving them static. It's only when
you try to set them dynamically using init-params in a servlet
container which clearly is never going to work reliably (e.g. multiple
Filters using JCIFS).

Second, the Config class (and thus all of it's properties) is global
and static. Therefore, there's no way to make them dynamic without
significantly reorganizing the code. You cannot simply remove the
'static' qualifier. How would you then set those properties? I eluded
to what was necessary to fix this in a previous message. The changes
are non-trivial and will not be applied in 1.x.

Finally, the NTLM HTTP Filter does not work reliably (google for
'jcifs hiccup bug') and will never work reliably and can never ever
support NTLMv2 because of the "man in the middle" technique that it
uses. The details of this issue have been discussed on the mailing
list a few times. If you drudge through the archives you should be
able to find it. Therefore, it will be removed. I was going to remove
it in 2.0 but the reasons for removing it sooner are piling up.

But. An alternative there will be ...

Yoda

On Fri, Dec 5, 2008 at 10:59 AM,  <John.Baker at barclayscapital.com>
wrote:
> Mike,
>
> I pasted you the stack trace - what part of the bug report do you
> believe could be incorrect?  It's pretty obvious from the code that
the
> statics will cause problems.
>
> While I'm unsure of the underlying architecture, removing one of the
> best features of jcifs (that may or may not rely on the underlying
> codebase) because of an easily fixable set of statics, doesn't seem a
> terribly good way forward.
>
> This could have been fixed in the time we've spent discussing it.
Happy
> to do so if you want to give me svn access - a few clicks of the
Eclipse
> refactoring tool and we're done.
>
> I did mention that one needed to set the logging on through a filter
> parameter to make it happen every time - I don't know what that does,
> but it does seem to ensure the class at fault is loaded by the VM
before
> Config.set has been called by the filter.
>
> Further to that, do the statics not limit the product in some way?
Can
> I create multiple instances of Jcifs objects and point them at
different
> domain controllers, or am I stuck with one through the use of the
> Config?
>
>
> John
>
>> -----Original Message-----
>> From: Michael B Allen [mailto:ioplex at gmail.com]
>> Sent: 05 December 2008 15:53
>> To: Baker, John: IT (LDN)
>> Cc: jcifs at lists.samba.org
>> Subject: Re: [jcifs] Version 1.3.1 bug (and fix)
>>
>> On Fri, Dec 5, 2008 at 4:09 AM,
>> <John.Baker at barclayscapital.com> wrote:
>> > Mike,
>> >
>> > I appreciate 1.3 offers no functional difference to 1.2 (at
>> present).
>> >
>> > I'm not having trouble setting a property - your code is
>> having trouble.
>> > The NtlmHttpFilter tries to set the lmCompatibility flag,
>> and this was
>> > a recent fix to another bug, but it doesn't work.  It doesn't work
>> > because you're using static attributes within the class in question
>> > and they are sometimes set when the VM starts, and other
>> times not.
>> > The filter will now only work sometimes, and that's not a terribly
>> > good selling point for the product.  To a new user, it's a
disaster.
>> >
>> > The static attributes serve no useful purpose.  They can
>> all easily be
>> > set up non-statically when the object is instantiated.  I'd
>> do it all
>> > myself but I don't have access to svn.
>> >
>> > Of course, the flag could be set via a command line
>> parameter, but it
>> > doesn't fix the bug.
>>
>> If everything you say is in fact true, then I will probably
>> simply remove the NTLM HTTP Filter from JCIFS 1.3.
>>
>> But no one else has reported this issue so I will wait for
>> confirmation before I remove it.
>>
>> Please use the Filter from JCIFS 1.2. I will maintain the
>> Filter in that package.
>>
>> Thanks,
>> Mike
>>
>> >
>> >
>> > John
>> >
>> >> -----Original Message-----
>> >> From: Michael B Allen [mailto:ioplex at gmail.com]
>> >> Sent: 04 December 2008 23:06
>> >> To: Baker, John: IT (LDN)
>> >> Cc: jcifs at lists.samba.org
>> >> Subject: Re: [jcifs] Version 1.3.1 bug (and fix)
>> >>
>> >> On Thu, Dec 4, 2008 at 10:17 AM,
>> >> <John.Baker at barclayscapital.com> wrote:
>> >> > Mike,
>> >> >
>> >> > Are you saying that for anyone who wishes to use the
>> >> NtlmHttpFilter,
>> >> > they should now stick with version 1.2.25 of the product?
>> >> Hence, the
>> >> > product will drop support for this filter because it
>> >> doesn't reliably
>> >> > work in 1.3.x ?
>> >>
>> >> No I will maintain the Filter in 1.3.x. I was just saying that 1.3
>> >> *currently* provides no advantage over 1.2 so you can very easily
>> >> solve your problem by simply using 1.2. I'm just not going
>> to change
>> >> properties that are static to dynamic which is what you're
>> ultimately
>> >> asking me to do.
>> >>
>> >> > I must be confused because that doesn't seem a terribly good way
>> >> > forward and there's clearly a bug that needs addressing.
>> >>
>> >> My understanding is that the problem you are experiencing is not a
>> >> bug.
>> >>
>> >> If you are having trouble setting static properties like
>> >> jcifs.lmCompatibility using Filter init-params because of how the
>> >> container loads classes, they you will have to use commandline
>> >> options to set static properties. See the "Setting Client
>> Properties"
>> >> documentation for details about how to set JCIFS properties.
>> >>
>> >> Mike
>> >>
>> >> >> -----Original Message-----
>> >> >> From: Michael B Allen [mailto:ioplex at gmail.com]
>> >> >> Sent: 03 December 2008 22:32
>> >> >> To: Baker, John: IT (LDN)
>> >> >> Subject: Re: [jcifs] Version 1.3.1 bug (and fix)
>> >> >>
>> >> >> No. This will not be changed.
>> >> >>
>> >> >> On Wed, Dec 3, 2008 at 3:53 PM,
>> >> >> <John.Baker at barclayscapital.com> wrote:
>> >> >> > Mike,
>> >> >> >
>> >> >> > Are there any plans to move the statics - it shouldn't be too
>> >> >> > difficult and we can't have the NtlmHttpFilter breaking
>> >> >> randomly, even
>> >> >> > if it can be fixed via the command line parameters..
>> >> >> >
>> >> >> > Thanks,
>> >> >> >
>> >> >> >
>> >> >> > John
>> >> >> >
>> >> >> >> -----Original Message-----
>> >> >> >> From: Michael B Allen [mailto:ioplex at gmail.com]
>> >> >> >> Sent: 03 December 2008 18:31
>> >> >> >> To: Baker, John: IT (LDN)
>> >> >> >> Cc: jcifs at lists.samba.org
>> >> >> >> Subject: Re: [jcifs] Version 1.3.1 bug (and fix)
>> >> >> >>
>> >> >> >> On Wed, Dec 3, 2008 at 6:44 AM,
>> >> >> >> <John.Baker at barclayscapital.com> wrote:
>> >> >> >> > Hi,
>> >> >> >> >
>> >> >> >> > I think I've found a bug in version 1.3.1 when using the
>> >> >> >> NtlmHttpFilter.
>> >> >> >> > Having configured the system and watched it work, I then
>> >> >> configured
>> >> >> >> > logging:
>> >> >> >> >
>> >> >> >> >            <init-param>
>> >> >> >> >
<param-name>jcifs.util.loglevel</param-name>
>> >> >> >> >                <param-value>4</param-value>
>> >> >> >> >            </init-param>
>> >> >> >> >
>> >> >> >> > Restarted and it failed with this exception:
>> >> >> >> >
>> >> >> >> > jcifs.smb.SmbException: NTLMv2 requires extended security
>> >> >> >> > (jcifs.smb.client.useExtendedSecurity must be true if
>> >> >> >> > jcifs.smb.lmCompatibility >= 3)
>> >> >> >> >        at
>> >> >> >> >
>> >> >> >>
>> >> >>
>> >>
>> jcifs.smb.NtlmPasswordAuthentication.getSigningKey(NtlmPasswordAuthen
>> >> >> >> t
>> >> >> >> > ic
>> >> >> >> > ation.java:473)
>> >> >> >> >        at
>> >> jcifs.smb.SmbSession.sessionSetup(SmbSession.java:295)
>> >> >> >> >        at jcifs.smb.SmbSession.send(SmbSession.java:234)
>> >> >> >> >        at jcifs.smb.SmbTree.treeConnect(SmbTree.java:161)
>> >> >> >> >        at
>> jcifs.smb.SmbSession.interrogate(SmbSession.java:83)
>> >> >> >> >        at
>> >> >> >> >
>> >> jcifs.smb.SmbSession.getChallengeForDomain(SmbSession.java:115)
>> >> >> >> >        at
>> >> >> >> jcifs.http.NtlmHttpFilter.negotiate(NtlmHttpFilter.java:157)
>> >> >> >> >        at
>> >> >> >> jcifs.http.NtlmHttpFilter.doFilter(NtlmHttpFilter.java:121)
>> >> >> >> >        at
>> >> >> >> >
>> >> >>
>> org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(A
>> >> >> p
>> >> >> >> > p
>> >> >> >> >
>> >> >> >> > (Be sure to start a new browser window.)
>> >> >> >> >
>> >> >> >> > Looking at NtlmPasswordAuthentication:
>> >> >> >> >
>> >> >> >> >    private static final int LM_COMPATIBILITY =
>> >> >> >> >            Config.getInt("jcifs.smb.lmCompatibility", 3);
>> >> >> >> >
>> >> >> >> > That is clearly wrong as it sets the compatibility when
the
>> >> >> >> class is
>> >> >> >> > first created (but I guess, never again).  Hence, when
>> >> >> the logging
>> >> >> >> > occurs, the value is set incorrectly and before it's been
>> >> >> >> reset by the
>> >> >> >> > NtlmHttpFilter!
>> >> >> >> >
>> >> >> >> > Removing 'static' fixes the problem.
>> >> >> >>
>> >> >> >> Actually I didn't think you could even set the log.level
>> >> >> from within
>> >> >> >> the filter. Ok.
>> >> >> >>
>> >> >> >> Setting things when the class is first loaded is not
>> >> >> "clearly wrong".
>> >> >> >> On Windows systems lmCompatibility is set in the
>> >> registry and is
>> >> >> >> therefore a global fixed value.
>> >> >> >>
>> >> >> >> But clearly it would be better to make some of these
>> properties
>> >> >> >> dynamic. Unfortunately the code is simply not organized
>> >> for this
>> >> >> >> property to be dynamic. The Config class is static and
>> >> >> global so if
>> >> >> >> one thread sets lmCompatibility to
>> >> >> >> 3 another thread could come a long and set it to 0 before
>> >> >> the first
>> >> >> >> thread tried to use it. There needs to be a Map of
>> >> properties or
>> >> >> >> context containing said map that is passed around
>> >> >> throughout the API
>> >> >> >> or perhaps associated with the thread. But that's not
>> >> >> something that
>> >> >> >> is going to change in 1.x.
>> >> >> >>
>> >> >> >> One thing you could do is to set things on the command
>> >> like using
>> >> >> >> -Djcifs.smb.lmCompatibility=0. Or better still set the
>> >> properties
>> >> >> >> file on the commandline like -Djcifs.properties=jcifs.prp
>> >> >> so that you
>> >> >> >> can add and remove properties to that file instead of those
>> >> >> >> abominable XML configuration files. At any rate, those
>> >> commandline
>> >> >> >> properties are loaded when the Config class loads so it
>> >> should be
>> >> >> >> sufficient to work around the problem.
>> >> >> >>
>> >> >> >> Also note that so far JCIFS 1.3 offers no real benefits over
>> >> >> >> 1.2 wrt the Filter. The JCIFS NTLM HTTP Filter does
>> not support
>> >> >> >> NTLMv2 (and can never support NTLMv2) and there are
>> >> otherwise no
>> >> >> >> Filter related fixes in 1.3 releative to 1.2.25 (aside
>> >> >> from a fix for
>> >> >> >> something that was broken in 1.3.0).
>> >> >> >>
>> >> >> >> Mike
>> >> >> >>
>> >> >> >> --
>> >> >> >> Michael B Allen
>> >> >> >> PHP Active Directory SPNEGO SSO http://www.ioplex.com/
>> >> >> >>
>> >> >> > _______________________________________________
>> >> >> >
>> >> >> > This e-mail may contain information that is confidential,
>> >> >> privileged or otherwise protected from disclosure. If you
>> >> are not an
>> >> >> intended recipient of this e-mail, do not duplicate or
>> >> redistribute
>> >> >> it
>> >> >
>> >> >> by any means. Please delete it and any attachments and
>> notify the
>> >> >> sender that you have received it in error. Unless specifically
>> >> >> indicated, this e-mail is not an offer to buy or sell or a
>> >> >> solicitation to buy or sell any securities, investment
>> products or
>> >> >> other financial product or service, an official
>> >> confirmation of any
>> >> >> transaction, or an official statement of Barclays. Any views or
>> >> >> opinions presented are solely those of the author and do not
>> >> >> necessarily represent those of Barclays. This e-mail is
>> subject to
>> >> >> terms available at the following link:
>> >> >> www.barcap.com/emaildisclaimer. By messaging with Barclays you
>> >> >> consent
>> >> >
>> >> >> to the foregoing.  Barclays Capital is the investment banking
>> >> >> division
>> >> >
>> >> >> of Barclays Bank PLC, a company registered in England (number
>> >> >> 1026167)
>> >> >
>> >> >> with its registered office at 1 Churchill Place,
>> London, E14 5HP.
>> >> >> This email may relate to or be sent from other members of the
>> >> >> Barclays
>> >> >
>> >> >> Group.
>> >> >> > _______________________________________________
>> >> >> >
>> >> >>
>> >> >>
>> >> >>
>> >> >> --
>> >> >> Michael B Allen
>> >> >> PHP Active Directory SPNEGO SSO
>> >> >> http://www.ioplex.com/
>> >> >>
>> >> > _______________________________________________
>> >> >
>> >> > This e-mail may contain information that is confidential,
>> >> privileged or otherwise protected from disclosure. If you
>> are not an
>> >> intended recipient of this e-mail, do not duplicate or
>> redistribute
>> >> it by any means. Please delete it and any attachments and
>> notify the
>> >> sender that you have received it in error. Unless specifically
>> >> indicated, this e-mail is not an offer to buy or sell or a
>> >> solicitation to buy or sell any securities, investment products or
>> >> other financial product or service, an official
>> confirmation of any
>> >> transaction, or an official statement of Barclays. Any views or
>> >> opinions presented are solely those of the author and do not
>> >> necessarily represent those of Barclays. This e-mail is subject to
>> >> terms available at the following link:
>> >> www.barcap.com/emaildisclaimer. By messaging with Barclays you
>> >> consent to the foregoing.  Barclays Capital is the
>> investment banking
>> >> division of Barclays Bank PLC, a company registered in England
>> >> (number 1026167) with its registered office at 1 Churchill Place,
>> >> London, E14 5HP.  This email may relate to or be sent from other
>> >> members of the Barclays Group.
>> >> > _______________________________________________
>> >> >
>> >>
>> >>
>> >>
>> >> --
>> >> Michael B Allen
>> >> PHP Active Directory SPNEGO SSO
>> >> http://www.ioplex.com/
>> >>
>> > _______________________________________________
>> >
>> > This e-mail may contain information that is confidential,
>> privileged or otherwise protected from disclosure. If you are
>> not an intended recipient of this e-mail, do not duplicate or
>> redistribute it by any means. Please delete it and any
>> attachments and notify the sender that you have received it
>> in error. Unless specifically indicated, this e-mail is not
>> an offer to buy or sell or a solicitation to buy or sell any
>> securities, investment products or other financial product or
>> service, an official confirmation of any transaction, or an
>> official statement of Barclays. Any views or opinions
>> presented are solely those of the author and do not
>> necessarily represent those of Barclays. This e-mail is
>> subject to terms available at the following link:
>> www.barcap.com/emaildisclaimer. By messaging with Barclays
>> you consent to the foregoing.  Barclays Capital is the
>> investment banking division of Barclays Bank PLC, a company
>> registered in England (number 1026167) with its registered
>> office at 1 Churchill Place, London, E14 5HP.  This email may
>> relate to or be sent from other members of the Barclays Group.
>> > _______________________________________________
>> >
>>
>>
>>
>> --
>> Michael B Allen
>> PHP Active Directory SPNEGO SSO
>> http://www.ioplex.com/
>>
> _______________________________________________
>
> This e-mail may contain information that is confidential, privileged
or otherwise protected from disclosure. If you are not an intended
recipient of this e-mail, do not duplicate or redistribute it by any
means. Please delete it and any attachments and notify the sender that
you have received it in error. Unless specifically indicated, this
e-mail is not an offer to buy or sell or a solicitation to buy or sell
any securities, investment products or other financial product or
service, an official confirmation of any transaction, or an official
statement of Barclays. Any views or opinions presented are solely those
of the author and do not necessarily represent those of Barclays. This
e-mail is subject to terms available at the following link:
www.barcap.com/emaildisclaimer. By messaging with Barclays you consent
to the foregoing.  Barclays Capital is the investment banking division
of Barclays Bank PLC, a company registered in England (number 1026167)
with its registered office at 1 Churchill Place, London, E14 5HP.  This
email may relate to or be sent from other members of the Barclays Group.
> _______________________________________________
>



-- 
Michael B Allen
PHP Active Directory SPNEGO SSO
http://www.ioplex.com/



More information about the jcifs mailing list