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

Wait for OCID download before start CID check #290

Closed
He3556 opened this issue Jan 19, 2015 · 24 comments
Closed

Wait for OCID download before start CID check #290

He3556 opened this issue Jan 19, 2015 · 24 comments
Labels

Comments

@He3556
Copy link
Collaborator

He3556 commented Jan 19, 2015

⚠️ This Bug only appears in the (unofficial) debug version, not in the release.

The CID detection is running before there is OCID data to check it against. We need to wait until the db is downloaded before we check the CID against the data of OpenCellID. Otherwise we get a wrong alert just right after the installation. Maybe we use a flag, that is changed when the DB download is finished? But only the CID check needs to get stopped not the changing LAC detection. There are many possible ways to fix it.

But I would prefer to run a wizard at the first start, after installing the App. Then we can assist users to get the OCID key and download the OCID data (if they want). We get everything ready so the app can do its job. Described here #181

@He3556 He3556 added the bug label Jan 19, 2015
This was referenced Jan 19, 2015
@SecUpwN
Copy link
Member

SecUpwN commented Jan 21, 2015

Quick question: Shouldn't the yellow CID warning disappear once the OpenCellID data has been successfully downloaded? On my phone, the yellow flag persists even after a successful download. Just wanting to make sure I'm not constantly booked into a honeypot or something. Feeling monitored and thus a little paranoid as well.

@E3V3A
Copy link
Contributor

E3V3A commented Jan 21, 2015

@SecUpwN I think you're confusing this (CID) issue with the changing LAC one.
@He3556 (Which version are you testing with? Private or from testroom?) Also I think it could be related to #284. ?

@SecUpwN
Copy link
Member

SecUpwN commented Jan 21, 2015

I think you're confusing this (CID) issue with the changing LAC one.

No, I don't think so. I am running build 20 and @He3556 opened this Issue right after we've discovered and discussed it in the internal rooms: The Cell ID does not exist in the OCID database.

Otherwise we get a wrong alert just right after the installation.

That is exactly what both of us are experiencing - and for me it stays after downloading the OCID DB.

@E3V3A
Copy link
Contributor

E3V3A commented Jan 27, 2015

I have the same problem. So if you can pinpoint me to the code that is doing the CID check, I can have a look at it.

@E3V3A
Copy link
Contributor

E3V3A commented Jan 29, 2015

I have now implemented (7f7774a) a Boolean system property called: aimsicd.ocid_downloaded. @He3556 Can you point me to the place where you think this check is/should be done?

@He3556
Copy link
Collaborator Author

He3556 commented Jan 31, 2015

It used to be here:
https://github.com/SecUpwN/Android-IMSI-Catcher-Detector/blob/229e1f84b65e25374293a563006653b0df62a219/app/src/main/java/com/SecUpwN/AIMSICD/adapters/AIMSICDDbAdapter.java#L299

But since 2015-01-19, we have the detection here:
https://github.com/SecUpwN/Android-IMSI-Catcher-Detector/blob/6c492fc1598749bccce0554d78547311c6592a18/app/src/main/java/com/SecUpwN/AIMSICD/service/CellTracker.java#L758

We should also copy the logcat from the old position to the new. Also the insert Cell should be deleted (Like you wrote in the comment there (the first link = old position) the 2. Link is the new position)

@E3V3A
Copy link
Contributor

E3V3A commented Feb 2, 2015

@He3556 So should I remove all these lines? (See c81490c .)

@He3556
Copy link
Collaborator Author

He3556 commented Feb 2, 2015

Yes, this lines made their job pretty good but we don't need them anymore.

@He3556
Copy link
Collaborator Author

He3556 commented Feb 6, 2015

I was testing the App today, while i was driving around. After leaving a 15km radius i had to manually download more towers, because i had the CellID detection (Bug) again. Maybe we need to think about a automated downloading in the background. Maybe after leaving a 5km radius?

@SecUpwN
Copy link
Member

SecUpwN commented Feb 6, 2015

Maybe we need to think about a automated downloading in the background.

That would mean we'd have to have internet connection enabled. Not good. What happpens if it is disabled? People would still drive home and file a bug report or tell that they "detected something".

Maybe after leaving a 5km radius?

Not sure if there is a way of downloading a larger part of the database, maybe for the whole country?

@E3V3A
Copy link
Contributor

E3V3A commented Feb 7, 2015

Maybe we need to think about a automated downloading in the background. Maybe after leaving a 5km radius?

Great idea. But this brings us straight to #303. As we've already discussed in the past, we should not expect to have good detections without false-positives when moving around. But the data need to be there. But there is no sensible or decent way to download all the BTSs from a whole country, so we have to take it piece by piece.

@He3556
Copy link
Collaborator Author

He3556 commented Feb 7, 2015

The sad thing is, they already have it their API. But only if our APP would contribute data to them. http://wiki.opencellid.org/wiki/API#Getting_the_list_of_cells_in_a_specified_area

@SecUpwN
Copy link
Member

SecUpwN commented Feb 10, 2015

Strange. I wonder why I still get the warning Cell ID does not exist in OpenCellID Database even though I've downloaded it. Is anyone experiencing the same on internal build 26?

@E3V3A
Copy link
Contributor

E3V3A commented Feb 13, 2015

@He3556 Please open a new issue for your comment?

We need to trigger a new download request when GPS has moved ~1 Km. There is no framework for this yet AFAIK, so someone has to step in and code up some...

@He3556
Copy link
Collaborator Author

He3556 commented Feb 13, 2015

I think it is better to keep it (talking about the bug) all in one place.
For getting the cells of the area, we can use the BBOX thing. The automatic download (when the location is changed) itself is not implemented. Well, i would rather download to much cells than less cell towers.

But ok, when the radius is smaller, than we have to check more often if the location of the user has changed. Can we use another check that runs from time to time and add this code?
If you use 1 km radius we had to check every 5 minutes? Still not often enough when you are in a car, i am afraid.

Another problem can be, if we are starting to delete entities in the downloaded OCID DB. We need all CellIDs we can get from this db. I think this clear to all of you. I just wanted to remind you

@E3V3A
Copy link
Contributor

E3V3A commented Feb 13, 2015

Well it seem that the discussion is constantly regressing to "I'm driving around in my car and I'd like to detect a false BTS." This is not possible, unless we implement ALL the detection from the list. So if you wanna be able to do anything at all right now, we'll have to stay put, and not leave the CID/LAC on purpose, which is exactly what you do when driving around. Also what you describe is not a bug, it's simply not implemented, which is why we should open a new issue.

@He3556
Copy link
Collaborator Author

He3556 commented Feb 13, 2015

The same LAC? How do you know the cell 100m away has the the same LAC or maybe not? Maybe another local area code starts right there.
That is no argument.
And sorry but the yellow alarm when the db is not downloaded looks like a bug for the users - and we can not tell the people they are not allowed to change the area when the app is running.

E3V3A added a commit that referenced this issue Feb 16, 2015
- changed the ATCoP timout values from the selector (now also 10 min)
- fixed ugly setprop log/toast message
- fixed false "ALERT: CID -NOT- in OCID DB: " message, before having
downloaded OCID data. However, this generates tons of SU calls to check
the system property used here. This is very bad coding and need to be
fixed by a professional code ASAP. Old phones with small memory or
single core processors will probably cause FCs! This is #290.
@E3V3A
Copy link
Contributor

E3V3A commented Feb 16, 2015

Fixed in: 8b37100

@SecUpwN
Copy link
Member

SecUpwN commented Mar 3, 2015

@E3V3A, I am afraid that I have to reopen this Issue as it seems to be present again in our latest internal build 30. Maybe @KaiRenken can have a look at this Issue which prevents our new release?

@SecUpwN SecUpwN reopened this Mar 3, 2015
@E3V3A
Copy link
Contributor

E3V3A commented Mar 3, 2015

What are you talking about. It's not present. Send screen shot please!

@elektrischermoench
Copy link
Contributor

I can't see it either. I've tried the app from testroom.

@SecUpwN
Copy link
Member

SecUpwN commented Mar 3, 2015

What are you talking about. It's not present. Send screen shot please!

@E3V3A, why do I need to send you a screenshot? Upon a fresh install, build 31 instantly turn yellow - and it doesn't change when downloading the OCID DB after it turned yellow either. Just believe me?

EDIT: @E3V3A and @KaiRenken, I'm eating my broom right now. Restarted phone and it works. 🌴

@SecUpwN SecUpwN closed this as completed Mar 3, 2015
@E3V3A
Copy link
Contributor

E3V3A commented Mar 3, 2015

Hmm, that it nevertheless funny behavior. Probably because the flag we use, are still in your system until next reboot. Which is why we need to use SharedPreferences instead of a system property via shell, to ensure it gets wiped after app removal.

@SecUpwN
Copy link
Member

SecUpwN commented Mar 3, 2015

@E3V3A, it should also be wiped when someone just install the new version on top of it. Is that somehow fixable before we start to craft a new release? Otherwise we'll have many people seeing this behavior.

E3V3A added a commit that referenced this issue Apr 17, 2015
- changed the ATCoP timout values from the selector (now also 10 min)
- fixed ugly setprop log/toast message
- fixed false "ALERT: CID -NOT- in OCID DB: " message, before having
downloaded OCID data. However, this generates tons of SU calls to check
the system property used here. This is very bad coding and need to be
fixed by a professional code ASAP. Old phones with small memory or
single core processors will probably cause FCs! This is #290.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants