erickok / transdroid

Manage your torrents from your Android device
GNU General Public License v3.0
1.28k stars 201 forks source link

Deluge RPC Adapter #407

Closed alonalbert closed 6 years ago

alonalbert commented 6 years ago

Adds a new Deluge Adapter that uses the native RPC API rather than the Web API. Issue #404

This new Adapter does not replace the existing one, it just offers another choice.

The Native RPC API is based on a Secure Socket. Since Transdroid does not maintain an open connection for Adapters, the new Adapter opens a new Socket and logs in (if needed) for each DaemonTask is it executes.

The new adapter depends on a Java Rencode GitHub project aegnor/rencode-java. I could not find a way to add a Gradle dependency so I added the files directly.

The new Adapter shares many constants an a few methods with the old Adapter so a these were refactored into a common class.

alonalbert commented 6 years ago

Hey Eric

Have you had a chance to look at this yet? I've been using it for a few weeks now without issues.

I have another PR pending that adds remote RSS support to Deluge.

erickok commented 6 years ago

Sorry didn't work on it yet (had a crazy week). I want to fix the dependency though, should be straightforward not to have to include the files in the project.

alonalbert commented 6 years ago

Ah, I forgot about the rencode stuff. sg.

alonalbert commented 6 years ago

Removed se.dimovski.rencode files in favor pulling them from jitpack with:

        compile 'com.github.aegnor:rencode-java:-SNAPSHOT'
erickok commented 6 years ago

Hmm I cannot get this to work here locally. Perhaps I am doing something wrong. I have checked (in the running Deluge desktop client) to accept remote connections and left the default port.

Perhaps it is because you forcefully use an SSLSocket via the IgnoreSSLTrustManager: https://github.com/alonalbert/transdroid/blob/deluge-direct-initial/app/src/main/java/org/transdroid/daemon/Deluge/DelugeRpcClient.java#L151

First, this ignores any SSL certificates, am I right? That is a big security issue. Second, it forces SSL which might not strictly be correct. In fact, how would one use an SSL layer with this RPC connection? Via a tunnel or procy?

erickok commented 6 years ago

I tried to replace it with this:

    @NonNull
    private Socket openSocket(DaemonSettings settings) throws NoSuchAlgorithmException, KeyManagementException, IOException {
        if (!settings.getSsl()) {
            // Non-ssl connections
            return new Socket(settings.getAddress(), settings.getPort());
        }
        final TlsSniSocketFactory socketFactory;
        if (settings.getSslTrustKey() != null && settings.getSslTrustKey().length() != 0) {
            socketFactory = new TlsSniSocketFactory(settings.getSslTrustKey());
        } else if (settings.getSslTrustAll()) {
            socketFactory = new TlsSniSocketFactory(true);
        } else {
            socketFactory = new TlsSniSocketFactory();
        }
        return socketFactory.createSocket(null, settings.getAddress(), settings.getPort(), false);
    }

but doesn't seem to connect here. Not sure if it is my setup or if we can make non-SSL connections at all to Deluge?

alonalbert commented 6 years ago

Are you running deluge in Classic mode or as a daemon?

Deluge can run as standalone application (Classic mode) or as a daemon. See Settings/Interface.

When running as a daemon, the UI acts as a thin client using RPC over an SSL Socket. See here on how to set this up.It is this RPC that I use in my Adapter.

I wrote a small pure Java test program that you can play with. I was not able to make it work with Deluge running in Classic mode. I can only make it work with a daemon. I suspect the Classic mode does not expose a listening socket at all. At least, I can't find it.

I don't think the trust manager has anything to do with it. From what I understand, the Deluge daemon uses an SSL socket with a self signed certificate so the "Trust All" manager is required. It's not really a security issue IMO. The connection is still protected and encrypted. All it means is that the client accepts the server for what it is.

The RPC Client I wrote is based on this project (written by the same guy who wrote the Rencoding library)

alonalbert commented 6 years ago

Yeah, looks like the Deluge Daemon certificate is indeed self signed:

  Version: V1
  Subject: CN=Deluge Daemon
  Signature Algorithm: SHA256withRSA, OID = 1.2.840.113549.1.1.11

  Key:  Sun RSA public key, 2048 bits
  modulus:  ....
  public exponent: 65537
  Validity: [From: Mon Jan 08 13:45:28 IST 2018,
               To: Thu Jan 07 13:45:28 IST 2021]
  Issuer: CN=Deluge Daemon
  SerialNumber: [    00]

Issuer: CN=Deluge Daemon

If you really feel strongly about this, I suppose we can check the signature and see that it matches a copy we hold locally.

erickok commented 6 years ago

I do actually, if we can, though I wonder if it would generate a certificate for every running instance, or always use the same for every Deluge daemon?

alonalbert commented 6 years ago

Yeah, I just tested with my 2 servers, they have a different signature. Not sure if it's because it's a different instance or because it's a completely different installation. One runs on my Debian machine and the other on a Synology NAS.

alonalbert commented 6 years ago

It generates a certificate on the fly.

erickok commented 6 years ago

Then I will make it use the same logic as HTTP adapters: a user has to manually accept all certificates if it thinks this is not a security issue, and otherwise it can rely on a manually-entered SHA1 of the certificate or check a real signed certificate (for proxies).

alonalbert commented 6 years ago

Fair enough. You want me to make these changes or you gonna do it?

erickok commented 6 years ago

I am testing now on my alonalbert-deluge-direct-initial branch (I pushed it).

erickok commented 6 years ago

Cool, this works! Going to merge now. Thanks!

tmetro commented 6 years ago

I'm trying to test this, but am running into:

Transdrone 2.5.10 (230) DelugeRpc settings: http://192.168.2.200:58846 ... 67897 -- Tue Feb 20 11:31:33 EST 2018 -- 3 -- ConnectionError exception: null 67898 -- Tue Feb 20 11:39:37 EST 2018 -- 3 -- Boot signal received, starting server and rss checker background services 67899 -- Tue Feb 20 11:41:17 EST 2018 -- 3 -- ConnectionError exception: null

(Could use a bit more info in that error message.)

I've done a few things to troubleshoot, like verified that allow_remote is true and that a process is listening on port 58846 from the LAN IP.

I haven't recently tried connecting to this deluge instance from a remote machine with a client that uses RPC. I also haven't looked into classic vs. daemon mode referenced above.

One possibility is that auth is failing, if the RPC link uses different auth than the web API. (The latter I have set to an empty password.) Or the adapter doesn't support an empty password. Better error logging would help.

Is there a better place for this troubleshooting discussion, seeing as the PR itself has already been accepted. (Neither here nor the ticket seemed like a good place for this.)

erickok commented 6 years ago

You should at least enable ssl and either accept all certificates or enter the SHA1 hash of the generated Deluge certificate.

On 20 Feb 2018 6:05 p.m., "Tom Metro" notifications@github.com wrote:

I'm trying to test this, but am running into:

Transdrone 2.5.10 (230) DelugeRpc settings: http://192.168.2.200:58846 ... 67897 -- Tue Feb 20 11:31:33 EST 2018 -- 3 -- ConnectionError exception: null 67898 -- Tue Feb 20 11:39:37 EST 2018 -- 3 -- Boot signal received, starting server and rss checker background services 67899 -- Tue Feb 20 11:41:17 EST 2018 -- 3 -- ConnectionError exception: null

(Could use a bit more info in that error message.)

I've done a few things to troubleshoot, like verified that allow_remote is true and that a process is listening on port 58846 from the LAN IP.

I haven't recently tried connecting to this deluge instance from a remote machine with a client that uses RPC. I also haven't looked into classic vs. daemon mode referenced above.

One possibility is that auth is failing, if the RPC link uses different auth than the web API. (The latter I have set to an empty password.) Or the adapter doesn't support an empty password. Better error logging would help.

Is there a better place for this troubleshooting discussion, seeing as the PR itself has already been accepted. (Neither here nor the ticket seemed like a good place for this.)

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/erickok/transdroid/pull/407#issuecomment-367046253, or mute the thread https://github.com/notifications/unsubscribe-auth/AAL_ZaC0ORQfdswGhdayAB9cK8SHPr4Wks5tWvs8gaJpZM4RRZ7c .

alonalbert commented 6 years ago

I'll see if I can work in better error messages

Sent from a mobile device

On Tue, Feb 20, 2018, 7:27 PM Eric Kok notifications@github.com wrote:

You should at least enable ssl and either accept all certificates or enter the SHA1 hash of the generated Deluge certificate.

On 20 Feb 2018 6:05 p.m., "Tom Metro" notifications@github.com wrote:

I'm trying to test this, but am running into:

Transdrone 2.5.10 (230) DelugeRpc settings: http://192.168.2.200:58846 ... 67897 -- Tue Feb 20 11:31:33 EST 2018 -- 3 -- ConnectionError exception: null 67898 -- Tue Feb 20 11:39:37 EST 2018 -- 3 -- Boot signal received, starting server and rss checker background services 67899 -- Tue Feb 20 11:41:17 EST 2018 -- 3 -- ConnectionError exception: null

(Could use a bit more info in that error message.)

I've done a few things to troubleshoot, like verified that allow_remote is true and that a process is listening on port 58846 from the LAN IP.

I haven't recently tried connecting to this deluge instance from a remote machine with a client that uses RPC. I also haven't looked into classic vs. daemon mode referenced above.

One possibility is that auth is failing, if the RPC link uses different auth than the web API. (The latter I have set to an empty password.) Or the adapter doesn't support an empty password. Better error logging would help.

Is there a better place for this troubleshooting discussion, seeing as the PR itself has already been accepted. (Neither here nor the ticket seemed like a good place for this.)

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/erickok/transdroid/pull/407#issuecomment-367046253, or mute the thread < https://github.com/notifications/unsubscribe-auth/AAL_ZaC0ORQfdswGhdayAB9cK8SHPr4Wks5tWvs8gaJpZM4RRZ7c

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/erickok/transdroid/pull/407#issuecomment-367053531, or mute the thread https://github.com/notifications/unsubscribe-auth/ABlVkEC9ojqVLL43OMtaaKIM1KGH-W8Sks5tWwCTgaJpZM4RRZ7c .

tmetro commented 6 years ago

alonalbert wrote:

I'll see if I can work in better error messages Thank you.

erickok wrote:

You should at least enable ssl and either accept all certificates...

On first read I wasn't sure you were referring to the client or server side when you say "enable ssl".

I took a look on the server side and found scant documentation covering the SSL setup with respect to the RPC port. FAQs cover SSL setup for the web UI, but not RPC. Eventually I ran across a release note that says that SSL is enabled always for RPC (starting with some release in 2010 I think), and seemed to suggest it uses the same keys found in the ssl/ directory that are auto-generated on install.

Next I dug up my notes on the daemon and found I had set up a user/pass for remote access, so I plugged that into the adapter settings. Unchecked the option I had set earlier to ignore sending login credentials. That still didn't work.

Then going on your SSL advice above, I checked the boxes to enable HTTPS and accept any cert. That worked.

One of the things that threw me off about the HTTPS option is that it doesn't sound related to the RPC transport. I actually have no idea what the RPC transport uses, as the documentation and release notes never seem to mention that. Also, I'm presuming all the references to SSL are also inaccurate, and the transport is actually using TLS.

In any case, given all but nearly decade old version of Deluge require SSL, it seems like that should be at least set by default in the adapter, if not hard coded. As for the language, referring to it as SSL rather than HTTPS would seem to fit the Deluge documentation better.

In my case validating the cert was unnecessary, with both client and server being on a LAN with private IPs. I would guess that rarely will a user be concerned about the threat model of a rogue Deluge server being substituted for their genuine server, and them adding torrents to the rogue server, or having their interactions man-in-the-middle snooped. So I'd probably set the accept any cert as default, but keep the option to upgrade to validation by hash.

Thanks for the suggestions.

Oh, and by the way, the RPC adapter did seem to solve the frequent communications errors I saw when using the web API. So thanks alonalbert for contributing that.

alonalbert commented 6 years ago

I tend to agree that there is little to no risk by accepting the self signed cert. There's really nothing to be gained by an attacker masquerading as my server.

I'll see if I can do something about auto enabling SSL.

Maybe we can do this: If a Deluge RPC adapter is selected, we enable SSL and allow all certificates by default but we show a popup that warns the user that we did that so he can change it if he wants to.

That said, I have a pull request out that does a better job at communicating the errors so users can configure it correctly. #424

alonalbert commented 6 years ago

Just sent #426.