processone / ejabberd

Robust, Ubiquitous and Massively Scalable Messaging Platform (XMPP, MQTT, SIP Server)
https://www.process-one.net/en/ejabberd/
Other
6.01k stars 1.5k forks source link

23.10 update broke ldaps #4110

Closed luizluca closed 8 months ago

luizluca commented 8 months ago

Before creating a ticket, please consider if this should fit the discussion forum better.

Environment

Configuration (only if needed): grep -Ev '^$|^\s*#' ejabberd.yml

auth_method: [ldap]
ldap_servers:
  - "exampe.com"
ldap_encrypt: tls
ldap_base: "dc=example,dc=com"
ldap_uids: {
    mail: "%u@example.com",
    samAccountName: "%u"
}
ldap_filter: "(!(samAccountName=*-*))"
ldap_tls_verify: hard
ldap_tls_cacertfile: "/etc/ssl/certs/MyExampleCA.pem"

Errors from error.log/crash.log

LDAP connection to example.com:636 failed: Invalid option fail_if_no_peer_cert server_only

Bug description

Our jabber stopped to work after we updated to ejabberd-23.10-1.x86_64. I couldn't find a specific option that could change the error.

prefiks commented 8 months ago

Hi, do you have line number in that error message? Could you also see if changing ldap_tls_verify: hard to soft changes that message?

prefiks commented 8 months ago

I guess this will work with ldap_tls_verify: false, i think newest version of erlang don't accept option that we use to force certificate verification.

luizluca commented 8 months ago

Hi, do you have line number in that error message? Could you also see if changing ldap_tls_verify: hard to soft changes that message?

"ldap_tls_verify: soft" does allow it to work, but it is a bad workaround as it will not fail with a broken certificate.

prefiks commented 8 months ago

It looks like this happens when using erlang 26, i did commit c2d04bc478e8507fb00e07add720171d71cb8a2f, that should make it work on this version (and older as well).

nosnilmot commented 8 months ago

"ldap_tls_verify: soft" does allow it to work, but it is a bad workaround as it will not fail with a broken certificate.

actually ldap_tls_verify: soft will (in ejabberd =< 23.10) verify certificate and fail if it is broken. fail_if_no_peer_cert: false says to not fail if no peer certificate is presented, but that only applies to client certificates, which are not relevant when verifying ldap server (hence the error from erlang 26)

It looks like this happens when using erlang 26, i did commit c2d04bc, that should make it work on this version (and older as well).

this should use {verify, verify_peer} for both ldap_tls_verify: soft and ldap_tls_verify: hard otherwise ldap_tls_verify: soft is misleading and not doing any certificate verification, and a significant change in behaviour!

prefiks commented 8 months ago

I was looking at this

This option specifies whether to verify LDAP server "
               "certificate or not when TLS is enabled. When 'hard' is set, "
               "ejabberd doesn't proceed if the certificate is invalid. "
               "When 'soft' is set, ejabberd proceeds even if the check has failed. "
               "The default is 'false', which means no checks are performed.

Which would suggest that we should ignore errors about invalid certs for soft, it's pretty much what verify_none does (it's still check certificate but function that does verification just ignore bad_cert error). If that's not correct logic here i am ok with changing it.

prefiks commented 8 months ago

Ok, after checking what those two version initially did, i agree that they should be changed to verify_peer. also looks like for false case we also need to add verify_none, as on 26 this option defaults to verify_peer, and fails to connect because of missing cacerts.

prefiks commented 8 months ago

I did that in 08a78a1654947046d2288a6660775feab5416b42