Skip to content
Permalink
Browse files

tls: Use socket activation instead of spawning

Instead of fork()/exec()ing the cockpit-ws instances for ourselves,
simply connect to the sockets that systemd is now creating for us.

In addition to shrinking the code by about 600 lines, this has the
following advantages:

  - the cockpit-ws instances are now running as a separate user for
    improved isolation

  - the cockpit-ws instances are now also each in their own cgroup

  - there used to be a race where we would drop an incoming web
    connection if we attempted to connect to a cockpit-ws instance as it
    was exiting due to inactivity.  systemd socket activation avoids
    this.

Use the socket-activation-helper to tweak the tests to adjust to the
changes.

This temporarily drops the per-client-certificate ws instances, there is
only one https instance for now; this is not a practical regression as
we don't support client certificates yet. This will be brought back
separately.
  • Loading branch information...
allisonkarlitskaya authored and martinpitt committed Sep 27, 2019
1 parent 777c590 commit 708671171d1d791d8f4bb68431099d5d25fc868b
@@ -107,8 +107,8 @@ depcomp
/mock/
*.min.html
/src/ws/cockpit.appdata.xml
/src/ws/cockpit.service
/src/ws/cockpit.socket
/src/ws/cockpit*.service
/src/ws/cockpit*.socket
/src/ws/cockpit-tempfiles.conf
src/common/cockpitassets.c
src/common/cockpitassets.h
@@ -119,6 +119,8 @@ SUBST_RULE = \
-e 's,[@]SUDO[@],$(SUDO),g' \
-e 's,[@]user[@],$(COCKPIT_USER),g' \
-e 's,[@]group[@],$(COCKPIT_GROUP),g' \
-e 's,[@]wsinstanceuser[@],$(COCKPIT_WSINSTANCE_USER),g' \
-e 's,[@]wsinstancegroup[@],$(COCKPIT_WSINSTANCE_GROUP),g' \
-e 's,[@]selinux_config_type[@],$(COCKPIT_SELINUX_CONFIG_TYPE),g' \
-e 's,[@]with_networkmanager_needs_root[@],$(with_networkmanager_needs_root),g' \
-e 's,[@]with_storaged_iscsi_sessions[@],$(with_storaged_iscsi_sessions),g' \
@@ -410,7 +410,7 @@ fi
AM_CONDITIONAL(WITH_ASAN, test "$enable_asan" = "yes")
AC_MSG_RESULT($asan_status)

# User and group for running cockpit-ws
# User and group for running cockpit web server (cockpit-tls or -ws in customized setups)

AC_ARG_WITH(cockpit_user,
AS_HELP_STRING([--with-cockpit-user=<user>],
@@ -437,6 +437,35 @@ fi
AC_SUBST(COCKPIT_USER)
AC_SUBST(COCKPIT_GROUP)

# User for running cockpit-ws instances from cockpit-tls

AC_ARG_WITH(cockpit_ws_instance_user,
AS_HELP_STRING([--with-cockpit-ws-instance-user=<user>],
[User for running cockpit-ws instances from cockpit-tls (root)]
)
)
AC_ARG_WITH(cockpit_ws_instance_group,
AS_HELP_STRING([--with-cockpit-ws-instance-group=<group>],
[Group for running cockpit-ws instances from cockpit-tls]
)
)
if test -z "$with_cockpit_ws_instance_user"; then
if test "$COCKPIT_USER" != "root"; then
AC_MSG_ERROR([--with-cockpit-ws-instance-user is required when setting --with-cockpit-user])
fi
COCKPIT_WSINSTANCE_USER=root
else
COCKPIT_WSINSTANCE_USER=$with_cockpit_ws_instance_user
if test -z "$with_cockpit_ws_instance_group"; then
COCKPIT_WSINSTANCE_GROUP=$with_cockpit_ws_instance_user
else
COCKPIT_WSINSTANCE_GROUP=$with_cockpit_ws_instance_group
fi
fi

AC_SUBST(COCKPIT_WSINSTANCE_USER)
AC_SUBST(COCKPIT_WSINSTANCE_GROUP)

AC_ARG_WITH(selinux_config_type,
AS_HELP_STRING([--with-selinux-config-type=<type>],
[SELinux context type for cockpit config files]
@@ -586,6 +615,8 @@ echo "

cockpit-ws user: ${COCKPIT_USER}
cockpit-ws group: ${COCKPIT_GROUP}
cockpit-ws instance user: ${COCKPIT_WSINSTANCE_USER}
cockpit-ws instance group: ${COCKPIT_WSINSTANCE_GROUP}
selinux config type: ${COCKPIT_SELINUX_CONFIG_TYPE}

Maintainer mode: ${USE_MAINTAINER_MODE}
@@ -595,7 +626,7 @@ echo "
With coverage: ${enable_coverage}
With address sanitizer: ${asan_status}
With PCP: ${enable_pcp}
Branding: ${BRAND}
Branding: ${BRAND}

cockpit-ssh: ${enable_ssh}
Supports key auth: ${key_auth}
@@ -2,8 +2,6 @@ libexec_PROGRAMS += cockpit-tls

cockpit_tls_SOURCES = \
src/tls/utils.h \
src/tls/wsinstance.h \
src/tls/wsinstance.c \
src/tls/connection.h \
src/tls/connection.c \
src/tls/server.h \
@@ -34,7 +32,6 @@ TEST_LDADD = \

TLS_TESTS = \
test-tls-connection \
test-tls-wsinstance \
test-tls-server \
$(NULL)

@@ -50,16 +47,7 @@ test_tls_connection_SOURCES = \
test_tls_connection_CFLAGS = $(TEST_CFLAGS)
test_tls_connection_LDADD = $(TEST_LDADD)

test_tls_wsinstance_SOURCES = \
src/tls/wsinstance.c \
src/tls/test-wsinstance.c \
$(NULL)

test_tls_wsinstance_CFLAGS = $(TEST_CFLAGS)
test_tls_wsinstance_LDADD = $(TEST_LDADD)

test_tls_server_SOURCES = \
src/tls/wsinstance.c \
src/tls/connection.c \
src/tls/server.c \
src/tls/test-server.c \
@@ -40,6 +40,7 @@ connection_new (int client_fd)
con->client_fd = client_fd;
con->buf_client.connection = con;
con->buf_ws.connection = con;
con->ws_fd = -1;

debug (CONNECTION, "new connection on fd %i", con->client_fd);
return con;
@@ -20,8 +20,7 @@
#pragma once

#include <stdbool.h>

#include "wsinstance.h"
#include <gnutls/gnutls.h>

typedef enum { CLIENT, WS } DataSource;
typedef enum { SUCCESS, PARTIAL, CLOSED, RETRY, FATAL } ConnectionResult;
@@ -41,7 +40,6 @@ struct Connection {
int client_fd;
bool is_tls;
gnutls_session_t session;
WsInstance *ws;
int ws_fd;
struct ConnectionBuffer buf_client;
struct ConnectionBuffer buf_ws;
@@ -30,8 +30,6 @@
#include "utils.h"
#include "server.h"

#define COCKPIT_WS PACKAGE_LIBEXEC_DIR "/cockpit-ws"

/* CLI arguments */
struct arguments {
uint16_t port;
@@ -112,7 +110,7 @@ main (int argc, char **argv)
}

/* TODO: Add cockpit.conf option to enable client-certificate auth, once we support that */
server_init (COCKPIT_WS, arguments.port, certfile, NULL, CERT_NONE);
server_init ("/run/cockpit/wsinstance", arguments.port, certfile, NULL, CERT_NONE);
free (certfile);

server_run (arguments.idle_timeout);

0 comments on commit 7086711

Please sign in to comment.
You can’t perform that action at this time.