-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Implementing SSL certificate validation for LDAP authentication. #8569
Conversation
9807464
to
82ce838
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Tested with https://github.com/Fmstrat/samba-domain and importing the self signed cert into the jks keystore.
|
@bernd This change should be mentioned in the release notes, as this might break customer setups |
|
@mpfz0r @dennisoelkers Thanks for the heads-up. Can one of you create an update for the |
|
Will this be part of 3.3.3 release? As I know of several setups that will break when this behavior is changed I hope it will not be part of a minor release without some highly visible warning and/or opt-in. |
|
@fadenb: We do not include breaking changes in minor/patch releases. In this case we do, as it is a security-related bugfix. We are aware that it might break setups which were configured incorrectly before this change, but do consider it to be required for secure operations. The upgrade notes for |
Description
Motivation and Context
Note: This needs a backport to
v3.3.xBefore this change, SSL certificates used to secure the connection between Graylog and an LDAP server used for authentication over a SSL/TLS-secured connection were not validated, even if the
Allow self-signed certificatesoption was left unchecked.This change is implementing SSL certificate validation by configuring a trust manager that uses the default keystore to validate certificates against. In addition, it consolidates different places where the
LdapConnectionConfigwas created.Fixes #5906.
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: