You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2022/08/30 22:43:26 UTC

[GitHub] [guacamole-server] jmuehlner opened a new pull request, #392: GUACAMOLE-1674: Warn about NLA mode if FIPS mode is enabled, or disable if possible.

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

   As discussed in https://issues.apache.org/jira/browse/GUACAMOLE-1674, this change does the following when FIPS mode is active on the guacd server:
   
   - Disables NLA security mode when "any" mode is selected in Guacamole
   - Warns the user that their selected mode is known not to work if they explicitly select NLA security mode
   
   NOTE: Theoretically both TLS and RDP modes _should_ work, though I was not able to get RDP security mode to work with FIPS enabled, either with a Windows or XRDP remote machine.
   
   In practice, this probably won't matter too much, since the security mode negotiation always seems to prefer TLS over RDP anyway. 
   
   I can also disable/warn about RDP mode too, if people think that's a good idea, though I think we should probably just leave it as is for now. Maybe there's some combination of FreeRDP version / remote environment where it will work, though I wasn't able to find it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [guacamole-server] mike-jumper commented on a diff in pull request #392: GUACAMOLE-1674: Warn about NLA mode if FIPS mode is enabled, or disable if possible.

Posted by GitBox <gi...@apache.org>.
mike-jumper commented on code in PR #392:
URL: https://github.com/apache/guacamole-server/pull/392#discussion_r959000647


##########
src/protocols/rdp/settings.c:
##########
@@ -706,12 +707,31 @@ guac_rdp_settings* guac_rdp_parse_args(guac_user* user,
     if (strcmp(argv[IDX_SECURITY], "nla") == 0) {
         guac_user_log(user, GUAC_LOG_INFO, "Security mode: NLA");
         settings->security_mode = GUAC_SECURITY_NLA;
+
+        /*
+         * NLA is known not to work with FIPS; allow the mode selection but
+         * warn that it will not work.
+         */
+        if (guac_fips_enabled())
+            guac_user_log(user, GUAC_LOG_WARNING,
+                    "NLA security mode is selected, "
+                    "but is known not to work, as FIPS mode is enabled.");

Review Comment:
   I think we should additionally note the anticipated effect and, since we have it, the expected solution. For example:
   
   > NLA security mode was selected, but is known to be currently incompatible with FIPS mode (see https://github.com/FreeRDP/FreeRDP/issues/3412). Security negotiation with the RDP server may fail unless TLS security mode is selected instead.



##########
src/protocols/rdp/settings.c:
##########
@@ -1529,7 +1549,20 @@ void guac_rdp_push_settings(guac_client* client,
         case GUAC_SECURITY_ANY:
             rdp_settings->RdpSecurity = TRUE;
             rdp_settings->TlsSecurity = TRUE;
-            rdp_settings->NlaSecurity = guac_settings->username && guac_settings->password;
+
+            /* Explicitly disable NLA if FIPS mode is enabled - it won't work */
+            if (guac_fips_enabled()) {
+
+                guac_client_log(client, GUAC_LOG_WARNING,
+                        "Disabling NLA security mode when FIPS mode is enabled.");
+                rdp_settings->NlaSecurity = FALSE;
+
+            }
+
+            /* Enable NLA security mode if both username and password are set */
+            else
+                rdp_settings->NlaSecurity = guac_settings->username && guac_settings->password;

Review Comment:
   I think this is a holdover from back when we didn't support prompting for credentials and can be safely replaced with just `rdp_settings->NlaSecurity = TRUE;`.



##########
src/protocols/rdp/settings.c:
##########
@@ -1529,7 +1549,20 @@ void guac_rdp_push_settings(guac_client* client,
         case GUAC_SECURITY_ANY:
             rdp_settings->RdpSecurity = TRUE;
             rdp_settings->TlsSecurity = TRUE;
-            rdp_settings->NlaSecurity = guac_settings->username && guac_settings->password;
+
+            /* Explicitly disable NLA if FIPS mode is enabled - it won't work */
+            if (guac_fips_enabled()) {
+
+                guac_client_log(client, GUAC_LOG_WARNING,
+                        "Disabling NLA security mode when FIPS mode is enabled.");

Review Comment:
   1. This should be at the info level. Nothing is failing or anticipated to fail here (yet); it's just a noteworthy piece of information.
   2. As written, this is always true. I suggest rephrasing to explicitly note the recognition of system state and the action taken: `FIPS mode is enabled. Excluding NLA security mode from security negotiation (see: https://github.com/FreeRDP/FreeRDP/issues/3412).`



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


[GitHub] [guacamole-server] jmuehlner commented on a diff in pull request #392: GUACAMOLE-1674: Warn about NLA mode if FIPS mode is enabled, or disable if possible.

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #392:
URL: https://github.com/apache/guacamole-server/pull/392#discussion_r959007643


##########
src/protocols/rdp/settings.c:
##########
@@ -1529,7 +1549,20 @@ void guac_rdp_push_settings(guac_client* client,
         case GUAC_SECURITY_ANY:
             rdp_settings->RdpSecurity = TRUE;
             rdp_settings->TlsSecurity = TRUE;
-            rdp_settings->NlaSecurity = guac_settings->username && guac_settings->password;
+
+            /* Explicitly disable NLA if FIPS mode is enabled - it won't work */
+            if (guac_fips_enabled()) {
+
+                guac_client_log(client, GUAC_LOG_WARNING,
+                        "Disabling NLA security mode when FIPS mode is enabled.");

Review Comment:
   Ah oops, I had updated this to be INFO, but that change got lost somewhere. I'll put it back.



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


[GitHub] [guacamole-server] jmuehlner commented on a diff in pull request #392: GUACAMOLE-1674: Warn about NLA mode if FIPS mode is enabled, or disable if possible.

Posted by GitBox <gi...@apache.org>.
jmuehlner commented on code in PR #392:
URL: https://github.com/apache/guacamole-server/pull/392#discussion_r959005258


##########
src/protocols/rdp/settings.c:
##########
@@ -706,12 +707,31 @@ guac_rdp_settings* guac_rdp_parse_args(guac_user* user,
     if (strcmp(argv[IDX_SECURITY], "nla") == 0) {
         guac_user_log(user, GUAC_LOG_INFO, "Security mode: NLA");
         settings->security_mode = GUAC_SECURITY_NLA;
+
+        /*
+         * NLA is known not to work with FIPS; allow the mode selection but
+         * warn that it will not work.
+         */
+        if (guac_fips_enabled())
+            guac_user_log(user, GUAC_LOG_WARNING,
+                    "NLA security mode is selected, "
+                    "but is known not to work, as FIPS mode is enabled.");

Review Comment:
   Sure



##########
src/protocols/rdp/settings.c:
##########
@@ -1529,7 +1549,20 @@ void guac_rdp_push_settings(guac_client* client,
         case GUAC_SECURITY_ANY:
             rdp_settings->RdpSecurity = TRUE;
             rdp_settings->TlsSecurity = TRUE;
-            rdp_settings->NlaSecurity = guac_settings->username && guac_settings->password;
+
+            /* Explicitly disable NLA if FIPS mode is enabled - it won't work */
+            if (guac_fips_enabled()) {
+
+                guac_client_log(client, GUAC_LOG_WARNING,
+                        "Disabling NLA security mode when FIPS mode is enabled.");
+                rdp_settings->NlaSecurity = FALSE;
+
+            }
+
+            /* Enable NLA security mode if both username and password are set */
+            else
+                rdp_settings->NlaSecurity = guac_settings->username && guac_settings->password;

Review Comment:
   Ah ok



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


[GitHub] [guacamole-server] mike-jumper merged pull request #392: GUACAMOLE-1674: Warn about NLA mode if FIPS mode is enabled, or disable if possible.

Posted by GitBox <gi...@apache.org>.
mike-jumper merged PR #392:
URL: https://github.com/apache/guacamole-server/pull/392


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