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

Clone server certificate dynamically #94

Closed
xshill opened this issue Apr 10, 2019 · 7 comments · Fixed by #243
Closed

Clone server certificate dynamically #94

xshill opened this issue Apr 10, 2019 · 7 comments · Fixed by #243
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest
Milestone

Comments

@xshill
Copy link
Collaborator

xshill commented Apr 10, 2019

It would be useful to clone the server's certificate dynamically on connection rather than having to do it manually.

@xshill xshill added the enhancement New feature or request label Apr 10, 2019
@Res260
Copy link
Collaborator

Res260 commented Apr 10, 2019

That would be an option? Since it adds a noticable delay

@xshill
Copy link
Collaborator Author

xshill commented Apr 10, 2019

The delay shouldn't really be noticeable at all, we already connect to the server and get the certificate somehow. It's just a matter of reordering the steps.

@Res260
Copy link
Collaborator

Res260 commented Apr 10, 2019

Ah I thought you meant something more ugly like running the existing cloner python file before lol

@xshill
Copy link
Collaborator Author

xshill commented Apr 10, 2019

😱😱😱

@alxbl alxbl added this to the vNext milestone Aug 17, 2020
@alxbl alxbl self-assigned this Aug 17, 2020
@alxbl
Copy link
Collaborator

alxbl commented Aug 18, 2020

I started looking at this a little bit today. It will require some refactoring and reordering of the connection sequence processing in PyRDP. I'm not quite clear on the scope of work yet, but it might be more complicated than anticipated.

@alxbl
Copy link
Collaborator

alxbl commented Aug 20, 2020

I had managed to re-order the StartTLS order, but ran into other issues with the current layer architecture:

The X224 layer is (purposefully) unaware of what happens below it, which results in the server sending the ConnectConfirm PDU before the client's StartTLS is done, which means that either the PDU is sent before TLS (critical error) or the PDU is never sent, depending on what PyRDP does.

I'll need to look into refactoring the layers so that the TCP layer is aware that a StartTLS is in progress and that messages are buffered until both TLS channels are established. I'm not sure how I'll make this work cleanly yet... to be continued.

@alxbl
Copy link
Collaborator

alxbl commented Sep 3, 2020

After discussing with @obilodeau, we will make this enabled by default.

alxbl added a commit that referenced this issue Sep 14, 2020
Feature(#94) Add Dynamic Certificate Cloning Support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants