Samba VirusFilter nearing time for another review (was Re: Way to tell if smb_fname->base_name is a fully qualified path (w/ possible streams_depot bug))
Trever L. Adams
trever at middleearth.sapphiresunday.org
Thu Dec 1 19:48:07 UTC 2016
On 12/01/2016 11:43 AM, Jeremy Allison wrote:
> On Thu, Dec 01, 2016 at 08:10:23AM -0700, Trever L. Adams wrote:
>> On 11/30/2016 10:39 AM, Trever L. Adams wrote:
>>> With most files, the virus filter code has to prepend the connectpath.
>>> With streams_depot the connectpath appears to be already prepended, in
>>> the case of the default setting for where the streams are stored. I am
>>> aware it can be outside the connectpath, which is one reason I need to
>>> fix the above.
>>> In the case of smb_fname->base_name (fname above) with streams_depot,
>>> when one is working with the non-default stream, contains a fully
>>> qualified path. In other words, with the default streams_depot setting,
>>> you end up with something like this:
>>> Actual file: /CONNECTPATH/.streams/0F/17/02FD000000000000EF005C0400000000/:attached.txt:$DATA: Eicar-Test-Signature
>>> Appending connect path: /CONNECTPATH//CONNECTPATH/.streams/0F/17/02FD000000000000EF005C0400000000/:attached.txt:$DATA
>>> When not a stream and appending connect path: /CONNECTPATH/actualfile.txt
>>> Is there any way to tell easily when one needs to prepend the connect
>>> path and when one doesn't?
>>> Thank you.
>> One possible bug in streams_depot, the file name above
>> "OF/17/02FD000...." will not store in /tmp/streams or /home/streams on
>> Fedora 25, but will in the default streams directory
>> (CONNECTPATH/.streams). I do not know why. Permissions and all seem ok.
>> Another stream named ok.txt does. So, the below hasn't been tested there
>> yet, but seems like it would work.
>> The solution appears to be rather simple. I have tested it for the
>> file/default stream and for alternative data streams. The answer is just
>> check the first character to see if it is '/'.
> Yep, that's usually the way to check for an absolute path :-).
> I thought you wanted to check if it had the connectpath prepended
> though which is why I didn't point out the obvious 'check for /' :-).
I was wanting to check that. I realized that would only solve the
problem in the default, or similar, case for streams_depot. I then
realized the real solution was to know if it was an absolute path or not
(solving all cases). I am sorry for being so slow on this.
Per your request, it now scans the streams if the streams vfs module is
loaded first. However, in the case of rename, delete or quarantine, the
alternative data streams (if it is the main stream/file being scanned)
will be left in place, i.e. not deleted. I am not sure how to fix this
as the streams_depot or similar must be above, not below, yet to have it
know about the delete, rename, etc. it seems like it has to be above.
Beyond that, I believe I have made all of the changes you have requested
(I may be reverting some changes back to talloc_tos() for the context,
but those changes were done by me and not at your request) save the
using of environment variables when calling scripts, etc. You said you
needed to think about that.
Additionally, I have contacted Sophos and F-Protect without success
seeing if I could get a temporary license to make sure the code still
works. I haven't had responses. I haven't been able to test them. I
would prefer to do this before inclusion, even though the changes in
them were the same/similar done for clamav and actually simpler. The
clamav changes were slightly flawed. I have rechecked the others
visually. Anyone out there willing to help with this? You just need a
license for those (free/trial won't work as they lack the daemons
needed) and Samba, and the module
If you are ok, other than the review, I am ready to merge it back down
into the separate patches and resubmit. Thank you for all of your help.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 872 bytes
Desc: OpenPGP digital signature
More information about the samba-technical