You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by "necouchman (via GitHub)" <gi...@apache.org> on 2026/05/04 01:35:57 UTC
[PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
necouchman opened a new pull request, #670:
URL: https://github.com/apache/guacamole-server/pull/670
This pull request implements the `AuthenticateEx` callback for FreeRDP, and handles versions of FreeRDP where the `Authenticate` callback has been deprecated and compiled out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3642327802
##########
src/protocols/rdp/rdp.c:
##########
@@ -231,6 +232,164 @@ static BOOL rdp_freerdp_pre_connect(freerdp* instance) {
return TRUE;
}
+#if defined(HAVE_FREERDP_AUTHENTICATE) || defined(HAVE_FREERDP_AUTHENTICATEEX)
+
+/**
+ * Provided a guac_client, a NULL-terminated list of parameters, and the
+ * pointers to the various authentication-related parameters, send the
+ * authentication parameter request on to the client, wait for the
+ * results back from the client, and then update the various settings
+ * and locations where that authentication information is stored.
+ *
+ * @param client
+ * The guac_client that is requesting the updated authentication
+ * information.
+ *
+ * @param params
+ * A NULL-terminated list of parameters that will be requested
+ * from the client.
+ *
+ * @param username
+ * A pointer to the memory location where the RDP client currently
+ * has stored the username.
+ *
+ * @param password
+ * A pointer to the memory location where the RDP client currently
+ * has stored the password.
+ *
+ * @param domain
+ * A pointer to the memory location where the RDP client currently
+ * has stored the domain.
+ */
+static void guac_rdp_send_auth_params(guac_client* client, const char** params,
+ char** username, char** password, char** domain) {
+
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+
+ /* Send required parameters to the owner and wait for the response. */
+ guac_client_owner_send_required(client, params);
+ guac_argv_await(params);
+
+ /* Free old values and get new values from settings. */
+ for (int i = 0; params[i] != NULL; i++) {
+
+ if (!strcmp(params[i], GUAC_RDP_ARGV_USERNAME)) {
+ guac_mem_free(*username);
+ *username = guac_strdup(settings->username);
+ }
+
+ if (!strcmp(params[i], GUAC_RDP_ARGV_DOMAIN)) {
+ guac_mem_free(*domain);
+ *domain = guac_strdup(settings->domain);
+ }
+
+ if (!strcmp(params[i], GUAC_RDP_ARGV_PASSWORD)) {
+ guac_mem_free(*password);
+ *password = guac_strdup(settings->password);
+ }
+ }
+
+}
+
+/**
+ * Given the RDP settings for the connection, check the authentication
+ * settings and add the required parameters to the params array and then
+ * return the number of parameters that have been added to the params
+ * array after NULL-terminating the array.
+ *
+ * @param settings
+ * The RDP settings for the connection.
+ *
+ * @param params
+ * The array that will store the authentication items that should
+ * be sent to the client.
+ *
+ * @return
+ * The number of items added to the params array.
+ */
+static int guac_rdp_register_required_auth(guac_rdp_settings* settings, char** params) {
+
+ int i = 0;
+
+ /* If the username is undefined, add it to the requested parameters. */
+ if (settings->username == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_USERNAME;
+ i++;
+
+ /* If username is undefined and domain is also undefined, request domain. */
+ if (settings->domain == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_DOMAIN;
+ i++;
+ }
+
+ }
+
+ /* If the password is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+ }
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+
+ return i;
+
+}
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * A function called when FreeRDP receives a request for gateway-related
+ * authentication settings, returning the number of settings that will be
+ * requested from the client.
+ *
+ * @param settings
+ * The settings member from the RDP client.
+ *
+ * @param params
+ * The structure used to store the parameters.
+ *
+ * @return
+ * The number of settings that will be requested from the client.
+ */
+static int guac_rdp_register_required_gateway_auth(guac_rdp_settings* settings, char** params) {
+
+ int i = 0;
+
+ /* If the gateway username is null, we'll request that. */
+ if (settings->gateway_username == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_GATEWAY_USERNAME, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_GATEWAY_USERNAME;
+ i++;
+
+ /* If the gateway domain is null, we'll also request that. */
+ if (settings->gateway_domain == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_GATEWAY_DOMAIN, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_GATEWAY_DOMAIN;
+ i++;
+ }
+ }
+
+ /* If the gateway password is null, request that. */
+ if (settings->gateway_password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_GATEWAY_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_GATEWAY_PASSWORD;
+ i++;
+ }
Review Comment:
Updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3611322181
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +386,113 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
Review Comment:
The `password` and `domain` parameters here are swapped vs. the `guac_rdp_send_auth_params()` function definition.
Rather than just correcting the call, though, I'd suggest that the function either use the same ordering as the connection parameters, or the same ordering as the FreeRDP callback.
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +386,113 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
+ }
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+ case AUTH_SMARTCARD_PIN:
+ case AUTH_FIDO_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+ case AUTH_RDSTLS:
+ case AUTH_TLS:
+ case AUTH_RDP:
+ case AUTH_NLA:
+ case GW_AUTH_HTTP:
+ case GW_AUTH_RDG:
+ case GW_AUTH_RPC:
+ i = guac_rdp_register_required_auth(settings, params);
+ break;
+
+ /* We should never get here. */
+ default:
+ return TRUE;
+
}
+
+ /* If one or more params have been required, send them. */
+ if (i > 0)
+ guac_rdp_auth_send_params(client, params, username, password, domain);
Review Comment:
This call doesn't match the function's defined name: `guac_rdp_send_auth_params()`
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +386,113 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
+ }
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+ case AUTH_SMARTCARD_PIN:
+ case AUTH_FIDO_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+ case AUTH_RDSTLS:
+ case AUTH_TLS:
+ case AUTH_RDP:
+ case AUTH_NLA:
+ case GW_AUTH_HTTP:
+ case GW_AUTH_RDG:
+ case GW_AUTH_RPC:
+ i = guac_rdp_register_required_auth(settings, params);
Review Comment:
As written, `guac_rdp_register_required_auth()` only deals with the main username/password/domain parameters, not the gateway-specific variations of these. If `GW_AUTH_HTTP`, etc. are actually encountered, the user will be prompted, but the user's responses will populate the wrong settings, the gateway-specific credentials will remain empty, and auth will fail.
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +386,113 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
Review Comment:
Please document the `reason` parameter.
##########
configure.ac:
##########
@@ -1010,6 +1010,22 @@ then
[[#include <freerdp/freerdp.h>]])
fi
+# Check for either AuthenticateEx or Authenticate callbacks.
+if test "x${have_freerdp}" = "xyes"
+then
+ AC_CHECK_MEMBERS([freerdp.AuthenticateEx],,
+ [AC_CHECK_MEMBERS([freerdp.Authenticate],,
+ [AC_MSG_WARN([
+ ----------------------------------------------------
+ This version of FreeRDP appears to lack support
+ for both Authenticate and AuthenticateEx functions.
+ RDP connections may fail if credentials are not
+ provided with the connection.
+ ----------------------------------------------------])],
+ [[#include <freerdp/freerdp.h>]])],
+ [[#include <freerdp/freerdp.h>]])
Review Comment:
With `-Werror`, this check will fail upon merely including `freerdp/freerdp.h` for recent FreeRDP 3.x, so this may erroneously report "no".
##########
configure.ac:
##########
@@ -1010,6 +1010,22 @@ then
[[#include <freerdp/freerdp.h>]])
fi
+# Check for either AuthenticateEx or Authenticate callbacks.
+if test "x${have_freerdp}" = "xyes"
+then
+ AC_CHECK_MEMBERS([freerdp.AuthenticateEx],,
+ [AC_CHECK_MEMBERS([freerdp.Authenticate],,
+ [AC_MSG_WARN([
+ ----------------------------------------------------
+ This version of FreeRDP appears to lack support
+ for both Authenticate and AuthenticateEx functions.
+ RDP connections may fail if credentials are not
+ provided with the connection.
Review Comment:
Things will currently fail to build with a warning in the event that neither of these callbacks are available, since the `static` utility functions that they use remain defined but no longer have any callers.
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +386,113 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
+ }
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+ case AUTH_SMARTCARD_PIN:
+ case AUTH_FIDO_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+ case AUTH_RDSTLS:
+ case AUTH_TLS:
+ case AUTH_RDP:
+ case AUTH_NLA:
+ case GW_AUTH_HTTP:
+ case GW_AUTH_RDG:
+ case GW_AUTH_RPC:
+ i = guac_rdp_register_required_auth(settings, params);
+ break;
+
+ /* We should never get here. */
+ default:
+ return TRUE;
Review Comment:
We should either log a warning here (so the underlying issue is visible) or remove the `default` (so that the compiler will yell at us for any missing cases).
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +386,113 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
+ }
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+ case AUTH_SMARTCARD_PIN:
+ case AUTH_FIDO_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+ case AUTH_RDSTLS:
+ case AUTH_TLS:
+ case AUTH_RDP:
+ case AUTH_NLA:
+ case GW_AUTH_HTTP:
+ case GW_AUTH_RDG:
+ case GW_AUTH_RPC:
+ i = guac_rdp_register_required_auth(settings, params);
+ break;
+
+ /* We should never get here. */
+ default:
+ return TRUE;
Review Comment:
At least `AUTH_FIDO_PIN` and `AUTH_RDSTLS` were not defined until more recent versions of FreeRDP 3.x, while the `AuthenticateEx` callback has existed prior to that. If an older version of FreeRDP 3.x is used, this may fail to build.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3642261835
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +437,124 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @param reason
+ * This enum value that tracks why authentication is being
+ * requested, which informs what types of values the authenticate function
+ * will request.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
}
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+#if (FREERDP_VERSION_MAJOR >= 3 && FREERDP_VERSION_MINOR >= 25)
+ /* FIDO PIN support was added in 3.25.0 */
+ case AUTH_FIDO_PIN:
+#endif
+ case AUTH_SMARTCARD_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+#if (FREERDP_VERSION_MAJOR >= 3 && FREERDP_VERSION_MINOR >= 18)
+ /* RDSTLS support was added in 3.18.0 */
+ case AUTH_RDSTLS:
+#endif
+ case AUTH_TLS:
+ case AUTH_RDP:
+ case AUTH_NLA:
+ i = guac_rdp_register_required_auth(settings, params);
+ break;
+
+ case GW_AUTH_HTTP:
+ case GW_AUTH_RDG:
+ case GW_AUTH_RPC:
+ i = guac_rdp_register_required_gateway_auth(settings, params);
+ break;
Review Comment:
I think there are a couple of situations where it would be called multiple times:
* If Gateway authentication is requested, it could be call for both Gateway and then Server authentication. That would be twice.
* If authentication fails, the connection would retry, but I don't think this necessarily qualifies as being called multiple times, since the guacd connection would close and then restart when authentication fails.
Is there some way that multiple calls should be dealt with?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#issuecomment-4464956226
@mike-jumper @corentin-soriano @sschiffli Anyone do a quick review of this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3258465129
##########
src/protocols/rdp/rdp.c:
##########
@@ -326,6 +327,137 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication used to
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = {NULL};
+ int i = 0;
+
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
+ }
Review Comment:
I'll take a look and see what might make sense. It's a little tricky because the newer function is broken up by the `switch` statement trying to check for the type of authentication that is being requested, but there are still probably a couple of items that could become `static` functions and shared by both versions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3658439371
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +453,126 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
-
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @param reason
+ * This enum value that tracks why authentication is being
+ * requested, which informs what types of values the authenticate function
+ * will request.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
}
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+#if (FREERDP_VERSION_MAJOR >= 4) || \
+ (FREERDP_VERSION_MAJOR == 3 && FREERDP_VERSION_MINOR >= 25)
+ /* FIDO PIN support was added in 3.25.0 */
+ case AUTH_FIDO_PIN:
+#endif
+ case AUTH_SMARTCARD_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+#if (FREERDP_VERSION_MAJOR >= 4) || \
+ (FREERDP_VERSION_MAJOR == 3 && FREERDP_VERSION_MINOR >= 18)
+ /* RDSTLS support was added in 3.18.0 */
+ case AUTH_RDSTLS:
Review Comment:
Oops...fixed via rebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3616933839
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +386,113 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
+ }
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+ case AUTH_SMARTCARD_PIN:
+ case AUTH_FIDO_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+ case AUTH_RDSTLS:
+ case AUTH_TLS:
+ case AUTH_RDP:
+ case AUTH_NLA:
+ case GW_AUTH_HTTP:
+ case GW_AUTH_RDG:
+ case GW_AUTH_RPC:
+ i = guac_rdp_register_required_auth(settings, params);
+ break;
+
+ /* We should never get here. */
+ default:
+ return TRUE;
+
}
+
+ /* If one or more params have been required, send them. */
+ if (i > 0)
+ guac_rdp_auth_send_params(client, params, username, password, domain);
Review Comment:
Corrected via rebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3616940951
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +386,113 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
Review Comment:
Documented.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3617779038
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +386,113 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
+ }
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+ case AUTH_SMARTCARD_PIN:
+ case AUTH_FIDO_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+ case AUTH_RDSTLS:
+ case AUTH_TLS:
+ case AUTH_RDP:
+ case AUTH_NLA:
+ case GW_AUTH_HTTP:
+ case GW_AUTH_RDG:
+ case GW_AUTH_RPC:
+ i = guac_rdp_register_required_auth(settings, params);
+ break;
+
+ /* We should never get here. */
+ default:
+ return TRUE;
Review Comment:
`FREERDP_VERSION` expects just the major number (2, 3, etc.) - to completely override with a specific tag or commit, you'll need to use `WITH_FREERDP`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3658345589
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +453,126 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
-
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @param reason
+ * This enum value that tracks why authentication is being
+ * requested, which informs what types of values the authenticate function
+ * will request.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
}
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+#if (FREERDP_VERSION_MAJOR >= 4) || \
+ (FREERDP_VERSION_MAJOR == 3 && FREERDP_VERSION_MINOR >= 25)
+ /* FIDO PIN support was added in 3.25.0 */
+ case AUTH_FIDO_PIN:
+#endif
+ case AUTH_SMARTCARD_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+#if (FREERDP_VERSION_MAJOR >= 4) || \
+ (FREERDP_VERSION_MAJOR == 3 && FREERDP_VERSION_MINOR >= 18)
+ /* RDSTLS support was added in 3.18.0 */
+ case AUTH_RDSTLS:
Review Comment:
Beware that several of the comments here have become unindented by one level.
##########
src/protocols/rdp/rdp.c:
##########
@@ -231,6 +232,180 @@ static BOOL rdp_freerdp_pre_connect(freerdp* instance) {
return TRUE;
}
+#if defined(HAVE_FREERDP_AUTHENTICATE) || defined(HAVE_FREERDP_AUTHENTICATEEX)
+
+/**
+ * Provided a guac_client, a NULL-terminated list of parameters, and the
+ * pointers to the various authentication-related parameters, send the
+ * authentication parameter request on to the client, wait for the
+ * results back from the client, and then update the various settings
+ * and locations where that authentication information is stored.
+ *
+ * @param client
+ * The guac_client that is requesting the updated authentication
+ * information.
+ *
+ * @param params
+ * A NULL-terminated list of parameters that will be requested
+ * from the client.
+ *
+ * @param username
+ * A pointer to the memory location where the RDP client currently
+ * has stored the username.
+ *
+ * @param password
+ * A pointer to the memory location where the RDP client currently
+ * has stored the password.
+ *
+ * @param domain
+ * A pointer to the memory location where the RDP client currently
+ * has stored the domain.
+ */
+static void guac_rdp_send_auth_params(guac_client* client, const char** params,
+ char** username, char** password, char** domain) {
+
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+
+ /* Send required parameters to the owner and wait for the response. */
+ guac_client_owner_send_required(client, params);
+ guac_argv_await(params);
+
+ /* Free old values and get new values from settings. */
+ for (int i = 0; params[i] != NULL; i++) {
+
+ if (strcmp(params[i], GUAC_RDP_ARGV_USERNAME) == 0) {
+ guac_mem_free(*username);
+ *username = guac_strdup(settings->username);
+ }
+
+ if (strcmp(params[i], GUAC_RDP_ARGV_GATEWAY_USERNAME) == 0) {
+ guac_mem_free(*username);
+ *username = guac_strdup(settings->gateway_username);
+ }
+
+ if (strcmp(params[i], GUAC_RDP_ARGV_DOMAIN) == 0) {
+ guac_mem_free(*domain);
+ *domain = guac_strdup(settings->domain);
+ }
+
+ if (strcmp(params[i], GUAC_RDP_ARGV_GATEWAY_DOMAIN) == 0) {
+ guac_mem_free(*domain);
+ *domain = guac_strdup(settings->gateway_domain);
+ }
+
+ if (strcmp(params[i], GUAC_RDP_ARGV_PASSWORD) == 0) {
+ guac_mem_free(*password);
+ *password = guac_strdup(settings->password);
+ }
+
+ if (strcmp(params[i], GUAC_RDP_ARGV_GATEWAY_PASSWORD) == 0) {
+ guac_mem_free(*password);
+ *password = guac_strdup(settings->gateway_password);
+ }
Review Comment:
These should be `else if` to avoid scanning the `params[i]` string repeatedly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3616939746
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +386,113 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
+ }
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+ case AUTH_SMARTCARD_PIN:
+ case AUTH_FIDO_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+ case AUTH_RDSTLS:
+ case AUTH_TLS:
+ case AUTH_RDP:
+ case AUTH_NLA:
+ case GW_AUTH_HTTP:
+ case GW_AUTH_RDG:
+ case GW_AUTH_RPC:
+ i = guac_rdp_register_required_auth(settings, params);
+ break;
+
+ /* We should never get here. */
+ default:
+ return TRUE;
Review Comment:
Removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3618371714
##########
configure.ac:
##########
@@ -1010,6 +1010,22 @@ then
[[#include <freerdp/freerdp.h>]])
fi
+# Check for either AuthenticateEx or Authenticate callbacks.
+if test "x${have_freerdp}" = "xyes"
+then
+ AC_CHECK_MEMBERS([freerdp.AuthenticateEx],,
+ [AC_CHECK_MEMBERS([freerdp.Authenticate],,
+ [AC_MSG_WARN([
+ ----------------------------------------------------
+ This version of FreeRDP appears to lack support
+ for both Authenticate and AuthenticateEx functions.
+ RDP connections may fail if credentials are not
+ provided with the connection.
Review Comment:
Okay, I've added another `#if` around these functions, too.
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +386,113 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
+ }
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+ case AUTH_SMARTCARD_PIN:
+ case AUTH_FIDO_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+ case AUTH_RDSTLS:
+ case AUTH_TLS:
+ case AUTH_RDP:
+ case AUTH_NLA:
+ case GW_AUTH_HTTP:
+ case GW_AUTH_RDG:
+ case GW_AUTH_RPC:
+ i = guac_rdp_register_required_auth(settings, params);
+ break;
+
+ /* We should never get here. */
+ default:
+ return TRUE;
Review Comment:
Aha, got it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3618375491
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +386,113 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
+ }
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+ case AUTH_SMARTCARD_PIN:
+ case AUTH_FIDO_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+ case AUTH_RDSTLS:
+ case AUTH_TLS:
+ case AUTH_RDP:
+ case AUTH_NLA:
+ case GW_AUTH_HTTP:
+ case GW_AUTH_RDG:
+ case GW_AUTH_RPC:
+ i = guac_rdp_register_required_auth(settings, params);
+ break;
+
+ /* We should never get here. */
+ default:
+ return TRUE;
Review Comment:
I've implemented version checks to cover this based on the FreeRDP versions in which these options showed up. Couldn't figure out a good way to check `enum` members with Autoconf.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3642332119
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +437,124 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @param reason
+ * This enum value that tracks why authentication is being
+ * requested, which informs what types of values the authenticate function
+ * will request.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
}
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+#if (FREERDP_VERSION_MAJOR >= 3 && FREERDP_VERSION_MINOR >= 25)
Review Comment:
Okay, I've adjusted this to be similar to the checks in `gdi.c` that break out the major versions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3589438114
##########
src/protocols/rdp/rdp.c:
##########
@@ -326,6 +327,137 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication used to
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = {NULL};
+ int i = 0;
+
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
+ }
Review Comment:
Okay, I've done some combining of the authentication items in the RDP code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3616932531
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +386,113 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
Review Comment:
I've corrected by correcting the function definition so that it matches the callback ordering.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3624532408
##########
configure.ac:
##########
@@ -1010,6 +1010,22 @@ then
[[#include <freerdp/freerdp.h>]])
fi
+# Check for either AuthenticateEx or Authenticate callbacks.
+if test "x${have_freerdp}" = "xyes"
+then
+ AC_CHECK_MEMBERS([freerdp.AuthenticateEx],,
+ [AC_CHECK_MEMBERS([freerdp.Authenticate],,
+ [AC_MSG_WARN([
+ ----------------------------------------------------
+ This version of FreeRDP appears to lack support
+ for both Authenticate and AuthenticateEx functions.
+ RDP connections may fail if credentials are not
+ provided with the connection.
+ ----------------------------------------------------])],
+ [[#include <freerdp/freerdp.h>]])],
+ [[#include <freerdp/freerdp.h>]])
Review Comment:
Yeah, same - I think we're safe to assume I'm just wrong here until we encounter a real-world failure to detect, if ever.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3591449253
##########
src/protocols/rdp/rdp.c:
##########
@@ -549,11 +718,20 @@ static int guac_rdp_handle_connection(guac_client* client) {
* If the freerdp instance has a LoadChannels callback for loading plugins
* we use that instead of the PreConnect callback to load plugins.
*/
- #ifdef RDP_INST_HAS_LOAD_CHANNELS
+#ifdef RDP_INST_HAS_LOAD_CHANNELS
rdp_inst->LoadChannels = rdp_freerdp_load_channels;
- #endif
+#endif
+
rdp_inst->PreConnect = rdp_freerdp_pre_connect;
+
+#ifdef HAVE_FREERDP_AUTHENTICATE
rdp_inst->Authenticate = rdp_freerdp_authenticate;
+#endif
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+ rdp_inst->AuthenticateEx = rdp_freerdp_authenticate_ex;
+#endif
Review Comment:
If `AuthenticateEx` is the replacement for now-deprecated `Authenticate`, should `Authenticate` not be used at all when `AuthenticateEx` is available?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3642325428
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +437,124 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @param reason
+ * This enum value that tracks why authentication is being
+ * requested, which informs what types of values the authenticate function
+ * will request.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
}
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+#if (FREERDP_VERSION_MAJOR >= 3 && FREERDP_VERSION_MINOR >= 25)
+ /* FIDO PIN support was added in 3.25.0 */
+ case AUTH_FIDO_PIN:
+#endif
+ case AUTH_SMARTCARD_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+#if (FREERDP_VERSION_MAJOR >= 3 && FREERDP_VERSION_MINOR >= 18)
+ /* RDSTLS support was added in 3.18.0 */
Review Comment:
Oops. Removed.
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +437,124 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @param reason
+ * This enum value that tracks why authentication is being
+ * requested, which informs what types of values the authenticate function
+ * will request.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
}
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+#if (FREERDP_VERSION_MAJOR >= 3 && FREERDP_VERSION_MINOR >= 25)
+ /* FIDO PIN support was added in 3.25.0 */
Review Comment:
Removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3589456440
##########
src/protocols/rdp/rdp.c:
##########
@@ -326,6 +327,137 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication used to
Review Comment:
Oops. Should be fixed via rebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3617744503
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +386,113 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
+ }
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+ case AUTH_SMARTCARD_PIN:
+ case AUTH_FIDO_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+ case AUTH_RDSTLS:
+ case AUTH_TLS:
+ case AUTH_RDP:
+ case AUTH_NLA:
+ case GW_AUTH_HTTP:
+ case GW_AUTH_RDG:
+ case GW_AUTH_RPC:
+ i = guac_rdp_register_required_auth(settings, params);
+ break;
+
+ /* We should never get here. */
+ default:
+ return TRUE;
Review Comment:
@mike-jumper I'm trying to use the `FREERDP_VERSION=3.0.0` in the `Dockerfile` to build against a specific version of FreeRDP, but getting the following error:
```
Cloning into 'FreeRDP'...
Updating files: 100% (2962/2962), done.
Build architecture: X86
Build parallelism: 4 job(s)
Building FreeRDP @ 3.0.0(\.\d+)+ ...
error: pathspec '3.0.0(\.\d+)+' did not match any file(s) known to git
```
Am I doing something wrong?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3624540346
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +437,124 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @param reason
+ * This enum value that tracks why authentication is being
+ * requested, which informs what types of values the authenticate function
+ * will request.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
}
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+#if (FREERDP_VERSION_MAJOR >= 3 && FREERDP_VERSION_MINOR >= 25)
+ /* FIDO PIN support was added in 3.25.0 */
Review Comment:
Sneaky tab here, too.
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +437,124 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @param reason
+ * This enum value that tracks why authentication is being
+ * requested, which informs what types of values the authenticate function
+ * will request.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
}
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+#if (FREERDP_VERSION_MAJOR >= 3 && FREERDP_VERSION_MINOR >= 25)
+ /* FIDO PIN support was added in 3.25.0 */
+ case AUTH_FIDO_PIN:
+#endif
+ case AUTH_SMARTCARD_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+#if (FREERDP_VERSION_MAJOR >= 3 && FREERDP_VERSION_MINOR >= 18)
+ /* RDSTLS support was added in 3.18.0 */
Review Comment:
Tab snuck in here.
##########
src/protocols/rdp/rdp.c:
##########
@@ -231,6 +232,164 @@ static BOOL rdp_freerdp_pre_connect(freerdp* instance) {
return TRUE;
}
+#if defined(HAVE_FREERDP_AUTHENTICATE) || defined(HAVE_FREERDP_AUTHENTICATEEX)
+
+/**
+ * Provided a guac_client, a NULL-terminated list of parameters, and the
+ * pointers to the various authentication-related parameters, send the
+ * authentication parameter request on to the client, wait for the
+ * results back from the client, and then update the various settings
+ * and locations where that authentication information is stored.
+ *
+ * @param client
+ * The guac_client that is requesting the updated authentication
+ * information.
+ *
+ * @param params
+ * A NULL-terminated list of parameters that will be requested
+ * from the client.
+ *
+ * @param username
+ * A pointer to the memory location where the RDP client currently
+ * has stored the username.
+ *
+ * @param password
+ * A pointer to the memory location where the RDP client currently
+ * has stored the password.
+ *
+ * @param domain
+ * A pointer to the memory location where the RDP client currently
+ * has stored the domain.
+ */
+static void guac_rdp_send_auth_params(guac_client* client, const char** params,
Review Comment:
Since this never handles the gateway parameters, a gateway reason in the authenticate callback won't result in a gateway prompt.
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +437,124 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @param reason
+ * This enum value that tracks why authentication is being
+ * requested, which informs what types of values the authenticate function
+ * will request.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
}
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+#if (FREERDP_VERSION_MAJOR >= 3 && FREERDP_VERSION_MINOR >= 25)
Review Comment:
This won't be quite right for FreeRDP 4.x (it will match >= 3.25 for 3.x, >= 4.25 for 4.x, etc.).
I think we can check enum constants using `AC_CHECK_DECL`.
##########
src/protocols/rdp/rdp.c:
##########
@@ -231,6 +232,164 @@ static BOOL rdp_freerdp_pre_connect(freerdp* instance) {
return TRUE;
}
+#if defined(HAVE_FREERDP_AUTHENTICATE) || defined(HAVE_FREERDP_AUTHENTICATEEX)
+
+/**
+ * Provided a guac_client, a NULL-terminated list of parameters, and the
+ * pointers to the various authentication-related parameters, send the
+ * authentication parameter request on to the client, wait for the
+ * results back from the client, and then update the various settings
+ * and locations where that authentication information is stored.
+ *
+ * @param client
+ * The guac_client that is requesting the updated authentication
+ * information.
+ *
+ * @param params
+ * A NULL-terminated list of parameters that will be requested
+ * from the client.
+ *
+ * @param username
+ * A pointer to the memory location where the RDP client currently
+ * has stored the username.
+ *
+ * @param password
+ * A pointer to the memory location where the RDP client currently
+ * has stored the password.
+ *
+ * @param domain
+ * A pointer to the memory location where the RDP client currently
+ * has stored the domain.
+ */
+static void guac_rdp_send_auth_params(guac_client* client, const char** params,
+ char** username, char** password, char** domain) {
+
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+
+ /* Send required parameters to the owner and wait for the response. */
+ guac_client_owner_send_required(client, params);
+ guac_argv_await(params);
+
+ /* Free old values and get new values from settings. */
+ for (int i = 0; params[i] != NULL; i++) {
+
+ if (!strcmp(params[i], GUAC_RDP_ARGV_USERNAME)) {
+ guac_mem_free(*username);
+ *username = guac_strdup(settings->username);
+ }
+
+ if (!strcmp(params[i], GUAC_RDP_ARGV_DOMAIN)) {
+ guac_mem_free(*domain);
+ *domain = guac_strdup(settings->domain);
+ }
+
+ if (!strcmp(params[i], GUAC_RDP_ARGV_PASSWORD)) {
+ guac_mem_free(*password);
+ *password = guac_strdup(settings->password);
+ }
+ }
+
+}
+
+/**
+ * Given the RDP settings for the connection, check the authentication
+ * settings and add the required parameters to the params array and then
+ * return the number of parameters that have been added to the params
+ * array after NULL-terminating the array.
+ *
+ * @param settings
+ * The RDP settings for the connection.
+ *
+ * @param params
+ * The array that will store the authentication items that should
+ * be sent to the client.
+ *
+ * @return
+ * The number of items added to the params array.
+ */
+static int guac_rdp_register_required_auth(guac_rdp_settings* settings, char** params) {
+
+ int i = 0;
+
+ /* If the username is undefined, add it to the requested parameters. */
+ if (settings->username == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_USERNAME;
+ i++;
+
+ /* If username is undefined and domain is also undefined, request domain. */
+ if (settings->domain == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_DOMAIN;
+ i++;
+ }
+
+ }
+
+ /* If the password is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+ }
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+
+ return i;
+
+}
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * A function called when FreeRDP receives a request for gateway-related
+ * authentication settings, returning the number of settings that will be
+ * requested from the client.
+ *
+ * @param settings
+ * The settings member from the RDP client.
+ *
+ * @param params
+ * The structure used to store the parameters.
+ *
+ * @return
+ * The number of settings that will be requested from the client.
+ */
+static int guac_rdp_register_required_gateway_auth(guac_rdp_settings* settings, char** params) {
+
+ int i = 0;
+
+ /* If the gateway username is null, we'll request that. */
+ if (settings->gateway_username == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_GATEWAY_USERNAME, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_GATEWAY_USERNAME;
+ i++;
+
+ /* If the gateway domain is null, we'll also request that. */
+ if (settings->gateway_domain == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_GATEWAY_DOMAIN, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_GATEWAY_DOMAIN;
+ i++;
+ }
+ }
+
+ /* If the gateway password is null, request that. */
+ if (settings->gateway_password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_GATEWAY_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_GATEWAY_PASSWORD;
+ i++;
+ }
Review Comment:
The `argv` callback will also need to be updated for these parameters to be handled when received.
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +437,124 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @param reason
+ * This enum value that tracks why authentication is being
+ * requested, which informs what types of values the authenticate function
+ * will request.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
}
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+#if (FREERDP_VERSION_MAJOR >= 3 && FREERDP_VERSION_MINOR >= 25)
+ /* FIDO PIN support was added in 3.25.0 */
+ case AUTH_FIDO_PIN:
+#endif
+ case AUTH_SMARTCARD_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+#if (FREERDP_VERSION_MAJOR >= 3 && FREERDP_VERSION_MINOR >= 18)
+ /* RDSTLS support was added in 3.18.0 */
+ case AUTH_RDSTLS:
+#endif
+ case AUTH_TLS:
+ case AUTH_RDP:
+ case AUTH_NLA:
+ i = guac_rdp_register_required_auth(settings, params);
+ break;
+
+ case GW_AUTH_HTTP:
+ case GW_AUTH_RDG:
+ case GW_AUTH_RPC:
+ i = guac_rdp_register_required_gateway_auth(settings, params);
+ break;
Review Comment:
Is there any potential that this callback will be invoked multiple times?
If not, then we're fine. If it _can_ be called multiple times, then these parameters will end up being registered repeatedly until `GUAC_ARGV_MAX_REGISTERED` is reached.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3655415063
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +437,124 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @param reason
+ * This enum value that tracks why authentication is being
+ * requested, which informs what types of values the authenticate function
+ * will request.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
}
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+#if (FREERDP_VERSION_MAJOR >= 3 && FREERDP_VERSION_MINOR >= 25)
+ /* FIDO PIN support was added in 3.25.0 */
+ case AUTH_FIDO_PIN:
+#endif
+ case AUTH_SMARTCARD_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+#if (FREERDP_VERSION_MAJOR >= 3 && FREERDP_VERSION_MINOR >= 18)
+ /* RDSTLS support was added in 3.18.0 */
+ case AUTH_RDSTLS:
+#endif
+ case AUTH_TLS:
+ case AUTH_RDP:
+ case AUTH_NLA:
+ i = guac_rdp_register_required_auth(settings, params);
+ break;
+
+ case GW_AUTH_HTTP:
+ case GW_AUTH_RDG:
+ case GW_AUTH_RPC:
+ i = guac_rdp_register_required_gateway_auth(settings, params);
+ break;
Review Comment:
No worries, this sounds fine as-is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper merged PR #670:
URL: https://github.com/apache/guacamole-server/pull/670
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "corentin-soriano (via GitHub)" <gi...@apache.org>.
corentin-soriano commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3257645181
##########
src/protocols/rdp/rdp.c:
##########
@@ -326,6 +327,137 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication used to
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = {NULL};
+ int i = 0;
+
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
+ }
Review Comment:
Part of the code in `rdp_freerdp_authenticate_ex` seems to be redundant with `rdp_freerdp_authenticate`. Perhaps we could extract the common part?
##########
src/protocols/rdp/rdp.c:
##########
@@ -326,6 +327,137 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication used to
Review Comment:
I think the end of the sentence is missing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3618625535
##########
configure.ac:
##########
@@ -1010,6 +1010,22 @@ then
[[#include <freerdp/freerdp.h>]])
fi
+# Check for either AuthenticateEx or Authenticate callbacks.
+if test "x${have_freerdp}" = "xyes"
+then
+ AC_CHECK_MEMBERS([freerdp.AuthenticateEx],,
+ [AC_CHECK_MEMBERS([freerdp.Authenticate],,
+ [AC_MSG_WARN([
+ ----------------------------------------------------
+ This version of FreeRDP appears to lack support
+ for both Authenticate and AuthenticateEx functions.
+ RDP connections may fail if credentials are not
+ provided with the connection.
+ ----------------------------------------------------])],
+ [[#include <freerdp/freerdp.h>]])],
+ [[#include <freerdp/freerdp.h>]])
Review Comment:
Hmm...I tested up to 3.30.0, the latest tag in the FreeRDP code, and it seems to work properly?
```
...
checking for rdpSettings.AllowUnanouncedOrdersFromServer... yes
checking for freerdp.VerifyCertificateEx... yes
checking for freerdp.AuthenticateEx... yes
...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3618368667
##########
src/protocols/rdp/rdp.c:
##########
@@ -279,51 +386,113 @@ static BOOL rdp_freerdp_authenticate(freerdp* instance, char** username,
return TRUE;
}
- /* If the username is undefined, add it to the requested parameters. */
- if (settings->username == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_USERNAME, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_USERNAME;
- i++;
-
- /* If username is undefined and domain is also undefined, request domain. */
- if (settings->domain == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_DOMAIN, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_DOMAIN;
- i++;
- }
-
- }
+ /* Add the required parameters and send them over to the client. */
+ if (guac_rdp_register_required_auth(settings, params))
+ guac_rdp_send_auth_params(client, (const char**) params, username, password, domain);
- /* If the password is undefined, add it to the requested parameters. */
- if (settings->password == NULL) {
- guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
- params[i] = GUAC_RDP_ARGV_PASSWORD;
- i++;
- }
-
- /* NULL-terminate the array. */
- params[i] = NULL;
+ /* Always return TRUE allowing connection to retry. */
+ return TRUE;
+
+}
+#endif // HAVE_FREERDP_AUTHENTICATE
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+/**
+ * Callback invoked by FreeRDP when authentication is required but the required
+ * parameters have not been provided. In the case of Guacamole clients that
+ * support the "required" instruction, this function will send any of the three
+ * unpopulated RDP authentication parameters back to the client so that the
+ * connection owner can provide the required information. If the values have
+ * been provided in the original connection parameters the user will not be
+ * prompted for updated parameters. If the version of Guacamole Client in use
+ * by the connection owner does not support the "required" instruction then the
+ * connection will fail. This function always returns true. This callback is for
+ * the AuthenticateEx version, which also provides the "reason" field, indicating
+ * the type of authentication that the server is requesting from the client.
+ * Guacamole currently supports PIN and traditional username, password, and domain
+ * authentication.
+ *
+ * @param instance
+ * The FreeRDP instance associated with the RDP session requesting
+ * credentials.
+ *
+ * @param username
+ * Pointer to a string which will receive the user's username.
+ *
+ * @param password
+ * Pointer to a string which will receive the user's password.
+ *
+ * @param domain
+ * Pointer to a string which will receive the domain associated with the
+ * user's account.
+ *
+ * @return
+ * Always TRUE.
+ */
+static BOOL rdp_freerdp_authenticate_ex(freerdp* instance, char** username,
+ char** password, char** domain, rdp_auth_reason reason) {
+
+ rdpContext* context = GUAC_RDP_CONTEXT(instance);
+ guac_client* client = ((rdp_freerdp_context*) context)->client;
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+ char* params[4] = { NULL };
+ int i = 0;
- if (i > 0) {
-
- /* Send required parameters to the owner and wait for the response. */
- guac_client_owner_send_required(client, (const char**) params);
- guac_argv_await((const char**) params);
-
- /* Free old values and get new values from settings. */
- guac_mem_free(*username);
- guac_mem_free(*password);
- guac_mem_free(*domain);
- *username = guac_strdup(settings->username);
- *password = guac_strdup(settings->password);
- *domain = guac_strdup(settings->domain);
-
+ /* If the client does not support the "required" instruction, warn and
+ * quit.
+ */
+ if (!guac_client_owner_supports_required(client)) {
+ guac_client_log(client, GUAC_LOG_WARNING, "Client does not support the "
+ "\"required\" instruction. No authentication parameters will "
+ "be requested.");
+ return TRUE;
+ }
+
+ switch (reason) {
+
+ /* In the case that we just need a PIN, request only the PIN/password. */
+ case AUTH_SMARTCARD_PIN:
+ case AUTH_FIDO_PIN:
+
+ /* If the password/PIN is undefined, add it to the requested parameters. */
+ if (settings->password == NULL) {
+ guac_argv_register(GUAC_RDP_ARGV_PASSWORD, guac_rdp_argv_callback, NULL, 0);
+ params[i] = GUAC_RDP_ARGV_PASSWORD;
+ i++;
+
+ /* NULL-terminate the array. */
+ params[i] = NULL;
+ }
+
+ break;
+
+ /* For all other cases, request anything that isn't configured. */
+ case AUTH_RDSTLS:
+ case AUTH_TLS:
+ case AUTH_RDP:
+ case AUTH_NLA:
+ case GW_AUTH_HTTP:
+ case GW_AUTH_RDG:
+ case GW_AUTH_RPC:
+ i = guac_rdp_register_required_auth(settings, params);
Review Comment:
I've implemented gateway authentication requests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3642334432
##########
src/protocols/rdp/rdp.c:
##########
@@ -231,6 +232,164 @@ static BOOL rdp_freerdp_pre_connect(freerdp* instance) {
return TRUE;
}
+#if defined(HAVE_FREERDP_AUTHENTICATE) || defined(HAVE_FREERDP_AUTHENTICATEEX)
+
+/**
+ * Provided a guac_client, a NULL-terminated list of parameters, and the
+ * pointers to the various authentication-related parameters, send the
+ * authentication parameter request on to the client, wait for the
+ * results back from the client, and then update the various settings
+ * and locations where that authentication information is stored.
+ *
+ * @param client
+ * The guac_client that is requesting the updated authentication
+ * information.
+ *
+ * @param params
+ * A NULL-terminated list of parameters that will be requested
+ * from the client.
+ *
+ * @param username
+ * A pointer to the memory location where the RDP client currently
+ * has stored the username.
+ *
+ * @param password
+ * A pointer to the memory location where the RDP client currently
+ * has stored the password.
+ *
+ * @param domain
+ * A pointer to the memory location where the RDP client currently
+ * has stored the domain.
+ */
+static void guac_rdp_send_auth_params(guac_client* client, const char** params,
Review Comment:
Okay, I've updated that function to handle the gateway parameters, as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3658437855
##########
src/protocols/rdp/rdp.c:
##########
@@ -231,6 +232,180 @@ static BOOL rdp_freerdp_pre_connect(freerdp* instance) {
return TRUE;
}
+#if defined(HAVE_FREERDP_AUTHENTICATE) || defined(HAVE_FREERDP_AUTHENTICATEEX)
+
+/**
+ * Provided a guac_client, a NULL-terminated list of parameters, and the
+ * pointers to the various authentication-related parameters, send the
+ * authentication parameter request on to the client, wait for the
+ * results back from the client, and then update the various settings
+ * and locations where that authentication information is stored.
+ *
+ * @param client
+ * The guac_client that is requesting the updated authentication
+ * information.
+ *
+ * @param params
+ * A NULL-terminated list of parameters that will be requested
+ * from the client.
+ *
+ * @param username
+ * A pointer to the memory location where the RDP client currently
+ * has stored the username.
+ *
+ * @param password
+ * A pointer to the memory location where the RDP client currently
+ * has stored the password.
+ *
+ * @param domain
+ * A pointer to the memory location where the RDP client currently
+ * has stored the domain.
+ */
+static void guac_rdp_send_auth_params(guac_client* client, const char** params,
+ char** username, char** password, char** domain) {
+
+ guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
+ guac_rdp_settings* settings = rdp_client->settings;
+
+ /* Send required parameters to the owner and wait for the response. */
+ guac_client_owner_send_required(client, params);
+ guac_argv_await(params);
+
+ /* Free old values and get new values from settings. */
+ for (int i = 0; params[i] != NULL; i++) {
+
+ if (strcmp(params[i], GUAC_RDP_ARGV_USERNAME) == 0) {
+ guac_mem_free(*username);
+ *username = guac_strdup(settings->username);
+ }
+
+ if (strcmp(params[i], GUAC_RDP_ARGV_GATEWAY_USERNAME) == 0) {
+ guac_mem_free(*username);
+ *username = guac_strdup(settings->gateway_username);
+ }
+
+ if (strcmp(params[i], GUAC_RDP_ARGV_DOMAIN) == 0) {
+ guac_mem_free(*domain);
+ *domain = guac_strdup(settings->domain);
+ }
+
+ if (strcmp(params[i], GUAC_RDP_ARGV_GATEWAY_DOMAIN) == 0) {
+ guac_mem_free(*domain);
+ *domain = guac_strdup(settings->gateway_domain);
+ }
+
+ if (strcmp(params[i], GUAC_RDP_ARGV_PASSWORD) == 0) {
+ guac_mem_free(*password);
+ *password = guac_strdup(settings->password);
+ }
+
+ if (strcmp(params[i], GUAC_RDP_ARGV_GATEWAY_PASSWORD) == 0) {
+ guac_mem_free(*password);
+ *password = guac_strdup(settings->gateway_password);
+ }
Review Comment:
Fixed via rebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GUACAMOLE-2273: Implement FreeRDP AuthenticateEx callback and handle deprecation of Authenticate callback. [guacamole-server]
Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #670:
URL: https://github.com/apache/guacamole-server/pull/670#discussion_r3591922991
##########
src/protocols/rdp/rdp.c:
##########
@@ -549,11 +718,20 @@ static int guac_rdp_handle_connection(guac_client* client) {
* If the freerdp instance has a LoadChannels callback for loading plugins
* we use that instead of the PreConnect callback to load plugins.
*/
- #ifdef RDP_INST_HAS_LOAD_CHANNELS
+#ifdef RDP_INST_HAS_LOAD_CHANNELS
rdp_inst->LoadChannels = rdp_freerdp_load_channels;
- #endif
+#endif
+
rdp_inst->PreConnect = rdp_freerdp_pre_connect;
+
+#ifdef HAVE_FREERDP_AUTHENTICATE
rdp_inst->Authenticate = rdp_freerdp_authenticate;
+#endif
+
+#ifdef HAVE_FREERDP_AUTHENTICATEEX
+ rdp_inst->AuthenticateEx = rdp_freerdp_authenticate_ex;
+#endif
Review Comment:
Yes, that makes sense - I've addressed this via `configure` compile-time checks and a rebase.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org