CID 11851: (#1 of 1): Structurally dead code (UNREACHABLE)
unreachable: This code cannot be reached
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Instead of exposing protected data to the global environment, provide
a special purpose accessor to satisfy AppControl needs.
Signed-off-by: Mark Deric <jmark@openvpn.net>
Currently PG only allows to either send or withhold the reason to the
client but there are certain circumstances where you want to have more
detailed internal reason but still want to send some reason to the
client.
Signed-off-by: Arne Schwabe <arne@openvpn.net>
Some systems like to see the mapped IPv4 addresses as real IP addresses.
This commit adds the ability to show IP addresses as such.
Signed-off-by: Arne Schwabe <arne@openvpn.net>
This adds the capability to implement a custom app level protocol
that support message passing over the OpenVPN control channel.
The protocol is agnostic to the data that is transported over it
and the message splitting/reassmbly is handled transparently by the
OpenVPN library itself.
Signed-off-by: Arne Schwabe <arne@openvpn.net>
This is an obscure and never used feature to trigger sending
the SSO web auth URL from the client instead of server.
Signed-off-by: Arne Schwabe <arne@openvpn.net>
Using the raw pointer constructor only really works if the pointer is
intrusive. Ensure this with a static assert
Signed-off-by: Arne Schwabe <arne@openvpn.net>
In class Stop, the stop_called member is safe to read
without locking, but make it volatile to document this.
Signed-off-by: James Yonan <james@openvpn.net>
Older clients, 2.5.x and below, send an ACK_V1 packet in response to
the server's HARD_RESET packet whereas 2.6.x clients send a CONTROL_V1
packet. The code that checked the packet length of the client's
response failed to comprehend the fact the ACK_V1 packet does not
include a packet id field following the peer session id field. So the
code rejected the ACK_V1 packet as being too short.
The fix was to require packet length only up to and including the peer
session id field. This works to allow safe parsing for both the
ACK_V1 response and the CONTROL_V1 response.
Signed-off-by: Mark Deric <jmark@openvpn.net>
hash() will return a hash of the raw time. It's useful
for unordered map/set collections where the key is a tuple
that includes a Time object.
Because hash() is a template method, it doesn't introduce
any new dependencies for class Time.
Signed-off-by: James Yonan <james@openvpn.net>
By default, the Netblock constructor already sets the server
gateway to the .1 address of the subnet, but the new method
override_server_gw() can now be used to override that
setting.
Signed-off-by: James Yonan <james@openvpn.net>
It was only supported by mbedTLS and is very easily used wrong since it
is just a boolean value. Other TLS stacks were using the regular strength
PRNG no matter what. Also we should not weaken a crypto strength PRNG,
now that we have the StrongRandomAPI type in place. It might give the
wrong sense of strength, when in reality we might reseed a hundred times
less often.
In places where prng was passed as true before, use MTRand now instead.
Signed-off-by: Heiko Hund <heiko@openvpn.net>
RFC 5077 talks about the ticket key_names to be randomly generated to
avoid collision between servers. To enforce unpredictable randomness,
require a strong PRNG to construct such names.
This is a formality, since all users of the class already provide such a
strong randmon number source.
Signed-off-by: Heiko Hund <heiko@openvpn.net>
Since predictable names for temporary files can potentially cause a
security issue, require such filenames to be generated with
unpredictable randomness.
Signed-off-by: Heiko Hund <heiko@openvpn.net>
Since session ids should always be truly random require a
cryptographically strong random number generator.
Since all places in the codes (besides tests) already pass a strong
random source, this is just a formality.
Signed-off-by: Heiko Hund <heiko@openvpn.net>
The need of having to call the assert_crypto() member function to ensure
that a cryptographically strong RNG is used where needed, was reported
as potentially insecure, since calling it manually can easily be missed.
In the commit the two new classes StrongRandomAPI and WeakRandomAPI are
introduced. They are to be used instead of just RandomAPI, unless it
doesn't matter what strength the RNG is.
All the places the assert_crypto() was called were converted to using
StrongRandomAPI instead. Also the RNGs for which assert_crypto() was not
throwing are now inheriting from StrongRandomAPI.
Variable names, which have the StrongRandomAPI type, but were called
prng, are changed to rng instead to follow the source code convention.
Signed-off-by: Heiko Hund <heiko@openvpn.net>
We're pretty sure that these warnings are false positives.
Both are related to destructing MultiCompleteType objects.
GCC 12 (thread_unsafe_refcount):
rc.hpp:322:18: pointer used after ‘void operator delete(void*, std::size_t)’
[-Wuse-after-free]
GCC 13 (thread_safe_refcount, only arm64):
inlined from ‘...thread_safe_refcount::operator--()’ at .../rc.hpp:404:39
atomic_base.h:645:34: ‘long unsigned int __atomic_sub_fetch_8(...)’ writing
8 bytes into a region of size 0 overflows the destination
[-Werror=stringop-overflow=]
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Refactors the call of a virtual member function in the CTOR
by adding a private _impl function, which is non-virtual and
which both the CTOR and the original virtual function
delegate to.
Signed-off-by: Charlie Vigue <charlie.vigue@openvpn.com>
It's possible that someone might try to derive from and override
here, and if so the result will be perhaps not as expected. This
change will make it slightly harder to produce an unexpected
call to the wrong vtable dispatch from the ctor.
Signed-off-by: Charlie Vigue <charlie.vigue@openvpn.com>
This was changed in commit
ae663c573a ("Using new numeric
conversion tools") to avoid some conversion warnings. But
after understanding the workings of the function better, the
change turns out to have been wrong. Instead the function was
changed to use different intermediate variables for different
purposes.
This change ripples through the whole Netlink/SITNL interface.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Past changes make it clear that the interface was not well
understood.
While here, clean up the code to make it easier to understand.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
The BN_bn2dec() can return NULL if the input is not parseable.
This would cause the conversion of char* to std::string to throw
an exception. Instead check the result and return an empty string
on errors.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
This causes only warnings with -Wpedantic, which we don't
intend to use. But doesn't hurt to fix anyway.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Fixed one issue while at it, with parse() not clearing
the username and password arguments.
The general issue that overflow doesn't throw is reflected in
a disabled test. This will need to be fixed in SplitLines,
probably.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
This really has very different implications than the
others overloads. So make it distinct.
I would also rename the others to parse_opt, but feel
that causes too much churn in the code. parse_file has
only one use in our own code base.
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
This adds two routines:
AWS::Route::create_route_table
This creates route table in given VPC and
assigns "Name" tag to it with provided value.
AWS::Route::get_route_table_by_name
This searches for route table with given "Name"
tag and either returns route table id or empty string
if route table doesn't exist.
These routines are used by Linux client AWS addon.
Signed-off-by: Lev Stipakov <lev@openvpn.net>
-- Instantiate the PsidCookieImpl and spot check it's correctness
-- Check for client replies within and outside of the allowable time
window
Signed-off-by: Mark Deric <jmark@openvpn.net>
Conversion fixes:
-- use the protocol op field as a byte, not an int
-- fix the valid time hmac component calculation to avoid implicit
64-to-32 narrowing.
Signed-off-by: Mark Deric <jmark@openvpn.net>
The intent of the offset argument to the calculate_session_id_hmac()
function was to allow the server to accept hmac values calculated for
the previous time interval even as the server's clock moved into a new
time interval.
The bug was that the implementation checked for a match with the
server's current time interval and its _future_ time interval, and not
it's immediate past time interval.
This bug was pointed out during code review by Frank Lichtenheld. The
code review subject was numeric conversions, in this case signed vs
unsigned. This author changed the offset to unsigned, arguing that the
code would never need to check the client hmac cookie based on both
sides of the server's current time. The check only needed to be in
one direction, hence unsigned was the solution to the conversion
warning. That was when Frank pointed out that implementation with the
"+ offset" snippet checked to see if the client provided a psid from
the future time interval and what was needed was to check for the past
interval, i.e., "- offset".
Hence, the change implements the unsigned offset, change add to
subtract from the time range computed for the server's currrent time
window.
Signed-off-by: Mark Deric <jmark@openvpn.net>
