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

Remove Bad reasons section #114

Closed
wants to merge 1 commit into from

Conversation

ghoneycutt
Copy link

This section is redundant as under Good reasons it mentions that compilers are dev tools and should not be on a system. The section is also very biased to the authors ideas and their own experiences and environments. Compilers not on systems for security is a very established practice and has corresponding policies in many environments and regulations such as DISA STIG's.

This section is redundant as under Good reasons it mentions that compilers are dev tools and should not be on a system. The section is also very biased to the authors ideas and their own experiences and environments. Compilers not on systems for security is a very established practice and has corresponding policies in many environments and regulations such as DISA STIG's.
@majormoses
Copy link
Member

I am clearly biased as I am the one who wrote it, I would not say its redundant nor do I agree that a system is that much more secure because it does not have a compiler just because it's an established pattern as there are plenty of terrible established patterns. To me its security theater unless you also remove all (including shell) scripting languages so only pre-compiled and approved software is run. As that is not realistic or possible with sensu that was the stance I took when writing this.

I will defer to @eheydrick and @mattyjones if they feel differently but I'd rather either leave it as is or add/modify to clarify.

@mattyjones
Copy link
Member

mattyjones commented Jun 2, 2018

@majormoses @ghoneycutt
I don't understand the issue here. Is it because the same thing is said twice? Is it the personal view on why having a compiler on a system is bad?

I agree 100% the passage comes off as personal, the simple use of first person pronouns means this should be refactored at the least. Concerning if it should be deleted, I think it should be removed but I also think the point about about dev tools not being on a production system should be written as such to say this is a common practice and not just a security concern.

I am in the security space FWIW, and agree compilers and headers have no place in production, but for many reasons including disk space, need, cruft, and security. With that said I also see the point that every system has python/perl/bash and as much damage can be wrought with them as well.

I say remove it and say dev tools are frowned upon due to the following reasons:

  • increased attack surface
  • encourages dev's to "do it live"
  • machines should be lean

Here is a some what real world example.
If a malicious user is compiling code on the box they already have a foot hold and Python/Perl/Shell can be just as dangerous. The flipside to Python/Perl vs C is that many dangerous system calls are not as readily available with Python/Perl. An attacker may also try and use debugging tools to attack your company's code that they may not be otherwise able to access or if a dev forgot to delete the source they can now have source code access and can introduce a flaw and compile/install it possibly. At least that is what I would try and do ;)

In the end, I approve of the commit but would like to see the changes I talked about implemented in a cleaner fashion than my rambling or further discussion on this.

@majormoses
Copy link
Member

I agree 100% the passage comes off as personal, the simple use of first person pronouns means this should be refactored at the least.

Agreed

Concerning if it should be deleted, I think it should be removed but I also think the point about about dev tools not being on a production system should be written as such to say this is a common practice and not just a security concern.

That is fine by me, the original intent behind this was to provide some push back on security folks trying to get you to do things that do not make sense. If you look at what sensu is and what it is capable of you realize sensu is a remote code execution platform, the least of your concerns is the presence of a compiler if not properly secured. I have encountered very few places that have configured safe_mode, disabled sockets (some of which are fairly new features) or configured them for auth, properly restricted sudoers, etc which have way more value in reducing the overall attack surface and its containment in an attack than simply not have a compiler will ever bring and will be much less effort.

My point is focus on the things that really matter and if you are doing everything right (chances are you are not) then yes it does have a somewhat meaningful mitigation. If you do not properly handle those more real attack vectors an attacker could just as easily have sensu install a compiler rendering your efforts useless and only wasting your time. I think that maybe we should add some verbiage around this being an extremely advanced topic and if you care about sensu's security you should only put in effort to do this after completing all the other outlined steps.

@majormoses
Copy link
Member

majormoses commented Jun 19, 2018

@ghoneycutt I have not heard back from you, are you planning on circling back to this?

@ghoneycutt
Copy link
Author

You agreed that it should be removed. What else is there?

@majormoses
Copy link
Member

@ghoneycutt it should be removed and new content should be added to address points myself and Matty made. I am only supportive of removing it with the additions discussed.

@ghoneycutt
Copy link
Author

I think the good reasons listed are enough and we don't need to go into anything further regarding security. If you think something else needs to be said, send a pull request and we can comment on it.

@majormoses
Copy link
Member

majormoses commented Jun 19, 2018

I am going to respectfully disagree, you can either make the requested changes/additions or we should close this and open an issue to properly address your concern with someone who will put in the time to properly flesh it out in a new pull request.

Copy link
Member

@majormoses majormoses left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Address the comments. People are not gonna find this especially given the PR title.

@majormoses
Copy link
Member

majormoses commented Jun 19, 2018

I have created #117 for whoever wants to improve the documentation. @ghoneycutt thank you for bring up valid points/concerns and when someone who has enough time to properly write this up will grab it when they can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants