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

Explicitly adding channel bindings on the client #240

Open
inejge opened this issue May 13, 2024 · 4 comments
Open

Explicitly adding channel bindings on the client #240

inejge opened this issue May 13, 2024 · 4 comments

Comments

@inejge
Copy link

inejge commented May 13, 2024

In the current API I see no way to add channel bindings to an Ntlm context when authenticating a client. (Motivation: AD can be configured to refuse authentication from an LDAP client doing an NTLM bind over a TLS connection if the channel bindings AvP is not supplied.)

Adding a method to Ntlm which sets the channel_bindings field to the supplied ChannelBindings struct does the right thing protocol-wise, i.e., the channel bindings AvP is correctly calculated and added to the AUTHENTICATE message. Would a PR with this kind of addition be appropriate, or is there another preferred way to add this functionality to the library?

@CBenoit
Copy link
Member

CBenoit commented May 14, 2024

Hi!

PRs are absolutely welcomed, and encouraged!!

About the suggested change, however, my guess is that it was not intended for the channel_bindings field to be overridden by the user, as-is. It is already set internally in some code paths for NTML challenge and NTLM authenticate:

self.channel_bindings = Some(ChannelBindings::from_bytes(&sec_buffer.buffer)?);

If we add a method to set the channel_bindings, it’s likely we should carefully consider the various new interactions.

Since I’m not the most familiar with NTLM, I’ll cc other people: @TheBestTvarynka @RRRadicalEdward

@inejge
Copy link
Author

inejge commented May 14, 2024

I believe that the quoted line is meant to be executed on the server when accepting the security context sent from the client. So, the channel bindings must have been already supplied by the client -- for which the current API lacks the method, hence my inquiry. But of course, let the other contributors chime in.

@awakecoding
Copy link
Contributor

For reference on channel bindings in the SSPI API just search "channel_bindings" in the FreeRDP source code: https://github.com/search?q=repo%3AFreeRDP%2FFreeRDP%20channel_bindings&type=code

I had forgotten that detail, but it looks like the channel binding is passed through an SSPI input buffer of type SECBUFFER_CHANNEL_BINDINGS.

However, I suppose you want a Rust API as well?

@inejge
Copy link
Author

inejge commented May 14, 2024

I would like to have the rough equivalent of the chan_bindings parameter to gss_init_sec_context() in GSSAPI (RFC 2743), which lets one specify the channel bindings when initializing the security context on the client side. In sspi::Ntlm, it could be part of the security context builder chain, or a separate method. (Those with a better understanding of the crate's structure would be best placed to decide.)

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

No branches or pull requests

3 participants