Tobias Brunner [Thu, 27 Feb 2025 10:23:52 +0000 (11:23 +0100)]
Merge branch 'android-reauth-fix'
Fixes issues with reauthentication, in particular, to reestablish the
SA if MOBIKE is disabled. The app currently can't handle
make-before-break reauthentication. In part because necessary events are
currently not triggered. So for now, we switch back to the classic
reauthentication approach.
The service implementation with its handling of reauth callbacks and
no-DNS TUN device etc. can't handle make-before-break reauthentication
at the moment.
There was an issue with OPENSSL_armcap_P in Android's static build for
OpenSSL 3.1.1+. This was finally fixed with this release (and was also
backported to older versions).
Tobias Brunner [Tue, 4 Feb 2025 14:53:46 +0000 (15:53 +0100)]
android: Ignore empty strings for settings in managed profiles
Unspecified settings should be set to null, while some MDMs might send
them as empty strings, which could cause issues (like an empty password
or trying to parse an empty DNS server address).
Tobias Brunner [Tue, 4 Feb 2025 13:07:37 +0000 (14:07 +0100)]
android: Properly deinit library if parsing an IP fails
This can happen with empty strings, which might be set for managed
profiles, which caused the refcounting to be askew and the resolver not
to work after connecting once because it was flushed and disabled.
Tobias Brunner [Fri, 31 Jan 2025 10:21:04 +0000 (11:21 +0100)]
Merge branch 'dhcp-receive'
This fixes a regression introduced with pf_handler_t in 5.9.14. It also
binds the packet sockets correctly to the configured interface, and adds
an option for the dhcp plugin that allows binding the send and receive
sockets to different interfaces.
Tobias Brunner [Wed, 29 Jan 2025 16:23:31 +0000 (17:23 +0100)]
dhcp: Add option to bind the receive socket to a different interface
This can be useful if the DHCP server runs on the same server. On Linux,
the response is then sent via `lo`, so packets won't be received if both
sockets are bound to e.g. a bridge interface.
Tobias Brunner [Mon, 27 Jan 2025 08:40:56 +0000 (09:40 +0100)]
pf-handler: Accept loopback interfaces as packet source
In some setups the responses from the DHCP server are sent via lo, which
does not have an address of type `ARPHRD_ETHER` (the address length is
the same, though, just all zeros, by default). Note that the dhcp plugin
doesn't actually care for the MAC address or interface details, that's
only used by the farp plugin.
Fixes: 187c72d1afdc ("dhcp: Port the plugin to FreeBSD/macOS")
Nathan Tran [Tue, 28 Jan 2025 05:48:35 +0000 (21:48 -0800)]
ikev2: Handle INVALID_SYNTAX after IKE_AUTH by deleting IKE_SA
When serving as a responder and receiving an INFORMATIONAL exchange
containing INVALID_SYNTAX after IKE_AUTH, the IKE_SA should be deleted.
Currently, it only gets deleted after receiving AUTHENTICATION_FAILED.
RFC7296 section 2.21.2 says:
In an IKE_AUTH exchange, or in the INFORMATIONAL exchange immediately
following it (in case an error happened when processing a response to
IKE_AUTH), the UNSUPPORTED_CRITICAL_PAYLOAD, INVALID_SYNTAX, and
AUTHENTICATION_FAILED notifications are the only ones to cause the
IKE SA to be deleted or not created, without a Delete payload.
Tobias Brunner [Fri, 10 Jan 2025 14:43:11 +0000 (15:43 +0100)]
tnc-imv: Add missing argument to IMV recommendations constructor
This avoids the following warning/error:
tnc_imv_manager.c:244:39: error: passing arguments to 'tnc_imv_recommendations_create' without a prototype is deprecated in all versions of C and is not supported in C23 [-Werror,-Wdeprecated-non-prototype]
244 | return tnc_imv_recommendations_create(this->imvs);
| ^
Tobias Brunner [Thu, 9 Jan 2025 15:05:39 +0000 (16:05 +0100)]
ctr: Remove parameter-less constructor prototype
Useless and causes a compiler warning/error:
error: a function declaration without a prototype is deprecated in all versions of C and is treated as a zero-parameter prototype in C23, conflicting with a subsequent declaration [-Werror,-Wdeprecated-non-prototype]
The lines in the gperf-generated proposal_keywords_static.c are now
mapped to the (much shorter) .txt source file, which causes mismatches
like these:
genhtml: ERROR: no data for line:190, TLA:GNC, file:/home/runner/work/strongswan/strongswan/src/libstrongswan/crypto/proposal/proposal_keywords_static.txt
We could ignore "unmapped" errors in genhtml, but since the file is
generated anyway, we can also exclude it from the results and still
get such errors in case this happens for other files. Another alternative
would be to remove the `#line` macros in the generated file. Then the
coverage of the actual C file would get reported (but again, it's
generated, so there isn't much value in it).
Also updated the branch coverage option as the one with `lcov_` prefix
is deprecated.
Tobias Brunner [Fri, 10 Jan 2025 14:14:11 +0000 (15:14 +0100)]
github: Enable SRP in OpenSSL build for clang AddressSanitizer build
On Ubuntu 24.04, llvm-symbolizer-18, which is used to resolve symbols
in backtraces, links libcurl.so.4 for some reason. And that in turn
requires SRP. If our custom build doesn't provide it, we get stuff
like this
/usr/bin/llvm-symbolizer-18: symbol lookup error: /lib/x86_64-linux-gnu/libcurl.so.4: undefined symbol: SSL_CTX_set_srp_password, version OPENSSL_3.0.0
and the symbols are not resolved and can't be whitelisted.
This also makes sure ASan is actually disabled if our own leak-detective
is used.
Tobias Brunner [Wed, 8 Jan 2025 12:51:21 +0000 (13:51 +0100)]
constraints: Exclude self-signed root CAs without policies from policy validation
Self-signed trust anchors are not part of the certificate path validation
according to RFC 8280, section 6.1:
When the trust anchor is provided in the form of a self-signed
certificate, this self-signed certificate is not included as part of
the prospective certification path.
But policies in them could still be used, as stated in section 6.2:
Where a CA distributes self-signed certificates to specify trust
anchor information, certificate extensions can be used to specify
recommended inputs to path validation. For example, a policy
constraints extension could be included in the self-signed
certificate to indicate that paths beginning with this trust anchor
should be trusted only for the specified policies. [...]
Implementations that use self-signed certificates to specify trust
anchor information are free to process or ignore such information.
So unconditionally enforcing that self-signed root certificates contain
the policies is probably too strict. Often they won't contain the
extension at all. With this change, we allow that but still enforce the
policies in case such a certificate contains them. The other
policy-related constraints are also enforced still should they be
contained.
Tobias Brunner [Tue, 10 Dec 2024 15:48:40 +0000 (16:48 +0100)]
testing: Ignore errors when dumping our routing table
Some scenarios disable route installation and if they are executed before
any scenarios that don't, there won't be a rule for table 220 and we get
"FIB table does not exist" errors.
Tobias Brunner [Mon, 9 Dec 2024 14:21:52 +0000 (15:21 +0100)]
vici: Update Python build
Directly calling setup.py is deprecated (apparently has been for a while,
but now we get large warnings). Direct installation is also discouraged.
So this removes that option. The built wheel (the old egg format is not
used/built anymore) can be installed manually in a venv or the like.
Tobias Brunner [Fri, 6 Dec 2024 10:33:37 +0000 (11:33 +0100)]
vici: Delay creation of raw public keys until we know the identity
The previous approach had two drawbacks:
First, it caused duplicate public keys because when the `certificate_t`
object was created and added to the credential set it had no subject
assigned yet. So it defaulted to the key ID. However, all previously
loaded keys had their subject already changed to an identity, so there
never was a match and new objects were always added whenever a config
with raw public keys was loaded.
Second, the subject was replaced in a way that's not thread-safe on an
object that's already shared in the public credential set. So other
threads could potentially access the `identification_t` object that's
destroyed during that process.
Rob Shearman [Thu, 14 Nov 2024 13:15:36 +0000 (13:15 +0000)]
child-create: Fix double free of list of labels after migrate
If a migrate of a child-create occurs then labels_i and labels_r are
freed, but the pointers are left set. If the task is subsequently
destroyed without being reused, then both of these will be double
freed.
Fix this by setting labels_i and labels_r to NULL in the migrate
method after freeing, similar to other fields that are freed.
Closes strongswan/strongswan#2552
Fixes: f9b895b49f49 ("child-create: Add support to handle security labels")
Tobias Brunner [Mon, 2 Dec 2024 18:07:51 +0000 (19:07 +0100)]
testing: Improve replacing IP addresses in test files
There are a lot of files without patterns and running them all through
sed is quite slow. Using grep first makes this quicker (about 0.5s per
test). Ignoring PEM files is also helpful.
Tobias Brunner [Fri, 29 Nov 2024 17:24:18 +0000 (18:24 +0100)]
testing: Collect test results with an on-guest script in parallel
In particular the swanctl calls all take a while and this allows doing
them in parallel if multiple hosts are involved. This reduces the runtime
of each test by 1-3 seconds.
Thomas Egerer [Mon, 25 Nov 2024 15:37:40 +0000 (15:37 +0000)]
child-sa: Update status flags based on success of SA addition
Both variables `inbound_installed` and `outbound_state` are used in
`child_sa_t::destroy()` to determine whether inbound and outbound state
have to be deleted. They are assigned prior to the call to
`kernel_interface_t::add_sa()`. As this call may fail, the destructor may
try to delete a state which it has not been added.
By making the assignment of these variables dependent on the success of
the state addition, we can make sure, a `child_sa_t::destroy()` only
deletes states it has added.
Also removed the redundant checks for `my_spi` and `other_spi` being set
along with the check for the above flags. It seems that when the flags
are set, the SPIs *must* be set.
Signed-off-by: Thomas Egerer <thomas.egerer@secunet.com>
Andreas Steffen [Mon, 25 Nov 2024 14:35:53 +0000 (15:35 +0100)]
cert-enroll: Support three generations of CA certificates
If the lifetime of an issuing or sub CA is twice the lifetime of
the end entity certificates issued by it and the renewal cycle of
the issuing CAs is a little shorter than the validity of the end
entity certificates then three generations of CA certificates have
to be handled by the cert-enroll scripts.
Tobias Brunner [Mon, 25 Nov 2024 15:46:04 +0000 (16:46 +0100)]
unit-tests: Remove RSA/ECDSA schemes with weak hash algorithms (MD5/SHA-1)
These have been discouraged for a long time and there are now more and
more crypto libraries that have them disabled by default. However, for
some we only can detect this at runtime, in particular in FIPS mode, so
tests would fail as the plugins would still announce them. So instead
we just remove the schemes from these tests for now (at least for RSA,
removing signatures with SHA-1 completely isn't an option yet as that's
still the default with some clients).
Tobias Brunner [Mon, 25 Nov 2024 10:40:57 +0000 (11:40 +0100)]
testing: Make timing for TKM rekey scenarios a bit more stable
In particular for the first one randomization could trigger an additional
rekeying, which let the "Adding ESA ..." check fail. But even without
randomization (could be seen in the second scenario that already uses
`rand_time=0`) 4 seconds can apparently be too low some time.