Set default umask of 0027 for all Beats-created files #14119

ycombinator · 2019-10-17T19:48:25Z

Resolves #14005

This PR sets a default umask of 0027 on non-Windows systems for all Beats. An individual Beat may choose to override this default umask via the libbeat/cmd/instance.Settings.Umask field.

This means that, by default:

any files created by any Beat will have at most 0640 permissions, and
any directories created by any Beat will have at most 0750 permissions.

dedemorton

LGTM

urso · 2019-10-21T15:30:59Z

libbeat/cmd/instance/umask_other.go

+
+import "syscall"
+
+func setUmask(newmask int) int {


we should return an error instead of a number. See: syscall.Errno for example.

urso · 2019-10-21T15:31:45Z

libbeat/cmd/instance/beat.go

@@ -147,6 +147,7 @@ func initRand() {
 // instance.
 // XXX Move this as a *Beat method?
 func Run(settings Settings, bt beat.Creator) error {
+	setUmaskWithSettings(settings)


Should we log failures? E.g. add an ErrNotImplemented and only log if err != nil && err != ErrNotImplemented.

Yes, I think this is a good idea. Thanks, will implement.

ph · 2019-10-21T16:26:46Z

implementation look good, I would be + to add @urso comment.

urso · 2019-10-21T17:06:56Z

libbeat/cmd/instance/umask_windows.go

+
+func setUmask(newmask int) error {
+	// No way to set umask on Windows
+	return types.ErrNotImplemented


This is an internal error. We should not reuse sentinal errors from 3rd party packages (packages vendored by beats).

Thanks, wasn't sure about whether this was a good idea or not. Will change.

ycombinator · 2019-10-21T17:18:25Z

@urso @ph I've addressed the feedback so far. Please re-review when you get a chance. Thanks!

ph

@ycombinator the code LGTM but can you add a changelog?

ycombinator · 2019-10-23T20:39:37Z

can you add a changelog?

Thanks, 🤦‍♂, I keep forgetting to do that!

ycombinator · 2019-10-24T00:30:06Z

Travis CI is green. All jobs except x-pack/agent are green on Jenkins CI as well. Looks like that job is yellow for master builds as well.

So merging this PR.

…14216) * Set default umask of 0027 for all Beats-created files (#14119) * Set umask of 027 (on non-windows systems) for all Beats-created files * Fixing comment * Update tests * Updating docs for filebeat.registry.file_permissions * Denoting octal * Allow beats to override umask * Removing accidentally-committed file * Adding system test for default umask * Make setUmask return error * Defining ErrNotImplemented locally * Defining not implemented error locally * Fixing typo * Skip umask test on Windows * Adding missed imports * Adding CHANGELOG entry * Fixing up CHANGELOG

tomrade · 2020-02-17T17:59:05Z

How does this affect the "file" output as I have been unable to make log file of 0644 and I figured this was why?
I want a file with this as im writing out auditbeat json which is then forwarded by rsyslog (which does not run as root, but auditbeat does run as root)

ycombinator · 2020-02-20T19:49:50Z

@tomrade I just tested this with an older version of Metricbeat (version 7.2.0, released before the change in this PR was made). And I also tested the latest version of Metricbeat (built from master). The permissions of the resulting folder + log file under it were the same between versions. So this change has no effect on the file output's behavior.

* Set umask of 027 (on non-windows systems) for all Beats-created files * Fixing comment * Update tests * Updating docs for filebeat.registry.file_permissions * Denoting octal * Allow beats to override umask * Removing accidentally-committed file * Adding system test for default umask * Make setUmask return error * Defining ErrNotImplemented locally * Defining not implemented error locally * Fixing typo * Skip umask test on Windows * Adding missed imports * Adding CHANGELOG entry

larryon · 2021-05-13T06:55:25Z

Would be good to document these kind of changes

…sk (#20584) (#28347) Changes implemented in the (#14119) made all Beats-created files and folders apply an umask of 0027 (on POSIX systems). Co-authored-by: dplavcic <dplavcic@users.noreply.github.com>

…sk (#20584) (#28347) Changes implemented in the (#14119) made all Beats-created files and folders apply an umask of 0027 (on POSIX systems). Co-authored-by: dplavcic <dplavcic@users.noreply.github.com> (cherry picked from commit ecd68db)

…sk (#20584) (#28347) (#29503) Changes implemented in the (#14119) made all Beats-created files and folders apply an umask of 0027 (on POSIX systems). Co-authored-by: dplavcic <dplavcic@users.noreply.github.com> (cherry picked from commit ecd68db) Co-authored-by: Dalibor P <9079844+dplavcic@users.noreply.github.com>

…c#14119)" This reverts commit 2bb87cf.

…14119) (elastic#14216) * Set default umask of 0027 for all Beats-created files (elastic#14119) * Set umask of 027 (on non-windows systems) for all Beats-created files * Fixing comment * Update tests * Updating docs for filebeat.registry.file_permissions * Denoting octal * Allow beats to override umask * Removing accidentally-committed file * Adding system test for default umask * Make setUmask return error * Defining ErrNotImplemented locally * Defining not implemented error locally * Fixing typo * Skip umask test on Windows * Adding missed imports * Adding CHANGELOG entry * Fixing up CHANGELOG

ycombinator changed the title ~~Set umask of 027 for all Beats-created files~~ Set umask of 0027 for all Beats-created files Oct 18, 2019

ycombinator changed the title ~~Set umask of 0027 for all Beats-created files~~ Set default umask of 0027 for all Beats-created files Oct 18, 2019

ycombinator requested a review from dedemorton October 18, 2019 19:03

ycombinator marked this pull request as ready for review October 18, 2019 19:04

ycombinator requested review from urso and ph October 18, 2019 19:04

ycombinator added Filebeat Filebeat libbeat review v7.6.0 v8.0.0 labels Oct 18, 2019

dedemorton approved these changes Oct 18, 2019

View reviewed changes

urso reviewed Oct 21, 2019

View reviewed changes

ycombinator added 11 commits October 22, 2019 11:21

Set umask of 027 (on non-windows systems) for all Beats-created files

cfee09e

Fixing comment

f9eeffb

Update tests

35f2ad2

Updating docs for filebeat.registry.file_permissions

b1c0efd

Denoting octal

e7686d4

Allow beats to override umask

66f3ca9

Removing accidentally-committed file

9f8bb39

Adding system test for default umask

5217121

Make setUmask return error

2e9be6e

Defining ErrNotImplemented locally

a2b157b

Defining not implemented error locally

0290863

ycombinator force-pushed the lb-umask branch from 2135488 to 0290863 Compare October 22, 2019 18:22

Fixing typo

62ae1f1

ycombinator added 2 commits October 22, 2019 14:37

Skip umask test on Windows

c213c50

Adding missed imports

d408ac2

ph approved these changes Oct 23, 2019

View reviewed changes

Adding CHANGELOG entry

2d30f98

urso approved these changes Oct 23, 2019

View reviewed changes

ycombinator merged commit 2bb87cf into elastic:master Oct 24, 2019

ycombinator added the v7.5.1 label Oct 24, 2019

ycombinator mentioned this pull request Oct 24, 2019

[7.5] Set default umask of 0027 for all Beats-created files (#14119) #14216

Merged

ycombinator deleted the lb-umask branch December 25, 2019 11:09

vaclavms mentioned this pull request May 15, 2020

[libbeat] output.file ignores permissions #18563

Closed

ycombinator mentioned this pull request Aug 13, 2020

[Docs] Update logging.files.permissions documentation to consider umask #20584

Closed

AndyHunt66 mentioned this pull request Mar 16, 2021

logging.files.permissions setting is not taken into account #24555

Closed

saibug mentioned this pull request Apr 30, 2021

[filebeat] default owner of output file #25455

Closed

axw mentioned this pull request Aug 6, 2021

[docs] logging.files.permissions config is misleading elastic/apm-server#5899

Closed

dplavcic mentioned this pull request Oct 11, 2021

[Docs] Update logging.files.permissions documentation to consider umask (#20584) #28347

Merged

5 tasks

rdner mentioned this pull request Jan 5, 2022

Move umask from code to service files #29708

Merged

6 tasks

rdner added a commit to rdner/beats that referenced this pull request Jan 14, 2022

Revert "Set default umask of 0027 for all Beats-created files (elasti…

f78884d

…c#14119)" This reverts commit 2bb87cf.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set default umask of 0027 for all Beats-created files #14119

Set default umask of 0027 for all Beats-created files #14119

ycombinator commented Oct 17, 2019 •

edited

Loading

dedemorton left a comment

urso Oct 21, 2019

urso Oct 21, 2019 •

edited

Loading

ycombinator Oct 21, 2019

ph commented Oct 21, 2019

urso Oct 21, 2019

ycombinator Oct 21, 2019

ycombinator commented Oct 21, 2019

ph left a comment

ycombinator commented Oct 23, 2019

ycombinator commented Oct 24, 2019

tomrade commented Feb 17, 2020

ycombinator commented Feb 20, 2020

larryon commented May 13, 2021

Set default umask of 0027 for all Beats-created files #14119

Set default umask of 0027 for all Beats-created files #14119

Conversation

ycombinator commented Oct 17, 2019 • edited Loading

dedemorton left a comment

Choose a reason for hiding this comment

urso Oct 21, 2019

Choose a reason for hiding this comment

urso Oct 21, 2019 • edited Loading

Choose a reason for hiding this comment

ycombinator Oct 21, 2019

Choose a reason for hiding this comment

ph commented Oct 21, 2019

urso Oct 21, 2019

Choose a reason for hiding this comment

ycombinator Oct 21, 2019

Choose a reason for hiding this comment

ycombinator commented Oct 21, 2019

ph left a comment

Choose a reason for hiding this comment

ycombinator commented Oct 23, 2019

ycombinator commented Oct 24, 2019

tomrade commented Feb 17, 2020

ycombinator commented Feb 20, 2020

larryon commented May 13, 2021

ycombinator commented Oct 17, 2019 •

edited

Loading

urso Oct 21, 2019 •

edited

Loading