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

Add HTTPS support for remote session #1294

Closed
HotMykeul opened this issue Jun 4, 2020 · 7 comments · Fixed by #4622
Closed

Add HTTPS support for remote session #1294

HotMykeul opened this issue Jun 4, 2020 · 7 comments · Fixed by #4622

Comments

@HotMykeul
Copy link

HotMykeul commented Jun 4, 2020

I can't connect to my secure webgui transmission through transmission.

Could you please add a fonctionnality like a checkbox 'HTTPS connection' in the change session window?

@y377
Copy link

y377 commented Jun 5, 2020

use nginx proxy

@ckerr
Copy link
Member

ckerr commented Feb 20, 2022

The blocker on this is that we're using evhttp for serving web pages, and it doesn't support SSL.

Doing a bit of Googling this morning, I see this exists and could work.

https://github.com/yhirose/cpp-httplib could also work now that Transmission is written in C++

@mikedld
Copy link
Member

mikedld commented Feb 20, 2022

"Adding a checkbox" has been offered before (#13). Although simple, it's not necessarily extensible to other use-cases (#2574).

@LaserEyess
Copy link
Contributor

For clients like transmission-qt, I think "adding a checkbox" is probably the right thing to do, it's certainly the easiest thing to do. I know of no such way of detecting https without first timing out on 443, it's not a very good user experience, but even browsers do this so it's probably the only option.

For the RPC server itself, that's a lot more complicated. For now, if you want https then you should be using a reverse proxy since something like nginx or haproxy will handle this much better than transmission could with its current http code. That being said, there are use cases where transmission's backend using https does make sense, such as a haproxy/nginx instance on a different machine for load balancing. I think, ultimately the right thing to do is to stop using libevent for http entirely, and stop assuming that the RPC server is http only. But if you're going to do that, you might as well do it intelligently:

  • make sure http/2 and further protocols will be supported (which could mean e.g. UDP and gttp/3 in the future)
  • Obviously, it must play nice with TLS sockets from the crypto libraries transmission supports
  • Make http an abstraction on top of the base RPC server so that alternative options can be supported
    • on the surface that sounds hard, but really it's possible to just have JSON terminated with \r\n or something similar as a sentinel, I know mpv does this successfully
  • RPC should be able to listen on multiple addresses/ports to cover use cases where local access may be different than remote access
  • If you want to get real fancy support uwsgi for high performance proxying

These would likely give RPC a lot of performance boosts as well. In particular, a raw unix socket with JSON RPC entirely bypasses the network stack as well as http processing. Though, until #2462 is fixed, none of that will really matter as RPC blocks with disk I/O.

@anarcat
Copy link
Contributor

anarcat commented Jan 15, 2023

FWIW, i have this exact use case here. i wrapper the rpc port behind a web proxy (Apache, in my case, but it would of course work as well if not better with nginx) that does the HTTPS and decapsulates it to pass it down to transmission-daemon.

The problem, then is on the other end. I noticed it is hardcoded in Session::start, which is

transmission/qt/Session.cc

Lines 328 to 345 in 096db96

void Session::start()
{
if (prefs_.get<bool>(Prefs::SESSION_IS_REMOTE))
{
QUrl url;
url.setScheme(QStringLiteral("http"));
url.setHost(prefs_.get<QString>(Prefs::SESSION_REMOTE_HOST));
url.setPort(prefs_.get<int>(Prefs::SESSION_REMOTE_PORT));
url.setPath(QStringLiteral("/transmission/rpc"));
if (prefs_.get<bool>(Prefs::SESSION_REMOTE_AUTH))
{
url.setUserName(prefs_.get<QString>(Prefs::SESSION_REMOTE_USERNAME));
url.setPassword(prefs_.get<QString>(Prefs::SESSION_REMOTE_PASSWORD));
}
rpc_.start(url);
}

notice the hardcoded http there. i think that's the bit that needs to be flipped to https for things to work. Down in the RpcClient class, it ends up calling the QNetworkRequest and that seems to support HTTPS, although i'm not 100% sure...

i'm tempted to make a quick patch to hijack that code with an environment variable for a quick toggle, would that be an acceptable stopgap measure? i understand we want a checkbox (personnally, i'd just put the URL itself directly there, to simplify things), but that's a bit beyond how far i want to dig in here, as it involves touching the settings and so on...

anarcat added a commit to anarcat/transmission that referenced this issue Jan 15, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

I would have much rather this be turned in a single URL instead of
having flags, UI elements and settings for what is ultimately just a
string, but that is yet another yak to shave...

Closes: transmission#1294
@anarcat
Copy link
Contributor

anarcat commented Jan 15, 2023

i understand we want a checkbox (personnally, i'd just put the URL itself directly there, to simplify things), but that's a bit beyond how far i want to dig in here, as it involves touching the settings and so on...

i actually went ahead and tried to add a checkbox since that seemed like a low-hanging fruit, see #4593

anarcat added a commit to anarcat/transmission that referenced this issue Jan 20, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

This explicitely does *not* add GUI elements as those were found to be
too confusing, as the backend does not support HTTPS. See transmission#4593 for
the details of that discussion.

I actually would have much rather this be turned in a single URL
instead of having flags, UI elements and settings for what is
ultimately just a string, but that is yet another yak to shave...

Closes: transmission#1294
anarcat added a commit to anarcat/transmission that referenced this issue Jan 20, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

This explicitely does *not* add GUI elements as those were found to be
too confusing, as the backend does not support HTTPS. See transmission#4593 for
the details of that discussion.

I actually would have much rather this be turned in a single URL
instead of having flags, UI elements and settings for what is
ultimately just a string, but that is yet another yak to shave...

Closes: transmission#1294
anarcat added a commit to anarcat/transmission that referenced this issue Jan 20, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

This explicitely does *not* add GUI elements as those were found to be
too confusing, as the backend does not support HTTPS. See transmission#4593 for
the details of that discussion.

I actually would have much rather this be turned in a single URL
instead of having flags, UI elements and settings for what is
ultimately just a string, but that is yet another yak to shave...

Closes: transmission#1294
anarcat added a commit to anarcat/transmission that referenced this issue Jan 20, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

This explicitely does *not* add GUI elements as those were found to be
too confusing, as the backend does not support HTTPS. See transmission#4593 for
the details of that discussion.

I actually would have much rather this be turned in a single URL
instead of having flags, UI elements and settings for what is
ultimately just a string, but that is yet another yak to shave...

Closes: transmission#1294
anarcat added a commit to anarcat/transmission that referenced this issue Jan 20, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

This explicitely does *not* add GUI elements as those were found to be
too confusing, as the backend does not support HTTPS. See transmission#4593 for
the details of that discussion.

I actually would have much rather this be turned in a single URL
instead of having flags, UI elements and settings for what is
ultimately just a string, but that is yet another yak to shave...

Closes: transmission#1294
ckerr pushed a commit to anarcat/transmission that referenced this issue Jan 21, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

This explicitely does *not* add GUI elements as those were found to be
too confusing, as the backend does not support HTTPS. See transmission#4593 for
the details of that discussion.

I actually would have much rather this be turned in a single URL
instead of having flags, UI elements and settings for what is
ultimately just a string, but that is yet another yak to shave...

Closes: transmission#1294
anarcat added a commit to anarcat/transmission that referenced this issue Jan 22, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

This explicitely does *not* add GUI elements as those were found to be
too confusing, as the backend does not support HTTPS. See transmission#4593 for
the details of that discussion.

I actually would have much rather this be turned in a single URL
instead of having flags, UI elements and settings for what is
ultimately just a string, but that is yet another yak to shave...

Closes: transmission#1294
ckerr pushed a commit to anarcat/transmission that referenced this issue Jan 22, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

This explicitely does *not* add GUI elements as those were found to be
too confusing, as the backend does not support HTTPS. See transmission#4593 for
the details of that discussion.

I actually would have much rather this be turned in a single URL
instead of having flags, UI elements and settings for what is
ultimately just a string, but that is yet another yak to shave...

Closes: transmission#1294
ckerr pushed a commit to anarcat/transmission that referenced this issue Jan 24, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

This explicitely does *not* add GUI elements as those were found to be
too confusing, as the backend does not support HTTPS. See transmission#4593 for
the details of that discussion.

I actually would have much rather this be turned in a single URL
instead of having flags, UI elements and settings for what is
ultimately just a string, but that is yet another yak to shave...

Closes: transmission#1294
ckerr pushed a commit to anarcat/transmission that referenced this issue Jan 25, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

This explicitely does *not* add GUI elements as those were found to be
too confusing, as the backend does not support HTTPS. See transmission#4593 for
the details of that discussion.

I actually would have much rather this be turned in a single URL
instead of having flags, UI elements and settings for what is
ultimately just a string, but that is yet another yak to shave...

Closes: transmission#1294
anarcat added a commit to anarcat/transmission that referenced this issue Jan 26, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

This explicitely does *not* add GUI elements as those were found to be
too confusing, as the backend does not support HTTPS. See transmission#4593 for
the details of that discussion.

I actually would have much rather this be turned in a single URL
instead of having flags, UI elements and settings for what is
ultimately just a string, but that is yet another yak to shave...

Closes: transmission#1294
anarcat added a commit to anarcat/transmission that referenced this issue Jan 26, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

This explicitely does *not* add GUI elements as those were found to be
too confusing, as the backend does not support HTTPS. See transmission#4593 for
the details of that discussion.

I actually would have much rather this be turned in a single URL
instead of having flags, UI elements and settings for what is
ultimately just a string, but that is yet another yak to shave...

Closes: transmission#1294
ckerr pushed a commit to anarcat/transmission that referenced this issue Jan 26, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

This explicitely does *not* add GUI elements as those were found to be
too confusing, as the backend does not support HTTPS. See transmission#4593 for
the details of that discussion.

I actually would have much rather this be turned in a single URL
instead of having flags, UI elements and settings for what is
ultimately just a string, but that is yet another yak to shave...

Closes: transmission#1294
ckerr pushed a commit to anarcat/transmission that referenced this issue Jan 26, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

This explicitely does *not* add GUI elements as those were found to be
too confusing, as the backend does not support HTTPS. See transmission#4593 for
the details of that discussion.

I actually would have much rather this be turned in a single URL
instead of having flags, UI elements and settings for what is
ultimately just a string, but that is yet another yak to shave...

Closes: transmission#1294
ckerr pushed a commit to anarcat/transmission that referenced this issue Jan 26, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

This explicitely does *not* add GUI elements as those were found to be
too confusing, as the backend does not support HTTPS. See transmission#4593 for
the details of that discussion.

I actually would have much rather this be turned in a single URL
instead of having flags, UI elements and settings for what is
ultimately just a string, but that is yet another yak to shave...

Closes: transmission#1294
ckerr pushed a commit to anarcat/transmission that referenced this issue Jan 26, 2023
This is a rather naive implementation that copies parts of the
SESSION_REMOTE_HOST settings and replaces HOST with HTTPS,
basically. We pull some ideas from the SESSION_REMOTE_AUTH parameter
as well (because it's a boolean) and we otherwise do not really know
what we are doing here.

In particular, we didn't add a new commandline flag for this, as I am
not sure what it would be called.

This explicitely does *not* add GUI elements as those were found to be
too confusing, as the backend does not support HTTPS. See transmission#4593 for
the details of that discussion.

I actually would have much rather this be turned in a single URL
instead of having flags, UI elements and settings for what is
ultimately just a string, but that is yet another yak to shave...

Closes: transmission#1294
@anarcat
Copy link
Contributor

anarcat commented Jan 26, 2023

so just to make things clear to people around here, the commit that was merged in #4622 is (unfortunately) a bit of a misnomer because while the Qt GUI will now have the capability of connecting to HTTPS remotes, the GUI elements itself will not offer that option. You will need to manually edit the configuration file (in, say,
~/.config/transmission/settings.json) to add a line like:

    "remote-session-https": true,

The rationale of why the GUI elements were removed are in #4593 (comment) and that PR also has code to introduce those GUI elements if someone still wants to push that through. :)

Thanks @ckerr for the hand-holding and the merge! :)

@ckerr ckerr added notes:none Should not be listed in release notes and removed notes:none Should not be listed in release notes labels Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

6 participants