Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config settings for sockets permissions and some minor fixes. #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 5 additions & 20 deletions debian/rmilter.init
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
# Description: another spam-defense service
### END INIT INFO



# Based on skeleton by Miquel van Smoorenburg and Ian Murdock

PATH=/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/bin
Expand All @@ -25,8 +23,7 @@ DESC="Rmilter Mail Filter Daemon"
PIDFILE="/var/run/$NAME.pid"
PNAME="rmilter"
USER="rmilter"
SOCKET=/var/lib/rmilter/rmilter.sock

SOCKET=/var/spool/postfix/rmilter/rmilter.sock
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a wrong path actually. The problem is that rmilter is not intended to run with Postfix MTA only, therefore it should create socket into some other path. The proper solution is to read from /etc/defaults/rmilter the name of the socket and use it in the init script.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible to override any variable, because bellow occured that line:

test -f /etc/default/rmilter && . /etc/default/rmilter


[ -x $DAEMON ] || DAEMON=/usr/sbin/rmilter
[ -x $DAEMON ] || exit 0
Expand Down Expand Up @@ -68,7 +65,7 @@ set -e
case "$1" in
start)
echo -n "Starting $DESC: "
rm -f /var/lib/rmilter/rmilter.sock
rm -f $SOCKET
start-stop-daemon --start --background --make-pidfile --pidfile $PIDFILE \
--chuid $USER --name $PNAME $NICE --oknodo --startas $DAEMON -- \
$OPTIONS $DOPTIONS
Expand All @@ -78,27 +75,15 @@ case "$1" in
stop)
echo -n "Stopping $DESC: "
start-stop-daemon --stop --pidfile $PIDFILE --name $PNAME --oknodo
rm -f /var/lib/rmilter/rmilter.sock
echo "$NAME."
;;

restart|force-reload)
echo -n "Restarting $DESC: "
start-stop-daemon --stop --pidfile $PIDFILE --name $PNAME \
--retry 5 --oknodo
rm -f /var/lib/rmilter/rmilter.sock
start-stop-daemon --start --background --make-pidfile --pidfile $PIDFILE \
--chuid $USER --name $PNAME $NICE --oknodo --startas $DAEMON -- \
$OPTIONS $DOPTIONS

rm -f $SOCKET
echo "$NAME."
;;

reload)
restart|reload|force-reload)
echo -n "Restarting $DESC: "
start-stop-daemon --stop --pidfile $PIDFILE --name $PNAME \
--retry 5 --oknodo
rm -f /var/lib/rmilter/rmilter.sock
rm -f $SOCKET
start-stop-daemon --start --background --make-pidfile --pidfile $PIDFILE \
--chuid $USER --name $PNAME $NICE --oknodo --startas $DAEMON -- \
$OPTIONS $DOPTIONS
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, both old and new versions are wrong. Rmilter supports graceful reload without a requirement to perform the full restart (implemented by handling SIGHUP signal). Hence, I guess that this behaviour should also be reworked.

Expand Down
2 changes: 2 additions & 0 deletions include/cfg_file.h
Original file line number Diff line number Diff line change
Expand Up @@ -248,10 +248,12 @@ struct config_file {
char *temp_dir;

char *sock_cred;
unsigned int sock_cred_mode;
size_t sizelimit;

struct clamav_server clamav_servers[MAX_CLAMAV_SERVERS];
size_t clamav_servers_num;
unsigned int clamav_file_mode;
unsigned int clamav_error_time;
unsigned int clamav_dead_time;
unsigned int clamav_maxerrors;
Expand Down
7 changes: 5 additions & 2 deletions src/cfg_file.l
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ spam_server return SPAM_SERVER;
error_time return ERROR_TIME;
dead_time return DEAD_TIME;
maxerrors return MAXERRORS;
file_mode return FILE_MODE;
connect_timeout return CONNECT_TIMEOUT;
port_timeout return PORT_TIMEOUT;
results_timeout return RESULTS_TIMEOUT;
Expand Down Expand Up @@ -223,6 +224,7 @@ sha256 return DKIM_SHA256;
protocol return PROTOCOL;
spf_domains return SPF;
bind_socket return BINDSOCK;
bind_socket_mode return BINDSOCK_MODE;
max_size return MAXSIZE;
use_dcc return USEDCC;
greylisting return GREYLISTING;
Expand Down Expand Up @@ -270,10 +272,11 @@ yes|YES|no|NO|[yY]|[nN] yylval.flag=parse_flag(yytext); return FLAG;
\n /* ignore EOL */;
[ \t]+ /* ignore whitespace */;
\".+\" yylval.string=strdup(yytext); return QUOTEDSTRING;
[0-9]+ yylval.number=strtol(yytext, NULL, 10); return NUMBER;
[1-9][0-9]+ yylval.number=strtol(yytext, NULL, 10); return NUMBER;
0[xX][0-9a-fA-F]+|0[0-7]+ yylval.number=strtoul(yytext, NULL, 0); return NUMBER;
[0-9]+\.[0-9]* yylval.frac=strtod(yytext, NULL); return FLOAT;
[0-9]+[kKmMgG]? yylval.limit=parse_limit(yytext); return SIZELIMIT;
[0-9]+[sShHdD]|[0-9]+[mM][sS] yylval.seconds=parse_seconds(yytext); return SECONDS;
[0-9]+[sSmMhHdD]? yylval.seconds=parse_seconds(yytext); return SECONDS;
[0-9]+:[0-9]+[.]?[0-9]* parse_bucket(yytext, &yylval.bucket); return BUCKET;
unix:[a-zA-Z0-9\/.-]+ yylval.string=strdup(yytext); return SOCKCRED;
local:[a-zA-Z0-9\/.-]+ yylval.string=strdup(yytext); return SOCKCRED;
Expand Down
17 changes: 15 additions & 2 deletions src/cfg_file.y
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ uint8_t cur_flags = 0;
%token CONNECT HELO ENVFROM ENVRCPT HEADER MACRO BODY
%token AND OR NOT
%token TEMPDIR LOGFILE PIDFILE RULE CLAMAV SERVERS ERROR_TIME DEAD_TIME MAXERRORS CONNECT_TIMEOUT PORT_TIMEOUT RESULTS_TIMEOUT SPF DCC
%token FILENAME REGEXP QUOTE SEMICOLON OBRACE EBRACE COMMA EQSIGN
%token BINDSOCK SOCKCRED DOMAIN_STR IPADDR IPNETWORK HOSTPORT NUMBER GREYLISTING WHITELIST TIMEOUT EXPIRE EXPIRE_WHITE
%token FILE_MODE FILENAME REGEXP QUOTE SEMICOLON OBRACE EBRACE COMMA EQSIGN
%token BINDSOCK BINDSOCK_MODE SOCKCRED DOMAIN_STR IPADDR IPNETWORK HOSTPORT NUMBER GREYLISTING WHITELIST TIMEOUT EXPIRE EXPIRE_WHITE
%token MAXSIZE SIZELIMIT SECONDS BUCKET USEDCC MEMCACHED PROTOCOL AWL_ENABLE AWL_POOL AWL_TTL AWL_HITS SERVERS_WHITE SERVERS_LIMITS SERVERS_GREY
%token LIMITS LIMIT_TO LIMIT_TO_IP LIMIT_TO_IP_FROM LIMIT_WHITELIST LIMIT_WHITELIST_RCPT LIMIT_BOUNCE_ADDRS LIMIT_BOUNCE_TO LIMIT_BOUNCE_TO_IP
%token SPAMD REJECT_MESSAGE SERVERS_ID ID_PREFIX GREY_PREFIX WHITE_PREFIX RSPAMD_METRIC ALSO_CHECK DIFF_DIR CHECK_SYMBOLS SYMBOLS_DIR
Expand Down Expand Up @@ -102,6 +102,7 @@ command :
| spamd
| spf
| bindsock
| bindsock_mode
| maxsize
| usedcc
| memcached
Expand Down Expand Up @@ -313,6 +314,7 @@ clamavbody:

clamavcmd:
clamav_servers
| clamav_file_mode
| clamav_connect_timeout
| clamav_port_timeout
| clamav_results_timeout
Expand Down Expand Up @@ -371,6 +373,11 @@ clamav_maxerrors:
cfg->clamav_maxerrors = $3;
}
;
clamav_file_mode:
FILE_MODE EQSIGN NUMBER {
cfg->clamav_file_mode = $3;
}
;
clamav_connect_timeout:
CONNECT_TIMEOUT EQSIGN SECONDS {
cfg->clamav_connect_timeout = $3;
Expand Down Expand Up @@ -805,6 +812,12 @@ bindsock:
}
;

bindsock_mode:
BINDSOCK_MODE EQSIGN NUMBER {
cfg->sock_cred_mode = $3;
}
;

maxsize:
MAXSIZE EQSIGN SIZELIMIT {
cfg->sizelimit = $3;
Expand Down
5 changes: 5 additions & 0 deletions src/libclamc.c
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,11 @@ clamscan_socket(const char *file, const struct clamav_server *srv, char *strres,
msg_warn("clamav: realpath, %d: %m", errno);
return -1;
}

/* Set temp file permissions if them specified in config */
if (cfg->clamav_file_mode)
chmod(path, cfg->clamav_file_mode);

/* unix socket, use 'SCAN <filename>' command on clamd */
r = snprintf(buf, sizeof(buf), "SCAN %s\n", path);

Expand Down
9 changes: 3 additions & 6 deletions src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -252,12 +252,9 @@ main(int argc, char *argv[])
srand (time (NULL));
#endif

/*
* Hack to set milter unix socket permissions, but it also affect
* temporary file too :( temporary directory shuld be owned by user
* rmilter-clam and have permissions 700
*/
umask(0007);
/* Set unix socket permissions if specified in config */
if (cfg->sock_cred_mode)
umask(0777 & ~cfg->sock_cred_mode);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with this code is that a user may specify insecure attributes for temporary files. Nevertheless, I don't know indeed what to do with brain-damaged libmilter that has absolutely fucked API...


smfi_setconn(cfg->sock_cred);
if (smfi_register(smfilter) == MI_FAILURE) {
Expand Down