Add "mechanism" in output.kafka to support SCRAM-SHA-512 and SCRAM-SHA-256 by zvictorino · Pull Request #12867 · elastic/beats

zvictorino · 2019-07-11T13:16:46Z

New config key mechanism was introduced.
How to use it:

output.kafka:
  codec.format:
    string: '%{[@timestamp]} %{[message]}'

  hosts: ["localhost:9092"]
  topic: 'mytopic'
  partition.round_robin:
    reachable_only: false
  required_acks: 1
  compression: gzip
  max_message_bytes: 1000000
  username: user
  password: pass
  sasl.mechanism: SCRAM-SHA-512

@timestamp

… and SCRAM-SHA-256 mechanism. How to use it: ``` output.kafka: codec.format: string: '%{[@timestamp]} %{[message]}' hosts: ["localhost:9092"] topic: 'mytopic' partition.round_robin: reachable_only: false required_acks: 1 compression: gzip max_message_bytes: 1000000 username: user password: pass mechanism: SCRAM-SHA-512 ```

elasticmachine · 2019-07-11T13:16:48Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

elasticmachine · 2019-07-11T13:16:48Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

houndci-bot · 2019-07-11T13:16:48Z

libbeat/outputs/kafka/config.go

+	return
+}
+
+func (x *XDGSCRAMClient) Done() bool {


exported method XDGSCRAMClient.Done should have comment or be unexported

houndci-bot · 2019-07-11T13:16:49Z

libbeat/outputs/kafka/config.go

+	return nil
+}
+
+func (x *XDGSCRAMClient) Step(challenge string) (response string, err error) {


exported method XDGSCRAMClient.Step should have comment or be unexported

houndci-bot · 2019-07-11T13:16:49Z

libbeat/outputs/kafka/config.go

+	scram.HashGeneratorFcn
+}
+
+func (x *XDGSCRAMClient) Begin(userName, password, authzID string) (err error) {


exported method XDGSCRAMClient.Begin should have comment or be unexported

houndci-bot · 2019-07-11T13:16:49Z

libbeat/outputs/kafka/config.go

+var SHA256 scram.HashGeneratorFcn = func() hash.Hash { return sha256.New() }
+var SHA512 scram.HashGeneratorFcn = func() hash.Hash { return sha512.New() }
+
+type XDGSCRAMClient struct {


exported type XDGSCRAMClient should have comment or be unexported

houndci-bot · 2019-07-11T13:16:49Z

libbeat/outputs/kafka/config.go

 )

+var SHA256 scram.HashGeneratorFcn = func() hash.Hash { return sha256.New() }
+var SHA512 scram.HashGeneratorFcn = func() hash.Hash { return sha512.New() }


exported var SHA512 should have comment or be unexported

houndci-bot · 2019-07-11T13:16:50Z

libbeat/outputs/kafka/config.go

+	"github.com/xdg/scram"
 )

+var SHA256 scram.HashGeneratorFcn = func() hash.Hash { return sha256.New() }


exported var SHA256 should have comment or be unexported

chlunde · 2019-08-14T07:28:45Z

@jsoriano this would fix #8387

jsoriano · 2019-08-14T09:31:29Z

@zvictorino thanks for opening this PR! Tests are failing because there is a missing package, you will need to add it with govendor fetch.
It'd be also nice to add some docs for the new option, but we can add it in a follow up.

urso · 2019-08-14T11:29:27Z

let's name the setting sasl_mechanism or create an sasl namespace so the setting becomes sasl.mechanism (assuming we might add more settings in the future).

The setting will also need some docs.

Almost all symbols are private to the package, don't export them.

urso · 2019-08-14T11:47:03Z

libbeat/outputs/kafka/config.go

 	Username           string                    `config:"username"`
 	Password           string                    `config:"password"`
 	Codec              codec.Config              `config:"codec"`
+	Mechanism          string                    `config:"mechanism"`


this calls for an enum like type (over string):

type saslMechanism string const ( saslTypePlaintext = sarama.SASLTypePlaintext ... ) func (m *saslMechanism) Unpack(in string) error { in = strings.ToUpper(in) // try not to force users to use all upper case switch in { case saslTypePlaintext, ...: *m = saslMechanism(in) default: return fmt.Errorf("not valid mechanism '%v', only supported with PLAIN|SCRAM-SHA-512|SCRAM-SHA-256", in) } return nil }

The unpack method will be called by (*Config).Unpack. If it fails the full config name and the config file the setting comes from will be reported.

There is another 'string' comparison in order to fill in the sarama SASL settings. Having magic strings in different places is a good way to introduce Bugs (future developer might have typo when adding another mechanism). Better use an enum and use contanstants (compiler will complain if there is a typo). By having our own type we can also do this (optional):

func (m saslMechanism) configureSarama(... <other input>?, config *sarama.Config) error { switch m { case saslScramSHA256: ... case sasl<type>: ... default: panic(<we shouldn't get here, assuming no developer messed up>) } }

ghost · 2019-09-02T07:20:38Z

Hi @zvictorino, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in your Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?

Move some codes to separate file.

libbeat/outputs/kafka/scram.go

houndci-bot · 2019-09-06T09:26:50Z

libbeat/outputs/kafka/scram.go

+	return nil
+}
+
+func (x *XDGSCRAMClient) Step(challenge string) (response string, err error) {


exported method XDGSCRAMClient.Step should have comment or be unexported

houndci-bot · 2019-09-06T09:26:51Z

libbeat/outputs/kafka/scram.go

+	scram.HashGeneratorFcn
+}
+
+func (x *XDGSCRAMClient) Begin(userName, password, authzID string) (err error) {


exported method XDGSCRAMClient.Begin should have comment or be unexported

houndci-bot · 2019-09-06T09:26:51Z

libbeat/outputs/kafka/scram.go

+var SHA256 scram.HashGeneratorFcn = func() hash.Hash { return sha256.New() }
+var SHA512 scram.HashGeneratorFcn = func() hash.Hash { return sha512.New() }
+
+type XDGSCRAMClient struct {


exported type XDGSCRAMClient should have comment or be unexported

houndci-bot · 2019-09-06T09:26:51Z

libbeat/outputs/kafka/scram.go

+)
+
+var SHA256 scram.HashGeneratorFcn = func() hash.Hash { return sha256.New() }
+var SHA512 scram.HashGeneratorFcn = func() hash.Hash { return sha512.New() }


exported var SHA512 should have comment or be unexported

houndci-bot · 2019-09-06T09:26:51Z

libbeat/outputs/kafka/scram.go

+	"github.com/xdg/scram"
+)
+
+var SHA256 scram.HashGeneratorFcn = func() hash.Hash { return sha256.New() }


exported var SHA256 should have comment or be unexported

houndci-bot · 2019-09-06T09:26:52Z

libbeat/outputs/kafka/scram.go

@@ -0,0 +1,37 @@
+// https://github.com/Shopify/sarama/blob/master/examples/sasl_scram_client/scram_client.go


package comment should be of the form "Package kafka ..."

ph · 2019-10-18T14:16:00Z

Hello @zvictorino this is a really nice addition, its there anything we could help to move it forward with you?

jsoriano · 2019-11-25T10:42:19Z

jenkins, test this

armysheng · 2020-01-20T06:08:30Z

can anyone merge this pull request plz ?

jsoriano · 2020-01-20T09:10:15Z

@zvictorino there is a merge conflict, could you please update the branch with master? Thanks!

cla-checker-service · 2020-02-26T08:12:12Z

💚 CLA has been signed

Update code to master

jsoriano · 2020-02-26T11:09:08Z

ok to test

jsoriano · 2020-02-27T15:50:52Z

jenkins, test this again please

jsoriano · 2020-02-27T19:15:59Z

Code LGTM, but there would be some pending things. We would need a changelog entry, docs for the new options, and some tests.

@zvictorino let me know if you can continue with this, if not I am ok with merging this as is and create an issue with some follow up tasks.

Thanks!

jsoriano

Merging this as is by now, but not backporting to 7.x, I will create an issue for follow ups. Thanks @zvictorino!

jsoriano · 2020-03-02T09:50:27Z

Follow up issue for this: #16723

…A-256 (elastic#12867) (cherry picked from commit e935b26)

(cherry picked from commit 87ff5c0)

… SCRAM-SHA-512 and SCRAM-SHA-256 (#23110) Add "mechanism" in output.kafka to support SCRAM-SHA-512 and SCRAM-SHA-256. (cherry picked from commit e935b26) (cherry picked from commit 87ff5c0) Co-authored-by: zvictorino <zvictorino@gmail.com>

zvictorino requested a review from a team as a code owner July 11, 2019 13:16

houndci-bot reviewed Jul 11, 2019

View reviewed changes

jsoriano added libbeat review labels Aug 14, 2019

jsoriano added the :Outputs label Aug 14, 2019

urso reviewed Aug 14, 2019

View reviewed changes

Allow unconfigured SASL.

abac3f1

Move some codes to separate file.

houndci-bot reviewed Sep 6, 2019

View reviewed changes

zvictorino added 3 commits November 22, 2019 20:54

Merge branch 'master' of https://github.com/elastic/beats

6865884

Add docs to sasl.mechanism.

f000b1c

Add license headers

2f80299

Fix SCRAM-SHA-512/SCRAM-SHA-256

08b7eb6

jsoriano self-assigned this Jan 20, 2020

jsoriano added the Team:Services (Deprecated) Label for the former Integrations-Services team label Feb 6, 2020

andresrc added [zube]: Inbox [zube]: In Review and removed [zube]: Inbox labels Feb 7, 2020

zvictorino force-pushed the master branch from f6a17e1 to 2f80299 Compare February 26, 2020 08:33

zvictorino added 2 commits February 26, 2020 16:43

Remove output-kafka.asciidoc

ec6d58e

Merge pull request #2 from elastic/master

be78f00

Update code to master

jsoriano approved these changes Mar 2, 2020

View reviewed changes

jsoriano merged commit e935b26 into elastic:master Mar 2, 2020

zube bot added [zube]: Done and removed [zube]: In Review labels Mar 2, 2020

jsoriano added the v8.0.0 label Mar 2, 2020

jsoriano mentioned this pull request Mar 2, 2020

Complete support for SCRAM-SHA-256/512 in Kafka output #16723

Closed

3 tasks

jsoriano mentioned this pull request Mar 2, 2020

[libbeat] Kafka Output: Add support for SASL_SSL authentication #8387

Closed

andresrc removed the [zube]: Done label Mar 4, 2020

l0x-c0d3z mentioned this pull request Jul 3, 2020

Add SCRAM authentication to kafka metrics source #19648

Closed

jsoriano mentioned this pull request Dec 14, 2020

Add docs and change log for #12867 #23103

Merged

jsoriano added a commit that referenced this pull request Dec 14, 2020

Add docs and changelog for #12867 (#23103)

87ff5c0

jsoriano mentioned this pull request Dec 14, 2020

Cherry-pick #12867 to 7.x: Add "mechanism" in output.kafka to support SCRAM-SHA-512 and SCRAM-SHA-256 #23110

Merged

jsoriano pushed a commit to jsoriano/beats that referenced this pull request Dec 14, 2020

Add "mechanism" in output.kafka to support SCRAM-SHA-512 and SCRAM-SH…

76fb8ab

…A-256 (elastic#12867) (cherry picked from commit e935b26)

jsoriano added a commit to jsoriano/beats that referenced this pull request Dec 14, 2020

Add docs and changelog for elastic#12867 (elastic#23103)

cc27426

(cherry picked from commit 87ff5c0)

jsoriano added v7.11.0 test-plan Add this PR to be manual test plan and removed test-plan Add this PR to be manual test plan labels Dec 14, 2020

fholzer mentioned this pull request Mar 29, 2021

feat: add support for SCRAM authentication for kafka metricbeat module, fixes #19648 #24810

Merged

6 tasks

jsoriano mentioned this pull request Apr 1, 2021

Cherry-pick #24810 to 7.x: feat: add support for SCRAM authentication for kafka metricbeat module, fixes #19648 #24889

Merged

6 tasks

bmorelli25 mentioned this pull request Apr 28, 2021

docs: Sync Libbeat documentation elastic/apm-server#5185

Merged

3 tasks

		@@ -0,0 +1,37 @@
		// https://github.com/Shopify/sarama/blob/master/examples/sasl_scram_client/scram_client.go

Conversation

zvictorino commented Jul 11, 2019 • edited by jsoriano Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Jul 11, 2019

Uh oh!

elasticmachine commented Jul 11, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chlunde commented Aug 14, 2019

Uh oh!

jsoriano commented Aug 14, 2019

Uh oh!

urso commented Aug 14, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Sep 2, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ph commented Oct 18, 2019

Uh oh!

jsoriano commented Nov 25, 2019

Uh oh!

armysheng commented Jan 20, 2020

Uh oh!

jsoriano commented Jan 20, 2020

Uh oh!

cla-checker-service bot commented Feb 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jsoriano commented Feb 26, 2020

Uh oh!

jsoriano commented Feb 27, 2020

Uh oh!

jsoriano commented Feb 27, 2020

Uh oh!

jsoriano left a comment

Choose a reason for hiding this comment

Uh oh!

jsoriano commented Mar 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

zvictorino commented Jul 11, 2019 •

edited by jsoriano

Loading

cla-checker-service bot commented Feb 26, 2020 •

edited

Loading