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

Randomised name generation #1873

Closed
supersteves opened this issue Jul 6, 2016 · 16 comments
Closed

Randomised name generation #1873

supersteves opened this issue Jul 6, 2016 · 16 comments
Assignees

Comments

@supersteves
Copy link
Contributor

I'd like to be able to randomise the name generation such that two compilations result in entirely different names. This makes it harder to reverse-engineer the compilation since every continuous integration build of the software would have far less in common.

I propose either of the following, and will put together a PR if it would be likely to be accepted:

  1. Make DefaultNameGenerator and NameGenerator public, and add a public accessor in CompilerOptions (I'll supply my own, likely a subclass of DefaultNameGenerator)
  2. Or, add CompilerOption.randomiseNameGeneration, and have DefaultNameGenerator.reset() shuffle instead of sort those two arrays.
@MatrixFrog
Copy link
Contributor

I'm leaning toward saying no to this, because we want builds to be completely deterministic. It's not a bad idea though so I would encourage you to publish it and maybe we can link to it from the wiki somewhere even if we don't accept it into the official project.

@acleung
Copy link

acleung commented Jul 6, 2016

It's been done before. At one point we have a special NameGenerator that
takes in a seed to randomize the variable names.

The level of obfuscation and the size penalty makes it not worth it so I
deleted it at some point (IIRC).

On Wed, Jul 6, 2016 at 11:02 PM, Tyler Breisacher notifications@github.com
wrote:

I'm leaning toward saying no to this, because we want builds to be
completely deterministic. It's not a bad idea though so I would encourage
you to publish it and maybe we can link to it from the wiki somewhere even
if we don't accept it into the official project.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#1873 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABjAWisb8d4lNFA1hP78BolHHaBIDhwuks5qTBfggaJpZM4JGQ6S
.

@supersteves
Copy link
Contributor Author

@acleung I've found a way to prototype it using reflection (to work around the package protection), by calling favors() followed by reset() to establish randomised values of CharPriority.occurrence while preserving the order of sections lowercase, uppercase, numbers, symbols. This results in identical file size but entirely random output. Would this impact compressed size? Otherwise I'm not sure what the size penalty could be. So my PR would be just to make this class and interface public, with a public options setter.

@ChadKillingsworth
Copy link
Collaborator

I've actually considered adding this in as well. It makes it harder for malware to target your code if it is constantly changing.

I was simply going to make a custom build to do this however.

@acleung
Copy link

acleung commented Jul 11, 2016

If there is enough interest, it is probably worth rewriting one. If it
takes in a seed, it doesn't break the compiler's determinism requirement.

It impacted compressed size because the name generator does tricky things
that compress well after gzip.

On Thu, Jul 7, 2016 at 5:42 AM, Chad Killingsworth <notifications@github.com

wrote:

I've actually considered adding this in as well. It makes it harder for
malware to target your code if it is constantly changing.

I was simply going to make a custom build to do this however.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1873 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABjAWoN2MjexvTJsQ6Cdqsh3OJwf2pP4ks5qTPRFgaJpZM4JGQ6S
.

@ChadKillingsworth
Copy link
Collaborator

Isn't it enough to simply randomize the order of supported characters?

https://github.com/google/closure-compiler/blob/master/src/com/google/javascript/jscomp/DefaultNameGenerator.java#L84-L96

@supersteves
Copy link
Contributor Author

@supersteves
Copy link
Contributor Author

I have a working prototype, which I'll clean up and post as a PR if I it's likely to be accepted.

@acleung
Copy link

acleung commented Jul 11, 2016

Randomizing the support letters was the previous approach. It still have a
bit of size penalty but probably not significant enough for most
applications.

On Mon, Jul 11, 2016 at 7:59 AM, Steve Spencer notifications@github.com
wrote:

I have a working prototype
#1873 (comment),
which I'll clean up and post as a PR if I it's likely to be accepted.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1873 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABjAWpNCGl-eza2IYlQ6CBZZd6sU0eJgks5qUlpTgaJpZM4JGQ6S
.

@dimvar
Copy link
Contributor

dimvar commented Jul 11, 2016

have a working prototype, which I'll clean up and post as a PR if I it's likely to be accepted.

@supersteves can you relink to your patch? The earlier link isn't right.

@supersteves
Copy link
Contributor Author

@dimvar I was linking to the comment above.

by calling favors() followed by reset() to establish randomised values of CharPriority.occurrence while preserving the order of sections lowercase, uppercase, numbers, symbols.

Effectively it's as if I've shuffled each subsection of the char arrays if it weren't for the Array.sort recent additions. By subsection I preserve the grouping of lowercase, uppercase and numbers, but reorder the array within those subsequences (sort-of preserving the same camelcase).

This approach works well, but no patch yet, it's a reflection based prototype/hack in external code, I'll need to rework it into a PR and will only do it if it's likely to be of interest.

@dimvar
Copy link
Contributor

dimvar commented Jul 12, 2016

It seems that your approach is what others in this thread also recommend, so yes, we'll likely accept it.

@blickly
Copy link
Contributor

blickly commented Jul 12, 2016

I'm certain I've seen existing code that does this already. Let me see if I can get it open-sourced so you don't have to duplicate the work.

@dimvar
Copy link
Contributor

dimvar commented Jul 12, 2016

OK, @blickly just assigned the issue to you.

@supersteves
Copy link
Contributor Author

Great!

@blickly
Copy link
Contributor

blickly commented Jul 14, 2016

Should be fixed by 2bb100f

@blickly blickly closed this as completed Jul 14, 2016
@supersteves
Copy link
Contributor Author

👍 Thanks!

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

No branches or pull requests

6 participants