[Hilt] Disable "keepnames" proguard rule

[evowizz]

It looks like the following Proguard rule prevents Proguard from obfuscating ViewModel names https://github.com/google/dagger/blob/47e76741575bedcdd0a765d775c2424fcf6c9235/java/dagger/hilt/android/lifecycle/proguard-rules.pro#L1-L2

Is there a way to disable that Keep rule by chance? The first line indicates that the class names are used as multibinding map keys, but do these keys really need to use the original class names instead of the obfuscated ones?

[bcorso]

The first line indicates that the class names are used as multibinding map keys, but do these keys really need to use the original class names instead of the obfuscated ones?

Hi @evowizz , the reason we need to keep the class name is because the multibinding map is keyed by String on the class name, e.g. @IntoMap @StringKey("some.pkg.MyViewModel"). The issue is that when the class gets obfuscated the class name within the @StringKey is not and then the key no longer matches.

Note: we could have used @ClassKey for the multibinding map, however that has a runtime performance cost -- just creating the map would require class loading every viewmodel class -- so we decided against that approach too.

[evowizz]

Thanks for your reply @bcorso. Wouldn't it be possible to create a new Gradle task that replaces the "some.pkg.MyViewModel" String key with the obfuscated class name instead?

The reason I'm asking is because I believe this breaks (one of) the purposes of obfuscation. Having the ViewModel names within a release can considerably help reverse engineering an app. It may also keep a ton of text within the app depending on how many ViewModels there are, and how long the names of those ViewModels are.

[Chang-Eric]

Discussed this with @bcorso, we're going to add a flag to use -identifiernamestring instead of -keepnames. The default will be to use -identifiernamestring. Unfortunately, this is only supported in R8 so you will have to use that (and not just Proguard for example) in order to take advantage of this change. Hopefully that works for you.

[evowizz]

Thank you for looking into other solutions. It looks like there's barely any documentation about -identifiernamestring online. I believe it obfuscates strings? I believe I use R8 in my project, but it's quite difficult to differentiate Proguard from R8. But since I don't have any flags for disabling R8 that very likely means I'm probably using R8.

Obfuscating the key string (if that's what -identifiernamestring does) is a great solution, as it would also allow the class name to be obfuscated. Please let me know if I got something wrong. Thanks!

Edit: This commit from the Chromium team was actually quite helpful: https://chromium-review.googlesource.com/c/chromium/src/+/2451388

[Chang-Eric]

Right, it basically tells R8 that the string is a class name and should be obfuscated with the classes.

Thanks for that pointer, it looks like there may be some extra complications to using -identifiernamestring that I'll need to look into before doing this. It gets a little complicated because the string outputted by Hilt isn't the string actually used at runtime (that string is just a string in an annotation that is copied by Dagger into the component class that is generated), so we need to make sure there is a way to get R8 to follow it all the way through.

[mubashir86]

@bcorso how i can use @ClassKey,, please can you give an example with to use @ClassKey this in viewmodel ?

[jonathan-caryl]

I'd also be interested in a @ClassKey approach: we had these before migrating to Hilt, and require something to make our code work with Arxan. The -keepnames should work, but isn't ideal, from @Chang-Eric 's last comment I'm not sure if the -identifiernamestring works or not — but even if it does that's not something Arxan supports, so we'd need to make our protection step somewhat more complex.

[Chang-Eric]

We're unlikely to do anything with @ClassKey as an approach due to the performance issues @bcorso mentioned above.

However, using something like -identifiernamestring (actually I think we might need -adaptclassstring) is still probably the refinement we're looking to do, though last time I looked into it there were some complications.

@jonathan-caryl Either way though, if/when we make those changes, I think for something like Arxan (which I only first heard of through your comment), I think you'll just have to have some sort of equivalent rule for Arxan to understand how to rewrite these identifiers.

[Monabr]

Hi. So I should use -identifiernamestring to get my viewModel's name and package obfuscated? Please explain.

[Chang-Eric]

I don't have a solution (hence why this bug is still open). I'm just saying I think there is a path with -identifiernamestring or -adaptclassstring, but it still needs digging and possible work on our side.

[Monabr]

It's almost a year from the issue being opened, why is it still not resolved?

[Chang-Eric]

We have other priorities we are working on.

[Monabr]

This is a big security issue for applications using this library, I'm shocked that a year has passed and this is still not resolved! How long will this issue be open? 10, 20, 50 years?

[evowizz]

This is in no way a security issue. While it might prevent full obfuscation as stated in one of my first comments, it has nothing to do with security. It's an inconvenience at most.

If you are affected by this issue, feel free to subscribe to it and react to the first comment. Non-constructive comments do not help the community, nor developers.

[Monabr]

Showing part of your logic without obfuscating it is a problem. Moreover, if there are viewModels in different parts of the project, this reveals not only the names of the view models, potentially making it clear to which part of the application each of them belongs, but also the structure of the packages, since the path to them is saved. In fact, if you do not move the viewModels to a separate folder, the project becomes "at a glance" for potential "bad" people. I insist that this is a big problem for the security of the application.

[krioru]

Is this rule applied by default in my app or do I need to explicitly add it to the proguard-rules.pro?

[Monabr]

Hello. Is there any progress on this issue? It would be possible to replace identifiers in the form of class names with custom strings that would be written in the annotations to the ViewModel as is done for classes in Kotlin serialization. Please implement this. This is very necessary! It's been almost a year since my last comment!

[Monabr]

@Chang-Eric Is there any updated on this issue?

[Chang-Eric]

We've been looking at this recently but it is complicated because using Proguard's -adaptclassstrings may over apply to the whole app or to the whole Dagger component (since the class filter is used for the class containing the strings). Also, things like -identifiernamestring in R8 are a bit hard because it isn't a string variable being passed around like normal code. When you do @StringKey, the Dagger generator is copying the string into the Dagger component code so they are seen as two different instances, and so it is difficult to create a config on the Hilt side that goes through Dagger (Dagger and Hilt are still separate projects). Finally, similar to that last issue, string constants can be inlined by the compiler which similarly can break the tracking for those configs.

So we're looking at this, but there are a number of issues getting in the way.

[Monabr]

@Chang-Eric Please give the opportunity to manually set @StringKey and then you will not need to take it from the class name and, accordingly, keep these class names and the package path to these classes not obfuscated.

[o3androiddev]

Im working on banking application, and because of this issue, i think i need to remove hilt from the project. is there is any way around available?

[Chang-Eric]

Just to clarify, this issue should only prevent obfuscation of the static class names of the Hilt ViewModels. This isn't going to leak any runtime or user data. Just clarifying since you mentioned a banking application.

With that said, there isn't really a workaround, besides doing something like renaming your @HiltViewModel classes yourself to some name that you don't mind end up being in the APK. Not a great solution obviously, but that is all that I'm aware of as a workaround.

However, we are currently actively working on this, and hope to have the first part out pretty soon. The first part will be a new feature in Dagger, and then once that is done we'll have to update Hilt to use that new feature. We ran into some complications though, so this new feature will only work with R8 and not Proguard. For Proguard users we're going to keep using -keepnames.

[NomiTiger]

What's the expected time for an update on this issue?

[wanyingd1996]

Hi, I'm perform a release now, should be done in 1~2 days