The default MTU of 1500 is too high if kernel-libipsec is used (considering
the overhead of UDP-encapsulated ESP), but might also have an effect if
a TUN device is only used to install a virtual IP (the route points to it,
so the system might use its MTU and 1500 would still be too high).
This also works around an issue on macOS 12 where no RTM_IFINFO event
is sent for the newly created TUN device (neither for the creation,
setting it "up", nor adding the address). Changing the MTU, however,
triggers such an event and we can detect the virtual IP.
Closesstrongswan/strongswan#707
NULL parameters (for classic PKCS#1 signature schemes) are explicitly
allowed (for any schemes for now), but we only expect parameters for
RSASSA-PSS. Before enforcing this, it was possible to modify the
parameters in the signatureAlgorithm field of the outer X.509 Certificate
structure to something different than the signature field of the signed,
inner tbsCertificate structure, allowing generating infinite versions
of valid certificates with different binary encodings. Now we accept at
most two (NULL and absent parameters).
Previously, only parameters of type OID, SEQUENCE and OCTET STRING were
returned (so e.g. random integers could be put in parameters and we
wouldn't know about it).
Log output is basically the same as with asn1_parser_t before, except
that parameters are always dumped (if any), that wasn't the case before
because ASN1_RAW (instead of ASN1_OBJ) was used.
random() allocates values in the range [0, RAND_MAX], with RAND_MAX usually
equaling INT_MAX = 2^31-1. Previously, values between 0 and 31 were added
directly to that offset before applying`% CACHE_SIZE` to get an index into
the cache array. If the random value was very high, this resulted in an
integer overflow and a negative index value and, therefore, an out-of-bounds
access of the array and in turn dereferencing invalid pointers when trying
to acquire the read lock. This most likely results in a segmentation fault.
Fixes: 764e8b2211 ("reimplemented certificate cache")
Fixes: CVE-2021-41991
The `salt_len` field is signed because negative values are used to indicate
automatic salt lengths when generating signatures. This must never be the
case when validating them.
Not checking this could lead to an integer overflow below. The value is
assigned to the `len` field of a chunk (`size_t`), which is further used
in calculations to check the padding structure and (if that is passed by
a matching crafted signature value) eventually a memcpy() that will result
in a segmentation fault.
Fixes: 7d6b81648b ("gmp: Add support for RSASSA-PSS signature verification")
Fixes: CVE-2021-41990
The `salt_len` member in the struct is of type `ssize_t` because we use
negative values for special automatic salt lengths when generating
signatures. This change ensures that `salt_len` will not overflow the
`len` fields of chunks (`size_t`), which could lead to integer overflows
when validating signatures (see the next commit).
Fixes: a22316520b ("signature-params: Add functions to parse/build ASN.1 RSASSA-PSS params")
If the randomly allocated serial starts with 0x80, the fix added with
e49197f15e ("pki: Don't generate negative random serial numbers in
X.509 certificates") causes the value to become zero. So the encoded
ASN.1 integer will start with a zero byte, which is only correct if the
integer would otherwise be interpreted as negative, i.e. the next byte
starts with the most significant bit set. If that isn't the case, the
encoding is technically invalid and might get rejected by strict parsers.
Note that e49197f15e did not actually fix a violation of RFC 5280 as
asn1_integer() assumes all passed numbers are positive and automatically
adds a zero prefix if the MSB is set. What it did instead (or at least
attempted to) is ensure that the generated serial is a positive 64-bit
number in two's complement. The difference can be seen in the output of
`openssl x509 -text`. While 8-byte serials with the MSB set are printed
as hex dump:
Serial Number:
af:e2:e1:47:0f:66:b5:a4
(The encoding is 02:09:00:af:e2:e1:47:0f:66:b5:a4)
those without MSB set are actually printed as number:
Serial Number: 289805014144645117 (0x405981ffa37f7fd)
(The encoding is 02:08:04:05:98:1F:FA:37:F7:FD)
The reason is that OpenSSL only does the latter if the number fits into a
signed `long` variable, which isn't the case if a positive 64-bit number
has the MSB set (i.e. has a zero prefix) as it would be interpreted as
negative number in two's complement. OpenSSL does print negative serial
numbers (even if it's a violation of the RFC), but only if they were
encoded as such, i.e. if there was no zero prefix:
Serial Number: -5492225205882687294 (-0x4c384db9c7158f3e)
(The encoding is 02:08:b3:c7:b2:46:38:ea:70:c2)
Fixes: e49197f15e ("pki: Don't generate negative random serial numbers in X.509 certificates")
Closesstrongswan/strongswan#631
This is mostly for the non-mmap case as with mmap available, access to the
unmapped memory isn't easily possible (e.g. opening the same area with
MAP_ANONYMOUS | MAP_UNINITIALIZED is usually prevented by the missing
CONFIG_MMAP_ALLOW_UNINITIALIZED option in most kernels).
New versions of Bind limit the maximum UDP message size to 1232 bytes,
which is the same that newer versions of libunbound propose as maximum via
EDNS in requests, so increasing the limit on the server wouldn't help.
Instead we allow DNS via TCP so the client can switch after receiving the
truncated UDP response.
Using the workaround with the EVP interface, which we use to derive shared
keys since 74e02ff5e6 ("openssl: Mainly use EVP interface for ECDH"),
would actually require us to register the OIDs of these curves as NID.
Otherwise, the two EC_GROUPs used by private and public key objects
are not considered the same and the key derivation fails.
Since the curves are supported by OpenSSL since 1.0.2 it's probably rare to
find a version without them nowadays. One exception is the old BoringSSL
version we still use on Android, which defines the NIDs but not the curve
data. However, that version also lacks support to register OIDs as NIDs,
so the only option to support these groups there would be to got back to not
using the EVP interface, which isn't in anyone's interest. If there really
is a need for them there, we could probably patch BoringSSL or use OpenSSL.
The "openssl" alias now defaults to OpenSSL 3.0, which produces a lot of
deprecation warnings. To avoid build failures due to `-Werror`, stay with
OpenSSL 1.1 until we can get rid of these issues.
With the update to Python 3 the encoding of the values in vici messages
changed to bytestrings (the keys are properly decoded). And getting the
first CHILD_SA also needs a change.
The logger is now also initialized after daemonizing to avoid that opened
sockets are closed etc.
If there are many CHILD_SAs, the time between initiating the new IKE_SA
and checking it in might be longer (depending on what else is going on
in the daemon) than the retransmission timeout and no retransmits might
be sent afterwards for this SA (it will just linger around dead).
Calling initiate() last should avoid that (we do this similarly for MBB
reauthentication).
If we are using make-before-break reauthentication, this could lead to
duplicates as the new IKE_SA wouldn't be able to delete the previous
one if it was replaced by a rekeying.
If we initiated a make-before-break reauthentication and the peer
concurrently deletes the IKE_SA (e.g. because it uses break-before-make
reauthentication), we would create a duplicate IKE_SA (the condition forces
a recreation of all existing CHILD_SAs because reestablish() is also called
to complete a break-before-make reauthentication).
According to the documentation for actions/cache, the lookup is already
scoped to the current branch (with fallback to any base branch including
the default branch).
Previously, only the tpm plugin initialized the library, so in order
to use a TPM 2.0 (a required TCTI library is loaded via init), it was
necessary to load it even if none of its actual features were used.
OpenSSL enforces a minimum of 14 bytes (112 bits) on the key size when
used in FIPS-mode (as required by SP 800-131A). So by using an empty
string, instantiation always failed. 32 bytes (256 bits) should be safe
for now.
Closesstrongswan/strongswan#557
Adds a button to install user certificates and updates the target SDK
version for Android 11 (and the related deprecation fixes), which will
be mandatory later this year.
The release also includes an older commit that changed how DNS servers
are applied to TUN devices (cd10ae2ff0 ("android: Explicitly apply DNS
servers to the TUN device")).
Also added are metadata for F-Droid.
This is required when targeting Android 11 (API 30) in order to see all
packages, which we use to allow selecting apps ex-/included from VPN
profiles and for the EAP-TNC use case.
As suggested by the Android docs, we use a global thread pool and handler
to avoid recreating them repeatedly. Four threads should be more than
enough as we only use this to load CA certificates when the app starts
initially and to load user certs when editing a profile.
This will be mandatory for new apps in August and for existing apps
in November. However, several classes like AsyncTask are now deprecated
so this needs some work to avoid warnings and problems in the future.
This way we get updated versions automatically (referencing "master"
required manually deleting the downloaded archives and the unpacked
directories). It also allows switching versions when working in different
branches (note that REV can also be set to a commit ID, e.g. to test
changes before tagging them later and merging the branch).
Use the same configure options etc. for both builds (no need for the cert
options as we don't use TLS or X.509 parsing) and switch to a Git commit
that includes the SHA-3 OID fix (it's actually the fix itself).
This also restores the test as it was before the referenced commit so it
again, as written in the description, demonstrates that venus is unable
to ping sun without IPsec tunnel.
Fixes: f27fb58ae0 ("testing: Update description and test evaluation of host2host-transport-nat")
Note that the mobike-nat test has been removed as it basically did the same
as the mobike-virtual-ip-nat test. Instead, the mobike-nat-mapping scenario
is added, which simulates a NAT router restart.
The addresses observed by the client behind the NAT are exactly the same if
the NAT router gets restarted.
Fixes: 2b255f01af ("ike-mobike: Use ike_sa_t::update_hosts() to trigger events")
If two threads are waiting in find_entry() and remove_entry(),
respectively, and the former is woken first, the latter remains stuck
as it won't get signaled.
If there are threads waiting in find_entry() and one in remove_entry()
and the latter is woken first by a thread calling put_entry(), the
former threads would remain stuck as they get never signaled.
This can happen if an IKE_SA is terminated forcefully shortly before
terminating the daemon. The thread that handles the terminate command
will call checkin_and_destroy(), which unregisters the IKE_SA from the
manager before destroying it. The main thread that calls flush() on the
IKE_SA manager won't wait for this SA (its entry is already gone), so
the processor and in turn the watcher job/thread might get canceled
before the first thread started deleting the VIP. It would then wait
indefinitely for a signal that can never be sent.
There is still a small chance the thread hangs in wait() if the state check
happens right before the watcher is canceled and it wasn't yet able to
deliver the event from the kernel, we counter that by rechecking the state
after a while.
If a PKCS#11 library/token doesn't provide one or more attributes via
C_GetAttributeValue(), we get back CKR_ATTRIBUTE_TYPE_INVALID (similar
for protected attributes where CKR_ATTRIBUTE_SENSITIVE is returned).
This is not an error as the spec demands that all attributes have been
processed with the unavailable attributes having set their length
field to CK_UNAVAILABLE_INFORMATION.
We use this to handle the CKA_TRUSTED attribute, which some tokens
apparently don't support. We previously used a version check to remove
the attribute from the call but even the latest spec doesn't make the
attribute mandatory (it's just in a list of "common" attributes for
CKO_CERTIFICATE objects, without a default value), so there are current
tokens that don't support it and prevent us from enumerating certificates.
Depending on how CLOCK_MONOTONIC is implemented, time_monotonic() might
return 0 within 1 second after the system is started. If that's the
case, we just default to 0 for now to avoid a crash (doesn't "hide" the
system time, but it's only the uptime anyway in this case).
Closesstrongswan/strongswan#435.