Add Opportunistic TLS implementation #302

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

Jump to bottom

Open

WyriHaximus wants to merge 1 commit into reactphp:1.x

from WyriHaximus-labs:1.x-opportunistic-tls-connection

Open

Add Opportunistic TLS implementation #302

WyriHaximus wants to merge 1 commit into reactphp:1.x from WyriHaximus-labs:1.x-opportunistic-tls-connection

Conversation

@WyriHaximus

Copy link

Member

@WyriHaximus WyriHaximus commented Dec 3, 2022 •

edited

Loading

This PR introduces the functionality required to build opportunistic TLS clients and servers with ReactPHP. It does so by introducing a prefix to tls://, namely opportunistic, to create opportunistic+tls://example.com:5432 for example as the full URL. This will create an OpportunisticTlsConnectionInterface (instead of a ConnectionInterface) that extends the ConnectionInterface and exposes the enableEncryption method to enable TLS encryption at the desired moment. Inside this PR is an example of a server and client negotiating when to enable TLS and enable it when ready.

Opportunistic Security described in RFC7435: https://www.rfc-editor.org/rfc/rfc7435
External PR using the proposed changes in this PR: voryx/PgAsync#52

@WyriHaximus WyriHaximus added this to the v1.13.0 milestone

Dec 3, 2022

@WyriHaximus WyriHaximus force-pushed the 1.x-opportunistic-tls-connection branch 5 times, most recently from c45aa34 to 6bf2080 Compare

December 4, 2022 16:09

@WyriHaximus WyriHaximus mentioned this pull request

Dec 4, 2022

[WIP] Cleanly stop() loop when awaiting promise without fiber reactphp/async#65

Open

@WyriHaximus WyriHaximus force-pushed the 1.x-opportunistic-tls-connection branch from 6bf2080 to 454673d Compare

December 5, 2022 10:56

@WyriHaximus WyriHaximus added the new feature label

Dec 5, 2022

@WyriHaximus WyriHaximus force-pushed the 1.x-opportunistic-tls-connection branch 2 times, most recently from da6bc3c to 1c1cb5a Compare

December 5, 2022 16:02

@WyriHaximus WyriHaximus marked this pull request as ready for review

December 5, 2022 16:02

@WyriHaximus WyriHaximus requested review from SimonFrings and clue

December 5, 2022 16:02

@WyriHaximus WyriHaximus force-pushed the 1.x-opportunistic-tls-connection branch 4 times, most recently from 0275788 to 5216b9e Compare

December 7, 2022 18:15

@WyriHaximus

Copy link

Member Author

WyriHaximus commented Dec 7, 2022

Thanks to a suggestion from @clue I managed to drop the dependency on reactphp/async#65

SimonFrings

SimonFrings reviewed

Dec 15, 2022

View reviewed changes

Copy link

Member

@SimonFrings SimonFrings left a comment

Choose a reason for hiding this comment

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

I added some remarks and suggestions for the documentation, same suggestions are also important for the doc-blocks inside the added classes.

README.md Outdated Show resolved Hide resolved

@WyriHaximus WyriHaximus force-pushed the 1.x-opportunistic-tls-connection branch from 5216b9e to f86e4f8 Compare

December 16, 2022 09:15

@WyriHaximus WyriHaximus requested a review from SimonFrings

December 17, 2022 12:16

@WyriHaximus

Copy link

Member Author

WyriHaximus commented Dec 17, 2022

I added some remarks and suggestions for the documentation, same suggestions are also important for the doc-blocks inside the added classes.

@SimonFrings Let me know if you have any more pointers? Will apply them to the docblocks later today

@WyriHaximus


 Add Opportunistic TLS implementation

fb5c2e7

This commit introduces the functionality required to build opportunistic TLS clients and servers with
ReactPHP. It does so by introducing a prefix to `tls://`, namely `opportunistic`, to create
`opportunistic+tls://example.com:5432` for example as the full URL. This will create an
`OpportunisticTlsConnectionInterface` (instead of a `ConnectionInterface`) that extends the
`ConnectionInterface` and exposes the `enableEncryption` method to enable TLS encryption at the
desired moment. Inside this PR is an example of a server and client negotiating when to enable TLS
and enable it when ready.
Opportunistic Security described in RFC7435: https://www.rfc-editor.org/rfc/rfc7435
External PR using the proposed changes in this commit: voryx/PgAsync#52

@WyriHaximus WyriHaximus force-pushed the 1.x-opportunistic-tls-connection branch from f86e4f8 to fb5c2e7 Compare

December 20, 2022 09:50

@WyriHaximus

Copy link

Member Author

WyriHaximus commented Dec 20, 2022

@SimonFrings Updated the doc blocks

SimonFrings

SimonFrings approved these changes

Jan 2, 2023

View reviewed changes

Copy link

Member

@SimonFrings SimonFrings left a comment

Choose a reason for hiding this comment

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

Thanks for updating, texts and the overall code structure are looking fine 👍

I am not very experienced when it comes to opportunistic TLS, so I can't really review if this is exactly the way to introduce this to the project, but the rest looks good to me.

@WyriHaximus

Copy link

Member Author

WyriHaximus commented Jan 2, 2023

Thanks for updating, texts and the overall code structure are looking fine 👍

👍

I am not very experienced when it comes to opportunistic TLS, so I can't really review if this is exactly the way to introduce this to the project, but the rest looks good to me.

The only reason I found out STARTSSL is a flavor of opportunistic TLS is because I started working on voryx/PgAsync#52, did a writedown of the why at https://blog.wyrihaximus.net/2023/01/migrating-from-self-hosted-in-k8s-databases-to-managed-hosted-at-digital-ocean/. That is also why I started with STARTTLS and ended up with opportunistic TLS just providing the barebones to do it and not implement some standard on top of it.