ECDH_compute_key() was not used because it only gives x-coordinate of
the result. However, the default setting, as per the errata mentioned,
is to use x-coordinate only.
Use ECDH_compute_key() for this setting as it additionally allows HW
offload of the computation using dynamic engine feature in OpenSSL.
EC_POINT_mul() doesn't allow HW offload.
Signed-off-by: Mahantesh Salimath <mahantesh@nvidia.com>
The previous approach would lead to additional zero prefixes in the
encoding of the serial (which is a positive integer, not an arbitrary
blob).
Fixes#3667.
To align with RFC 4519, section 2.31/32, the abbreviation for surname
is changed to "SN" that was previously used for serialNumber, which does
not have an abbreviation.
This mapping had its origins in the X.509 patch for FreeS/WAN that was
started in 2000. It was aligned with how OpenSSL did this in earlier
versions. However, there it was changed already in March 2002 (commit
ffbe98b7630d604263cfb1118c67ca2617a8e222) to make it compatible with
RFC 2256 (predecessor of RFC 4519).
Co-authored-by: Tobias Brunner <tobias@strongswan.org>
Closesstrongswan/strongswan#179.
Also assign online leases to a peer connecting from the same endpoint
when it requests any virtual IP. This is mainly a workaround for
Windows clients that remember the virtual IPv6 address and re-request it
the next time the connection is initiated (even if it is not a
reauthentication) but don't do the same for virtual IPv4 addresses.
This can result in duplicate policies with different reqids because
these are allocated for unique sets of traffic selectors.
Fixes#3541.
This allows more fine grained control over what's updated and does not
require multiple calls of the method. Plus we'll be able to use it in
the ike-mobike task.
Verify certificate chains starting from the root CA certificate and
moving towards the leaf/user certificate.
Also update TKM-RPC and TKM in testing scripts to version supporting the
reworked CC handling.
Load CA certificate id mapping from config and pass the correct CA ID to
TKM when checking certificate chains. The mapping of CA certificate to
CA ID is done via SHA-1 hash of the CA certificates subjectPublicKey.
Debian, Ubuntu, Fedora et. al. started to apply usrmerge to their
latest Linux distributions, i.e. /bin, /sbin, and /lib are now
symbolical links to /usr/bin, /usr/sbin, and /usr/lib, respectively.
Since executables and libraries are contained only once in Linux
packages (e.g. /bin/cp in coreutils but not /usr/bin/cp) this leads
to missing file measurments due to the symlinks when doing remote
attestation.
The new ita_attr_symlinks PA-TNC attribute fixes this problem by
collecting symbolic links pointing to directories on the client
platform.
Depending on from where bison is called, the file might not end up in
the same directory as the .y file, but the location of the Makefile.
This has been seen on FreeBSD.
There is a conflict between Flex's bison-bridge and Bison's api.prefix
options. Apparently, the former was added without consulting the Bison
devs and requires YYSTYPE, which is not added to the header anymore by
the latter. Instead, we just provide the proper definition of yyflex()
manually (as recommended by the Bison docs), so the option is not
required anymore.
DESTROY_IF() checks if the given value is not NULL, before calling
destroy() on it, which does not work for sub-structs. If
port_filter_attr is NULL, this could crash.
If we are already deleting the old/redundant CHILD_SA, we must not
migrate the child-create task as that would destroy the new CHILD_SA we
already moved to the IKE_SA.
Fixes#3644.
Without threads handling the resolution, there is no point waiting
for a reply. If no subsequent resolution successfully starts a
thread (there might not even be one), we'd wait indefinitely.
Fixes#3634.
The DN is otherwise not parsed until compared/printed. This avoids
false detections as ASN.1 DN if e.g. an email address starts with "0",
which is 0x30 = ASN.1 sequence tag, and the next character denotes
the exact length of the rest of the string (see the unit tests for an
example).
For apps targeting Android 10, where a method to change this was added, the
default changed so that all VPN connections are marked as metered. This means
certain background operations (e.g. syncing data) are not performed anymore
even when connected to a WiFi. By setting this to false, the metered state
of the VPN connection reflects that of the underlying networks.
None of our build environments seem to require these declarations. And
current versions of MinGW-w64 define them as inline functions in stdio.h
so these declarations clashed with that ("static declaration of '...'
follows non-static declaration").
If no callback is specified, terminate_ike_execute() is invoked without the
listener waiting on the IKE state change.
Now, if 'force' is false, then ike_sa->delete() just queues an
IKE_DELETE task, and returns SUCCESS - indicating successful task
manager initiation.
However, terminate_ike_execute() ignored this success and set the
status to FAILED.
This is not ideal, as it will be the overall return code of
terminate_ike(), although no failure did occur. This eventually leads
vici's "terminate" to return "Command failed: terminating SA failed",
as seen in this example:
In [9]: list(session.terminate({'ike-id': 2960, 'timeout': -1}))
---------------------------------------------------------------------------
CommandException Traceback (most recent call last)
<ipython-input-9-5f95b5cea88f> in <module>()
----> 1 list(session.terminate({'ike-id': 2960, 'timeout': -1}))
vici/session.pyc in streamed_request(self, command, event_stream_type, message)
136 raise CommandException(
137 "Command failed: {errmsg}".format(
--> 138 errmsg=command_response["errmsg"]
139 )
140 )
CommandException: Command failed: terminating SA failed
If we consider both queueing the task and actually destroying the IKS_SA
a success, we can just always return SUCCESS if we don't have a
callback. There is also no need to explicitly set the status to FAILED
if a listener is waiting as that's the default anyway.
Co-authored-by: Tobias Brunner <tobias@strongswan.org>
Closesstrongswan/strongswan#185.
In FIPS mode, libgcrypt uses a DRBG, which behaves differently when the
length passed to gcry_create_nonce() or gcry_randomize() is <= 0. It
expects a struct and explicitly checks that the passed pointer is not
NULL.
This ensures that e.g. ike/child-updown messages are sent that were
queued but couldn't be sent (even the job to enable to on_write() callback
requires a worker thread that's not around anymore during shutdown).
References #3602.
The message_t object used for defragmentation was only cleared after
all fragments have been received and the message was delivered. So
if we received only some fragments of a retransmitted message, the
fragments of the next message were not processed (message_t returns
INVALID_ARG if the message ID does not match causing the message to
get ignored). This rendered the IKE_SA unusable as the client
obviously never retransmitted the fragments of that previous message
after it received our response.
The parser is quite picky and e.g. doesn't accept UUIDs without dashes.
Even without a specific error, this at least points the users into the
right direction.
Fixes#3583.
If the activity is not active when the service connection is
established and handleIntent() is called, the activity's state is already
saved and any fragment transaction would result in an illegalStateException
due to state loss. We just ignore this and wait for another initiation
attempt (via onNewIntent()).
With the flag set, we basically ignore the resent intent, which is not
ideal if we have not yet actually started another activity. The information
dialog we show first would disappear when closing and reopening the app
or even just rotating it (we hide all dialogs when receiving an intent),
but since the flag was restored, the dialog was not shown again even
when attempting to start other connections.
Note that manually adding an IPv6 address without disabling duplicate
address detection (DAD, e.g. via `nodad` when using iproute2) will cause
a roam event due to a flag change after about 1-2 seconds (TENTATIVE is
removed). If this is a problem, we might have to ignore addresses with
TENTATIVE flag when we receive a RTM_NEWADDR message until that flag is
eventually removed.
Fixes#3511.
We create the child_sa_t object when initiating the CREATE_CHILD_SA
request, however, the IP addresses/ports might have changed once we
eventually receive the response (potentially to a retransmit sent to
a different address). So update them before installing the SA and
policies.
If the local address changed too and depending on the kernel
implementation, the temporary SA created to allocate the inbound SPI
might remain as it can't be updated. This could cause issues if e.g.
the address switches back before that SA expired (the updated inbound
SA conflicts with the temporary one), or if that happens close together
and the expire (having to wait for the address update) causes the
updated SA to get deleted.
Fixes#3164.
The previous code required explicit support for a particular key type,
of which Ed25519 and Ed448 were missing. While a fallback to `any` would
have been possible (this is already the case for unencrypted keys in the
`private` and `pkcs8` directories, which are not parsed by swanctl), it's
not necessary (as long as swanctl and the daemon are from the same release)
and does not require the daemon to detect the key type again.
Fixes#3586.
Commit 27756b081c (revocation: Check that nonce in OCSP response matches)
introduced strict nonce validation to prevent replay attacks with OCSP
responses having a longer lifetime. However, many commercial CAs (such as
Digicert) do not support nonces in responses, as they reuse once-issued OCSP
responses for the OCSP lifetime. This can be problematic for replay attack
scenarios, but is nothing we can fix at our end.
With the mentioned commit, such OCSP responses get completely unusable,
requiring the fallback to CRL based revocation. CRLs don't provide any
replay protection either, so there is nothing gained security-wise, but may
require a download of several megabytes CRL data.
To make use of replay protection where available, but fix OCSP verification
where it is not, do nonce verification only if the response actually contains
a nonce. To be safe against replay attacks, one has to fix the OCSP responder
or use a different CA, but this is not something we can enforce.
Fixes#3557.
The x509 plugin accepted CRL signers since forever, to be precise, since
dffb176f2b ("CRLSign keyUsage or CA basicConstraint are sufficient
for CRL validation")).
References #3529.
A new global option enables sending this vendor ID to prevent Cisco
devices from narrowing the initiator's local traffic selector to the
requested virtual IP, so e.g. 0.0.0.0/0 can be used instead.
This has been tested with a "tunnel mode ipsec ipv4" Cisco template but
should also work for GRE encapsulation.
Closesstrongswan/strongswan#180.
While `pos` was moved to the end, `len` was not adjusted (i.e. set to 0)
so later calls could write beyond the buffer. However, the last port
written might have been incomplete, so instead we just reset the string.
If it takes a while to start one of the threads, another thread might already
have passed the usleep() call previously used and re-enabled cancelability
so that the loop that checked for it would never terminate.
This way we only have one reference for each CA certificate, whether it
is loaded in an authority section, a connection or via load-certs() command.
It also avoids enumerating CA certificates multiple times if they are
loaded in different ways.
With the previous approach, CA certificates that were not re-loaded via
load-cert() (e.g. from tokens or via absolute paths) would not be available
anymore after the clear-creds() command was used. This avoids this
issue, but can cause duplicate CA certificates to get stored and enumerated,
so there might be a scaling factor.
This reduces the clustering problem (primary clustering) but is not
completely free of it (secondary clustering) it still reduces the maximum
and average probing lengths.
With the previous approach we'd require at least an additional pointer
per item to store them in a list (15-18% increase in the overhead per
item). Instead we switch from handling collisions with overflow lists to
an open addressing scheme and store the actual table as variable-sized
indices pointing into an array of all inserted items in their original
order.
This can reduce the memory overhead even compared to the previous
implementation (especially for smaller tables), but because the array for
items is preallocated whenever the table is resized, it can be worse for
certain numbers of items. However, avoiding all the allocations required
by the previous design is actually a big advantage.
Depending on the usage pattern, the performance can improve quite a bit (in
particular when inserting many items). The raw lookup performance is a bit
slower as probing lengths increase with open addressing, but there are some
caching benefits due to the compact storage. So for general usage the
performance should be better. For instance, one test I did was counting the
occurrences of words in a list of 1'000'000 randomly selected words from a
dictionary of ~58'000 words (i.e. using a counter stored under each word as
key). The new implementation was ~8% faster on average while requiring
10% less memory.
Since we can't remove items from the array (would change the indices of all
items that follow it) we just mark them as removed and remove them once the
hash table is resized/rehashed (the cells in the hash table for these may
be reused). Due to this the latter may also happen if the number of stored
items does not increase e.g. after a series of remove/put operations (each
insertion requires storage in the array, no matter if items were removed).
So if the capacity is exhausted, the table is resized/rehashed (after lots
of removals the size may even be reduced) and all items marked as removed
are simply skipped.
Compared to the previous implementation the load factor/capacity is
lowered to reduce chances of collisions and to avoid primary clustering to
some degree. However, the latter in particular, but the open addressing
scheme in general, make this implementation completely unsuited for the
get_match() functionality (purposefully hashing to the same value and,
therefore, increasing the probing length and clustering). And keeping the
keys optionally sorted would complicate the code significantly. So we just
keep the existing hashlist_t implementation without adding code to maintain
the overall insertion order (we could add that feature optionally later, but
with the mentioned overhead for one or two pointers).
The maximum size is currently not changed. With the new implementation
this translates to a hard limit for the maximum number of items that can be
held in the table (=CAPACITY(MAX_SIZE)). Since this equals 715'827'882
items with the current settings, this shouldn't be a problem in practice,
the table alone would require 20 GiB in memory for that many items. The
hashlist_t implementation doesn't have that limitation due to the overflow
lists (it can store beyond it's capacity) but it itself would require over
29 GiB of memory to hold that many items.
The main intention here is that we can change the hashtable_t
implementation without being impeded by the special requirements imposed
by get_match() and sorting the keys/items in buckets.
This can improve negative lookups, but is mostly intended to be used
with get_match() so keys/items can be matched/enumerated in a specific
order. It's like storing sorted linked lists under a shared key but
with less memory overhead.
They are not marked as temporary addresses so make sure we always return
them whether temporary addresses are preferred as source addresses or not
as we need to enumerate them when searching for addresses in traffic selectors
to install routes.
Fixes: 9f12b8a61c ("kernel-netlink: Enumerate temporary IPv6 addresses according to config")
We don't track CHILD_SA down events anymore and rely on NM's initial timeout
to let the user know if the connection failed initially. So we also don't
have to explicitly differentiate between initial connection failures and
later ones like we do an Android. Also, with the default retransmission
settings, there will only be one keying try as NM's timeout is lower than
the combined retransmission timeout of 165s.
There is no visual indicator while the connection is reestablished later.
Fixes#3300.
If a MOBIKE task is deferred, the retransmission counter is reset to 0
when reinitiating. So if there were retransmits before, this alert would
not be triggered if a response is received now without retransmits.
We usually have a local IP already via ike_sa_t::resolve_hosts() before
build_i() is called but if that's not the case, it's more likely we have
one after we processed the first response (it might also have changed).
There is a potential chance we still don't have one if the socket API
doesn't provide us with the destination address of received messages,
but that seems not very likely nowadays.
We need the PSK/identity already when deriving the keys in process_i().
Fixes: 1665a4e050 ("ikev1: Use actual local identity as initiator or aggressive mode responder")
The native parseInetAddressBytes() method called by that method is not
available when running the tests.
Not very pretty and there are some warnings because PowerMock does
reflection in some illegal way but it fixes the unit tests and does
not require any new dependencies like Apache Commons or Guava just to
parse IP addresses without DNS lookup.
Fixes: 2ef473be15 ("android: Use helper to parse IP addresses where appropriate")
Fixes#3443.
This allows users to ignore whether the app is on the device's power
whitelist without a warning. The flag is currently not set
automatically if the user denies the request.
This is necessary so we can actually schedule events accurately in Doze
mode. Otherwise, we'd only get woken in intervals of several minutes (up to
15 according to the docs) after about an hour.
This uses AlarmManager to schedule events in a way that ensures the app
is woken up (requires whitelisting when in Doze mode to be woken up at
the exact time, otherwise there are delays of up to 15 minutes).
Retransmission jobs for old requests for which we already received a
response previously left the impression that messages were sent more
recently than was actually the case.
task_manager_t always defined INVALID_STATE as possible return value if
no retransmit was sent, this just was never actually returned.
I guess we could further differentiate between actual invalid states
(e.g. if we already received the response) and when we don't send a
retransmit for other reasons e.g. because the IKE_SA became stale.
Previously, if the two utility functions were called while the VPN
connection was established (i.e. charon was initialized) the logger for
libstrongswan would get reset to the initial log handler. So certain
log messages would not get logged to the log file after the TUN device
was created (one of the helpers is used to convert IPs there).
A new NAT mapping might be created even if the IP stays the same. Due to
the DPD fallback with NAT keep-alives this might only be necessary in
corner cases, if at all.
This is useful on Android where the app might not be able to send
keep-alives if the device is asleep for a while. If the NAT mapping
has been deleted in the mean time, the NAT-D payloads allow detecting
this and connectivity can be restored by doing a MOBIKE update or
recreating the SA if the peer already deleted it because the client
wasn't reachable.
This allows measuring the delay between events more accurately if a
device is often suspended.
While CLOCK_BOOTTIME would be preferable, Android's bionic C library
does not support it for condvars.
On some systems it might be preferable to use e.g. CLOCK_BOOTTIME
instead of CLOCK_MONOTONIC, which is also not affected by time
adjustments but includes times when the system was suspended.
XML resources are apparently not supported there. Moving the icon to
the mipmap folders should fix that. Aliases are defined for the icons on
Android < 8.0.
Evidently, onClick() may be called either before onStartListening() or
after onStopListening() has been called, which causes a crash when
trying to load a VpnProfile via mDataSource.
This partially reverts 3716af079e ("android: Avoid crash related to
TileService on Huawei devices").
This change allows to customize the previously hard-coded remote traffic
selectors.
This does not actually write the newly added "remote-ts" configuration option
into NetworkManager's configuration file, but will use an existing value.
Exposing the config setting in the GUI could be done later if this is a
desired change.
Use case: remote firewall appliance wrongly accepts the `0.0.0.0/0` TS but
does not actually route external traffic, leaving the user with a partially
working internet connection.
Closesstrongswan/strongswan#173.
If we fail connecting to the host we got redirected to, we should restart
with the original host where we might get redirected to a different host.
We must not reset this when retrying due to INVALID_KE_PAYLOAD or COOKIE
notifies. Since we keep the initiator SPI in those cases, we use that
flag as indicator.
Since we don't store the original remote_host value, we can't restore
that. So there is a potential conflict with MIPv6.
Closesstrongswan/strongswan#171.
The height of the dialog increased due to the recently added additional
fields for certificate selection and identities. On some screens the
fields to configure custom proposals were not visible anymore.
Together with less spacing on the top level GtkBox this change reduces
the height by about 80 pixels.
Fixes#3448.
The path '/usr/share/appdata' is deprecated as is the .appdata.xml
extension, files should be in installed in '/usr/share/metainfo' with
a .metainfo.xml extension.
According to the docs, the metainfo path should be well supported even
by older distros like Ubuntu 16.04.
Reference: 2.1.2. Filesystem locations
https://www.freedesktop.org/software/appstream/docs/chap-Metadata.html
The need_secrets() method is called before connect() (where we clear the
previous secrets too), so e.g. a password-protected private could be
decrypted with the cached password from earlier but if the password was not
stored with the connection, it would later fail as no password was requested
from the user that could be passed to connect().
References #3428.
On newer desktops the auth dialog is called with --external-ui-mode and
it seems that the password flag has to be set, otherwise the password is
not stored temporarily in the profile and passed to charon-nm (not sure
how this works exactly as need_secrets() is called multiple times even
after the password was already entered, only before doing so the last
time is the password available in that callback, but only if the flag
was set). This now also allows storing the password for the private key
with the profile.
Fixes#3428.
Due to the exponential backoff a high number of retransmits only
makes sense if retransmit_limit is set. However, even with that there
was a problem.
We first calculated the timeout for the next retransmit and only then
compared that to the configured limit. Depending on the configured
base and timeout the calculation overflowed the range of uint32_t after
a relatively low number of retransmits (with the default values after 23)
causing the timeout to first get lower (on a high level) before constantly
resulting in 0 (with the default settings after 60 retransmits).
Since that's obviously lower than any configured limit, all remaining
retransmits were then sent without any delay, causing a lot of concurrent
messages if the number of retransmits was high.
This change determines the maximum number of retransmits until an
overflow occurs based on the configuration and defaults to UINT32_MAX
if that value is exceeded. Note that since the timeout is in milliseconds
UINT32_MAX equals nearly 50 days.
The calculation in task_manager_total_retransmit_timeout() uses a double
variable and the result is in seconds so the maximum number would be higher
there (with the default settings 1205). However, we want its result to
be based on the actual IKE retransmission behavior.
Some peers apparently don't send the notify and still expect to
authenticate with EAP-only authentication. This option allows forcing
the configured use of EAP-only authentication in that scenario.
If such a task was active while reestablishing it will get queued on the
new IKE_SA. If the DH group is already set, the DH groups won't be
stripped from the proposals and a KE payload will be sent, which is invalid
during IKE_AUTH. We don't want to reset the group if the task is part of a
child-rekey task.
Otherwise, the output is buffered when e.g. piping the output to another
command (or file). And it avoids having to call fflush() in the
interactive mode.
Fixes#3404.
The file is usually opened/created by root, however, if user/group IDs
are configured and the configuration is reloaded, the file will be reopened
as configured user. Like with UNIX sockets we only attempt to change
the user if we have CAP_CHOWN allowing a start as regular user.
We don't have chown() on Windows, so check for it.
If cards/libraries don't support signature mechanisms with hashing, we fall
back to do it ourselves in software and pass the PKCS#1 digestInfo ASN.1
structure to sign via CKM_RSA_PKCS mechanism.
Closesstrongswan/strongswan#168.
OpenSSL currently doesn't support squeezing bytes out of an XOF multiple
times. Unfortunately, EVP_DigestFinalXOF() completely resets the context
and later calls not simply fail, they cause a null-pointer dereference in
libcrypto. This fixes the crash at the cost of repeating initializing
the whole state and allocating too much data for subsequent calls.
There is an open issue and PR that might add a function that allows
squeezing more data from an XOF in a future version of OpenSSL.
strtol(3) accepts values in the range of [LONG_MIN;LONG_MAX]. Based
on the architecture (32 or 64 bits), these values expand to either
0x8000000000000000/0x7fffffffffffffff for 64-bit builds, or
0x80000000/0x7fffffff for 32-bit builds.
The behavior when retrieving non-default values for charon.spi_min or
charon.spi_max, for example, depends on the architecture of the target
platform. While 0xC000001/0xCFFFFFFE work fine on a 64-bit build, on a
32-bit build, due to the use of strtol(3), an ERANGE causes get_int()
to return the default values.
By using strtoul(3) the default is only returned if the input value
exceeds 32 or 64 bits, based on the platform. Negative values are still
parsed correctly.
Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
Enables us to ignore any future kernel features for routes unless
we actually need to consider them for the source IP routes.
Also enables us to actually really skip IPsec processing for those networks
(because even the routes don't touch those packets). It's more what
users expect.
Co-authored-by: Tobias Brunner <tobias@strongswan.org>
This is mainly an issue on FreeBSD where the current kernel still only
allows the daemon to use reqids < IPSEC_MANUAL_REQID_MAX (0x3fff = 16383).
Fixes#2315.
Charon refuses to make use of algorithms IDs from the private space
for unknown peer implementations [1]. If you chose to ignore and violate
that section of the RFC since you *know* your peers *must* support those
private IDs, there's no way to disable that behavior.
With this commit a strongswan.conf option is introduced which allows to
deliberately ignore parts of section 3.12 from the standard.
[1] http://tools.ietf.org/html/rfc7296#section-3.12
Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
Doing this for the self-signed check also (i.e. if this and issuer are
the same) is particularly useful if the issuer uses a different key type.
Otherwise, we'd try to verify the signature with an incompatible key
that would result in a log message.
Fixes#3357.
Previously, we simply used the lifetimes of the first
proposal/transform, which is not correct if the initiator uses different
lifetimes in its proposals/transforms.
If an IKE_SA is terminated while a task is active, the delete task is
simply queued (unless the deletion is forced). If the active task times
out before any optional timeout associated with the termination hits, the
IKE_SA previously was reestablished without considering the termination
request.
Fixes#3335.
Also makes it configurable via configure script. Depending on `$datadir` is
not ideal as package maintainers might set that to a custom value. Depending
on `$datarootdir` might have been better, the default if pkg-config fails is
now based on that.
References #3339.
The code is structured similar to that in the Android client, but two-round
authentication (cert+EAP) is not supported as that might require multiple
secrets ("password" is currently the only secret field used for every
method) and other details are currently missing too (like configurable
client identities).
There are a ton of libsoup/GLib-related "leaks" that we can't whitelist
and with leak detective active there is a delay that interestingly doesn't
happen with soup_session_sync_new(), so tests failed with a timeout (actually
they hung due to the lock in the fetcher manager).
On Travis, the curl plugin is used for the tests, so that's not an issue
there (and without LD the tests complete quickly and successfully).
As noted in 8ea13bbc5c newer compilers might optimize out the
assignment leading to invalid values in the keyUsage extension (as the
length was still set, the extension was encoded, just not with the
intended values).
Fixes#3249.
While it's unlikely that so many (large) items are allocated, this is
technically more correct. The result previously could overflow an
unsigned int (the conversion to size_t happened afterwards).
Mahantesh Salimath