Skip to content

Commit

Permalink
Add support for pipelined SASL handshakes
Browse files Browse the repository at this point in the history
sd-bus sends the BEGIN command in at the same time as the AUTH command,
assuming that the authentication will succeed. This ends up confusing
xdg-dbus-proxy, as it assumes that BEGIN is not sent at the same time as
any other messages. As a result, the server's authentication
replies end up interpreted as standard D-Bus messages, failing parsing
and resulting in the client being unable to connect.

This changes the code to keep track of the number of authentication
commands pipelined with BEGIN, then counts the responses from the server
to know when the authentication phase of the connection has actually
completed.

Fixes #21 (finally!)

Signed-off-by: Ryan Gonzalez <[email protected]>
  • Loading branch information
refi64 authored and mardy committed Jun 25, 2024
1 parent 1bcfaea commit 40caaad
Showing 1 changed file with 73 additions and 9 deletions.
82 changes: 73 additions & 9 deletions flatpak-proxy.c
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,16 @@ typedef struct FlatpakProxyClient FlatpakProxyClient;
// Use a relatively hight number since there are not a lot of fake requests we need to do
#define MAX_CLIENT_SERIAL (G_MAXUINT32 - 65536)

typedef enum {
/* The client has not sent BEGIN yet */
AUTH_WAITING_FOR_BEGIN,
/* The client sent BEGIN, but the server has not yet responded to the auth
messages that the client sent before */
AUTH_WAITING_FOR_BACKLOG,
/* Authentication is fully complete */
AUTH_COMPLETE,
} AuthState;

typedef enum {
EXPECTED_REPLY_NONE,
EXPECTED_REPLY_NORMAL,
Expand Down Expand Up @@ -291,7 +301,9 @@ struct FlatpakProxyClient

FlatpakProxy *proxy;

gboolean authenticated;
AuthState auth_state;
/* Only set if auth_state == AUTH_WAITING_FOR_BACKLOG */
gsize auth_server_backlog;
GByteArray *auth_buffer;

ProxySide client_side;
Expand Down Expand Up @@ -2415,7 +2427,7 @@ got_buffer_from_client (FlatpakProxyClient *client, ProxySide *side, Buffer *buf
{
ExpectedReplyType expecting_reply = EXPECTED_REPLY_NONE;

if (client->authenticated && client->proxy->filter)
if (client->auth_state == AUTH_COMPLETE && client->proxy->filter)
{
g_autoptr(Header) header = NULL;
g_autoptr(GError) error = NULL;
Expand Down Expand Up @@ -2582,7 +2594,7 @@ got_buffer_from_client (FlatpakProxyClient *client, ProxySide *side, Buffer *buf
static void
got_buffer_from_bus (FlatpakProxyClient *client, ProxySide *side, Buffer *buffer)
{
if (client->authenticated && client->proxy->filter)
if (client->auth_state == AUTH_COMPLETE && client->proxy->filter)
{
g_autoptr(Header) header = NULL;
g_autoptr(GError) error = NULL;
Expand Down Expand Up @@ -2827,11 +2839,19 @@ auth_line_is_begin (guint8 *line)
next_char == '\t';
}

static guint8 *
find_auth_line_end (guint8 *line_start, gsize buffer_size)
{
return memmem (line_start, buffer_size,
AUTH_LINE_SENTINEL, strlen (AUTH_LINE_SENTINEL));
}

static gssize
find_auth_end (FlatpakProxyClient *client, Buffer *buffer)
{
goffset offset = 0;
gsize original_size = client->auth_buffer->len;
gsize lines_skipped = 0;

/* Add the new data to the remaining data from last iteration */
g_byte_array_append (client->auth_buffer, buffer->data, buffer->pos);
Expand All @@ -2842,8 +2862,7 @@ find_auth_end (FlatpakProxyClient *client, Buffer *buffer)
gsize remaining_data = client->auth_buffer->len - offset;
guint8 *line_end;

line_end = memmem (line_start, remaining_data,
AUTH_LINE_SENTINEL, strlen (AUTH_LINE_SENTINEL));
line_end = find_auth_line_end (line_start, remaining_data);
if (line_end) /* Found end of line */
{
offset = (line_end + strlen (AUTH_LINE_SENTINEL) - client->auth_buffer->data);
Expand All @@ -2853,9 +2872,13 @@ find_auth_end (FlatpakProxyClient *client, Buffer *buffer)

*line_end = 0;
if (auth_line_is_begin (line_start))
return offset - original_size;
{
client->auth_server_backlog = lines_skipped;
return offset - original_size;
}

/* continue with next line */
++lines_skipped;
}
else
{
Expand Down Expand Up @@ -2886,7 +2909,7 @@ side_in_cb (GSocket *socket, GIOCondition condition, gpointer user_data)
{
if (!side->got_first_byte)
buffer = buffer_new (1, NULL);
else if (!client->authenticated)
else if (client->auth_state != AUTH_COMPLETE)
buffer = buffer_new (64, NULL);
else
buffer = side->current_read_buffer;
Expand All @@ -2898,7 +2921,7 @@ side_in_cb (GSocket *socket, GIOCondition condition, gpointer user_data)
break;
}

if (!client->authenticated)
if (client->auth_state == AUTH_WAITING_FOR_BEGIN)
{
if (buffer->pos > 0)
{
Expand Down Expand Up @@ -2940,7 +2963,48 @@ side_in_cb (GSocket *socket, GIOCondition condition, gpointer user_data)
got_buffer_from_side (side, buffer);

if (found_auth_end)
client->authenticated = TRUE;
client->auth_state = client->auth_server_backlog
? AUTH_WAITING_FOR_BACKLOG
: AUTH_COMPLETE;
}
else
{
buffer_unref (buffer);
}
}
else if (client->auth_state == AUTH_WAITING_FOR_BACKLOG)
{
if (buffer->pos > 0)
{
if (side == &client->bus_side)
{
guint8 *line_start = buffer->data;
guint8 *line_end = NULL;
gsize extra_data = 0;

while ((line_end = find_auth_line_end (line_start, buffer->pos)))
{
line_start = line_end + strlen (AUTH_LINE_SENTINEL);

if (--client->auth_server_backlog == 0)
{
buffer->size = line_start - buffer->data;
extra_data = buffer->pos - buffer->size;

/* We may have gotten some extra data which is not part of
the auth handshake, keep it for the next iteration. */
if (extra_data > 0)
side->extra_input_data = g_bytes_new (line_start, extra_data);

break;
}
}
}

got_buffer_from_side (side, buffer);

if (client->auth_server_backlog == 0)
client->auth_state = AUTH_COMPLETE;
}
else
{
Expand Down

0 comments on commit 40caaad

Please sign in to comment.