Converting SMB1 tests to SMB2

[Ralph Boehme]

Hi Noel,

On 11/27/19 11:58 PM, Noel Power wrote:
> Hi Ralph & *
> 
> Some observations
> 
> On 26/11/2019 16:09, Noel Power wrote:
>> Hi Ralph
>>
>> On 26/11/2019 11:45, Ralph Boehme via samba-technical wrote:
> [...]
>>> Stepping back a little I think we only need the following:
>>>
>>> 1. The file with the list of failing tests should not be used as a skip
>>> list. Just maintain a private branch with the changes needed to generate
>>> the list. The list can go into master as eg selftest/smb1-todo or
>>> similarily named.
>>>
>>> Ideally the commit that adds the list also included instructions and the
>>> patchset as file that is needed to regenerate the list on sn-devel.
>> currently the skip file is effectively documentation, the commits around
>> this at the start of the patch set are just to prove it was a valid list
>> (after migrating the envs to default only negotiate >= SMB2)
>>
>> There are no 'changes' to generate the the list of skip tests other than
>> making all envs by default not negotiate SMB1 and then trying to see
>> what fails (and unfortunately this isn't as straight forward as running
>> the tests with FAIL_IMMEDIATELY=0 as some tests hang etc.
>>
>> So I am not sure which changes you mean should be in a private branch. I
>> guess I misunderstand what you mean
> 
> ok, I poked around a bit with this again today, with the ad_dc_ntvfs
> changes the landscape of errors seems a little different and my
> impression is not as many now hang. So maybe such instructions could be
> 
>   + apply a small patch to the existing skip file [1]
> 
>   + apply patches we have already to make the envs default not negotiate
> SMB1 (with the exception of ad_dc_ntvfs)
> 
>   + run autobuild or submit to gitlab and scrape the stdout logs with [2]
> 
> That should suffice for (re)creating a skip file
> 
> skip file at
> 
> https://gitlab.com/samba-team/devel/samba/commits/npower_with_smb2_ntvfs
> 
> was generated using above steps, verified here
> 
> https://gitlab.com/samba-team/devel/samba/pipelines/99046676

Looking much better.

Many, many torture test really could be run only against one env instead
of ad_dc and nt4_dc, eg

samba3.base -> fileserver (and then fileserver_smb1)
samba3.raw -> dito

But that is probably a lot of work only saving one smb1 env.

>>> 5. Add correspondong _smb1_done alias envs.
>>>
>>> 6. Go through the todo list, either
>>>    a) just fixing the test if it should genuinely
>>>       work with smb2 => remove test from todo list
>>>    b) convert failing smb1 tests to use the
>>>       envs from 4 => update used test env in todo list
>> I guess this answers my question above wrt. above
> 
> It would be great to know what environments are perhaps suitable
> alternatives to problematic ones especially if there is a need to keep
> the number of new environments at  minimum.
> 
> Looking even at existing tests that already are already split between
> SMB1 & >= SMB2 the following environments are affected
> 
> e.g. The following are just a few examples of legitimate tests (that
> don't need porting) that currently would fail if run against
> environments that cannot negotiate SMB1
> 
> samba3.blackbox.acl_xattr.NT1\(fileserver\)

remove this one, only run the SMB3 variant.

> samba3.blackbox.inherit_owner.default.NT1\(fileserver\)

dito. Also likely true for all the other blackbox tests that run NT1 and
SMB3. BUt again please make micro commits so we can review and test
individually.

> samba3.blackbox.smbclient_basic.NT1\(nt4_dc_schannel\)

remove it.

> samba3.blackbox.smbclient_large_file -mNT1 -e NTLM\(nt4_dc:local\)

remove it.

> there are many more, plus tests than need to be split (because they
> currently mix SMB1 & SMB2 tests in the same test driver script file)
> 
> quickly scanning the following tests involved use the following environments
> 
>     fileserver, nt4_dc, nt4_member, nt4_dc_schannel, fl2000dc,
> maptoguest, s4member, ad_member, ad_dc
> 
> There are quite a few environments here, certainly more than the 3, any
> advice on substitutions ?

See my previous mail.

> 
> Additionally there are quite a few tests that  use smbclient4 (and other
> s4 clients, e.g. cifsdd, locktest etc.) and these all will fail because
> they afaics cannot handle smb2. Could we replace use of smbclient4 with
> s3 smbclient in these tests?

Oh, that's a good question. :) I guess at least the
samba4.blackbox.kinit tests could use smbclient from s3.

> what is the plan for smbclient4, it doesn't
> seem fully featured, is it really used outside the tests, is it worth
> investing time to convert it to use smb2 ?

No.

> I see cifsdd is part of the
> samba suite so this one needs adjusting to be able to use >= SMB2

If feel like removing it, but maybe someone want to convert it. Until
then the test uses ad_dc_ntvfs env, so it can stay as it is.

> What about s4member? (and other envs that are dependant on ad_dc_ntvfs)
> should these be additionally treated the same as ad_dc_ntvfs and allow
> SMB1 also ?

Iirc s4member still uses the NTVFS s4 fileserver in my branch, so yes,
iirc my idea was that all those should keep smb1.

> it seems strange to set up an environment which is going to
> have communication issues because half it can talk SMB1 and the other
> half can't
> 
>>>    Step 6 can be spread across volunteers.
>>>
>>> 7. When the list is fully processed and all remaining tests on the todo
>>> list that still need smb1 use one of the new _smb1 envs, disable smb1 in
>>> all other envs except ad_dc_ntvfs and s4member.
>> I don't like that disabling smb1 by default for the 'normal'
>> environments is now a final step, that isn't as nice as starting from an
>> initial point where we can clearly see what runs >=SMB2 and what
>> doesn't, not only see but actually know (because the default env setup
>> would prevent any accidently SMB1 usage) and similarly when you port a
>> test you know it isn't still using some SMB1 that you didn't notice.
>>
>> I wonder how far off we would be if I merged your slow-ad_dc_ntvfs in
>> terms of what extra (if any) envs are needed, I will try
> 
> well current 13 envs appear in the latest skip file
> 
> ad_dc
> ad_dc_default
> ad_member
> chgdcpass
> fileserver
> fl2000dc
> maptoguest
> nt4_dc
> nt4_dc_schannel
> nt4_member
> rpc_proxy
> s4member
> simpleserver
> 
> btw I opened https://gitlab.com/samba-team/samba/merge_requests/947 for
> some prep work to split tests so they can be divided across envs that
> are SMB2 only and still support SMB1 (if we ever get to that point :->>>>)

I'll take a look tomorrow!

Thanks!
-slow

-- 
Ralph Boehme, Samba Team                https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG-Fingerprint   FAE2C6088A24252051C559E4AA1E9B7126399E46