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

[v9] Backport #12891 #13391

Merged
merged 7 commits into from
Jun 14, 2022
Merged

[v9] Backport #12891 #13391

merged 7 commits into from
Jun 14, 2022

Conversation

xacrimon
Copy link
Contributor

Backport of #12891. This was a bit of a messy backport with some surgery involved. Tests pass and it seems to work when tested with AD.

Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com>
Co-authored-by: Zac Bergquist <zmb3@users.noreply.github.com>
@xacrimon xacrimon marked this pull request as ready for review June 10, 2022 20:52
@zmb3
Copy link
Collaborator

zmb3 commented Jun 13, 2022

I tried to run this and couldn't make a connection to the desktop.

Logs show:

[2022-06-13T14:48:26Z WARN  rustls::check] Received a ServerHelloDone handshake message while expecting [CertificateRequest]

I confirmed that connecting to the same desktop with master (4742327) and branch/v9 (197f06a) works fine.

@xacrimon @LKozlowski could you take a look?

@xacrimon
Copy link
Contributor Author

xacrimon commented Jun 13, 2022

@zmb3 Noting this for context, said log message is a bug in Rustls that is somehow spurious since it's valid to receive a ServerHelloDone before a CertificateRequest, see rustls/rustls#1012 for details. Do you see anything else in the logs that could be relevant?

@zmb3
Copy link
Collaborator

zmb3 commented Jun 13, 2022

Interesting - I went back to this branch and tried again and now it is working, and I do not see this rustls message in the logs when it works.

Plus, based on the changes here, I wouldn't expect these to cause issues with rustls.

Is our rustls out of date by chance though? Maybe an update could help?

@xacrimon
Copy link
Contributor Author

xacrimon commented Jun 13, 2022

@zmb3 As expected this is a transient issue due to rustls. I went ahead and made a semver compatible update of all deps using cargo update. It did update rustls from 0.20.4 to 0.20.6 which fixes this according to patchnotes. Should be good to go now.

@xacrimon
Copy link
Contributor Author

Does the unicode clipboard work too?

@zmb3
Copy link
Collaborator

zmb3 commented Jun 13, 2022

@xacrimon Yep, works a charm.

@zmb3
Copy link
Collaborator

zmb3 commented Jun 13, 2022

@xacrimon sorry, one more thing - since this was already merged to master, our Rust deps are probably out of sync. Would you mind opening a PR to master as well with the cargo update results?

@xacrimon
Copy link
Contributor Author

Yep, @zmb3 could you approve this though?

@xacrimon xacrimon merged commit aa59d88 into branch/v9 Jun 14, 2022
@zmb3 zmb3 deleted the joel/v9-backport-12891 branch June 15, 2022 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants