From 0de34abe1bee240137362dd21c24a857d0eb8cf8 Mon Sep 17 00:00:00 2001 From: Jaipradeesh Date: Thu, 15 Mar 2018 19:08:37 +0530 Subject: [PATCH] Filebeat: Makes registry_file_permission configurable (#6455) This supports use-cases where the registry file is read by some other external service/component. --- CHANGELOG.asciidoc | 1 + filebeat/_meta/common.reference.p2.yml | 5 + filebeat/beater/filebeat.go | 2 +- filebeat/config/config.go | 28 ++-- .../docs/filebeat-general-options.asciidoc | 18 ++- filebeat/filebeat.reference.yml | 5 + filebeat/registrar/registrar.go | 18 +-- filebeat/tests/load/filebeat.yml | 2 +- filebeat/tests/system/config/filebeat.yml.j2 | 1 + filebeat/tests/system/test_registrar.py | 130 ++++++++++++++++++ 10 files changed, 186 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index bc8931fc3871..69e2e271d711 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -213,6 +213,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di - Add stream filtering when using `docker` prospector. {pull}6057[6057] - Add support for CRI logs format. {issue}5630[5630] - Add json.ignore_decoding_error config to not log json decoding erors. {issue}6547[6547] +- Make registry file permission configurable. {pull}6455[6455] *Heartbeat* diff --git a/filebeat/_meta/common.reference.p2.yml b/filebeat/_meta/common.reference.p2.yml index a1e610612690..2d188e68bdaa 100644 --- a/filebeat/_meta/common.reference.p2.yml +++ b/filebeat/_meta/common.reference.p2.yml @@ -261,6 +261,11 @@ filebeat.inputs: # data path. #filebeat.registry_file: ${path.data}/registry +# The permissions mask to apply on registry file. The default value is 0600. +# Must be a valid Unix-style file permissions mask expressed in octal notation. +# This option is not supported on Windows. +#filebeat.registry_file_permissions: 0600 + # These config files must have the full filebeat config part inside, but only # the input part is processed. All global options like spool_size are ignored. # The config_dir MUST point to a different directory then where the main filebeat config file is in. diff --git a/filebeat/beater/filebeat.go b/filebeat/beater/filebeat.go index b1a53283da51..88d45b8bfddf 100644 --- a/filebeat/beater/filebeat.go +++ b/filebeat/beater/filebeat.go @@ -218,7 +218,7 @@ func (fb *Filebeat) Run(b *beat.Beat) error { finishedLogger := newFinishedLogger(wgEvents) // Setup registrar to persist state - registrar, err := registrar.New(config.RegistryFile, config.RegistryFlush, finishedLogger) + registrar, err := registrar.New(config.RegistryFile, config.RegistryFilePermissions, config.RegistryFlush, finishedLogger) if err != nil { logp.Err("Could not init registrar: %v", err) return err diff --git a/filebeat/config/config.go b/filebeat/config/config.go index d7235941df70..df01d9a5841a 100644 --- a/filebeat/config/config.go +++ b/filebeat/config/config.go @@ -21,23 +21,25 @@ const ( ) type Config struct { - Inputs []*common.Config `config:"inputs"` - Prospectors []*common.Config `config:"prospectors"` - RegistryFile string `config:"registry_file"` - RegistryFlush time.Duration `config:"registry_flush"` - ConfigDir string `config:"config_dir"` - ShutdownTimeout time.Duration `config:"shutdown_timeout"` - Modules []*common.Config `config:"modules"` - ConfigInput *common.Config `config:"config.inputs"` - ConfigProspector *common.Config `config:"config.prospectors"` - ConfigModules *common.Config `config:"config.modules"` - Autodiscover *autodiscover.Config `config:"autodiscover"` + Inputs []*common.Config `config:"inputs"` + Prospectors []*common.Config `config:"prospectors"` + RegistryFile string `config:"registry_file"` + RegistryFilePermissions os.FileMode `config:"registry_file_permissions"` + RegistryFlush time.Duration `config:"registry_flush"` + ConfigDir string `config:"config_dir"` + ShutdownTimeout time.Duration `config:"shutdown_timeout"` + Modules []*common.Config `config:"modules"` + ConfigInput *common.Config `config:"config.inputs"` + ConfigProspector *common.Config `config:"config.prospectors"` + ConfigModules *common.Config `config:"config.modules"` + Autodiscover *autodiscover.Config `config:"autodiscover"` } var ( DefaultConfig = Config{ - RegistryFile: "registry", - ShutdownTimeout: 0, + RegistryFile: "registry", + RegistryFilePermissions: 0600, + ShutdownTimeout: 0, } ) diff --git a/filebeat/docs/filebeat-general-options.asciidoc b/filebeat/docs/filebeat-general-options.asciidoc index 80076d8124cc..eff4954377ee 100644 --- a/filebeat/docs/filebeat-general-options.asciidoc +++ b/filebeat/docs/filebeat-general-options.asciidoc @@ -32,6 +32,23 @@ It is not possible to use a symlink as registry file. NOTE: The registry file is only updated when new events are flushed and not on a predefined period. That means in case there are some states where the TTL expired, these are only removed when new event are processed. +[float] +==== `registry_file_permissions` + +The permissions mask to apply on registry 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. + +This option is not supported on Windows. + +Examples: + + 0644: give read and write access to the file owner, and read access to all others. + 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] +------------------------------------------------------------------------------------- +filebeat.registry_file_permissions: 0600 +------------------------------------------------------------------------------------- [float] ==== `config_dir` @@ -81,4 +98,3 @@ filebeat.shutdown_timeout: 5s ------------------------------------------------------------------------------------- include::../../libbeat/docs/generalconfig.asciidoc[] - diff --git a/filebeat/filebeat.reference.yml b/filebeat/filebeat.reference.yml index a7d7d5dc6e21..cd1cea2d99b6 100644 --- a/filebeat/filebeat.reference.yml +++ b/filebeat/filebeat.reference.yml @@ -556,6 +556,11 @@ filebeat.inputs: # data path. #filebeat.registry_file: ${path.data}/registry +# The permissions mask to apply on registry file. The default value is 0600. +# Must be a valid Unix-style file permissions mask expressed in octal notation. +# This option is not supported on Windows. +#filebeat.registry_file_permissions: 0600 + # These config files must have the full filebeat config part inside, but only # the input part is processed. All global options like spool_size are ignored. # The config_dir MUST point to a different directory then where the main filebeat config file is in. diff --git a/filebeat/registrar/registrar.go b/filebeat/registrar/registrar.go index 2bb16253c24a..02a2479b8bf2 100644 --- a/filebeat/registrar/registrar.go +++ b/filebeat/registrar/registrar.go @@ -16,11 +16,12 @@ import ( ) type Registrar struct { - Channel chan []file.State - out successLogger - done chan struct{} - registryFile string // Path to the Registry File - wg sync.WaitGroup + Channel chan []file.State + out successLogger + done chan struct{} + registryFile string // Path to the Registry File + registryFilePermissions os.FileMode // Permissions to apply on the Registry File + wg sync.WaitGroup states *file.States // Map with all file paths inside and the corresponding state gcRequired bool // gcRequired is set if registry state needs to be gc'ed before the next write @@ -40,9 +41,10 @@ var ( registryWrites = monitoring.NewInt(nil, "registrar.writes") ) -func New(registryFile string, flushTimeout time.Duration, out successLogger) (*Registrar, error) { +func New(registryFile string, registryFilePermissions os.FileMode, flushTimeout time.Duration, out successLogger) (*Registrar, error) { r := &Registrar{ - registryFile: registryFile, + registryFile: registryFile, + registryFilePermissions: registryFilePermissions, done: make(chan struct{}), states: file.NewStates(), Channel: make(chan []file.State, 1), @@ -259,7 +261,7 @@ func (r *Registrar) writeRegistry() error { logp.Debug("registrar", "Write registry file: %s", r.registryFile) tempfile := r.registryFile + ".new" - f, err := os.OpenFile(tempfile, os.O_RDWR|os.O_CREATE|os.O_TRUNC|os.O_SYNC, 0600) + f, err := os.OpenFile(tempfile, os.O_RDWR|os.O_CREATE|os.O_TRUNC|os.O_SYNC, r.registryFilePermissions) if err != nil { logp.Err("Failed to create tempfile (%s) for writing: %s", tempfile, err) return err diff --git a/filebeat/tests/load/filebeat.yml b/filebeat/tests/load/filebeat.yml index ae0e861577cb..93217983de38 100644 --- a/filebeat/tests/load/filebeat.yml +++ b/filebeat/tests/load/filebeat.yml @@ -15,7 +15,7 @@ filebeat: spool_size: 4096 idle_timeout: 5s registry_file: registry - + registry_file_permissions: 0600 ############################# Output ########################################## diff --git a/filebeat/tests/system/config/filebeat.yml.j2 b/filebeat/tests/system/config/filebeat.yml.j2 index c29bfdbcc21b..52e099d85113 100644 --- a/filebeat/tests/system/config/filebeat.yml.j2 +++ b/filebeat/tests/system/config/filebeat.yml.j2 @@ -93,6 +93,7 @@ filebeat.{{input_config | default("inputs")}}: filebeat.shutdown_timeout: {{ shutdown_timeout|default(0) }} {% if not skip_registry_config %} filebeat.registry_file: {{ beat.working_dir + '/' }}{{ registryFile|default("registry")}} +filebeat.registry_file_permissions: {{ registryFilePermissions|default("0600") }} {%endif%} {% if reload or reload_path -%} diff --git a/filebeat/tests/system/test_registrar.py b/filebeat/tests/system/test_registrar.py index e07cd2d6b5be..5b4799c06ba2 100644 --- a/filebeat/tests/system/test_registrar.py +++ b/filebeat/tests/system/test_registrar.py @@ -5,6 +5,7 @@ import platform import time import shutil +import stat from filebeat import BaseTest from nose.plugins.skip import SkipTest @@ -161,6 +162,135 @@ def test_custom_registry_file_location(self): assert os.path.isfile(os.path.join(self.working_dir, "a/b/c/registry")) + def test_registry_file_default_permissions(self): + """ + Test that filebeat default registry permission is set + """ + + if os.name == "nt": + # This test is currently skipped on windows because file permission + # configuration isn't implemented on Windows yet + raise SkipTest + + self.render_config_template( + path=os.path.abspath(self.working_dir) + "/log/*", + registryFile="a/b/c/registry", + ) + os.mkdir(self.working_dir + "/log/") + testfile_path = self.working_dir + "/log/test.log" + with open(testfile_path, 'w') as testfile: + testfile.write("hello world\n") + filebeat = self.start_beat() + self.wait_until( + lambda: self.output_has(lines=1), + max_timeout=15) + # wait until the registry file exist. Needed to avoid a race between + # the logging and actual writing the file. Seems to happen on Windows. + self.wait_until( + lambda: os.path.isfile(os.path.join(self.working_dir, + "a/b/c/registry")), + max_timeout=1) + filebeat.check_kill_and_wait() + + registry_file_perm_mask = oct(stat.S_IMODE(os.lstat(os.path.join(self.working_dir, + "a/b/c/registry")).st_mode)) + self.assertEqual(registry_file_perm_mask, "0600") + + def test_registry_file_custom_permissions(self): + """ + Test that filebeat registry permission is set as per configuration + """ + + if os.name == "nt": + # This test is currently skipped on windows because file permission + # configuration isn't implemented on Windows yet + raise SkipTest + + self.render_config_template( + path=os.path.abspath(self.working_dir) + "/log/*", + registryFile="a/b/c/registry", + registryFilePermissions=0644 + ) + os.mkdir(self.working_dir + "/log/") + testfile_path = self.working_dir + "/log/test.log" + with open(testfile_path, 'w') as testfile: + testfile.write("hello world\n") + filebeat = self.start_beat() + self.wait_until( + lambda: self.output_has(lines=1), + max_timeout=15) + # wait until the registry file exist. Needed to avoid a race between + # the logging and actual writing the file. Seems to happen on Windows. + self.wait_until( + lambda: os.path.isfile(os.path.join(self.working_dir, + "a/b/c/registry")), + max_timeout=1) + filebeat.check_kill_and_wait() + + registry_file_perm_mask = oct(stat.S_IMODE(os.lstat(os.path.join(self.working_dir, + "a/b/c/registry")).st_mode)) + self.assertEqual(registry_file_perm_mask, "0644") + + def test_registry_file_update_permissions(self): + """ + Test that filebeat registry permission is updated along with configuration + """ + + if os.name == "nt": + # This test is currently skipped on windows because file permission + # configuration isn't implemented on Windows yet + raise SkipTest + + self.render_config_template( + path=os.path.abspath(self.working_dir) + "/log/*", + registryFile="a/b/c/registry_x", + ) + os.mkdir(self.working_dir + "/log/") + testfile_path = self.working_dir + "/log/test.log" + with open(testfile_path, 'w') as testfile: + testfile.write("hello world\n") + filebeat = self.start_beat() + self.wait_until( + lambda: self.output_has(lines=1), + max_timeout=15) + # wait until the registry file exist. Needed to avoid a race between + # the logging and actual writing the file. Seems to happen on Windows. + self.wait_until( + lambda: os.path.isfile(os.path.join(self.working_dir, + "a/b/c/registry_x")), + max_timeout=1) + filebeat.check_kill_and_wait() + + registry_file_perm_mask = oct(stat.S_IMODE(os.lstat(os.path.join(self.working_dir, + "a/b/c/registry_x")).st_mode)) + self.assertEqual(registry_file_perm_mask, "0600") + + self.render_config_template( + path=os.path.abspath(self.working_dir) + "/log/*", + registryFile="a/b/c/registry_x", + registryFilePermissions=0644 + ) + + filebeat = self.start_beat() + self.wait_until( + lambda: self.output_has(lines=1), + max_timeout=15) + # wait until the registry file exist. Needed to avoid a race between + # the logging and actual writing the file. Seems to happen on Windows. + self.wait_until( + lambda: os.path.isfile(os.path.join(self.working_dir, + "a/b/c/registry_x")), + max_timeout=1) + + # Wait a momemt to make sure registry is completely written + time.sleep(1) + + filebeat.check_kill_and_wait() + + registry_file_perm_mask = oct(stat.S_IMODE(os.lstat(os.path.join(self.working_dir, + "a/b/c/registry_x")).st_mode)) + self.assertEqual(registry_file_perm_mask, "0644") + def test_rotating_file(self): """ Checks that the registry is properly updated after a file is rotated