From 312ce1d67601c047b53d6cc69eaa82d0d0a60379 Mon Sep 17 00:00:00 2001 From: Isaac Connor Date: Sat, 13 Jun 2026 21:12:49 -0500 Subject: [PATCH] fix: pin ONVIF WS-Security Timestamp and UsernameToken Created together ONVIF PullMessages intermittently failed with ter:NotAuthorized (logged as the misleading clock-drift error) every few thousand requests, then ZM tore down a healthy subscription and re-subscribed. SOAP logs showed the trigger: the failing request always had wsu:Timestamp/Created one second behind UsernameToken/Created, while every successful request had them identical. The cause is in set_credentials(): soap_wsse_add_Timestamp() and soap_wsse_add_UsernameTokenDigest() each call time(NULL) on their own, so when the two calls straddle a one-second boundary the two Created values diverge by a second and Hikvision rejects the request. This is probabilistic, which is why it hit roughly hourly per camera and constantly across a fleet. Capture time(NULL) once, re-stamp the Timestamp Created/Expires from it, and use soap_wsse_add_UsernameTokenDigest_at() so the token Created and its password digest are pinned to the same instant. Both Created values are then always identical. Add tests/zm_onvif_wsse.cpp asserting the two Created values match. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/zm_monitor_onvif.cpp | 29 ++++++++++++- tests/CMakeLists.txt | 1 + tests/zm_onvif_wsse.cpp | 94 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 tests/zm_onvif_wsse.cpp diff --git a/src/zm_monitor_onvif.cpp b/src/zm_monitor_onvif.cpp index fd92c79f7..a1516e460 100644 --- a/src/zm_monitor_onvif.cpp +++ b/src/zm_monitor_onvif.cpp @@ -1219,11 +1219,34 @@ void ONVIF::set_credentials(struct soap *soap) { return; } soap_wsse_delete_Security(soap); + + // Pin a single timestamp for the whole security header. gSOAP's + // soap_wsse_add_Timestamp() and soap_wsse_add_UsernameTokenDigest() each call + // time(NULL) independently, so when the two calls straddle a one-second + // boundary the wsu:Timestamp Created and the UsernameToken Created differ by a + // second. Hikvision (and some other cameras) reject that mismatch as + // NotAuthorized, which surfaced as intermittent PullMessages auth failures + // every few thousand requests despite correct credentials and synced clocks. + // Capturing time(NULL) once and forcing both Created values to it removes the + // race. + time_t wsse_now = time(nullptr); + // Use configurable timestamp validity (default 60 seconds) to handle clock drift // between ZoneMinder and the camera. The old value of 10 seconds was too short // and caused "not authorized" errors when clocks were slightly out of sync. soap_wsse_add_Timestamp(soap, "Time", timestamp_validity_seconds); + // soap_wsse_add_Timestamp() stamped Created/Expires from its own time(NULL). + // Re-stamp them from wsse_now so they match the UsernameToken Created below. + _wsse__Security *security = soap_wsse_add_Security(soap); + if (security && security->wsu__Timestamp) { + security->wsu__Timestamp->Created = soap_strdup(soap, soap_dateTime2s(soap, wsse_now)); + if (security->wsu__Timestamp->Expires) { + security->wsu__Timestamp->Expires = + soap_strdup(soap, soap_dateTime2s(soap, wsse_now + timestamp_validity_seconds)); + } + } + const char *username = parent->onvif_username.empty() ? parent->user.c_str() : parent->onvif_username.c_str(); const char *password = parent->onvif_username.empty() ? parent->pass.c_str() : parent->onvif_password.c_str(); @@ -1232,9 +1255,11 @@ void ONVIF::set_credentials(struct soap *soap) { Debug(2, "ONVIF: Using UsernameToken (plain) authentication"); soap_wsse_add_UsernameTokenText(soap, "Auth", username, password); } else { - // Try UsernameTokenDigest authentication (default) + // Try UsernameTokenDigest authentication (default). + // Use the _at variant so the token's Created (and the password digest + // computed from it) is pinned to the same wsse_now as the Timestamp above. Debug(2, "ONVIF: Using UsernameTokenDigest authentication"); - soap_wsse_add_UsernameTokenDigest(soap, "Auth", username, password); + soap_wsse_add_UsernameTokenDigest_at(soap, "Auth", username, password, wsse_now); } } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c13a6f3d4..2fe1fdf68 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -18,6 +18,7 @@ set(TEST_SOURCES zm_crypt.cpp zm_font.cpp zm_onvif_renewal.cpp + zm_onvif_wsse.cpp zm_pixformat.cpp zm_poly.cpp zm_time.cpp diff --git a/tests/zm_onvif_wsse.cpp b/tests/zm_onvif_wsse.cpp new file mode 100644 index 000000000..11ce0e31d --- /dev/null +++ b/tests/zm_onvif_wsse.cpp @@ -0,0 +1,94 @@ +/* + * This file is part of the ZoneMinder Project. See AUTHORS file for Copyright information + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see . + */ + +#include "zm_catch2.h" + +#include +#include + +#include "soapH.h" +#include "plugin/wsseapi.h" + +// Regression test for the WS-Security Created race in ONVIF::set_credentials(). +// +// gSOAP's soap_wsse_add_Timestamp() and soap_wsse_add_UsernameTokenDigest() +// each call time(NULL) on their own. When those two calls straddle a one-second +// boundary, the wsu:Timestamp Created and the UsernameToken Created end up one +// second apart, and Hikvision (and some other cameras) reject the request as +// NotAuthorized. set_credentials() avoids this by capturing a single time(NULL) +// and forcing both Created values to it (re-stamping the Timestamp and using the +// _at variant for the token digest). +TEST_CASE("ONVIF WS-Security Created timestamps are pinned together") { + SECTION("Timestamp Created matches UsernameToken Created (digest auth)") { + struct soap *soap = soap_new(); + REQUIRE(soap != nullptr); + soap_register_plugin(soap, soap_wsse); + + const int validity = 60; + + // Reproduce the exact sequence set_credentials() uses for digest auth. + soap_wsse_delete_Security(soap); + time_t wsse_now = time(nullptr); + soap_wsse_add_Timestamp(soap, "Time", validity); + + _wsse__Security *security = soap_wsse_add_Security(soap); + REQUIRE(security != nullptr); + REQUIRE(security->wsu__Timestamp != nullptr); + security->wsu__Timestamp->Created = soap_strdup(soap, soap_dateTime2s(soap, wsse_now)); + if (security->wsu__Timestamp->Expires) { + security->wsu__Timestamp->Expires = + soap_strdup(soap, soap_dateTime2s(soap, wsse_now + validity)); + } + + soap_wsse_add_UsernameTokenDigest_at(soap, "Auth", "admin", "password", wsse_now); + + REQUIRE(security->wsu__Timestamp->Created != nullptr); + REQUIRE(security->UsernameToken != nullptr); + REQUIRE(security->UsernameToken->wsu__Created != nullptr); + + // The whole point of the fix: these two must always be identical, so the + // camera never sees the off-by-one second that triggers NotAuthorized. + REQUIRE(std::string(security->wsu__Timestamp->Created) == + std::string(security->UsernameToken->wsu__Created)); + + // And the pinned value is exactly the one we captured. + REQUIRE(std::string(security->wsu__Timestamp->Created) == + std::string(soap_dateTime2s(soap, wsse_now))); + + soap_wsse_delete_Security(soap); + soap_destroy(soap); + soap_end(soap); + soap_free(soap); + } + + SECTION("A one-second gap really does change the Created string") { + // Documents why the un-pinned (racy) code fails: two time(NULL) results one + // second apart serialise to different Created values, which is precisely the + // mismatch Hikvision rejected in the captured SOAP logs. + struct soap *soap = soap_new(); + REQUIRE(soap != nullptr); + + time_t now = time(nullptr); + std::string a(soap_dateTime2s(soap, now)); + std::string b(soap_dateTime2s(soap, now + 1)); + REQUIRE(a != b); + + soap_destroy(soap); + soap_end(soap); + soap_free(soap); + } +}