[Pkg-clamav-devel] Bug#636881: Milter socket permissions not set properly
Sebastian Andrzej Siewior
sebastian at breakpoint.cc
Thu Mar 13 20:52:06 UTC 2014
On 2011-10-27 07:16:54 [-0700], Dara Adib wrote:
> See my reply to #636877, but basically one either has to make clamav a
> member of group postfix or set SOCKET_RWGROUP
> in /etc/default/clamav-milter but not in clamav-milter.conf.
>
> > root at domine:/var/spool/postfix/clamav# grep Milter /etc/clamav/clamav-milter.conf
> > MilterSocket /var/spool/postfix/clamav/clamav-milter.ctl
> > MilterSocketGroup postfix
> > MilterSocketMode 660
>
> clamav needs to be a member of group postfix so that it can set postfix
> group ownership for the milter socket.
>
> > s--------- 1 clamav clamav 0 Aug 6 19:20 clamav-milter.ctl
>
> Reproducing this problem, it seems that this is the behavior when
> clamav-milter cannot change the socket group ownership. There should be
> an error message "Failed to change socket ownership to group postfix"
> in syslog.
That error message appears in a recent version.
> And it does that as root. It seems the MilterSocket settings in
> clamav-milter.conf are applied by default after privileges are dropped,
> as clamav by default which can't change group ownership unless it is a
> member of the group.
>
> What works for me (besides adding clamav to group postfix, which might
> be an extra security risk?):
>
> $ grep Milter /etc/clamav/clamav-milter.conf
> MilterSocket /var/spool/postfix/clamav/clamav-milter.ctl
> #MilterSocketGroup postfix # handled by /etc/default/clamav-milter
> MilterSocketMode 660
> $ ls -l
> total 0
> srw-rw---- 1 clamav postfix 0 Oct 27 07:13 clamav-milter.ctl
> $ grep -v ^\# /etc/default/clamav-milter
> SOCKET_RWGROUP=postfix
> SOCKET_PATH=/var/spool/postfix/clamav/clamav-milter.ctl
>
> Since clamav-milter is started as root anyways and then drops privileges
> to user clamav in the default configuration, I would assume that the
> socket group ownership as specified in clamav-milter.conf could be
> changed earlier on as root, and that this would be the preferred fix
> (depending on upstream), obsoleting /etc/default/clamav-milter.
I have the following patch to get this behaviour.
>From dca6bd7cd4544fb2d30cf79fb3b9c4117e6b52c4 Mon Sep 17 00:00:00 2001
From: Sebastian Andrzej Siewior <sebastian at breakpoint.cc>
Date: Tue, 11 Mar 2014 21:09:32 +0100
Subject: [PATCH] clamav-milter: setup socket before dropping root permission
If the milter socket should be setup as
|User clamav
|MilterSocket /var/run/clamav/clamav-milter.ctl
|MilterSocketGroup postfix
|MilterSocketMode 660
and the user clamav is not a member of the postfix then the start fails
with
| -> ERROR: Failed to change socket ownership to group postfix
This is not really required because the process is started as root. It
can first setup the socket and then drop the root permission and change
to the clamav user.
This patch does so. BTS #636881
Signed-off-by: Sebastian Andrzej Siewior <sebastian at breakpoint.cc>
---
clamav-milter/clamav-milter.c | 193 +++++++++++++++++++++---------------------
1 file changed, 98 insertions(+), 95 deletions(-)
diff --git a/clamav-milter/clamav-milter.c b/clamav-milter/clamav-milter.c
index 95ff8a0..49cca71 100644
--- a/clamav-milter/clamav-milter.c
+++ b/clamav-milter/clamav-milter.c
@@ -114,6 +114,104 @@ int main(int argc, char **argv) {
}
}
+ if(!(my_socket = optget(opts, "MilterSocket")->strarg)) {
+ logg("!Please configure the MilterSocket directive\n");
+ logg_close();
+ optfree(opts);
+ return 1;
+ }
+
+ if(smfi_setconn(my_socket) == MI_FAILURE) {
+ logg("!smfi_setconn failed\n");
+ logg_close();
+ optfree(opts);
+ return 1;
+ }
+ if(smfi_register(descr) == MI_FAILURE) {
+ logg("!smfi_register failed\n");
+ logg_close();
+ optfree(opts);
+ return 1;
+ }
+ opt = optget(opts, "FixStaleSocket");
+ umsk = umask(0777); /* socket is created with 000 to avoid races */
+ if(smfi_opensocket(opt->enabled) == MI_FAILURE) {
+ logg("!Failed to create socket %s\n", my_socket);
+ logg_close();
+ optfree(opts);
+ return 1;
+ }
+ umask(umsk); /* restore umask */
+ if(strncmp(my_socket, "inet:", 5) && strncmp(my_socket, "inet6:", 6)) {
+ /* set group ownership and perms on the local socket */
+ char *sock_name = my_socket;
+ mode_t sock_mode;
+ if(!strncmp(my_socket, "unix:", 5))
+ sock_name += 5;
+ if(!strncmp(my_socket, "local:", 6))
+ sock_name += 6;
+ if(*my_socket == ':')
+ sock_name ++;
+
+ if(optget(opts, "MilterSocketGroup")->enabled) {
+ char *gname = optget(opts, "MilterSocketGroup")->strarg, *end;
+ gid_t sock_gid = strtol(gname, &end, 10);
+ if(*end) {
+ struct group *pgrp = getgrnam(gname);
+ if(!pgrp) {
+ logg("!Unknown group %s\n", gname);
+ logg_close();
+ optfree(opts);
+ return 1;
+ }
+ sock_gid = pgrp->gr_gid;
+ }
+ if(chown(sock_name, -1, sock_gid)) {
+ logg("!Failed to change socket ownership to group %s\n", gname);
+ logg_close();
+ optfree(opts);
+ return 1;
+ }
+ }
+
+ if ((opt = optget(opts, "User"))->enabled) {
+ struct passwd *user;
+ if ((user = getpwnam(opt->strarg)) == NULL) {
+ logg("ERROR: Can't get information about user %s.\n",
+ opt->strarg);
+ logg_close();
+ optfree(opts);
+ return 1;
+ }
+
+ if(chown(sock_name, user->pw_uid, -1)) {
+ logg("!Failed to change socket ownership to user %s\n", user->pw_name);
+ optfree(opts);
+ logg_close();
+ return 1;
+ }
+ }
+
+ if(optget(opts, "MilterSocketMode")->enabled) {
+ char *end;
+ sock_mode = strtol(optget(opts, "MilterSocketMode")->strarg, &end, 8);
+ if(*end) {
+ logg("!Invalid MilterSocketMode %s\n", optget(opts, "MilterSocketMode")->strarg);
+ logg_close();
+ optfree(opts);
+ return 1;
+ }
+ } else
+ sock_mode = 0777 & ~umsk;
+
+ if(chmod(sock_name, sock_mode & 0666)) {
+ logg("!Cannot set milter socket permission to %s\n", optget(opts, "MilterSocketMode")->strarg);
+ logg_close();
+ optfree(opts);
+ return 1;
+ }
+ }
+
if(geteuid() == 0 && (opt = optget(opts, "User"))->enabled) {
struct passwd *user = NULL;
if((user = getpwnam(opt->strarg)) == NULL) {
@@ -246,15 +344,6 @@ int main(int argc, char **argv) {
multircpt = optget(opts, "SupportMultipleRecipients")->enabled;
- if(!(my_socket = optget(opts, "MilterSocket")->strarg)) {
- logg("!Please configure the MilterSocket directive\n");
- localnets_free();
- whitelist_free();
- logg_close();
- optfree(opts);
- return 1;
- }
-
if(!optget(opts, "Foreground")->enabled) {
if(daemonize() == -1) {
logg("!daemonize() failed\n");
@@ -269,92 +358,6 @@ int main(int argc, char **argv) {
logg("^Can't change current working directory to root\n");
}
- if(smfi_setconn(my_socket) == MI_FAILURE) {
- logg("!smfi_setconn failed\n");
- localnets_free();
- whitelist_free();
- logg_close();
- optfree(opts);
- return 1;
- }
- if(smfi_register(descr) == MI_FAILURE) {
- logg("!smfi_register failed\n");
- localnets_free();
- whitelist_free();
- logg_close();
- optfree(opts);
- return 1;
- }
- opt = optget(opts, "FixStaleSocket");
- umsk = umask(0777); /* socket is created with 000 to avoid races */
- if(smfi_opensocket(opt->enabled) == MI_FAILURE) {
- logg("!Failed to create socket %s\n", my_socket);
- localnets_free();
- whitelist_free();
- logg_close();
- optfree(opts);
- return 1;
- }
- umask(umsk); /* restore umask */
- if(strncmp(my_socket, "inet:", 5) && strncmp(my_socket, "inet6:", 6)) {
- /* set group ownership and perms on the local socket */
- char *sock_name = my_socket;
- mode_t sock_mode;
- if(!strncmp(my_socket, "unix:", 5))
- sock_name += 5;
- if(!strncmp(my_socket, "local:", 6))
- sock_name += 6;
- if(*my_socket == ':')
- sock_name ++;
-
- if(optget(opts, "MilterSocketGroup")->enabled) {
- char *gname = optget(opts, "MilterSocketGroup")->strarg, *end;
- gid_t sock_gid = strtol(gname, &end, 10);
- if(*end) {
- struct group *pgrp = getgrnam(gname);
- if(!pgrp) {
- logg("!Unknown group %s\n", gname);
- localnets_free();
- whitelist_free();
- logg_close();
- optfree(opts);
- return 1;
- }
- sock_gid = pgrp->gr_gid;
- }
- if(chown(sock_name, -1, sock_gid)) {
- logg("!Failed to change socket ownership to group %s\n", gname);
- localnets_free();
- whitelist_free();
- logg_close();
- optfree(opts);
- return 1;
- }
- }
- if(optget(opts, "MilterSocketMode")->enabled) {
- char *end;
- sock_mode = strtol(optget(opts, "MilterSocketMode")->strarg, &end, 8);
- if(*end) {
- logg("!Invalid MilterSocketMode %s\n", optget(opts, "MilterSocketMode")->strarg);
- localnets_free();
- whitelist_free();
- logg_close();
- optfree(opts);
- return 1;
- }
- } else
- sock_mode = 0777 & ~umsk;
-
- if(chmod(sock_name, sock_mode & 0666)) {
- logg("!Cannot set milter socket permission to %s\n", optget(opts, "MilterSocketMode")->strarg);
- localnets_free();
- whitelist_free();
- logg_close();
- optfree(opts);
- return 1;
- }
- }
-
maxfilesize = optget(opts, "MaxFileSize")->numarg;
if(!maxfilesize) {
logg("^Invalid MaxFileSize, using default (%d)\n", CLI_DEFAULT_MAXFILESIZE);
--
1.9.0
any comments?
> Dara
Sebastian
More information about the Pkg-clamav-devel
mailing list