PATCH: Samba-VirusFilter
Alexander Bokovoy
ab at samba.org
Mon Oct 17 17:35:19 UTC 2016
Hi Trever,
I think there are few more places were clean up is needed. See below.
On ma, 17 loka 2016, Trever L. Adams wrote:
> On 10/16/2016 12:54 PM, SATOH Fumiyasu wrote:
> > On Fri, 14 Oct 2016 22:08:03 +0900,
> > Trever L. Adams wrote:
> >> [1 PATCH: Samba-VirusFilter <multipart/mixed (7bit)>]
> >> [1.1 <text/plain; utf-8 (quoted-printable)>]
> >> This patch relies on the one in the message with subject "Proposed
> >> Patch: write_data_iov_timeout, etc." Beyond that, it should be atomic.
> > Signed-off-by: SATOH Fumiyasu <fumiyas at osstech.co.jp>
> I missed a few coding style related cleanups. Also, the become_root and
> unbecome_root in the quarantine action has been cleaned up some. I now
> only drop it in one place. Before I dropped it in two and one was
> actually a few lines early.
>
> I didn't merge the become_root conditionally or unconditionally with the
> block below as it is my understanding that it is preferred that
> become_root is not conditional and that it should cover the minimal
> amount of code.
>
> I can change this if it is preferred.
>
> Thank you.
>
> Trever
> diff --git a/buildtools/wafsamba/stale_files.py b/buildtools/wafsamba/stale_files.py
> index 2dd08e1..0afbfe4 100644
> --- a/buildtools/wafsamba/stale_files.py
> +++ b/buildtools/wafsamba/stale_files.py
> @@ -88,7 +88,7 @@ def replace_refill_task_list(self):
> link = os.readlink(p)
> if link[0:bin_base_len] == bin_base:
> p = link
> - if f in ['config.h']:
> + if f in ['config.h', 'vfs_virusfilter_config.h']:
> continue
> (froot, fext) = os.path.splitext(f)
> if fext not in [ '.c', '.h', '.so', '.o' ]:
> diff --git a/docs-xml/manpages/vfs_virusfilter_clamav.8.xml b/docs-xml/manpages/vfs_virusfilter_clamav.8.xml
> new file mode 100644
> index 0000000..2483368
> --- /dev/null
> +++ b/docs-xml/manpages/vfs_virusfilter_clamav.8.xml
> @@ -0,0 +1,295 @@
> +<?xml version="1.0" encoding="iso-8859-1"?>
> +<!DOCTYPE refentry PUBLIC "-//Samba-Team//DTD DocBook V4.2-Based Variant V1.0//EN" "http://www.samba.org/samba/DTD/samba-doc">
> +<refentry id="vfs_virusfilter_clamav.8">
> +
> +<refmeta>
> + <refentrytitle>vfs_virusfilter_clamav</refentrytitle>
> + <manvolnum>8</manvolnum>
> + <refmiscinfo class="source">Samba</refmiscinfo>
> + <refmiscinfo class="manual">System Administration tools</refmiscinfo>
> + <refmiscinfo class="version">4.4</refmiscinfo>
> +</refmeta>
This is going to git master, so update the version to 4.6, please.
> diff --git a/docs-xml/manpages/vfs_virusfilter_fsav.8.xml b/docs-xml/manpages/vfs_virusfilter_fsav.8.xml
> new file mode 100644
> index 0000000..a9171e6
> --- /dev/null
> +++ b/docs-xml/manpages/vfs_virusfilter_fsav.8.xml
> @@ -0,0 +1,319 @@
> +<?xml version="1.0" encoding="iso-8859-1"?>
> +<!DOCTYPE refentry PUBLIC "-//Samba-Team//DTD DocBook V4.2-Based Variant V1.0//EN" "http://www.samba.org/samba/DTD/samba-doc">
> +<refentry id="vfs_virusfilter_fsav.8">
> +
> +<refmeta>
> + <refentrytitle>vfs_virusfilter_fsav</refentrytitle>
> + <manvolnum>8</manvolnum>
> + <refmiscinfo class="source">Samba</refmiscinfo>
> + <refmiscinfo class="manual">System Administration tools</refmiscinfo>
> + <refmiscinfo class="version">4.4</refmiscinfo>
> +</refmeta>
Same here.
> diff --git a/docs-xml/manpages/vfs_virusfilter_sophos.8.xml b/docs-xml/manpages/vfs_virusfilter_sophos.8.xml
> new file mode 100644
> index 0000000..34f783a
> --- /dev/null
> +++ b/docs-xml/manpages/vfs_virusfilter_sophos.8.xml
> @@ -0,0 +1,312 @@
> +<?xml version="1.0" encoding="iso-8859-1"?>
> +<!DOCTYPE refentry PUBLIC "-//Samba-Team//DTD DocBook V4.2-Based Variant V1.0//EN" "http://www.samba.org/samba/DTD/samba-doc">
> +<refentry id="vfs_virusfilter_sophos.8">
> +
> +<refmeta>
> + <refentrytitle>vfs_virusfilter_sophos</refentrytitle>
> + <manvolnum>8</manvolnum>
> + <refmiscinfo class="source">Samba</refmiscinfo>
> + <refmiscinfo class="manual">System Administration tools</refmiscinfo>
> + <refmiscinfo class="version">4.4</refmiscinfo>
> +</refmeta>
Same here.
> --- /dev/null
> +++ b/source3/modules/vfs_virusfilter_fsav.c
> @@ -0,0 +1,375 @@
> +/*
> + Samba-VirusFilter VFS modules
> + F-Secure Anti-Virus fsavd support
> + Copyright (C) 2010-2016 SATOH Fumiyasu @ OSS Technology Corp., Japan
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>.
> +*/
> +
> +#define VIRUSFILTER_ENGINE fsav
> +#define VIRUSFILTER_MODULE_ENGINE "fsav"
> +
> +/* Default values for standard "extra" configuration variables */
> +#define VIRUSFILTER_DEFAULT_SOCKET_PATH "/tmp/.fsav-0"
> +#define VIRUSFILTER_DEFAULT_CONNECT_TIMEOUT 30000 /* msec */
> +#define VIRUSFILTER_DEFAULT_TIMEOUT 60000 /* msec */
> +#define VIRUSFILTER_DEFAULT_SCAN_ARCHIVE false
> +#define VIRUSFILTER_DEFAULT_MAX_NESTED_SCAN_ARCHIVE 1
> +#define VIRUSFILTER_DEFAULT_SCAN_REQUEST_LIMIT 0
> +#define VIRUSFILTER_DEFAULT_SCAN_MIME false
> +/* Default values for module-specific configuration variables */
> +/* 5 = F-Secure Linux 7 or later? */
> +#define VIRUSFILTER_DEFAULT_FSAV_PROTOCOL 5
> +#define VIRUSFILTER_DEFAULT_SCAN_RISKWARE false
> +#define VIRUSFILTER_DEFAULT_STOP_SCAN_ON_FIRST true
> +#define VIRUSFILTER_DEFAULT_FILTER_FILENAME false
> +
> +#define VIRUSFILTER_MODULE_CONFIG_MEMBERS \
> + int fsav_protocol; \
> + bool scan_riskware; \
> + bool stop_scan_on_first; \
> + bool filter_filename; \
> + /* End of VIRUSFILTER_MODULE_CONFIG_MEMBERS */
> +
> +#define virusfilter_module_connect virusfilter_fsav_connect
> +#define virusfilter_module_destruct_config \
> + virusfilter_fsav_destruct_config
> +#define virusfilter_module_scan_init virusfilter_fsav_scan_init
> +#define virusfilter_module_scan_end virusfilter_fsav_scan_end
> +#define virusfilter_module_scan virusfilter_fsav_scan
> +
> +#include "vfs_virusfilter_vfs.c"
> +
> +/* ====================================================================== */
> +
> +#include "vfs_virusfilter_utils.h"
> +
> +/* ====================================================================== */
> +
> +static int virusfilter_fsav_connect(
> + vfs_handle_struct *vfs_h,
> + virusfilter_handle *virusfilter_h,
> + const char *svc,
> + const char *user)
> +{
> + int snum = SNUM(vfs_h->conn);
> +
> + virusfilter_h->fsav_protocol = lp_parm_int(snum,
> + VIRUSFILTER_MODULE_NAME, "fsav protocol",
> + VIRUSFILTER_DEFAULT_FSAV_PROTOCOL);
> +
> + virusfilter_h->scan_riskware = lp_parm_bool(snum,
> + VIRUSFILTER_MODULE_NAME, "scan riskware",
> + VIRUSFILTER_DEFAULT_SCAN_RISKWARE);
> +
> + virusfilter_h->stop_scan_on_first = lp_parm_bool(snum,
> + VIRUSFILTER_MODULE_NAME, "stop scan on first",
> + VIRUSFILTER_DEFAULT_STOP_SCAN_ON_FIRST);
> +
> + virusfilter_h->filter_filename = lp_parm_bool(snum,
> + VIRUSFILTER_MODULE_NAME, "filter filename",
> + VIRUSFILTER_DEFAULT_FILTER_FILENAME);
> +
> + return 0;
> +}
> +
> +static int virusfilter_fsav_destruct_config(
> + virusfilter_handle *virusfilter_h)
> +{
> + virusfilter_fsav_scan_end(virusfilter_h);
> +
> + return 0;
> +}
> +
> +static virusfilter_result virusfilter_fsav_scan_init(
> + virusfilter_handle *virusfilter_h)
> +{
> + virusfilter_io_handle *io_h = virusfilter_h->io_h;
> + virusfilter_result result;
> +
> + if (io_h->socket != -1) {
> + DEBUG(10,("fsavd: Checking if connection is alive\n"));
> +
> + /* FIXME: I don't know the correct PING command format... */
> + if (virusfilter_io_writefl_readl(io_h, "PING") ==
> + VIRUSFILTER_RESULT_OK)
> + {
> + if (strn_eq(io_h->r_buffer, "ERROR\t", 6)) {
> + DEBUG(10,("fsavd: Re-using existent "
> + "connection\n"));
> + return VIRUSFILTER_RESULT_OK;
> + }
> + }
> +
> + DEBUG(10,("fsavd: Closing dead connection\n"));
> + virusfilter_fsav_scan_end(virusfilter_h);
> + }
> +
> + DEBUG(7,("fsavd: Connecting to socket: %s\n",
> + virusfilter_h->socket_path));
> +
> + become_root();
> + result = virusfilter_io_connect_path(io_h, virusfilter_h->socket_path);
> + unbecome_root();
> +
> + if (result != VIRUSFILTER_RESULT_OK) {
> + DEBUG(0,("fsavd: Connecting to socket failed: %s: %s\n",
> + virusfilter_h->socket_path, strerror(errno)));
> + return VIRUSFILTER_RESULT_ERROR;
> + }
> +
> + if (virusfilter_io_readl(io_h) != VIRUSFILTER_RESULT_OK) {
> + DEBUG(0,("fsavd: Reading greeting message failed: %s\n",
> + strerror(errno)));
> + goto virusfilter_fsav_init_failed;
> + }
> + if (!strn_eq(io_h->r_buffer, "DBVERSION\t", 10)) {
> + DEBUG(0,("fsavd: Invalid greeting message: %s\n",
> + io_h->r_buffer));
> + goto virusfilter_fsav_init_failed;
> + }
> +
> + DEBUG(10,("fsavd: Connected\n"));
> +
> + DEBUG(7,("fsavd: Configuring\n"));
> +
> + if (virusfilter_io_writefl_readl(io_h,
> + "PROTOCOL\t%d", virusfilter_h->fsav_protocol)
> + != VIRUSFILTER_RESULT_OK) {
> + DEBUG(0,("fsavd: PROTOCOL: I/O error: %s\n", strerror(errno)));
> + goto virusfilter_fsav_init_failed;
> + }
> + if (!strn_eq(io_h->r_buffer, "OK\t", 3)) {
> + DEBUG(0,("fsavd: PROTOCOL: Not accepted: %s\n",
> + io_h->r_buffer));
> + goto virusfilter_fsav_init_failed;
> + }
> +
> +#if 0 /* FIXME */
> + if (virusfilter_io_writefl_readl(io_h,
> + "CONFIGURE\tTIMEOUT\t%d", virusfilter_h->timeout / 1000)
> + != VIRUSFILTER_RESULT_OK) {
> + DEBUG(0,("fsavd: CONFIGURE TIMEOUT: I/O error: %s\n",
> + strerror(errno)));
> + goto virusfilter_fsav_init_failed;
> + }
> + if (!strn_eq(io_h->r_buffer, "OK\t", 3)) {
> + DEBUG(0,("fsavd: CONFIGURE TIMEOUT: Not accepted: %s\n",
> + io_h->r_buffer));
> + goto virusfilter_fsav_init_failed;
> + }
> +#endif
Could you please either add a more detailed comment what's broken here
or remove this '#if 0' at all?
> + } else if (str_eq(reply_token, "SUSPECTED") ||
> + str_eq(reply_token, "ARCHIVE_SUSPECTED") ||
> + str_eq(reply_token, "MIME_SUSPECTED")) {
> +#if 0
> + // FIXME: Block if "block suspected file" option is
> + // true
> + result = VIRUSFILTER_RESULT_SUSPECTED;
> + ...
> +#else
> + /* Ignore */
> +#endif
Could you please use single style for comments?
(There are more places like this)
> + }
> + *dst_c++ = *src;
> + dst_size--;
> + }
> + }
> +
> + *dst_c = '\0';
> +
> + return (dst_c - dst);
> +}
> +
> +#if SAMBA_VERSION_NUMBER >= 30600
> +/*********************************************************
> + For rename across filesystems initial Patch from Warren Birnbaum
> + <warrenb at hpcvscdp.cv.hp.com>
> +**********************************************************/
It is good to have compatibility code but if we are going to merge this
to git master, can we remove old code or wrapping against the old code?
(There are more places like this)
> + {
> + case -1:
> + return VIRUSFILTER_RESULT_ERROR;
> + default:
> + return VIRUSFILTER_RESULT_OK;
> + }
> +
> +#if 0
> + /* Not reached */
> + return VIRUSFILTER_RESULT_OK;
> +#endif
> +}
Can we remove this '#if 0'?
(There are more places like this)
--
/ Alexander Bokovoy
More information about the samba-technical
mailing list