From 6140a1921d8e77922e529b5e0fe6ec87261077a5 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Mon, 28 Oct 2019 10:46:01 -0700 Subject: [PATCH] [7.5] Set default umask of 0027 for all Beats-created files (#14119) (#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 --- CHANGELOG.next.asciidoc | 1 + .../docs/filebeat-general-options.asciidoc | 6 ++-- filebeat/tests/system/test_registrar.py | 8 ++--- libbeat/cmd/instance/beat.go | 12 +++++++ libbeat/cmd/instance/settings.go | 2 ++ libbeat/cmd/instance/umask_other.go | 32 +++++++++++++++++ libbeat/cmd/instance/umask_windows.go | 27 ++++++++++++++ libbeat/tests/system/config/mockbeat.yml.j2 | 3 ++ libbeat/tests/system/test_umask.py | 36 +++++++++++++++++++ 9 files changed, 121 insertions(+), 6 deletions(-) create mode 100644 libbeat/cmd/instance/umask_other.go create mode 100644 libbeat/cmd/instance/umask_windows.go create mode 100644 libbeat/tests/system/test_umask.py diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 71120b1a09f..70834485e6d 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -11,6 +11,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d *Affecting all Beats* - Update to Golang 1.12.1. {pull}11330[11330] +- By default, all Beats-created files and folders will have a umask of 0027 (on POSIX systems). {pull}14119[14119] *Auditbeat* diff --git a/filebeat/docs/filebeat-general-options.asciidoc b/filebeat/docs/filebeat-general-options.asciidoc index 9de97f2949a..d0310ec92dc 100644 --- a/filebeat/docs/filebeat-general-options.asciidoc +++ b/filebeat/docs/filebeat-general-options.asciidoc @@ -42,13 +42,15 @@ NOTE: The content stored in filebeat/data.json is compatible to the old registry The permissions mask to apply on registry data file. The default value is 0600. The permissions option must be a valid Unix-style file permissions mask expressed in octal notation. In Go, numbers in octal notation must start with 0. +The most permissive mask allowed is 0640. If a higher permissions mask is +specified via this setting, it will be subject to a umask of 0027. + This option is not supported on Windows. Examples: - 0644: give read and write access to the file owner, and read access to all others. + 0640: give read and write access to the file owner, and read access to members of the group associated with the file. 0600: give read and write access to the file owner, and no access to all others. - 0664: give read and write access to the file owner and members of the group associated with the file, as well as read access to all other users. [source,yaml] ------------------------------------------------------------------------------------- diff --git a/filebeat/tests/system/test_registrar.py b/filebeat/tests/system/test_registrar.py index db2ad49f2d8..6604361457b 100644 --- a/filebeat/tests/system/test_registrar.py +++ b/filebeat/tests/system/test_registrar.py @@ -206,7 +206,7 @@ def test_registry_file_custom_permissions(self): self.render_config_template( path=os.path.abspath(self.working_dir) + "/log/*", registry_home=registry_home, - registry_file_permissions=0644, + registry_file_permissions=0640, ) os.mkdir(self.working_dir + "/log/") testfile_path = self.working_dir + "/log/test.log" @@ -223,7 +223,7 @@ def test_registry_file_custom_permissions(self): max_timeout=1) filebeat.check_kill_and_wait() - self.assertEqual(self.file_permissions(registry_file), "0644") + self.assertEqual(self.file_permissions(registry_file), "0640") def test_registry_file_update_permissions(self): """ @@ -262,7 +262,7 @@ def test_registry_file_update_permissions(self): self.render_config_template( path=os.path.abspath(self.working_dir) + "/log/*", registry_home="a/b/c/registry_x", - registry_file_permissions=0644 + registry_file_permissions=0640 ) filebeat = self.start_beat() @@ -280,7 +280,7 @@ def test_registry_file_update_permissions(self): filebeat.check_kill_and_wait() - self.assertEqual(self.file_permissions(registry_file), "0644") + self.assertEqual(self.file_permissions(registry_file), "0640") def test_rotating_file(self): """ diff --git a/libbeat/cmd/instance/beat.go b/libbeat/cmd/instance/beat.go index 6a2107d8eef..c9f84a9fb19 100644 --- a/libbeat/cmd/instance/beat.go +++ b/libbeat/cmd/instance/beat.go @@ -147,6 +147,11 @@ func initRand() { // instance. // XXX Move this as a *Beat method? func Run(settings Settings, bt beat.Creator) error { + err := setUmaskWithSettings(settings) + if err != nil && err != errNotImplemented { + return errw.Wrap(err, "could not set umask") + } + name := settings.Name idxPrefix := settings.IndexPrefix version := settings.Version @@ -1037,3 +1042,10 @@ func initPaths(cfg *common.Config) error { } return nil } + +func setUmaskWithSettings(settings Settings) error { + if settings.Umask != nil { + return setUmask(*settings.Umask) + } + return setUmask(0027) // 0640 for files | 0750 for dirs +} diff --git a/libbeat/cmd/instance/settings.go b/libbeat/cmd/instance/settings.go index 7ada3870daa..138809557b0 100644 --- a/libbeat/cmd/instance/settings.go +++ b/libbeat/cmd/instance/settings.go @@ -43,4 +43,6 @@ type Settings struct { ILM ilm.SupportFactory Processing processing.SupportFactory + + Umask *int } diff --git a/libbeat/cmd/instance/umask_other.go b/libbeat/cmd/instance/umask_other.go new file mode 100644 index 00000000000..e3af909f785 --- /dev/null +++ b/libbeat/cmd/instance/umask_other.go @@ -0,0 +1,32 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// +build !windows + +package instance + +import ( + "errors" + "syscall" +) + +var errNotImplemented = errors.New("not implemented on platform") + +func setUmask(newmask int) error { + syscall.Umask(newmask) + return nil // the umask syscall always succeeds: http://man7.org/linux/man-pages/man2/umask.2.html#RETURN_VALUE +} diff --git a/libbeat/cmd/instance/umask_windows.go b/libbeat/cmd/instance/umask_windows.go new file mode 100644 index 00000000000..e52886301fb --- /dev/null +++ b/libbeat/cmd/instance/umask_windows.go @@ -0,0 +1,27 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package instance + +import "errors" + +var errNotImplemented = errors.New("not implemented on windows") + +func setUmask(newmask int) error { + // No way to set umask on Windows + return errNotImplemented +} diff --git a/libbeat/tests/system/config/mockbeat.yml.j2 b/libbeat/tests/system/config/mockbeat.yml.j2 index df185c8ca58..6023545f660 100644 --- a/libbeat/tests/system/config/mockbeat.yml.j2 +++ b/libbeat/tests/system/config/mockbeat.yml.j2 @@ -62,6 +62,9 @@ output: path: {{ output_file_path|default(beat.working_dir + "/output") }} filename: "{{ output_file_filename|default("mockbeat") }}" rotate_every_kb: 1000 + {% if output_file_permissions %} + permissions: {{ output_file_permissions }} + {% endif %} #number_of_files: 7 {%- endif %} diff --git a/libbeat/tests/system/test_umask.py b/libbeat/tests/system/test_umask.py new file mode 100644 index 00000000000..dd3a6df96a0 --- /dev/null +++ b/libbeat/tests/system/test_umask.py @@ -0,0 +1,36 @@ +from base import BaseTest + +import os +import stat +import unittest +import sys + +INTEGRATION_TESTS = os.environ.get('INTEGRATION_TESTS', False) + + +class TestUmask(BaseTest): + """ + Test default umask + """ + + DEFAULT_UMASK = 0027 + + def setUp(self): + super(BaseTest, self).setUp() + + self.output_file_permissions = 0666 + + self.render_config_template(output_file_permissions=self.output_file_permissions) + proc = self.start_beat() + self.wait_until(lambda: self.output_lines() > 0, max_timeout=2) + proc.check_kill_and_wait() + + @unittest.skipIf(sys.platform.startswith("win"), "umask is not available on Windows") + def test_output_file_perms(self): + """ + Test that output file permissions respect default umask + """ + output_file_path = os.path.join(self.working_dir, "output", "mockbeat") + perms = stat.S_IMODE(os.lstat(output_file_path).st_mode) + + self.assertEqual(perms, self.output_file_permissions & ~TestUmask.DEFAULT_UMASK)