Skip to content

Commit

Permalink
conn_sock: add full-attach option
Browse files Browse the repository at this point in the history
unfortunately, older podman versions rely on conmon truncating the unix socket length when setting up the attach socket.

Introduce a flag to gate the changed behavior, allowing newer podman versions to be able to handle longer UID in rootless mode,
but also allowing older podman versions to work.

This also removes the logic for `do_chdir` in favor of using a /proc/self/fd entry for the sock path.
That allows conmon to not chdir before attaching to the unix socket.

Signed-off-by: Peter Hunt <pehunt@redhat.com>
  • Loading branch information
haircommander committed Jan 14, 2021
1 parent 7bc96c7 commit 93f5b30
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 28 deletions.
2 changes: 2 additions & 0 deletions src/cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ char *opt_log_tag = NULL;
gboolean opt_sync = FALSE;
gboolean opt_no_sync_log = FALSE;
char *opt_sdnotify_socket = NULL;
gboolean opt_full_attach_path = FALSE;
GOptionEntry opt_entries[] = {
{"api-version", 0, 0, G_OPTION_ARG_NONE, &opt_api_version, "Conmon API version to use", NULL},
{"bundle", 'b', 0, G_OPTION_ARG_STRING, &opt_bundle_path, "Location of the OCI Bundle path", NULL},
Expand Down Expand Up @@ -97,6 +98,7 @@ GOptionEntry opt_entries[] = {
{"terminal", 't', 0, G_OPTION_ARG_NONE, &opt_terminal, "Allocate a pseudo-TTY. The default is false", NULL},
{"timeout", 'T', 0, G_OPTION_ARG_INT, &opt_timeout, "Kill container after specified timeout in seconds.", NULL},
{"version", 0, 0, G_OPTION_ARG_NONE, &opt_version, "Print the version and exit", NULL},
{"full-attach", 0, 0, G_OPTION_ARG_NONE, &opt_full_attach_path, "Don't truncate the path to the attach socket.", NULL},
{NULL, 0, 0, 0, NULL, NULL, NULL}};


Expand Down
1 change: 1 addition & 0 deletions src/cli.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ extern gboolean opt_no_sync_log;
extern gboolean opt_sync;
extern char *opt_sdnotify_socket;
extern GOptionEntry opt_entries[];
extern gboolean opt_full_attach_path;

int initialize_cli(int argc, char *argv[]);
void process_cli();
Expand Down
71 changes: 43 additions & 28 deletions src/conn_sock.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ static void remote_sock_shutdown(struct remote_sock_s *sock, int how);
static void schedule_local_sock_write(struct local_sock_s *local_sock);
static void sock_try_write_to_local_sock(struct remote_sock_s *sock);
static gboolean local_sock_write_cb(G_GNUC_UNUSED int fd, G_GNUC_UNUSED GIOCondition condition, G_GNUC_UNUSED gpointer user_data);
static char *bind_unix_socket(char *socket_relative_name, int sock_type, mode_t perms, struct remote_sock_s *remote_sock);
static char *bind_unix_socket(char *socket_relative_name, int sock_type, mode_t perms, struct remote_sock_s *remote_sock,
gboolean use_full_attach_path);
static char *get_sock_path(gboolean use_full_attach_path, char *base_path, size_t desired_len);
/*
Since our socket handling is abstract now, handling is based on sock_type, so we can pass around a structure
that contains everything we need to handle I/O. Callbacks used to handle IO, for example, and whether this
Expand Down Expand Up @@ -109,7 +111,8 @@ char *setup_console_socket(void)

char *setup_attach_socket(void)
{
char *symlink_dir_path = bind_unix_socket("attach", SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0700, &remote_attach_sock);
char *symlink_dir_path =
bind_unix_socket("attach", SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0700, &remote_attach_sock, opt_full_attach_path);

if (listen(remote_attach_sock.fd, 10) == -1)
pexitf("Failed to listen on attach socket: %s/%s", symlink_dir_path, "attach");
Expand All @@ -134,80 +137,92 @@ void setup_notify_socket(char *socket_path)
/* No _cleanup_free_ here so we don't get a warning about unused variables
* when compiling with clang */
char *symlink_dir_path =
bind_unix_socket("notify/notify.sock", SOCK_DGRAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0777, &remote_notify_sock);
bind_unix_socket("notify/notify.sock", SOCK_DGRAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0777, &remote_notify_sock, TRUE);
g_unix_fd_add(remote_notify_sock.fd, G_IO_IN | G_IO_HUP | G_IO_ERR, remote_sock_cb, &remote_notify_sock);

g_free(symlink_dir_path);
}

/* REMEMBER to g_free() the return value! */
static char *bind_unix_socket(char *socket_relative_name, int sock_type, mode_t perms, struct remote_sock_s *remote_sock)
static char *bind_unix_socket(char *socket_relative_name, int sock_type, mode_t perms, struct remote_sock_s *remote_sock,
gboolean use_full_attach_path)
{
int socket_fd = -1;
struct sockaddr_un socket_addr = {0};
socket_addr.sun_family = AF_UNIX;
_cleanup_free_ char *cwd = NULL;

/*
* Create a symlink so we don't exceed unix domain socket
* path length limit.
*
* We do NOT free this because it's returned to the parent, who is responsible for freeing it!
*/
char *base_path = g_build_filename(opt_socket_path, opt_cuuid, NULL);
char *base_path =
get_sock_path(use_full_attach_path, g_build_filename(opt_socket_path, opt_cuuid, NULL), sizeof(socket_addr.sun_path));

/*
* Create a symlink so we don't exceed unix domain socket
* path length limit. We use the base path passed in from our parent.
* Instead of creating a symlink, open the desired socket path and use the fd reference
* from /proc/self/fd.
*/
if (unlink(base_path) == -1 && errno != ENOENT)
pexitf("Failed to remove existing symlink for socket directory %s", base_path);
_cleanup_free_ char *sock_fullpath = g_build_filename(base_path, socket_relative_name, NULL);

if (symlink(opt_bundle_path, base_path) == -1)
pexit("Failed to create symlink for notify socket");
_cleanup_close_ int sock_fullpath_fd = open(sock_fullpath, O_PATH);
if (sock_fullpath_fd < 0)
pexitf("failed to open sockpath %s", sock_fullpath);

_cleanup_free_ char *sock_proc_entry = g_strdup_printf("/proc/self/fd/%d", sock_fullpath_fd);

_cleanup_free_ char *sock_fullpath = g_build_filename(base_path, socket_relative_name, NULL);
_cleanup_free_ char *sock_relpath = g_build_filename(opt_cuuid, socket_relative_name, NULL);
ninfof("socket path: %s", sock_fullpath);

strncpy(socket_addr.sun_path, sock_relpath, sizeof(socket_addr.sun_path) - 1);
strncpy(socket_addr.sun_path, sock_proc_entry, sizeof(socket_addr.sun_path) - 1);
ninfof("addr{sun_family=AF_UNIX, sun_path=%s}", socket_addr.sun_path);

/*
* We make the socket non-blocking to avoid a race where client aborts connection
* before the server gets a chance to call accept. In that scenario, the server
* accept blocks till a new client connection comes in.
*/
if ((cwd = getcwd(NULL, 0)) == NULL)
pexitf("Failed to get CWD for socket %s", sock_fullpath);

socket_fd = socket(AF_UNIX, sock_type, 0);
if (socket_fd == -1)
pexitf("Failed to create socket %s", sock_fullpath);

if (fchmod(socket_fd, perms))
pexitf("Failed to change socket permissions %s", sock_fullpath);

if (chdir(opt_socket_path) == -1)
pexitf("Could not chdir to %s", opt_socket_path);

if (unlink(sock_fullpath) == -1 && errno != ENOENT)
pexitf("Failed to remove existing socket: %s", sock_fullpath);

if (bind(socket_fd, (struct sockaddr *)&socket_addr, sizeof(struct sockaddr_un)) == -1)
pexitf("Failed to bind socket: %s", sock_fullpath);

if (chmod(sock_fullpath, perms))
pexitf("Failed to change socket permissions %s", sock_fullpath);

if (chdir(cwd) == -1)
pexitf("Could not chdir to %s", cwd);

remote_sock->fd = socket_fd;

return base_path;
}

/*
* get_sock_path decides whether to truncate the socket path, to match
* the caller's expectation.
* use_full_attach_path is whether conmon was told to not truncate the path.
* base_path is the path of the socket
* desired_len is the length of socket_addr.sun_path (should be strlen(char[108]) on linux).
*/
char *get_sock_path(gboolean use_full_attach_path, char *base_path, size_t desired_len)
{
if (use_full_attach_path || strlen(base_path) != (desired_len - 1))
return base_path;
/*
* This is to address a corner case where the symlink path length can end up being
* the same as the socket. When it happens, the symlink prevents the socket from being
* be created. This could still be a problem with other containers, but it is safe
* to assume the CUUIDs don't change length in the same directory. As a workaround,
* in such case, make the symlink one char shorter.
*
* If we're using using_full_attach_path, this is unnecessary.
*/
base_path[desired_len - 2] = '\0';
return base_path;
}


void schedule_main_stdin_write()
{
Expand Down

0 comments on commit 93f5b30

Please sign in to comment.