Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Changed the way running external commands are hanlded to address issu…

…es with it not working on some systems/configurations. Just using system and popen and fw commands are run with stdout and stderr tied to gether.

git-svn-id: file:///home/mbr/svn/fwknop/trunk@230 510a4753-2344-4c79-9c09-4d669213fbeb
  • Loading branch information...
commit b6c57aa6a0f8a7e29aeebd9588ca49278c870e62 1 parent 3c3d75a
Damien Stuart authored
View
63 configure.ac
@@ -133,6 +133,37 @@ AS_IF([test "x$with_gpgme" != xno],
fi
], [have_gpgme=no])], [have_gpgme=no])
+dnl Add various common way to sbin dir to the path (just in case)
+ APP_PATH=$PATH$PATH_SEPARATOR/sbin$PATH_SEPARATOR/usr/sbin$PATH_SEPARATOR/usr/local/sbin
+
+dnl Check for gpg (not gpg2)
+dnl
+AC_ARG_WITH([gpg],
+ [AS_HELP_STRING([--with-gpg=/path/to/gpg],
+ [Specify path to the gpg executable that gpgme will use @<:@default=check path@:>@])],
+ [
+ AS_IF([ test "x$withval" = x -o "x$withval" = xyes -o "x$withval" = xno ],
+ [AC_MSG_ERROR([--with-gpg requires an argument specifying a path to gpg])],
+ [
+ AC_CHECK_FILE([$withval], [], [
+ AC_MSG_WARN([Specified path to gpg does not exist on this system])
+ gpg_exe_warn="*not found*"
+ ])
+ GPG_EXE=$withval
+ ]
+ )
+ ],
+ [
+ AC_PATH_PROG(GPG_EXE, [gpg], [], [$APP_PATH])
+ ]
+)
+AS_IF([test "x$GPG_EXE" != x],
+ [
+ AC_DEFINE_UNQUOTED([GPG_EXE], ["$GPG_EXE"], [Path to gpg executable])
+ gpg_exe=$GPG_EXE
+ ], [ gpg_exe="(not found)"]
+)
+
dnl Check for libpcap, gdbm (or ndbm) if we are building the server component
dnl
AS_IF([test "$want_server" = yes], [
@@ -150,38 +181,6 @@ AS_IF([test "$want_server" = yes], [
)]
)
-dnl Add various common way to sbin dir to the path (just in case)
- APP_PATH=$PATH$PATH_SEPARATOR/sbin$PATH_SEPARATOR/usr/sbin$PATH_SEPARATOR/usr/local/sbin
-
-dnl Check for gpg (not gpg2)
-dnl
- AC_ARG_WITH([gpg],
- [AS_HELP_STRING([--with-gpg=/path/to/gpg],
- [Specify path to the gpg executable that gpgme will use @<:@default=check path@:>@])],
- [
- AS_IF([ test "x$withval" = x -o "x$withval" = xyes -o "x$withval" = xno ],
- [AC_MSG_ERROR([--with-gpg requires an argument specifying a path to gpg])],
- [
- AC_CHECK_FILE([$withval], [], [
- AC_MSG_WARN([Specified path to gpg does not exist on this system])
- gpg_exe_warn="*not found*"
- ])
- GPG_EXE=$withval
- ]
- )
- ],
- [
- AC_PATH_PROG(GPG_EXE, [gpg], [], [$APP_PATH])
- ]
- )
- AS_IF([test "x$GPG_EXE" != x],
- [
- AC_DEFINE_UNQUOTED([GPG_EXE], ["$GPG_EXE"], [Path to gpg executable])
- gpg_exe=$GPG_EXE
- ], [ gpg_exe="(not found)"]
- )
-
-
dnl Check for iptables
dnl
AC_ARG_WITH([iptables],
View
121 server/extcmd.c
@@ -5,21 +5,25 @@
*
*/
+/*
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
-#include <errno.h>
-#include <sys/types.h>
-#include <sys/wait.h>
#include <fcntl.h>
+#include <sys/types.h>
#include <sys/select.h>
+*/
+#include <errno.h>
#include <signal.h>
+#include <sys/wait.h>
+#include "fwknopd_common.h"
#include "extcmd.h"
+#include "log_msg.h"
+static sig_atomic_t got_sigalrm;
/* Takes a file descriptor and makes it non-blocking.
-*/
static int
set_nonblock(int fd)
{
@@ -41,37 +45,97 @@ set_nonblock(int fd)
return(0);
}
+*/
+
+static void
+alarm_handler(int sig)
+{
+ got_sigalrm = 1;
+}
-/* Run en external command returning exit status, and filling provided
- * buffers with STDOUT an STDERR up to the size provided.
+/* Run en external command returning exit status, and optionally filling
+ * provided buffer with STDOUT output up to the size provided.
+ *
+ * Note: XXX: We are not using the timeout parameter at present. We still need
+ * to implement a reliable timeout mechanism.
*/
int
-_run_extcmd(uid_t user_uid, char *cmd, char *so_buf, char *se_buf, size_t so_buf_sz, size_t se_buf_sz, int *status)
+_run_extcmd(uid_t user_uid, char *cmd, char *so_buf, size_t so_buf_sz, int timeout)
{
- pid_t pid;
+ FILE *ipt;
+ int retval = 0;
+ char so_read_buf[IO_READ_BUF_LEN];
+ pid_t pid;
+
+ if(so_buf == NULL)
+ {
+ /* Since we do not have to capture output, we will fork here (which we
+ * would have to do anyway if we are running as another user as well).
+ */
+ pid = fork();
+ if(pid == -1)
+ {
+ log_msg(LOG_ERR, "run_extcmd: fork failed: %s", strerror(errno));
+ return(EXTCMD_FORK_ERROR);
+ }
+ else if (pid == 0)
+ {
+ /* We are the child */
+ /* If user is not null, then we setuid to that user before running the
+ * command.
+ */
+ if(user_uid > 0)
+ {
+ if(setuid(user_uid) < 0)
+ {
+ exit(EXTCMD_SETUID_ERROR);
+ }
+ }
+
+ exit(WEXITSTATUS(system(cmd)));
+ }
+
+ /* Retval is forced to 0 as we don't care about the exit status of
+ * the child (for now)>
+ */
+ retval = 0;
+ }
+ else
+ {
+ /* Looking for output use popen and fill the buffer to its limit.
+ */
+ ipt = popen(cmd, "r");
+
+ if(ipt == NULL)
+ {
+ log_msg(LOG_ERR, "Got popen error %i: %s", errno, strerror(errno));
+ retval = -1;
+ }
+ else
+ {
+ memset(so_buf, 0x0, so_buf_sz);
- struct timeval tv;
- fd_set rfds;
- fd_set efds;
- int selval;
+ while((fgets(so_read_buf, IO_READ_BUF_LEN, ipt)) != NULL)
+ {
+ if(so_buf == NULL)
+ continue;
- int so[2], se[2]; /* For our pipes */
+ strlcat(so_buf, so_read_buf, so_buf_sz);
- int bytes_read;
+ if(strlen(so_buf) >= so_buf_sz-1)
+ break;
+ }
- char so_read_buf[IO_READ_BUF_LEN];
- char se_read_buf[IO_READ_BUF_LEN];
+ pclose(ipt);
+ }
+ }
- /* Set our remaining_buf counters to one less than the given size so we
- * can leave room for a terminating '\0'.
- */
- int so_buf_remaining = (so_buf_sz > 0) ? so_buf_sz-1 : 0;
- int se_buf_remaining = (se_buf_sz > 0) ? se_buf_sz-1 : 0;
+ return(retval);
+}
- /* Be optimistic :)
- */
- int retval = EXTCMD_SUCCESS_ALL_OUTPUT;
+/*** END TEST Section ***/
+#if 0
/* Create the pipes we will use for getting stdout and stderr
* from the child process.
*/
@@ -278,18 +342,19 @@ _run_extcmd(uid_t user_uid, char *cmd, char *so_buf, char *se_buf, size_t so_buf
*/
return(retval);
}
+#endif
/* Run an external command. This is wrapper around _run_extcmd()
*/
-run_extcmd(char *cmd, char *so_buf, char *se_buf, size_t so_buf_sz, size_t se_buf_sz, int *status)
+run_extcmd(char *cmd, char *so_buf, size_t so_buf_sz, int timeout)
{
- return _run_extcmd(0, cmd, so_buf, se_buf, so_buf_sz, se_buf_sz, status);
+ return _run_extcmd(0, cmd, so_buf, so_buf_sz, timeout);
}
/* Run an external command as the specified user. This is wrapper around _run_extcmd()
*/
-run_extcmd_as(uid_t user_uid, char *cmd, char *so_buf, char *se_buf, size_t so_buf_sz, size_t se_buf_sz, int *status)
+run_extcmd_as(uid_t user_uid, char *cmd, char *so_buf, size_t so_buf_sz, int timeout)
{
- return _run_extcmd(user_uid, cmd, so_buf, se_buf, so_buf_sz, se_buf_sz, status);
+ return _run_extcmd(user_uid, cmd, so_buf, so_buf_sz, timeout);
}
View
4 server/extcmd.h
@@ -66,8 +66,8 @@ enum {
/* Function prototypes
*/
-int run_extcmd(char *cmd, char *so_buf, char *se_buf, size_t so_buf_sz, size_t se_buf_sz, int *status);
-int run_extcmd_as(uid_t uid, char *cmd, char *so_buf, char *se_buf, size_t so_buf_sz, size_t se_buf_sz, int *status);
+int run_extcmd(char *cmd, char *so_buf, size_t so_buf_sz, int timeout);
+int run_extcmd_as(uid_t uid, char *cmd, char *so_buf, size_t so_buf_sz, int timeout);
#endif /* EXTCMD_H */
View
49 server/fw_util.c
@@ -155,10 +155,10 @@ delete_all_chains(void)
);
//printf("CMD: '%s'\n", cmd_buf);
- res = run_extcmd(cmd_buf, NULL, err, 0, CMD_BUFSIZE, &status);
+ res = run_extcmd(cmd_buf, err, CMD_BUFSIZE, 0);
/* Expect full success on this */
if(! EXTCMD_IS_SUCCESS(res))
- parse_extcmd_error(res, status, err);
+ log_msg(LOG_ERR, "Error %i from cmd:'%s': %s", res, cmd_buf, err);
}
memset(cmd_buf, 0x0, CMD_BUFSIZE);
@@ -176,10 +176,10 @@ delete_all_chains(void)
);
//printf("CMD: '%s'\n", cmd_buf);
- res = run_extcmd(cmd_buf, NULL, err, 0, CMD_BUFSIZE, &status);
+ res = run_extcmd(cmd_buf, err, CMD_BUFSIZE, 0);
/* Expect full success on this */
if(! EXTCMD_IS_SUCCESS(res))
- parse_extcmd_error(res, status, err);
+ log_msg(LOG_ERR, "Error %i from cmd:'%s': %s", res, cmd_buf, err);
}
}
@@ -207,11 +207,12 @@ create_fw_chains(void)
);
//printf("(%i) CMD: '%s'\n", i, cmd_buf);
- res = run_extcmd(cmd_buf, NULL, err, 0, CMD_BUFSIZE, &status);
+ res = run_extcmd(cmd_buf, err, CMD_BUFSIZE, 0);
+
/* Expect full success on this */
if(! EXTCMD_IS_SUCCESS(res))
{
- parse_extcmd_error(res, status, err);
+ log_msg(LOG_ERR, "Error %i from cmd:'%s': %s", res, cmd_buf, err);
got_err++;
}
@@ -228,11 +229,12 @@ create_fw_chains(void)
);
//printf("(%i) CMD: '%s'\n", i, cmd_buf);
- res = run_extcmd(cmd_buf, NULL, err, 0, CMD_BUFSIZE, &status);
+ res = run_extcmd(cmd_buf, err, CMD_BUFSIZE, 0);
+
/* Expect full success on this */
if(! EXTCMD_IS_SUCCESS(res))
{
- parse_extcmd_error(res, status, err);
+ log_msg(LOG_ERR, "Error %i from cmd:'%s': %s", res, cmd_buf, err);
got_err++;
}
}
@@ -485,7 +487,7 @@ process_spa_request(fko_srv_options_t *opts, spa_data_t *spadat)
//--DSS tmp
//fprintf(stderr, "ADD CMD: %s\n", cmd_buf);
- res = run_extcmd(cmd_buf, NULL, err, 0, CMD_BUFSIZE, &status);
+ res = run_extcmd(cmd_buf, err, CMD_BUFSIZE, 0);
if(EXTCMD_IS_SUCCESS(res))
{
log_msg(LOG_INFO, "Added Rule to %s for %s, %s expires at %u",
@@ -502,7 +504,7 @@ process_spa_request(fko_srv_options_t *opts, spa_data_t *spadat)
in_chain->next_expire = exp_ts;
}
else
- parse_extcmd_error(res, status, err);
+ log_msg(LOG_ERR, "Error %i from cmd:'%s': %s", res, cmd_buf, err);
/* If we have to make an corresponding OUTPUT rule if out_chain target
* is not NULL.
@@ -524,7 +526,7 @@ process_spa_request(fko_srv_options_t *opts, spa_data_t *spadat)
//--DSS tmp
//fprintf(stderr, "ADD OUTPUT CMD: %s\n", cmd_buf);
- res = run_extcmd(cmd_buf, NULL, err, 0, CMD_BUFSIZE, &status);
+ res = run_extcmd(cmd_buf, err, CMD_BUFSIZE, 0);
if(EXTCMD_IS_SUCCESS(res))
{
log_msg(LOG_INFO, "Added OUTPUT Rule to %s for %s, %s expires at %u",
@@ -541,7 +543,7 @@ process_spa_request(fko_srv_options_t *opts, spa_data_t *spadat)
out_chain->next_expire = exp_ts;
}
else
- parse_extcmd_error(res, status, err);
+ log_msg(LOG_ERR, "Error %i from cmd:'%s': %s", res, cmd_buf, err);
}
@@ -591,7 +593,7 @@ process_spa_request(fko_srv_options_t *opts, spa_data_t *spadat)
//--DSS tmp
//fprintf(stderr, "ADD OUTPUT CMD: %s\n", cmd_buf);
- res = run_extcmd(cmd_buf, NULL, err, 0, CMD_BUFSIZE, &status);
+ res = run_extcmd(cmd_buf, err, CMD_BUFSIZE, 0);
if(EXTCMD_IS_SUCCESS(res))
{
log_msg(LOG_INFO, "Added FORWARD Rule to %s for %s, %s expires at %u",
@@ -608,7 +610,7 @@ process_spa_request(fko_srv_options_t *opts, spa_data_t *spadat)
fwd_chain->next_expire = exp_ts;
}
else
- parse_extcmd_error(res, status, err);
+ log_msg(LOG_ERR, "Error %i from cmd:'%s': %s", res, cmd_buf, err);
}
if(dnat_chain->to_chain != NULL && strlen(dnat_chain->to_chain))
@@ -630,7 +632,7 @@ process_spa_request(fko_srv_options_t *opts, spa_data_t *spadat)
//--DSS tmp
//fprintf(stderr, "ADD DNAT CMD: %s\n", cmd_buf);
- res = run_extcmd(cmd_buf, NULL, err, 0, CMD_BUFSIZE, &status);
+ res = run_extcmd(cmd_buf, err, CMD_BUFSIZE, 0);
if(EXTCMD_IS_SUCCESS(res))
{
log_msg(LOG_INFO, "Added DNAT Rule to %s for %s, %s expires at %u",
@@ -647,7 +649,7 @@ process_spa_request(fko_srv_options_t *opts, spa_data_t *spadat)
dnat_chain->next_expire = exp_ts;
}
else
- parse_extcmd_error(res, status, err);
+ log_msg(LOG_ERR, "Error %i from cmd:'%s': %s", res, cmd_buf, err);
}
/* If SNAT (or MASQUERADE) is wanted, then we add those rules here as well.
@@ -691,7 +693,7 @@ process_spa_request(fko_srv_options_t *opts, spa_data_t *spadat)
snat_target
);
- res = run_extcmd(cmd_buf, NULL, err, 0, CMD_BUFSIZE, &status);
+ res = run_extcmd(cmd_buf, err, CMD_BUFSIZE, 0);
if(EXTCMD_IS_SUCCESS(res))
{
log_msg(LOG_INFO, "Added Source NAT Rule to %s for %s, %s expires at %u",
@@ -708,7 +710,7 @@ process_spa_request(fko_srv_options_t *opts, spa_data_t *spadat)
snat_chain->next_expire = exp_ts;
}
else
- parse_extcmd_error(res, status, err);
+ log_msg(LOG_ERR, "Error %i from cmd:'%s': %s", res, cmd_buf, err);
}
}
@@ -765,16 +767,15 @@ check_firewall_rules(fko_srv_options_t *opts)
memset(cmd_out, 0x0, STANDARD_CMD_OUT_BUFSIZE);
- res = run_extcmd(cmd_buf, cmd_out, err,
- STANDARD_CMD_OUT_BUFSIZE, CMD_BUFSIZE, &status);
+ res = run_extcmd(cmd_buf, cmd_out, STANDARD_CMD_OUT_BUFSIZE, 0);
if(!EXTCMD_IS_SUCCESS(res))
{
- parse_extcmd_error(res, status, err);
+ log_msg(LOG_ERR, "Error %i from cmd:'%s': %s", res, cmd_buf, cmd_out);
continue;
}
- if(opts->verbose > 1)
+ if(opts->verbose > 2)
log_msg(LOG_INFO, "RES=%i, CMD_BUF: %s\nRULES LIST: %s", res, cmd_buf, cmd_out);
ndx = strstr(cmd_out, "_exp_");
@@ -855,7 +856,7 @@ check_firewall_rules(fko_srv_options_t *opts)
//fprintf(stderr, "DELETE RULE CMD: %s\n", cmd_buf);
- res = run_extcmd(cmd_buf, NULL, err, 0, CMD_BUFSIZE, &status);
+ res = run_extcmd(cmd_buf, err, CMD_BUFSIZE, 0);
if(EXTCMD_IS_SUCCESS(res))
{
log_msg(LOG_INFO, "Removed rule %s from %s with expire time of %u.",
@@ -866,7 +867,7 @@ check_firewall_rules(fko_srv_options_t *opts)
ch[i].active_rules--;
}
else
- parse_extcmd_error(res, status, err);
+ log_msg(LOG_ERR, "Error %i from cmd:'%s': %s", res, cmd_buf, err);
}
else
View
22 server/fw_util.h
@@ -35,17 +35,17 @@
/* iptables command args
*/
-#define IPT_ADD_RULE_ARGS "-t %s -A %s -p %i -s %s --dport %i -m comment --comment _exp_%u -j %s"
-#define IPT_ADD_OUT_RULE_ARGS "-t %s -A %s -p %i -d %s --sport %i -m comment --comment _exp_%u -j %s"
-#define IPT_ADD_FWD_RULE_ARGS "-t %s -A %s -p %i -s %s -d %s --dport %i -m comment --comment _exp_%u -j %s"
-#define IPT_ADD_DNAT_RULE_ARGS "-t %s -A %s -p %i -s %s --dport %i -m comment --comment _exp_%u -j %s --to-destination %s:%i"
-#define IPT_ADD_SNAT_RULE_ARGS "-t %s -A %s -p %i -d %s --dport %i -m comment --comment _exp_%u -j %s %s"
-#define IPT_DEL_RULE_ARGS "-t %s -D %s %i"
-#define IPT_NEW_CHAIN_ARGS "-t %s -N %s"
-#define IPT_FLUSH_CHAIN_ARGS "-t %s -F %s"
-#define IPT_DEL_CHAIN_ARGS "-t %s -X %s"
-#define IPT_ADD_JUMP_RULE_ARGS "-t %s -I %s %i -j %s"
-#define IPT_LIST_RULES_ARGS "-t %s -L %s --line-numbers -n"
+#define IPT_ADD_RULE_ARGS "-t %s -A %s -p %i -s %s --dport %i -m comment --comment _exp_%u -j %s 2>&1"
+#define IPT_ADD_OUT_RULE_ARGS "-t %s -A %s -p %i -d %s --sport %i -m comment --comment _exp_%u -j %s 2>&1"
+#define IPT_ADD_FWD_RULE_ARGS "-t %s -A %s -p %i -s %s -d %s --dport %i -m comment --comment _exp_%u -j %s 2>&1"
+#define IPT_ADD_DNAT_RULE_ARGS "-t %s -A %s -p %i -s %s --dport %i -m comment --comment _exp_%u -j %s --to-destination %s:%i 2>&1"
+#define IPT_ADD_SNAT_RULE_ARGS "-t %s -A %s -p %i -d %s --dport %i -m comment --comment _exp_%u -j %s %s 2>&1"
+#define IPT_DEL_RULE_ARGS "-t %s -D %s %i 2>&1"
+#define IPT_NEW_CHAIN_ARGS "-t %s -N %s 2>&1"
+#define IPT_FLUSH_CHAIN_ARGS "-t %s -F %s 2>&1"
+#define IPT_DEL_CHAIN_ARGS "-t %s -X %s 2>&1"
+#define IPT_ADD_JUMP_RULE_ARGS "-t %s -I %s %i -j %s 2>&1"
+#define IPT_LIST_RULES_ARGS "-t %s -L %s --line-numbers -n 2>&1"
#define DEF_FW_ACCESS_TIMEOUT 60
View
12 server/incoming_spa.c
@@ -461,12 +461,10 @@ incoming_spa(fko_srv_options_t *opts)
res = run_extcmd_as(acc->cmd_exec_uid,
- spadat.spa_message_remain,
- NULL, NULL, 0, 0, &status);
+ spadat.spa_message_remain, NULL, 0, 0);
}
else /* Just run it as we are (root that is). */
- res = run_extcmd(spadat.spa_message_remain,
- NULL, NULL, 0, 0, &status);
+ res = run_extcmd(spadat.spa_message_remain, NULL, 0, 5);
/* --DSS XXX: I have found that the status (and res for that
* matter) have been unreliable indicators of the
@@ -475,11 +473,11 @@ incoming_spa(fko_srv_options_t *opts)
*/
status = WEXITSTATUS(status);
- if(opts->verbose > 1)
+ if(opts->verbose > 2)
log_msg(LOG_WARNING,
- "CMD_EXEC: result: %i, command returned %i", res, status);
+ "CMD_EXEC: command returned %i", res);
- if(res != 0 || status != 0)
+ if(res != 0)
res = SPA_MSG_COMMAND_ERROR;
}
View
6 server/pcap_capture.c
@@ -161,8 +161,10 @@ pcap_capture(fko_srv_options_t *opts)
if(WIFSIGNALED(status))
log_msg(LOG_WARNING, "TCP server got signal: %i", WTERMSIG(status));
- log_msg(LOG_WARNING, "TCP server exited with status of %i. Attempting restart.",
- WEXITSTATUS(status));
+ log_msg(LOG_WARNING,
+ "TCP server exited with status of %i. Attempting restart.",
+ WEXITSTATUS(status)
+ );
opts->tcp_server_pid = 0;
View
1  server/sig_handler.c
@@ -61,6 +61,7 @@ sig_handler(int sig)
return;
case SIGCHLD:
got_sigchld = 1;
+ waitpid(-1, NULL, WNOHANG);
return;
}
}
View
2  server/tcp_server.c
@@ -188,7 +188,7 @@ run_tcp_server(fko_srv_options_t *opts)
{
memset(sipbuf, 0x0, MAX_IP_STR_LEN);
inet_ntop(AF_INET, &(caddr.sin_addr.s_addr), sipbuf, MAX_IP_STR_LEN);
- log_msg(LOG_INFO|LOG_STDERR, "tcp_server: Got TCP connection from %s.", sipbuf);
+ log_msg(LOG_INFO, "tcp_server: Got TCP connection from %s.", sipbuf);
}
/* Though hacky and clunky, we just sleep for a second then
Please sign in to comment.
Something went wrong with that request. Please try again.