This is documented as a requirement in the man page, so the caller of the
conversation function might expect such an array. Although, since we
only accept non-interactive requests, for which no response is expected,
it's doubtful whether it will actually try to access elements in the
array. And if the passed pointer was initialized to NULL, passing it to
free() is probably also fine.
Fixes: 2312504d1e ("xauth-pam: Open/close a PAM session for each connected client")
Even if `poll()` indicates that the socket is ready it might block if
it's in blocking mode. This change avoids blocking in such cases (accept
will fail with EAGAIN/EWOULDBLOCK and `watch()` will return TRUE).
As the non-blocking mode is inherited on Windows (on Linux, the man page
documents the non-inheritance as a Linux specialty), we set the mode for
the accepted socket explicitly to blocking to match the expectations of
`stream_t`.
Fixes: daf1880b39 ("stream: add a stream service class abstracting services using BSD sockets")
Only skip the header, the `ip6e_len` field is reserved in these headers
and does not represent the length (I guess it's usually 0, but still).
Fixes: bdcaca76c5 ("ip-packet: Correctly determine protocol in fragmented IPv6 packets")
The referenced commit moved the key derivation to `get_shared_secret()`
and broke the handling of ECDH public value as the copied struct now
referred to a buffer allocated on the stack.
Also fixes potential session leaks if generating key pairs fails.
Fixes: 26ca0c9f70 ("pkcs11: Move shared secret calculation to get_shared_secret()")
The shift would overflow the value which could produce garbage output
that might get interpreted as real OIDs (in case strings are compared).
This limit allows OID sub-identifiers to consist of at most 4 bytes,
which should be enough for any real-world OIDs (it's also the maximum we
used in tests so far).
Fixes: f813069e89 ("fixed asn1_oid_to_string() conversion")
Seems difficult to exploit when wrapped in EAP/IKEv2, and the keys for
those TLS sessions are short-lived anyway.
Fixes: c0bf721357 ("tls: Check all bytes of the padding if they equal the padding length")
While we could use _Generic() C11 expression to let the compiler select
between the different versions, this only allows selection based on one
of the arguments, which seems a bit fragile. So make this explicit for
now. In the future we might consider using the overloadable attribute.
The presence of a peer ID is enforced by the message rules for ME_CONNECT,
this just mirrors the safety check also present in `process_r_ms()`.
Fixes: d5cc175833 ("experimental P2P-NAT-T for IKEv2 merged back from branch")
Fixes: 7fbe2e27ec ("tls-server: TLS 1.3 support for TLS server implementation")
Fixes: d2fc9b0961 ("tls-server: Mutual authentication support for TLS 1.3")
Fixes: 760f3b730f ("tls-server: Add flag that makes client authentication optional")
These were never correct. And the first referenced commit, unfortunately,
just simplified the incorrect code.
Fixes: 08a3ee0cce ("bus: Change ike_update() signature and only call it once")
Fixes: b8973b2661 ("connmark: Add CONNMARK rules to select correct output SA based on conntrack")
If only parts of the total data could be written to the kernel, the result
of the next read chunk would incorrectly get written at the beginning of
the output buffer again.
Also makes sure to close the accepted FD in error cases.
Fixes: 1b5de7ce3b ("Use a generic AF_ALG wrapper for common operations")
The `transaction` variable was previously still set to the one that
was enumerated last even if the received DHCP OFFER did not match
it. So the transaction was incorrectly modified. Once the real
DHCP OFFER would arrive, it would get modified again, but it could
allow adding unintended DNS servers.
The previous code could potentially truncate and change ownership of
a file that's a symlink to an unintended target file (requires the
attacker to be able to create the symlink in the directory the PID file
is located, which generally requires root privileges).
When initiating a Child SA with a CREATE_CHILD_SA exchange, the initial
exchange could fail with an INVALID_KE_PAYLOAD notify. The initiation
would then get retried from scratch (the `child_sa_t` object is destroyed
and a new one created). If it fails again, the previous code didn't
signal the waiting thread. If it did not set a timeout, it would wait
indefinitely.
Using a different `status_t` value avoids any confusion with NEED_MORE
that's used to indicate that the callback returned FALSE (e.g. if the
IKE_SA would go down before concluding the retry).
Fixes: c39b0c883d ("controller: Keep following initiate() if the first DH guess was wrong")
Replace `set()` with `extend()` in verify methods so the PCR value is
independently computed rather than taken from the evidence.
Also make `pcr_before` mismatch a hard failure instead of a warning.
This is not necessarily an issue, but we should avoid not using the
full identity data as best as possible. The change also avoids the
dynamically sized buffer on the stack.
Fixes: 324528700d ("Added identification constructor using a chunk of data, guessing id type")
If we find a stale CRL in the cache and finding a newer one via
CRLIssuer fails for some reason, the validation state would get
overwritten with VALIDATION_SKIPPED. This would then prevent
fetching delta CRLs.
Fixes: 7d7beaa1fa ("Use certificate CRLIssuer information to look up cacched CRLs or CDPs")
EAP-PEAP transports results as separate AVPs, we expect one after the
first phase 2 authentication (if not using certificates). However, with
the previous caching of the result of that, the client could then send
another confirmation as response to the EAP-TNC request to skip it.
Since no group membership is set if EAP-TNC is not complete, it shouldn't
really affect its outcome, though.
Fixes: 1be296dfb2 ("implemented the PEAP tunneling protocol as an EAP plugin")
Holding the lock could potentially cause a deadlock depending the
behavior of the called cleanup functions. The TLS removal happens in
the context of the respective thread, so no locking is necessary.
Looks like removing these lines was missed when the referenced commit
partly reverted 204098a752 ("thread-value: Immediately cleanup all
Windows TLS values on destroy"), which added the locking originally.
Fixes: 23750961d5 ("thread-value: Defer cleanup handling to thread termination on Windows")
The entry was updated and the previous EAP method and IKE SA destroyed
under a read lock, which is not safe (there could be threads in
get_state(), or even some that called it before and now still hold
that read lock while interacting with the EAP method).
Fixes: 37884ab10f ("Add locking to TNC-PDP connections")
Because no lock was held while comparing the type of the already stored
cache entry, it was theoretically possible that a NULL-pointer dereference
or use-after-free was caused if another thread concurrently replaced the
stored certificate.
While the cached type is also read without lock, the worst that can
happen is that we acquire the lock of an entry that doesn't match
eventually. The double check actually uses the stored certificate,
just in case the cached value gets out of sync due to a future
refactoring.
Fixes: 2271ebb325 ("Newer CRLs replace older versions of the CRL in the cache")
Fixes: 47e8b21c76 ("cert_cache: Replace cached stale OCSP responses in-place")
We only allow unencrypted change_cipher_spec records (as before), which
are handled by the upper layers accordingly. Without the check, we
also accepted unprotected alerts and handshake records that could
potentially cause state confusion.
Fixes: 7a2b02667c ("libtls: Implement TLS 1.3 handshake on client-side")
As `tv_sec` is a `time_t`, i.e. typically 64 bits, assigning the result
of `htonl()` leaves the upper 32 bits zero. Copying from the
`sizeof(time_t) - 4` offset then copies those zeroes on little-endian
systems, which is not what was intended according to the comments.
Another issue was that the type of `tv_usec`, `suseconds_t`, is typically
a `long`, i.e. signed, so shifting the maximum value 0x000f423f (999'999)
by 12 bits technically overflows this. The cast fixes that.
Fixes: 1aba82bfd7 ("eap-aka-3gpp: Add plugin that implements 3GPP MILENAGE algorithm in software")
Basically the same as the previous commit.
Fixes: edcb2dd35b ("Moved reauth/pseudonym functionality from eap-sim-file to separate plugins, usable by any SIM/AKA backend")
This also protects access to the RNG, which is not always thread-safe.
Fixes: edcb2dd35b ("Moved reauth/pseudonym functionality from eap-sim-file to separate plugins, usable by any SIM/AKA backend")
Due to the `sleep()` call, a continuous stream of failed authentication
attempts can potentially exhaust the thread pool. While delays can also
happen due to RADIUS or DHCP on the server side, this can directly be
triggered by clients. It's questionable whether the delay ever had the
intended effect. But since the retry functionality is also quite
strange, let's just remove it so retries will require creating an IKE_SA
from scratch. To avoid leaking much of a timing difference if no secret
is found now that the two second delay is gone, we do the calculations
with a dummy NT hash.
Fixes: f98cdf7a47 ("adding plugin for EAP-MS-CHAPv2")
Because `assess()` returns a `status_t`, `FALSE` is interpreted as
`SUCCESS`. So a failure while processing PB-TNC batches terminated
the process successfully.
Fixes: 1e92d5f114 ("Process PB-TNC batches received via PT-TLS asynchronously")
Tobias Brunner