Bug 9974 - smbclient does not support SMB2
Summary: smbclient does not support SMB2
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: libsmbclient (show other bugs)
Version: 4.1.0rc2
Hardware: All All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
: 9829 (view as bug list)
Depends on:
Blocks: 9306
  Show dependency treegraph
 
Reported: 2013-06-27 17:24 UTC by Jeremy Allison
Modified: 2013-09-02 07:35 UTC (History)
3 users (show)

See Also:


Attachments
initial git-am patch for 4.1.x (93.24 KB, patch)
2013-07-02 20:06 UTC, Jeremy Allison
no flags Details
git-am fix for master and 4.1.0 (135.46 KB, patch)
2013-08-09 21:22 UTC, Jeremy Allison
no flags Details
git-am fix for 4.1.0. (199.92 KB, patch)
2013-08-14 22:38 UTC, Jeremy Allison
no flags Details
Patches for v4-1-test (204.24 KB, patch)
2013-08-15 10:32 UTC, Stefan Metzmacher
jra: review+
Details
git-am fix for 4.1.0 (22.13 KB, patch)
2013-08-17 00:05 UTC, Jeremy Allison
no flags Details
Complete (jumbo) patchset git-am format for 4.1.0. (227.31 KB, patch)
2013-08-19 21:02 UTC, Jeremy Allison
no flags Details
git-am fix for 4.1.0. (234.32 KB, patch)
2013-08-23 18:04 UTC, Jeremy Allison
obnox: review+
metze: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Allison 2013-06-27 17:24:00 UTC
I'm currently adding the support to do:

smbclient //server/share -mSMB2

and have it work. I have connections and opening/closing/deleting files and directories working. Still finishing up directory listing and read/write.

Will attach the patchset here for 4.1 once I'm done.

Jeremy.
Comment 1 Jeremy Allison 2013-07-02 20:06:47 UTC
Created attachment 9014 [details]
initial git-am patch for 4.1.x

Not asking for review, this is a big blob of a patch that I'll break up into reviewable patches.

Just parking it here so I don't lose it :-).

Jeremy.
Comment 2 Jeremy Allison 2013-08-09 15:25:59 UTC
FYI. I've almost finished my splitting up of this patch into micro-commits. Should be ready for review by Monday.

Jeremy.
Comment 3 Jeremy Allison 2013-08-09 21:22:17 UTC
Created attachment 9123 [details]
git-am fix for master and 4.1.0

Here's the "official" patchset, broken up for 4.1.x and master.

Works well (as far as my testing goes). Significantly improves read speeds.

I'll propose for master now.

Jeremy.
Comment 4 Jeremy Allison 2013-08-14 22:38:06 UTC
Created attachment 9135 [details]
git-am fix for 4.1.0.

Will upload final version that went into master once that's done.

Jeremy.
Comment 5 Stefan Metzmacher 2013-08-15 10:32:30 UTC
Created attachment 9136 [details]
Patches for v4-1-test

Jeremy, please also provide documentation updates for the release...
Comment 6 Jeremy Allison 2013-08-17 00:05:25 UTC
Created attachment 9139 [details]
git-am fix for 4.1.0

Submitted to master (storing here so I don't lose it :-).

This patchset adds support for --encrypt, or -e command
line options to smbclient and smbcacls when SMB3 protocol
is requested using -mSMB3.

It also adds a new "timeout" command and -t option to smbclient
to set the per-operation timeout.

This is needed as once SMB3 encryption is selected the server
response time can be very slow when requesting large numbers
(256) of large encrypted packets (1MB) from a Windows 2012
virtual machine. This allows a user to tune their allowable
wait time.

Also contains documentation updates for the changes made
to complete SMB2+ support in our client tools.

Please review and push if you're happy.

Cheers,

        Jeremy.
Comment 7 Karolin Seeger 2013-08-19 08:20:29 UTC
(In reply to comment #5)
> Created attachment 9136 [details]
> Patches for v4-1-test
> 
> Jeremy, please also provide documentation updates for the release...

Good point!
Documentation updates are needed for 4.1.0.
Comment 8 Michael Adam 2013-08-19 08:54:29 UTC
Setting Product to Samba 4.1 (instead of 4.0).
Comment 9 Michael Adam 2013-08-19 08:56:18 UTC
While this is great stuff, indeed, it this new feature really
for 4.1 so short before the release...
Comment 10 Jeremy Allison 2013-08-19 15:38:18 UTC
Yes :-). This feature has been in development for a while, plus I explicitly pointed out (before freeze) this is something we need in 4.1.

OEMs are running into environments with *NO* SMB enabled. We must have working SMB2+ client tools for 4.1.0.

Jeremy.
Comment 11 Jeremy Allison 2013-08-19 21:02:51 UTC
Created attachment 9148 [details]
Complete (jumbo) patchset git-am format for 4.1.0.

Here is the jumbo patchset - containing the patches reviewed by Metze and myself, the additional patch created by Volker to fix the fnum == -1 Coverity problem, and finally the ability to encrypt smbclient SMB3 traffic using the -e flag and the documentation updates.

Once the final part of the patchset (the -e and docs updates) are reviewed and pushed into master this should be the complete patch set for 4.1.0.

Jeremy.
Comment 12 Jeremy Allison 2013-08-19 21:37:16 UTC
*** Bug 9829 has been marked as a duplicate of this bug. ***
Comment 13 Karolin Seeger 2013-08-20 09:28:37 UTC
(In reply to comment #11)
> Created attachment 9148 [details]
> Complete (jumbo) patchset git-am format for 4.1.0.
> 
> Here is the jumbo patchset - containing the patches reviewed by Metze and
> myself, the additional patch created by Volker to fix the fnum == -1 Coverity
> problem, and finally the ability to encrypt smbclient SMB3 traffic using the -e
> flag and the documentation updates.
> 
> Once the final part of the patchset (the -e and docs updates) are reviewed and
> pushed into master this should be the complete patch set for 4.1.0.
> 
> Jeremy.

Should we ship another release candidate containing this patchset?
Comment 14 Michael Adam 2013-08-20 11:18:15 UTC
(In reply to comment #10)
> Yes :-). This feature has been in development for a while, plus I explicitly
> pointed out (before freeze) this is something we need in 4.1.
> 
> OEMs are running into environments with *NO* SMB enabled. We must have working
> SMB2+ client tools for 4.1.0.

While I still don't buy the argument (why should OEMs have *NO* SMB
support when smbclient can't do SMB2? and why was this not a problem
for 4.0?) and we are violating many of our release policies, I won't
object the addition, because luckily the impact on the server code
is minimal. :-)

I looked at the patchset, and it generally looks good.
I will do more tests, but at least the last part
(encryption via "-e" still needs to go through master.

Stay tuned...
Comment 15 Jeremy Allison 2013-08-20 16:00:20 UTC
1). Yes, I think shipping another release candidate is a good idea.

2). OEMs are finding there are customer environments with SMB1 disabled, only SMB2+ allowed. This isn't a problem for Samba server-only OEMs, but there are other OEMs (I can name them privately) who depend on the client tools. They also need working cross-compiling (but that's another argument :-).

3). I don't think we're violating release policies. Back in July I raised the case that this was required for a 4.1.0 release. Everyone ignored me of course, but I did request this :-).

2). This provides additional functionality we really need for a good 4.1 release announcement (otherwise we don't have much to announce :-).

I'm happy for this to go in via master first of course, then I'll update the patchset with the "cherry-picked-from" meta-data for the 4.1.0 patchset.

Cheers,

Jeremy.
Comment 16 Jeremy Allison 2013-08-20 18:19:11 UTC
One more thing in support of this patchset for 4.1.0..

I really want to enable -e encryption for SMB3 connections from out client tools. In the current climate the only rational response is encrypt *everything*. Let's make that possible for our users !

Jeremy.
Comment 17 Volker Lendecke 2013-08-21 09:25:41 UTC
(In reply to comment #16)
> One more thing in support of this patchset for 4.1.0..
> 
> I really want to enable -e encryption for SMB3 connections from out client
> tools. In the current climate the only rational response is encrypt
> *everything*. Let's make that possible for our users !

Make that default if the server supports it? :-)

Volker
Comment 18 Jeremy Allison 2013-08-21 18:50:43 UTC
I don't think we're quite at the place where we can encrypt by default (yet:-).

Jeremy.
Comment 19 Volker Lendecke 2013-08-21 19:43:15 UTC
(In reply to comment #18)
> I don't think we're quite at the place where we can encrypt by default (yet:-).

Why not?
Comment 20 Jeremy Allison 2013-08-21 21:18:33 UTC
(a) Because it really affects our default speed.
(b) Because it means a bunch of code/docs changes to negate the -e option.

Jeremy.
Comment 21 Jeremy Allison 2013-08-23 18:04:24 UTC
Created attachment 9159 [details]
git-am fix for 4.1.0.

Ok, here's the complete patchset cherry-picked from master.

Applies cleanly to 4.1.0.

Jeremy.
Comment 22 Karolin Seeger 2013-08-28 07:26:19 UTC
Obnox, how long will it approximately take to review the latest patchset?
Does it make sense to add alternative reviewers in this case?
Comment 23 Stefan Metzmacher 2013-08-29 06:50:59 UTC
Comment on attachment 9159 [details]
git-am fix for 4.1.0.

Looks good to me for 4.1
Comment 24 Michael Adam 2013-08-29 14:59:48 UTC
Comment on attachment 9159 [details]
git-am fix for 4.1.0.

OK.
Comment 25 Michael Adam 2013-08-29 15:00:08 UTC
Karolin, please pick for 4.1
Comment 26 Karolin Seeger 2013-08-30 09:51:53 UTC
(In reply to comment #25)
> Karolin, please pick for 4.1

Pushed to autobuild-v4-1-test.
Comment 27 Karolin Seeger 2013-09-02 07:35:22 UTC
Pushed to v4-1-test.
Closing out bug report.

Thanks!