[ldap-user-sync] Add LDAP user sync plugin - #207
Open
ModdingFriendly wants to merge 8 commits into
Open
Conversation
Syncs Admin/Streamer accounts from any standard LDAP directory into Dispatcharr on a schedule or on demand, with XC password provisioning and email delivery.
dispatcharr-plugins-bot
Bot
added
the
New Plugin
This comment has been minimized.
This comment has been minimized.
Stop passing an explicit TLS version into ldap3's Tls() so it takes the hardened ssl.create_default_context() path (TLS 1.2+ floor) instead of a manual SSLContext with no such floor - this plugin's own choice was what triggered ldap3's py/insecure-protocol-flagged branch. Remove the lazily-imported, never-used ntlm.py (only reached via explicit NTLM bind, which this plugin never performs), and suppress the one unavoidable weak-hashing finding in digestMd5.py with an inline codeql[] comment plus an explanation: MD5 there implements the DIGEST-MD5 SASL mechanism's RFC 2831 challenge-response computation, not password storage, and the function is kept only because core/connection.py imports it unconditionally.
This comment has been minimized.
This comment has been minimized.
Collaborator
|
I'll take a quick look as to why the inline suppression comment isn't working on |
Collaborator
The fix is to move the suppression comment to its own line above the line it's intended to suppress: # codeql[py/weak-sensitive-data-hashing]
return hashlib.md5(value).digest()Regarding the |
The previous fix didn't hold: CodeQL's py/insecure-protocol query flags tls.py's own permissiveness (its SSLContext(self.version) branch enforces no minimum version by default) regardless of what any specific caller passes in, and the inline `codeql[]` suppression comment on digestMd5.py's md5_h() was not honored by this repo's SARIF-parsing report step. Fix both properly instead of relying on caller behavior or suppression: - tls.py: explicitly set minimum_version = TLSv1_2 on that branch, so the library itself can no longer negotiate an old protocol version. - digestMd5.py: stub md5_h/md5_kd/md5_hex/md5_hmac to raise NotImplementedError, removing every hashlib.md5 call from the file instead of just commenting around it. core/connection.py and the strategy modules still import these names fine; DIGEST-MD5 SASL auth (which is all that ever calls them) is simply unsupported by this vendored copy, matching the ntlm.py removal in the previous commit.
This comment has been minimized.
This comment has been minimized.
The previous minimum_version fix ran too late: SSLContext(self.version) can pin the context to a single legacy protocol before that line ever executes, so setting minimum_version afterward doesn't undo it - CodeQL correctly kept flagging this as reachable. Validate `version` in Tls.__init__ instead and raise LDAPSSLConfigurationError for any SSLv2/SSLv3/TLSv1/TLSv1.1/SSLv23 constant, checked defensively via hasattr since several no longer exist on newer Python. This plugin never passes an explicit version at all (defaults to None, i.e. ssl.create_default_context()), so this only ever closes off a capability nothing here uses. Keeps the earlier minimum_version assignment as defense in depth.
This comment has been minimized.
This comment has been minimized.
Third attempt at the same finding: CodeQL's py/insecure-protocol flags the mere existence of ssl.SSLContext(self.version) feeding a wrap_socket() call, regardless of runtime validation on `version` (the previous commit's __init__ guard didn't change the result). Delete that branch outright and always build the context via create_default_context() (TLS 1.2+ floor, verified certs) - this plugin never passes an explicit `version` at all, so nothing depends on the removed path.
This comment has been minimized.
This comment has been minimized.
CodeQL's py/insecure-protocol treats create_default_context() itself as not guaranteed to exclude TLSv1/TLSv1.1 unless the returned context also has minimum_version set explicitly - the previous commit removed the custom-version branch but didn't set this on the remaining create_default_context() path. Add it there too.
This comment has been minimized.
This comment has been minimized.
…mments A maintainer pointed out the inline suppression comment needs to be on its own line above the flagged statement, not a trailing same-line comment - that's why it wasn't honored before. Revert all of the previous commits' vendored-source modifications (ntlm.py deletion, digestMd5.py stubbing, tls.py SSLContext rewrite) back to byte-identical upstream, and instead annotate the actual flagged lines in place with correctly-formatted `codeql[<rule-id>]` comments plus an explanation of why each is a protocol-mandated construct this plugin never exercises (SIMPLE bind only, no DIGEST-MD5/NTLM, no explicit TLS version).
Plugin Validation ResultsModified plugins: 1 Plugin:
|
| Check | Status | Details |
|---|---|---|
| Required fields | ✅ | All required fields present |
| Maintainers | ✅ | ModdingFriendly |
| License | ✅ | MIT - MIT License |
| Permission | ✅ | New plugin - ModdingFriendly listed in author/maintainers |
| Version | ✅ | 1.0.0 |
| Version bump | ✅ | New plugin |
❌ CodeQL found 6 high or critical issue(s) - these must be fixed before merging.
| Rule | Location | Description |
|---|---|---|
py/weak-sensitive-data-hashing |
plugins/ldap-user-sync/vendor/ldap3/protocol/sasl/digestMd5.py:49 | Sensitive data (password) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. |
py/weak-sensitive-data-hashing |
plugins/ldap-user-sync/vendor/ldap3/utils/ntlm.py:491 | Sensitive data (password) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD5) that is insecure for password hashing, since it is not a computationally expensive hash function. |
py/weak-sensitive-data-hashing |
plugins/ldap-user-sync/vendor/ldap3/utils/ntlm.py:506 | Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. |
py/weak-sensitive-data-hashing |
plugins/ldap-user-sync/vendor/ldap3/utils/ntlm.py:510 | Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. Sensitive data (password) is used in a hashing algorithm (MD4) that is insecure for password hashing, since it is not a computationally expensive hash function. |
py/insecure-protocol |
plugins/ldap-user-sync/vendor/ldap3/core/tls.py:208 | Insecure SSL/TLS protocol version TLSv1 allowed by call to create_default_context. Insecure SSL/TLS protocol version TLSv1 allowed by call to ssl.SSLContext. Insecure SSL/TLS protocol version TLSv1_1 allowed by call to create_default_context. Insecure SSL/TLS protocol version TLSv1_1 allowed by call to ssl.SSLContext. |
py/insecure-protocol |
plugins/ldap-user-sync/vendor/ldap3/core/tls.py:211 | Insecure SSL/TLS protocol version TLSv1 allowed by call to create_default_context. Insecure SSL/TLS protocol version TLSv1 allowed by call to ssl.SSLContext. Insecure SSL/TLS protocol version TLSv1_1 allowed by call to create_default_context. Insecure SSL/TLS protocol version TLSv1_1 allowed by call to ssl.SSLContext. |
CodeQL found 1 medium severity issue(s)
These are not blocking, but are included for visibility.
| Rule | Location | Description |
|---|---|---|
py/bind-socket-all-network-interfaces |
plugins/ldap-user-sync/vendor/ldap3/core/server.py:337 | Binding a socket to all interfaces (using '') is a security risk. |
❌ Validation failed
Some checks failed. Please review the errors above and update your PR.
Collaborator
|
The workflow should be honouring those comments, apologies for all the troubles on this one. Edit: Coming back around this week (Aug 10) - thanks for your patience. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ldap3(LGPL-3.0-only) andpyasn1(BSD-2-Clause) as unmodified source undervendor/, since neither is installed in the Dispatcharr venv and plugins have no dependency-install mechanism. Licenses preserved verbatim undervendor/licenses/.is_active=False), toggle-able via settings.Test plan
ldap3MOCK_SYNC directory (no real network): create with correct role/unusable-password/XC-password generation, XC password stays stable across a repeat sync, disable-on-group-removal leaves XC password untouched, and the reset action rotates the password independently of email delivery outcome - all exercised against a real Postgres via the Django ORM, test users cleaned up after.Test LDAP Connectioncorrectly resolved group membership, and a realSync Nowrun correctly created/updated the expected accounts, including correctly applying "admin takes precedence" when a user is a member of both groups.