-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Filebeat] Update docs for unix sockets #18009
Merged
andrewstucki
merged 8 commits into
elastic:master
from
andrewstucki:filebeat-unix-socket-doc-update
Apr 28, 2020
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
1ba1d98
update docs for unix sockets
865a721
Update filebeat/docs/inputs/input-unix.asciidoc
andrewstucki ad6b276
Add socket cleanup code, and socket ownership modification
8981dfb
rearrange imports
1c448a3
updated docs
9c06bb0
Switch to group chown and chmod only
5024ba6
Fix docs
7d275e2
Bypass refusal check for windows due to Windows unix socket buf for F…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
:type: unix | ||
|
||
[id="{beatname_lc}-input-{type}"] | ||
=== Unix input | ||
|
||
beta[] | ||
|
||
++++ | ||
<titleabbrev>Unix</titleabbrev> | ||
++++ | ||
|
||
Use the `unix` input to read events over a stream-oriented Unix domain socket. | ||
|
||
Example configuration: | ||
|
||
["source","yaml",subs="attributes"] | ||
---- | ||
{beatname_lc}.inputs: | ||
- type: unix | ||
max_message_size: 10MiB | ||
path: "/var/run/filebeat.sock" | ||
---- | ||
|
||
|
||
==== Configuration options | ||
|
||
The `unix` input supports the following configuration options plus the | ||
<<{beatname_lc}-input-{type}-common-options>> described later. | ||
|
||
include::../inputs/input-common-unix-options.asciidoc[] | ||
|
||
[id="{beatname_lc}-input-{type}-common-options"] | ||
include::../inputs/input-common-options.asciidoc[] | ||
|
||
:type!: |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,7 @@ func init() { | |
"access", | ||
"brk", | ||
"chmod", | ||
"chown", | ||
"clock_gettime", | ||
"clone", | ||
"close", | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,7 @@ func init() { | |
"bind", | ||
"brk", | ||
"chmod", | ||
"chown", | ||
"clock_gettime", | ||
"clone", | ||
"close", | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to document some of the behavior. Like must the file exist when the input starts? What are the file permission requirements?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, unix socket behavior is generally kind of tricky. If the file descriptor used for controlling the socket lifecycle is already on the filesystem, we'll actually get an error trying to bind. Additionally, go will actually unlink the fd automatically when the socket is closed. So WDYT about something like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So Filebeat controls the creation and destruction? Perhaps Filebeat should automatically unlink it to try to resolve there error without user intervention.
What will the file mode be of the created unix socket? What permissions will be needed by the writing app?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, ideally filebeat would fully control the lifecycle, but I hesitate to unlink any file that a user specifies in a configuration at startup time.
AFAIK unix sockets are always created with
755
by the underlying OS. So their permissions look likesrwxr-xr-x
with thes
indicator saying that this is actually a reference to a socket fd.If you feel strongly about the attempted
os.Remove
call at startup I can add that, but I think we'd want to put a bolded section in the docs saying, "make sure that no file exists with this path, otherwise it'll be removed"There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is some precedence for removing the socket if it exists in libbeat. But one thing I would add is a
stat
call to check if it is a socket before removing it.I think you're right with 755 for when filebeat is running as root as a service since the default umask is 0022 for root and for systemd. So this means that only root users can pass logs to Filebeat in most deployments. I would explain this so users understand the permission requirements.
Users will probably ask for a way to configure the mode and/or the group of the socket. So they don't have to run their app as root.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 added the cleanup code with a refusal error if the file isn't a socket. Also added some permission modification code to address the permissions issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking this through a little more while reviewing your changes. Maybe a better route is to only allow
chgrp
andchmod
changes. chgrp works for unprivileged users (with restrictions to groups the user is a member of) whereas chown is only available to root. This would leave the owner of the socket intact which I think is nice too.As a user I would probably create a
filebeat-writers
group on the system and configure Filebeat tochgrp
the file to filebeat-writers and configure it tochmod
it to 770 (or whatever the least privs are). Ideally it create the socket with these attributes so there's no chance of a race condition where the socket exists but it not accessible to the app that needs to use it.chgrp
(andchown
) is current prohibited in Beats by a seccomp policy. You'll have to whitelistchgrp
in order to allow it in the Filebeat process.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, modified the implementation to only use
chown
with agid
set andchmod
-- WRT creating the fd with the proper umask--from the looks of golang/go#11822 our only two options are doing asyscall.Umask
prior to the call toListen
which is going to affect all files created by the process or changing it after the fact. So, I figure changing the file mode after the fact is probably the lesser of the two evils.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say to use chgrp so we can be finer grained in our seccomp policy but it looks like
chgrp
isn't currently exposed in any readily available Go API. So LGTM.