Add Kerberos support to Elasticsearch output #17927
Merged
Conversation
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
kvch
added
[zube]: In Review
Team:Services
|
Pinging @elastic/integrations-services (Team:Services) |
kvch
force-pushed
the
feature-libbeat-kerberos-elasticsearch
branch
from
73c4256
to
03daa15
Compare
ycombinator
reviewed
Apr 23, 2020
kvch
force-pushed
the
feature-libbeat-kerberos-elasticsearch
branch
from
1ceba6e
to
f213960
Compare
ycombinator
reviewed
Apr 24, 2020
kvch
force-pushed
the
feature-libbeat-kerberos-elasticsearch
branch
from
f213960
to
d88714d
Compare
WDYT about starting a discussion issue about this in https://github.com/elastic/go-elasticsearch? |
ycombinator
reviewed
Apr 24, 2020
There was a problem hiding this comment.
Gave an LGTM too early, sorry! I assume you want to add docs (https://www.elastic.co/guide/en/beats/filebeat/current/elasticsearch-output.html) and a CHANGELOG entry to this PR as well?
urso
reviewed
Apr 24, 2020
| @@ -59,19 +64,21 @@ func (t *AuthType) Unpack(value string) error { | |||
| } | |||
|
|
|||
| func (c *Config) Validate() error { | |||
| if c.AuthType == AUTH_PASSWORD { | |||
| switch c.AuthType { | |||
| case AUTH_PASSWORD: | |||
There was a problem hiding this comment.
nits: Do not use C like screaming identifiers for consts. Plus add the actual type to the const if we already have an enum type. Do we need to export the consts or not?
I understand these have not been introduced in this PR, but would be nice to cleanup existing code a little when we touch it :)
There was a problem hiding this comment.
TIL the all-caps identifiers are also known as screaming identifiers! 😄
|
How about adding an |
kvch
force-pushed
the
feature-libbeat-kerberos-elasticsearch
branch
from
d88714d
to
8b7e959
Compare
|
LGTM. Can you please add a changelog? |
kvch
force-pushed
the
feature-libbeat-kerberos-elasticsearch
branch
from
d765e52
to
89b44b8
Compare
bmorelli25
reviewed
Apr 28, 2020
| @@ -0,0 +1,85 @@ | |||
| [[configuration-kerberos]] | |||
There was a problem hiding this comment.
It looks like you haven't included this file anywhere. It's not being built, which is why the link here fails.
There was a problem hiding this comment.
Thank you for your help!
💔 Build Failed
Expand to view the summary
Build stats
Test stats 🧪
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
|
Failing tests are unrelated. |
zube
bot
added
[zube]: Done
and removed
[zube]: In Review
needs_backport
6 tasks
kvch
removed
the
needs_backport
kvch
added a commit
to kvch/beats
that referenced
this pull request
Apr 29, 2020
This PR adds support for Kerberos authentication to Elasticsearch output. Users can authenticate using either passwords or keytabs. The option `service_name` is not exposed as in case of ES it has be `HTTP`. Thus, the [SPN](https://web.mit.edu/kerberos/krb5-1.5/krb5-1.5.4/doc/krb5-user/What-is-a-Kerberos-Principal_003f.html) of the output is always `HTTP/{output.elasticsearch.host}@{output.elasticsearch.kerberos.realm}`. ```yaml ``` (cherry picked from commit f66b079)
kvch
added a commit
that referenced
this pull request
Apr 30, 2020
…ut (#18080) * Add Kerberos support to Elasticsearch output (#17927) This PR adds support for Kerberos authentication to Elasticsearch output. Users can authenticate using either passwords or keytabs. The option `service_name` is not exposed as in case of ES it has be `HTTP`. Thus, the [SPN](https://web.mit.edu/kerberos/krb5-1.5/krb5-1.5.4/doc/krb5-user/What-is-a-Kerberos-Principal_003f.html) of the output is always `HTTP/{output.elasticsearch.host}@{output.elasticsearch.kerberos.realm}`. ```yaml ``` (cherry picked from commit f66b079) * "fix" reference template
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What does this PR do?
This PR adds support for Kerberos authentication to Elasticsearch output.
Configuration
Users can authenticate using either passwords or keytabs.
The option
service_nameis not exposed as in case of ES it has beHTTP. Thus, the SPN of the output is alwaysHTTP/{output.elasticsearch.host}@{output.elasticsearch.kerberos.realm}.Architecture
Kerberos authentication is implemented by using the
spnego.Clientfromgopkg.in/jcmturner/gokrb5/v7.5. It is an HTTP client which is able to authenticate to a Kerberos KDC and get the required session key to authenticate to Elasticsearch.In order to use the client when communicating with ES, I introduced a new interface named
esHTTPClientwith the following functions:So the newly created
kerberos.Clientandhttp.Clientcan be used inConnection.The only drawback of this solution is that it is not compatible with
github.com/elastic/go-elasticsearch. The main issue is the constructor of that package only lets us configurehttp.Transport. However,http.Transportshould be used for authentication according to Golang documentation. For the record, it is possible to implement Kerberos authentication in a specialRoundTripper, but I am not a huge fan of that approach as the Golang docs advise against it.Reference:
Testing
I am planning to add tests in a follow-up PR. I wanted to raise this first, so we can agree on the selected architecture.
Why is it important?
Users now can authenticate to Kerberos-aware Elasticsearch instances.
Checklist
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.