necouchman commented on a change in pull request #321:
URL: https://github.com/apache/guacamole-server/pull/321#discussion_r549847653
##########
File path: src/protocols/vnc/vnc.c
##########
@@ -133,6 +139,18 @@ rfbClient* guac_vnc_get_client(guac_client* client) {
/* TLS Locking and Unlocking */
rfb_client->LockWriteToTLS = guac_vnc_lock_write_to_tls;
rfb_client->UnlockWriteToTLS = guac_vnc_unlock_write_to_tls;
+#endif
+
+#ifdef LIBVNCSERVER_WITH_CLIENT_GCRYPT
+
+ guac_client_log(client, GUAC_LOG_DEBUG, "GCrypt initialization started.");
+ gcry_check_version(NULL);
+ gcry_control(GCRYCTL_SUSPEND_SECMEM_WARN);
+ gcry_control(GCRYCTL_INIT_SECMEM, 16384, 0);
+ gcry_control(GCRYCTL_RESUME_SECMEM_WARN);
+ gcry_control(GCRYCTL_INITIALIZATION_FINISHED, 0);
+ guac_client_log(client, GUAC_LOG_DEBUG, "GCrypt initialization
completed.");
Review comment:
* At this point I'm not sure if there's a reason to go with
`SOMETHING_SPECIFIC` versus `NULL` - unfortunately the documentation on LibVNC
integration with GCrypt seems to be sparse, and I'm not sure if there's a
specific version of GCrypt that is required for LibVNC to function correctly.
The scant examples I can find (vino from Gnome, for example) just use `NULL`.
I'm happy to just stick with `GCRYPT_VERSION` for consistency, if that is
preferred. However, I'm not sure that necessarily accomplishes anything useful,
as it just checks to make sure that the version of GCrypt that you built
against is identical to the version you're running with. Maybe that's
worthwhile, maybe not? My impression is that specifying a version is more for
the purpose of making sure that GCrypt supports the features/algorithms you
need for a particular application?
* The GCrypt documentation on initializing the library
(https://www.gnupg.org/documentation/manuals/gcrypt/Initializing-the-library.html)
talks about initializing secure memory to protect things if the memory gets
swapped out to disk. I don't know how much of a concern this is - for either
VNC or SSH - it was in the documentation, so I went with it.
* Not sure if there are adverse effects of initializing the library twice,
but there is a method for checking on status of initialization, so it's
probably worth just doing that to make sure it hasn't already been initialized.
That said, I'm not sure what the implications are of initializing it in one
place with the thread-safe options (SSH) and not doing so in another place
(VNC). So, I might need some guidance on how that should be handled, since one
initialization is in the common-ssh code and the other in the VNC protocol
code. My familiarity with GCrypt is very, very limited...
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
