feat(core): throttle ActiveSync auth and harden user resolution#187
Merged
Conversation
Add a cache-backed brute-force throttle to the ActiveSync driver's authenticate(): credential failures are counted per (username, client IP) and further attempts are refused once a configurable threshold is reached within a window, closing the gap where the ActiveSync endpoint bypassed Horde's interactive-login protection. A successful credential check clears the counter, so normally-configured devices never accumulate. Tunable via $conf['activesync']['auth']['throttle'] (enabled, max_attempts, window); automatically inert when no cache backend is configured. Harden getUser() so an authenticated identity (auth flow, then registry) always takes precedence over the client-controlled ?User= query parameter. The ?User= value is now only a last resort for the transparent X509 provisioning flow, is logged, and is documented as not being proof of identity (the certificate path verifies it before serving data). Update the getUser() precedence unit tests accordingly.
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
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
authenticate().getUser()so an authenticated identity always takes precedence over the client-supplied?User=query parameter.Motivation
The ActiveSync endpoint runs a full backend authentication on every request but had no failure throttling, so it could be used for credential guessing in a way that bypasses Horde's interactive-login protection. Separately,
getUser()consulted the client-controlled?User=query parameter before the registry-authenticated identity, which is fragile for a value that is not proof of identity.Changes
Horde_Core_ActiveSync_Driver::authenticate()): count credential failures per(username, client IP)in the cache and refuse further attempts once a threshold is reached within a window. A successful credential check clears the counter, so normally-configured devices never accumulate. Tunable via$conf['activesync']['auth']['throttle'](enabled,max_attemptsdefault 15,windowdefault 300s). Automatically inert when no cache backend is configured, so existing deployments without a cache are unaffected. When tripped it returnsAUTH_REASON_UNAVAILABLEso the client backs off.getUser()): reorder precedence to (1) authenticated user, (2) registry-authenticated user, (3)?User=query parameter. The?User=value is now only a last resort for the transparent X509 provisioning flow, is logged, and is documented as not proof of identity (the certificate path inauthenticate()verifies it against the certificate-authenticated identity, clearing the session on mismatch, before any data is served).DriverGetUserTestto assert the new precedence (registry over?User=).Test plan
php -lclean on changed filestest/Unit/ActiveSyncpasses (22/22)