Tobias Brunner [Fri, 12 May 2023 16:36:30 +0000 (18:36 +0200)]
credential-manager: Improve selection of local certificate and trust chain
The previous code was problematic if a certificate request for a known
but unrelated CA was received and the local trust chain was incomplete.
Due to the received anchor, the incomplete trust chain was dismissed and
any intermediate CA certificates were, therefore, not sent to the peer.
This new approach doesn't dismiss an incomplete trust chain, but prefers
one that can be resolved to a received anchor. If no such chain is found,
the first one is used.
Tobias Brunner [Tue, 23 May 2023 11:19:57 +0000 (13:19 +0200)]
Merge branch 'libipsec-raw-esp'
This adds support for sending/receiving ESP packets without UDP
encapsulation to libipsec and kernel-libipsec. Only Linux is currently
supported and the feature is disabled by default.
Tobias Brunner [Thu, 4 May 2023 12:20:09 +0000 (14:20 +0200)]
ipsec-sa-mgr: Optionally keep track of acquires for outbound SAs
Currently just based on the reqid. An acquire for the same reqid is
triggered at most every 10 seconds (gets ignored in trap_manager_t if
the SA is still getting established).
Entries are only cleaned up if an SA is eventually installed (similar to
the allocated SPIs). Should that ever be a problem, we could probably
schedule a job that regularly flushes old entries.
Tobias Brunner [Thu, 11 May 2023 10:29:10 +0000 (12:29 +0200)]
pki: Make --dn optional for certificate renewals via --scep command
When using OpenXPKI, the subject DN in the renewal request has to match
the previous DN exactly. However, because OpenXPKI may add a bunch of
DC/O RDNs to subjects of issued certificates, running --scep with the
same --dn that was used for the original request won't work (results in
a "Client error / malformed request badRequest" error even after enabling
`renewal_via_pkcs_req`). This simplifies renewals as --dn can just be
omitted and extracted from the original certificate to avoid this issue.
Tobias Brunner [Tue, 16 May 2023 10:48:22 +0000 (12:48 +0200)]
Merge branch 'debug-level-build'
This fixes the build with DEBUG_LEVEL < 4, which was broken when building
from the repository since --enable-warnings was made the default.
Although, most issues only occurred with the level reduced to 0/-1. And
while removing debug statements at compile time completely is probably
not useful in production, there might be use cases in certain benchmarking
scenarios. Also, with the recent changes to the controller there should
only rarely be a listener registered at a higher log level so the overhead
for those higher-level DBG statements is minimal.
Anyway, reducing the log level at compile time is a documented feature and
at least DEBUG_LEVEL=3 could be useful to prevent leaking sensitive
information via logs from the outset. So we should make sure compilation
doesn't fail.
Tobias Brunner [Mon, 1 May 2023 08:44:42 +0000 (10:44 +0200)]
debug: Add macro to mark variables that are only used in DBG statements
Some variables that are only assigned to be used in DBG statements
will otherwise trigger a "set but not used" warning/error if DEBUG_LEVEL
is too low.
vici: Improve log messages for terminate/rekey() in case of combined filters
As long as any `child*` selector is received, only CHILD_SAs will be
terminated or rekeyed. Any passed `ike*` selectors will only be used to
filter the IKE_SAs when looking for matching CHILD_SAs. However, the
previous log messages seemed to indicate that IKE_SAs will also be
terminated/rekeyed.
charon-systemd: Add a log message when the daemon is starting
While there is a status message sent to systemd (can be seen e.g. in
systemctl status), the version etc. is currently not logged to the
journal, syslog or any log files.
controller: Add parameter for maximum log level to initiate/terminate_*()
Previously, the logger installed by the controller always announced
LEVEL_PRIVATE(4), which produced completely useless logging calls with
the common clients (vici/stroke) whose default log level is LEVEL_CTRL(1).
This can produce quite some overhead if there are e.g. a lot of concurrent
initiate() calls.
controller: Ignore log messages unrelated to IKE_SA affected by a command
Until we know which IKE_SA is affected by an initiate() or terminate_*()
command, unrelated log messages that don't have any IKE context (i.e.
the passed `ike_sa` is NULL) would previously get logged.
watcher: Prevent busy wait if callback is active and other FDs have events
Exiting the loop previously could cause watcher to busy wait (i.e.
rebuild the array and call poll() repeatedly) until the active callback
was done.
Assume watcher observes two FDs 15 and 22, which are in the list in that
order. FD 15 is signaled and its callback gets triggered. The array of
FDs is rebuilt and does not include 15 anymore. Now FD 22 is ready for
reading. However, when enumerating all registered FDs, the loop previously
was exited when reaching FD 15 and seeing that it's active. FD 22 was
never checked and the array was immediately rebuilt and poll() called.
If the callback for 15 took longer, this was repeated over and over.
This basically reverts d16d5a245f0b ("watcher: Avoid queueing multiple
watcher callbacks at the same time"), whose goal is quite unclear to me.
If it really wanted to allow only a single callback for all FDs, it didn't
achieve that as any FD before an active one would get notified and if
multiple FDs are ready concurrently, they'd all get triggered too.
Skipping entries with active callback makes sense as it avoids a lookup
in the FD array and subsequent revents checks. But why would we need to
rebuild the array if we see such an entry? Once the callback is done,
the watcher is notified and the array rebuilt anyway (also if any other
FD was ready and jobs get queued).
The list of FDs is recreated quite often (e.g. due to the kernel-netlink
event sockets) and if a logger depends on watcher_t in some way this
might cause conflicts if the mutex is held.
watcher: Make sure to re-activate the correct entry after a callback
Since the same FD may be added multiple times (e.g. with separate
callbacks for WATCHER_READ and WATCHER_WRITE), the previous check
might not have found the correct entry. As the entry can't be removed
while in a callback, the pointer comparison is fine.
watcher: Avoid logging on level 1 while holding the mutex
This could be problematic in case loggers in some way rely on watcher_t
themselves. This particular log message should rarely occur if at all,
but still avoid holding the mutex.
vici: Don't lock connection in write mode when enabling on_write() callback
This should prevent a deadlock that could previously be caused when a
control-log event was raised. The deadlock looked something like this:
* Thread A holds the read lock on bus_t and raises the control-log event.
This requires acquiring the connection entry in write mode to queue the
outgoing message. If it is already held by another thread, this blocks
on a condvar.
* Thread B is registering the on_write() callback on the same connection's
stream due to a previous log message. Before this change, the code
acquired the entry in write mode as well, thus, blocking thread A. To
remove/add the stream, the mutex in watcher_t needs to be acquired.
* Thread C is in watcher_t's watch() and holds the mutex while logging on
level 2 or 3. The latter requires the read lock on bus_t, which should
usually be acquirable even if thread A holds it. Unless writers are
concurrently waiting on the lock and the implementation is blocking
new readers to prevent writer starvation.
* Thread D is removing a logger from the bus (e.g. after an initiate()
call) and is waiting to acquire the write lock on bus_t and is thereby
blocking thread C.
With this change, thread B should not block thread A anymore. So thread D
and thread C should eventually be able to proceed as well.
Thread A could be held up a bit if there is a thread already sending
messages for the same connection, but that should only cause a delay, no
deadlock, as on_write() and do_write() don't log (or lock) anything while
keeping the entry locked in write mode.
Tobias Brunner [Fri, 31 Mar 2023 07:29:12 +0000 (09:29 +0200)]
Merge branch 'pkcs7-signatures'
Adds support for CMS-style signatures in PKCS#7 containers, which allows
verifying RSA-PSS and ECDSA signatures.
Ed25519 signatures should be supported when verifying, however, they
currently can't be created. Ed448 signatures are currently not supported.
That's because RFC 8419 has very strict requirements in regards to the
hash algorithms used for signed attributes. With Ed25519 only SHA-512 is
allowed (pki currently has an issue with Ed25519 in combination with
SHA-512 due to its associated HASH_IDENTITY) and with Ed448 only SHAKE256
with 512-bit output, which has to be encoded in the algorithmIdentifier
parameters (something we currently don't support at all).