[PATCH 2/7] Samba-VirusFilter: common headers and sources. version 3

SATOH Fumiyasu fumiyas at osstech.co.jp
Wed Nov 2 09:08:25 UTC 2016


Hi,

Thanks for reviewing and adjusting my code...

On Wed, 02 Nov 2016 04:21:24 +0900,
Jeremy Allison wrote:
> > +		buffer[read_size] = '\0';
> > +
> > +		if (read_size == 0) { /* EOF */
> > +			return VIRUSFILTER_RESULT_OK;
> > +		}
> > +
> > +		io_h->r_size += read_size;
> > +
> > +		eol = memmem(io_h->r_buffer, read_size, io_h->r_eol,
> > +			io_h->r_eol_size);
> > +		if (eol) {
> > +			*eol = '\0';
> > +			DEBUG(11, ("Read line data from socket: %s\n",
> > +			      io_h->r_buffer));
> > +			io_h->r_size = eol - io_h->r_buffer;
> > +			io_h->r_rest_size = read_size - (eol - buffer +
> > +				io_h->r_eol_size);
> > +			if (io_h->r_rest_size > 0) {
> > +				io_h->r_rest_buffer = eol + io_h->r_eol_size;
> > +				DEBUG(11, ("Rest data in read buffer: %s, "
> > +				      "size=%ld\n", io_h->r_rest_buffer,
> > +				      (long)io_h->r_rest_size));
> > +			}
> > +			return VIRUSFILTER_RESULT_OK;
> > +		}
> > +
> > +		buffer += read_size;
> > +		buffer_size -= read_size;
> > +	}
> > +
> > +	errno = E2BIG;
> > +
> > +	return VIRUSFILTER_RESULT_ERROR;
> > +}
> > +	if (strncmp("::ffff:", server_addr_p, 7) == 0) {
> > +		server_addr_p += 7;
> > +	}
> > +	virusfilter_env_set(mem_ctx, env_list, "VIRUSFILTER_SERVER_IP",
> > +			    server_addr_p);
> > +	virusfilter_env_set(mem_ctx, env_list, "VIRUSFILTER_SERVER_NAME",
> > +			    myhostname());
> > +	virusfilter_env_set(mem_ctx, env_list,
> > +			    "VIRUSFILTER_SERVER_NETBIOS_NAME",
> > +			    local_machine_name);
> > +	slprintf(pidstr,sizeof(pidstr)-1, "%ld", (long)getpid());
> > +	virusfilter_env_set(mem_ctx, env_list, "VIRUSFILTER_SERVER_PID",
> > +			    pidstr);
> > +
> > +	virusfilter_env_set(mem_ctx, env_list, "VIRUSFILTER_SERVICE_NAME",
> > +			    lp_const_servicename(snum));
> > +	virusfilter_env_set(mem_ctx, env_list, "VIRUSFILTER_SERVICE_PATH",
> > +			    conn->connectpath);
> > +
> > +	client_addr_p = conn_client_addr(conn);
> > +	if (strncmp("::ffff:", client_addr_p, 7) == 0) {
> > +		client_addr_p += 7;
> > +	}
> > +	virusfilter_env_set(mem_ctx, env_list, "VIRUSFILTER_CLIENT_IP",
> > +			    client_addr_p);
> > +	virusfilter_env_set(mem_ctx, env_list, "VIRUSFILTER_CLIENT_NAME",
> > +			    conn_client_name(conn));
> > +	virusfilter_env_set(mem_ctx, env_list,
> > +			    "VIRUSFILTER_CLIENT_NETBIOS_NAME",
> > +			    get_remote_machine_name());
> > +
> > +	virusfilter_env_set(mem_ctx, env_list, "VIRUSFILTER_USER_NAME",
> > +			    get_current_username());
> > +	virusfilter_env_set(mem_ctx, env_list, "VIRUSFILTER_USER_DOMAIN",
> > +			    current_user_info.domain);
> > +
> > +	return 0;
> > +}
> > +
> > +/* Wrapper to Samba's smbrun() in smbrun.c */
> > +int virusfilter_shell_run(
> > +	TALLOC_CTX *mem_ctx,
> > +	const char *cmd,
> > +	char **env_list,
> > +	connection_struct *conn,
> > +	bool sanitize)
> > +{
> > +	if (conn != NULL && virusfilter_shell_set_conn_env(mem_ctx, env_list,
> > +							   conn) == -1) {
> > +		return -1;
> > +	}
> > +
> > +	if (sanitize) {
> > +		return smbrun(cmd, NULL, strv_to_env(talloc_tos(), *env_list));
> > +	} else {
> > +		return smbrun_no_sanitize(cmd, NULL, strv_to_env(talloc_tos(),
> > +					  *env_list));
> > +	}
> > +}
> 
> Urgggh. I *hate* passing these parameters as environment variables.

Why?

> I guess it's more dangerous to pass them as command line parameters
> though, due to shell expansions ?

Yes. I *hate* variable substitutions for command line parameters
in smb.conf. It is too hard to implement safe code for all shell
variants. I *hate* Samba's escape_shell_string() funciton too.

> Can we think more about this ?

With my virusfilter_shell_run() function, the following example
in smb.conf is SAFE without complex ugly escape_shell_string():

  virusfilter_clamav:infected file command = \
    /usr/local/bin/your-virusfilter-notify-command \
      --mail-to virusmaster at example.com \
      --cc "$VIRUSFILTER_USER_NAME at example.com" \
      --from samba-virus-notify at example.com \
      --subject "Samba: Infected File: $VIRUSFILTER_SERVICE_PATH" \
    ;

-- 
-- Name: SATOH Fumiyasu @ OSS Technology Corp. (fumiyas @ osstech co jp)
-- Business Home: http://www.OSSTech.co.jp/
-- GitHub Home: https://GitHub.com/fumiyas/
-- PGP Fingerprint: BBE1 A1C9 525A 292E 6729  CDEC ADC2 9DCA 5E1C CBCA



More information about the samba-technical mailing list