1041852 - Consider enabling Proguard's obfuscation feature

Status: NEW

(Firefox Build System :: Android Studio and Gradle Integration, defect)

Description

[Chris Kitching [:ckitching]]

Proguard comes with an "obfuscation" feature that works by renaming all your classes and members to systematically-selected short names: "a", "b", "c"...

Why do we care?

While the stated goal of this feature is to hinder decompilation, due to a quirk of the Java classfile format it also has a very significant effect on binary size. My local build shrank by about 350kb when I enabled the feature.

(Try push at https://tbpl.mozilla.org/?tree=Try&rev=0ae5d954fb69 to get a more authoratative measure of the size difference)

Why does this happen?

The Java classfile format refers to members by name and type. For each referenced member, an entry is placed in the constant pool containing the name and canonical type string of that member. Bytecode instructions then use the index in the pool of that entry when referring to that member.
Since these strings contain the fully qualified name-and-type (Such as "org.mozilla.gecko.sync.setup.activities.SendTabData.fromBundle:(Landroid.os.Bundle)Lorg.mozilla.gecko.sync.setup.activities.SendTabData"), these strings end up occupying huge amounts of space.

The downside: Stack traces are ruined. Bright side: Proguard provides a utility for converting a mangled stack trace into a usable one (it records in a file the renaming transformations it applied and provides a program that can use this to fix your stacktrace. If you're using IntelliJ IDEA or Eclipse and the stacktrace is generated on a terminal in the IDE it'll do this magically behind the scenes for you, which is cool.).

Thought I'd bring this up because shrinking Fennec seems to be a matter of active discussion at the moment, and this doesn't seem to be something anyone's looked into before.

Perhaps it's worth the hassle?

Comment 1

[Richard Newman [:rnewman]]

We tend to rely on stacks produced by release builds, so enabling this would be contingent on being able to automatically recover those stacks.

Comment 2

[Chris Kitching [:ckitching]]

(In reply to Richard Newman [:rnewman] from comment #1)
> We tend to rely on stacks produced by release builds, so enabling this would
> be contingent on being able to automatically recover those stacks.

That is completely possible, you just need to store the mapping proguard spits out when you do the release build somewhere and go use that one to decode the stacks.
Plausibly that could be integrated into the crash reporter frontend so you never have to think about it.

Comment 3

[Kevin Brosnan [Ex-Mozilla]]

We would need to update the crash server processor. Code lives at https://github.com/mozilla/socorro lars and lonnen on irc should know the codebase.

Comment 4

[Kevin Brosnan [Ex-Mozilla]]

I would consider getting socorro to properly parse the crash report a blocker for any non-developer builds: nightly, aurora, beta and release.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Kevin Brosnan [:kbrosnan] from comment #4)
> I would consider getting socorro to properly parse the crash report a
> blocker for any non-developer builds: nightly, aurora, beta and release.

Indeed. This bug is blocked on any server work that needs to be done. Consider that against the savings in APK size. I think this is a lower priority than other work going on.

Comment 6

[Chris Kitching [:ckitching]]

The Proguard developer replied to my SO question about these failures:

http://stackoverflow.com/questions/24882343/how-to-stop-proguard-from-deleting-return-types

This is sort of alarming, as it means that our current configuration isn't strictly safe. (activating obfuscation causes absolutely every unprotected entry point to be broken, so it turns out to be a rather nice way to test).

This means that it isn't impossible for random Proguard failure to occur in the future if a class that is referenced only as a return type of an entry point is optimised out. That's sort of unlikely, but not impossible.
This also means that enabling obfuscation is impossible without doing one of:

1) Manually adding -keep directives for the classes it's deleting that it shouldn't (Ew)

2) Generating -keep directives for any class that is vulnerable in this way. This could be done right before the proguard step by a static analyser: look for everything annotated and spit out -keep directives for all their return types.

Okay. Yes. So, not nearly as easy as I first though. Interesting, though...

Comment 7

[Richard Newman [:rnewman]]

Caveats:

http://proguard.sourceforge.net/index.html#manual/limitations.html

including:

If an input jar and a library jar contain classes in the same package, the obfuscated output jar may contain class names that overlap with class names in the library jar. This is most likely if the library jar has been obfuscated before, as it will then probably contain classes named 'a', 'b', etc. Packages should therefore never be split across input jars and library jars.

Comment 8

[Nick Alexander :nalexander [he/him]]

I think we should re-consider enabling obfuscation.  As a first step, we might symbolicate only the third-party libraries that we ship, like support-v4, recyclerview, and things that we build that are static, like ch.boye.*.  Local experimentation suggests we might win ~250kb off classes.dex with this approach.

We already strip line number debugging information from such libraries.  We would, of course, lose some stack trace detail; but we'd almost always see a com.android or org.mozilla.gecko entrypoint in the stack, followed by obfuscated library entries.

Comment 9

[Chris Kitching [:ckitching]]

(In reply to Nick Alexander :nalexander from comment #8)
> I think we should re-consider enabling obfuscation.  As a first step, we
> might symbolicate only the third-party libraries that we ship, like
> support-v4, recyclerview, and things that we build that are static, like
> ch.boye.*.  Local experimentation suggests we might win ~250kb off
> classes.dex with this approach.
> 
> We already strip line number debugging information from such libraries.  We
> would, of course, lose some stack trace detail; but we'd almost always see a
> com.android or org.mozilla.gecko entrypoint in the stack, followed by
> obfuscated library entries.

Remember, you can use Proguard's "retracer" utility to get perfect stacktraces back, so even this isn't a downside except insofar as it requires you to set up slightly more build system trickery (capturing the obfuscator's retrace file and hooking it up to your debugger)