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

feat: Add support for SCRAM-SHA-256 authentication #282

Open
wants to merge 6 commits into
base: development
Choose a base branch
from

Conversation

mhill-anynines
Copy link

Support more future proof hashing algorithm for SCRAM authentication.
Compatible with mongoDB 4.0 and higher.

@gogolok
Copy link

gogolok commented Oct 5, 2018

👍

@domodwyer
Copy link

Hi @mhill-anynines

Supper happy to have this in! Unfortunately one of the tests you added (thank you for tests!) is failing on MongoDB 4.0 (SHA-256 isn't supported before 4.0):

[LOG] 59.43813 Socket 0xc420420240 to localhost:40002: received document: bson.M{"ok":0, "errmsg":"Authentication failed.", "code":18, "codeName":"AuthenticationFailed"}
[LOG] 59.43816 Socket 0xc420420240 to localhost:40002: login error: server returned error on SASL authentication step: Authentication failed.
auth_test.go:911:
    c.Assert(err, IsNil)
... value *errors.errorString = &errors.errorString{s:"server returned error on SASL authentication step: Authentication failed."} ("server returned error on SASL authentication step: Authentication failed.")

I had a quick look and noticed the protocol for SHA-256 based SCRAM has changed from SHA-1:

  • Uses the SHA-256 hashing function.
  • Requires featureCompatibilityVersion set to 4.0.
  • Requires passwordDigestor to be server.

I'm guessing it is down to this? I've not looked in depth so let me know if it's not!

Thanks again!

Dom

@mhill-anynines
Copy link
Author

Yep the server digest is the kicker. Is there a reason for not using external libraries or is using https://github.com/xdg-go/scram acceptable?

My current inclination is to remove the internal SCRAM implementation and wrap the xdg-go implementation to fit the Stepper interface.

@mhill-anynines mhill-anynines force-pushed the auth-scram-sha-256 branch 3 times, most recently from 223c75e to 25b6594 Compare October 8, 2018 14:16
@mhill-anynines
Copy link
Author

Waiting on PR #285

@Fank
Copy link

Fank commented Oct 19, 2018

@mhill-anynines PR is merged

@mhill-anynines
Copy link
Author

mhill-anynines commented Oct 19, 2018

@mhill-anynines PR is merged

It was merged into master not develop. When the change is cherry picked back into develop then this PR can progress.

@maitesin
Copy link

@mhill-anynines the fix has been merged into development. Can you rebase from it, please?

Support more future proof hashing algorithm for SCRAM authentication.
Compatible with mongoDB 4.0 and higher.
Removes custom SCRAM implementation replacing it with a wrapper for the
existing xdg-go/scram library. Changes the saslNewScram interface to
take a new type *scram.Method argument replacing the  func () hash.Hash
type. Adds a scram.NewMethod function that validates and returns a
supported method.
Responsibility for checking if the authentication process is completed
it placed in the Step function, hence there should be no need to check
the response object done field independently.
@eminano
Copy link

eminano commented Jan 16, 2019

Hi @mhill-anynines,

Thanks for the time taken to implement this! Are you still blocked? The PR you refer to was merged into development via #287, is there anything else you need from us to proceed?

Thanks,
Esther

@mhill-anynines
Copy link
Author

Hi @mhill-anynines,

Thanks for the time taken to implement this! Are you still blocked? The PR you refer to was merged into development via #287, is there anything else you need from us to proceed?

Thanks,
Esther

Nothing from the project is blocking me. The lack of progress is due a reprioritisation. When I get some time I'll come back to this. If anybody wants to pick this up contact me and I'll pass on what's in my head if needed.

@ngalantowicz
Copy link

@mhill-anynines I'm interested in picking this up. I know it's been a while since you've maybe thought of this work, but any info on the vision still lingering would be helpful.

@eminano any info on getting development environment setup and running would be appreciated as well.

@Neustradamus
Copy link

Neustradamus commented Jan 8, 2022

@ all: I wish you a Happy New Year 2022!

Any news about it?

Linked to:

@pyrotechnics-io
Copy link

Hi guys .. Is this likely to be merged anytime soon?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants