Revert "Make `Dictionary::Remove(Iterator)` return the next iterator" and
change the one location that now made use of that return value to increment the
iterator itself beforehand. I don't see anything that would be wrong with the
previous code and I'm pretty certain this is a compiler bug. While at it, I've
also removed a stray semicolon and changed the increment in the other branch to
the prefix operator as the value is discarded.
[35/151] Building CXX object lib/base/CMakeFiles/base.dir/base_unity.cpp.o
FAILED: lib/base/CMakeFiles/base.dir/base_unity.cpp.o
/usr/bin/c++ -DBOOST_ASIO_USE_TS_EXECUTOR_AS_DEFAULT -DBOOST_COROUTINES_NO_DEPRECATION_WARNING -DBOOST_FILESYSTEM_NO_DEPRECATED -DOPENSSL_SUPPRESS_DEPRECATED -D_GNU_SOURCE -I/builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f -I/builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib -isystem /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/third-party/nlohmann_json -isystem /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/third-party/utf8cpp/source -isystem /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/third-party/mmatch -isystem /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/third-party/socketpair -O2 -g -Wsuggest-override -g -pthread -Winvalid-pch -O2 -g -DNDEBUG -std=c++1z -MD -MT lib/base/CMakeFiles/base.dir/base_unity.cpp.o -MF lib/base/CMakeFiles/base.dir/base_unity.cpp.o.d -o lib/base/CMakeFiles/base.dir/base_unity.cpp.o -c /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/base_unity.cpp
In file included from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/configobject.cpp:8:0,
from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/base_unity.cpp:9:
/builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/serializer.hpp:20:14: warning: 'virtual const char* icinga::CircularReferenceError::what() const' can be marked override [-Wsuggest-override]
const char *what(void) const throw() final;
^~~~
In file included from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/filelogger-ti.hpp:12:0,
from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/filelogger.hpp:8,
from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/filelogger.cpp:4,
from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/base_unity.cpp:25:
/builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/streamlogger.hpp:33:7: warning: 'virtual void icinga::StreamLogger::ProcessLogEntry(const icinga::LogEntry&)' can be marked override [-Wsuggest-override]
void ProcessLogEntry(const LogEntry& entry) final;
^~~~~~~~~~~~~~~
/builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/streamlogger.hpp:34:7: warning: 'virtual void icinga::StreamLogger::Flush()' can be marked override [-Wsuggest-override]
void Flush() final;
^~~~~
In file included from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/config/configitem.hpp:8:0,
from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/scriptutils.cpp:17,
from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/base_unity.cpp:56:
/builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/config/expression.hpp:264:19: warning: 'virtual const icinga::DebugInfo& icinga::DebuggableExpression::GetDebugInfo() const' can be marked override [-Wsuggest-override]
const DebugInfo& GetDebugInfo() const final;
^~~~~~~~~~~~
In file included from /builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/base_unity.cpp:78:0:
/builds/packages/icinga2/packaging/sles/15.7/BUILD/icinga2-2.16.3+111.g8ff3d586f/lib/base/workqueue.cpp:80:1: internal compiler error: Segmentation fault
}
^
Please submit a full bug report,
with preprocessed source if appropriate.
See <https://bugs.opensuse.org/> for instructions.
This reverts commit 3cb6e5a152.
to fix a build error against OpenSSL 4.0.
OpenSSL 4.0 made `ASN1_INTEGER` an opaque type.
Also, `X509_get0_serialNumber()` must be used now,
which returns `const ASN1_INTEGER*`.
to fix a build error against OpenSSL 4.0.
OpenSSL 4.0 made `X509_NAME*` pointers returned by `X509_get_subject_name()`,
`X509_REQ_get_subject_name()` and related getters const.
In contrast, under OpenSSL 1.x, setters expect non-const pointers,
hence our new typedef `X509NameConstPtr`.
- Use `X509NameConstPtr` parameters in `GetX509NameCN()`, `CreateCert()` and,
`CreateCertIcingaCA()` to match the const expectations per OpenSSL version
- Replace `X509_REQ_get_subject_name()` with `X509_NAME_new()` +
`X509_REQ_set_subject_name()` for CSR subject modification,
since the returned data can't be directly modified now
`DiagnosticInformation()` wasn't able to take a `std::exception_ptr` due
to the missing conversion on older boost versions, so now everything uses
the std::exception_ptr instead. There are still a few reasons to use
`boost::exception` in some places, but for exception pointers, the standard
one should be better in most cases and almost never requires to include an
extra header.
This was likely meant as a fallback for non-`std::exception`
exception pointers, but it would never have been reached, because
`(std|boost)::rethrow_exception()` would have thrown those out of
the function scope.
This fixes the fallback by making it a catch handler and also using
the more direct way of getting this fallback diagnostic info inside
a catch handler.
There is some race condition when the `async_write()`/`async_flush()` operation
for the `icinga::Hello` message fails (connection reset by peer for example)
around the same time the connect timeout fires and calls `cancel()` on the
stream, the following call to `async_shutdown()` may block indefinitely. If
that happens, the endpoint remains in the connecting state and no new
connection attemps are initiated.
This commit fixes the issue by removing the `Defer` containing the
`async_shutdown()`. The purpose of `async_shutdown()` is to signal a clean
termination of the connection to the peer, which really isn't something that
makes sense to to in a `Defer` block that is also executed in case of errors.
For the one situation where doing a clean TLS shutdown makes some sense
(closing anonymous client connections), a call to GracefulShutdown() is added
to that specific code path.
A large part of the change is just changing the indentation of the code, given
that a now unnecessary `try`/`catch` block is removed.
The following Go code creates a TLS server that can be used to demonstrate the
issue. Note that given that a race condition is involved, this is not reliable
and the sleep duration may need some fine-tuning. For this to work,
`ApiListener.tls_handshake_timeout` needs to be set to a large-enough value
like 60s to disable the timeout for `async_handshake()` itself so that the
overall connect timeout is the one that fires. However, changing the timeout is
not a prerequisite for the problem, it just makes it easier to reproduce. The
error can also happen with the default timeouts if the TCP connect takes long
enough so that the handshake is started late enough that its timeout expires
after the connect timeout.
package main
import (
"crypto/tls"
"log"
"net"
"time"
)
func main() {
cert, err := tls.LoadX509KeyPair("bad-agent.crt", "bad-agent.key")
if err != nil {
panic(err)
}
listener, err := tls.Listen("tcp", ":1337", &tls.Config{
Certificates: []tls.Certificate{cert},
})
if err != nil {
panic(err)
}
log.Println("Listening on", listener.Addr())
for {
conn, err := listener.Accept()
if err != nil {
panic(err)
}
go handle(conn.(*tls.Conn))
}
}
func handle(conn *tls.Conn) {
addr := conn.RemoteAddr().String()
log.Println(addr, "new connection")
time.Sleep(15*time.Second - 10*time.Millisecond)
log.Println(addr, "SetLinger(0)", conn.NetConn().(*net.TCPConn).SetLinger(0))
log.Println(addr, "Handshake()", conn.Handshake())
log.Println(addr, "conn.NetConn().Close()", conn.NetConn().Close())
}
With additional logging in the `catch` block for `boost::system::system_error`
and `Defer shutdownSslConn` (both removed by this commit), this showed the
following. Note that in particular, `async_shutdown()` never returned,
indicating that it hangs in there.
[2026-04-24 17:32:56 +0200] information/ApiListener: Reconnecting to endpoint 'bad-agent' via host 'host.docker.internal' and port '1337'
[2026-04-24 17:33:11 +0200] critical/ApiListener: Timeout while reconnecting to endpoint 'bad-agent' via host 'host.docker.internal' and port '1337', cancelling attempt
[2026-04-24 17:33:11 +0200] information/ApiListener: New client connection for identity 'bad-agent' to [172.17.0.1]:1337
[2026-04-24 17:33:12 +0200] information/ApiListener: rethrowing for bad-agent: Error: Connection reset by peer [system:104 at /usr/include/boost/asio/detail/reactive_socket_send_op.hpp:137 in function 'do_complete']
[2026-04-24 17:33:12 +0200] information/ApiListener: doing async_shutdown for bad-agent
The recursion depth limit added to JsonDecode() in 2.16.2 gave the C++
function a second parameter with a default value. Function pointers do not
carry default arguments, so the DSL function binding deduced an arity of 2
via boost::function_types::function_arity and required two arguments. As a
result `Json.decode("...")` failed with "Too few arguments for function",
an undocumented breaking change in a patch release.
Wrap JsonDecode() in a single-argument shim (mirroring the existing
JsonEncodeShim) so the registered function keeps its one-parameter contract
while still applying the default depth limit internally.
refs #10913
More relaxed memory_order = less safety guarantees = faster execution.
This is safe because std::shared_ptr does the same. See also:
https://en.cppreference.com/w/cpp/atomic/memory_order
"Typical use for relaxed memory ordering is incrementing counters, such as the reference counters of std::shared_ptr, since this only requires atomicity, but not ordering or synchronization (note that decrementing the std::shared_ptr counters requires acquire-release synchronization with the destructor)."
This is a false-positive because the functions in question return
a reference to memory in a static variable that remains fixed after
program start.
This commit adds a macro for the `[[gnu::no_dangling]]` attribute to
ignore the warning for the tagged functions and uses that macro on
both functions returning references to the static variable.
This is currently a false positive on the debian:11 target and my
assumption is that the older GCC can't prove that when `ProcessWaitPID()`
returns early, `WIFSIGNALED(status)` is never used.
However leaving the variables uninitialized like that is bad anyway, since
this might easily be missed when slightly refactoring the function or
if/else blocks and then lead to undefined behavior. So now they just get
initialized to zero.
implicit conversions between signed `char` and `unsigned char` and
vice versa should be save and converting to unsigned char first
circumvents this warning on platforms where `char` is signed.
In the `boost::asio::spawn()` call for newer Boost versions with
`std::allocator_arg`, switch from `fixedsize_stack` to
`protected_fixedsize_stack` in order to allocate the stacks with guard pages.
This is done as an additional safeguard in case there was still some way to
overflow, this at least reliably crashes the process instead of going into
undefined behavior, which could even result in code execution.
Unfortunately, the old-style `spawn()` function with
`boost::coroutines::attributes` does not - at least to my knowledge - provide a
way to request a stack allocated with guard pages, hence this is only enabled
for Boost 1.87 and later with this commit.
Add validation checks to code paths reachable from the HTTP API (except full
config file deployments via /v1/config) that prevent creating deeply nested
data structures that could later cause a stack overflow.
With the limit from the previous commit, if a JSON-RPC now message fails to
parse due to being nested to deep, it would have torn down the whole
connection. It is still possible to trigger that scenario from DSL config (for
example by returning nested structures from a lambda that is used in a check
with command_endpoint). In order to fail more gracefully, only discard the
single message and don't kill the whole connection.
If parsing JSON is rejected due to the depth limit introduced in the last
commit, also include the path (like root["object"]["children"]...) that exceeds
the allowed nesting depth.
Data structures parsed from JSON may be accessed recursively, so deeply nested
structures may wreak havoc by overflowing the stack. Thus, enforce a general
nesting depth limit of 24 by default (which should be more than enough for
reasonable use), with the ability to pass a different limit to JsonDecode() if
needed.
Since perfdata is set once when a check result is created and
never changed again, locking this is unnecessary. This avoids
components unnecessarily waiting on each other when processing
perfdata.
This fixes the locking cascade observed sometimes when the perfdata
writer work queue blocks, where it extends to a lock on the entire
check result eventually, affecting even more components.
Icinga (DB) Web has a special permission to show the check command line.
Well-written plugins receive passwords via env vars which don't appear even there.
Now ifw-api also hides the password using curl -u USER, not USER:PASS.
The new command is still working, it just prompts for the password.
int is just not the right type to use for an index variable, so change it to
size_t, even if it's unlikely to be called on sufficently large inputs that it
would actually make a difference.
Forgot to replace one of the two uses of SHA_DIGEST_LENGTH with length in
6cd3a483a0. All users use it for SHA1 so far, so
there was no wrong usage yet.
