In the BillingService module, what needs to be modified to increase security?

Question

The comment for the class BillingService recommends that:

You should modify and obfuscate this code before using it.

OK, but what must be modified?

The class name? The TAG used for logging? Method names and data members? The logic and program flow itself? Other?

In other words, I can understand the need for obfuscation, but how can I get away with implementing the recommendation without rewriting everything from scratch (potentially with bugs that are worse than not modifying anything)?

Answer 1

I'm working on this at this moment and my approach, so far, is as follows:

1. I'm using the BillingReceiver, Billing Service, PurchaseObserver and ResponseHandler.
2. I've moved all the Constants into my own Constants class and all the above classes are included in my own package.
3. I've done away with the PurchaseDatabase class and integrated parts of it into my own SQLite database, DBAdapter and data access classes.
4. I've changed the CatalogEntry into my own model object and my UI will be quite different to the example e.g. RadioButton group rather than Spinner for product items (I only have 4).
5. It says in the Security class 'For a secure implementation, all of this code should be implemented on a server that communicates with the application'. I'm 'fortunate' that my app has to contact my server anyway so I'll be implementing these security measures on the server and I'll be doing my own validation of the purchase info passed to the server. I'm looking to secure this part of the comms using SSL and I already require a prior username/password (hashed and salted) which is stored on my server.
6. I'm cutting out any other superfluous code which I'm not using e.g. payload editing.
7. Some of the methods have 5 or 6 parameters in their signature e.g. onPurchasestateChanged() - I was thinking about combining these into a single wrapper object (but haven't done so yet).
8. I'm testing it slowly and thoroughly, so that I understand what's going on, and following the recommendations. I used the complete sample at first to make sure it worked and tested the static responses. Then I started making my own changes while still doing static testing. I'm still testing with static responses and I will follow the flow of messages to understand the interchanges going on. Once I'm happy with this I'll test with my own product Id's and try and satisfy myself on the data and its security.
9. I've thought that the developerPayload string could also be signed and encypted and when returned to my server, decrypted and checked for integrity.
10. Finally, I'll obfuscate the code using ProGuard and follow some of the tips for doing so which are available on StackOverflow.

Hope this helps.

Answer 2

No good news here: You need to change anything you can, in addition to using Proguard. This includes merging classes, splitting them, moving certain methods from one module to another, and especially encrypting the purchase information stored into the database, as the description for the PurchaseDatabase class suggests:

You should use an obfuscator before storing any information to persistent storage. The obfuscator should use a key that is specific to the device and/or user. Otherwise an attacker could copy a database full of valid purchases and distribute it to others.

The reason is that with tools like AntiLVL it is very easy to compare your decompiled (obfuscated!) code to the original sample and deduct from it whatever needed to compromise it. It is impossible to completely prevent cracking, but you should try to make it as difficult as possible.