Checking specifically for /proc/net/pfkey is not ideal as af_key will
eventually be removed in Linux kernels. Support for KLIPS is long gone.
The detection also wasn't used for anything anymore (failures were just
ignored since the ports to BSD-based systems). And modprobing doesn't seem
to be necessary either (charon-systemd doesn't do that, for instance).
Usually, changing this won't be necessary (actually, some plugins
specifically use different DRGBs for RNG_WEAK in order to separate
the public nonces from random data used for e.g. DH).
But for experts with special plugin configurations this might be
more flexible and avoids code changes.
Also expose a method to call arbitrary commands, which allows calling not
yet wrapped commands. Exceptions are raised for all commands if the response
includes a negative "success" key (similar to how it's done in the Python
bindings).
Luckily, the type is only used once when generating payloads and there it
doesn't matter because the encoding rules are the same.
Closesstrongswan/strongswan#135.
The main fixes are
* the generation of fingerprints for RSA, ECDSA, and EdDSA
* the encoding of ECDSA private keys
* calculating p and q for RSA private keys
* deriving the public key for raw Ed25519 private keys
Also, instead of numeric literals for buffer lengths ASN.1 related
constants are used.
Instead of assuming passwords are simply ASCII-encoded we now assume they are
provided UTF-8-encoded, which is quite likely nowadays. The UTF-8 byte
sequences are not validated, however, only valid code points are encoded
as UTF-16LE.
Fixes#3014.
Previously, the initiator would install the SA in transport mode if the
peer sent back the USE_TRANSPORT_MODE notify, even if that was not
requested originally.
The only messages that are generally sent encrypted but could be sent
unencrypted are INFORMATIONALs (currently only used for IKEv1 and ME
connectivity checks). This should prevent issues if the keymat_t behaves
incorrectly and does not return an aead_t when it actually should.
Might be useful for users of other daemons too. Note that compared to the
previous implementation in charon-tkm, the mask/label are applied in
network order.
Closesstrongswan/strongswan#134.
Previously, attributes in an incorrectly sent CFG_REPLY would still be passed
to attribute handlers. This does not prevent handlers from receiving
unrequested attributes if they requested at least one other.
This is mainly to see what's necessary to create them (in case we
integrate this into the daemon) and to experiment in our testing
environment without having to add a patched version of iproute2 (the
4.20.0 version in stretch-backports doesn't support XFRM interfaces
yet). The regular version of iproute2 can be used for other operations
with these interfaces (delete, up, addrs etc.).
The PB-TNC finite state machine according to section 3.2 of RFC 5793
was not correctly implemented when sending either a CRETRY or SRETRY
batch. These batches can only be sent in the "Decided" state and a
CRETRY batch can immediately carry all messages usually transported
by a CDATA batch. strongSwan currently is not able to send a SRETRY
batch since full-duplex mode for PT-TLS isn't supported yet.
The bits not written to are marked tainted by valgrind, don't print
them in the debug messages. Also use more specific printf-specifiers
for other values.
There was a race condition between install() and uninstall()
where one thread was in the process of installing a trap
entry, and had destroyed the child_sa, while the other
thread was uninstalling the same trap entry and ended up
trying to destroy the already destroyed child_sa, resulting
in a segmentation fault in the destroy_entry() function.
The uninstall() function needs to wait until all the threads
are done with the installing before proceeding to uninstall
a trap entry.
Closesstrongswan/strongswan#131.
This can be the case for IKEv1 since 419ae9a20a ("ikev1: Default remote
identity to %any for PSK lookup if not configured").
Closesstrongswan/strongswan#128.
This seems to avoid broadcast loops (i.e. processing and reinjecting the
same broadcast packet over and over again) as the packets we send via
AF_PACKET socket are neither marked nor from that interface.
In order to avoid that the kernel uses virtual tunnel IPs for traffic
over physical interfaces we previously deprecated the virtual IP. While
this is working it is not ideal. This patch adds address labels for
virtual IPs, which should force the kernel to avoid such addresses to
reach any destination unless there is an explicit route that uses it as
source address.
Using parse_time() directly actually overwrites the next member in the
child_cfg_create_t struct, which is start_action, which can cause
incorrect configs if inactivity is parsed after start_action.
Fixes#2954.
Some users requests something like that to use different server IPs.
Interestingly, it's actually also possible to configure multiple
hostnames/IPs, separated by commas, as server address in the profile, which
are then tried one after another.
It's also useful when testing stuff to quickly compare the behavior with
some setting changed between two otherwise identical profiles.
A temporary DROP policy is added to avoid traffic leak
while the SA is being updated. It is added with
manual_prio set but when the temporary policy is removed
it is removed with manual_prio parameter set to 0.
The call to del_policies_outbound does not match the original
policy and we end up with an ever increasing refcount.
If we try to manually remove the policy, it is not removed
due to the positive refcount. Then new SA requests fail with
"unable to install policy out for reqid 1618,
the same policy for reqid 1528 exists"
Fixes: 35ef1b032d ("child-sa: Install drop policies while updating IPsec SAs and policies")
Closesstrongswan/strongswan#129.
Instead, create a socket when necessary. Apparently, it can prevent
the agent from getting terminated (e.g. during system shutdown) if e.g.
charon-nm is still running with an open connection to the agent.
In 7b7290977 ("controller: Add option to force destruction of an IKE_SA")
the 'force' option was added as 3rd parameter to controller_t::terminate_ike.
However in vici's 'clear_start_action', the argument was incorrectly
placed as the 2nd parameter - constantly sending 0 (FALSE) as the
'unique_id' to terminate, rendering calls to 'handle_start_actions'
having undo=TRUE being unable to terminate the relevant conn.
For example, this is log of such a bogus 'unload-conn':
strongswan[498]: 13[CFG] vici client 96 requests: unload-conn
strongswan[498]: 13[CFG] closing IKE_SA #9
strongswan[498]: 13[IKE] unable to terminate IKE_SA: ID 0 not found
strongswan[498]: 09[CFG] vici client 96 disconnected
here, the unloaded conn's IKE id was 9, alas 'terminate_ike_execute'
reports failure to terminate "ID 0".
Fix by passing 'id, FALSE' arguments in the correct order.
Fixes: 7b7290977 ("controller: Add option to force destruction of an IKE_SA")
Signed-off-by: Shmulik Ladkani <shmulik@metanetworks.com>
Closesstrongswan/strongswan#127.
Up to now it was assumed that the RSA public key exponent is equal to 2^16+1.
Although this is probably true in most if not all cases, it is not correct
according to the TPM 2.0 specification.
This patch fixes that by reading the exponent from the structure returned
by TPM2_ReadPublic.
Closesstrongswan/strongswan#121.
All directories are now considered relative to the loaded swanctl.conf
file, in particular, when loading it from a custom location via --file
argument. The base directory, which is used if no custom location for
swanctl.conf is specified, is now also configurable at runtime via
SWANCTL_DIR environment variable.
Closesstrongswan/strongswan#120.
The functional reference created by ENGINE_init() was never released,
only the structural one created by ENGINE_by_id(). The functional
reference includes an implicit structural reference, which is also
released by ENGINE_finish().
Closesstrongswan/strongswan#119.
This is particularly important for higher number of segments, but even
with small numbers there is a significant difference. For instance,
with 4 segments the fourth segment had no IPs assigned with the old
code, no matter how large the pool, because none of the eight bits used
for the segment check hashed/mapped to it.
Upcoming versions of FreeBSD will include an SADB_X_EXT_SA2 extension in
acquires that contains the reqid set on the matching policy. This allows
handling acquires even when no policies are installed (e.g. to work with
FreeBSD's implementation of VTI interfaces, which manage policies
themselves).
Same issue with signature malleability as with Ed25519 and apparently
OpenSSL doesn't even explicitly verify that the most significant 10 bits
are all zero.
As per RFC 8032, section 5.1.7 (and section 8.4) we have to make sure s, which
is the scalar in the second half of the signature value, is smaller than L.
Without that check, L can be added to most signatures at least once to create
another valid signature for the same public key and message.
This could be problematic if, for instance, a blacklist is based on hashes
of certificates. A new certificate could be created with a different
signature (without knowing the signature key) by simply adding L to s.
Currently, both OpenSSL 1.1.1 and Botan 2.8.0 are vulnerable to this, which is
why the unit test currently only warns about it.
OpenSSL 1.1.1 introduces DRGBs and provides two sources (same security
profile etc. but separate internal state), which allows us to use one for
RNG_WEAK (e.g. for nonces that are directly publicly visible) and the other
for stronger random data like keys.
While X25519 was already added with 1.1.0a, its use would be a lot more
complicated, as the helpers like EVP_PKEY_new_raw_public_key() were only
added in 1.1.1, which also added X448.
In case a subnet is moved from one interface to another the policies can
remain as is but the route has to change. This currently doesn't happen
automatically and there is no option to update the policy or route so
removing and reinstalling the policies is the only option.
Fixes#2820.
The peer might not have seen the CREATE_CHILD_SA response yet, receiving a
DELETE for the SA could then trigger it to abort the rekeying, causing
the deletion of the newly established SA (it can't know whether the
DELETE was sent due to an expire or because the user manually deleted
it). We just treat this SA as if we received a DELETE for it. This is
not an ideal situation anyway, as it causes some traffic to get dropped,
so it should usually be avoided by setting appropriate soft and hard limits.
References #2815.
Because the file is not available on all platforms the inclusion comes
after the user options in order to disable including it. But that means
the inclusion also follows after the defined scanner states, which are
generated as simple #defines to numbers. If the included unistd.h e.g.
uses variables in function definitions with the same names this could
result in compilation errors.
Interactive mode has to be disabled too as it relies on isatty() from
unistd.h. Since we don't use the scanners interactively, this is not a
problem and might even make the scanners a bit faster.
Fixes#2806.
According to gcrypt.h these callbacks are not used anymore since
version 1.6 and with clang these actually cause deprecation warnings
that let the build on travis (-Werror) fail.
This is because OpenSSL 1.1 started to use atexit()-handlers of its own
to clean up. Since the plugin is loaded and initialized after libcharon,
OpenSSL's cleanup functions ran before the daemon was properly
deinitialized (i.e. worker threads were still running and OpenSSL might
still be used during the deinit). So several of OpenSSL's internal
structures were already destroyed when libcharon_deinit() was eventually
called via our own atexit()-handler.
The observed behavior was that the daemon couldn't be terminated properly
anymore for some test scenarios (only three TNC scenarios were affected
actually). When the daemon tried to send the DELETE for the established
IKE_SA during its termination it got stuck in OpenSSL's RNG_WEAK
implementation (used to allocate random padding), which apparently tries
to acquire an rwlock that was already destroyed. The main thread then
just busy-waited indefinitely on the lock, i.e. until systemd killed
it eventually after a rather long timeout.
We'll probably have to apply similar changes to other apps/scripts that
load plugins and currently use atexit() to clean up. Although some
scripts (e.g. dh_speed or hash_burn) are not affected because they
register the deinitialization after loading the plugins.
If a lot of QUICK_MODE tasks are queued and the other side
sends a DPD request, there is a good chance for timeouts.
Observed this in cases where other side is quite slow in responding
QUICK_MODE requests (e.g. Cisco ASA v8.x) and about 100 CHILD_SAs
are to be spawned.
Closesstrongswan/strongswan#115.
Checking for whitelisted functions in every backtrace is not very
efficient. And because OpenSSL 1.1 does no proper cleanup anymore until
the process is terminated there are now a lot more "leaks" to ignore.
For instance, in the openssl-ikev2/rw-cert scenario, just starting and
stopping the daemon (test vectors are checked) now causes 3594 whitelisted
leaks compared to the 849 before. This prolonged the shutdown of the
daemon on each guest in every scenario, amounting to multiple seconds of
additional runtime for every affected scenario. But even with this
patch there is still some overhead, compared to running the scenarios on
jessie.
SHA-384 is not supported but is selected by signature_schemes_for_key()
for keys between 3072 and 7680 bits.
Since this is only called for IKEv2 signature authentication we don't
even provide SHA-1 anymore. We always provide both schemes, though,
which is what pubkey-authenticator does too for RSA.
Older agents apparently just ignore the flags and always return a SHA-1
signature. If that's the case, charon.signature_authentication has to
be disabled.
On newer Android versions (8+) this does not seem to be necessary (adding
the onClick handler also sets "clickable" and that in turn seems to make
it focusable), however, for older releases it is (tested with 7.1.1
keyboard navigation just skips over the button). This was seen on a
Fire TV stick.
It looks like Android 9 incorrectly continues to use the regular DNS
servers after the blocking TUN device is replaced with the actual
interface. Setting DNS servers prevents that (since all traffic is
blocked, which ones doesn't really matter but local/loopback addresses
are rejected).
Interestingly, if the VPN server later does not assign any DNS servers, there
is no fallback to the non-VPN DNS servers for some reason (that's definitely
not as documented). This could potentially be a problem as we don't
offer an option to configure DNS servers in the VPN profile.
Neither issue is seen on older Android versions (only tested on 7.1.1).
Not sure when this happens exactly, in particular because the reported
stack traces look like this
java.lang.NullPointerException:
at org.strongswan.android.ui.VpnTileService.updateTile (VpnTileService.java:220)
at org.strongswan.android.ui.VpnTileService.onStartListening (VpnTileService.java:97)
at android.service.quicksettings.TileService$H.handleMessage (TileService.java:407)
which violates the API documentation for getQsTile(), which states:
This tile is only valid for updates between onStartListening() and
onStopListening().
But apparently that's not always the case. There have been two reports
of such a crash, both on Android 8.0 and on Xiaomi Mi 5/6 devices, so
maybe it's a bug in that particular image.
The previous code lost track of the selected profile IDs, but the
widgets maintained their state (i.e. the list item was still selected and the
edit button still enabled). Clicking the edit button then caused a crash when
trying to get the first item in the set.
The task manager for IKEv1 issues a retransmit send alert in the
retransmit_packet() function. The corresponding retransmit cleared alert
however is only issued for exchanges we initiated after processing the
response in process_response().
For quick mode exchanges we may retransmit the second packet if the peer
(the initiator) does not send the third message in a timely manner. In
this case the retransmit send alert may never be cleared.
With this patch the retransmit cleared alert is issued for packets that
were retransmitted also when we are the responding party when we receive
the outstanding response.
Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
Because `keylen` is unsigned the subtraction results in an integer
underflow if the key length is < 11 bytes.
This is only a problem when verifying signatures with a public key (for
private keys the plugin enforces a minimum modulus length) and to do so
we usually only use trusted keys. However, the x509 plugin actually
calls issued_by() on a parsed certificate to check if it is self-signed,
which is the reason this issue was found by OSS-Fuzz in the first place.
So, unfortunately, this can be triggered by sending an invalid client
cert to a peer.
Fixes: 5955db5b12 ("gmp: Don't parse PKCS1 v1.5 RSA signatures to verify them")
Fixes: CVE-2018-17540
Instead we generate the expected signature encoding and compare it to the
decrypted value.
Due to the lenient nature of the previous parsing code (minimum padding
length was not enforced, the algorithmIdentifier/OID parser accepts arbitrary
data after OIDs and in the parameters field etc.) it was susceptible to
Daniel Bleichenbacher's low-exponent attack (from 2006!), which allowed
forging signatures for keys that use low public exponents (i.e. e=3).
Since the public exponent is usually set to 0x10001 (65537) since quite a
while, the flaws in the previous code should not have had that much of a
practical impact in recent years.
Fixes: CVE-2018-16151, CVE-2018-16152
This also changes how unknown/corrupted memory is handled in the free()
and realloc() hooks in general.
Incorporates changes provided by Thomas Egerer who ran into a similar
issue.
Due to the mangled C++ function names it's tricky to be more specific. The
"leaked" allocations are from a static hashtable containing EC groups.
There is another leak caused by the locking allocator singleton
(triggered by the first function that uses it, usually initialization of
a cipher, but could be a hasher in other test runners), but we can avoid
that with a Botan config option.
Simplifies public key loading and this way unencrypted PKCS#8-encoded
keys can be loaded directly without pkcs8 plugin (code for encrypted
keys could probably later be added, if necessary).
It also simplifies the implementation of private_key_t::get_public_key()
a lot.
Without OID we can't generate an algorithmIdentifier when loading the
key again. And older versions of OpenSSL insist on a public key when
e.g. converting a key to PKCS#8.
Simply unwrapping the ECPrivateKey structure avoids log messages when
parsing other keys in the KEY_ANY case.
Support MD5 in the Botan plugin if supported by Botan.
MD5 is required for RADIUS and obviously EAP-MD5,
and also for non-PKCS#8 encoded, encrypted private keys.
Botan only allows RSA generating keys >= 1,024 bits, which makes
the RSA test suite fail. It is questionable whether it makes
sense to test 768 bit RSA keys anymore. They are too weak
from today's perspective anyway.
This requires config changes if filelog is used with a path that
contains dots. This path must now be defined in the `path` setting of an
arbitrarily named subsection of `filelog`. Without that change the
whole strongswan.conf file will fail to load, which some users might
not notice immediately.
Tobias Brunner