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

Cannont load classes which come from different package names which have a common relevant prefix #3

Closed
jrocharodrigues opened this issue Jan 14, 2016 · 14 comments
Assignees

Comments

@jrocharodrigues
Copy link

Hi,
First of all, thanks for you excellent work on this library. this is really usefull. :)
I'm trying to use Grab-n-Run to dynamically different classes on one apk.
In my apk i have a structure like this:

com
|-->impecabel
      |-->playerwrapper
             |-->Player.class
             |-->utils
                   |-->CustomSettings.class

Based on the sample app provided i made the following code:

protected void setUpProfileSecureDexClassLoader() {

        // First check: this operation can only start after
        // that the proper button has just been pressed..
        String classNameInAPK = "com.impecabel.playerwrapper.Player";
        //String classNameInAPK = "com.impecabel.playerwrapper.utils.CustomSettings";

        Log.d(TAG_MAIN, "Setting up SecureDexClassLoader for profiling..");
        // For the profiling test with SecureDexClassLoader I consider the worst performance scenario and so:
        // 1. The container is in a remote location and must be downloaded first
        // 2. The certificate, as well it's not cached but found at a remote URL and then imported
        // 3. The container in the end is correct so the full signature verification step is performed
        // 4. After the loading operation, the method to wipe out both the certificate and the container is invoked

        // Create an instance of SecureLoaderFactory..
        // It needs as a parameter a Context object (an Activity is an extension of such a class..)
        SecureLoaderFactory mSecureLoaderFactory = new SecureLoaderFactory(this);

        SecureDexClassLoader mSecureDexClassLoader;

        // Creating the apk paths list (only one path to a remote container in this case)
        String listAPKPaths = "https://dl.dropboxusercontent.com/u/XXXXXXXXX/Player.apk";

        // Filling the associative map to link package name and certificate..
        Map<String, URL> packageNamesToCertMap = new HashMap<String, URL>();
        // 1st Entry: valid REMOTE certificate location
        try {
            packageNamesToCertMap.put("com.impecabel.playerwrapper", new URL("https://dl.dropboxusercontent.com/u/XXXXXXXXX/cert.pem"));
        } catch (MalformedURLException e) {
            // An invalid URL was provided for remote certificate location
            Log.e(TAG_MAIN, "Invalid URL for remote certificate location!");
        }

        // Instantiation of SecureDexClassLoader
        mSecureDexClassLoader = mSecureLoaderFactory.createDexClassLoader(  listAPKPaths,
                null,
                ClassLoader.getSystemClassLoader().getParent(),
                packageNamesToCertMap);


        try {

            // Attempt to load dynamically the target class..
            Class<?> loadedClass = mSecureDexClassLoader.loadClass(classNameInAPK);

            // Immediately wipe out all the cached data (certificate, container)
            mSecureDexClassLoader.wipeOutPrivateAppCachedData(true, true);

            if (loadedClass != null) {


                Log.i(TAG_MAIN, "Found valid class: " + loadedClass.getSimpleName() + " : Success!");

                toastHandler.post(new Runnable() {

                    @Override
                    public void run() {
                        Toast.makeText(MainActivity.this,
                                "SecureDexClassLoader was successful! ",
                                Toast.LENGTH_SHORT).show();
                    }

                });

            } else {

                Log.w(TAG_MAIN, "This time the chosen class should pass the security checks!");
            }

        } catch (ClassNotFoundException e) {
            e.printStackTrace();
            Log.w(TAG_MAIN, "Class should be present in the provided path!!");
        } 
    }

When i try to load com.impecabel.playerwrapper.Player everything works fine and the class is loaded, but when i try to load the com.impecabel.playerwrapper.utils.CustomSettings it doesn't work and i get "This time the chosen class should pass the security checks!" on the logs.
Lookint at the documentation i think this should work. I'm i missing something?
If i put all the classes under com.impecabel.playerwrapper the both load.

Can you help? Do you need any logs?

Tanks in advance.

@lukeFalsina lukeFalsina self-assigned this Jan 14, 2016
@lukeFalsina
Copy link
Owner

Hello.
Thanks to you for your interest in Grab'n Run and I'm really pleased to hear that you find it useful :)

Given the structure of your APK, and the code that you provided, it also seems to me that you should be able to load both classes as long as the APK is properly signed using the private key associated to the certificate linked to the "com.impecabel.playerwrapper" package name prefix.

So this could be indeed a bug in the code.

I've just released a new version of Grab'n Run (1.0.3), where I have fixed several bugs and written a proper test suite for all the classes of the library. Please update your build.gradle file in the app module to use version 1.0.3, run again your code, and let me know whether the problem is solved or not.

Hope this helps :)

@jrocharodrigues
Copy link
Author

Hi,

Ok, tomorrow i'll try with 1.0.3 and let you know how it goes.

Thanks for the quick reply.

@jrocharodrigues
Copy link
Author

Hi,

I've just tried with version 1.0.3 and the result is the same :(
I'll make more tests try to narrow down the problem.
Please let me know if theres is some specific test (or log) you want me to make that you think that you think it might help to find the problem.

Thanks in advance

@jrocharodrigues
Copy link
Author

Hi,

I've changed the extension of my library from apk to jar and i was able to load all the classes in all sublevels.
I've narrow down the problem to SecureDexClassLoader.getPackageNamesFromContainerPath. When using an .apk file only the sublevels of the package are not parsed and not inserted to the map.

@lukeFalsina
Copy link
Owner

Now I see the problem, and I can also state that this is not a bug in Grab'n Run, but indeed the correct behavior.

APK by definition should only have one unique package name, which is defined when you create your application in an IDE, it is stored in the manifest, and it is used to identify your app on Google Play store.
Thus, it's indeed wrong to create an apk with several classes with different package names.

The easiest fix is, as you said, to simply move all the classes that you want to dynamically load into a jar archive (there you will be able to maintain your current hierarchy) and you can have more than one package name.

Does it make sense to you? :)

@jrocharodrigues
Copy link
Author

I understand what you've said, and so far i see no problem with the extension change (apk -> jar) workaround.
Can you elaborate, or point me to some documentation where i can find more info on why is wrong to create an apk with multiple hierarchy levels for the package name?

Thanks in advance

@lukeFalsina
Copy link
Owner

After a quick search I have not found any resource stating directly whether an apk with multiple hierarchy levels for its java classes should be considered as a good, or bad practice.

Anyway, I still tend to think that using different package names for the java classes into the same APK could be confusing or misleading (especially now that Gradle allows you to add flavors with different suffix to the application id), so I would rather prefer not doing it.

However, since I'm missing data points, I can't really state that the other approach is wrong.

@lukeFalsina
Copy link
Owner

Anyway, if you are able to provide me with some references making your point on having multiple package names in the same apk, I'll be more than happy to change the implementation to support this case. This would actually simplify the code (in the SecureDexClassLoader.getPackageNamesFromContainerPath method, I would have just to remove the distinction between apk and jar, and consider everything as a jar archive).

Otherwise, let me know and I will close this issue since it should have been solved.

@jrocharodrigues
Copy link
Author

Hi,
I see the usage of multiple hierarchy levels useful for code organization.
For instance, if my base package name is pt.impecabel.sampleapp, if i have lots of activities, and lots of fragments, i usually put all the activities under pt.impecavel.sampleapp.activities, and the fragments under pt.impecabel.sampleapp.fragments.
I'm short on time so i can't find any documentation supporting that method, but i see lot's of project doing the same, including the google apps and samples, eg: https://github.com/google/iosched

@reyammer
Copy link
Collaborator

Jose,

we internally discussed this, and we agree with you! @lukeFalsina will take care of this ASAP and will let you know!

@jrocharodrigues
Copy link
Author

Ok, thanks. :)

@lukeFalsina
Copy link
Owner

Hi again Jose,

I've released a new version (1.0.4) that should solve your issue.
In particular, I added a (now successful) test case to document what you presented with your example:
https://github.com/lukeFalsina/Grab-n-Run/blob/master/gnr/app/src/test/java/it/necst/grabnrun/SecureDexClassLoaderTest.java#L372

Let me know whether everything works now :)
Best

@lukeFalsina
Copy link
Owner

Jose,
I'll close this issue. In case of further troubles, let me know and I'll reopen it.

@jrocharodrigues
Copy link
Author

Hi,
Sorry i didn't answer before, I've only had the chance to test the new version today.
It works fine, the issue is solved.

Thanks for the help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants