Hi, my setup is as follows: I use Qsmtp[1] to do the SMTP part, use (net)qmail for the queueing, use vpopmail 5.4.33 for the user/domain stuff. The users get their mail through IMAP or POP using dovecot. For some users I have maildrop and/or spamassassin enabled.
When spamassassin is enabled but maildrop is not I see failed assertions in dovecots POP server[2], which are caused by vdelivermail using a wrong filename. The filename, more exactly the S=<size> value is calculated _before_ the mail is piped into spamassassin, which adds two more header lines with it's scan results, so the actual size afterwards is bigger than what is recorded. The attached patch #5 fixes this for me, with some cleanups in #1-#4 I did on the way to find the culprit. Also attached is another patch I on my server to make the user and domain directories world-accessible so the incoming SMTP process can read config files from the user and domain directories for user-defined filtering. The maildirs itself are still 0700 so it has no access to them. While talking about patches, here are the patches that Gentoo applies to vpopmail, which may or may not be useful for being taken upstream: http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/net-mail/vpopmail/files/ Eike 1) shameless plug: http://opensource.sf-tec.de/Qsmtp/ 2) http://dovecot.org/pipermail/dovecot/2014-August/097548.html
From e402c2f49b25d78af3e9ee90b030678972294755 Mon Sep 17 00:00:00 2001
From: Rolf Eike Beer <k...@opensource.sf-tec.de>
Date: Thu, 21 Aug 2014 17:34:27 +0200
Subject: [PATCH 1/5] vdelivermail: add static
---
vdelivermail.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/vdelivermail.c b/vdelivermail.c
index d94129f..241106e 100644
--- a/vdelivermail.c
+++ b/vdelivermail.c
@@ -322,7 +322,7 @@ int process_valias(void)
#endif
/* Forks off qmail-inject. Returns PID of child, or 0 for failure. */
-pid_t qmail_inject_open(char *address)
+static pid_t qmail_inject_open(char *address)
{
int pim[2];
pid_t pid;
@@ -351,7 +351,7 @@ pid_t qmail_inject_open(char *address)
return(pid);
}
-int fdcopy (int write_fd, int read_fd, const char *extra_headers, size_t headerlen, char *address)
+static int fdcopy (int write_fd, int read_fd, const char *extra_headers, size_t headerlen, char *address)
{
char msgbuf[4096];
ssize_t file_count;
--
1.8.4.5
From af7e1c5ede39340e12f93253b6a53f5459c97900 Mon Sep 17 00:00:00 2001
From: Rolf Eike Beer <k...@opensource.sf-tec.de>
Date: Thu, 21 Aug 2014 17:36:36 +0200
Subject: [PATCH 2/5] fix those vfork() instances that do more than exec*()
---
vdelivermail.c | 4 ++--
vpopmail.c | 8 ++++----
vqmaillocal.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/vdelivermail.c b/vdelivermail.c
index 241106e..be83a2a 100644
--- a/vdelivermail.c
+++ b/vdelivermail.c
@@ -330,7 +330,7 @@ static pid_t qmail_inject_open(char *address)
if ( pipe(pim) == -1) return 0;
- switch(pid=vfork()){
+ switch(pid=fork()){
case -1:
close(pim[0]);
close(pim[1]);
@@ -381,7 +381,7 @@ static int fdcopy (int write_fd, int read_fd, const char *extra_headers, size_t
!(vpw->pw_gid & NO_SPAMASSASSIN) ) {
if (!pipe(pim)) {
- pid = vfork();
+ pid = fork();
switch (pid) {
case -1:
close(pim[0]);
diff --git a/vpopmail.c b/vpopmail.c
index a2bdc0b..7a4657f 100644
--- a/vpopmail.c
+++ b/vpopmail.c
@@ -1472,9 +1472,9 @@ int update_newu()
{
int pid;
- pid=vfork();
+ pid=fork();
if ( pid==0){
- umask(022);
+ umask(022);
execl(QMAILNEWU,"qmail-newu", NULL);
exit(127);
} else {
@@ -3360,9 +3360,9 @@ long unsigned tcprules_open()
/* create a pair of filedescriptors for our pipe */
if (pipe(pim) == -1) { return(-1);}
- switch( pid=vfork()){
+ switch( pid=fork()){
case -1:
- /* vfork error. close pipes and exit */
+ /* fork error. close pipes and exit */
close(pim[0]); close(pim[1]);
return(-1);
case 0:
diff --git a/vqmaillocal.c b/vqmaillocal.c
index 6d3068c..80efa24 100644
--- a/vqmaillocal.c
+++ b/vqmaillocal.c
@@ -359,7 +359,7 @@ long unsigned qmail_inject_open(char *address)
if ( pipe(pim) == -1) return(-1);
- switch(pid=vfork()){
+ switch(pid=fork()){
case -1:
close(pim[0]);
close(pim[1]);
--
1.8.4.5
From 88d305d9f072639ed4bc2705b46708e706a50ffc Mon Sep 17 00:00:00 2001 From: Rolf Eike Beer <k...@opensource.sf-tec.de> Date: Thu, 21 Aug 2014 17:45:28 +0200 Subject: [PATCH 3/5] remove unneeded forward declaration --- vchkpw.c | 1 - 1 file changed, 1 deletion(-) diff --git a/vchkpw.c b/vchkpw.c index b1c8a5d..d7d4351 100644 --- a/vchkpw.c +++ b/vchkpw.c @@ -91,7 +91,6 @@ void login_system_user(); void read_user_pass(); void vlog(int verror, char *TheUser, char *TheDomain, char *ThePass, char *TheName, char *IpAddr, char *LogLine); void vchkpw_exit(int err); -void run_command(char *prog); int authcram( char *response, char *challenge, char *password); int authapop( char *password, char *timestamp, char *clearpass); -- 1.8.4.5
From 15eb41ec3f89a1ee853e0ce392bf16959ed04618 Mon Sep 17 00:00:00 2001
From: Rolf Eike Beer <k...@opensource.sf-tec.de>
Date: Thu, 21 Aug 2014 17:55:27 +0200
Subject: [PATCH 4/5] clean up calling maildrop
-add const for arguments
-do not prepend "| " to call for preline, run_command() will then just skip
over this anyway
-put the buffer for the maildrop command in the most local scope
---
vdelivermail.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/vdelivermail.c b/vdelivermail.c
index be83a2a..2ad2e12 100644
--- a/vdelivermail.c
+++ b/vdelivermail.c
@@ -94,7 +94,7 @@ ssize_t get_message_size();
void deliver_mail(char *address, char *quota);
int check_forward_deliver(char *dir);
int is_looping( char *address );
-void run_command(char *prog);
+static void run_command(const char *prog);
void checkuser(void);
void usernotfound(void);
int is_loop_match( const char *dt, const char *address);
@@ -360,9 +360,6 @@ static int fdcopy (int write_fd, int read_fd, const char *extra_headers, size_t
long unsigned pid;
int pim[2];
#endif
-#ifdef MAILDROP
- char maildrop_command[256];
-#endif
/* write the Return-Path: and Delivered-To: headers */
if (headerlen > 0) {
@@ -409,7 +406,8 @@ static int fdcopy (int write_fd, int read_fd, const char *extra_headers, size_t
#ifdef MAILDROP
if ( limits.disable_maildrop==0 && vpw!=NULL &&
!(vpw->pw_gid & NO_MAILDROP) ) {
- sprintf(maildrop_command, "| preline %s", MAILDROP_PROG);
+ char maildrop_command[256];
+ sprintf(maildrop_command, "preline %s", MAILDROP_PROG);
run_command(maildrop_command);
DeleteMail = 1;
return(0);
@@ -896,13 +894,13 @@ void (*f)();
/* open a pipe to a command
* return the pid or -1 if error
*/
-void run_command(char *prog)
+void run_command(const char *prog)
{
#define MAX_ENV_BUFF 100
int child;
- char *(args[4]);
+ const char *(args[4]);
int wstat;
while ((*prog == ' ') || (*prog == '|')) ++prog;
@@ -915,7 +913,7 @@ void run_command(char *prog)
case 0:
putenv("SHELL=/bin/sh");
- args[0] = "/bin/sh"; args[1] = "-c"; args[2] = prog; args[3] = 0;
+ args[0] = "/bin/sh"; args[1] = "-c"; args[2] = prog; args[3] = NULL;
sig_catch(SIGPIPE,SIG_DFL);
execv(*args,args);
printf("Unable to run /bin/sh: %d.", errno);
--
1.8.4.5
From 620adb37e092af6030f2b29eee9d18d2134b3f0c Mon Sep 17 00:00:00 2001
From: Rolf Eike Beer <k...@opensource.sf-tec.de>
Date: Thu, 21 Aug 2014 18:48:52 +0200
Subject: [PATCH 5/5] fix ,S= tag in case spamassassin changed the file size
---
vdelivermail.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/vdelivermail.c b/vdelivermail.c
index 2ad2e12..fd48389 100644
--- a/vdelivermail.c
+++ b/vdelivermail.c
@@ -351,6 +351,12 @@ static pid_t qmail_inject_open(char *address)
return(pid);
}
+/**
+ * @returns if delivery worked
+ * @retval 0 message was delivered as is
+ * @retval 1 the file has changed the size during delivery
+ * @retval -1 error
+ */
static int fdcopy (int write_fd, int read_fd, const char *extra_headers, size_t headerlen, char *address)
{
char msgbuf[4096];
@@ -360,6 +366,7 @@ static int fdcopy (int write_fd, int read_fd, const char *extra_headers, size_t
long unsigned pid;
int pim[2];
#endif
+ int has_changed_size = 0;
/* write the Return-Path: and Delivered-To: headers */
if (headerlen > 0) {
@@ -399,6 +406,7 @@ static int fdcopy (int write_fd, int read_fd, const char *extra_headers, size_t
close(pim[1]);
dup2(pim[0], 0);
close(pim[0]);
+ has_changed_size = 1;
}
}
#endif
@@ -431,7 +439,7 @@ static int fdcopy (int write_fd, int read_fd, const char *extra_headers, size_t
if ( write(write_fd, msgbuf, file_count) == -1 ) return -1;
}
- return 0;
+ return has_changed_size;
}
void read_quota_from_maildir (const char *maildir, char *qbuf, size_t qlen)
@@ -493,6 +501,7 @@ int deliver_to_maildir (
size_t headerlen;
int write_fd;
char quota[80];
+ int fdr;
headerlen = strlen (extra_headers);
msgsize += headerlen;
@@ -518,7 +527,8 @@ int deliver_to_maildir (
}
local = 1;
- if (fdcopy(write_fd, read_fd, extra_headers, headerlen, maildir_to_email(maildir)) != 0) {
+ fdr = fdcopy(write_fd, read_fd, extra_headers, headerlen, maildir_to_email(maildir));
+ if (fdr < 0) {
/* Did the write fail because we were over quota? */
if ( errno == EDQUOT ) {
close(write_fd);
@@ -530,6 +540,14 @@ int deliver_to_maildir (
unlink (local_file_tmp);
return -2;
}
+ } else if (fdr == 1) {
+ /* the file has changed it's size during delivery, e.g. because
+ * SpamAssassin has written it's report to it. */
+ struct stat st;
+
+ if (fstat(write_fd, &st) == 0 && st.st_size != msgsize)
+ snprintf(local_file_new, sizeof(local_file_new), "%snew/%lu.%lu.%.32s,S=%lu",
+ maildir, tm, pid, hostname, (long unsigned) st.st_size);
}
/* completed write to tmp directory, now move it into the new directory */
@@ -777,7 +795,7 @@ void deliver_mail(char *address, char *quota)
}
local = 0;
- if (fdcopy (fdm, 0, DeliveredTo, strlen(DeliveredTo), address) != 0) {
+ if (fdcopy (fdm, 0, DeliveredTo, strlen(DeliveredTo), address) < 0) {
printf ("write to qmail-inject failed: %d\n", errno);
close(fdm);
waitpid(inject_pid,&child,0);
--
1.8.4.5
diff -Naurp a/vpopmail-5.4.33/Makefile.am b/vpopmail-5.4.33/Makefile.am
--- a/vpopmail-5.4.33/Makefile.am 2011-02-28 18:00:45.000000000 +0100
+++ b/vpopmail-5.4.33/Makefile.am 2013-12-25 11:45:43.720000000 +0100
@@ -141,7 +141,7 @@ install-exec-am:
done
install-data-local:
- $(INSTALL) -d -g @vpopgroup@ -m 0700 -o @vpopuser@ \
+ $(INSTALL) -d -g @vpopgroup@ -m 0751 -o @vpopuser@ \
$(DESTDIR)@vpopmaildir@/@domains_dir@
echo "-I@vpopmaildir@/include" > \
@@ -212,7 +212,7 @@ fix-priv:
@echo "If the recursive chown is taking a long time"
@echo "go ahead and break out of it by pressing control-C"
@echo "this is the last stage of the install and can be skipped"
- chmod 700 $(DESTDIR)@vpopmaildir@/@domains_dir@
+ chmod 751 $(DESTDIR)@vpopmaildir@/@domains_dir@
chown -R @vpopuser@ $(DESTDIR)@vpopmaildir@/@domains_dir@
chgrp -R @vpopgroup@ $(DESTDIR)@vpopmaildir@/@domains_dir@
diff -Naurp a/vpopmail-5.4.33/Makefile.in b/vpopmail-5.4.33/Makefile.in
--- a/vpopmail-5.4.33/Makefile.in 2011-02-28 18:00:45.000000000 +0100
+++ b/vpopmail-5.4.33/Makefile.in 2013-12-25 11:45:43.720000000 +0100
@@ -1167,7 +1167,7 @@ install-exec-am:
done
install-data-local:
- $(INSTALL) -d -g @vpopgroup@ -m 0700 -o @vpopuser@ \
+ $(INSTALL) -d -g @vpopgroup@ -m 0751 -o @vpopuser@ \
$(DESTDIR)@vpopmaildir@/@domains_dir@
echo "-I@vpopmaildir@/include" > \
@@ -1238,7 +1238,7 @@ fix-priv:
@echo "If the recursive chown is taking a long time"
@echo "go ahead and break out of it by pressing control-C"
@echo "this is the last stage of the install and can be skipped"
- chmod 700 $(DESTDIR)@vpopmaildir@/@domains_dir@
+ chmod 751 $(DESTDIR)@vpopmaildir@/@domains_dir@
chown -R @vpopuser@ $(DESTDIR)@vpopmaildir@/@domains_dir@
chgrp -R @vpopgroup@ $(DESTDIR)@vpopmaildir@/@domains_dir@
diff -Naurp a/vpopmail-5.4.33/vpopmail.c b/vpopmail-5.4.33/vpopmail.c
--- a/vpopmail-5.4.33/vpopmail.c 2011-02-28 18:00:45.000000000 +0100
+++ b/vpopmail-5.4.33/vpopmail.c 2013-12-25 15:25:18.124000000 +0100
@@ -179,7 +179,7 @@ int vadddomain( char *domain, char *dir,
/* set our file creation mask for machines where the
* sysadmin has tightened default permissions
*/
- umask(VPOPMAIL_UMASK);
+ umask(VPOPMAIL_UMASK & ~VPOPMAIL_PUBLIC_DIR_MODE);
/* store the calling directory */
call_dir = open(".", O_RDONLY);
@@ -194,7 +194,7 @@ int vadddomain( char *domain, char *dir,
if ( chdir(DOMAINS_DIR) != 0 ) {
/* if it's not there, no problem, just try to create it */
- if ( mkdir(DOMAINS_DIR, VPOPMAIL_DIR_MODE) != 0 ) {
+ if ( mkdir(DOMAINS_DIR, VPOPMAIL_PUBLIC_DIR_MODE) != 0 ) {
fchdir(call_dir); close(call_dir);
return(VA_CAN_NOT_MAKE_DOMAINS_DIR);
}
@@ -248,7 +248,8 @@ int vadddomain( char *domain, char *dir,
fchdir(call_dir); close(call_dir);
return(VA_COULD_NOT_MAKE_DOMAIN_DIR);
}
-
+ umask(VPOPMAIL_UMASK);
+
if ( chdir(DomainSubDir) != 0 ) {
/* back out of changes made so far */
vdelfiles(DomainSubDir);
@@ -689,7 +690,6 @@ int vadduser( char *username, char *doma
if ( strlen(password) > MAX_PW_CLEAR_PASSWD ) return(VA_PASSWD_TOO_LONG);
if ( strlen(gecos) > MAX_PW_GECOS ) return(VA_GECOS_TOO_LONG);
- umask(VPOPMAIL_UMASK);
lowerit(username);
lowerit(domain);
@@ -724,7 +724,8 @@ int vadduser( char *username, char *doma
if (verrori != 0 ) return(verrori);
else return(VA_BAD_U_DIR);
}
-
+
+ umask(VPOPMAIL_UMASK);
/* add the user to the auth backend */
/* NOTE: We really need to update this method to include the quota. */
if (vauth_adduser(username, domain, password, gecos, user_hash, apop )!=0) {
@@ -2424,13 +2425,14 @@ char *make_user_dir(char *username, char
}
/* create the users dir, including all the Maildir structure */
- if ( mkdir(username, VPOPMAIL_DIR_MODE) != 0 ) {
+ if ( mkdir(username, VPOPMAIL_PUBLIC_DIR_MODE) != 0 ) {
/* need to add some code to remove the hashed dirs we created above... */
verrori = VA_EXIST_U_DIR;
fchdir(call_dir); close(call_dir);
return(NULL);
}
+ umask(VPOPMAIL_UMASK);
if ( chdir(username) != 0 ) {
/* back out of changes made above */
chdir(domain_dir); chdir(user_hash); vdelfiles(username);
@@ -2485,13 +2487,15 @@ int r_mkdir(char *path, uid_t uid, gid_t
int err;
int i;
struct stat sb;
+ mode_t oldumask;
if (*path == '\0') return 0;
+ oldumask = umask(~VPOPMAIL_PUBLIC_DIR_MODE);
for(i=0; ;++i){
if ( (i > 0) && ((path[i] == '/') || (path[i] == '\0')) ) {
tmpbuf[i] = 0;
- err = mkdir(tmpbuf,VPOPMAIL_DIR_MODE);
+ err = mkdir(tmpbuf, VPOPMAIL_PUBLIC_DIR_MODE);
if (err == 0)
chown(tmpbuf, uid, gid);
else if (errno != EEXIST) {
@@ -2500,12 +2504,14 @@ int r_mkdir(char *path, uid_t uid, gid_t
* directory exists and is a directory at the end of the function.
*/
warn ("Unable to create directory %s: ", tmpbuf);
+ umask(oldumask);
return -1;
}
if (path[i] == '\0') break;
}
tmpbuf[i] = path[i];
}
+ umask(oldumask);
if (stat (path, &sb) != 0) {
warn ("Couldn't stat %s: ", path);
return -1;
@@ -3113,9 +3119,6 @@ int vmake_maildir(char *domain, char *di
/* record which dir the command was launched from */
call_dir = open(".", O_RDONLY);
- /* set the mask for file creation */
- umask(VPOPMAIL_UMASK);
-
/* check if domain exists.
* if domain exists, store the dir into tmpbuf, and store uid and gid
*/
@@ -3145,6 +3148,9 @@ int vmake_maildir(char *domain, char *di
*/
r_mkdir(tmpstr, uid, gid);
+ /* set the mask for file creation */
+ umask(VPOPMAIL_UMASK);
+
/* we should now be able to cd into the user's dir */
if ( chdir(dir) != 0 ) { fchdir(call_dir); close(call_dir); return(-1); }
diff -Naurp a/vpopmail-5.4.33/vpopmail.h b/vpopmail-5.4.33/vpopmail.h
--- a/vpopmail-5.4.33/vpopmail.h 2011-02-28 18:00:45.000000000 +0100
+++ b/vpopmail-5.4.33/vpopmail.h 2013-12-25 11:45:43.720000000 +0100
@@ -54,6 +54,7 @@
#define VPOPMAIL_UMASK 0077
#define VPOPMAIL_TCPRULES_UMASK 0022
#define VPOPMAIL_DIR_MODE 0750
+#define VPOPMAIL_PUBLIC_DIR_MODE 0751
#define VPOPMAIL_QMAIL_MODE S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH
#define USE_POP 0x00
signature.asc
Description: This is a digitally signed message part.
!DSPAM:540f6d4d56441870099925!
