Every request through index.php starts a session and always dirties it:
zm_session_set_remote_addr() writes remoteAddr, and index.php stores skin,
css and navbar_type. ZMSessionHandler::write() then persisted that session
unconditionally, so any request arriving without a ZMSESSID cookie left a
Sessions row behind that nothing would ever load again.
Viewing an event polls the event's server every ZM_WEB_REFRESH_STATUS
seconds via monitorUrl, which is absolute when the monitor has a Server
row. Those cross-origin ajax polls carry auth in the URL and no cookie, so
each one added a Sessions row every few seconds. Bot scans of the login
page did the same.
Persist a session only when the client presented our cookie, or when
zm_session_persist() marks it as one we are issuing: login, and the
postLoginQuery stashed before redirecting to the login page.
Verified on a live install by logging row counts from the save handler:
three cookieless requests skipped the write and left the count unchanged,
while a cookie-jar run wrote on the request that returned the cookie.
php -l clean on both files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GAFKf86P78WqniPEP2b45J
database.php called dbConnect() at file scope, so including it opened a socket,
and on failure rendered views/no_database_connection.php and exit()ed from
inside a library include. Every model in web/includes requires this file, so
merely loading a class did both.
Connect on first use instead. $dbConn becomes tri-state - false for "not
attempted", null for "attempt failed", a PDO for connected - and two accessors
sit on top of it:
zmDbConn() opens if needed; on failure renders the error view and
stops, which is what the include used to do, just at the
point a query is actually attempted.
zmDbConnOrNull() opens if needed but returns null instead of ending the
request, for callers with a fallback.
dbQuery() is the funnel every fetch helper goes through, so routing it plus
dbEscape(), dbError() and dbInsertId() through the accessors covers the library.
The five callers that reached for the raw global are updated: config.php.in,
Event.php and ajax/console.php need a connection and take zmDbConn(); logger.php
takes zmDbConnOrNull() and falls through to its error_log target, so a logging
call can no longer end the request or open a connection by itself.
ZMSessionHandler captured $dbConn in its constructor. It is constructed while
session.php is being included, before anything has needed the database, so with
a lazy connection that captured false. It now resolves per call and its methods
return "no session" rather than dereferencing a bool.
Two smaller fixes fall out. The error view was included by a relative path that
only resolved when the cwd was web/, so it never worked for requests served out
of web/api/; it is now anchored with __DIR__. And dbDisconnect() set $dbConn to
null, which in the new tri-state means "connecting failed" and would send the
next query to the error page; it sets false so a later query can reconnect.
Nothing calls dbDisconnect() today.
This does NOT make database.php includable without a database. It requires
logger.php, which requires config.php, which reads ZoneMinder's configuration
out of the Config table at include time. Until that cycle is broken the
connection still happens during bootstrap, just from config.php rather than from
here.
Tests: tests/php/test_database_lazy_connect.php, 7 assertions, all pass. It
tokenises database.php and asserts nothing runs at include time, that dbQuery()
goes through the accessor, and that only the connection plumbing touches the
global. Verified it reports the pre-refactor file's `if ( !dbConnect() )` - an
earlier version of the check skipped tokens inside parentheses and so passed on
exactly the code it exists to reject.
Not covered by tests: behaviour when the database is genuinely unreachable, and
the session handler against a live database. Needs manual testing on an
installed tree, including stopping mysql to confirm the error view still renders
for both a web request and an API request.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01477mR97vfnK6zczbHgzq6T
ZM_AUTH_HASH_IPS binds the auth hash to the client address. When that address
changes mid-session - a phone moving between wifi and cellular is the common
case - the hash the browser is still holding no longer matches the address we
now see, and the user is bounced to the login page. The usual workaround is to
turn ZM_AUTH_HASH_IPS off entirely.
Accept the address the request arrives from plus the one it arrived from
immediately before, so an in-flight hash validates once and generateAuthHash()
then reissues against the new address. Addresses are matched exactly. A netmask
was considered and rejected: accepting a whole subnet would let any other host
on the client's network replay a stolen hash, which on a home LAN includes the
cameras themselves.
The previous address is only accepted for as long as a hash issued to it would
itself still be valid (ZM_AUTH_HASH_TTL), so this widens which address is
accepted without extending how long any hash lives. A login clears it, since
nothing from before a privilege boundary should stay acceptable, and only one
previous address is ever retained.
userFromSession() needed the same treatment: it looks the cached hash up by the
live address, so after a change the slot does not exist yet and the user was
reported as not logged in regardless of what getAuthUser() would have accepted.
Also centralises the X-Forwarded-For/REMOTE_ADDR handling in getRemoteAddr(),
replacing four duplicated copies across session.php and auth.php. Those copies
sat on both the generation and validation sides, so any drift between them broke
authentication outright behind a reverse proxy. Network.php holds only that
address parsing; which addresses an auth hash is accepted from is auth policy
and lives in auth.php.
This is web-side only; zms has no session, so a stream request still fails once
on an address change and recovers through the existing auth-refresh path in
MonitorStream.js.
Tests: tests/php/test_remote_addr.php covers getRemoteAddr() parsing and the
session address rotation, and needs no config or database - 18 assertions, all
pass. tests/php/test_auth_hash_candidate_addrs.php covers the acceptance window
including both sides of the TTL boundary; it bootstraps config.php as the other
tests in that directory do and so needs an installed tree to run.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01477mR97vfnK6zczbHgzq6T
On successful login auth.php called zm_session_clear() followed by
zm_session_regenerate_id(), which together emitted three Set-Cookie
ZMSESSID headers: a deletion, a throwaway intermediate id, and the final
authenticated id. This is the multiple-cookie behaviour reported in #2471.
Add zm_session_regenerate_id_login(), which clears the pre-auth session
data and calls session_regenerate_id(true) to issue one new id while
deleting the old session server-side. Same anti-session-fixation
guarantee in a single Set-Cookie. Logout (zm_session_clear) and the
periodic mid-session regeneration are left unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The session garbage collector ran DELETE FROM Sessions WHERE access < ?
against an unindexed column, forcing a full table scan and taking gap
locks across the access range. With REPLACE INTO Sessions happening on
every authenticated request, this is a deadlock hotspot.
- Add Sessions_access_idx on Sessions(access) in both fresh-install
schema (zm_create.sql.in) and a migration (zm_update-1.39.10.sql).
- Rewrite ZMSessionHandler::gc to a two-phase delete: SELECT up to 100
expired ids via the new index (consistent read, no locks), then
DELETE WHERE id IN (...) by primary key. InnoDB takes record locks
only on the matched rows, not gap locks on the access range.
- Bump version to 1.39.10 so zmupdate.pl picks up the new migration.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When AUTH_HASH_IPS is enabled and ZoneMinder is behind a reverse proxy
(e.g. Nginx in front of Apache), the hash is generated using
HTTP_X_FORWARDED_FOR (the real client IP) but was validated using only
REMOTE_ADDR (the proxy's IP), causing all authentication to fail.
Fix by consistently using HTTP_X_FORWARDED_FOR (first IP only, to guard
against spoofed multi-value headers) with REMOTE_ADDR as fallback in
all three places:
- web/includes/session.php: where remoteAddr is stored for hash generation
- web/includes/auth.php: getAuthUser() validation (PHP, also used by zms CGI)
- src/zm_user.cpp: zmLoadAuthUser() validation (C++ zms binary)
refs #4758
Agent-Logs-Url: https://github.com/ZoneMinder/zoneminder/sessions/959dfe9d-edea-4de5-a3a0-f90b758e5628
Co-authored-by: connortechnology <925519+connortechnology@users.noreply.github.com>
Change ZM_OPT_USE_REMEMBER_ME from a boolean to a tri-state string:
- None: checkbox hidden, sessions persist for ZM_COOKIE_LIFETIME (old disabled)
- Yes: checkbox shown and pre-checked by default
- No: checkbox shown and unchecked by default (old enabled behavior)
Update ConfigData.pm.in with new type definition, login.php to honor the
checked state, and session/action handlers to recognize the new values.
Migration in zm_update-1.39.4.sql maps old '1' to 'No' and '0' to 'None'.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add ZM_OPT_USE_REMEMBER_ME config option (auth section, requires
ZM_OPT_USE_AUTH) that controls whether a Remember Me checkbox appears
on the login form. When enabled and unchecked, the session cookie
lifetime is set to 0 so the browser discards it on close, logging the
user out. When checked, the session persists for ZM_COOKIE_LIFETIME.
When the option is disabled, behavior is unchanged.
- ConfigData.pm.in: new ZM_OPT_USE_REMEMBER_ME boolean option
- login.php: checkbox between password field and reCAPTCHA/submit
- session.php: use lifetime=0 when remember me is off
- actions/login.php: set/clear ZM_REMEMBER_ME cookie on login, also
update $_COOKIE so zm_session_start sees it in the same request
- auth.php: clear ZM_REMEMBER_ME cookie on logout
- en_gb.php: add RememberMe translation string
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* If token is present do token based auth and do not do anything with session
* update HostController. Use config constants, don't use sessions
* Remove Session from the components list
* spacing
* Remove Session from App Components list.
* Move APIEnabled check to the api from auth.php
* Rework auth. login using username and password only occurs on login action now. Including auth.php should not touch the session. auth_hash logins no longer touch the session. replace userLogin with a function called validateUser which matches the semantics of validateToken.
* remove debugging
* Add session storage if stateful query param is on, but only for LEGACY_API_AUTH
* fix mUser to username, etc.
* shuffle lines
* use instead of session when generating auth hash.
* Add docs regarding the use of cookies and stateful query param
* Only open/close session if we are clearing a session var
* Use zm_session_start instead of session_start
* Should use zm_session_start instead of session_start
* document that zm_session_start should be called previously to session_regenerate_id
* Don't actually write out the session when generating auth hashes. Means they should never actually persist.
* More backticking of SQL
* add .. to fix#2686
* Use material icons for sort because they look nicer
* fix typo
* have to add authhash to session on login
* restore username&password login for all urls
* fix
* fixes
* Introduce ZM_COOKIE_LIFETIME which sets the life of the SESSION cookie, instead of using what is in php.ini
* Use zm specific session functions, which are now located in includes/session.php. Be more agressive about clearing session on logout.
* Move session code to includes/session.php
* remove duplicate line
* Move is_session_open to session.php. Move code to clear a session into session.php
* improve debug line when there is a problem updating config entry
* split description into description and help text for COOKIE_LIFETIME
* Remove redirect on line. We do it in javascript on postlogin view so that we can say logging in before switching to console
* If there is a username in the session, then we are logged in, but we need to load the user object from the db. We can't just trust it from the session. The user may have been deleted and having that data in the session can be a security risk. So load the user object on every request.
* Use session_regenerate_id instead of our broken code to do the same
* Move auth code to includes/auth.php
* add autocomplete tags to username and password inputs
* Don't redirect to login if we are already viewing login. Put auth before including skin includes
* need to include session.php in auth.php
* update to php namespace
