Revert "libnm: buffer reads in nm_vpn_service_plugin_read_vpn_details()"

This partially reverts commit 4a9fcb0fc3, which replaced one-byte
reads with buffered ones in the VPN service plugin.

Unfortunately the buffering means that commands coming after the magic
"DONE" string were being pulled into the buffer. Secrets agents expect
a "QUIT" to come after the "DONE", and since with buffering "QUIT" was
in the buffer, this led to a twenty-second delay on every VPN
connection using a secrets manager.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1164

Fixes: 4a9fcb0fc3 ('libnm: buffer reads in nm_vpn_service_plugin_read_vpn_details()')
This commit is contained in:
Bryan Jacobs
2022-03-27 13:07:29 +11:00
committed by Thomas Haller
parent bdcc85de76
commit 8ae9cf4698

View File

@@ -729,54 +729,6 @@ nm_vpn_service_plugin_secrets_required(NMVpnServicePlugin *plugin,
/*****************************************************************************/
typedef struct {
char *buf;
gsize n_buf;
int fd;
bool eof : 1;
char buf_full[1024];
} ReadFdBuf;
static inline gboolean
_read_fd_buf_c(ReadFdBuf *read_buf, char *ch)
{
gssize n_read;
if (read_buf->n_buf > 0)
goto out_data;
if (read_buf->eof)
return FALSE;
again:
n_read = read(read_buf->fd, read_buf->buf_full, sizeof(read_buf->buf_full));
if (n_read <= 0) {
if (n_read < 0 && errno == EAGAIN) {
struct pollfd pfd;
int r;
memset(&pfd, 0, sizeof(pfd));
pfd.fd = read_buf->fd;
pfd.events = POLLIN;
r = poll(&pfd, 1, -1);
if (r > 0)
goto again;
/* error or timeout. Fall through and set EOF. */
}
read_buf->eof = TRUE;
return FALSE;
}
read_buf->buf = read_buf->buf_full;
read_buf->n_buf = n_read;
out_data:
read_buf->n_buf--;
*ch = read_buf->buf[0];
read_buf->buf++;
return TRUE;
}
#define DATA_KEY_TAG "DATA_KEY="
#define DATA_VAL_TAG "DATA_VAL="
#define SECRET_KEY_TAG "SECRET_KEY="
@@ -808,7 +760,7 @@ nm_vpn_service_plugin_read_vpn_details(int fd, GHashTable **out_data, GHashTable
nm_auto_free_gstring GString *val = NULL;
nm_auto_free_gstring GString *line = NULL;
GString *str = NULL;
ReadFdBuf read_buf;
char c;
if (out_data)
g_return_val_if_fail(*out_data == NULL, FALSE);
@@ -819,27 +771,31 @@ nm_vpn_service_plugin_read_vpn_details(int fd, GHashTable **out_data, GHashTable
secrets =
g_hash_table_new_full(nm_str_hash, g_str_equal, g_free, (GDestroyNotify) nm_free_secret);
read_buf.buf = NULL;
read_buf.n_buf = 0;
read_buf.fd = fd;
read_buf.eof = FALSE;
line = g_string_new(NULL);
/* Read stdin for data and secret items until we get a DONE */
while (1) {
gboolean eof;
char c = '\0';
ssize_t nr;
eof = !_read_fd_buf_c(&read_buf, &c);
nr = read(fd, &c, 1);
if (nr < 0) {
if (errno == EAGAIN) {
struct pollfd pfd;
int r;
if (!eof && c == '\0') {
/* On the first '\0' char, we also assume the data is finished. Abort. */
read_buf.eof = TRUE;
eof = TRUE;
memset(&pfd, 0, sizeof(pfd));
pfd.fd = fd;
pfd.events = POLLIN;
r = poll(&pfd, 1, -1);
if (r > 0)
continue;
/* error or timeout. Fall through and break. */
}
break;
}
if (!eof && c != '\n') {
if (nr > 0 && c != '\n') {
g_string_append_c(line, c);
if (line->len > 512 * 1024) {
/* we are about to read a huge line. That is not right, abort. */
@@ -893,7 +849,7 @@ nm_vpn_service_plugin_read_vpn_details(int fd, GHashTable **out_data, GHashTable
next:
g_string_truncate(line, 0);
if (eof)
if (nr == 0)
break;
}