Skip to content

Conversation

vtnerd
Copy link
Contributor

@vtnerd vtnerd commented Sep 19, 2023

First, my apologies to @moneromooo-monero for NACKing his/her SSL proposal years ago, only for me to bring it back.


This does exactly as the title suggests, with options to disable P2P encryption entirely, and an option to re-use a SSL certificate between "runs". The default generates a new SSL certificate each monerod run, so that the node cannot be tracked across IP address changes.

Nodes do not trust encryption information from peers. Instead, every peer is assumed to be in autodetect mode, unless overridden on the CLI or via handshake/ping messages directly from the peer.

Possibly bad:
If a node chooses to re-use SSL certificates, a change in certificates will cause connection failures until the node is removed from the white+gray lists OR the node makes a direct connection and provides the new SSL certificate.


EDIT:
I will also attempt some unit test changes; there is enough to review here that I expect this diff to be up a while.

@vtnerd vtnerd force-pushed the feature/p2p_ssl_squashed branch from 22ca764 to 5208f25 Compare October 6, 2023 20:30
@vtnerd
Copy link
Contributor Author

vtnerd commented Oct 6, 2023

Added sodium to the dependency list, hopefully builds pass now.

@vtnerd vtnerd force-pushed the feature/p2p_ssl_squashed branch from 5208f25 to d8e410c Compare November 16, 2023 17:33
@vtnerd
Copy link
Contributor Author

vtnerd commented Nov 16, 2023

Force pushed a change based on my last comment (ssl_support_t doesn't specify numeric values).

@vtnerd vtnerd force-pushed the feature/p2p_ssl_squashed branch from d8e410c to 6f1e246 Compare November 16, 2023 17:35
@vtnerd
Copy link
Contributor Author

vtnerd commented Nov 16, 2023

Did another force-push with a rebase, to get rid of the external changes that were not intended.

@vtnerd vtnerd force-pushed the feature/p2p_ssl_squashed branch 3 times, most recently from af7d5c0 to 5d35a9a Compare November 18, 2023 22:22
@vtnerd
Copy link
Contributor Author

vtnerd commented Nov 19, 2023

I added a crude functional test for SSL. It just tests that autodetection and disabling SSL works.

@j-berman
Copy link
Collaborator

j-berman commented Apr 19, 2024

Started a review and have an initial comment. I didn't get around to fixing connect_async as we discussed over here, and that needs to be done for this PR now that connect_async can get called with an SSL setting.

Running this code on a node that can get incoming connections, you can see that the back ping to an incoming node always fails (log level 2 you'll see back ping invoke failed, and no PING SUCCESS) after an incoming node connects to you.

I'm happy to do this since I said I would over in that PR, my bad for not getting to it sooner.

@vtnerd vtnerd force-pushed the feature/p2p_ssl_squashed branch from 5d35a9a to da4fc6c Compare April 28, 2024 21:04
@vtnerd
Copy link
Contributor Author

vtnerd commented Apr 28, 2024

@j-berman Fixed SSL connections with connect_async.

@vtnerd vtnerd force-pushed the feature/p2p_ssl_squashed branch from da4fc6c to 3d88cc0 Compare April 28, 2024 21:24
@vtnerd
Copy link
Contributor Author

vtnerd commented Apr 28, 2024

Forgot to include newest code in last push. Trying again!

@vtnerd vtnerd force-pushed the feature/p2p_ssl_squashed branch from 3d88cc0 to a7c49c9 Compare April 29, 2024 02:03
@vtnerd
Copy link
Contributor Author

vtnerd commented Apr 29, 2024

Rebased, and trying to fix a linking issue that I cannot reproduce locally.

@SyntheticBird45
Copy link

Please excuse my ignorance, I'm not familiar with the boost library and I'm therefore wondering what cipher are actually available ? afaiu, SSL namespace include TLS up to TLSv1.3: https://www.boost.org/doc/libs/1_74_0/boost/asio/ssl/context_base.hpp

I'm probably blind but i don't find in the commit changes where are cipher options. That would be good to enforce at least TLSv1.2

@vtnerd
Copy link
Contributor Author

vtnerd commented Jun 17, 2024

The code already forces TLS 1.2+. It also limits the cipher list a few lines below that.

@vtnerd vtnerd force-pushed the feature/p2p_ssl_squashed branch from a7c49c9 to b3c8322 Compare June 17, 2024 23:25
@vtnerd
Copy link
Contributor Author

vtnerd commented Jun 18, 2024

Bad rebase, will fix.

@vtnerd vtnerd force-pushed the feature/p2p_ssl_squashed branch from b3c8322 to 60925f1 Compare June 18, 2024 02:44
@vtnerd
Copy link
Contributor Author

vtnerd commented Jun 18, 2024

The test that just failed should be spurious.

@vtnerd
Copy link
Contributor Author

vtnerd commented Jun 18, 2024

I rebased against latest master changes, which included a test merge and a seed node merge.

@vtnerd
Copy link
Contributor Author

vtnerd commented Mar 3, 2025

Force pushed a rebase. Nothing should have changed. Looking at the diff again.

@vtnerd vtnerd force-pushed the feature/p2p_ssl_squashed branch from 0b18c43 to 0bc4259 Compare March 3, 2025 01:22
@vtnerd
Copy link
Contributor Author

vtnerd commented Mar 3, 2025

Force pushed another small update removing some unintended changes.

@vtnerd
Copy link
Contributor Author

vtnerd commented Mar 3, 2025

The failed test is not related. May push a dummy update so that all unit tests get a chance to run.

@vtnerd
Copy link
Contributor Author

vtnerd commented Aug 20, 2025

Force pushed a rebase merge for p2p functional tests.

@vtnerd vtnerd force-pushed the feature/p2p_ssl_squashed branch from eb47b3f to c814bf3 Compare August 29, 2025 21:29
@vtnerd vtnerd force-pushed the feature/p2p_ssl_squashed branch from c814bf3 to ba7a086 Compare August 30, 2025 21:27
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.

3 participants