GUACAMOLE-2223: Add require-recording flag#666
Conversation
necouchman
left a comment
There was a problem hiding this comment.
Overall this looks good, and I'm pretty happy with the changes. A couple of things that need to be addressed:
- Consistency of using
intvs.boolfor tracking the setting. We should probably pick one and stick with it - RDP usesint, the other protocols usebool. - Fix the commit message - it should be descriptive and should contain the
GUACAMOLE-2223prefix. - Should this functionality also be added for the Kubernetes protocol?
Also, it might be worth considering what happens if the recording fails during the connection? If this is being added for a case, for example, where Guacamole is being used for audit purposes for Privileged Access Management or the like, it seems like it might also be good to find a way to disrupt the connection if the recording stops in the middle (a disk fills up, for example)?
| /** | ||
| * Non-zero if the connection should be closed when a recording cannot be | ||
| * created. Zero by default, meaning the connection will proceed even if | ||
| * recording fails. | ||
| */ | ||
| bool require_recording; |
There was a problem hiding this comment.
You use "zero" but are using a bool here (instead of int as in the RDP code). It should probably be both consistent and the language in the comments should match the type used.
yes, will need to add the check in https://github.com/apache/guacamole-server/blob/main/src/libguac/socket-tee.c. I'm also thinking of adding a new error code on the frontend that reflects the connection disruption is due to recording failure. |
JIRA: https://issues.apache.org/jira/browse/GUACAMOLE-2223
Naming convention for recording files may not always be unique leading to suffix exhaustion or file creation could fail due to various reasons. This pr adds a new
require-recordingflag to enforce recording file creation when a connection attempt is made.Current behavior:
Connection is allowed even if it can't be recorded due to recording file creation failure.
Expected behavior:
When the session can't be recorded, reject the attempt for compliance.