Skip to content

KAFKA-4589: SASL/SCRAM documentation#2369

Closed
rajinisivaram wants to merge 3 commits intoapache:trunkfrom
rajinisivaram:KAFKA-4589
Closed

KAFKA-4589: SASL/SCRAM documentation#2369
rajinisivaram wants to merge 3 commits intoapache:trunkfrom
rajinisivaram:KAFKA-4589

Conversation

@rajinisivaram
Copy link
Copy Markdown
Contributor

No description provided.

@rajinisivaram
Copy link
Copy Markdown
Contributor Author

This is currently a commit on top of the docs for sasl.jaas.config. I will rebase after that is committed.

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 13, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/843/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 13, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/841/
Test PASSed (JDK 8 and Scala 2.12).

Copy link
Copy Markdown
Contributor

@gwenshap gwenshap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. One minor phrasing change.

Comment thread docs/security.html Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The phrasing is a bit awkward - "We support SASL (Kerberos), and then we added SASL/PLAIN and then we added SASL/SCRAM". Almost makes it seem like the additional protocols are an afterthought and may deter usage.

Perhaps just something like this will be clearer:

We support the following mechanisms:

  • SASL/KERBEROS - starting at version 0.9.0.0
  • SASL/PLAIN - starting at version 0.10.0.0
  • SASL/SCRAM-SHA-256 and SASL/SCRAM-SHA-512 - starting at version 0.10.2.0

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gwenshap Thank you for the review and the suggestion. I have updated the doc.

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 13, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/854/
Test FAILed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 13, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/852/
Test FAILed (JDK 7 and Scala 2.10).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 13, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/841/
Test FAILed (JDK 7 and Scala 2.10).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 13, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/852/
Test PASSed (JDK 8 and Scala 2.12).

@harshach
Copy link
Copy Markdown

+1

Comment thread docs/security.html Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question about this: why did we do 2 separate mechanisms instead of a single mechanism with a config for the hashing algorithm used? It's a bit late to be asking this, but this hasn't been released yet, so there's still a bit of time. :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ijuma SCRAM mechanism names are defined in RFC-5802 (https://tools.ietf.org/html/rfc5802#section-4). So we are using the standard name rather than defining our own.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense, thanks for the explanation.

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jan 14, 2017

cc @junrao since he reviewed the main PR.

@rajinisivaram
Copy link
Copy Markdown
Contributor Author

@ijuma I have rebased this on top of the JAAS doc. Also, in the second commit in this PR (to make it easy to undo if required), I have tried to remove duplication. Not sure if I have made it better or worse. Can you take a look, please? Thank you!

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 17, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/932/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 17, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/930/
Test PASSed (JDK 8 and Scala 2.12).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 17, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/930/
Test PASSed (JDK 7 and Scala 2.10).

Copy link
Copy Markdown
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rajinisivaram : Thanks for the patch. Left a couple of comments.

Comment thread docs/security.html Outdated
The login module describes how the clients like producer and consumer can connect to the Kafka Broker.
The following is an example configuration for a client for the SCRAM mechanisms:
<pre>
asl.jaas.config=org.apache.kafka.common.security.scram.ScramLoginModule required \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing an s at the beginning?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Comment thread docs/security.html
<li>Kafka supports only the strong hash functions SHA-256 and SHA-512 with a minimum iteration count
of 4096. Strong hash functions combined with strong passwords and high iteration counts protect
against brute force attacks if Zookeeper security is compromised.</li>
<li>SCRAM should be used only with TLS-encryption to prevent interception of SCRAM exchanges. This
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, does SCRAM need to be used with TLS? I thought the main advantage of SCRAM over PLAIN is that you don't need to encrypt the wire transfer since it's hard to compromise the password even if the wire transfer is intercepted.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@junrao Thank you for the review. SCRAM is safe against replay attacks and the information obtained using interception is not sufficient to impersonate a client. Offline brute force attacks can be used with the intercepted data to try to recover the user's password, but this is mitigated with the use of strong hash functions, strong passwords etc.

If Zookeeper (the SCRAM credential store) security is compromised, then the intercepted data from a PLAINTEXT connection combined with the stored credential in Zookeeper can be used to impersonate the client. To protect against this, it is recommended that SCRAM is used with TLS. I think we should encourage the use of TLS in the docs so that anyone using PLAINTEXT with SCRAM looks more into the security considerations and understands the risks.

Comment thread docs/security.html Outdated
a login module in <tt>KafkaClient</tt> for the selected mechanism as described in the examples
for setting up <a href="#security_sasl_kerberos_clientconfig">GSSAPI (Kerberos)</a>,
<a href="#security_sasl_plain_clientconfig">PLAIN</a> or
<a href="#security_sasl_scram_clientconfig">SCRAM</a></li>.
Copy link
Copy Markdown
Contributor

@junrao junrao Jan 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "/li>" extra?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 19, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/1017/
Test PASSed (JDK 8 and Scala 2.12).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 19, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/1017/
Test PASSed (JDK 7 and Scala 2.10).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 19, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/1019/
Test PASSed (JDK 8 and Scala 2.11).

@junrao
Copy link
Copy Markdown
Contributor

junrao commented Jan 19, 2017

Thanks for the patch. LGTM

@asfgit asfgit closed this in 666abaf Jan 19, 2017
asfgit pushed a commit that referenced this pull request Jan 19, 2017
Author: Rajini Sivaram <rajinisivaram@googlemail.com>

Reviewers: Ismael Juma <ismael@juma.me.uk>, Gwen Shapira <cshapi@gmail.com>, Sriharsha Chintalapani <harsha@hortonworks.com>, Jun Rao <junrao@gmail.com>

Closes #2369 from rajinisivaram/KAFKA-4589

(cherry picked from commit 666abaf)
Signed-off-by: Jun Rao <junrao@gmail.com>
soenkeliebau pushed a commit to soenkeliebau/kafka that referenced this pull request Feb 7, 2017
Author: Rajini Sivaram <rajinisivaram@googlemail.com>

Reviewers: Ismael Juma <ismael@juma.me.uk>, Gwen Shapira <cshapi@gmail.com>, Sriharsha Chintalapani <harsha@hortonworks.com>, Jun Rao <junrao@gmail.com>

Closes apache#2369 from rajinisivaram/KAFKA-4589
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants