libtorrent icon indicating copy to clipboard operation
libtorrent copied to clipboard

Add `root_certificate` to `add_torrent_params`.

Open jameskeane opened this issue 9 months ago • 12 comments

This is so that a peer can join an ssl swarm with only a signed cert and infohash. Fixes #2192

@arvidn I couldn't get the simulator building with openssl, so I have no real way of testing this right now. Any guidance on how to solve that?

jameskeane avatar Mar 17 '25 19:03 jameskeane

unfortunately the simulator doesn't support the SSL wrappers in boost.asio (yet).

arvidn avatar Mar 23 '25 23:03 arvidn

There is a test for the SSL feature already, which seem the most natural one to extend (test/test_ssl.cpp).

What happens if this field is set on a magnet link but the torrent turns out not to be an SSL torrent? (it should ideally be documented in the comment).

Should the root certificate also be allowed to be passed on the magnet link URI as well?

arvidn avatar Mar 23 '25 23:03 arvidn

Added a rudimentary test for success using metadata plugin in test_ssl.cpp.

jameskeane avatar Mar 30 '25 00:03 jameskeane

What happens if this field is set on a magnet link but the torrent turns out not to be an SSL torrent?

I could see it going both ways:

  1. If the user specifies a root CA; their intent is to use an encrypted channel so it should error.
  2. The user intends to download a torrent at the specified infohash, download it even if the certificate is not necessary.

What is preferable to you?

Another edge case is what if the root CAs don't match. Mismatch from either torrent file vs. add params, or metadata exchange.

jameskeane avatar Mar 30 '25 00:03 jameskeane

Should the root certificate also be allowed to be passed on the magnet link URI as well?

I don't see why not, the vast majority of root certs would fit in the 2048 character limit of a URL.

jameskeane avatar Mar 30 '25 01:03 jameskeane

I think the safest approach is to require it to be an SSL torrent if the certificate is specified. i.e. failing closed.

arvidn avatar Apr 20 '25 09:04 arvidn

I think this change warrants a note in Changelog too

arvidn avatar Apr 20 '25 09:04 arvidn

Added a testcase for when the seeding torrent does not have a certificate. It was already correct behaviour, as the handshake failed during peer connection before metadata exchange.

Only outstanding thing is the python binding, is there a guide on building and testing it?

Looks like support for magnet links will require:

  1. Storing the root certificate on torrent or exposing a method on ssl_context to return the root certificate in encoded form (PEM?).
  2. Modifying torrent_handle and torrent to expose that.
  3. Adding support to make_magnet_uri, which will need to check the URL length and fail if the certificate would exceed the max length. Also error if is_ssl_torrent and no root certificate is set.
  4. Adding support to parse_magnet_uri.
  5. Associated tests.

I'm happy to take a stab at that, but maybe in a separate PR? Let me know your thoughts.

jameskeane avatar Apr 23 '25 03:04 jameskeane

Had to remove the message check on the peer_disconnected_alert as the handshake failed message is different under gnussl and windows.

jameskeane avatar Apr 29 '25 02:04 jameskeane

Extending add_torrent_params affects ABI. From the library's point of view, there's no way of telling whether the new or the old version of the struct was passed. This change is safe to make in master

arvidn avatar May 11 '25 07:05 arvidn

Updated the target branch to master.

jameskeane avatar May 24 '25 03:05 jameskeane

@arvidn Should be good to go, fixed failing build and tests are passing locally.

jameskeane avatar Jun 05 '25 21:06 jameskeane