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

Added extra checks and option to save result to SharedPreferences #15

Merged
merged 18 commits into from
Mar 13, 2017
Merged

Added extra checks and option to save result to SharedPreferences #15

merged 18 commits into from
Mar 13, 2017

Conversation

jahirfiquitiva
Copy link
Collaborator

@jahirfiquitiva jahirfiquitiva commented Mar 12, 2017

With these commits, the library is able to check:

  • If any pirate app is installed. (LuckyPatcher, Freedom or CreeHack).
  • If any third-party store is installed. (Aptoide, BlackMart, Mobogenie, 1Mobile, GetApk, GetJar or SlideMe).
  • If app is a debug build.
  • If app is being run in an emulator.

Also, allows the app to save the result of the license check in SharedPreferences defined by user.

And finally, removes the com.android.vending.CHECK_LICENSE permission from library's AndroidManifest.xml
From my experience, it makes LuckyPatcher detect app as "patcheable".

It can still be added by users in their apps' AndroidManifest.xml and they would be the ones who decide whether to use it or not. The README still says they should, but has a note for they to know about this and decide.

This should solve #13 and #14 .

@franciscofranco
Copy link

Everything seems fine to me. Good job.

@jahirfiquitiva
Copy link
Collaborator Author

Thanks @franciscofranco

@franciscofranco
Copy link

Hopefully @javiersantos will merge this asap.

@jahirfiquitiva
Copy link
Collaborator Author

@franciscofranco
If you are impatient, you can use my fork using this dependency:

dependencies{
	...
	compile 'me.jahirfiquitiva:PiracyChecker:55dd8f3'
}

@franciscofranco
Copy link

Awesome. Was fighting myself not to clone the source code, but that makes it easy. Will test.

@franciscofranco
Copy link

Warning: Exception while processing task java.io.IOException: proguard.ParseException: Unknown option 'com.github.javiersantos.**' in line 21 of file '/Users/francisco/.android/build-cache/ae8be0d3bfa426908e10daea6f07d94c9a367342/output/proguard.txt'

@franciscofranco
Copy link

franciscofranco commented Mar 12, 2017

Removing com.android.vending.CHECK_LICENSE is giving me an "Allow" even if I don't have internet access. I'm on a debug build so it needs to check for license since it only caches the result for 60 seconds (which is perfect for testing). Adding back com.android.vending.CHECK_LICENSE but still offline it'll say "LibraryValidator: Error contacting licensing server." and returning "don't allow".

I don't recall having test this with 0.0.3 of PiracyChecker, so I can't tell if it was caused by your changes. Will test now.

@jahirfiquitiva
Copy link
Collaborator Author

@franciscofranco
What you're trying to say, is that license should return "don't allow" when not connected to internet?

@franciscofranco
Copy link

Confirmed with 0.0.3 and without com.android.vending.CHECK_LICENSE and without internet access it correctly returns "don't allow".

@franciscofranco
Copy link

franciscofranco commented Mar 12, 2017

Well, it should not allow it if returns "LicenseValidator: Error contacting licensing server." On debug builds cached responses last for 60 secs, after the 60 secs it needs to contact the server again to make sure the app is licensed. I think cache for production lasts longer so it won't an error even if the user doesn't have internet access.

@jahirfiquitiva
Copy link
Collaborator Author

@franciscofranco
Can you please try this version?

dependencies{
	...
	compile 'me.jahirfiquitiva:PiracyChecker:9ab0ec8'
}

@franciscofranco
Copy link

On 9ab0ec8 if I remove com.android.vending.CHECK_LICENSE and I'm offline (without a response cached) now it doesn't even go through the allow() callback, it'll say "LibraryChecker: Binding to licensing service." and that's it, won't do anything else. As soon as I add com.android.vending.CHECK_LICENSE, while still offline and no cache what-so-ever it'll properly say "Error contacting the server" + "don't allow".

@@ -144,4 +146,11 @@ static boolean isDebug(Context context) {
return ((context.getApplicationInfo().flags & ApplicationInfo.FLAG_DEBUGGABLE) != 0);
}

static boolean isConnectedToInternet(Context context) {
Copy link
Owner

@javiersantos javiersantos Mar 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ConnectivityManager will check if the user has WiFi or data enabled. If LuckyPatcher or any other patcher uses an offline mode, this method will return true because the device is connected to the Internet.

This method doesn't fix the issue reported by @franciscofranco , since the license checker will return "Allow" when the user is connected to the Internet and using a simple bypass by LuckyPatcher (offline mode).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind if I ask you for a suggestion?

@jahirfiquitiva
Copy link
Collaborator Author

jahirfiquitiva commented Mar 12, 2017

I have made some tests, and definitely the only way for the checker to work properly is making sure it has the com.android.vending.CHECK_LICENSE permission.

In my last commit I put it in the library's AndroidManifest.xml and removed it from the app's one. All apps using the library will get its AndroidManifest.xml automatically merged with their apps' one, so no need to re-write it.

The bad part of this is LuckyPatcher will detect the apps as "patcheable", and from an user test, he can use lucky patcher to patch the app, then uninstall it, and the app would recognize the license as valid and lucky patcher as not installed.

I guess there's still a long way for a license check to be completely secure or successful.

@franciscofranco
Copy link

Hmm... there's got to be a way to fake the permission instead of declaring it into the Manifest. I'll go research.

@jahirfiquitiva
Copy link
Collaborator Author

jahirfiquitiva commented Mar 12, 2017

@franciscofranco @javiersantos

An user used the latest version from my fork.

He says LuckyPatcher detects it, but does not allow to apply a patch, although it allows to create a modified apk.

I guess what LuckyPatcher does is creating an app with the permission removed from the AndroidManifest.xml

He tried creating the modified app and license check still works, although that could be because of checking if app has been installed from Play Store or not.

He also uninstalled LuckyPatcher after these things, and said the app still said license was invalid.

He even cleared data of the modified apk and license still was invalid.

I am also saving the result to SharedPreferences, so that could be helping too.

For me, these changes are working good enough, although I would like to know what you think.

@javiersantos
Copy link
Owner

javiersantos commented Mar 12, 2017

Good job with your PR, @jahirfiquitiva. As you say, I think that the permission must be included since it could break some checkers of the licensing service.

For sure it will be detected by LuckyPatcher, but the main goal here is not to prevent LuckyPatcher from detecting the licensing library (which I don't think is possible) but to prevent LuckyPatcher from patching it.

I have been testing my commits from yesterday (297f180, 82ae6a4, e7d4756). This changes should make harder to apply a patch to an app and after having some tests, LuckyPatcher generates a modified apk and passes the first test, but the license checker is still working in the resulting apk.

@franciscofranco
Copy link

I have nothing valuable to add, you guys said it all. @javiersantos maybe merge this and call it version 1.0? There doesn't seem to be possible to protect this any further and it might be good to get this battle tested in some production apps.

@javiersantos javiersantos merged commit 1c66045 into javiersantos:master Mar 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants