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 support for creating LDAP clients from existing TcpStream/UnixStream. #132

Merged
merged 1 commit into from
Oct 23, 2024

Conversation

mateuszkj
Copy link

Allow creating Ldap client (sync and async) from exiting Tcp/Unix streams.

That's allows to create LDAP client from file descriptor and handle connection (with ssl) in sandboxed process.

Example scenario:

  1. Open tcp stream to LDAP server in normal mode.
  2. Go process into capsicum/seccomp mode.
  3. Handle ldap client (and SSL) in sandboxed process.

@mateuszkj mateuszkj changed the title Add support for creating LDAP clients from existing TcpStream and `… Add support for creating LDAP clients from existing TcpStream/UnixStream. Aug 7, 2024
…UnixStream`.

That's allows to create LDAP client from file descriptor and
handle connection (with ssl) in sandboxed process.
@inejge
Copy link
Owner

inejge commented Aug 10, 2024

FYI: I'll be mostly offline for the next ten days or so, and I won't be able to handle any issues.

@inejge
Copy link
Owner

inejge commented Sep 4, 2024

[Better late than never, eh?] Thanks for the PR, its concept is a worthwhile addition to the library and I'll be happy to have it included; however, I'm not keen to further complicate the connection API, which already has a lot of variations.

I would prefer to place the stream into LdapConnSettings (as an Option of an enum of Tcp|Unix stream, I imagine), provide the methods for initializing that field, and modify new_tcp() and new_unix() to use the field if present. Could you rework the PR along those lines?

inejge added a commit that referenced this pull request Oct 23, 2024
Per issue #132 discussion: the approach in 2697ae complicates the
API, the solution will be via LdapConnSettings, but I want to
record the contribution since the basic idea is sound and useful.
@inejge inejge merged commit 2697ae1 into inejge:master Oct 23, 2024
@inejge
Copy link
Owner

inejge commented Oct 23, 2024

Merged and reverted to rework with the LdapConnSettings method -- thanks for the contribution anyway.

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.

2 participants