Add a terminal-type parameter for SSH and Telnet connections, to specify
the terminal emulator type that is passed to programs. If not specified,
the default type of "linux" is used in keep with existing behavior.
Root Cause:
See the core dump and Valgrind report posted on Jira. guacd was reading a ssh terminal which had been freed. When a ssh connection is terminated, guac_ssh_client_free_handler() will be called from guacd_exec_proc() -> guac_client_free() with pointer client->free_handler. In guac_ssh_client_free_handler(), when ssh_client->term is freed, ssh_client->client_thread may still be using the ssh_client->term. It causes the crash reported in this bug.
The stack trace exposing the problem can be found by running guacd under Valgrind with a ssh test script. The test script repeats doing ssh login and logout for 5000 times.
Solution:
In guac_ssh_client_free_handler(), before calling guac_terminal_free(ssh_client->term), close the stdin pipe of the terminal to stop reading the pipe with guac_terminal_read_stdin() in ssh_input_thread(). So that ssh_input_thread() can be terminated in this case. Call pthread_join() to wait for ssh_client_thread() terminating before freeing the terminal.
Add a new function guac_terminal_stop() to close the pipe and set the fds to invalid (-1). Call it in guac_ssh_client_free_handler() and guac_terminal_free().
Checking the client running state in ssh_input_thread() and ssh_client_thread() to make sure they can be terminated when the client is stopped in guacd_exec_proc() by another thread.
Test:
- Confirmed ssh connection works normally.
- Observed the child process of guacd exits when ssh connection is terminated.
- Reran the ssh test script. Observed no crash.
Root Cause:
In the ssh library of guacd, function ssh_client_thread(), when guac_ssh_get_user() fails to load private key for ssh authentication, it will return NULL. In this case, the subsequent call to guac_common_ssh_create_session() with parameter 'user=0x0' will cause guacd crash in function guac_common_ssh_authenticate() by accessing 'user->username'.
Solution:
- Update the comment of function guac_ssh_get_user() to document that NULL will be returned if fails to import key for the user.
- In function ssh_client_thread(), verify the return of guac_ssh_get_user(). If ssh_client->user is NULL, return NULL.
Test:
- Configured a ssh app with an encrypted private key and a wrong passphrase.
- Ran the ssh app from web portal and observed guacd crash.
- Applied the fix and reran the ssh app. Observed no crash.
