Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GUACAMOLE-1910: Add locking around TLS socket #482

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

neandrake
Copy link
Contributor

When TLS is turned on the file descriptor for the socket does not use locking when writing to it. It results in corrupted guacamole instructions being written to the stream when there are multiple connected clients.

This adds a mutex in the same manner done in socket-fd.c.

Copy link
Contributor

@mike-jumper mike-jumper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Mind rebasing against staging/1.5.5 so this can be pulled into the upcoming 1.5.5 release?

When TLS is turned on the file descriptor for the socket does not use
locking when writing to it. It results in corrupted guacamole
instructions being written to the stream when there are multiple
connected clients.

This adds a mutex in the same manner done in `socket-fd.c`.
@neandrake neandrake changed the base branch from master to staging/1.5.5 February 16, 2024 18:22
@neandrake
Copy link
Contributor Author

neandrake commented Feb 16, 2024

LGTM! Mind rebasing against staging/1.5.5 so this can be pulled into the upcoming 1.5.5 release?

Okay I think I rebased it properly. Please take a look.

I pulled upstream/staging/1.5.5, rebased GUAC-1910 on to it and force-pushed to my forked branch. I then updated this pull request to merge onto staging/1.5.5 instead of master.

There were no conflicts (I did not expect any based on the changes to guacamole-server), so maybe there was an easier way to do this.

@neandrake
Copy link
Contributor Author

Do you prefer bringing up issues in the mailing list prior to filing an issue on jira and submitting a pull request?

@mike-jumper
Copy link
Contributor

mike-jumper commented Feb 16, 2024

When the issue is something that may be configuration-related and not actually a bug, we definitely prefer mailing list first. For something like this where it's clear that we're missing a lock, it makes sense to just open up the JIRA issue directly.

@mike-jumper mike-jumper merged commit 06a9aa9 into apache:staging/1.5.5 Feb 16, 2024
1 check passed
@neandrake neandrake deleted the GUAC-1910 branch February 21, 2024 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants