capture child: send an errno message if exec of dumpcap fails.

On at least some Linux distributions, dumpcap is either installed with
elevated privileges sufficient to support traffic capture by default or
can optionally be given those privileges.  If it has those privileges,
it's typically made group-executable but not world-executable and owned
by a special group, e.g. "wireshark", so that only users in that group
can use dumpcap to capture traffic.

The user installing the Wireshark package is *not* necessarily put into
that group by default; this means that any attempt by Wireshark or
TShark to run dumpcap will fail with EACCES.

If the exec call in the child process sends text error mesages, intended
for end users, up the message pipe, as we had been doing, then figuring
out *why* the exec failed would require some heuristic parsing to figure
out whether it's a permissions problem or not.

Instead of doing that, just send a message giving the errno for exec
failing.

For now, we just format an error message for that in the parent process,
but this leaves room to do a better job.

While we're at it, fix some cases where an empty error message could be
printed.
This commit is contained in:
Guy Harris 2023-09-09 22:14:08 -07:00
parent 656f00ef29
commit fe835ae95c
5 changed files with 99 additions and 15 deletions

View File

@ -305,7 +305,6 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd,
unsigned j;
interface_options *interface_opts;
#else
char errmsg[1024+1];
int sync_pipe[2]; /* pipe used to send messages from child to parent */
int data_pipe[2]; /* pipe used to send data from child to parent */
#endif
@ -535,9 +534,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd,
ws_close(sync_pipe[PIPE_READ]);
ws_close(sync_pipe[PIPE_WRITE]);
execv(argv[0], argv);
snprintf(errmsg, sizeof errmsg, "Couldn't run %s in child process: %s",
argv[0], g_strerror(errno));
sync_pipe_write_errmsgs_to_parent(2, errmsg, "");
sync_pipe_write_int_msg(2, SP_EXEC_FAILED, errno);
/* Exit with "_exit()", so that we don't close the connection
to the X server (and cause stuff buffered up by our parent but
@ -970,6 +967,7 @@ sync_pipe_run_command_actual(char* const argv[], char **data, char **primary_msg
char buffer[PIPE_BUF_SIZE+1] = {0};
ssize_t nread;
char indicator;
int32_t exec_errno = 0;
int primary_msg_len;
char *primary_msg_text;
int secondary_msg_len;
@ -1034,6 +1032,39 @@ sync_pipe_run_command_actual(char* const argv[], char **data, char **primary_msg
/* we got a valid message block from the child, process it */
switch(indicator) {
case SP_EXEC_FAILED:
/*
* Exec of dumpcap failed. Get the errno for the failure.
*/
if (!ws_strtoi32(buffer, NULL, &exec_errno)) {
ws_warning("Invalid errno: %s", buffer);
}
/*
* Pick up the child status.
*/
ret = sync_pipe_close_command(&data_pipe_read_fd, sync_pipe_read_io,
&fork_child, &msg);
if (ret == -1) {
/*
* Child process failed unexpectedly, or wait failed; msg is the
* error message.
*/
*primary_msg = msg;
*secondary_msg = NULL;
} else {
/*
* Child process failed, but returned the expected exit status.
* Return the messages it gave us, and indicate failure.
*/
*primary_msg = ws_strdup_printf("Couldn't run dumpcap in child process: %s",
g_strerror(exec_errno));
*secondary_msg = NULL;
ret = -1;
}
*data = NULL;
break;
case SP_ERROR_MSG:
/*
* Error from dumpcap; there will be a primary message and a
@ -1336,6 +1367,7 @@ sync_interface_stats_open(int *data_read_fd, ws_process_id *fork_child, char **m
char buffer[PIPE_BUF_SIZE+1] = {0};
ssize_t nread;
char indicator;
int32_t exec_errno = 0;
int primary_msg_len;
char *primary_msg_text;
int secondary_msg_len;
@ -1416,6 +1448,24 @@ sync_interface_stats_open(int *data_read_fd, ws_process_id *fork_child, char **m
/* we got a valid message block from the child, process it */
switch(indicator) {
case SP_EXEC_FAILED:
/*
* Exec of dumpcap failed. Get the errno for the failure.
*/
if (!ws_strtoi32(buffer, NULL, &exec_errno)) {
ws_warning("Invalid errno: %s", buffer);
}
*msg = ws_strdup_printf("Couldn't run dumpcap in child process: %s",
g_strerror(exec_errno));
/*
* Pick up the child status.
*/
ret = sync_pipe_close_command(data_read_fd, message_read_io,
fork_child, msg);
ret = -1;
break;
case SP_ERROR_MSG:
/*
* Error from dumpcap; there will be a primary message and a
@ -1663,6 +1713,7 @@ sync_pipe_input_cb(GIOChannel *pipe_io, capture_session *cap_session)
char buffer[SP_MAX_MSG_LEN+1] = {0};
ssize_t nread;
char indicator;
int32_t exec_errno = 0;
int primary_len;
char *primary_msg;
int secondary_len;
@ -1755,6 +1806,19 @@ sync_pipe_input_cb(GIOChannel *pipe_io, capture_session *cap_session)
cap_session->count += npackets;
cap_session->new_packets(cap_session, npackets);
break;
case SP_EXEC_FAILED:
/*
* Exec of dumpcap failed. Get the errno for the failure.
*/
if (!ws_strtoi32(buffer, NULL, &exec_errno)) {
ws_warning("Invalid errno: %s", buffer);
}
primary_msg = ws_strdup_printf("Couldn't run dumpcap in child process: %s",
g_strerror(exec_errno));
cap_session->error(cap_session, primary_msg, NULL);
/* the capture child will close the sync_pipe, nothing to do for now */
/* (an error message doesn't mean we have to stop capturing) */
break;
case SP_ERROR_MSG:
/* convert primary message */
pipe_convert_header((unsigned char*)buffer, 4, &indicator, &primary_len);
@ -1793,7 +1857,11 @@ sync_pipe_input_cb(GIOChannel *pipe_io, capture_session *cap_session)
break;
}
default:
ws_assert_not_reached();
if (g_ascii_isprint(indicator))
ws_warning("Unknown indicator '%c'", indicator);
else
ws_warning("Unknown indicator '\\x%02x", indicator);
break;
}
return true;

View File

@ -36,6 +36,7 @@
* (http://code.google.com/p/protobuf-c/) if we ever need to use more
* complex messages.
*/
#define SP_EXEC_FAILED 'X' /* errno value for the exec failing */
#define SP_FILE 'F' /* the name of the recently opened file */
#define SP_ERROR_MSG 'E' /* error message */
#define SP_BAD_FILTER 'B' /* error message for bad capture filter */
@ -63,14 +64,13 @@ sync_pipe_write_string_msg(int pipe_fd, char indicator, const char *msg);
extern void
sync_pipe_write_uint_msg(int pipe_fd, char indicator, unsigned int num);
/* Write a message, with an integer body, to the recipient pipe in the
standard format (1-byte message indicator, 3-byte message length
(excluding length and indicator field), and the integer, as a string. */
extern void
sync_pipe_write_int_msg(int pipe_fd, char indicator, int num);
/** the child encountered an error, notify the parent */
/* Write a message, with two message strings as the body, to the
recipient pipe. The header is an SP_ERROR_MSG header, with the
length being the length of two string submessages; the submessages
are the body of the message, with each submessage being a message
with an indicator of SP_ERROR_MSG, the first message having
first message string and the second message having the second message
string. */
extern void
sync_pipe_write_errmsgs_to_parent(int pipe_fd, const char *error_msg,
const char *secondary_error_msg);

View File

@ -97,6 +97,19 @@ sync_pipe_write_uint_msg(int pipe_fd, char indicator, unsigned int num)
sync_pipe_write_string_msg(pipe_fd, indicator, count_str);
}
/* Write a message, with an integer body, to the recipient pipe in the
standard format (1-byte message indicator, 3-byte message length
(excluding length and indicator field), and the unsigned integer,
as a string. */
void
sync_pipe_write_int_msg(int pipe_fd, char indicator, int num)
{
char count_str[SP_DECISIZE+1+1];
snprintf(count_str, sizeof(count_str), "%d", num);
sync_pipe_write_string_msg(pipe_fd, indicator, count_str);
}
/* Write a message, with a primary and secondary error message as the body,
to the recipient pipe. The header is an SP_ERROR_MSG header, with the
length being the length of two string submessages; the submessages

View File

@ -2871,7 +2871,10 @@ static void
capture_input_error(capture_session *cap_session _U_, char *error_msg, char *secondary_error_msg)
{
cmdarg_err("%s", error_msg);
cmdarg_err_cont("%s", secondary_error_msg);
if (secondary_error_msg != NULL && *secondary_error_msg != '\0') {
/* We have both primary and secondary messages. */
cmdarg_err_cont("%s", secondary_error_msg);
}
}
@ -3159,7 +3162,7 @@ capture_input_drops(capture_session *cap_session _U_, guint32 dropped, const cha
static void
capture_input_closed(capture_session *cap_session _U_, gchar *msg)
{
if (msg != NULL)
if (msg != NULL && *msg != '\0')
fprintf(stderr, "tshark: %s\n", msg);
report_counts();

View File

@ -620,7 +620,7 @@ capture_input_error(capture_session *cap_session _U_, char *error_msg,
ws_assert(cap_session->state == CAPTURE_PREPARING || cap_session->state == CAPTURE_RUNNING);
safe_error_msg = simple_dialog_format_message(error_msg);
if (*secondary_error_msg != '\0') {
if (secondary_error_msg != NULL && *secondary_error_msg != '\0') {
/* We have both primary and secondary messages. */
safe_secondary_error_msg = simple_dialog_format_message(secondary_error_msg);
simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, "%s%s%s\n\n%s",