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

add a server-side function to support sasl binding step by step #430

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

YangKeao
Copy link

close #429

Provide a function ServerBindStep to send sasl bind request step by step. It returns the result code, server cred and the errors. Now it only includes the server-side.

I have tested it in my situation, and it works pretty fine (but unfortunately I cannot show related codes now). If examples / tests are needed, I could add another commit to this PR several weeks later 🍻 .

Also, thanks for authors/reviewers of this repo. It's awesome 👍 .

Copy link
Member

@cpuschma cpuschma left a comment

Choose a reason for hiding this comment

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

Thank you very much for your pull request ❤️

// ServerBindStep sends the SASLBindRequest and return the result code, servercred and errors.
// If the result code is not SUCCESS(0x00) or IN_PROGRESS(0x0e), it's also considered as an error.
func (l *Conn) ServerBindStep(clientCred []byte, dn string, methodName string, controls []Control) (resultCode uint16, serverCred []byte, err error) {
packet := ber.Encode(ber.ClassUniversal, ber.TypeConstructed, ber.TagSequence, nil, "LDAP Request")
Copy link
Member

Choose a reason for hiding this comment

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

Most of this (until line 770) is a duplication of what doRequest does. The function signature also mismatches from the style how other functions like SimpleBind are structure. They take in a struct (which also satisfies the request interface). Additionally, there are simple wrappers for those functions like conn.Bind for simple use.

In my opinion we should stick to one style here and not mix them up. Could you please adapt your two functions please?

// TODO: add global constants. As this package used `uint16` to represent the result code nearly everywhere,
// these constants were
const LDAPResultCodeSuccess = 0
const LDAPResultCodeSASLBindInProgress = 0x0e
Copy link
Member

Choose a reason for hiding this comment

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

Both values are already defined in error.go, thought they're of type int unfortunately. I think it can stay like this and we take care of this later on 😄

@@ -733,3 +733,115 @@ RESP:

return nil, GetLDAPError(packet)
}

// ServerBindStep sends the SASLBindRequest and return the result code, servercred and errors.
Copy link
Member

Choose a reason for hiding this comment

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

Can you please add a short example on how to use this function to our tests? Thank you!

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.

Bind with raw credencials, but not calculate from clear text automatically
2 participants