openatv / enigma2

openatv-gui
GNU General Public License v2.0
200 stars 315 forks source link

InitGeolocation: run at every boot / incorrect info via VPN #1563

Closed Azureit closed 4 years ago

Azureit commented 4 years ago

What is the need to run this at every boot when we already manually configured language/timezone? And this will produce incorrect info if I VPN my internet.

Azureit commented 4 years ago

@IanSav but you do caching of geolocation dictionary already..

IanSav commented 4 years ago

@IanSav but you do caching of geolocation dictionary already..

Yes, once a lookup is done I don't do it again until the box restarts. I felt that was an appropriate level of caching. I provide the ability to refresh the cache more frequently should an application desire the latest update.

Unless you are a developer, how often do you restart your receiver? I suspect that for many users geolocation lookups will be very infrequent.

Azureit commented 4 years ago

@IanSav

Yes, once a lookup is done I don't do it again until the box restarts. I felt that was an appropriate level of caching. I provide the ability to refresh the cache more frequently should an application desire the latest update.

My way would not require lookup at boot, and would provide refresh if data older than a day, but you can tweak that down.

IanSav commented 4 years ago

The geolocation call is a small data request with minimal data exchange. What exactly is your issue with the geolocation lookup?

Captain has given you a way to completely block it.

I have already said that when Enigma2 is corrected to allow more user control during the start up process I will give users more control over the geolocation and many other initialisation tasks.

Azureit commented 4 years ago

The geolocation call is a small data request with minimal data exchange. What exactly is your issue with the lookup?

Why would you do a request if needed for nothing, all manually configured. When geolocation needed it would be initialized or refreshed. All happy. My enigma box turn off with my TV.

IanSav commented 4 years ago

As I updated my previous post. The design and implementation of Enigma2 is currently flawed such that user configuration settings are only enabled at the end of the system startup.

Refactoring mytest.py is a significant effort. It is not trivial to make fixes in this code. If I refactor this code I intend to make loading the user's preferences and settings the first thing the code does. That will enable me to skip the geolocation call completely if the user has already configured their preferred settings. This will also allow a significantly more optimised start up experience.

Azureit commented 4 years ago

@IanSav

As I updated my previous post. The design and implementation of Enigma2 is currently flawed such that user configuration settings are only enabled at the end of the system startup.

That's the problem, you should put user settings first.

Refactoring mytest.py is a significant effort. It is not trivial to make fixes in this code.

In my view you just comment the Tools.Geolocation.InitGeolocation() at mytest.py and do that before every geolocation.get("WHATEVER") like at Timezone.py

Azureit commented 4 years ago

@IanSav

If I refactor this code I intend to make loading the user's preferences and settings the first thing the code does. That will enable me to skip the geolocation call completely if the user has already configured their preferred settings. This will also allow a significantly more optimised start up experience.

That would be great, thanks.

IanSav commented 4 years ago

That is just adding more bad code that will have to be fixed again later.

Please tell me exactly what is the problem with the geolocation lookup? I am not talking about the code I am asking about the core reason for the objection to the concept within Enigma2. This issue has still not been made clear. I have, I believe, addressed every privacy concern raised so far.

Azureit commented 4 years ago

@IanSav Read my post above, this is what I would like to read from you, that you intend to load user settings first and skip the geolocation if not need. Case closed for me. Thanks

IanSav commented 4 years ago

That would be great, thanks.

I actually tried to make this change when the first concern was raised. Trust me when I say the Enigma2 crashed and burned. So many people have tried to work around this design flaw that when I moved the code many things broke. :(

I am definitely not ignoring the concerns expressed here it is just that fixing them is VERY complicated and will have widespread implications to much of Enigma2.

IanSav commented 4 years ago

You can also trust me that if I do try the refactor mytest.py I will receive many more complaints and objections. That change will be VERY hard to get accepted. A lot of Enigma2 developers resist and/or hate change.

(You may not be aware but I am a developer for OpenATV, OpenPLi and OpenViX. My code is also used in a number of other derivative Enigma2 images. I try to develop the same code and changes for all the images. It is one of my objectives to improve the user interface of Enigma2 for all images and to try and make more code, plugins and skins interoperable between the images. The same geolocation code is now running in at least those main images and may already have been integrated into the derivative images.)

Azureit commented 4 years ago

@IanSav

You can also trust me that if I do try the refactor mytest.py I will receive many more complaints and objections. That change will be VERY hard to get accepted. A lot of Enigma2 developers resist and/or hate change.

I'm starting to worry about that "if", please the right way to do it, is to load user settings first and skip if not needed.

You may not be aware but I am a developer for OpenATV, OpenPLi and OpenViX. My code is also used in a number of other derivative Enigma2 images

That's because I tell you "?fields=33288191" is a way to fingerprint the use of an enigma2, trust me most of the requests for other developers don't need the fields you need.

IanSav commented 4 years ago

The "if" must be used as I may be forbidden from making the change.

That's because I tell you "?fields=33288191" is a way to fingerprint the use of an enigma2, trust me most of the requests for other developers don't need the fields you need.

We have been through this. It is NOT a fingerprint. Any request to get the full data set will use the same number. A representative of the company has already confirmed that the data is not a fingerprint and is not kept or used in any way other than to return the data requested.

IanSav commented 4 years ago

It is now 7AM here (Australia) I have spent most of the night responding to the concerns expressed here. I trust that you will see that I am taking your comments seriously. Unfortunately I am now way over tired and am going to get some sleep.

IanSav commented 4 years ago

@jose-cruz78 you do seem to be a glass half empty kind of person. :(

I welcome your efforts to try and improve Enigma2.

ghost commented 4 years ago

I started to build OpenATV 6.4 with all geolocation code reverted. Local demand is high and I can't handle build for different hardware. Build machine is not very fast! 😓 Wacky workarounds won't do..., so I started to share old python files (mystest.py(o), SetupDevices.pyo, Timezones.pyo,..) so people can revert to old behavior more easily. Also, kinda wacky too, and of course, this will break if someone tries to update, but it effectively removes it.

I kindly ask @IanSav to at least consider reverting geolocation changes in the '6.3' branch. This approach could also be taken to start refactoring mystest.py code, could even move all geolocation code there ( to the new testing brach?), so, when it goes 'public', it already gives the user the power to control it using UI. Everyone's happy! At the moment, there's no alternative downloads at opena.tv, unless people go for a very outdated image. (6.2-)

Thanks for your time Bye.

IanSav commented 4 years ago

@Tony-il-Capo I only contribute to the 6.4 branch (as requested by Captain). I am not involved with code being moved to the 6.3 branch.

If you start modifying and diverting OpenATV to sabotage the new code then I suggest that you confirm this course of action with the OpenATV image administrators. This action is in direct contradiction to their requests that I include OpenATV in the Enigma2 improvement and common code developments. I will not be allocating limited resources to fixing OpenATV and undoing damage to future merges that your changes are likely to create! Your refusal to accept that geolocation is totally private and harmless is your issue. Changing OpenATV to interfere with the development efforts is going to hurt many others.

By the way, I have already started initial development work to refactor mytest.py to resolve the issues previously discussed.

Azureit commented 4 years ago

@jose-cruz78

Worry more because it could also mean never!

This is a open source project, and no one pays for it, so there is no accountability to the end user. @IanSav don't share my privacy concerns but I respect him for keeping an open dialogue, most developers at his position would just say no and ignore you.

Azureit commented 4 years ago

@IanSav Lets solve this in steps. step 1: auto initialization/auto refresh geolocation data step 2: read manual settings first I will forget for now step 2 and concentrate at step 1.

My vision was that no initialization should be needed, a simple geolocation.get("WHATEVER") should initialize the geolocation dictionary. Disclaimer: My knowledge about python is by example, nevertheless I research this and arrive to what maybe a solution for this, that is working and tested by me, with minimal changes to your code.

--- <mytest.py>
+++ <mytest.py>
@@ -29,8 +29,7 @@
 from traceback import print_exc

 profile("Geolocation")
-import Tools.Geolocation
-Tools.Geolocation.InitGeolocation()
+# lookup when needed

 profile("SetupDevices")
 import Components.SetupDevices
--- <Geolocation.py>
+++ <Geolocation.py>
@@ -1,7 +1,8 @@
 from json import loads
 from urllib2 import URLError, urlopen
+from time import time

-# Data available from http://ip-api.com/json/:
+# Data available from http://ip-api.com/json/?fields=33288191:
 #
 #  Name        Description             Example         Type
 #  --------------  --------------------------------------  ----------------------  ------
@@ -37,30 +38,31 @@
 #  hosting     Hosting, colocated or data center   true            bool
 #  query       IP used for the query           173.194.67.94       string

-geolocation = {}
+class Geolocation(dict):
+   def get(self, k, default=None):
+       # check interval = 1 day
+       if 'lastchecked' not in self or time() - self['lastchecked'] > 86400:
+               try:
+                   response = urlopen('http://ip-api.com/json/?fields=33288191', data=None, timeout=10).read()
+                   # print "[Geolocation] DEBUG:", response
+                   if response:
+                       try:
+                           self.update(loads(response))
+                       except:
+                           self = {}
+                   status = super(Geolocation, self).get('status', None)
+                   if status and self['status'] == 'success':
+                       print "[Geolocation] Geolocation data initialised."
+                       self['lastchecked'] = int(time())
+                   else:
+                       print "[Geolocation] Error: Geolocation lookup returned a '%s' status!  Message '%s' returned." % (status, super(Geolocation, self).get("message", None))
+               except URLError as err:
+                   if hasattr(err, 'code'):
+                       print "[Geolocation] Error: Geolocation data not available! (Code: %s)" % err.code
+                   if hasattr(err, 'reason'):
+                       print "[Geolocation] Error: Geolocation data not available! (Reason: %s)" % err.reason
+               except ValueError:
+                   print "[Geolocation] Error: Geolocation data returned can not be processed!"
+       return super(Geolocation, self).get(k, default)

-def InitGeolocation():
-   global geolocation
-   if len(geolocation) == 0:
-       try:
-           response = urlopen("http://ip-api.com/json/?fields=33288191", data=None, timeout=10).read()
-           # print "[Geolocation] DEBUG:", response
-           if response:
-               geolocation = loads(response)
-           status = geolocation.get("status", None)
-           if status and status == "success":
-               print "[Geolocation] Geolocation data initialised."
-           else:
-               print "[Geolocation] Error: Geolocation lookup returned a '%s' status!  Message '%s' returned." % (status, geolocation.get("message", None))
-       except URLError as err:
-           if hasattr(err, 'code'):
-               print "[Geolocation] Error: Geolocation data not available! (Code: %s)" % err.code
-           if hasattr(err, 'reason'):
-               print "[Geolocation] Error: Geolocation data not available! (Reason: %s)" % err.reason
-       except ValueError:
-           print "[Geolocation] Error: Geolocation data returned can not be processed!"
-
-def RefreshGeolocation():
-   global geolocation
-   geolocation = {}
-   InitGeolocation()
+geolocation = Geolocation()
IanSav commented 4 years ago

@Azureit it turns out that other developers are also annoyed about the initialisation issues in Enigma2. I am now working with another OpenViX developer to refactor, and possibly rename, mytest.py to fix these problems once and for all time. When we get the code working on OpenViX I will work with the OpenATV developers to try and make equivalent corrections to OpenATV. (Not that it matters here but the same will also happen with OpenPLi.)

I have already PEP8 cleaned the code and added lots of debugging code to work out what is going on now. I am going to get some sleep now but tomorrow when Tony (my OpenViX development partner in this project) and I are both available we are going to give this proposal a really good try.

Azureit commented 4 years ago

@IanSav Thanks for using your time to help the enigma2 community.

Azureit commented 4 years ago

@Tony-il-Capo

I started to build OpenATV 6.4 with all geolocation code reverted. Local demand is high and I can't handle build for different hardware. Build machine is not very fast! sweat Wacky workarounds won't do..., so I started to share old python files (mystest.py(o), SetupDevices.pyo, Timezones.pyo,..) so people can revert to old behavior more easily. Also, kinda wacky too, and of course, this will break if someone tries to update, but it effectively removes it.

I didn't test this and may not work or break your box, but if wanted to disable geolocation without recompiling, I would target the rootfs.bin update file. Edit with an hexeditor and search for the string "ip-api.com" and replace with a string of same length like "127.0.0.10"

I didn't test this and may not work or break your box

IanSav commented 4 years ago

@IanSav Thanks for using your time to help the enigma2 community.

This is a major project and change for Enigma2. I appreciate your acknowledgement but this will be a significant effort from a number of developers across a number of images.

By the way, the common/correct loopback IP address 127.0.0.1

ghost commented 4 years ago

Worry more because it could also mean never!

It was like a :rocket: to implement... but now a :turtle: to fix @jose-cruz78 I've seen evidence of fingerprinting... it's probably not so hard for (google) algorithms to pick up this code and scan other open source for the same fields requested. Probably not many or even nothing more asks for so many fields. Why the hell, lat, lon, ISP, org, currency.. is needed?

We have ISPs monitoring every connection made, and share data with third parties (ie google), here a guy that has like 7 stbs, shutting down/starting 3-4 times a day, found himself flooded with E2 related ads when browsing. The guy only browses email and weather. wtf?!

If 'some' could really see how many people reverted/stopped updating their images... but, I guess, someone with limited perspective, that judges their 'awesome' commits to E2 by the reaction of their dev pals and ignore real user base, can't really see that.

jenny-91 commented 4 years ago

Hey I don't use OpenATV anymore, but recently noticed this geolocation cr*p code in other dists, I searched and found this thread and decided to sign in and share my opinon. Afterwards I removed/disabled it for my builds. Luckly, ip-api.com is listed as a tracker in any tracking lists and my network setup blocked it anyway.

I have to say I'm very disapointed in what E2 project is becoming, thoughtless implementations will destroy this project. Contributors that don't value user opinions and refuse to understand them, should stay away from projects like this.

IanSav commented 4 years ago

@Jenny-91, can you please provide proof that the geolocation provider is tracking users?

The only data they get from Enigma2 users is their publically available WAN IP address. I have checked their privacy policy and spoken with the developers/maintainers of that site and received assurance that there is no use of any connection data, other than to return geolocation data and there is no long term retention of data other than the short term retention (in minutes) to detect service abuse.

Given your feeling for geolocation I trust that you don't use the Internet. If you do your are most certainly using geolocation services. The main difference is that here that data is being returned to you. On the vast majority of Internet servers the geolocation data is being given to the website provider.

I should also reiterate that I am working on changing mytest.py to allow users to control if the geolocation, and other initialisation tasks, are run at all.

AbuBaniaz commented 4 years ago

@jenny-91 Can you share how to disable these checks?

It does not even work. So every time the receiver reboots, it is checking/sharing your location without your permission. And does not even fulfil its purpose. If You flash PLI/ATV in the UK. Your timezones start as Berlin or Amsterdam respectively. I am sure if you flash ViX In the Netherlands, you will start off as London.

The website being used says it passes on some IP ranges to another website. There is no mention of which ranges.

In the second company's policy they state.

Information Collected Automatically We automatically collect information about your device and how your device interacts with our Services and other services. The categories of information we automatically collect, including in the last 12 months, and the methods by which we may collect such information include the following: (I have not coped rest of text, find it yourself)

Location Data MaxMind may also request access to or otherwise receive information about your device location when you access the Services. Your location data may be based on your IP address and other location-aware technologies. We use location data in connection with providing the Services and to help improve the Services.

atvcaptain commented 4 years ago

https://github.com/openatv/enigma2/commit/a35571adddd18a22c95b006fce7d56d669794a53