Handle failed database initialisation more gracefully
Actually check the result from dbinit and exit rather than continuing
if we failed to initialise it. In particular we hit this path with a
missing config file.
Our existing HTML escaping was very primitive, looking only for a single
< and > around an email address. Extend this to cover multiple instance
of <, >, &, ", ' which should cover all the important bits. Also move to
using a caller provided buffer rather than a static buffer.
Relax version check on parsing signature + key packets
Rather than ensuring signatures/keys are no more than v5 allow them to
be anything from v2 onwards; otherwise things like armor/dearmor won't
work properly.
Switch to passing the key packet in when checking a hash signature
Rather than passing the whole key in for verifying a hash signature,
explicitly pass in the key packet. This opens the way to being able to
verify signatures from subkeys.
Switch charfuncs to returning number of read/written characters
The charfuncs prototypes were returning 0 for success ("I wrote all the
characters you asked me to") or 1 for failure. When we get to verifying
signatures over data we may not know how much to read, so change them to
returning 0 on error/no more data and otherwise the count of the amount
of data read/written.
Fix systemd unit files to deal with socket activation
If we make the onak.service file require onak.socket then systemd will
take care of creating the Unix domain socket file, and we can avoid the
need to setup the RuntimeDirectory.
Test signature verification by adding my new ECC key with verification
enabled, which will strip all signatures, then adding my old RSA key,
then readding my ECC key. This should result in the ECC key having a
signature from my old RSA key.
Indicate when we've been started via socket activation
Update the starting log message for keyd so it's clear when we've been
started up via systemd socket activation, which would have helped avoid
the long standing bug with building properly with systemd support.
All of our systemd detection logic was in keydb/CMakeLists.txt because
only keyd actually cares about it. However that then means we don't
correctly update build-config.h with HAVE_SYSTEMD and so we link against
it, but don't correctly do socket activation. Split out the detection
into the main CMakeLists.txt so we correctly set the define.
Don't catch signals if we're using the keyd backend
Catching signals dates from when we didn't have the keyd backend and
install every process opened the DB4 files itself. This lead to major
issues if the process didn't clean up correctly. If we're using keyd
there's no such concern, so we can avoid catching signals and preventing
things like Ctrl-C working for the CLI tool.
Support shallow git checkouts when configuring cmake
git will bale out if we're using a shallow checkout with no tags. Just
fall back to the short commit hash when that happens, so we can still
try and identify when the build was from.
Move the CGI specific routines into cgi/ and split out UID escaping
The only piece from getcgi.c that anything other than the CGI needs is
the code to escape a UID. Move that into keyindex.c and shuffle the CGI
routines off to the subdirectory. The escaping routine could do with a
lot of improvement, but this is a start in terms of cleaning things up.
The free Travis bits stopped working ages ago, let's try out GitHub's
Actions support as a replacement. Just a simple build + test without
extra deps to start with.
Do a pass-through with "gcc -Werror -Wunused-parameter" and mark up all
the unused function parameters (and remove a few unused variables). Also
add a CMake check to ensure we have __attribute__((unused)) available to
us.
Fix issues found by llvm scan-build static analysis
A mixture of fixes: a number of uses of getenv into functions that don't
like NULL, a couple of potential double-setting of config file details
resulting in a brief memory leak, and a true, but not an issue, report
of setting without using of our offset within a packet.
Switch to RuntimeDirectory in systemd service file
Instead of the ExecStartPre commands for creating our runtime directory
switch to the correct approach, RuntimeDirectory. Thanks to Guillem
Jover for the pointer.
We only support signature checking on v4+ keys, so hadn't any support
for v3 signature packets. However keys have been observed in the wild
where the key itself is a v4 key, but it's making v3 signatures (in
particular as a revocation signature for the key). While this seems odd
(anything that supports the key has to support v4, so why generate
anything lower?) it's probably too strict a check to have in place.
This gives us the appropriate RUNSTATEDIR setting of /run, and is still
compatible with backports to Bullseye. Also use the new
debhelper-compat build-dep rather than debian/compat.
The keyd socket was previously moved under /run/onak so that
subdirectory could be owned by the onak user. However the systemd
ExecStartPre commands run as the onak user, so the creation + ownership
setup was not properly happening. Prefix with a +, which tells systemd
these commands ignore the User= setting.
In pg_delete_key() we deleted the key from onak_keys as the first
action, which would fail because the other tables had a reference to
that object via a foreign key relation. The correct approach is to
delete the key itself last, after the signature and UID tables have had
their entries deleted.
Instead of putting keyd.sock file directly in /run create a /run/onak
which can then be owned by the onak user. Otherwise keyd will have
problems creating the socket when activated directly instead of via the
socket unit file.
Two fixes related to the check that a key has another signature on it.
Firstly, if any of the UIDs has a signature from another key then allow
all of them. Otherwise it's not possible to add a new UID to an existing
key. Our primary concern is that the key is linked into the WoT, rather
than policing individual UIDs.
Secondly, if a key is already present in the backend database then don't
perform the other signature check. If we've added to the backend and all
of the cross signatures are removed then it would be no longer possible
to update the key, which isn't what we want. If we've trusted it at some
point and added it then we should allow verifiable updates, even if
there are no valid cross signatures left.
When looking at the subpackets for a signature don't use the unhashed
set to obtain the creation time, and only use them for the keyid if it
wasn't present in the hashed section.
Provide key_fetch routine that will not search subkey fingerprints
We have *_key_fetch_fp to search for keys by fingerprint, but that
includes subkeys. While unlikely a subkey could be bound to multiple
certification primary keys, resulting in multiple keys being returned
for a fingerprint query. Provide a plain *_fetch_key that only searches
primary keys and is thus guaranteed to return at most one key.
Cope with colliding 64 bit keyids when verifying signatures
Signature keys can be indicated by 64 bit keyid rather than full
fingerprint; there are very few of this collisions but they do exist and
we should handle them gracefully rather than incorrectly dropping a
signature.
For a long time it has been known that attacks against the keyserver
network were easy due to the lack of cryptographic verification on
signatures, as well as the lack of other checks and balances. Add the
ability to actually verify signatures, allowing those are that are not
valid (or that come from non-present keys) to be removed.
Remove v5 keyid support when libnettle not present
v5 key fingerprints are SHA256 based. We have fallbacks for MD5 + SHA1
when libnettle is not present, but there's no intent to provide a SHA256
fallback, and we're close to the point where support for building
without libnettle will be removed entirely.
Move key database backends into their own directory
These logically sit together and are separate from the core code, so
move them off to a separate directory. There are various bits of support
that should be hived off the core libonak and move in here too, but that
can wait until later.
Cleanup postinst to avoid recursive chown of database
The recursive chown of /var/lib/onak has the potential to be abused
during installation, so just chown the top level directory and then run
the onak that's initialising the DB as the onak user, rather than root.
Add option to only accept updates for existing keys
It's plausible in a curated keyring scenario where a keyserver should
accept updates to existing keys (new signatures, updated expiries, new
subkeys) but no entirely new keys. Add a configuration file option which
enforces this behaviour.
GCC -Wall found a valid issue with a & instead of a && in the keyring DB
backend, a buffer assignment missing a cast and some unused variables in
the DB4 backend.
GCC with the -fsanitize=undefined flag issues complaints like:
runtime error: left shift of 232 by 24 places cannot be represented in
type 'int'
due to the fact we shift the byte values into a 64 bit value. Rather
than adding lots of casts split out the formation into a set of steps
that doesn't end up shifting the byte value, just the final 64 bit
value.
There are various keys in the wild, such as Evil32 (https://evil32.com/)
which a keyserver is unlikely to want to carry in the general case.
Introduce the capability to have a blacklist of fingerprints which will
not be accepted into the keyserver.
The old .conf config file was intended to be compatible with the ancient
PKS keyserver. All new features are only supported with the .ini format
and this is going to make the automated tests supporting both more
awkward. So deprecate the old style and stop testing it.
If the first new key had no new components we'd remove it from the list
but not actually free it. Fix things up so we free it properly. Found
with gcc's -fsanitize=leak option.
We were building a linked list of words in each UID, but then not
cleaning up this linked list when we were done with it. Found with gcc
-fsanitize=leak
This is a long-standing memory leak when merging signed packet lists. It
hasn't been observed in the wild because the key merging is normally
done in a transient process. Found using GCC with -fsanitize=leak
Expose more of calculating the packet signature hash
As preparation for full signature verification move to calculating the
signature hash and exposing it, and then doing the verification in the
cleaning routine.
v3 keys have long been considered insecure. While we want to retain
support for them there's no reason most keyservers should actually store
them these days. So drop them by default when running cleankeys()
This expanded a 32 bit key ID into a 64 bit key ID. It's mostly
redundant, so encode 64 bit key IDs where necessary and rely on the
fetch fallback to 32 bit elsewhere.
delete_key was deleting a key based on the keyid, which is
insufficiently precise. Move over to the full fingerprint (even though
with some backends this is an effective no-op for now).
Originally key retrieval was all performed on the 64 bit key ID but back
in 2013 support was added for fetching by fingerprint. However not all
the pieces to deal with colliding 64 bit IDs were added at this time.
This commit improves things by:
* Using the fingerprint to retrieve the key to update in update_keys()
* Returning all keys that share a 64 bit key ID from the DB4 backend,
rather than just the first found.
* Adding a test to ensure multiple keys are returned for a colliding
key lookup.
It also removes the old compatibility code for the DB4 backend with key
lookups by 64 bit key ID.
Note this isn't yet common in the wild; unlike Evil32 it is not possible
to create collisions for arbitrary key IDs.
In failure paths we can leak the memory allocated to hold the directory
path. This isn't really a problem, as we'll exit shortly afterwards, but
scan-build complains and we should really fix for completeness.
Exit on failure to initialise a dynamic DB backend
The dynamic DB backend does a lot of checks to ensure it can load the
requested backend successfully, but didn't actually check that it
initialised correctly. Bomb out with a graceful error message if this
happens rather than leading the caller to think everything is ok.
Allow the use of an OpenPGP keyring (like keyring.gpg from GnuPG v1) as
a read-only database backend. This is just a collection of keys
concatenated together. The file is mmaped on load and parsed for keys,
then simple linear searches are done for keyids/fingerprints.
Performance with large numbers of keys will not be good.
Rather than only parsing the encoded OID to calculate the key size make
a helper function in decodekey and use that. This will be useful when
trying to parse key material for signature checks.
projects / onak.git / log