colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cli/polkit: make parsing polkit-agent-helper-1 protocol more conforming

Browse Source 
- in io_watch_have_data(), ensure that we handle incomplete lines
that don't yet have a newline by waiting for more data. That means,
if the current content of the in_buffer does not have a newline, we
wait longer.

- in io_watch_have_data(), implement (and ignore) certain commands
instead of failing the request.

- in io_watch_have_data(), no longer g_compress() the entire line.
"polkitagenthelper-pam.c" never backslash escapes the command, it
only escapes the arguments. Of course, there should be no difference
in practice, except that we don't want to handle escape sequences
in the commands.

- in io_watch_have_data(), compare SUCCESS/FAILURE literally.
"polkitagenthelper-pam.c" never appends any trailing garbage to these
commands, and we shouldn't handle that (although "polkitagentsession.c"
does).

- when io_watch_have_data() completes with success, we cannot destroy
AuthRequest right away. It probably still has data pending that we first
need to write to the polkit helper. Wait longer, and let io_watch_can_write()
complete the request.

- ensure we always answer the GDBusMethodInvocation. Otherwise, it gets
leaked.

- use NMStrBuf instead of GString.
This commit is contained in:
Thomas Haller
2020-04-06 10:54:35 +02:00
parent 277925c36a
commit 3e1e63e57d
3 changed files with 158 additions and 136 deletions
Download Patch File Download Diff File Expand all files Collapse all files

210
clients/common/nm-polkit-listener.c
View File

@@ -27,6 +27,7 @@
#include <fcntl.h>
#include "nm-glib-aux/nm-dbus-aux.h"
#include "nm-glib-aux/nm-str-buf.h"
#include "nm-glib-aux/nm-secret-utils.h"
#include "nm-glib-aux/nm-io-utils.h"
#include "nm-libnm-core-intern/nm-auth-subject.h"
@@ -84,19 +85,22 @@ typedef struct {
CList request_lst;
NMPolkitListener *listener;
GSource *child_stdout_watch_source;
GSource *child_stdin_watch_source;
GDBusMethodInvocation *dbus_invocation;
char *action_id;
char *message;
char *username;
char *cookie;
GString *in_buffer;
GString *out_buffer;
size_t out_buffer_offset;
NMStrBuf in_buffer;
NMStrBuf out_buffer;
gsize out_buffer_offset;
int child_stdout;
int child_stdin;
GSource *child_stdout_watch_source;
GSource *child_stdin_watch_source;
GDBusMethodInvocation *dbus_invocation;
bool request_any_response:1;
bool request_is_completed:1;
} AuthRequest;
static const GDBusInterfaceInfo interface_info = NM_DEFINE_GDBUS_INTERFACE_INFO_INIT (
@@ -123,10 +127,18 @@ static const GDBusInterfaceInfo interface_info = NM_DEFINE_GDBUS_INTERFACE_INFO_
);
static void
remove_request (AuthRequest *request)
auth_request_complete (AuthRequest *request, gboolean success)
{
c_list_unlink (&request->request_lst);
if (success)
g_dbus_method_invocation_return_value (request->dbus_invocation, NULL);
else {
g_dbus_method_invocation_return_dbus_error (request->dbus_invocation,
POLKIT_DBUS_ERROR_FAILED,
"");
}
nm_clear_g_free (&request->action_id);
nm_clear_g_free (&request->message);
nm_clear_g_free (&request->username);
@@ -134,10 +146,8 @@ remove_request (AuthRequest *request)
nm_clear_g_source_inst (&request->child_stdout_watch_source);
nm_clear_g_source_inst (&request->child_stdin_watch_source);
nm_explicit_bzero (request->out_buffer->str,
request->out_buffer->len);
g_string_free (request->out_buffer, TRUE);
g_string_free (request->in_buffer, TRUE);
nm_str_buf_destroy (&request->in_buffer);
nm_str_buf_destroy (&request->out_buffer);
if (request->child_stdout != -1) {
nm_close (request->child_stdout);
@@ -149,7 +159,7 @@ remove_request (AuthRequest *request)
request->child_stdin = -1;
}
g_slice_free (AuthRequest, request);
nm_g_slice_free (request);
}
static gboolean
@@ -384,60 +394,42 @@ retrieve_session_id (NMPolkitListener *self)
self);
}
static void
complete_authentication (AuthRequest *request,
gboolean result)
{
if (result) {
g_dbus_method_invocation_return_value (request->dbus_invocation, NULL);
} else {
g_dbus_method_invocation_return_dbus_error (request->dbus_invocation,
"org.freedesktop.PolicyKit1.Error.Failed",
"");
}
remove_request (request);
}
static gboolean
io_watch_can_write (int fd,
GIOCondition condition,
gpointer user_data)
{
AuthRequest *request = user_data;
ssize_t n_written;
gboolean done = FALSE;
gssize n_written;
if (condition & G_IO_HUP ||
condition & G_IO_ERR) {
done = TRUE;
if (NM_FLAGS_ANY (condition, (G_IO_HUP | G_IO_ERR)))
goto done;
}
n_written = write (request->child_stdin,
&request->out_buffer->str[request->out_buffer_offset],
request->out_buffer->len - request->out_buffer_offset);
&((nm_str_buf_get_str_unsafe (&request->out_buffer))[request->out_buffer_offset]),
request->out_buffer.len - request->out_buffer_offset);
if (n_written < 0 && errno != EAGAIN) {
done = TRUE;
if ( n_written < 0
&& errno != EAGAIN)
goto done;
}
if (n_written > 0) {
if ((size_t) n_written == (request->out_buffer->len - request->out_buffer_offset)) {
done = TRUE;
if ((gsize) n_written >= (request->out_buffer.len - request->out_buffer_offset)) {
nm_assert ((gsize) n_written == (request->out_buffer.len - request->out_buffer_offset));
goto done;
}
request->out_buffer_offset += n_written;
request->out_buffer_offset += (gsize) n_written;
}
return G_SOURCE_CONTINUE;
done:
if (done) {
nm_explicit_bzero (request->out_buffer->str,
request->out_buffer->len);
g_string_set_size (request->out_buffer, 0);
nm_str_buf_set_size (&request->out_buffer, 0, TRUE, FALSE);
request->out_buffer_offset = 0;
nm_clear_g_source_inst (&request->child_stdin_watch_source);
}
if (request->request_is_completed)
auth_request_complete (request, TRUE);
return G_SOURCE_CONTINUE;
}
@@ -447,11 +439,11 @@ queue_string_to_helper (AuthRequest *request, const char *response)
{
g_return_if_fail (response);
g_string_append (request->out_buffer, response);
if (!nm_str_buf_is_initalized (&request->out_buffer))
nm_str_buf_init (&request->out_buffer, strlen (response) + 2u, TRUE);
if ( request->out_buffer->len == 0
|| request->out_buffer->str[request->out_buffer->len - 1] != '\n')
g_string_append_c (request->out_buffer, '\n');
nm_str_buf_append (&request->out_buffer, response);
nm_str_buf_ensure_trailing_c (&request->out_buffer, '\n');
if (!request->child_stdin_watch_source) {
request->child_stdin_watch_source = nm_g_unix_fd_source_new (request->child_stdin,
@@ -471,41 +463,45 @@ io_watch_have_data (int fd,
gpointer user_data)
{
AuthRequest *request = user_data;
gs_free char *unescaped = NULL;
char *response = NULL;
char* line_terminator = 0;
gboolean auth_result = FALSE;
gboolean complete_auth = FALSE;
ssize_t n_read;
gboolean auth_result;
gssize n_read;
if (condition & G_IO_HUP ||
condition & G_IO_ERR) {
complete_auth = TRUE;
auth_result = FALSE;
goto out;
}
n_read = nm_utils_fd_read (fd, request->in_buffer);
if (NM_FLAGS_ANY (condition, G_IO_HUP | G_IO_ERR))
n_read = -EIO;
else
n_read = nm_utils_fd_read (fd, &request->in_buffer);
if (n_read <= 0) {
if (n_read == -EAGAIN) {
/* wait longer. */
return G_SOURCE_CONTINUE;
}
if (n_read < 0) {
complete_auth = TRUE;
auth_result = FALSE;
/* Either an error or EOF happened. The data we parsed so far was not relevant.
* Regardless of what we still have unprocessed in the receive buffers, we are done.
*
* We would expect that the other side completed with SUCCESS or FAILURE. Apparently
* it didn't. If we had any good request, we assume success. */
auth_result = request->request_any_response;
goto out;
}
line_terminator = strchr (request->in_buffer->str, '\n');
while (TRUE) {
char *line_terminator;
const char *line;
line = nm_str_buf_get_str (&request->in_buffer);
line_terminator = (char *) strchr (line, '\n');
if (!line_terminator) {
/* We don't have a complete line. Wait longer. */
return G_SOURCE_CONTINUE;
}
*line_terminator = '\0';
line_terminator[0] = '\0';
unescaped = g_strcompress (request->in_buffer->str);
if ( NM_STR_HAS_PREFIX (line, "PAM_PROMPT_ECHO_OFF ")
|| NM_STR_HAS_PREFIX (line, "PAM_PROMPT_ECHO_ON ")) {
nm_auto_free_secret char *response = NULL;
if (NM_STR_HAS_PREFIX (unescaped, "PAM_PROMPT_ECHO")) {
/* FIXME(cli-async): emit signal and wait for response (blocking) */
g_signal_emit (request->listener,
signals[REQUEST_SYNC],
@@ -517,28 +513,37 @@ io_watch_have_data (int fd,
if (response) {
queue_string_to_helper (request, response);
nm_free_secret (response);
request->request_any_response = TRUE;
goto erase_line;
}
auth_result = FALSE;
} else if (nm_streq (line, "SUCCESS"))
auth_result = TRUE;
else if (nm_streq (line, "FAILURE"))
auth_result = FALSE;
else if ( NM_STR_HAS_PREFIX (line, "PAM_ERROR_MSG ")
|| NM_STR_HAS_PREFIX (line, "PAM_TEXT_INFO ")) {
/* ignore. */
goto erase_line;
} else {
complete_auth = TRUE;
/* unknown command. Fail. */
auth_result = FALSE;
}
} else if (NM_STR_HAS_PREFIX (unescaped, "SUCCESS")) {
complete_auth = TRUE;
auth_result = TRUE;
} else if (NM_STR_HAS_PREFIX (unescaped, "FAILURE")) {
complete_auth = TRUE;
auth_result = FALSE;
} else {
complete_auth = TRUE;
auth_result = FALSE;
break;
erase_line:
nm_str_buf_erase (&request->in_buffer, 0, line_terminator - line + 1u, TRUE);
}
out:
g_string_set_size (request->in_buffer, 0);
request->request_is_completed = TRUE;
nm_clear_g_source_inst (&request->child_stdout_watch_source);
if ( auth_result
&& request->child_stdin_watch_source) {
/* we need to wait for the buffer to send the response. */
} else
auth_request_complete (request, auth_result);
if (complete_auth) {
complete_authentication (request, auth_result);
}
return G_SOURCE_CONTINUE;
}
@@ -553,7 +558,7 @@ begin_authentication (AuthRequest *request)
};
if (!request->username) {
complete_authentication (request, FALSE);
auth_request_complete (request, FALSE);
return;
}
@@ -576,7 +581,7 @@ begin_authentication (AuthRequest *request)
0,
"The PolicyKit setuid helper 'polkit-agent-helper-1' has not been found");
complete_authentication (request, FALSE);
auth_request_complete (request, FALSE);
return;
}
@@ -623,19 +628,21 @@ create_request (NMPolkitListener *listener,
char *username_take,
const char *cookie)
{
AuthRequest *request = g_slice_new0(AuthRequest);
AuthRequest *request;
request->listener = listener;
request->dbus_invocation = invocation;
request->action_id = g_strdup (action_id);
request->message = g_strdup (message);
request->username = g_steal_pointer (&username_take);
request->cookie = g_strdup (cookie);
request->in_buffer = g_string_new ("");
request = g_slice_new (AuthRequest);
*request = (AuthRequest) {
.listener = listener,
.dbus_invocation = invocation,
.action_id = g_strdup (action_id),
.message = g_strdup (message),
.username = g_steal_pointer (&username_take),
.cookie = g_strdup (cookie),
.request_any_response = FALSE,
.request_is_completed = FALSE,
};
/* preallocate a large enough buffer so that
* secrets don't get reallocated, thus leaked */
request->out_buffer = g_string_sized_new (1024);
nm_str_buf_init (&request->in_buffer, NM_UTILS_GET_NEXT_REALLOC_SIZE_1000, FALSE);
c_list_link_tail (&listener->request_lst_head, &request->request_lst);
return request;
@@ -696,7 +703,7 @@ dbus_method_call_cb (GDBusConnection *connection,
}
/* Complete a cancelled request with success. */
complete_authentication (request, TRUE);
auth_request_complete (request, TRUE);
return;
}
@@ -895,9 +902,8 @@ dispose (GObject *object)
nm_clear_g_cancellable (&self->cancellable);
while ((request = c_list_first_entry (&self->request_lst_head, AuthRequest, request_lst))) {
remove_request (request);
}
while ((request = c_list_first_entry (&self->request_lst_head, AuthRequest, request_lst)))
auth_request_complete (request, FALSE);
if (self->dbus_connection) {
nm_clear_g_dbus_connection_signal (self->dbus_connection,

42
shared/nm-glib-aux/nm-io-utils.c
View File

@@ -11,6 +11,7 @@
#include <sys/stat.h>
#include <fcntl.h>
#include "nm-str-buf.h"
#include "nm-shared-utils.h"
#include "nm-secret-utils.h"
#include "nm-errno.h"
@@ -418,27 +419,40 @@ nm_utils_file_stat (const char *filename, struct stat *out_st)
* @fd: the fd to read from.
* @out_string: (out): output string where read bytes will be stored.
*
* Returns: <0 on failure, which is -(errno)
* 0 on EOF or if the call would block (if the fd is nonblocking),
* >0 on success, which is the number of bytes read */
ssize_t
nm_utils_fd_read (int fd, GString *out_string)
* Returns: <0 on failure, which is -(errno).
* 0 on EOF.
* >0 on success, which is the number of bytes read. */
gssize
nm_utils_fd_read (int fd, NMStrBuf *out_string)
{
size_t start_len;
ssize_t n_read;
gsize buf_available;
gssize n_read;
int errsv;
g_return_val_if_fail (fd >= 0, -1);
g_return_val_if_fail (out_string, -1);
start_len = out_string->len;
g_string_set_size (out_string, start_len + 1024);
/* If the buffer size is 0, we allocate NM_UTILS_GET_NEXT_REALLOC_SIZE_1000 (1000 bytes)
* the first time. Afterwards, the buffer grows exponentially.
*
* Note that with @buf_available, we always would read as much buffer as we actually
* have reserved. */
nm_str_buf_maybe_expand (out_string, NM_UTILS_GET_NEXT_REALLOC_SIZE_1000, FALSE);
n_read = read (fd, &out_string->str[start_len], 1024);
if (n_read < 0)
return -NM_ERRNO_NATIVE (errno);
buf_available = out_string->allocated - out_string->len;
if (n_read > 0)
g_string_set_size (out_string, start_len + (gsize) n_read);
n_read = read (fd,
&((nm_str_buf_get_str_unsafe (out_string))[out_string->len]),
buf_available);
if (n_read < 0) {
errsv = errno;
return -NM_ERRNO_NATIVE (errsv);
}
if (n_read > 0) {
nm_assert ((gsize) n_read <= buf_available);
nm_str_buf_set_size (out_string, out_string->len + (gsize) n_read, TRUE, FALSE);
}
return n_read;
}

4
shared/nm-glib-aux/nm-io-utils.h
View File

@@ -47,7 +47,9 @@ gboolean nm_utils_file_set_contents (const char *filename,
int *out_errsv,
GError **error);
ssize_t nm_utils_fd_read (int fd, GString *out_string);
struct _NMStrBuf;
gssize nm_utils_fd_read (int fd, struct _NMStrBuf *out_string);
struct stat;