DCERPC Security Context Multiplexing (ready for review)

[Jeremy Allison]

On Wed, Jan 09, 2019 at 12:46:31PM -0800, Jeremy Allison via samba-technical wrote:
> On Thu, Dec 20, 2018 at 12:53:52AM +0100, Stefan Metzmacher wrote:
> > Am 18.12.18 um 17:48 schrieb Jeremy Allison:
> > > On Tue, Dec 18, 2018 at 02:31:32PM +0100, Stefan Metzmacher via samba-technical wrote:
> > >> Hi,
> > >>
> > >> I'm currently working on an implementation of the
> > >> SecurityContextMultiplexingSupported feature from [MS-RPCE] 3.3.1.5.4
> > >> Security Context Multiplexing in the source4/rpc_server code.
> > >>
> > >> Basically the concept is similar to having multiple session setups
> > >> on a single SMB connection, just for DCERPC.
> > >>
> > >> This is important in order to fix or avoid the following bugs:
> > >> https://bugzilla.samba.org/show_bug.cgi?id=7113
> > >> https://bugzilla.samba.org/show_bug.cgi?id=11892
> > >> https://bugzilla.samba.org/show_bug.cgi?id=13464
> > >>
> > >> The patches are in the following branch:
> > >> https://git.samba.org/?p=metze/samba/wip.git;a=shortlog;h=refs/heads/master4-dcerpc-ok
> > >>
> > >> The following pidl change to the python bindings:
> > >> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=052016f3415a
> > >> results in the following diff of the generated code:
> > >> https://git.samba.org/?p=metze/samba/wip.git;a=commitdiff;h=44d05e1b35a8
> > >> Before we had broken code like this:
> > >>
> > >>   r->in.unknown = NULL;
> > >>   *r->in.unknown = NULL;
> > >>
> > >> I opened the following merge request for the almost finished patches:
> > >> https://gitlab.com/samba-team/samba/merge_requests/173
> > >>
> > >> I still need to check some details and improve some commit messages, but
> > >> the review and already start...
> > 
> > I've fixed the missing settimeout in the pylibsmb bindings and fixed the
> > commit messages.
> > 
> > This is ready for review now.
> > 
> > I'm not attaching the patches here as they're over 500kb in size, which
> > is not allowed on the list.
> > 
> > They can be downloaded here:
> > https://gitlab.com/samba-team/samba/merge_requests/173.patch
> > 
> > >> Please review and comment:-)
> > 
> > Please review and push:-)
> 
> I'm reviewing the remaining this week, sorry for the delay
> over the holidays !

One quick comment. In [PATCH 044/103] s4:rpc_server/lsa: make use of dcesrv_call_auth_info()

you're removing the check for auth->auth_level < DCERPC_AUTH_LEVEL_INTEGRITY,
so this patch also needs the commit message to have the text:

"It's enough to check the auth_type for DCERPC_AUTH_TYPE_SCHANNEL,
there's no need to also check the auth_level for integrity or privacy.

The gensec layer already required at least DCERPC_AUTH_LEVEL_INTEGRITY,
see schannel_update_internal()."

added to it, as [PATCH 045/103] already does.

Just wanted to let you know I'm paying attention :-). FYI, I've
added that text into the commit message of my local copy and
when finished review will push/resend to the list.

Jeremy.