proto: add versioned Configure transactions

The same socket is used for pre-connection configuration by a portal and
for the actual client that then uses the data. The portal and the client
may need different protocol versions *and* there may be different REIS
intermediaries.

So let's allow version negotiation for the configuration through
transactions: a REIS intermediary must start/finish a transaction with a
given version number.

This is only partially implemented in libreis right now: each API call
is wrapped in a transaction. Since we support version 1 only anyway,
there's no need to do anything but send our version down the wire. In
the future where we actually need to negotiate, libreis will need a
reis_dispatch() so we can wait for the server version to arrive, parse
it, etc. before sending ConfigureVersion down the wire. It's likely this
will never be needed.

meson.build

@@ -51,6 +51,7 @@ config_h.set_quoted('EI_VERSION', meson.project_version())
 config_h.set_quoted('EIS_VERSION', meson.project_version())
 config_h.set('EI_PROTOCOL_VERSION', protocol_version)
 config_h.set('EIS_PROTOCOL_VERSION', protocol_version)
+config_h.set('REIS_PROTOCOL_VERSION', protocol_version)
 
 subdir('proto')
 

proto/ei.proto

@@ -63,6 +63,26 @@ message GetVersion {
   string ei = 1; /* always "EI" */
 }
 
+/* Start a transaction of one or more Configure* messages, using the given protocol version.
+ * This MUST be the first Configure message sent and the transaction MUST be
+ * concluded by a ConfigureFinish message.
+ *
+ * Multiple transactions are permitted.
+ */
+message ConfigureStart {
+  uint32 version = 1; /* Must be equal or less to the server's GetVersion response */
+}
+
+/* Signals the end of Configure transaction. Typically, this indicates that the
+ * connection will be passed to a diffent process (e.g. from a portal to the
+ * actual EI client).
+ *
+ * A new Configure transaction (with a different version, if applicable) may
+ * start after this transaction.
+ */
+message ConfigureFinish {
+}
+
 /* ConfigureName *must* be sent before the Connect event. Once a name is set,
  * subsequent attempts to change the name are ignored, including the name
  * provided in the Connect message.
@@ -212,8 +232,10 @@ message ClientMessage {
     Frame frame = 32;
 
     /* Pre-connection configuration */
-    ConfigureName configure_name = 101;
-    ConfigureCapabilities configure_capabilities = 102;
+    ConfigureStart configure_start = 100;
+    ConfigureFinish configure_finish = 101;
+    ConfigureName configure_name = 102;
+    ConfigureCapabilities configure_capabilities = 103;
   }
 }
 

src/libei-proto.c

@@ -259,6 +259,8 @@ log_wire_message(struct ei *ei, const ClientMessage *msg, int error)
 		MSG_STRING_CASE(TOUCH_MOTION);
 		MSG_STRING_CASE(TOUCH_UP);
 		MSG_STRING_CASE(FRAME);
+		MSG_STRING_CASE(CONFIGURE_START);
+		MSG_STRING_CASE(CONFIGURE_FINISH);
 		MSG_STRING_CASE(CONFIGURE_NAME);
 		MSG_STRING_CASE(CONFIGURE_CAPABILITIES);
 	}

src/libeis-client.c

@@ -278,6 +278,8 @@ eis_client_disconnect(struct eis_client *client)
 		eis_queue_disconnect_event(client);
 		_fallthrough_;
 	case EIS_CLIENT_STATE_NEW:
+	case EIS_CLIENT_STATE_CONFIGURING:
+	case EIS_CLIENT_STATE_CONFIGURED:
 		client_send_disconnect(client);
 		client->state = EIS_CLIENT_STATE_DISCONNECTED;
 		source_remove(client->source);
@@ -499,12 +501,83 @@ client_msg_touch_up(struct eis_client *client, uint32_t deviceid, uint32_t touch
 	return -EINVAL;
 }
 
+static tristate
+client_is_in_configure_transaction(struct eis_client *client)
+{
+	tristate t = tristate_connected;
+
+	switch (client->state) {
+	case EIS_CLIENT_STATE_NEW:
+	case EIS_CLIENT_STATE_CONFIGURED:
+		t = tristate_finished;
+		break;
+	case EIS_CLIENT_STATE_CONFIGURING:
+		t = tristate_started;
+		break;
+	case EIS_CLIENT_STATE_CONNECTING:
+	case EIS_CLIENT_STATE_CONNECTED:
+	case EIS_CLIENT_STATE_DISCONNECTED:
+		t = tristate_connected;
+		break;
+	}
+	return t;
+}
+
+static int
+client_msg_configure_start(struct eis_client *client, uint32_t version)
+{
+	tristate t = client_is_in_configure_transaction(client);
+
+	if (tristate_is_started(t))
+		return -EPROTO;
+
+	/* Once the client is connected, we silently ignore all Configure
+	   requests so a broken portal can't accidentally disconnect a client */
+	if (tristate_is_connected(t))
+		return 0;
+
+	if (version == 0)
+		return -EINVAL;
+
+	client->configure_version = version;
+	client->state = EIS_CLIENT_STATE_CONFIGURING;
+
+	return 0;
+}
+
+static int
+client_msg_configure_finish(struct eis_client *client)
+{
+	tristate t = client_is_in_configure_transaction(client);
+
+	if (tristate_is_finished(t))
+		return -EPROTO;
+
+	/* Once the client is connected, we silently ignore all Configure
+	   requests so a broken portal can't accidentally disconnect a client */
+	if (tristate_is_connected(t))
+		return 0;
+
+	client->configure_version = 0;
+	client->state = EIS_CLIENT_STATE_CONFIGURED;
+
+	return 0;
+}
 
 static int
 client_msg_configure_name(struct eis_client *client, const char *name)
 {
-	/* We silently ignore wrong configure messages */
-	if (client->state != EIS_CLIENT_STATE_NEW || client->name)
+	tristate t = client_is_in_configure_transaction(client);
+
+	if (tristate_is_finished(t))
+		return -EPROTO;
+
+	/* Once the client is connected, we silently ignore all Configure
+	   requests so a broken portal can't accidentally disconnect a client */
+	if (tristate_is_connected(t))
+		return 0;
+
+	if (client->name)
 		return 0;
 
 	client->name = xstrdup(name);
@@ -515,8 +588,14 @@ client_msg_configure_name(struct eis_client *client, const char *name)
 static int
 client_msg_configure_capabilities(struct eis_client *client, uint32_t allowed_caps)
 {
-	/* We silently ignore wrong configure messages */
-	if (client->state != EIS_CLIENT_STATE_NEW)
+	tristate t = client_is_in_configure_transaction(client);
+
+	if (tristate_is_finished(t))
+		return -EPROTO;
+
+	/* Once the client is connected, we silently ignore all Configure
+	   requests so a broken portal can't accidentally disconnect a client */
+	if (tristate_is_connected(t))
 		return 0;
 
 	/* restrictions can only be reduced */
@@ -591,6 +670,8 @@ static const struct eis_proto_interface intf_state_new = {
 	.disconnect = client_msg_disconnect,
 	.get_version = client_msg_get_version,
 
+	.configure_start = client_msg_configure_start,
+	.configure_finish = client_msg_configure_finish,
 	.configure_name = client_msg_configure_name,
 	.configure_capabilities = client_msg_configure_capabilities,
 };
@@ -600,6 +681,8 @@ static const struct eis_proto_interface intf_state_connecting = {
 	.disconnect = client_msg_disconnect,
 	.get_version = client_msg_get_version,
 
+	.configure_start = client_msg_configure_start,
+	.configure_finish = client_msg_configure_finish,
 	.configure_name = client_msg_configure_name,
 	.configure_capabilities = client_msg_configure_capabilities,
 };
@@ -627,12 +710,16 @@ static const struct eis_proto_interface intf_state_connected = {
 	.frame = client_msg_frame,
 
 	/* configuration */
+	.configure_start = client_msg_configure_start,
+	.configure_finish = client_msg_configure_finish,
 	.configure_name = client_msg_configure_name,
 	.configure_capabilities = client_msg_configure_capabilities,
 };
 
 static const struct eis_proto_interface *interfaces[] = {
 	[EIS_CLIENT_STATE_NEW] = &intf_state_new,
+	[EIS_CLIENT_STATE_CONFIGURING] = &intf_state_new,
+	[EIS_CLIENT_STATE_CONFIGURED] = &intf_state_new,
 	[EIS_CLIENT_STATE_CONNECTING] = &intf_state_connecting,
 	[EIS_CLIENT_STATE_CONNECTED] = &intf_state_connected,
 	[EIS_CLIENT_STATE_DISCONNECTED] = NULL,
@@ -663,6 +750,8 @@ client_dispatch(struct source *source, void *userdata)
 
 	static const char *client_states[] = {
 		"NEW",
+		"CONFIGURING",
+		"CONFIGURED",
 		"CONNECTING",
 		"CONNECTED",
 		"DISCONNECTED",
@@ -673,6 +762,8 @@ client_dispatch(struct source *source, void *userdata)
 		log_warn(eis_client_parent(client), "Client error: %s",
 			 strerror(-rc));
 	if (old_state != client->state) {
+		assert(old_state < ARRAY_LENGTH(client_states));
+		assert(client->state < ARRAY_LENGTH(client_states));
 		log_debug(eis_client_parent(client), "Client dispatch: %s -> %s",
 			  client_states[old_state],
 			  client_states[client->state]);

src/libeis-private.h

@@ -58,6 +58,8 @@ struct eis {
 
 enum eis_client_state {
 	EIS_CLIENT_STATE_NEW,		/* just connected */
+	EIS_CLIENT_STATE_CONFIGURING,   /* in a Configure transaction */
+	EIS_CLIENT_STATE_CONFIGURED,    /* finished a Configure transaction */
 	EIS_CLIENT_STATE_CONNECTING,	/* client requested connect */
 	EIS_CLIENT_STATE_CONNECTED,	/* server has sent connect */
 	EIS_CLIENT_STATE_DISCONNECTED,
@@ -69,6 +71,7 @@ struct eis_client {
 	struct list link;
 	struct source *source;
 	uint32_t version;
+	uint32_t configure_version;
 	uint32_t id;
 	enum eis_client_state state;
 	char *name;

src/libeis-proto.c

@@ -624,6 +624,13 @@ eis_proto_handle_message(struct eis_client *client,
 	case CLIENT_MESSAGE__MSG_FRAME:
 		rc = call(frame, client, proto->frame->deviceid, proto->frame->timestamp);
 		break;
+	case CLIENT_MESSAGE__MSG_CONFIGURE_START:
+		rc = call(configure_start, client,
+			  proto->configure_start->version);
+		break;
+	case CLIENT_MESSAGE__MSG_CONFIGURE_FINISH:
+		rc = call(configure_finish, client);
+		break;
 	case CLIENT_MESSAGE__MSG_CONFIGURE_NAME:
 		rc = call(configure_name, client,
 			  proto->configure_name->name);

src/libeis-proto.h

@@ -63,6 +63,8 @@ struct eis_proto_interface {
 	int (*frame) (struct eis_client *client, uint32_t deviceid, uint64_t time);
 
 	/* configuration */
+	int (*configure_start)(struct eis_client *client, uint32_t version);
+	int (*configure_finish)(struct eis_client *client);
 	int (*configure_name)(struct eis_client *client, const char *name);
 	int (*configure_capabilities)(struct eis_client *client, uint32_t allow);
 };

src/libreis.c

@@ -32,12 +32,16 @@
 #include "util-strings.h"
 #include "util-mem.h"
 #include "util-io.h"
+#include "util-version.h"
 
 #include "proto/ei.pb-c.h"
 
 struct reis {
 	struct object object;
 	int eisfd;
+
+	uint32_t version;
+	bool in_transaction;
 };
 
 static void
@@ -74,6 +78,16 @@ reis_new(int eisfd)
 	if (reis->eisfd == -1)
 		return NULL;
 
+	/* FIXME: technically we should check the server's version before we
+	 * send our own, but that requires adding reis_dispatch() and the code
+	 * in the caller for this too.
+	 *
+	 * Since all we support right now is version 1 and any server will
+	 * support that, so we don't need to care much.
+	 */
+
+	reis->version = VERSION_V(REIS_PROTOCOL_VERSION);
+
 	return steal(&reis);
 }
 
@@ -83,6 +97,33 @@ reis_new(int eisfd)
 	msg.msg_case = CLIENT_MESSAGE__MSG_##_type; \
 	msg._field = &_field
 
+static int
+reis_start(struct reis *reis)
+{
+	if (reis->in_transaction)
+		return 0;
+
+	reis->in_transaction = true;
+
+	prepare_msg(CONFIGURE_START, ConfigureStart, configure_start);
+	configure_start.version = reis->version;
+
+	return send_msg(reis->eisfd, &msg);
+}
+
+static int
+reis_finish(struct reis *reis)
+{
+	if (!reis->in_transaction)
+		return 0;
+
+	reis->in_transaction = false;
+
+	prepare_msg(CONFIGURE_FINISH, ConfigureFinish, configure_finish);
+
+	return send_msg(reis->eisfd, &msg);
+}
+
 _public_ int
 reis_set_property_with_permissions(struct reis *reis,
 				   const char *name, const char *value,
@@ -100,15 +141,29 @@ reis_set_property_with_permissions(struct reis *reis,
 _public_ int
 reis_set_name(struct reis *reis, const char *name)
 {
+	int rc;
+
+	if ((rc = reis_start(reis)) != 0)
+		return rc;
+
 	prepare_msg(CONFIGURE_NAME, ConfigureName, configure_name);
 	configure_name.name = (char*)name;
 
-	return send_msg(reis->eisfd, &msg);
+	rc = send_msg(reis->eisfd, &msg);
+	if (rc == 0)
+		return rc;
+
+	return reis_finish(reis);
 }
 
 _public_ int
 reis_allow_capability(struct reis *reis, enum reis_device_capability capability, ...)
 {
+	int rc;
+
+	if ((rc = reis_start(reis)) != 0)
+		return rc;
+
 	enum reis_device_capability cap = capability;
 	uint32_t caps = 0;
 	va_list args;
@@ -131,5 +186,9 @@ reis_allow_capability(struct reis *reis, enum reis_device_capability capability,
 	prepare_msg(CONFIGURE_CAPABILITIES, ConfigureCapabilities, configure_capabilities);
 	configure_capabilities.allowed_capabilities = caps;
 
-	return send_msg(reis->eisfd, &msg);
+	rc = send_msg(reis->eisfd, &msg);
+	if (rc)
+		return rc;
+
+	return reis_finish(reis);
 }