Skip to content

Disable storing core dumps.#4650

Merged
yuumasato merged 3 commits intoComplianceAsCode:masterfrom
adelton:coredump_storage_none_processsizemax_0
Jul 26, 2019
Merged

Disable storing core dumps.#4650
yuumasato merged 3 commits intoComplianceAsCode:masterfrom
adelton:coredump_storage_none_processsizemax_0

Conversation

@adelton
Copy link
Copy Markdown
Contributor

@adelton adelton commented Jul 24, 2019

Description:

  • Disable storing core dumps.

Rationale:

  • A core dump includes a memory image taken at the time the operating system terminates an application. The memory image could contain sensitive data and is generally useful only for developers trying to debug problems.

@adelton
Copy link
Copy Markdown
Contributor Author

adelton commented Jul 24, 2019

Technically the coredump.conf is an ini file, not a plain config file. However, there is only one section, [Coredump] there, so we won't be looking for the value in wrong section. However, if there is no section in that file, me might match and/or add the value outside of the expected section.

Is that a problem?

Copy link
Copy Markdown
Member

@ggbecker ggbecker left a comment

Choose a reason for hiding this comment

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

Hi @adelton, thanks for the patch. I have a few requests:

1 - The rule id for the rules you've created need to reflect the meaning of the configuration. I suggest to change coredump_processsizemax_0 to something like coredump_disable_backtraces, same situation with coredump_storage_none -> coredump_disable_storage.

2 - Regarding the ini configuration file, I've made some inline comments. But overall is ok to leave like that because we still don't support ini configuration files in the newly introduced macros/functions. But I'd like to ask @yuumasato if this is ok.

3 - Please add basic path test scenarios so the rules can be easily tested.

Comment thread shared/macros-oval.jinja Outdated
Comment thread shared/macros-ansible.jinja Outdated
Comment thread shared/bash_remediation_functions/include_lineinfile.sh Outdated
@adelton adelton force-pushed the coredump_storage_none_processsizemax_0 branch from aa1c164 to 4cf02c2 Compare July 25, 2019 13:26
@adelton
Copy link
Copy Markdown
Contributor Author

adelton commented Jul 25, 2019

I've renamed the rules, added the section to OVAL, added the tests -> 4cf02c2.

Copy link
Copy Markdown
Member

@yuumasato yuumasato left a comment

Choose a reason for hiding this comment

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

I think we should treat it like an ini file, and mention correct section in rule.yml

By default /etc/systemd/coredump.conf already exists and [Coredump] section is there.
For this rules, we can state that the supported use case is remediation from default installation state, and remediation when file doesn't exist or doesn't have [Coredump] section is unsupported.
I suggest adding a general warning in the rule.yml, describing supported use case.

For the remediation, I suggest that they don't create the file, and add to end of file.
The ocil text should help remediate manually, so I think the correct section should be listed there as well.

@ggbecker
Copy link
Copy Markdown
Member

ggbecker commented Jul 25, 2019

I think we should treat it like an ini file, and mention correct section in rule.yml

By default /etc/systemd/coredump.conf already exists and [Coredump] section is there.
For this rules, we can state that the supported use case is remediation from default installation state, and remediation when file doesn't exist or doesn't have [Coredump] section is unsupported.
I suggest adding a general warning in the rule.yml, describing supported use case.

In order to illustrate the warnings attribute:

warnings:
- general: |-
If the <tt>.json</tt> file in
<tt>/etc/chromium/policies/managed</tt> is not formatted correctly,
no policies will be configured or set correctly.

@yuumasato
Copy link
Copy Markdown
Member

I also suggest creating an issue to track that this rule needs to be updated to use ini capabilities when ready.

@adelton adelton force-pushed the coredump_storage_none_processsizemax_0 branch from 4cf02c2 to a0ad45d Compare July 26, 2019 08:27
@adelton
Copy link
Copy Markdown
Contributor Author

adelton commented Jul 26, 2019

I've now rebased on master and added service_systemd-coredump_disabled -> a0ad45d.

@adelton
Copy link
Copy Markdown
Contributor Author

adelton commented Jul 26, 2019

Added an explicit commit to add the bits about [Coredump] section and to also not create the file when it is missing (as we would not add that section line) -> bb59ff9.

@yuumasato
Copy link
Copy Markdown
Member

Thanks for the changes.

@yuumasato yuumasato merged commit 1d3e8d2 into ComplianceAsCode:master Jul 26, 2019
@yuumasato yuumasato self-assigned this Jul 26, 2019
@yuumasato yuumasato added this to the 0.1.46 milestone Jul 26, 2019
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

Successfully merging this pull request may close these issues.

3 participants