[PATCH 07/13] discover/discover-server: Restrict clients based on uid

Samuel Mendoza-Jonas sam at mendozajonas.com
Fri Nov 23 10:36:24 AEDT 2018


If crypt support is enabled restrict what actions clients can perform by
default. Initial authorisation is set at connection time; clients
running as root are unrestricted, anything else runs as restricted until
it makes an authentication to pb-discover.

Unprivileged clients may only perform the following actions:
- Boot the default boot option.
- Cancel the autoboot timeout.
- Make an authentication request.

If a group named "petitgroup" exists then the socket permissions are
also modified so that only clients running as root or in that group may
connect to the socket.
The user-event socket is only usable by root since the two main
usecases are by utilities called by pb-discover or by a user in the
shell who will need to su to root anyway.

Signed-off-by: Samuel Mendoza-Jonas <sam at mendozajonas.com>
---
 discover/discover-server.c | 236 ++++++++++++++++++++++++++++++++++++-
 discover/discover-server.h |   3 +
 discover/pb-discover.c     |   3 +
 discover/platform.c        |  13 ++
 discover/platform.h        |   4 +
 discover/user-event.c      |   7 +-
 6 files changed, 263 insertions(+), 3 deletions(-)

diff --git a/discover/discover-server.c b/discover/discover-server.c
index 34d82be8..434b4966 100644
--- a/discover/discover-server.c
+++ b/discover/discover-server.c
@@ -1,3 +1,4 @@
+#define _GNU_SOURCE
 
 #include <unistd.h>
 #include <stdlib.h>
@@ -10,11 +11,15 @@
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <asm/byteorder.h>
+#include <grp.h>
+#include <sys/stat.h>
 
 #include <pb-config/pb-config.h>
 #include <talloc/talloc.h>
 #include <waiter/waiter.h>
 #include <log/log.h>
+#include <crypt/crypt.h>
+#include <i18n/i18n.h>
 
 #include "pb-protocol/pb-protocol.h"
 #include "list/list.h"
@@ -31,6 +36,7 @@ struct discover_server {
 	struct list clients;
 	struct list status;
 	struct device_handler *device_handler;
+	bool restrict_clients;
 };
 
 struct client {
@@ -39,6 +45,8 @@ struct client {
 	struct waiter *waiter;
 	int fd;
 	bool remote_closed;
+	bool can_modify;
+	struct waiter *auth_waiter;
 };
 
 
@@ -245,11 +253,126 @@ static int write_config_message(struct discover_server *server,
 	return client_write_message(server, client, message);
 }
 
+static int write_authenticate_message(struct discover_server *server,
+		struct client *client)
+{
+	struct pb_protocol_message *message;
+	struct auth_message auth_msg;
+	int len;
+
+	auth_msg.op = AUTH_MSG_RESPONSE;
+	auth_msg.authenticated = client->can_modify;
+
+	len = pb_protocol_authenticate_len(&auth_msg);
+
+	message = pb_protocol_create_message(client,
+			PB_PROTOCOL_ACTION_AUTHENTICATE, len);
+	if (!message)
+		return -1;
+
+	pb_protocol_serialise_authenticate(&auth_msg, message->payload, len);
+
+	return client_write_message(server, client, message);
+}
+
+static int client_auth_timeout(void *arg)
+{
+	struct client *client = arg;
+	int rc;
+
+	client->can_modify = false;
+
+	rc = write_authenticate_message(client->server, client);
+	if (rc)
+		pb_log("failed to send client auth timeout\n");
+
+	return 0;
+}
+
+static int discover_server_handle_auth_message(struct client *client,
+		struct auth_message *auth_msg)
+{
+	struct status *status;
+	char *hash;
+	int rc;
+
+	status = talloc_zero(client, struct status);
+
+	switch (auth_msg->op) {
+	case AUTH_MSG_REQUEST:
+		if (!crypt_check_password(auth_msg->password)) {
+			rc = -1;
+			pb_log("Client failed to authenticate\n");
+			status->type = STATUS_ERROR;
+			status->message = talloc_asprintf(status,
+					_("Password incorrect"));
+		} else {
+			client->can_modify = true;
+			rc = write_authenticate_message(client->server,
+					client);
+			if (client->auth_waiter)
+				waiter_remove(client->auth_waiter);
+			client->auth_waiter = waiter_register_timeout(
+					client->server->waitset,
+					300000, /* 5 min */
+					client_auth_timeout, client);
+			pb_log("Client authenticated\n");
+			status->type = STATUS_INFO;
+			status->message = talloc_asprintf(status,
+					_("Authenticated successfully"));
+		}
+		break;
+	case AUTH_MSG_SET:
+		if (client->server->restrict_clients) {
+			if (!crypt_check_password(auth_msg->set_password.password)) {
+				rc = -1;
+				pb_log("Wrong password for set request\n");
+				status->type = STATUS_ERROR;
+				status->message = talloc_asprintf(status,
+						_("Password incorrect"));
+				break;
+			}
+		}
+
+		rc = crypt_set_password(auth_msg,
+				auth_msg->set_password.new_password);
+		if (rc) {
+			pb_log("Failed to set password\n");
+			status->type = STATUS_ERROR;
+			status->message = talloc_asprintf(status,
+					_("Error setting password"));
+		} else {
+			hash = crypt_get_hash(auth_msg);
+			platform_set_password(hash);
+			discover_server_set_auth_mode(client->server,
+				auth_msg->set_password.new_password != NULL);
+			pb_log("System password changed\n");
+			status->type = STATUS_ERROR;
+			status->message = talloc_asprintf(status,
+					_("Password updated successfully"));
+			talloc_free(hash);
+
+		}
+		break;
+	default:
+		pb_log("%s: unknown op\n", __func__);
+		rc = -1;
+		break;
+	}
+
+	write_boot_status_message(client->server, client, status);
+	talloc_free(status);
+
+	return rc;
+}
+
 static int discover_server_process_message(void *arg)
 {
 	struct autoboot_option *autoboot_opt;
 	struct pb_protocol_message *message;
 	struct boot_command *boot_command;
+	struct auth_message *auth_msg;
+	struct status *status;
 	struct client *client = arg;
 	struct config *config;
 	char *url;
@@ -262,6 +385,56 @@ static int discover_server_process_message(void *arg)
 		return 0;
 	}
 
+	/*
+	 * If crypt support is enabled, non-authorised clients can only delay
+	 * boot, not configure options or change the default boot option.
+	 */
+	if (!client->can_modify) {
+		switch (message->action) {
+		case PB_PROTOCOL_ACTION_BOOT:
+			boot_command = talloc(client, struct boot_command);
+
+			rc = pb_protocol_deserialise_boot_command(boot_command,
+					message);
+			if (rc) {
+				pb_log("%s: no boot command?", __func__);
+				return 0;
+			}
+
+			device_handler_boot(client->server->device_handler,
+					client->can_modify, boot_command);
+			break;
+		case PB_PROTOCOL_ACTION_CANCEL_DEFAULT:
+			device_handler_cancel_default(client->server->device_handler);
+			break;
+		case PB_PROTOCOL_ACTION_AUTHENTICATE:
+			auth_msg = talloc(client, struct auth_message);
+			rc = pb_protocol_deserialise_authenticate(
+					auth_msg, message);
+			if (rc) {
+				pb_log("Couldn't parse client's auth request\n");
+				break;
+			}
+
+			rc = discover_server_handle_auth_message(client,
+					auth_msg);
+			talloc_free(auth_msg);
+			break;
+		default:
+			pb_log("non-root client tried to perform action %d\n",
+					message->action);
+			status = talloc_zero(client, struct status);
+			if (status) {
+				status->type = STATUS_ERROR;
+				status->message = talloc_asprintf(status,
+						"Client must run as root to make changes");
+				write_boot_status_message(client->server, client,
+						status);
+				talloc_free(status);
+			}
+		}
+		return 0;
+	}
 
 	switch (message->action) {
 	case PB_PROTOCOL_ACTION_BOOT:
@@ -275,7 +448,7 @@ static int discover_server_process_message(void *arg)
 		}
 
 		device_handler_boot(client->server->device_handler,
-				boot_command);
+				client->can_modify, boot_command);
 		break;
 
 	case PB_PROTOCOL_ACTION_CANCEL_DEFAULT:
@@ -327,6 +500,19 @@ static int discover_server_process_message(void *arg)
 				autoboot_opt);
 		break;
 
+	/* For AUTH_MSG_SET */
+	case PB_PROTOCOL_ACTION_AUTHENTICATE:
+		auth_msg = talloc(client, struct auth_message);
+		rc = pb_protocol_deserialise_authenticate(
+				auth_msg, message);
+		if (rc) {
+			pb_log("Couldn't parse client's auth request\n");
+			break;
+		}
+
+		rc = discover_server_handle_auth_message(client, auth_msg);
+		talloc_free(auth_msg);
+		break;
 	default:
 		pb_log_fn("invalid action %d\n", message->action);
 		return 0;
@@ -336,12 +522,27 @@ static int discover_server_process_message(void *arg)
 	return 0;
 }
 
+void discover_server_set_auth_mode(struct discover_server *server,
+		bool restrict_clients)
+{
+	struct client *client;
+
+	server->restrict_clients = restrict_clients;
+
+	list_for_each_entry(&server->clients, client, list) {
+		client->can_modify = !restrict_clients;
+		write_authenticate_message(server, client);
+	}
+}
+
 static int discover_server_process_connection(void *arg)
 {
 	struct discover_server *server = arg;
 	struct statuslog_entry *entry;
 	int fd, rc, i, n_devices, n_plugins;
 	struct client *client;
+	struct ucred ucred;
+	socklen_t len;
 
 	/* accept the incoming connection */
 	fd = accept(server->socket, NULL, NULL);
@@ -362,6 +563,30 @@ static int discover_server_process_connection(void *arg)
 				WAIT_IN, discover_server_process_message,
 				client);
 
+	/*
+	 * get some info on the connecting process - if the client is being
+	 * run as root allow them to make changes
+	 */
+	if (server->restrict_clients) {
+		len = sizeof(struct ucred);
+		rc = getsockopt(client->fd, SOL_SOCKET, SO_PEERCRED, &ucred,
+				&len);
+		if (rc) {
+			pb_log("Failed to get socket info - restricting client\n");
+			client->can_modify = false;
+		} else {
+			pb_log("Client details: pid: %d, uid: %d, egid: %d\n",
+					ucred.pid, ucred.uid, ucred.gid);
+			client->can_modify = ucred.uid == 0;
+		}
+	} else
+		client->can_modify = true;
+
+	/* send auth status to client */
+	rc = write_authenticate_message(server, client);
+	if (rc)
+		return 0;
+
 	/* send sysinfo to client */
 	rc = write_system_info_message(server, client, system_info_get());
 	if (rc)
@@ -508,6 +733,7 @@ struct discover_server *discover_server_init(struct waitset *waitset)
 {
 	struct discover_server *server;
 	struct sockaddr_un addr;
+	struct group *group;
 
 	server = talloc(NULL, struct discover_server);
 	if (!server)
@@ -527,7 +753,6 @@ struct discover_server *discover_server_init(struct waitset *waitset)
 	}
 
 	talloc_set_destructor(server, server_destructor);
-
 	addr.sun_family = AF_UNIX;
 	strcpy(addr.sun_path, PB_SOCKET_PATH);
 
@@ -536,6 +761,13 @@ struct discover_server *discover_server_init(struct waitset *waitset)
 		goto out_err;
 	}
 
+	/* Allow all clients to communicate on this socket */
+	group = getgrnam("petitgroup");
+	if (group) {
+		chown(PB_SOCKET_PATH, 0, group->gr_gid);
+		chmod(PB_SOCKET_PATH, 0660);
+	}
+
 	if (listen(server->socket, 8)) {
 		pb_log("server socket listen: %s\n", strerror(errno));
 		goto out_err;
diff --git a/discover/discover-server.h b/discover/discover-server.h
index 9f3aa627..9722e173 100644
--- a/discover/discover-server.h
+++ b/discover/discover-server.h
@@ -20,6 +20,9 @@ void discover_server_destroy(struct discover_server *server);
 void discover_server_set_device_source(struct discover_server *server,
 		struct device_handler *handler);
 
+void discover_server_set_auth_mode(struct discover_server *server,
+		bool restrict_clients);
+
 void discover_server_notify_device_add(struct discover_server *server,
 		struct device *device);
 void discover_server_notify_boot_option_add(struct discover_server *server,
diff --git a/discover/pb-discover.c b/discover/pb-discover.c
index c494eeb3..e2b36dd4 100644
--- a/discover/pb-discover.c
+++ b/discover/pb-discover.c
@@ -189,6 +189,9 @@ int main(int argc, char *argv[])
 	if (config_get()->debug)
 		pb_log_set_debug(true);
 
+	if (platform_restrict_clients())
+		discover_server_set_auth_mode(server, true);
+
 	system_info_init(server);
 
 	handler = device_handler_init(server, waitset, opts.dry_run == opt_yes);
diff --git a/discover/platform.c b/discover/platform.c
index 237da3a9..7712ef14 100644
--- a/discover/platform.c
+++ b/discover/platform.c
@@ -213,6 +213,19 @@ int platform_get_sysinfo(struct system_info *info)
 	return -1;
 }
 
+bool platform_restrict_clients(){
+	if (platform && platform->restrict_clients)
+		return platform->restrict_clients(platform);
+	return false;
+}
+
+int platform_set_password(const char *hash)
+{
+	if (platform && platform->set_password)
+		return platform->set_password(platform, hash);
+	return -1;
+}
+
 int config_set(struct config *newconfig)
 {
 	int rc;
diff --git a/discover/platform.h b/discover/platform.h
index 29405626..f7d3d1c4 100644
--- a/discover/platform.h
+++ b/discover/platform.h
@@ -12,6 +12,8 @@ struct platform {
 	void		(*pre_boot)(struct platform *,
 				const struct config *);
 	int		(*get_sysinfo)(struct platform *, struct system_info *);
+	bool		(*restrict_clients)(struct platform *);
+	int		(*set_password)(struct platform *, const char *hash);
 	uint16_t	dhcp_arch_id;
 	void		*platform_data;
 };
@@ -20,6 +22,8 @@ int platform_init(void *ctx);
 int platform_fini(void);
 const struct platform *platform_get(void);
 int platform_get_sysinfo(struct system_info *info);
+bool platform_restrict_clients(void);
+int platform_set_password(const char *hash);
 void platform_pre_boot(void);
 
 /* configuration interface */
diff --git a/discover/user-event.c b/discover/user-event.c
index 734f77b3..d3d4a5e8 100644
--- a/discover/user-event.c
+++ b/discover/user-event.c
@@ -24,6 +24,7 @@
 #include <errno.h>
 #include <string.h>
 #include <sys/socket.h>
+#include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/un.h>
 
@@ -507,7 +508,7 @@ static int user_event_boot(struct user_event *uev, struct event *event)
 		cmd->boot_args = talloc_strdup(cmd, event_get_param(event, "args"));
 	}
 
-	device_handler_boot(handler, cmd);
+	device_handler_boot(handler, false, cmd);
 
 	talloc_free(cmd);
 
@@ -749,6 +750,10 @@ struct user_event *user_event_init(struct device_handler *handler,
 			strerror(errno));
 	}
 
+	/* Don't allow events from non-priviledged users */
+	chown(PBOOT_USER_EVENT_SOCKET, 0, 0);
+	chmod(PBOOT_USER_EVENT_SOCKET, 0660);
+
 	waiter_register_io(waitset, uev->socket, WAIT_IN,
 			user_event_process, uev);
 
-- 
2.19.1



More information about the Petitboot mailing list