Skip to content

Commit

Permalink
launch/config: use AT_RANDOM for XML hash salt
Browse files Browse the repository at this point in the history
Forward the entropy from AT_RANDOM to the hash-salt used by expat. Use
XML_SetHashSalt() for this (available and fixed since expat-2.1).

This fixes an issue where libexpat might read from `/dev/urandom` and
thus block until the entropy pool is initialized. This hidden
dependency is very hard to debug. Instead, we require the service
launcher to delay startup until suitable entropy is available. This
explicit dependency is much easier to manage, debug, and control.

Reported-by: Stefan Agner <stefan@agner.ch>
Signed-off-by: David Rheinsberg <david@readahead.eu>
  • Loading branch information
dvdhrm committed Jul 5, 2023
1 parent a53071a commit 7c70e9c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 0 deletions.
17 changes: 17 additions & 0 deletions src/launch/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <c-stdaux.h>
#include <expat.h>
#include <stdlib.h>
#include <sys/auxv.h>
#include "dbus/protocol.h"
#include "launch/config.h"
#include "launch/nss-cache.h"
Expand Down Expand Up @@ -1227,10 +1228,25 @@ static void config_parser_blob_fn(void *userdata, const XML_Char *data, int n_da
* config_parser_init() - XXX
*/
void config_parser_init(ConfigParser *parser) {
void *random;

*parser = (ConfigParser)CONFIG_PARSER_NULL(*parser);

parser->xml = XML_ParserCreate(NULL);
c_assert(parser->xml);

/*
* The hash-tables of libexpat require a reliable random seed.
* Depending on libexpat compilation flags, this might end up using
* `/dev/urandom` and thus block until random-initialization is
* finished. We avoid this hidden dependency and instead use the
* entropy provided via `AT_RANDOM`. Hence, entropy availability is
* tightly coupled to process startup, and it is the job of the
* service manager to order processes accordingly.
*/
random = (void *)getauxval(AT_RANDOM);
c_assert(random);
c_memcpy(&parser->salt, random, sizeof(parser->salt));
}

/**
Expand Down Expand Up @@ -1274,6 +1290,7 @@ static int config_parser_include(ConfigParser *parser, ConfigRoot *root, ConfigN
}

XML_ParserReset(parser->xml, NULL);
XML_SetHashSalt(parser->xml, parser->salt);
XML_SetUserData(parser->xml, &parser->state);
XML_SetElementHandler(parser->xml, config_parser_begin_fn, config_parser_end_fn);
XML_SetCharacterDataHandler(parser->xml, config_parser_blob_fn);
Expand Down
1 change: 1 addition & 0 deletions src/launch/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ struct ConfigRoot {

struct ConfigParser {
struct XML_ParserStruct *xml;
unsigned long salt;

struct ConfigState {
NSSCache *nss;
Expand Down

0 comments on commit 7c70e9c

Please sign in to comment.