The move `String(Value&&)` constructor tries to partially move `String`
values from a `Value` type. However, since there was no an appropriate
`Value::Get<T>()` implementation that binds to the requested move
operation, the compiler will actually not move the value but copy it
instead as the only available implementation of `Value::Get<T>()`
returns a const reference `const T&`. This commit adds a new overload
that returns a non-const reference and allows to optionally move the string
value of a Value type.
The Icinga DB code performs intensive operations on certain STL containers,
primarily on `std::vector<String>`. Specifically, it inserts 2-3 new elements
at the beginning of a vector containing thousands of elements. Without this commit,
all the existing elements would be unnecessarily copied just to accommodate the new
elements at the front. By making this change, the compiler is able to optimize STL
operations like `push_back`, `emplace_back`, and `insert`, enabling it to prefer the
move constructor over copy operations, provided it is guaranteed that no exceptions
will be thrown.
It's not used. Also, the callback shall run completely at once. This ensures that it won't (continue to) run once another coroutine on the strand calls Timeout#Cancel().
Calling `AsioTlsStream::async_shutdown()` performs a TLS shutdown which
exchanges messages (that's why it takes a `yield_context`) and thus has the
potential to block the coroutine. Therefore, it should be protected with a
timeout. As `async_shutdown()` doesn't simply take a timeout, this has to be
implemented using a timer. So far, these timers are scattered throughout the
codebase with some places missing them entirely. This commit adds helper
functions to properly shutdown a TLS connection with a single function call.
The .ti files call `DependencyGraph::AddDependency(this, service.get())`. Obviously, `service.get()` is the parent and `this` (Downtime, Notification, ...) is the child. The DependencyGraph terminology should reflect this not to confuse its future users.
The previous validation in set_verify_callback() could be bypassed, tricking
Icinga 2 into treating invalid certificates as valid. To fix this, the
validation checks were moved into the IsVerifyOK() function.
This is tracked as CVE-2024-49369, more details will be published at a later time.
This allows the function to be used both with a double timestamp or a pointer
to a tm struct. With this, a similar implementation inside the tests can simply
use our regular function.
So far, the return value of strftime() was simply ignored and the output buffer
passed to the icinga::String constructor. However, there are error conditions
where strftime() returns 0 to signal an error, like if the buffer was too small
for the output. In that case, there's no guarantee on the buffer contents and
reading it can result in undefined behavior. Unfortunately, returning 0 can
also indicate success and strftime() doesn't set errno, so there's no reliable
way to distinguish both situations. Thus, the implementation now returns the
empty string in both cases.
I attempted to use std::put_time() at first as that allows for better error
handling, however, there were problems with the implementation on Windows (see
inline comment), so I put that plan on hold at left strftime() there for the
time being.
localtime() is not thread-safe as it returns a pointer to a shared tm struct.
Everywhere except on Windows, localtime_r() is used already which avoids the
problem by using a struct allocated by the caller for the output.
Windows actually has a similar function called localtime_s() which has the same
properties, just with a different name and order of arguments.
The previous implementation actually had undefined behavior when called with a
double that can't be represented as time_t. With boost::numeric_cast, there's a
convenient cast available that avoids this and throws an exceptions on
overflow.
It's undefined behavior ([0], where the implicit conversion rule comes into
play because the C-style cast uses static_cast [1] which in turn uses the
imlicit conversion as per rule 5 of [2]):
> A prvalue of floating-point type can be converted to a prvalue of any integer
> type. The fractional part is truncated, that is, the fractional part is
> discarded.
>
> * If the truncated value cannot fit into the destination type, the behavior
> is undefined (even when the destination type is unsigned, modulo arithmetic
> does not apply).
Note that on Linux amd64, the undefined behavior typically manifests itself in
the result being the minimal value of time_t which then results in localtime_r
failing with EOVERFLOW.
[0]: https://en.cppreference.com/w/cpp/language/implicit_conversion#Floating.E2.80.93integral_conversions
[1]: https://en.cppreference.com/w/cpp/language/explicit_cast
[2]: https://en.cppreference.com/w/cpp/language/static_cast
While analyzing a possible memory leak, we encountered several coroutine
exception messages, which unfortunately do not provide any information
about what exactly went wrong, as exception diagnostics were previously
only logged at the notice level.
Non-ECC DHE ciphers in the `cipher_list` attribute of `ApiListener` (the
default value includes these) had no effect as no DH parameters were available
and therefore the server wouldn't offer these ciphers. OpenSSL provides
built-in DH parameters starting from version 1.1.0, however, these have to be
enables explicitly using the `SSL_CTX_set_dh_auto()` function. This commit does
so and thereby makes it possible to establish a connection to an Icinga 2
server using a DHE cipher.
