- Added CLIENT and SERVER variables to VirusAction script environment.

  - Even better logging for network errors.

5 files changed, 75 insertions, 37 deletions

diff --git a/ChangeLog b/ChangeLog
index cb8d938..98130e7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+1.1
+  - Added CLIENT and SERVER variables to VirusAction script environment.
+  - Even better logging for network errors.
+
 1.0
   - Removed user option from startup script (use User config option)
   - Don't send lines that are too long to syslog [Ben Mesman]
diff --git a/common/smtppass.c b/common/smtppass.c
index 95b9205..88cfafd 100644
--- a/common/smtppass.c
+++ b/common/smtppass.c
@@ -178,7 +178,7 @@ static void pid_file(int write);
 static void connection_loop(int sock);
 static void* thread_main(void* arg);
 static int smtp_passthru(spctx_t* ctx);
-static int connect_out(spctx_t* ctx);
+static int make_connections(spctx_t* ctx, int client);
 static int read_server_response(spctx_t* ctx);
 static int parse_config_file(const char* configfile);
 static char* parse_address(char* line);
@@ -584,16 +584,16 @@ static spctx_t* init_thread(int fd)
                 g_unique_id++;
         sp_unlock();
 
-        ctx->client.fd = fd;
-        ASSERT(ctx->client.fd != -1);
-        sp_messagex(ctx, LOG_DEBUG, "processing %d on thread %x", ctx->client.fd, (int)pthread_self());
+        sp_messagex(ctx, LOG_DEBUG, "processing %d on thread %x", fd, (int)pthread_self());
 
         /* Connect to the outgoing server ... */
-        if(connect_out(ctx) == -1)
+        if(make_connections(ctx, fd) == -1)
         {
             cb_del_context(ctx);
             ctx = NULL;
         }
+
+        ASSERT(ctx->client.fd != -1);
     }
 
     return ctx;
@@ -694,7 +694,7 @@ cleanup:
     return (void*)(ret == 0 ? 0 : 1);
 }
 
-static int connect_out(spctx_t* ctx)
+static int make_connections(spctx_t* ctx, int client)
 {
     struct sockaddr_any peeraddr;
     struct sockaddr_any addr;
@@ -702,15 +702,11 @@ static int connect_out(spctx_t* ctx)
     char buf[MAXPATHLEN];
     const char* outname;
 
-    memset(&peeraddr, 0, sizeof(peeraddr));
-    SANY_LEN(peeraddr) = sizeof(peeraddr);
+    ASSERT(client != -1);
 
-    /* Get the peer name */
-    if(getpeername(ctx->client.fd, &SANY_ADDR(peeraddr), &SANY_LEN(peeraddr)) == -1 ||
-       sock_any_ntop(&peeraddr, buf, MAXPATHLEN, SANY_OPT_NOPORT) == -1)
-        sp_message(ctx, LOG_WARNING, "couldn't get peer address");
-    else
-        sp_messagex(ctx, LOG_INFO, "accepted connection from: %s", buf);
+    /* Setup the incoming connection. This also fills in peeraddr for us */
+    spio_attach(ctx, &(ctx->client), client, &peeraddr);
+    sp_messagex(ctx, LOG_INFO, "accepted connection from: %s", ctx->client.peername);
 
     /* Create the server connection address */
     outaddr = &(g_state.outaddr);
@@ -1427,6 +1423,12 @@ void sp_setup_forked(spctx_t* ctx, int file)
     if(file && ctx->cachename[0])
         setenv("EMAIL", ctx->cachename, 1);
 
+    if(spio_valid(&(ctx->client)))
+        setenv("CLIENT", ctx->client.peername, 1);
+
+    if(spio_valid(&(ctx->server)))
+        setenv("SERVER", ctx->server.peername, 1);
+
     setenv("TMPDIR", g_state.directory, 1);
 }
 
diff --git a/common/smtppass.h b/common/smtppass.h
index 34758ba..418686e 100644
--- a/common/smtppass.h
+++ b/common/smtppass.h
@@ -67,6 +67,7 @@ typedef struct spio
     int fd;                             /* The file descriptor wrapped */
     const char* name;                   /* The name for logging */
     time_t last_action;                 /* Time of last action on descriptor */
+    char peername[MAXPATHLEN];          /* Name of the peer on other side of socket */
 
     /* Internal use only */
     char line[SP_LINE_LENGTH];
@@ -77,9 +78,12 @@ spio_t;
 
 #define spio_valid(io)      ((io)->fd != -1)
 
-/* Setup the io structure (allocated elsewhere */
+/* Setup the io structure (allocated elsewhere) */
 void spio_init(spio_t* io, const char* name);
 
+/* Attach an open descriptor to a socket, optionally returning the peer */
+void spio_attach(struct spctx* ctx, spio_t* io, int fd, struct sockaddr_any* peer);
+
 /* Connect and disconnect from sockets */
 int  spio_connect(struct spctx* ctx, spio_t* io, const struct sockaddr_any* sany, const char* addrname);
 void spio_disconnect(struct spctx* ctx, spio_t* io);
diff --git a/common/spio.c b/common/spio.c
index f3247da..e8425bf 100644
--- a/common/spio.c
+++ b/common/spio.c
@@ -110,43 +110,67 @@ void spio_init(spio_t* io, const char* name)
     io->fd = -1;
 }
 
+void spio_attach(spctx_t* ctx, spio_t* io, int fd, struct sockaddr_any* peer)
+{
+    struct sockaddr_any peeraddr;
+
+    io->fd = fd;
+
+    /* If the caller doesn't want the peer then use our own */
+    if (peer == NULL)
+        peer = &peeraddr;
+
+    memset(peer, 0, sizeof(*peer));
+    SANY_LEN(*peer) = sizeof(*peer);
+
+    if(getpeername(fd, &SANY_ADDR(*peer), &SANY_LEN(*peer)) == -1 ||
+       sock_any_ntop(peer, io->peername, MAXPATHLEN, SANY_OPT_NOPORT) == -1)
+    {
+        sp_message(ctx, LOG_WARNING, "%s: couldn't get peer address", GET_IO_NAME(io));
+        strlcpy(io->peername, "UNKNOWN", MAXPATHLEN);
+    }
+
+    /* As a double check */
+    io->line[0] = 0;
+    io->_nx = NULL;
+    io->_ln = 0;
+}
+
 int spio_connect(spctx_t* ctx, spio_t* io, const struct sockaddr_any* sany,
                  const char* addrname)
 {
     int ret = 0;
+    int fd;
 
     ASSERT(ctx && io && sany && addrname);
     ASSERT(io->fd == -1);
 
-    if((io->fd = socket(SANY_TYPE(*sany), SOCK_STREAM, 0)) == -1)
+    if((fd = socket(SANY_TYPE(*sany), SOCK_STREAM, 0)) == -1)
         RETURN(-1);
 
-    if(setsockopt(io->fd, SOL_SOCKET, SO_RCVTIMEO, &(g_state.timeout), sizeof(g_state.timeout)) == -1 ||
-       setsockopt(io->fd, SOL_SOCKET, SO_SNDTIMEO, &(g_state.timeout), sizeof(g_state.timeout)) == -1)
-        sp_messagex(ctx, LOG_WARNING, "couldn't set timeouts on connection");
+    if(setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &(g_state.timeout), sizeof(g_state.timeout)) == -1 ||
+       setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &(g_state.timeout), sizeof(g_state.timeout)) == -1)
+        sp_messagex(ctx, LOG_WARNING, "%s: couldn't set timeouts on connection", GET_IO_NAME(io));
 
-    fcntl(io->fd, F_SETFD, fcntl(io->fd, F_GETFD, 0) | FD_CLOEXEC);
+    fcntl(fd, F_SETFD, fcntl(fd, F_GETFD, 0) | FD_CLOEXEC);
 
-    if(connect(io->fd, &SANY_ADDR(*sany), SANY_LEN(*sany)) == -1)
+    if(connect(fd, &SANY_ADDR(*sany), SANY_LEN(*sany)) == -1)
         RETURN(-1);
 
-    /* As a double check */
-    io->line[0] = 0;
-    io->_nx = NULL;
-    io->_ln = 0;
+    spio_attach(ctx, io, fd, NULL);
 
 cleanup:
     if(ret < 0)
     {
-        if(io->fd != -1)
-            close(io->fd);
+        if(spio_valid(io))
+            close_raw(&(io->fd));
 
-        sp_message(ctx, LOG_ERR, "couldn't connect to: %s", addrname);
+        sp_message(ctx, LOG_ERR, "%s: couldn't connect to: %s", GET_IO_NAME(io), addrname);
         return -1;
     }
 
     ASSERT(io->fd != -1);
-    sp_messagex(ctx, LOG_DEBUG, "%s connected to: %s", GET_IO_NAME(io), addrname);
+    sp_messagex(ctx, LOG_DEBUG, "%s connected to: %s", GET_IO_NAME(io), io->peername);
     return 0;
 }
 
@@ -340,11 +364,11 @@ int read_raw(spctx_t* ctx, spio_t* io, int opts)
             }
 
             if(errno == ECONNRESET) /* Not usually a big deal so supresse the error */
-                sp_messagex(ctx, LOG_DEBUG, "connection disconnected by peer: %s", GET_IO_NAME(io));
+                sp_messagex(ctx, LOG_DEBUG, "%s: connection disconnected by peer", GET_IO_NAME(io));
             else if(errno == EAGAIN)
-                sp_messagex(ctx, LOG_WARNING, "network read operation timed out: %s", GET_IO_NAME(io));
+                sp_messagex(ctx, LOG_WARNING, "%s: network read operation timed out", GET_IO_NAME(io));
             else
-                sp_message(ctx, LOG_ERR, "couldn't read data from socket: %s", GET_IO_NAME(io));
+                sp_message(ctx, LOG_ERR, "%s: couldn't read data from socket", GET_IO_NAME(io));
 
             /*
              * The basic logic here is that if we've had a fatal error
@@ -434,7 +458,7 @@ int spio_read_line(spctx_t* ctx, spio_t* io, int opts)
 
     if(!spio_valid(io))
     {
-        sp_messagex(ctx, LOG_WARNING, "tried to read from a closed connection");
+        sp_messagex(ctx, LOG_WARNING, "%s: tried to read from a closed connection", GET_IO_NAME(io));
         return 0;
     }
 
@@ -478,7 +502,7 @@ int spio_write_data(spctx_t* ctx, spio_t* io, const char* data)
 
     if(!spio_valid(io))
     {
-        sp_message(ctx, LOG_ERR, "connection closed. can't write data");
+        sp_message(ctx, LOG_ERR, "%s: connection closed. can't write data", GET_IO_NAME(io));
         return -1;
     }
 
@@ -527,9 +551,9 @@ int spio_write_data_raw(spctx_t* ctx, spio_t* io, unsigned char* buf, int len)
             close_raw(&(io->fd));
 
             if(errno == EAGAIN)
-                sp_messagex(ctx, LOG_WARNING, "network write operation timed out: %s", GET_IO_NAME(io));
+                sp_messagex(ctx, LOG_WARNING, "%s: network write operation timed out", GET_IO_NAME(io));
             else
-                sp_message(ctx, LOG_ERR, "couldn't write data to socket: %s", GET_IO_NAME(io));
+                sp_message(ctx, LOG_ERR, "%s: couldn't write data to socket", GET_IO_NAME(io));
 
             return -1;
         }
@@ -571,7 +595,7 @@ void spio_read_junk(spctx_t* ctx, spio_t* io)
 
         if(!said && *t)
         {
-            sp_messagex(ctx, LOG_DEBUG, "received junk data from daemon");
+            sp_messagex(ctx, LOG_DEBUG, "%s: received junk data from daemon", GET_IO_NAME(io));
             said = 1;
         }
     }
diff --git a/doc/clamsmtpd.8 b/doc/clamsmtpd.8
index 9501064..7fa18fb 100644
--- a/doc/clamsmtpd.8
+++ b/doc/clamsmtpd.8
@@ -159,6 +159,8 @@ may need to escape them properly before use in your favorite scripting
 language. Failure to do this could lead to a REMOTE COMPROMISE of your 
 machine.
 .Bl -tag -width Fl
+.It Ar CLIENT
+The network address of the SMTP client connected.
 .It Ar EMAIL
 When the
 .Ar Quarantine
@@ -168,6 +170,8 @@ The email addresses of the email recipients. These are specified one per
 line, in standard address format. 
 .It Ar SENDER
 The email address for the sender of the email. 
+.It Ar SERVER
+The network address of the SMTP server we're connected to.
 .It Ar TMPDIR
 The path to the temp directory in use. This is the same as the 
 .Ar TempDirectory