Skip to content
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

Remove normalisation of the password when using SCRAM auth #505

Merged
merged 2 commits into from
Sep 23, 2019

Conversation

ramesius
Copy link

@ramesius ramesius commented Sep 23, 2019

KafkaJS was incorrectly normalised the SCRAM password, resulting in , and = characters being encoded causing authentication failures.

According to the cited RFC in the source code (https://tools.ietf.org/html/rfc5802#section-5.1), just the username needs to be normalised.

Closes #506

@ramesius ramesius changed the title Remove sanitising of the password when using SCRAM auth Remove normalisation of the password when using SCRAM auth Sep 23, 2019
@Nevon
Copy link
Collaborator

Nevon commented Sep 23, 2019

Thank you very much for your contribution! I just want to reproduce this issue myself, in order to see that the proposed fix works, and then I'll merge it. :)

@ramesius
Copy link
Author

Thanks. I think I want to add some tests for special characters in the password but want some direction on how I should include something like that 👍

Include a character in the password that would be incorrectly sanitized
if the santization algorithm was applied.
@Nevon
Copy link
Collaborator

Nevon commented Sep 23, 2019

What I did was to simply change the password in scripts/createScramCredentials.sh to include =, update the passwords to match in testHelpers/index.js, and then run the existing tests. Without your fix, the tests failed, and with the fix, they pass. 👍

@ramesius
Copy link
Author

Perfect, thanks for that

@Nevon Nevon merged commit eeb8b9f into tulios:master Sep 23, 2019
@Nevon
Copy link
Collaborator

Nevon commented Sep 23, 2019

This fix will be available in pre-release version 1.11.0-beta.12 as soon as the build is done.

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.

Incorrect normalisation of SASL SCRAM password
2 participants