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

Unappropriate use_ssl and use_tls configuration options #757

Open
pini-gh opened this issue Feb 23, 2025 · 3 comments
Open

Unappropriate use_ssl and use_tls configuration options #757

pini-gh opened this issue Feb 23, 2025 · 3 comments

Comments

@pini-gh
Copy link

pini-gh commented Feb 23, 2025

I think there is a misunderstanding regarding use_ssl and use_tls.

As it is implemented, use_ssl enables protocol ldaps:// with makes php_ldap use TLS to establish the connection. This is what is reported into my OpenLDAP server's log:

conn=1472 fd=28 TLS established tls_ssf=256 ssf=256 tls_proto=TLSv1.3 tls_cipher=TLS_AES_256_GCM_SHA384

If I set use_tls instead, LdapRecord sticks to the ldap:// protocol and expects an upgrade to TLS via the STARTTLS protocol. It makes the connection to my OpenLDAP server fail because it is a TLS only server.

As I understand it the use_ssl configuration option should be renamed as use_tls because SSL is deprecated anf TLS replaces it. And current use_tls should be renamed use_starttls because it is what it does actually.

@stevebauman
Copy link
Member

Hey @pini-gh!

Thanks for the report -- you're right, these should be renamed.

Though it'll have to wait until the next major version, as changing them now would be major breaking change.

Work on v4 is already underway, so this rename will be available in that release! 👍

@miken32
Copy link

miken32 commented Feb 25, 2025

Agree these names are confusing because in the real world SSL and TLS do (or did) the same thing, but the two options cause very different behaviour. Rather than replacing the old names I would suggest adding new names that explicitly say what mechanisms are being used. e.g. use_starttls as a synonym for use_tls and use_ldaps as a synonym for use_ssl.

@stevebauman
Copy link
Member

stevebauman commented Feb 25, 2025

@miken32 I think that's a great idea! Though I would only want one set of configuration options that set these vs adding new ones as aliases (correct me if I'm misunderstanding of course). v4 will contain breaking changes so it'll be a good time to adjust these 👍

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

No branches or pull requests

3 participants