Since 5.9.6, the number of half-open SAs includes queued initial IKE
messages. So that number could be higher than the number of managed
SAs, which causes an underflow in the output.
Fixes: b866ee88bf ("ike: Track unprocessed initial IKE messages like half-open IKE_SAs")
This was primarily used in our labs to visualize some TNC aspects but
the third-party daemon and frontend we used have not seen any development
in a decade. There never was any industry interest in this protocol
anyway, so just remove it.
This plugin was developed for a customer who had very specific
requirements. It never did anything useful for regular users and
usually caused confusing errors if they enabled it by mistake. So
just remove it.
This was from a student project that has never been developed further.
And similar to the manager web application it lacks all sorts of modern
standards. So just remove it and the two plugins it relied on.
The test scenario is renamed to avoid confusion (neither of the two
p2pnat scenarios uses medsrv/medcli).
This fixes a potential TOCTOU issue with opening log files. The use of
`chown()` instead of `fchown()` could theoretically allow modifying the
ownership of an unintended file.
The log file now also is not world-readable anymore.
Also, the patch fixes the log groups for the two `(f)chown()` errors.
Fixes: d35d669180 ("Make syslog and file loggers configurable at runtime")
While for most uses the length is fixed and public (e.g. PRF/MAC outputs),
there are a few (e.g. in xauth-generic) that compare variable length
data.
The previous code directly leaked a differing length by short-circuiting
before comparing anything. While we could limit the comparison by the
minimum length (and call `memeq_const()`), that could still leak the
length because the time will plateau once the secret's length is reached.
Similarly, if the comparison was bound by the longer chunk (would prevent
the use of `memeq_const()`), the length could also be revealed once the
input gets longer than the secret and the time increases.
This changes the semantics of the function by declaring the first
argument the expected/reference secret and the second the variable input.
This strictly makes the function constant-time, bound by the secret's
length. So the length can't be guessed by providing different input (but
if an attacker can trigger the comparison against different secrets, of
potentially known lengths, it might still be possible). If the chunks
are known to have the same length, the order doesn't matter.
Callers of this function have been updated accordingly.
The check only references the original chunk, so for each parsed payload
it checks the same thing. The length of each individual payload is
checked by the parser anyway. So I think this was primarily added for
the IKEv1 "wrong PSK" use case. Let's keep it for now.
Fixes: dd5c3787dc ("Give a hint that decryption failed if payload length invalid")
Should be rare that the eap-identity plugin is not loaded when
authenticating clients with EAP. And the second error path will
currently never get used as `process()` always succeeds.
Fixes: 79f2102cb4 ("implemented server side support for EAP-TTLS")
If multiple clients call list commands concurrently, each would replace
the global certificate printer instance the previous client created
and then operate on shared state. The destruction then causes a
double-free or NULL-pointer dereference.
Fixes: 02d431022c ("Refactored certificate management for the vici and stroke interfaces")
This would cause a use-after-free because `lookup_iv()` inserts, removes
and destroys the still returned entry.
Fixes: aeaab528e8 ("ikev1: Factor out IV and QM management")
In the (very) unlikely case that a unique ID is reused (e.g. due to
counter wraparound) while the original SA is still in this manager,
the entries should properly get removed from the other lookup tables
before the entry is destroyed to prevent stale pointers from getting
used in later lookups.
Fixes: e732fb11a9 ("child-sa-manager: Add a global manager storing CHILD_SA relations")
This could prevent the VIP from getting installed later and actually
causes those threads to block indefinitely as they wait for the entry to
either get removed or the VIP marked as installed, which will never
happen.
Fixes: c6b401581a ("Changed how kernel-netlink handles virtual IP addresses")
This applies some of the same fixes found in the previous commit but also
ensures that the offsets are valid before accessing the bitmask. Because
of an off-by-one error in the latter, the last address could get released
incorrectly (the pool constructor explicitly excludes it).
Fixes: 98d0343870 ("Implemented a HA enabled in-memory address pool")
Due to the overflow when mapping addresses to offsets, no addresses could
get released when the pool was defined in a way that the last address
assignable is 255.255.255.255 (probably never the case in practice).
While not really useful in practice, the range 0.0.0.0-255.255.255.255
resulted in an empty pool (`size = 0xffffffff + 1`), which triggers the
%config like behavior. Since the implementation currently uses signed
integers throughout, we make sure the largest assignable offset is
INT_MAX.
The two off-by-one fixes (`>= pool->size`) were never an issue in practice
because `get_new()` has a guard that caps the offset at the size and
released addresses originally came from the pool so are always in range.
Fixes: 0897cda33b ("Add a constructor to create in-memory pools from an address range")
If the installation fails while a shunt is concurrently uninstalled,
the entry could already be destroyed when trying to remove and destroy
it after acquiring the lock again in `install()`.
This change handles the conflict the same way trap-manager does since
69cbe2ca3f ("trap-manager: Wait for install to finish before
uninstalling").
Fixes: 616ff9a236 ("shunt-manager: Remove stored entries if installation fails")
The return value is now also fixed. The reply allocation previously
prevented that sections with failed CA certs were rejected.
Also fixes two potential leaks if duplicate settings are sent by a VICI
client.
Fixes: 63d370387d ("vici: Certification Authority support added.")
These rules don't depend on traffic selectors, so for SAs with multiple
traffic selectors we'd install several duplicate rules. Since this
plugin is used for transport mode SAs, it probably never was an issue
in practice.
Fixes: b8973b2661 ("connmark: Add CONNMARK rules to select correct output SA based on conntrack")
On the responder, dc8b479eb8 ("ike-init: Fix key derivation if SA is
reset after IKE_INTERMEDIATE retransmits") fixed this. But the initiator
still accepted IKE_INTERMEDIATE exchanges without KE payload.
Fixes: 0d49ddec2e ("ike-init: Add support for multiple key exchanges")
This ensures that secrets that are associated with the server identity,
usually they are assigned to the username exclusively, will only be tried
if the username also matched. Otherwise, it could be possible for users
who know at least one password to impersonate another user.
Fixes: 462c9a4f72 ("Try all matching XAuth secrets we find, not only the first one")
The code was written with `child_sa_t::destroy` in mind, which deletes
the inbound SA before the outbound SA. The problem is that the rekeying
code was changed meanwhile so the outbound SA is removed before the
inbound SA in order to avoid traffic loss. That could cause a
use-after-free as the already destroyed item remained in the `isas` list.
This change fixes this so the SAs can be removed in any order. The SPIs
are used as marker for whether a specific direction is installed. It
also fixes an issue in `expire_job()`, which removed the entry from
`osas` without holding the lock.
Fixes: f351d9ef7d ("kernel-wfp: Reference SA/SP sets by SPI and destination, not reqid")
Fixes: 44107cb7b7 ("child-delete: Delay the removal of the inbound SA of rekeyed CHILD_SAs")
This prevents other processes from binding the same ports and
misusing the protocol/port-specific bypass rules installed in WFP to
bypass the VPN.
Fixes: 11e7d0677c ("socket-win: Install IKE bypass policies using bypass_socket()")
The previous code checked in the IKE_SA before using it again to
add attributes based on it to the interim update message. This
change ensures the IKE_SA is only accessed while it is checked out.
The Acct-Status-Type attribute is still sent first, but the
Acct-Session-Id and Class attributes are now sent after the IKE
parameters, which shouldn't really matter, though.
Fixes: d019764ab6 ("Add support for RADIUS Interim accounting updates")
On typical systems that have overcommitting enabled, the 16 MiB maximum
that's allocated via the 24-bit length field in the AVP header shouldn't
be an issue as there are various limits that affect how much data can
actually be written to the allocated buffer (e.g. the maximum IKE message
size, the maximum TLS record size, or the maximum number of accepted
EAP-TTLS payloads), so this is primarily a defense-in-depth measure.
The length field of an EAP payload, which is the only type of AVP we
accept, is 16 bits, so that's what we now enforce as maximum.
This is unlikely to be an issue in practice because only one caller
actually uses the ID and it does so immediately afterwards. So there is
only a tiny window in which the peer could terminate or rekey its SA to
cause the returned ID to get destroyed.
Fixes: d5cc175833 ("experimental P2P-NAT-T for IKEv2 merged back from branch")
Same as the previous commit for consistency, but not an issue here as
only transport mode SAs are handled.
Fixes: b8973b2661 ("connmark: Add CONNMARK rules to select correct output SA based on conntrack")
Not an issue in typical scenarios where the plugin is used on a gateway
with roadwarriors that use /32 addresses. But could be an issue if used
on a client that tunnels everything to the gateway.
Fixes: e5ad2e6614 ("forecast: Add the broadcast/multicast forwarding plugin called forecast")
There are multiple paths that can trigger a crash. One is if transport
mode is negotiated via NAT and `get_transport_nat_ts()` is called to
substitute addresses in the received TS. If that's not the case,
`narrow_ts()` will select the configured TS and continue. Then,
`narrow_and_check_ts()` eventually attempts to destroy `this->tsi/tsr`,
which triggers a crash in any case.
The referenced commit refactored `select_and_install()` into
`narrow_and_check_ts()` and `install_child_sa()`. It move a check
for TS from that function to only `build_r()`.
Fixes: d7760416d6 ("child-create: Add support for multiple key exchanges")
The policies reference the provider, so it might not actually get removed
and be left dangling in the WFP system (maybe Windows refcounts it and
still removes it).
Such endpoints are used to request a peer reflexive endpoint, but are
never expected in a response.
Fixes: d5cc175833 ("experimental P2P-NAT-T for IKEv2 merged back from branch")
Tobias Brunner