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

Where to reinstate SSL verification spec? #169

Open
grapland0 opened this issue Nov 29, 2023 · 4 comments · May be fixed by #176
Open

Where to reinstate SSL verification spec? #169

grapland0 opened this issue Nov 29, 2023 · 4 comments · May be fixed by #176

Comments

@grapland0
Copy link

in example/cpp20_intro_tls.cpp, it does

conn->next_layer().set_verify_mode(asio::ssl::verify_peer);
conn->next_layer().set_verify_callback(verify_certificate);

before async_exec.

I wonder whether this survives auto-reconnect? If not, where should I reinstate these for new connections?

@mzimbres
Copy link
Collaborator

mzimbres commented Dec 1, 2023

Reconnection is unfinished for ssl. I am think about adding a callback that will be called after this. Until there you can disable reconnection by setting the this to zero writing yourself the loop from the link above, it is easier if you are using coroutines.

@grapland0
Copy link
Author

grapland0 commented Dec 1, 2023

Before the reconnection ssl support is completed, can we let it throw an exception if users enable both reconnection and SSL?

Current implementation may introduce a vulnerability as:

step 1, the MITM forces the client to drop a connection.
step 2, MITM starts to intercept all connection from client to server, with a self-signed certificate.
step 3, boost.redis attempts to reconnect with refreshed default ssl modes, which in some implementation, to be ssl::verify_none equivalently.
step 4, connection won't fail by the forged certificate. then the MITM can monitor the traffic on this connection.

@mzimbres mzimbres linked a pull request Jan 4, 2024 that will close this issue
@mzimbres
Copy link
Collaborator

mzimbres commented Jan 4, 2024

I have opened a PR with the fix. Please have a look.

@mzimbres
Copy link
Collaborator

mzimbres commented Jan 4, 2024

@anarthal pointed out that if you set your options on the ssl::context, all subsequent connections created from it will use the new configuration. That would make my PR above unnecessary. @grapland0 Do you have a use case where setting options in the context directly is not possible?

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 a pull request may close this issue.

2 participants