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

[postfix] Run the check as postfix user, not root #619

Merged
merged 4 commits into from
Jul 26, 2017
Merged

Conversation

masci
Copy link
Contributor

@masci masci commented Jul 24, 2017

What does this PR do?

No need to run find as root, let's sudo -u postfix instead.
The actual user name for the postfix user is configurable to avoid to hardcode it.

Motivation

Improve the check security wise

Versioning

  • [x ] Bumped the version check in manifest.json
  • [x ] Updated CHANGELOG.md

@masci masci added this to the 5.16 milestone Jul 24, 2017
postfix/check.py Outdated
@@ -61,7 +63,8 @@ def _get_queue_count(self, directory, queues, tags):
# can dd-agent user run sudo?
test_sudo = os.system('setsid sudo -l < /dev/null')
if test_sudo == 0:
output, _, _ = get_subprocess_output(['sudo', 'find', queue_path, '-type', 'f'], self.log, False)
postfix_user = self.init_config.get('postfix_user')
Copy link
Member

Choose a reason for hiding this comment

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

without default value the new check will not work with an old configuration (ie: without the postfix_user). I believe that would be a breaking change (and the new version indicate a minor change). We should add a default value (root for backward compatibility ?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch

@kmshultz
Copy link
Contributor

Can we show the new option in the conf example in README, as well?

@truthbk truthbk modified the milestones: 5.16, 5.17 Jul 25, 2017
@masci masci merged commit 3d63c5e into master Jul 26, 2017
@masci masci deleted the massi/postfix branch July 26, 2017 10:55
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.

None yet

4 participants