Differential D21247

[pretty please] allow fetching of cover art in mpris plugin from connected computer
ClosedPublic
Actions

Authored by cotko on May 16 2019, 10:27 PM.

Details

Reviewers
nicolasfella
albertvaka
Group Reviewers
KDE Connect
Commits
R225:b8d327c2d979: [pretty please] allow fetching of cover art in mpris plugin from connected…
Summary

So from android 8 onwards it appears that fetching content from "cleartext" urls is disabled by default

https://stackoverflow.com/questions/45940861/android-8-cleartext-http-traffic-not-permitted

I have a mpris service running on my local computer which is connected to the android, that service is also serving cover art for currently playing song. Obviously I can not have a domain set for my computer on a local machine, so cover art urls look like
http://<ip>:<port>/<songid>.ext . Since this restriction was introduced into android 8, kde connect is not able to fetch the art from this url.

This is patch allows connections to raw IPs addresses as well. If there is any security issues regarding this change, I'm prepared to do more work, my suggestion would be to check if IP address in this case matches paired computer's IP address, although this would still be a bit annoying (especially since whatever malicious file can be served from a "cleartext" link as well).

Test Plan
  • start some sort of server which can serve an image on a paired computer
  • create mpris Player instance on a paired computer and set the mpris:artUrl to the served address where this image is available
  • open kde connect android app, select prepared player and see default cover art instead of the served image
  • change artUrl to some image from the web
  • check in kde connect again and see it correctly show a remote image

Diff Detail

Repository
R225 KDE Connect - Android application
Lint
Lint Skipped
Unit
Unit Tests Skipped
cotko created this revision.May 16 2019, 10:27 PM
Restricted Application added a project: KDE Connect. · View Herald TranscriptMay 16 2019, 10:27 PM
Restricted Application added a subscriber: kdeconnect. · View Herald Transcript
cotko requested review of this revision.May 16 2019, 10:27 PM
cotko retitled this revision from [pretty please] allow fetching of cover art in mpdris plugin from connected computer to [pretty please] allow fetching of cover art in mpris plugin from connected computer.May 16 2019, 10:32 PM
andyholmes added a subscriber: andyholmes.May 16 2019, 10:58 PM

I don't really have this problem anyways, but isn't the better solution to have the serving instance of KDE Connect do the art downloading and transfer the file as normal? I think the only external input to the android app should be from the remote paired device. This also ensures the Android app (or whichever client) has it's address isolated from any outside source.

FWIW, in GSConnect we use the GFile abstraction, which can take any URI, and use it as a source for an input stream when sending the payload. In this case whether the URI is file://, http://, sftp:// or whatever, the Android app and the original URI source have no knowledge of each other or direct connection.

I assume a similar file abstraction exists in Qt or KIO?

nicolasfella accepted this revision.May 16 2019, 11:46 PM
nicolasfella added a subscriber: nicolasfella.

Your problem description is a bit inaccurate. The problem is not a local IP, it's that you don't serve the covers over https. I have a similar problem myself: Spotify is shipping album art over http.

Obviously I can not have a domain set for my computer on a local machine

That's not true. You could set up your own DNS server that achieves that. But anyhow, not relevant here

@andyholmes It should be able to implement somenthing like that as well, but I don't see how it solves the problem in the bigger picture.
The problem is that unencrypted network connections have risks security and privacy wise. Those are there regardless of whether the cover is fetched on the desktop or the phone. Assuming that both devices are in a network with NAT I don't think those connections could be distinguished from the outside anyway. Privacy-wise the damage is pretty much already done since I assume that the Spotify (and possibly other clients) already have fetched the cover themselves.

TL;DR
Patch looks good to me, let's wait a few days if someone objects.

To merge this we need your full name + email address please.

This revision is now accepted and ready to land.May 16 2019, 11:46 PM
cotko added a comment.EditedMay 17 2019, 9:12 AM

Ok, great:) I'll wait then and make changes if necessary..

I'm not able to log into identity.kde.org to change my profile settings (login is not working, even after a few password resets) and I'm not able to change my real name setting using "edit profile" here in phabricator, so I'll write it here:
name: Mitja Cotic
email: mitja_cotic@yahoo.com

albertvaka accepted this revision.May 17 2019, 11:38 AM
albertvaka added a subscriber: albertvaka.

LGTM

nicolasfella closed this revision.May 18 2019, 8:51 AM

Revision Contents
Changeset List

Diff 58180

View Options

AndroidManifest.xml

Loading...
View Options

res/xml/network_security_config.xml

Loading...