monero icon indicating copy to clipboard operation
monero copied to clipboard

Add SSL support to P2P

Open vtnerd opened this issue 1 year ago • 13 comments

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 avatar Sep 19 '23 23:09 vtnerd

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

vtnerd avatar Oct 06 '23 20:10 vtnerd

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

vtnerd avatar Nov 16 '23 17:11 vtnerd

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

vtnerd avatar Nov 16 '23 17:11 vtnerd

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

vtnerd avatar Nov 19 '23 00:11 vtnerd

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.

j-berman avatar Apr 19 '24 00:04 j-berman

@j-berman Fixed SSL connections with connect_async.

vtnerd avatar Apr 28 '24 21:04 vtnerd

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

vtnerd avatar Apr 28 '24 21:04 vtnerd

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

vtnerd avatar Apr 29 '24 02:04 vtnerd

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

SyntheticBird45 avatar Jun 17 '24 17:06 SyntheticBird45

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

vtnerd avatar Jun 17 '24 21:06 vtnerd

Bad rebase, will fix.

vtnerd avatar Jun 18 '24 01:06 vtnerd

The test that just failed should be spurious.

vtnerd avatar Jun 18 '24 14:06 vtnerd

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

vtnerd avatar Jun 18 '24 14:06 vtnerd