You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by "aleitner (via GitHub)" <gi...@apache.org> on 2024/05/09 04:50:52 UTC

[PR] GUACAMOLE-1026: Add support for FreeRDP3. [guacamole-server]

aleitner opened a new pull request, #517:
URL: https://github.com/apache/guacamole-server/pull/517

   (no comment)


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2105205507

   I need to pull the code down and actually run it through some testing before I'm good with it, I'll try to get that done this weekend.
   
   Currently this is targeted at the `main` branch which would put it in the 1.6.0 release. In the past I think it was mentioned that this might go into a 2.0 release, so wanted to check and see if @mike-jumper or @jmuehlner had any thoughts on that? I'm fine with it going into 1.6.0 - it seems like it's a pretty minimal and compatible set of changes, just want to make sure we're all in agreement.


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2758638212

   @AxelDolislager : Please do not post discussion questions or issues to merged/closed pull requests. Please head over to the mailing list and post your discussion, there: https://guacamole.apache.org/support/#mailing-lists.


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2127595751

   > Huh that shouldn't happen. I just pushed changes a little bit ago to resolve that
   
   Ah ok - looks like a new build did kick off a bit ago - maybe that'll fix it.
   https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/2/


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1595758588


##########
src/libguac/string.c:
##########
@@ -124,18 +124,30 @@ char* guac_strdup(const char* str) {
     /* Do not attempt to duplicate if the length is somehow magically so
      * obscenely large that it will not be possible to add a null terminator */
     size_t length;
-    if (guac_mem_ckd_add(&length, strlen(str), 1))
+    size_t length_to_copy = strnlen(str, n);
+    if (guac_mem_ckd_add(&length, length_to_copy, 1))
         return NULL;
 
-    /* Otherwise just copy to a new string in same manner as strdup() */
-    void* new_str = guac_mem_alloc(length);
-    if (new_str != NULL)
-        memcpy(new_str, str, length);
+    /* Otherwise just copy to a new string in same manner as strndup() */
+    char* new_str = (char*)guac_mem_alloc(length);
+    if (new_str != NULL) {
+        memcpy(new_str, str, length_to_copy);
+        new_str[length_to_copy] = '\0';

Review Comment:
   Looks to me like `guac_strdup()` did add a NULL terminator... it added 1 to `strlen(str)` and then copied that many bytes over, which will include the terminator. What am I missing?
   
   Also, `guac_strdup()` _should add a NULL-terminator, as it's described as behaving identically to `strdup()` (which adds a NULL-terminator), aside from the NULL check.
   



##########
src/libguac/string.c:
##########
@@ -124,18 +124,30 @@ char* guac_strdup(const char* str) {
     /* Do not attempt to duplicate if the length is somehow magically so
      * obscenely large that it will not be possible to add a null terminator */
     size_t length;
-    if (guac_mem_ckd_add(&length, strlen(str), 1))
+    size_t length_to_copy = strnlen(str, n);
+    if (guac_mem_ckd_add(&length, length_to_copy, 1))
         return NULL;
 
-    /* Otherwise just copy to a new string in same manner as strdup() */
-    void* new_str = guac_mem_alloc(length);
-    if (new_str != NULL)
-        memcpy(new_str, str, length);
+    /* Otherwise just copy to a new string in same manner as strndup() */
+    char* new_str = (char*)guac_mem_alloc(length);
+    if (new_str != NULL) {
+        memcpy(new_str, str, length_to_copy);
+        new_str[length_to_copy] = '\0';

Review Comment:
   Looks to me like `guac_strdup()` did add a NULL terminator... it added 1 to `strlen(str)` and then copied that many bytes over, which will include the terminator. What am I missing?
   
   Also, `guac_strdup()` _should_ add a NULL-terminator, as it's described as behaving identically to `strdup()` (which adds a NULL-terminator), aside from the NULL check.
   



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1609047493


##########
src/protocols/rdp/settings.c:
##########
@@ -1451,11 +1440,279 @@ static int guac_rdp_get_performance_flags(guac_rdp_settings* guac_settings) {
 
 }
 
+int guac_rdp_get_width(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_DesktopWidth);
+#else
+    return rdp->settings->DesktopWidth;
+#endif
+}
+
+int guac_rdp_get_height(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_DesktopHeight);
+#else
+    return rdp->settings->DesktopHeight;
+#endif
+}
+
+int guac_rdp_get_depth(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_ColorDepth);
+#else
+    return rdp->settings->ColorDepth;
+#endif
+}
+
 void guac_rdp_push_settings(guac_client* client,
         guac_rdp_settings* guac_settings, freerdp* rdp) {
 
-    rdpSettings* rdp_settings = rdp->settings;
+    rdpSettings* rdp_settings = GUAC_RDP_CONTEXT(rdp)->settings;
+
+#ifdef HAVE_SETTERS_GETTERS
+    /* Authentication */
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Domain, guac_strdup(guac_settings->domain));
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Username, guac_strdup(guac_settings->username));
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Password, guac_strdup(guac_settings->password));
+
+    /* Connection */
+    freerdp_settings_set_string(rdp_settings, FreeRDP_ServerHostname, guac_strdup(guac_settings->hostname));
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_ServerPort, guac_settings->port);
+
+    /* Session */
+
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_DesktopWidth, guac_settings->width);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_DesktopHeight, guac_settings->height);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_ColorDepth, guac_settings->color_depth);
+    freerdp_settings_set_string(rdp_settings, FreeRDP_AlternateShell, guac_strdup(guac_settings->initial_program));
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_KeyboardLayout, guac_settings->server_layout->freerdp_keyboard_layout);
+
+
+    /* Performance flags */
+    /* Explicitly set flag value */
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_PerformanceFlags, guac_rdp_get_performance_flags(guac_settings));
+
+    /* Always request frame markers */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_FrameMarkerCommandEnabled, TRUE);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_SurfaceFrameMarkerEnabled, TRUE);
+
+    /* Enable RemoteFX / Graphics Pipeline */
+    if (guac_settings->enable_gfx) {
+
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SupportGraphicsPipeline, TRUE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteFxCodec, TRUE);
+
+        if (freerdp_settings_get_uint32(rdp_settings, FreeRDP_ColorDepth) != RDP_GFX_REQUIRED_DEPTH) {
+            guac_client_log(client, GUAC_LOG_WARNING, "Ignoring requested "
+                    "color depth of %i bpp, as the RDP Graphics Pipeline "
+                    "requires %i bpp.", freerdp_settings_get_uint32(rdp_settings, FreeRDP_ColorDepth), RDP_GFX_REQUIRED_DEPTH);
+        }
+
+        /* Required for RemoteFX / Graphics Pipeline */
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_FastPathOutput, TRUE);
+        freerdp_settings_set_uint32(rdp_settings, FreeRDP_ColorDepth, RDP_GFX_REQUIRED_DEPTH);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SoftwareGdi, TRUE);
+
+    }
+
+    /* Set individual flags - some FreeRDP versions overwrite the above */

Review Comment:
   So "the above" means "flags set by guac_rdp_get_performance_flags() above"?
   
   If so, mind updating the comment to be a bit more specific? 



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "aleitner (via GitHub)" <gi...@apache.org>.
aleitner commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1626734615


##########
src/protocols/rdp/gdi.c:
##########
@@ -412,7 +412,14 @@ BOOL guac_rdp_gdi_surface_frame_marker(rdpContext* context, const SURFACE_FRAME_
 
     guac_rdp_gdi_mark_frame(context, surface_frame_marker->frameAction != SURFACECMD_FRAMEACTION_END);
 
-    if (context->settings->FrameAcknowledge > 0)
+    int frame_acknowledge;
+#ifdef HAVE_SETTERS_GETTERS
+    frame_acknowledge = freerdp_settings_get_uint32(context->settings, FreeRDP_FrameAcknowledge);
+#else
+    frame_acknowledge = context->settings->FrameAcknowledge;
+#endif
+
+    if (frame_acknowledge> 0)

Review Comment:
   whoops! fixed



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2148409618

   And it's now passing! https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/13/
   > Looks like it failed again... https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/2/FREERDP_BRANCH=3.5.1,JENKINS_LABEL_EXPRESSION=ubuntu/
   
   


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1595796074


##########
src/protocols/rdp/settings.c:
##########
@@ -1451,11 +1440,279 @@ static int guac_rdp_get_performance_flags(guac_rdp_settings* guac_settings) {
 
 }
 
+int guac_rdp_get_width(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_DesktopWidth);
+#else
+    return rdp->settings->DesktopWidth;
+#endif
+}
+
+int guac_rdp_get_height(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_DesktopHeight);
+#else
+    return rdp->settings->DesktopHeight;
+#endif
+}
+
+int guac_rdp_get_depth(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_ColorDepth);
+#else
+    return rdp->settings->ColorDepth;
+#endif
+}
+
 void guac_rdp_push_settings(guac_client* client,
         guac_rdp_settings* guac_settings, freerdp* rdp) {
 
-    rdpSettings* rdp_settings = rdp->settings;
+    rdpSettings* rdp_settings = GUAC_RDP_CONTEXT(rdp)->settings;
+
+#ifdef HAVE_SETTERS_GETTERS
+    /* Authentication */
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Domain, guac_strdup(guac_settings->domain));
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Username, guac_strdup(guac_settings->username));
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Password, guac_strdup(guac_settings->password));
+
+    /* Connection */
+    freerdp_settings_set_string(rdp_settings, FreeRDP_ServerHostname, guac_strdup(guac_settings->hostname));
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_ServerPort, guac_settings->port);
+
+    /* Session */
+
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_DesktopWidth, guac_settings->width);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_DesktopHeight, guac_settings->height);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_ColorDepth, guac_settings->color_depth);
+    freerdp_settings_set_string(rdp_settings, FreeRDP_AlternateShell, guac_strdup(guac_settings->initial_program));
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_KeyboardLayout, guac_settings->server_layout->freerdp_keyboard_layout);
+
+
+    /* Performance flags */
+    /* Explicitly set flag value */
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_PerformanceFlags, guac_rdp_get_performance_flags(guac_settings));
+
+    /* Always request frame markers */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_FrameMarkerCommandEnabled, TRUE);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_SurfaceFrameMarkerEnabled, TRUE);
+
+    /* Enable RemoteFX / Graphics Pipeline */
+    if (guac_settings->enable_gfx) {
+
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SupportGraphicsPipeline, TRUE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteFxCodec, TRUE);
+
+        if (freerdp_settings_get_uint32(rdp_settings, FreeRDP_ColorDepth) != RDP_GFX_REQUIRED_DEPTH) {
+            guac_client_log(client, GUAC_LOG_WARNING, "Ignoring requested "
+                    "color depth of %i bpp, as the RDP Graphics Pipeline "
+                    "requires %i bpp.", freerdp_settings_get_uint32(rdp_settings, FreeRDP_ColorDepth), RDP_GFX_REQUIRED_DEPTH);
+        }
+
+        /* Required for RemoteFX / Graphics Pipeline */
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_FastPathOutput, TRUE);
+        freerdp_settings_set_uint32(rdp_settings, FreeRDP_ColorDepth, RDP_GFX_REQUIRED_DEPTH);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SoftwareGdi, TRUE);
+
+    }
+
+    /* Set individual flags - some FreeRDP versions overwrite the above */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_AllowFontSmoothing, guac_settings->font_smoothing_enabled);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DisableWallpaper, guac_settings->wallpaper_enabled);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DisableFullWindowDrag, guac_settings->full_window_drag_enabled);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DisableMenuAnims, guac_settings->menu_animations_enabled);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DisableThemes, guac_settings->theming_enabled);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_AllowDesktopComposition, guac_settings->desktop_composition_enabled);
+
+    /* Client name */
+    if (guac_settings->client_name != NULL) {
+        freerdp_settings_set_string(rdp_settings, FreeRDP_ClientHostname, 
+                guac_strndup(guac_settings->client_name, RDP_CLIENT_HOSTNAME_SIZE));
+    }
+
+    /* Console */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_ConsoleSession, guac_settings->console);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteConsoleAudio, guac_settings->console_audio);
+
+    /* Audio */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_AudioPlayback, guac_settings->audio_enabled);
+
+    /* Audio capture */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_AudioCapture, guac_settings->enable_audio_input);
+
+    /* Display Update channel */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_SupportDisplayControl, 
+            (guac_settings->resize_method == GUAC_RESIZE_DISPLAY_UPDATE));
+
+    /* Timezone redirection */
+    if (guac_settings->timezone) {
+        if (setenv("TZ", guac_settings->timezone, 1)) {
+            guac_client_log(client, GUAC_LOG_WARNING,
+                "Unable to forward timezone: TZ environment variable "
+                "could not be set: %s", strerror(errno));
+        }
+    }
+
+    /* Device redirection */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DeviceRedirection, 
+            (guac_settings->audio_enabled || guac_settings->drive_enabled || guac_settings->printing_enabled));
+
+    /* Security */
+    switch (guac_settings->security_mode) {
+
+        /* Legacy RDP encryption */
+        case GUAC_SECURITY_RDP:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_UseRdpSecurityLayer, TRUE);
+            freerdp_settings_set_uint32(rdp_settings, FreeRDP_EncryptionLevel, 
+                    ENCRYPTION_LEVEL_CLIENT_COMPATIBLE);
+            freerdp_settings_set_uint32(rdp_settings, FreeRDP_EncryptionMethods, 
+                    ENCRYPTION_METHOD_40BIT | ENCRYPTION_METHOD_128BIT | ENCRYPTION_METHOD_FIPS);
+            break;
+
+        /* TLS encryption */
+        case GUAC_SECURITY_TLS:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, FALSE);
+            break;
+
+        /* Network level authentication */
+        case GUAC_SECURITY_NLA:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, FALSE);
+            break;
+
+        /* Extended network level authentication */
+        case GUAC_SECURITY_EXTENDED_NLA:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, TRUE);
+            break;
+
+        /* Hyper-V "VMConnect" negotiation mode */
+        case GUAC_SECURITY_VMCONNECT:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_VmConnectMode, TRUE);
+            break;
+
+        /* All security types */
+        case GUAC_SECURITY_ANY:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, TRUE);
+
+            /* Explicitly disable NLA if FIPS mode is enabled - it won't work */
+            if (guac_fips_enabled()) {
+
+                guac_client_log(client, GUAC_LOG_INFO,
+                        "FIPS mode is enabled. Excluding NLA security mode from security negotiation "
+                        "(see: https://github.com/FreeRDP/FreeRDP/issues/3412).");
+                freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, FALSE);
+
+            }
+
+            /* NLA mode is allowed if FIPS is not enabled */
+            else
+                freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, TRUE);
+
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, FALSE);
+            break;
+
+    }
+
+    /* Security */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_Authentication, !guac_settings->disable_authentication);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_IgnoreCertificate, guac_settings->ignore_certificate);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_AutoAcceptCertificate, guac_settings->certificate_tofu);
+    if (guac_settings->certificate_fingerprints != NULL)
+        freerdp_settings_set_string(rdp_settings, FreeRDP_CertificateAcceptedFingerprints, 
+                guac_strdup(guac_settings->certificate_fingerprints));
+
+
+    /* RemoteApp */
+    if (guac_settings->remote_app != NULL) {
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_Workarea, TRUE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteApplicationMode, TRUE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteAppLanguageBarSupported, TRUE);
+        freerdp_settings_set_string(rdp_settings, FreeRDP_RemoteApplicationProgram, guac_strdup(guac_settings->remote_app));
+        freerdp_settings_set_string(rdp_settings, FreeRDP_ShellWorkingDirectory, guac_strdup(guac_settings->remote_app_dir));
+        freerdp_settings_set_string(rdp_settings, FreeRDP_RemoteApplicationCmdLine, guac_strdup(guac_settings->remote_app_args));
+    }
+
+    /* Preconnection ID */
+    if (guac_settings->preconnection_id != -1) {
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_NegotiateSecurityLayer, FALSE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SendPreconnectionPdu, TRUE);
+        freerdp_settings_set_uint32(rdp_settings, FreeRDP_PreconnectionId, guac_settings->preconnection_id);
+    }
+
+    /* Preconnection BLOB */
+    if (guac_settings->preconnection_blob != NULL) {
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_NegotiateSecurityLayer, FALSE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SendPreconnectionPdu, TRUE);
+        freerdp_settings_set_string(rdp_settings, FreeRDP_PreconnectionBlob, guac_strdup(guac_settings->preconnection_blob));
+    }
+
+    /* Enable use of RD gateway if a gateway hostname is provided */
+    if (guac_settings->gateway_hostname != NULL) {
+
+        /* Enable RD gateway */
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_GatewayEnabled, TRUE);
 
+        /* RD gateway connection details */
+        freerdp_settings_set_string(rdp_settings, FreeRDP_GatewayHostname, guac_strdup(guac_settings->gateway_hostname));
+        freerdp_settings_set_uint32(rdp_settings, FreeRDP_GatewayPort, guac_settings->gateway_port);
+
+        /* RD gateway credentials */
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_GatewayUseSameCredentials, FALSE);
+        freerdp_settings_set_string(rdp_settings, FreeRDP_GatewayDomain, guac_strdup(guac_settings->gateway_domain));
+        freerdp_settings_set_string(rdp_settings, FreeRDP_GatewayUsername, guac_strdup(guac_settings->gateway_username));
+        freerdp_settings_set_string(rdp_settings, FreeRDP_GatewayPassword, guac_strdup(guac_settings->gateway_password));
+
+    }
+
+    /* Store load balance info (and calculate length) if provided */
+    if (guac_settings->load_balance_info != NULL) {
+        freerdp_settings_set_pointer(rdp_settings, FreeRDP_LoadBalanceInfo, (BYTE*) guac_strdup(guac_settings->load_balance_info));
+        freerdp_settings_set_uint32(rdp_settings, FreeRDP_LoadBalanceInfoLength, strlen(guac_settings->load_balance_info));
+    }
+
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_BitmapCacheEnabled, !guac_settings->disable_bitmap_caching);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_OffscreenSupportLevel, !guac_settings->disable_offscreen_caching);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_GlyphSupportLevel, 
+            (!guac_settings->disable_glyph_caching ? GLYPH_SUPPORT_FULL : GLYPH_SUPPORT_NONE));
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_OsMajorType, OSMAJORTYPE_UNSPECIFIED);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_OsMinorType, OSMINORTYPE_UNSPECIFIED);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DesktopResize, TRUE);
+
+    /* Claim support only for specific updates, independent of FreeRDP defaults */
+	BYTE* order_support = freerdp_settings_get_pointer_writable(rdp_settings, FreeRDP_OrderSupport);
+	if (order_support) {
+        ZeroMemory(order_support, GUAC_RDP_ORDER_SUPPORT_LENGTH);
+        order_support[NEG_DSTBLT_INDEX] = TRUE;
+        order_support[NEG_SCRBLT_INDEX] = TRUE;
+        order_support[NEG_MEMBLT_INDEX] = !guac_settings->disable_bitmap_caching;
+        order_support[NEG_MEMBLT_V2_INDEX] = !guac_settings->disable_bitmap_caching;
+        order_support[NEG_GLYPH_INDEX_INDEX] = !guac_settings->disable_glyph_caching;
+        order_support[NEG_FAST_INDEX_INDEX] = !guac_settings->disable_glyph_caching;
+        order_support[NEG_FAST_GLYPH_INDEX] = !guac_settings->disable_glyph_caching;
+    }
+
+#ifdef HAVE_RDPSETTINGS_ALLOWUNANOUNCEDORDERSFROMSERVER

Review Comment:
   Where would `HAVE_RDPSETTINGS_ALLOWUNANOUNCEDORDERSFROMSERVER` be defined? I don't see it in our source or the FreeRDP source.



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1595849497


##########
src/libguac/string.c:
##########
@@ -124,18 +124,30 @@ char* guac_strdup(const char* str) {
     /* Do not attempt to duplicate if the length is somehow magically so
      * obscenely large that it will not be possible to add a null terminator */
     size_t length;
-    if (guac_mem_ckd_add(&length, strlen(str), 1))
+    size_t length_to_copy = strnlen(str, n);
+    if (guac_mem_ckd_add(&length, length_to_copy, 1))
         return NULL;
 
-    /* Otherwise just copy to a new string in same manner as strdup() */
-    void* new_str = guac_mem_alloc(length);
-    if (new_str != NULL)
-        memcpy(new_str, str, length);
+    /* Otherwise just copy to a new string in same manner as strndup() */
+    char* new_str = (char*)guac_mem_alloc(length);
+    if (new_str != NULL) {
+        memcpy(new_str, str, length_to_copy);
+        new_str[length_to_copy] = '\0';

Review Comment:
   > Looks to me like guac_strdup() did add a NULL terminator... it added 1 to strlen(str) and then copied that many bytes over, which will include the terminator. What am I missing?
   
   I would agree that it copies over the NULL terminator if the original string has one, but it does not manually add one to the end of the string. But maybe I'm missing something?? Here's the current source code:
   
   https://github.com/apache/guacamole-server/blob/3ad3b041f273ec1ed30e49be49f1adcf0edd0b61/src/libguac/string.c#L118-L137
   
   It looks to me like it:
   * Checks if `str` is `NULL`, returning `NULL` if that is the case.
   * Checks the string length, returning `NULL` if it cannot fit in memory.
   * Allocates the memory, and then uses `memcpy` to copy over the memory.
   * Returns the string.
   
   I don't see any explicit addition of a null terminator in that function, whereas the modified version, here, does explicitly add a null terminator, as does the `guac_strlcpy()` function earlier in the file.



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "aleitner (via GitHub)" <gi...@apache.org>.
aleitner commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1596125287


##########
configure.ac:
##########
@@ -891,17 +909,162 @@ fi
 
 # Updated certificate verification callback (introduced with 2.0.0, not present
 # in 2.0.0-rc4 or earlier)
-if test "x${have_freerdp2}" = "xyes"
+if test "x${have_freerdp}" = "xyes"
 then
     AC_CHECK_MEMBERS([freerdp.VerifyCertificateEx],,,
-                     [[#include <freerdp/freerdp.h>]])
+            [[#include <freerdp/freerdp.h>]])

Review Comment:
   ~~Updated formatting~~
   
   Nevermind looks like my editor is rendering differently than github



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1595990647


##########
src/libguac/string.c:
##########
@@ -124,18 +124,30 @@ char* guac_strdup(const char* str) {
     /* Do not attempt to duplicate if the length is somehow magically so
      * obscenely large that it will not be possible to add a null terminator */
     size_t length;
-    if (guac_mem_ckd_add(&length, strlen(str), 1))
+    size_t length_to_copy = strnlen(str, n);
+    if (guac_mem_ckd_add(&length, length_to_copy, 1))
         return NULL;
 
-    /* Otherwise just copy to a new string in same manner as strdup() */
-    void* new_str = guac_mem_alloc(length);
-    if (new_str != NULL)
-        memcpy(new_str, str, length);
+    /* Otherwise just copy to a new string in same manner as strndup() */
+    char* new_str = (char*)guac_mem_alloc(length);
+    if (new_str != NULL) {
+        memcpy(new_str, str, length_to_copy);
+        new_str[length_to_copy] = '\0';

Review Comment:
   Okay, so that means that adding this explicit addition of a null terminator to the new string probably doesn't matter much one way or the other - it's the "safest" option in the case that a string happens to be passed in that doesn't have a null terminator and gets through the various steps (`strlen` in particular) without causing issues.



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman merged PR #517:
URL: https://github.com/apache/guacamole-server/pull/517


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2133490000

   > 
   > Looks like it failed again... https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/2/FREERDP_BRANCH=3.5.1,JENKINS_LABEL_EXPRESSION=ubuntu/
   
   Looks like it doesn't like the attempts to build FreeRDP within the source tree:
   ```
   #8 16.99 CMake Error at cmake/PreventInSourceBuilds.cmake:28 (message):
   #8 16.99   
   #8 16.99 
   #8 16.99   CMake must not to be run in the source directory.  Rather create a
   #8 16.99   dedicated build directory and run CMake there.  CMake now already created
   #8 16.99   some files, to clean up after this aborted in-source compilation:
   #8 16.99 
   #8 16.99      rm -r CMakeCache.txt CMakeFiles
   #8 16.99 
   #8 16.99   or
   #8 16.99 
   #8 16.99      git clean -xdf
   #8 16.99 
   #8 16.99   
   #8 16.99 
   #8 16.99   If you happen to require in-source-tree builds for some reason rerun with
   #8 16.99   -DALLOW_IN_SOURCE_BUILD=ON
   ```


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "hjzhangr (via GitHub)" <gi...@apache.org>.
hjzhangr commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2650642763

   @necouchman Thanks, will the next release version be 1.5.6 or 1.6.0?


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2650648936

   > @necouchman Thanks, will the next release version be 1.5.6 or 1.6.0?
   
   1.6.0.
   
   Please do not continue to use closed PRs as a discussion forum. If you have further questions, subscribe to the mailing list and ask, there.


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "aleitner (via GitHub)" <gi...@apache.org>.
aleitner commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1596113631


##########
src/protocols/rdp/settings.c:
##########
@@ -1451,11 +1440,279 @@ static int guac_rdp_get_performance_flags(guac_rdp_settings* guac_settings) {
 
 }
 
+int guac_rdp_get_width(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_DesktopWidth);
+#else
+    return rdp->settings->DesktopWidth;
+#endif
+}
+
+int guac_rdp_get_height(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_DesktopHeight);
+#else
+    return rdp->settings->DesktopHeight;
+#endif
+}
+
+int guac_rdp_get_depth(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_ColorDepth);
+#else
+    return rdp->settings->ColorDepth;
+#endif
+}
+
 void guac_rdp_push_settings(guac_client* client,
         guac_rdp_settings* guac_settings, freerdp* rdp) {
 
-    rdpSettings* rdp_settings = rdp->settings;
+    rdpSettings* rdp_settings = GUAC_RDP_CONTEXT(rdp)->settings;
+
+#ifdef HAVE_SETTERS_GETTERS
+    /* Authentication */
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Domain, guac_strdup(guac_settings->domain));
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Username, guac_strdup(guac_settings->username));
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Password, guac_strdup(guac_settings->password));
+
+    /* Connection */
+    freerdp_settings_set_string(rdp_settings, FreeRDP_ServerHostname, guac_strdup(guac_settings->hostname));
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_ServerPort, guac_settings->port);
+
+    /* Session */
+
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_DesktopWidth, guac_settings->width);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_DesktopHeight, guac_settings->height);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_ColorDepth, guac_settings->color_depth);
+    freerdp_settings_set_string(rdp_settings, FreeRDP_AlternateShell, guac_strdup(guac_settings->initial_program));
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_KeyboardLayout, guac_settings->server_layout->freerdp_keyboard_layout);
+
+
+    /* Performance flags */
+    /* Explicitly set flag value */
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_PerformanceFlags, guac_rdp_get_performance_flags(guac_settings));
+
+    /* Always request frame markers */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_FrameMarkerCommandEnabled, TRUE);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_SurfaceFrameMarkerEnabled, TRUE);
+
+    /* Enable RemoteFX / Graphics Pipeline */
+    if (guac_settings->enable_gfx) {
+
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SupportGraphicsPipeline, TRUE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteFxCodec, TRUE);
+
+        if (freerdp_settings_get_uint32(rdp_settings, FreeRDP_ColorDepth) != RDP_GFX_REQUIRED_DEPTH) {
+            guac_client_log(client, GUAC_LOG_WARNING, "Ignoring requested "
+                    "color depth of %i bpp, as the RDP Graphics Pipeline "
+                    "requires %i bpp.", freerdp_settings_get_uint32(rdp_settings, FreeRDP_ColorDepth), RDP_GFX_REQUIRED_DEPTH);
+        }
+
+        /* Required for RemoteFX / Graphics Pipeline */
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_FastPathOutput, TRUE);
+        freerdp_settings_set_uint32(rdp_settings, FreeRDP_ColorDepth, RDP_GFX_REQUIRED_DEPTH);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SoftwareGdi, TRUE);
+
+    }
+
+    /* Set individual flags - some FreeRDP versions overwrite the above */

Review Comment:
   The `guac_rdp_get_performance_flags` function is called prior to this and that has some of the same settings.
   https://github.com/apache/guacamole-server/blob/32d232a44a1cd9cf7a4e99e6b63e0df3830f50f6/src/protocols/rdp/settings.c#L1493
   
   https://github.com/apache/guacamole-server/blob/32d232a44a1cd9cf7a4e99e6b63e0df3830f50f6/src/protocols/rdp/settings.c#L1410-L1441



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2125316508

   I've added this temporary build to help verify these changes against various 2.x and 3.x versions of FreeRDP:
   
   https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "mike-jumper (via GitHub)" <gi...@apache.org>.
mike-jumper commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2125261560

   No objection from me for inclusion in 1.6.0 either as long as we maintain compatibility with FreeRDP 2.x.


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "aleitner (via GitHub)" <gi...@apache.org>.
aleitner commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1626734454


##########
Dockerfile:
##########
@@ -145,7 +155,7 @@ RUN ${BUILD_DIR}/src/guacd-docker/bin/build-all.sh
 RUN ${BUILD_DIR}/src/guacd-docker/bin/list-dependencies.sh \
         ${PREFIX_DIR}/sbin/guacd               \
         ${PREFIX_DIR}/lib/libguac-client-*.so  \
-        ${PREFIX_DIR}/lib/freerdp2/*guac*.so   \
+        ${PREFIX_DIR}/lib/freerdp3/*guac*.so   \

Review Comment:
   Updated so now you can use 2 or 3 such as 
   
   `docker build --build-arg FREERDP_VERSION=2 -t guacamole-server-rdp .`



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2128144445

   > > Huh that shouldn't happen. I just pushed changes a little bit ago to resolve that
   > 
   > Ah ok - looks like a new build did kick off a bit ago - maybe that'll fix it. https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/2/
   
   Looks like it failed again... https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/2/FREERDP_BRANCH=3.5.1,JENKINS_LABEL_EXPRESSION=ubuntu/


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2135621813

   > I tweaked the build script slightly to create a build_root directory, cd to that, and then cmake from the parent directory. We'll see what happens.
   
   I updated the script, but that doesn't seem to have triggered an automatic rebuild, and I don't seem to have permissions to force a rebuild. @mike-jumper you might have to kick it off, again.


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "aleitner (via GitHub)" <gi...@apache.org>.
aleitner commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1596133087


##########
configure.ac:
##########
@@ -891,17 +909,162 @@ fi
 
 # Updated certificate verification callback (introduced with 2.0.0, not present
 # in 2.0.0-rc4 or earlier)
-if test "x${have_freerdp2}" = "xyes"
+if test "x${have_freerdp}" = "xyes"
 then
     AC_CHECK_MEMBERS([freerdp.VerifyCertificateEx],,,
-                     [[#include <freerdp/freerdp.h>]])
+            [[#include <freerdp/freerdp.h>]])

Review Comment:
   Should be cleaner now



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1595779776


##########
src/protocols/rdp/settings.c:
##########
@@ -1451,11 +1440,279 @@ static int guac_rdp_get_performance_flags(guac_rdp_settings* guac_settings) {
 
 }
 
+int guac_rdp_get_width(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_DesktopWidth);
+#else
+    return rdp->settings->DesktopWidth;
+#endif
+}
+
+int guac_rdp_get_height(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_DesktopHeight);
+#else
+    return rdp->settings->DesktopHeight;
+#endif
+}
+
+int guac_rdp_get_depth(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_ColorDepth);
+#else
+    return rdp->settings->ColorDepth;
+#endif
+}
+
 void guac_rdp_push_settings(guac_client* client,
         guac_rdp_settings* guac_settings, freerdp* rdp) {
 
-    rdpSettings* rdp_settings = rdp->settings;
+    rdpSettings* rdp_settings = GUAC_RDP_CONTEXT(rdp)->settings;
+
+#ifdef HAVE_SETTERS_GETTERS
+    /* Authentication */
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Domain, guac_strdup(guac_settings->domain));
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Username, guac_strdup(guac_settings->username));
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Password, guac_strdup(guac_settings->password));
+
+    /* Connection */
+    freerdp_settings_set_string(rdp_settings, FreeRDP_ServerHostname, guac_strdup(guac_settings->hostname));
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_ServerPort, guac_settings->port);
+
+    /* Session */
+
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_DesktopWidth, guac_settings->width);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_DesktopHeight, guac_settings->height);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_ColorDepth, guac_settings->color_depth);
+    freerdp_settings_set_string(rdp_settings, FreeRDP_AlternateShell, guac_strdup(guac_settings->initial_program));
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_KeyboardLayout, guac_settings->server_layout->freerdp_keyboard_layout);
+
+
+    /* Performance flags */
+    /* Explicitly set flag value */
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_PerformanceFlags, guac_rdp_get_performance_flags(guac_settings));
+
+    /* Always request frame markers */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_FrameMarkerCommandEnabled, TRUE);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_SurfaceFrameMarkerEnabled, TRUE);
+
+    /* Enable RemoteFX / Graphics Pipeline */
+    if (guac_settings->enable_gfx) {
+
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SupportGraphicsPipeline, TRUE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteFxCodec, TRUE);
+
+        if (freerdp_settings_get_uint32(rdp_settings, FreeRDP_ColorDepth) != RDP_GFX_REQUIRED_DEPTH) {
+            guac_client_log(client, GUAC_LOG_WARNING, "Ignoring requested "
+                    "color depth of %i bpp, as the RDP Graphics Pipeline "
+                    "requires %i bpp.", freerdp_settings_get_uint32(rdp_settings, FreeRDP_ColorDepth), RDP_GFX_REQUIRED_DEPTH);
+        }
+
+        /* Required for RemoteFX / Graphics Pipeline */
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_FastPathOutput, TRUE);
+        freerdp_settings_set_uint32(rdp_settings, FreeRDP_ColorDepth, RDP_GFX_REQUIRED_DEPTH);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SoftwareGdi, TRUE);
+
+    }
+
+    /* Set individual flags - some FreeRDP versions overwrite the above */

Review Comment:
   Overwrite which things above?



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "AxelDolislager (via GitHub)" <gi...@apache.org>.
AxelDolislager commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2758575916

   After compiling the code with freerdp3, making a docker image out of it and testing it out I am receiving the following error:
   
   **guacd[94]: WARNING:        Support for protocol "rdp" is not installed.**
   
   ![image](https://github.com/user-attachments/assets/30697823-2666-4b41-ade8-1739e0b176c9)
   
   I am trying to get this to work because we need to disable NTLM in out active directory.
   
   I was also receiving various errors with the current Dockerfile. This is my current working Dockerfile:
   
   `
   #
   # Licensed to the Apache Software Foundation (ASF) under one
   # or more contributor license agreements.  See the NOTICE file
   # distributed with this work for additional information
   # regarding copyright ownership.  The ASF licenses this file
   # to you under the Apache License, Version 2.0 (the
   # "License"); you may not use this file except in compliance
   # with the License.  You may obtain a copy of the License at
   #
   #   http://www.apache.org/licenses/LICENSE-2.0
   #
   # Unless required by applicable law or agreed to in writing,
   # software distributed under the License is distributed on an
   # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
   # KIND, either express or implied.  See the License for the
   # specific language governing permissions and limitations
   # under the License.
   #
   
   #
   # Dockerfile for guacamole-server
   #
   
   # The Alpine Linux image that should be used as the basis for the guacd image
   # NOTE: Using 3.18 because the required openssl1.1-compat-dev package was
   # removed in more recent versions.
   ARG ALPINE_BASE_IMAGE=3.18
   FROM alpine:${ALPINE_BASE_IMAGE} AS builder
   
   # FreeRDP version (default to version 3)
   ARG FREERDP_VERSION=3
   
   # Install build dependencies
   RUN apk add --no-cache                \
           autoconf                      \
           automake                      \
           build-base                    \
           cairo-dev                     \
           cjson-dev                     \
           cmake                         \
           cunit-dev                     \
           git                           \
           grep                          \
           krb5-dev                      \
           libjpeg-turbo-dev             \
           libpng-dev                    \
           libtool                       \
           libwebp-dev                   \
           make                          \
           openssl1.1-compat-dev         \
           pango-dev                     \
           pulseaudio-dev                \
           sdl2-dev                      \
           sdl2_ttf-dev                  \
           util-linux-dev                \
           webkit2gtk-dev                \
   	nasm			      \
   	cups-dev		      \
   	fuse3-dev		      \
   	libusb-dev		      \
   	ffmpeg-dev
   
   # Copy source to container for sake of build
   ARG BUILD_DIR=/tmp/guacamole-server
   COPY . ${BUILD_DIR}
   
   #
   # Base directory for installed build artifacts.
   #
   # NOTE: Due to limitations of the Docker image build process, this value is
   # duplicated in an ARG in the second stage of the build.
   #
   ARG PREFIX_DIR=/opt/guacamole
   
   #
   # Automatically select the latest versions of each core protocol support
   # library (these can be overridden at build time if a specific version is
   # needed)
   #
   ARG WITH_FREERDP="${FREERDP_VERSION}(\.\d+)+"
   ARG WITH_LIBSSH2='libssh2-\d+(\.\d+)+'
   ARG WITH_LIBTELNET='\d+(\.\d+)+'
   ARG WITH_LIBVNCCLIENT='LibVNCServer-\d+(\.\d+)+'
   ARG WITH_LIBWEBSOCKETS='v\d+(\.\d+)+'
   
   #
   # Default build options for each core protocol support library, as well as
   # guacamole-server itself (these can be overridden at build time if different
   # options are needed)
   #
   
   ARG FREERDP_OPTS="\
       -DBUILTIN_CHANNELS=OFF \
       -DCHANNEL_URBDRC=OFF \
       -DWITH_ALSA=OFF \
       -DWITH_CAIRO=ON \
       -DWITH_CHANNELS=ON \
       -DWITH_CLIENT=ON \
       -DWITH_CUPS=OFF \
       -DWITH_DIRECTFB=OFF \
       -DWITH_FFMPEG=OFF \
       -DWITH_FUSE=OFF \
       -DWITH_GSM=OFF \
       -DWITH_GSSAPI=OFF \
       -DWITH_IPP=OFF \
       -DWITH_JPEG=ON \
       -DWITH_KRB5=ON \
       -DWITH_LIBSYSTEMD=OFF \
       -DWITH_MANPAGES=OFF \
       -DWITH_OPENH264=OFF \
       -DWITH_OPENSSL=ON \
       -DWITH_OSS=OFF \
       -DWITH_PCSC=OFF \
       -DWITH_PKCS11=OFF \
       -DWITH_PULSE=OFF \
       -DWITH_SERVER=OFF \
       -DWITH_SERVER_INTERFACE=OFF \
       -DWITH_SHADOW_MAC=OFF \
       -DWITH_SHADOW_X11=OFF \
       -DWITH_SSE2=ON \
       -DWITH_SWSCALE=OFF \
       -DWITH_WAYLAND=OFF \
       -DWITH_X11=OFF \
       -DWITH_X264=OFF \
       -DWITH_XCURSOR=ON \
       -DWITH_XEXT=ON \
       -DWITH_XI=OFF \
       -DWITH_XINERAMA=OFF \
       -DWITH_XKBFILE=ON \
       -DWITH_XRENDER=OFF \
       -DWITH_XTEST=OFF \
       -DWITH_XV=OFF \
       -DWITH_ZLIB=ON"
   
   ARG GUACAMOLE_SERVER_OPTS="\
       --disable-guaclog"
   
   ARG LIBSSH2_OPTS="\
       -DBUILD_EXAMPLES=OFF \
       -DBUILD_SHARED_LIBS=ON"
   
   ARG LIBTELNET_OPTS="\
       --disable-static \
       --disable-util"
   
   ARG LIBVNCCLIENT_OPTS=""
   
   ARG LIBWEBSOCKETS_OPTS="\
       -DDISABLE_WERROR=ON \
       -DLWS_WITHOUT_SERVER=ON \
       -DLWS_WITHOUT_TESTAPPS=ON \
       -DLWS_WITHOUT_TEST_CLIENT=ON \
       -DLWS_WITHOUT_TEST_PING=ON \
       -DLWS_WITHOUT_TEST_SERVER=ON \
       -DLWS_WITHOUT_TEST_SERVER_EXTPOLL=ON \
       -DLWS_WITH_STATIC=OFF"
   
   # Build guacamole-server and its core protocol library dependencies
   **# Build FreeRDP
   RUN git clone https://github.com/FreeRDP/FreeRDP.git /tmp/FreeRDP
   WORKDIR /tmp/FreeRDP
   RUN git checkout 3.8.0
   RUN mkdir /tmp/FreeRDP/build
   WORKDIR /tmp/FreeRDP/build
   RUN cmake ..
   RUN cmake --build .
   RUN cmake --install .**
   
   **RUN ${BUILD_DIR}/src/guacd-docker/bin/build-all.sh**
   #I've commented out the FreeRDP installation because of errors.
   
   # Determine location of the FREERDP library based on the version.
   ARG FREERDP_LIB_PATH=${PREFIX_DIR}/lib/freerdp${FREERDP_VERSION}
   
   # Record the packages of all runtime library dependencies
   RUN ${BUILD_DIR}/src/guacd-docker/bin/list-dependencies.sh \
           ${PREFIX_DIR}/sbin/guacd               \
           ${PREFIX_DIR}/lib/libguac-client-*.so  \
           ${FREERDP_LIB_PATH}/*guac*.so   \
           > ${PREFIX_DIR}/DEPENDENCIES
   
   # Use same Alpine version as the base for the runtime image
   FROM alpine:${ALPINE_BASE_IMAGE}
   
   #
   # Base directory for installed build artifacts. See also the
   # CMD directive at the end of this build stage.
   #
   # NOTE: Due to limitations of the Docker image build process, this value is
   # duplicated in an ARG in the first stage of the build.
   #
   ARG PREFIX_DIR=/opt/guacamole
   
   # Runtime environment
   ENV LC_ALL=C.UTF-8
   ENV LD_LIBRARY_PATH=${PREFIX_DIR}/lib
   ENV GUACD_LOG_LEVEL=info
   
   # Copy build artifacts into this stage
   COPY --from=builder ${PREFIX_DIR} ${PREFIX_DIR}
   
   # Bring runtime environment up to date and install runtime dependencies
   RUN apk add --no-cache                \
           ca-certificates               \
           font-noto-cjk                 \
           ghostscript                   \
           netcat-openbsd                \
           shadow                        \
           terminus-font                 \
           ttf-dejavu                    \
           ttf-liberation                \
           util-linux-login && \
       xargs apk add --no-cache < ${PREFIX_DIR}/DEPENDENCIES
   
   # Checks the operating status every 5 minutes with a timeout of 5 seconds
   HEALTHCHECK --interval=5m --timeout=5s CMD nc -z 127.0.0.1 4822 || exit 1
   
   # Create a new user guacd
   ARG UID=1000
   ARG GID=1000
   RUN groupadd --gid $GID guacd
   RUN useradd --system --create-home --shell /sbin/nologin --uid $UID --gid $GID guacd
   
   # Run with user guacd
   USER guacd
   
   # Expose the default listener port
   EXPOSE 4822
   
   # Start guacd, listening on port 0.0.0.0:4822
   #
   # Note the path here MUST correspond to the value specified in the 
   # PREFIX_DIR build argument.
   #
   CMD /opt/guacamole/sbin/guacd -b 0.0.0.0 -L $GUACD_LOG_LEVEL -f
   `


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2105237244

   > I need to pull the code down and actually run it through some testing before I'm good with it, I'll try to get that done this weekend.
   > 
   > Currently this is targeted at the `main` branch which would put it in the 1.6.0 release. In the past I think it was mentioned that this might go into a 2.0 release, so wanted to check and see if @mike-jumper or @jmuehlner had any thoughts on that? I'm fine with it going into 1.6.0 - it seems like it's a pretty minimal and compatible set of changes, just want to make sure we're all in agreement.
   
   It doesn't drop compatibility with FreeRDP 2.x so I'm fine putting it in `main`, and in `1.6.0`.


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "aleitner (via GitHub)" <gi...@apache.org>.
aleitner commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1596109138


##########
src/protocols/rdp/settings.c:
##########
@@ -1451,11 +1440,279 @@ static int guac_rdp_get_performance_flags(guac_rdp_settings* guac_settings) {
 
 }
 
+int guac_rdp_get_width(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_DesktopWidth);
+#else
+    return rdp->settings->DesktopWidth;
+#endif
+}
+
+int guac_rdp_get_height(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_DesktopHeight);
+#else
+    return rdp->settings->DesktopHeight;
+#endif
+}
+
+int guac_rdp_get_depth(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_ColorDepth);
+#else
+    return rdp->settings->ColorDepth;
+#endif
+}
+
 void guac_rdp_push_settings(guac_client* client,
         guac_rdp_settings* guac_settings, freerdp* rdp) {
 
-    rdpSettings* rdp_settings = rdp->settings;
+    rdpSettings* rdp_settings = GUAC_RDP_CONTEXT(rdp)->settings;
+
+#ifdef HAVE_SETTERS_GETTERS
+    /* Authentication */
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Domain, guac_strdup(guac_settings->domain));
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Username, guac_strdup(guac_settings->username));
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Password, guac_strdup(guac_settings->password));
+
+    /* Connection */
+    freerdp_settings_set_string(rdp_settings, FreeRDP_ServerHostname, guac_strdup(guac_settings->hostname));
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_ServerPort, guac_settings->port);
+
+    /* Session */
+
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_DesktopWidth, guac_settings->width);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_DesktopHeight, guac_settings->height);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_ColorDepth, guac_settings->color_depth);
+    freerdp_settings_set_string(rdp_settings, FreeRDP_AlternateShell, guac_strdup(guac_settings->initial_program));
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_KeyboardLayout, guac_settings->server_layout->freerdp_keyboard_layout);
+
+
+    /* Performance flags */
+    /* Explicitly set flag value */
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_PerformanceFlags, guac_rdp_get_performance_flags(guac_settings));
+
+    /* Always request frame markers */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_FrameMarkerCommandEnabled, TRUE);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_SurfaceFrameMarkerEnabled, TRUE);
+
+    /* Enable RemoteFX / Graphics Pipeline */
+    if (guac_settings->enable_gfx) {
+
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SupportGraphicsPipeline, TRUE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteFxCodec, TRUE);
+
+        if (freerdp_settings_get_uint32(rdp_settings, FreeRDP_ColorDepth) != RDP_GFX_REQUIRED_DEPTH) {
+            guac_client_log(client, GUAC_LOG_WARNING, "Ignoring requested "
+                    "color depth of %i bpp, as the RDP Graphics Pipeline "
+                    "requires %i bpp.", freerdp_settings_get_uint32(rdp_settings, FreeRDP_ColorDepth), RDP_GFX_REQUIRED_DEPTH);
+        }
+
+        /* Required for RemoteFX / Graphics Pipeline */
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_FastPathOutput, TRUE);
+        freerdp_settings_set_uint32(rdp_settings, FreeRDP_ColorDepth, RDP_GFX_REQUIRED_DEPTH);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SoftwareGdi, TRUE);
+
+    }
+
+    /* Set individual flags - some FreeRDP versions overwrite the above */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_AllowFontSmoothing, guac_settings->font_smoothing_enabled);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DisableWallpaper, guac_settings->wallpaper_enabled);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DisableFullWindowDrag, guac_settings->full_window_drag_enabled);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DisableMenuAnims, guac_settings->menu_animations_enabled);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DisableThemes, guac_settings->theming_enabled);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_AllowDesktopComposition, guac_settings->desktop_composition_enabled);
+
+    /* Client name */
+    if (guac_settings->client_name != NULL) {
+        freerdp_settings_set_string(rdp_settings, FreeRDP_ClientHostname, 
+                guac_strndup(guac_settings->client_name, RDP_CLIENT_HOSTNAME_SIZE));
+    }
+
+    /* Console */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_ConsoleSession, guac_settings->console);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteConsoleAudio, guac_settings->console_audio);
+
+    /* Audio */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_AudioPlayback, guac_settings->audio_enabled);
+
+    /* Audio capture */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_AudioCapture, guac_settings->enable_audio_input);
+
+    /* Display Update channel */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_SupportDisplayControl, 
+            (guac_settings->resize_method == GUAC_RESIZE_DISPLAY_UPDATE));
+
+    /* Timezone redirection */
+    if (guac_settings->timezone) {
+        if (setenv("TZ", guac_settings->timezone, 1)) {
+            guac_client_log(client, GUAC_LOG_WARNING,
+                "Unable to forward timezone: TZ environment variable "
+                "could not be set: %s", strerror(errno));
+        }
+    }
+
+    /* Device redirection */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DeviceRedirection, 
+            (guac_settings->audio_enabled || guac_settings->drive_enabled || guac_settings->printing_enabled));
+
+    /* Security */
+    switch (guac_settings->security_mode) {
+
+        /* Legacy RDP encryption */
+        case GUAC_SECURITY_RDP:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_UseRdpSecurityLayer, TRUE);
+            freerdp_settings_set_uint32(rdp_settings, FreeRDP_EncryptionLevel, 
+                    ENCRYPTION_LEVEL_CLIENT_COMPATIBLE);
+            freerdp_settings_set_uint32(rdp_settings, FreeRDP_EncryptionMethods, 
+                    ENCRYPTION_METHOD_40BIT | ENCRYPTION_METHOD_128BIT | ENCRYPTION_METHOD_FIPS);
+            break;
+
+        /* TLS encryption */
+        case GUAC_SECURITY_TLS:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, FALSE);
+            break;
+
+        /* Network level authentication */
+        case GUAC_SECURITY_NLA:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, FALSE);
+            break;
+
+        /* Extended network level authentication */
+        case GUAC_SECURITY_EXTENDED_NLA:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, TRUE);
+            break;
+
+        /* Hyper-V "VMConnect" negotiation mode */
+        case GUAC_SECURITY_VMCONNECT:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_VmConnectMode, TRUE);
+            break;
+
+        /* All security types */
+        case GUAC_SECURITY_ANY:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, TRUE);
+
+            /* Explicitly disable NLA if FIPS mode is enabled - it won't work */
+            if (guac_fips_enabled()) {
+
+                guac_client_log(client, GUAC_LOG_INFO,
+                        "FIPS mode is enabled. Excluding NLA security mode from security negotiation "
+                        "(see: https://github.com/FreeRDP/FreeRDP/issues/3412).");
+                freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, FALSE);
+
+            }
+
+            /* NLA mode is allowed if FIPS is not enabled */
+            else
+                freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, TRUE);
+
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, FALSE);
+            break;
+
+    }
+
+    /* Security */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_Authentication, !guac_settings->disable_authentication);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_IgnoreCertificate, guac_settings->ignore_certificate);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_AutoAcceptCertificate, guac_settings->certificate_tofu);
+    if (guac_settings->certificate_fingerprints != NULL)
+        freerdp_settings_set_string(rdp_settings, FreeRDP_CertificateAcceptedFingerprints, 
+                guac_strdup(guac_settings->certificate_fingerprints));
+
+
+    /* RemoteApp */
+    if (guac_settings->remote_app != NULL) {
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_Workarea, TRUE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteApplicationMode, TRUE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteAppLanguageBarSupported, TRUE);
+        freerdp_settings_set_string(rdp_settings, FreeRDP_RemoteApplicationProgram, guac_strdup(guac_settings->remote_app));
+        freerdp_settings_set_string(rdp_settings, FreeRDP_ShellWorkingDirectory, guac_strdup(guac_settings->remote_app_dir));
+        freerdp_settings_set_string(rdp_settings, FreeRDP_RemoteApplicationCmdLine, guac_strdup(guac_settings->remote_app_args));
+    }
+
+    /* Preconnection ID */
+    if (guac_settings->preconnection_id != -1) {
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_NegotiateSecurityLayer, FALSE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SendPreconnectionPdu, TRUE);
+        freerdp_settings_set_uint32(rdp_settings, FreeRDP_PreconnectionId, guac_settings->preconnection_id);
+    }
+
+    /* Preconnection BLOB */
+    if (guac_settings->preconnection_blob != NULL) {
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_NegotiateSecurityLayer, FALSE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SendPreconnectionPdu, TRUE);
+        freerdp_settings_set_string(rdp_settings, FreeRDP_PreconnectionBlob, guac_strdup(guac_settings->preconnection_blob));
+    }
+
+    /* Enable use of RD gateway if a gateway hostname is provided */
+    if (guac_settings->gateway_hostname != NULL) {
+
+        /* Enable RD gateway */
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_GatewayEnabled, TRUE);
 
+        /* RD gateway connection details */
+        freerdp_settings_set_string(rdp_settings, FreeRDP_GatewayHostname, guac_strdup(guac_settings->gateway_hostname));
+        freerdp_settings_set_uint32(rdp_settings, FreeRDP_GatewayPort, guac_settings->gateway_port);
+
+        /* RD gateway credentials */
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_GatewayUseSameCredentials, FALSE);
+        freerdp_settings_set_string(rdp_settings, FreeRDP_GatewayDomain, guac_strdup(guac_settings->gateway_domain));
+        freerdp_settings_set_string(rdp_settings, FreeRDP_GatewayUsername, guac_strdup(guac_settings->gateway_username));
+        freerdp_settings_set_string(rdp_settings, FreeRDP_GatewayPassword, guac_strdup(guac_settings->gateway_password));
+
+    }
+
+    /* Store load balance info (and calculate length) if provided */
+    if (guac_settings->load_balance_info != NULL) {
+        freerdp_settings_set_pointer(rdp_settings, FreeRDP_LoadBalanceInfo, (BYTE*) guac_strdup(guac_settings->load_balance_info));
+        freerdp_settings_set_uint32(rdp_settings, FreeRDP_LoadBalanceInfoLength, strlen(guac_settings->load_balance_info));
+    }
+
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_BitmapCacheEnabled, !guac_settings->disable_bitmap_caching);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_OffscreenSupportLevel, !guac_settings->disable_offscreen_caching);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_GlyphSupportLevel, 
+            (!guac_settings->disable_glyph_caching ? GLYPH_SUPPORT_FULL : GLYPH_SUPPORT_NONE));
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_OsMajorType, OSMAJORTYPE_UNSPECIFIED);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_OsMinorType, OSMINORTYPE_UNSPECIFIED);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DesktopResize, TRUE);
+
+    /* Claim support only for specific updates, independent of FreeRDP defaults */
+	BYTE* order_support = freerdp_settings_get_pointer_writable(rdp_settings, FreeRDP_OrderSupport);
+	if (order_support) {
+        ZeroMemory(order_support, GUAC_RDP_ORDER_SUPPORT_LENGTH);
+        order_support[NEG_DSTBLT_INDEX] = TRUE;
+        order_support[NEG_SCRBLT_INDEX] = TRUE;
+        order_support[NEG_MEMBLT_INDEX] = !guac_settings->disable_bitmap_caching;
+        order_support[NEG_MEMBLT_V2_INDEX] = !guac_settings->disable_bitmap_caching;
+        order_support[NEG_GLYPH_INDEX_INDEX] = !guac_settings->disable_glyph_caching;
+        order_support[NEG_FAST_INDEX_INDEX] = !guac_settings->disable_glyph_caching;
+        order_support[NEG_FAST_GLYPH_INDEX] = !guac_settings->disable_glyph_caching;
+    }
+
+#ifdef HAVE_RDPSETTINGS_ALLOWUNANOUNCEDORDERSFROMSERVER

Review Comment:
   Hmm looks like the check was added but a variable was never defined... huh
   
   Added:
   https://github.com/apache/guacamole-server/commit/3b0abe376ef3477c32cf9907e68c9c4dafdaf865#diff-41751abb4e66eadfc6fdba9c3b53799a7a2ae0df8ed89c8f5e7ac7e137e22793
   



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1626657545


##########
src/protocols/rdp/gdi.c:
##########
@@ -412,7 +412,14 @@ BOOL guac_rdp_gdi_surface_frame_marker(rdpContext* context, const SURFACE_FRAME_
 
     guac_rdp_gdi_mark_frame(context, surface_frame_marker->frameAction != SURFACECMD_FRAMEACTION_END);
 
-    if (context->settings->FrameAcknowledge > 0)
+    int frame_acknowledge;
+#ifdef HAVE_SETTERS_GETTERS
+    frame_acknowledge = freerdp_settings_get_uint32(context->settings, FreeRDP_FrameAcknowledge);
+#else
+    frame_acknowledge = context->settings->FrameAcknowledge;
+#endif
+
+    if (frame_acknowledge> 0)

Review Comment:
   Missing space between `frame_acknowledge` and `>`.



##########
Dockerfile:
##########
@@ -145,7 +155,7 @@ RUN ${BUILD_DIR}/src/guacd-docker/bin/build-all.sh
 RUN ${BUILD_DIR}/src/guacd-docker/bin/list-dependencies.sh \
         ${PREFIX_DIR}/sbin/guacd               \
         ${PREFIX_DIR}/lib/libguac-client-*.so  \
-        ${PREFIX_DIR}/lib/freerdp2/*guac*.so   \
+        ${PREFIX_DIR}/lib/freerdp3/*guac*.so   \

Review Comment:
   What happens, here, if someone specifies that they want to build `WITH_FREERDP=2.x`?



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2133499853

   > > Looks like it failed again... https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/2/FREERDP_BRANCH=3.5.1,JENKINS_LABEL_EXPRESSION=ubuntu/
   > 
   > Looks like it doesn't like the attempts to build FreeRDP within the source tree:
   > 
   > ```
   > #8 16.99 CMake Error at cmake/PreventInSourceBuilds.cmake:28 (message):
   > #8 16.99   
   > #8 16.99 
   > #8 16.99   CMake must not to be run in the source directory.  Rather create a
   > #8 16.99   dedicated build directory and run CMake there.  CMake now already created
   > #8 16.99   some files, to clean up after this aborted in-source compilation:
   > #8 16.99 
   > #8 16.99      rm -r CMakeCache.txt CMakeFiles
   > #8 16.99 
   > #8 16.99   or
   > #8 16.99 
   > #8 16.99      git clean -xdf
   > #8 16.99 
   > #8 16.99   
   > #8 16.99 
   > #8 16.99   If you happen to require in-source-tree builds for some reason rerun with
   > #8 16.99   -DALLOW_IN_SOURCE_BUILD=ON
   > ```
   
   I tweaked the build script slightly to create a build_root directory, cd to that, and then cmake from the parent directory. We'll see what happenes.


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "hjzhangr (via GitHub)" <gi...@apache.org>.
hjzhangr commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2650468145

   > No objection from me for inclusion in 1.6.0 either as long as we maintain compatibility with FreeRDP 2.x.
   
   May I ask when the version 1.6.0 is scheduled to be released?


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2650627238

   @hjzhangr : There is no scheduled release date, yet. You can subscribe to the mailing lists to be notified when the release becomes available.
   
   https://guacamole.apache.org/support/#mailing-lists


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "necouchman (via GitHub)" <gi...@apache.org>.
necouchman commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1595594515


##########
configure.ac:
##########
@@ -891,17 +909,162 @@ fi
 
 # Updated certificate verification callback (introduced with 2.0.0, not present
 # in 2.0.0-rc4 or earlier)
-if test "x${have_freerdp2}" = "xyes"
+if test "x${have_freerdp}" = "xyes"
 then
     AC_CHECK_MEMBERS([freerdp.VerifyCertificateEx],,,
-                     [[#include <freerdp/freerdp.h>]])
+            [[#include <freerdp/freerdp.h>]])

Review Comment:
   There seem to be some alignment issues throughout the `configure.ac` file, now. If we're sticking to how things are generally done throughout the file, the start of this should be aligned with the `[freerdp.VerifyCertificateEx]` line above, but it seems to be less indented, now.. There are several of these occurrences throughout the changes in the file.



##########
src/libguac/string.c:
##########
@@ -124,18 +124,30 @@ char* guac_strdup(const char* str) {
     /* Do not attempt to duplicate if the length is somehow magically so
      * obscenely large that it will not be possible to add a null terminator */
     size_t length;
-    if (guac_mem_ckd_add(&length, strlen(str), 1))
+    size_t length_to_copy = strnlen(str, n);
+    if (guac_mem_ckd_add(&length, length_to_copy, 1))
         return NULL;
 
-    /* Otherwise just copy to a new string in same manner as strdup() */
-    void* new_str = guac_mem_alloc(length);
-    if (new_str != NULL)
-        memcpy(new_str, str, length);
+    /* Otherwise just copy to a new string in same manner as strndup() */
+    char* new_str = (char*)guac_mem_alloc(length);
+    if (new_str != NULL) {
+        memcpy(new_str, str, length_to_copy);
+        new_str[length_to_copy] = '\0';

Review Comment:
   This appears to change the behavior of the `guac_strdup()` function, which, previously, did not add a null terminator to the end of the string. I don't know if the previous behavior was intended or not, but it seems like this could cause some issues throughout other parts of the code that depend on the lack of a null terminator, either intentionally or not?
   
   Also, the documentation for the function states that the provided string should already be null-terminated - maybe this should be added only in the case where the new string is shorter than the original one?



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "aleitner (via GitHub)" <gi...@apache.org>.
aleitner commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1596109138


##########
src/protocols/rdp/settings.c:
##########
@@ -1451,11 +1440,279 @@ static int guac_rdp_get_performance_flags(guac_rdp_settings* guac_settings) {
 
 }
 
+int guac_rdp_get_width(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_DesktopWidth);
+#else
+    return rdp->settings->DesktopWidth;
+#endif
+}
+
+int guac_rdp_get_height(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_DesktopHeight);
+#else
+    return rdp->settings->DesktopHeight;
+#endif
+}
+
+int guac_rdp_get_depth(freerdp* rdp) {
+#ifdef HAVE_SETTERS_GETTERS
+    return freerdp_settings_get_uint32(rdp->context->settings, FreeRDP_ColorDepth);
+#else
+    return rdp->settings->ColorDepth;
+#endif
+}
+
 void guac_rdp_push_settings(guac_client* client,
         guac_rdp_settings* guac_settings, freerdp* rdp) {
 
-    rdpSettings* rdp_settings = rdp->settings;
+    rdpSettings* rdp_settings = GUAC_RDP_CONTEXT(rdp)->settings;
+
+#ifdef HAVE_SETTERS_GETTERS
+    /* Authentication */
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Domain, guac_strdup(guac_settings->domain));
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Username, guac_strdup(guac_settings->username));
+    freerdp_settings_set_string(rdp_settings, FreeRDP_Password, guac_strdup(guac_settings->password));
+
+    /* Connection */
+    freerdp_settings_set_string(rdp_settings, FreeRDP_ServerHostname, guac_strdup(guac_settings->hostname));
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_ServerPort, guac_settings->port);
+
+    /* Session */
+
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_DesktopWidth, guac_settings->width);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_DesktopHeight, guac_settings->height);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_ColorDepth, guac_settings->color_depth);
+    freerdp_settings_set_string(rdp_settings, FreeRDP_AlternateShell, guac_strdup(guac_settings->initial_program));
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_KeyboardLayout, guac_settings->server_layout->freerdp_keyboard_layout);
+
+
+    /* Performance flags */
+    /* Explicitly set flag value */
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_PerformanceFlags, guac_rdp_get_performance_flags(guac_settings));
+
+    /* Always request frame markers */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_FrameMarkerCommandEnabled, TRUE);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_SurfaceFrameMarkerEnabled, TRUE);
+
+    /* Enable RemoteFX / Graphics Pipeline */
+    if (guac_settings->enable_gfx) {
+
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SupportGraphicsPipeline, TRUE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteFxCodec, TRUE);
+
+        if (freerdp_settings_get_uint32(rdp_settings, FreeRDP_ColorDepth) != RDP_GFX_REQUIRED_DEPTH) {
+            guac_client_log(client, GUAC_LOG_WARNING, "Ignoring requested "
+                    "color depth of %i bpp, as the RDP Graphics Pipeline "
+                    "requires %i bpp.", freerdp_settings_get_uint32(rdp_settings, FreeRDP_ColorDepth), RDP_GFX_REQUIRED_DEPTH);
+        }
+
+        /* Required for RemoteFX / Graphics Pipeline */
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_FastPathOutput, TRUE);
+        freerdp_settings_set_uint32(rdp_settings, FreeRDP_ColorDepth, RDP_GFX_REQUIRED_DEPTH);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SoftwareGdi, TRUE);
+
+    }
+
+    /* Set individual flags - some FreeRDP versions overwrite the above */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_AllowFontSmoothing, guac_settings->font_smoothing_enabled);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DisableWallpaper, guac_settings->wallpaper_enabled);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DisableFullWindowDrag, guac_settings->full_window_drag_enabled);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DisableMenuAnims, guac_settings->menu_animations_enabled);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DisableThemes, guac_settings->theming_enabled);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_AllowDesktopComposition, guac_settings->desktop_composition_enabled);
+
+    /* Client name */
+    if (guac_settings->client_name != NULL) {
+        freerdp_settings_set_string(rdp_settings, FreeRDP_ClientHostname, 
+                guac_strndup(guac_settings->client_name, RDP_CLIENT_HOSTNAME_SIZE));
+    }
+
+    /* Console */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_ConsoleSession, guac_settings->console);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteConsoleAudio, guac_settings->console_audio);
+
+    /* Audio */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_AudioPlayback, guac_settings->audio_enabled);
+
+    /* Audio capture */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_AudioCapture, guac_settings->enable_audio_input);
+
+    /* Display Update channel */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_SupportDisplayControl, 
+            (guac_settings->resize_method == GUAC_RESIZE_DISPLAY_UPDATE));
+
+    /* Timezone redirection */
+    if (guac_settings->timezone) {
+        if (setenv("TZ", guac_settings->timezone, 1)) {
+            guac_client_log(client, GUAC_LOG_WARNING,
+                "Unable to forward timezone: TZ environment variable "
+                "could not be set: %s", strerror(errno));
+        }
+    }
+
+    /* Device redirection */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DeviceRedirection, 
+            (guac_settings->audio_enabled || guac_settings->drive_enabled || guac_settings->printing_enabled));
+
+    /* Security */
+    switch (guac_settings->security_mode) {
+
+        /* Legacy RDP encryption */
+        case GUAC_SECURITY_RDP:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_UseRdpSecurityLayer, TRUE);
+            freerdp_settings_set_uint32(rdp_settings, FreeRDP_EncryptionLevel, 
+                    ENCRYPTION_LEVEL_CLIENT_COMPATIBLE);
+            freerdp_settings_set_uint32(rdp_settings, FreeRDP_EncryptionMethods, 
+                    ENCRYPTION_METHOD_40BIT | ENCRYPTION_METHOD_128BIT | ENCRYPTION_METHOD_FIPS);
+            break;
+
+        /* TLS encryption */
+        case GUAC_SECURITY_TLS:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, FALSE);
+            break;
+
+        /* Network level authentication */
+        case GUAC_SECURITY_NLA:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, FALSE);
+            break;
+
+        /* Extended network level authentication */
+        case GUAC_SECURITY_EXTENDED_NLA:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, TRUE);
+            break;
+
+        /* Hyper-V "VMConnect" negotiation mode */
+        case GUAC_SECURITY_VMCONNECT:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, FALSE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_VmConnectMode, TRUE);
+            break;
+
+        /* All security types */
+        case GUAC_SECURITY_ANY:
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_RdpSecurity, TRUE);
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_TlsSecurity, TRUE);
+
+            /* Explicitly disable NLA if FIPS mode is enabled - it won't work */
+            if (guac_fips_enabled()) {
+
+                guac_client_log(client, GUAC_LOG_INFO,
+                        "FIPS mode is enabled. Excluding NLA security mode from security negotiation "
+                        "(see: https://github.com/FreeRDP/FreeRDP/issues/3412).");
+                freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, FALSE);
+
+            }
+
+            /* NLA mode is allowed if FIPS is not enabled */
+            else
+                freerdp_settings_set_bool(rdp_settings, FreeRDP_NlaSecurity, TRUE);
+
+            freerdp_settings_set_bool(rdp_settings, FreeRDP_ExtSecurity, FALSE);
+            break;
+
+    }
+
+    /* Security */
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_Authentication, !guac_settings->disable_authentication);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_IgnoreCertificate, guac_settings->ignore_certificate);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_AutoAcceptCertificate, guac_settings->certificate_tofu);
+    if (guac_settings->certificate_fingerprints != NULL)
+        freerdp_settings_set_string(rdp_settings, FreeRDP_CertificateAcceptedFingerprints, 
+                guac_strdup(guac_settings->certificate_fingerprints));
+
+
+    /* RemoteApp */
+    if (guac_settings->remote_app != NULL) {
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_Workarea, TRUE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteApplicationMode, TRUE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_RemoteAppLanguageBarSupported, TRUE);
+        freerdp_settings_set_string(rdp_settings, FreeRDP_RemoteApplicationProgram, guac_strdup(guac_settings->remote_app));
+        freerdp_settings_set_string(rdp_settings, FreeRDP_ShellWorkingDirectory, guac_strdup(guac_settings->remote_app_dir));
+        freerdp_settings_set_string(rdp_settings, FreeRDP_RemoteApplicationCmdLine, guac_strdup(guac_settings->remote_app_args));
+    }
+
+    /* Preconnection ID */
+    if (guac_settings->preconnection_id != -1) {
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_NegotiateSecurityLayer, FALSE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SendPreconnectionPdu, TRUE);
+        freerdp_settings_set_uint32(rdp_settings, FreeRDP_PreconnectionId, guac_settings->preconnection_id);
+    }
+
+    /* Preconnection BLOB */
+    if (guac_settings->preconnection_blob != NULL) {
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_NegotiateSecurityLayer, FALSE);
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_SendPreconnectionPdu, TRUE);
+        freerdp_settings_set_string(rdp_settings, FreeRDP_PreconnectionBlob, guac_strdup(guac_settings->preconnection_blob));
+    }
+
+    /* Enable use of RD gateway if a gateway hostname is provided */
+    if (guac_settings->gateway_hostname != NULL) {
+
+        /* Enable RD gateway */
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_GatewayEnabled, TRUE);
 
+        /* RD gateway connection details */
+        freerdp_settings_set_string(rdp_settings, FreeRDP_GatewayHostname, guac_strdup(guac_settings->gateway_hostname));
+        freerdp_settings_set_uint32(rdp_settings, FreeRDP_GatewayPort, guac_settings->gateway_port);
+
+        /* RD gateway credentials */
+        freerdp_settings_set_bool(rdp_settings, FreeRDP_GatewayUseSameCredentials, FALSE);
+        freerdp_settings_set_string(rdp_settings, FreeRDP_GatewayDomain, guac_strdup(guac_settings->gateway_domain));
+        freerdp_settings_set_string(rdp_settings, FreeRDP_GatewayUsername, guac_strdup(guac_settings->gateway_username));
+        freerdp_settings_set_string(rdp_settings, FreeRDP_GatewayPassword, guac_strdup(guac_settings->gateway_password));
+
+    }
+
+    /* Store load balance info (and calculate length) if provided */
+    if (guac_settings->load_balance_info != NULL) {
+        freerdp_settings_set_pointer(rdp_settings, FreeRDP_LoadBalanceInfo, (BYTE*) guac_strdup(guac_settings->load_balance_info));
+        freerdp_settings_set_uint32(rdp_settings, FreeRDP_LoadBalanceInfoLength, strlen(guac_settings->load_balance_info));
+    }
+
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_BitmapCacheEnabled, !guac_settings->disable_bitmap_caching);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_OffscreenSupportLevel, !guac_settings->disable_offscreen_caching);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_GlyphSupportLevel, 
+            (!guac_settings->disable_glyph_caching ? GLYPH_SUPPORT_FULL : GLYPH_SUPPORT_NONE));
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_OsMajorType, OSMAJORTYPE_UNSPECIFIED);
+    freerdp_settings_set_uint32(rdp_settings, FreeRDP_OsMinorType, OSMINORTYPE_UNSPECIFIED);
+    freerdp_settings_set_bool(rdp_settings, FreeRDP_DesktopResize, TRUE);
+
+    /* Claim support only for specific updates, independent of FreeRDP defaults */
+	BYTE* order_support = freerdp_settings_get_pointer_writable(rdp_settings, FreeRDP_OrderSupport);
+	if (order_support) {
+        ZeroMemory(order_support, GUAC_RDP_ORDER_SUPPORT_LENGTH);
+        order_support[NEG_DSTBLT_INDEX] = TRUE;
+        order_support[NEG_SCRBLT_INDEX] = TRUE;
+        order_support[NEG_MEMBLT_INDEX] = !guac_settings->disable_bitmap_caching;
+        order_support[NEG_MEMBLT_V2_INDEX] = !guac_settings->disable_bitmap_caching;
+        order_support[NEG_GLYPH_INDEX_INDEX] = !guac_settings->disable_glyph_caching;
+        order_support[NEG_FAST_INDEX_INDEX] = !guac_settings->disable_glyph_caching;
+        order_support[NEG_FAST_GLYPH_INDEX] = !guac_settings->disable_glyph_caching;
+    }
+
+#ifdef HAVE_RDPSETTINGS_ALLOWUNANOUNCEDORDERSFROMSERVER

Review Comment:
   ~~Hmm looks like the check was added but a variable was never defined... huh~~
   
   It's defined and set automatically by `AC_CHECK_MEMBERS` (see the resulting config.h)
   
   Added:
   https://github.com/apache/guacamole-server/commit/3b0abe376ef3477c32cf9907e68c9c4dafdaf865#diff-41751abb4e66eadfc6fdba9c3b53799a7a2ae0df8ed89c8f5e7ac7e137e22793
   



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "aleitner (via GitHub)" <gi...@apache.org>.
aleitner commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1596125287


##########
configure.ac:
##########
@@ -891,17 +909,162 @@ fi
 
 # Updated certificate verification callback (introduced with 2.0.0, not present
 # in 2.0.0-rc4 or earlier)
-if test "x${have_freerdp2}" = "xyes"
+if test "x${have_freerdp}" = "xyes"
 then
     AC_CHECK_MEMBERS([freerdp.VerifyCertificateEx],,,
-                     [[#include <freerdp/freerdp.h>]])
+            [[#include <freerdp/freerdp.h>]])

Review Comment:
   Updated formatting



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1595971781


##########
src/libguac/string.c:
##########
@@ -124,18 +124,30 @@ char* guac_strdup(const char* str) {
     /* Do not attempt to duplicate if the length is somehow magically so
      * obscenely large that it will not be possible to add a null terminator */
     size_t length;
-    if (guac_mem_ckd_add(&length, strlen(str), 1))
+    size_t length_to_copy = strnlen(str, n);
+    if (guac_mem_ckd_add(&length, length_to_copy, 1))
         return NULL;
 
-    /* Otherwise just copy to a new string in same manner as strdup() */
-    void* new_str = guac_mem_alloc(length);
-    if (new_str != NULL)
-        memcpy(new_str, str, length);
+    /* Otherwise just copy to a new string in same manner as strndup() */
+    char* new_str = (char*)guac_mem_alloc(length);
+    if (new_str != NULL) {
+        memcpy(new_str, str, length_to_copy);
+        new_str[length_to_copy] = '\0';

Review Comment:
   Well the `strlen()` call would certainly have undefined behavior (including probable segfault) if the input string isn't NULL-terminated. And given the documented requirement that the input must be NULL-terminated, it seems like the options are:
   
   1) Provide invalid non-NULL-terminated input and get undefined results / possible segfault
   2) Provide valid NULL-terminated input and receive a NULL-terminated result.



-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "aleitner (via GitHub)" <gi...@apache.org>.
aleitner commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2127583531

   > > I've added this temporary build to help verify these changes against various 2.x and 3.x versions of FreeRDP:
   > > https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/
   > > The above is identical to our existing build except that it builds against this `RDP-3` branch (not `main`) and includes FreeRDP's `main` and latest 3.x release as additional versions to be tested.
   > 
   > Looks like the FreeRDP 3 test build failed: https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/FREERDP_BRANCH=3.5.1,JENKINS_LABEL_EXPRESSION=ubuntu/lastBuild/console
   
   Huh that shouldn't happen. I just pushed changes a little bit ago to resolve that 


-- 
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-1026: Add support for FreeRDP3. [guacamole-server]

Posted by "jmuehlner (via GitHub)" <gi...@apache.org>.
jmuehlner commented on PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#issuecomment-2127580186

   > I've added this temporary build to help verify these changes against various 2.x and 3.x versions of FreeRDP:
   > 
   > https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/
   > 
   > The above is identical to our existing build except that it builds against this `RDP-3` branch (not `main`) and includes FreeRDP's `main` and latest 3.x release as additional versions to be tested.
   
   Looks like the FreeRDP 3 test build failed: https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/FREERDP_BRANCH=3.5.1,JENKINS_LABEL_EXPRESSION=ubuntu/lastBuild/console


-- 
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