GUACAMOLE-2281: Add generic req/done/cancel instructions to libguac. by aleitner · Pull Request #675 · apache/guacamole-server

aleitner · 2026-06-02T05:45:52Z

Adds a generic request/response RPC primitive to the Guacamole protocol in the form of three instructions: "req", "done", and "cancel".

A "req" instruction initiates a typed request with three arguments: a correlation id, a method name (by convention namespaced like "."), and an opaque payload. The receiving side is expected to settle the request with a "done" instruction carrying the same id, a status (typically "ok", "error", or "canceled"), and a response payload. Either side may issue a "cancel" with the matching id to abort an in-flight request before its "done" arrives.

The instructions themselves carry no feature-specific semantics; the method name argument identifies what kind of request is being made, and the payload is opaque to libguac. This leaves room for any RPC-style feature to share the same plumbing without needing its own dedicated opcodes.

Adds the libguac surface for the new primitive:

guac_protocol_send_req, guac_protocol_send_done, and guac_protocol_send_cancel in protocol.h / protocol.c.
guac_user_req_handler, guac_user_done_handler, and guac_user_cancel_handler typedefs in user-fntypes.h.
req_handler, done_handler, and cancel_handler callback fields on guac_user in user.h.
__guac_handle_req, __guac_handle_done, and __guac_handle_cancel dispatchers in user-handlers.c, registered in the instruction handler map.

The first intended consumer is WebAuthn passthrough, which will use this primitive to relay ceremonies between protocol plugins and the user's local authenticator with method names "webauthn.create" and "webauthn.get". The primitive is deliberately generic so other RPC-style features can adopt it without adding more opcodes to the protocol vocabulary.

Guacamole client PR

bbennett-ks · 2026-06-04T15:46:36Z

Maybe add a UT for the new framework? Similar to ./src/libguac/tests/socket/fd_send_instruction.c? May be worthwhile, since new framework code paths aren't exercise by the the Guacamole product.

bbennett-ks · 2026-06-04T16:24:02Z

Thinking through shared sessions: since send_req() broadcasts to all connected users, we would receive multiple "done" responses for a single request. Could we follow the existing guac_client_owner_send_required() pattern and target only the session owner instead of broadcasting?

This follows the existing RDP & VNC pattern of prompting only the session owner for authentication-related input.

bbennett-ks · 2026-06-05T01:12:47Z

I think this (and it's companion: apache/guacamole-client#1218) are actually drafts & not intended to be merged at this point. Maybe we should change the PR status?

necouchman · 2026-06-05T01:14:54Z

I think this (and it's companion: apache/guacamole-client#1218) are actually drafts & not intended to be merged at this point. Maybe we should change the PR status?

Okay, I've marked them as Draft for now.

aleitner · 2026-06-08T06:09:31Z

Added unit tests in the latest push

bbennett-ks

I looked over the existing primitives (msg, pipe, argv) to understand the need for a new RPC set, and this is a good extension: none of them offer a correlated request/response, whereas this provides:

Bidirectional request/response channel (either side can initiate).
Client-generated UUID correlation that stays unique across the multiple processes in the round-trip workflow (where a per-connection stream/object index couldn't).
Payload breakup & reassembly: the body streams over a paired pipe as blobs and is reassembled by libguac, allowing payloads larger than a single instruction can carry.
Cancellation support.

mike-jumper

I'm mainly not sure about the set of req/done/cancel instructions and use of pipe. The structure resembles the established "object" pattern that we already use for filesystem, so in this case I'd lean more toward an object-style service that can be given a name.

For example:

service declares a service object named webauthn.
Client retrieves the object's JSON index of available streams, which here would be create and get.
When the client wishes to issue a request, it creates a put stream to the relevant named stream of the object, including an opaque request ID in the path (ie: create/SOME_ID).
When the server needs to send the response for a request, it issues a body for the same path.

Would that align with what you're seeing with the structure of WebAuthN (and possibly plans for USB)?

aleitner · 2026-06-11T02:21:21Z

I'm mainly not sure about the set of req/done/cancel instructions and use of pipe. The structure resembles the established "object" pattern that we already use for filesystem, so in this case I'd lean more toward an object-style service that can be given a name.

For example:
1. `service` declares a service object named `webauthn`.

2. Client retrieves the object's JSON index of available streams, which here would be `create` and `get`.

3. When the client wishes to issue a request, it creates a `put` stream to the relevant named stream of the object, including an opaque request ID in the path (ie: `create/SOME_ID`).

4. When the server needs to send the response for a request, it issues a `body` for the same path.
Would that align with what you're seeing with the structure of WebAuthN (and possibly plans for USB)?

A couple clarifications before reshaping the PR.

WebAuthn passthrough is server initiated. The server forwards a challenge from upstream and the browser is the responder. The existing object pattern has get and put flowing client to server with body as the response back, and I couldn't find an example of a server-pushed body without a preceding client get. Would the shape you have in mind involve a server-issued put toward the client carrying the request, or is there an existing primitive for server-initiated push I'm missing

mike-jumper · 2026-06-11T03:38:28Z

A challenge/response flow could just be:

An auth-challenge stream, with the challenge having an arbitrary challenge ID distinct from the stream ID. The fact that the challenge is for consumption via WebAuthN could be embedded into the mimetype.
An auth-response stream, with the response pointing at the challenge ID.

I add the auth- prefixes here just because having a naked response opcode feels like we're commandeering a broad term for too specific a purpose.

aleitner · 2026-06-11T05:51:08Z

A challenge/response flow could just be:
1. An `auth-challenge` stream, with the challenge having an arbitrary challenge ID distinct from the stream ID. The fact that the challenge is for consumption via WebAuthN could be embedded into the mimetype.

2. An `auth-response` stream, with the response pointing at the challenge ID.
I add the auth- prefixes here just because having a naked response opcode feels like we're commandeering a broad term for too specific a purpose.

Okay so I am working on a redesign. Two new stream-opening instructions, auth-challenge and auth-response, each announcing a stream the body rides on directly. This way we have no paired pipe, reserved mimetype, or single-slot per-user state.

auth-challenge,<stream_idx>,<mimetype>,<challenge_id>
auth-response,<stream_idx>,<mimetype>,<challenge_id>

The mimetype identifies the ceremony kind. I went with application/x-webauthn-create+json and application/x-webauthn-get+json

mike-jumper · 2026-06-16T17:25:01Z

+     *     }
+     * @endcode
+     */
+    void (*destructor)(void* data);


The established pattern here is a free_handler, which would need its own stream-specific typedef (just like the other structs).

Renamed to free_handler and added a guac_stream_free_handler typedef in user-fntypes.h alongside the other stream handler typedefs

mike-jumper · 2026-06-16T17:31:44Z

+    /* Run the registered destructor for the stream's data, if any */
+    if (stream->destructor != NULL) {
+        stream->destructor(stream->data);
+        stream->destructor = NULL;
+        stream->data = NULL;
+    }


Good idea to NULL out stream->data here, but that should probably be done more broadly and regardless of whether a destructor is provided. For example:

if (stream->destructor != NULL) stream->destructor(stream->data); /* Clean up any remaining dangling pointers before permitting reuse of the * stream */ stream->data = NULL; stream->destructor = NULL; stream->another_handler = NULL; stream->yet_another_handler = NULL;

Moved the cleanup out of the if-block. data, ack_handler, blob_handler, end_handler, and free_handler are now all NULL'd unconditionally before the slot is released. Same fix in guac_client_free_stream

mike-jumper · 2026-06-16T17:32:51Z

+typedef struct auth_capture {
+    int call_count;
+    int stream_index;
+    char mimetype[64];
+    char challenge_id[64];
+} auth_capture;


Please document.

Added docs to each field

mike-jumper · 2026-06-16T17:33:34Z

+/**
+ * Handler that copies its arguments into the user's data pointer
+ * (assumed to be an auth_capture) so the test can assert them. Used for
+ * both the auth-challenge and auth-response paths since they share the
+ * same handler signature.
+ */
+static int capture_auth(guac_user* user, guac_stream* stream,
+        char* mimetype, char* challenge_id) {


We need to always document all parameters and return values.

Documented the params and return

mike-jumper · 2026-06-16T17:33:44Z

+
+}
+
+static void teardown_auth_user(guac_user* user) {


Please document this function.

Documented setup_auth_user and teardown_auth_user

mike-jumper · 2026-06-16T17:34:08Z

+ * Sender for the auth-response test: emits send_auth_response referencing
+ * an earlier challenge id.
+ */
+static void write_auth_response(int fd) {


Same here - we need to document all parameters.

Removed along with send_auth_response, nothing emits auth-response from the server. See the map entry for the removed auth-challenge

mike-jumper · 2026-06-16T17:35:16Z

   {"usbconnect",    __guac_handle_usbconnect},
   {"usbdata",       __guac_handle_usbdata},
   {"usbdisconnect", __guac_handle_usbdisconnect},
+   {"auth-challenge", __guac_handle_auth_challenge},


What are the semantics of receiving an auth challenge from the user?

Removed auth-challenge since nothing registers a handler. Also dropped send_auth_response, the typedef, and the field since the server only emits auth-challenge and only receives auth-response. Same on the JS side, dropped the auth-response handler, onauthresponse, and sendAuthChallenge. Can add the reverse direction back if anything else needs it

mike-jumper · 2026-06-16T17:36:06Z

+static void run_sender_check(void (*sender)(int), int write_fd, int read_fd,
+        const char* expected, int expected_len) {


Here, too, please - all parameters need to be documented.

Documented all params

mike-jumper · 2026-06-16T17:36:23Z

+ * heap-allocated buffer, returning the buffer and writing the length to
+ * *out_len. Caller takes ownership of the buffer. Closes the file descriptor.
+ */
+static char* read_all(int fd, int* out_len) {


And here - documentation needed for parameters and return values.

Documented the params and return

…libguac. Adds two new protocol instructions for authentication exchanges: - auth-challenge: opens a stream carrying the body of a challenge for a pending authentication exchange. The wire form is "auth-challenge,<stream_idx>,<mimetype>,<challenge_id>". The mimetype identifies the auth flavor; the challenge_id is an opaque identifier the peer references in its matching auth-response. - auth-response: opens a stream carrying the response body for a previously-issued auth-challenge identified by the same challenge_id. Same wire shape as auth-challenge. Each instruction's body rides as blobs on the announced stream, terminated by end. No paired pipe, no single-slot per-user buffering; per-stream blob and end handlers do the work. Also adds a destructor callback to guac_stream, called from guac_user_free_stream / guac_client_free_stream and from guac_user_free / guac_client_free for streams still open at disconnect, so consumers can attach per-stream state that needs cleanup if the peer drops mid-stream. The first consumer is WebAuthn passthrough (companion PR on guacamole-client) using application/x-webauthn-create+json and application/x-webauthn-get+json mimetypes.

aleitner mentioned this pull request Jun 2, 2026

GUACAMOLE-2281: Add WebAuthn passthrough relay over req/done/cancel. apache/guacamole-client#1218

Open

eugen-keeper reviewed Jun 2, 2026

View reviewed changes

Comment thread src/libguac/protocol.c Outdated

aleitner force-pushed the GUACAMOLE-2281-webauthn-passthrough branch 3 times, most recently from e5c7912 to 4eca0a8 Compare June 4, 2026 08:25

bbennett-ks suggested changes Jun 5, 2026

View reviewed changes

necouchman requested changes Jun 5, 2026

View reviewed changes

Comment thread src/libguac/user-handlers.c Outdated

Comment thread src/libguac/user-handlers.c Outdated

Comment thread src/libguac/user-handlers.c Outdated

necouchman marked this pull request as draft June 5, 2026 01:13

aleitner force-pushed the GUACAMOLE-2281-webauthn-passthrough branch 4 times, most recently from 8258dcc to 1688641 Compare June 8, 2026 06:07

aleitner marked this pull request as ready for review June 8, 2026 06:10

bbennett-ks approved these changes Jun 8, 2026

View reviewed changes

aleitner force-pushed the GUACAMOLE-2281-webauthn-passthrough branch 2 times, most recently from 5e30d6f to 6923c20 Compare June 9, 2026 03:07

mike-jumper reviewed Jun 10, 2026

View reviewed changes

aleitner force-pushed the GUACAMOLE-2281-webauthn-passthrough branch from 6923c20 to 9be1af5 Compare June 11, 2026 05:41

mike-jumper requested changes Jun 16, 2026

View reviewed changes

aleitner force-pushed the GUACAMOLE-2281-webauthn-passthrough branch from 9be1af5 to 3a1ec95 Compare June 17, 2026 03:59

aleitner force-pushed the GUACAMOLE-2281-webauthn-passthrough branch from 3a1ec95 to 64d2079 Compare June 17, 2026 04:16

		static void run_sender_check(void (*sender)(int), int write_fd, int read_fd,
		const char* expected, int expected_len) {

Conversation

aleitner commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

bbennett-ks commented Jun 4, 2026

Uh oh!

bbennett-ks commented Jun 4, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bbennett-ks commented Jun 5, 2026

Uh oh!

necouchman commented Jun 5, 2026

Uh oh!

aleitner commented Jun 8, 2026

Uh oh!

bbennett-ks left a comment

Choose a reason for hiding this comment

Uh oh!

mike-jumper left a comment

Choose a reason for hiding this comment

Uh oh!

aleitner commented Jun 11, 2026

Uh oh!

mike-jumper commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aleitner commented Jun 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

aleitner commented Jun 2, 2026 •

edited

Loading

mike-jumper commented Jun 11, 2026 •

edited

Loading