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

Default profile is not used if there is no "profile" key in the config #87

Open
Svieg opened this issue Oct 19, 2017 · 4 comments
Open
Labels
help wanted Extra attention is needed

Comments

@Svieg
Copy link
Collaborator

Svieg commented Oct 19, 2017

@ProloG-Shaman was using malboxes and we found out that the default profile wasn't used because there was a check to see if the config had a "profile" key and if not, didn't apply any profile. The problem is, that renders the default profile useless. We then have to make a choice if we want to apply the profile-example.js for a default built. I personally think that we should.

@obilodeau
Copy link
Contributor

I want to revert the change you merged in #88.

I explicitly disabled profile use by default. See my commit and explanation here: d564c93

Profiles is an experimental feature, the default config is easily fingerprintable and doesn't add value. This feature should be activated by end-users. Also, I disabled it because when you upgrade your malboxes the build would crash unless you modify your config (which malboxes doesn't do on upgrades). This is why I populate the profile with the default one now and I added the tests on template generation (see above commit).

If the default profile would do something like improve our fingerprinting resistance then I would see less of a problem enabling it by default. But right now it doesn't and it affected backwards compatibility.

What do you think?

@Svieg
Copy link
Collaborator Author

Svieg commented Oct 28, 2017

I think we should have a default profile (I agree we might want to do some prior testing before releasing it again) that would hide all the hypervisor artifacts (the use case where you want those is pretty rare). I think we mixed the concept of default profile and testing profile (that could be used to test for regressions). That being said, I agree we can revert this commit and change the default profile later to reflect those changes. Having a default profile highlights the way profiles work and highlights also the fact that they exist. Apart from that, can you explain the backwards compatibility issue?

@obilodeau
Copy link
Contributor

have a default profile [...] that would hide all the hypervisor artifacts

Yes! That would be good. Although it wouldn't use the "package" keyword and would not demonstrate all the features but we can leave a commented example.

can you explain the backwards compatibility issue?

Before I made the fixes in d564c93, if you upgraded and had no profile statement in config (like everyone because the concept didn't exist) you would get a packer config that would make it crash.

The more I think about malboxes and talk about it, the more I feel we will eventually move towards a Vagrantfile or Dockerfile approach. One good example is someone who did a malboxes spin in it's C:\Users\<user> and launched it without realizing that it would expose all it's files to the VM. Safe malware analysis is not safe enough 😉. This needs more thoughts though.

obilodeau added a commit that referenced this issue Oct 31, 2017
@obilodeau
Copy link
Contributor

I just reverted the commit. Let's keep this ticket open to track that we want to enable it once we have a good default profile (and perform backward compatibility testing).

@obilodeau obilodeau added the help wanted Extra attention is needed label Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants