[PATCH] Two dependent bugfixes for VSS bugs 13455 and 13688

[Jeremy Allison]

On Sat, Nov 24, 2018 at 11:57:38AM +0100, Ralph Böhme wrote:
> On Fri, Nov 23, 2018 at 01:50:09PM -0800, Jeremy Allison wrote:
> > On Fri, Nov 23, 2018 at 09:00:57PM +0100, Ralph Böhme wrote:
> > > On Fri, Nov 23, 2018 at 12:53:02PM -0800, Jeremy Allison wrote:
> > > > On Fri, Nov 23, 2018 at 12:45:47PM -0800, Jeremy Allison via samba-technical wrote:
> > > > > Ralph, I thin [PATCH 09/10] s3:smbd: GMT token handling on paths: prefix, don't append
> > > > > is incorrect.
> > > > >
> > > > > We used to do this, but found it breaks pathnames with a MS-DFS
> > > > > prefix (such as \\server\share\path).
> > > > >
> > > > > If you haven't already, please check this code against a
> > > > > server set up with MS-DFS pathnames.
> > > > >
> > > > > If you don't have time before then, let's talk when I'm back in at
> > > > > work on Monday.
> > > > > >
> > > > > > The ':' after "foor" sets stream_started to true, then the '\' after
> > > > > > "stream" triggers the error.
> > > > > >
> > > > > > Fix: prepend the GMT token to the path, that's what we're later doing in
> > > > > > canonicalize_snapshot_path() anyway. For the above example that results
> > > >
> > > > Note that canonicalize_snapshot_path() is called *after* any MS-DFS
> > > > processing is done, which is why it's safe to do that there.
> > > >
> > > > I think the correct fix here is to search in the SMB2 create
> > > > code for a ':' character and if found, insert the @GMT token
> > > > before that, otherwise append as we're currently doing.
> > > >
> > > > This will leave the MS-DFS processing working I think.
> > > 
> > > ok, thanks, I'll recheck. This is a real mess! :)
> > 
> > Sorry, yeah. Keeping pathname processing working over
> > SMB1/2/3 over MS-DFS and including the SMB1 @GMT-parsing
> > is a horrible compatibility matrix.
> 
> especially if there are no tests. :)
> 
> > Eventually we need to move to parsing the @GMT parsing
> > out into the start and adding a timstamp to struct smb_filename
> > (this is how Windows does it internally, I'm sure) but
> > that's a larger change for another day, I'm afraid :-).
> 
> ha!, we could start right away. :)
> 
> What about the attached patchset where I'm passing down the timestamp from
> the SMB2 layer down to filename_convert_internal() as an extra argument,
> processing it there, instead of doing it at the SMB2 layer?
> 
> Passes the new tests.

I really like that - fixes the issues with @GMT placement
by moving it until after the DFS redirect is examined !

RB+ from me and pushed.

Jeremy.