Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1629661 MPConfig calls in SSL initializes policy before NSS is in…
…itialized. r=mt

NSS has several config functions that multiprocess servers must call before NSS is initialized to set up shared memory caches between the processes. These functions call ssl_init(), which initializes the ssl policy. The ssl policy initialization, however needs to happen after NSS itself is initialized. Doing so before hand causes (in the best case) policy to be ignored by these servers, and crashes (in the worst case).

Instead, these cache functions should just initialize those things it needs (that is the NSPR ssl error codes).

This patch does:
1) fixes the cache init code to only initialize error codes.
2) fixes the selfserv MP code to 1) be compatible with ssl.sh's selfserv management (at least on Unix), and 2) mimic the way real servers handle the MP_Cache init code (calling NSS_Init after the cache set up).
3) update ssl.sh server policy test to test policy usage on an MP server. This
is only done for non-windows like OS's because they can't catch the kill signal
to force their children to shutdown.

I've verified that the test fails if 2 and 3 are included but 1 is not
(and succeeds if all three are included).

Differential Revision: https://phabricator.services.mozilla.com/D70948
  • Loading branch information
rjrelyea committed Apr 14, 2020
1 parent 10ba925 commit 108aa43
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 10 deletions.
49 changes: 41 additions & 8 deletions cmd/selfserv/selfserv.c
Expand Up @@ -2125,6 +2125,20 @@ haveAChild(int argc, char **argv, PRProcessAttr *attr)
return newProcess;
}

#ifdef XP_UNIX
void
sigusr1_parent_handler(int sig)
{
PRProcess *process;
int i;
fprintf(stderr, "SIG_USER: Parent got sig_user, killing children (%d).\n", numChildren);
for (i = 0; i < numChildren; i++) {
process = child[i];
PR_KillProcess(process); /* it would be nice to kill with a sigusr signal */
}
}
#endif

void
beAGoodParent(int argc, char **argv, int maxProcs, PRFileDesc *listen_sock)
{
Expand All @@ -2134,6 +2148,19 @@ beAGoodParent(int argc, char **argv, int maxProcs, PRFileDesc *listen_sock)
PRInt32 exitCode;
PRStatus rv;

#ifdef XP_UNIX
struct sigaction act;

/* set up the signal handler */
act.sa_handler = sigusr1_parent_handler;
sigemptyset(&act.sa_mask);
act.sa_flags = 0;
if (sigaction(SIGUSR1, &act, NULL)) {
fprintf(stderr, "Error installing signal handler.\n");
exit(1);
}
#endif

rv = PR_SetFDInheritable(listen_sock, PR_TRUE);
if (rv != PR_SUCCESS)
errExit("PR_SetFDInheritable");
Expand Down Expand Up @@ -2588,7 +2615,8 @@ main(int argc, char **argv)
exit(14);
}

if (pidFile) {
envString = PR_GetEnvSecure(envVarName);
if (!envString && pidFile) {
FILE *tmpfile = fopen(pidFile, "w+");

if (tmpfile) {
Expand All @@ -2613,13 +2641,6 @@ main(int argc, char **argv)
if (!tmp)
tmp = PR_GetEnvSecure("TEMP");

/* Call the NSS initialization routines */
rv = NSS_Initialize(dir, certPrefix, certPrefix, SECMOD_DB, NSS_INIT_READONLY);
if (rv != SECSuccess) {
fputs("NSS_Init failed.\n", stderr);
exit(8);
}

if (envString) {
/* we're one of the children in a multi-process server. */
listen_sock = PR_GetInheritedFD(inheritableSockName);
Expand All @@ -2642,6 +2663,12 @@ main(int argc, char **argv)
if (rv != SECSuccess)
errExit("SSL_InheritMPServerSIDCache");
hasSidCache = PR_TRUE;
/* Call the NSS initialization routines */
rv = NSS_Initialize(dir, certPrefix, certPrefix, SECMOD_DB, NSS_INIT_READONLY);
if (rv != SECSuccess) {
fputs("NSS_Init failed.\n", stderr);
exit(8);
}
} else if (maxProcs > 1) {
/* we're going to be the parent in a multi-process server. */
listen_sock = getBoundListenSocket(port);
Expand All @@ -2652,6 +2679,12 @@ main(int argc, char **argv)
beAGoodParent(argc, argv, maxProcs, listen_sock);
exit(99); /* should never get here */
} else {
/* Call the NSS initialization routines */
rv = NSS_Initialize(dir, certPrefix, certPrefix, SECMOD_DB, NSS_INIT_READONLY);
if (rv != SECSuccess) {
fputs("NSS_Init failed.\n", stderr);
exit(8);
}
/* we're an ordinary single process server. */
listen_sock = getBoundListenSocket(port);
prStatus = PR_SetFDInheritable(listen_sock, PR_FALSE);
Expand Down
15 changes: 13 additions & 2 deletions lib/ssl/sslsnce.c
Expand Up @@ -276,6 +276,17 @@ typedef struct inheritanceStr inheritance;

/************************************************************************/

/* SSL Session Cache has a smaller set of functions to initialize than
* ssl does. some ssl_functions can't be initialized before NSS has been
* initialized, and the cache may be configured before NSS is initialized
* so thus the special init function */
static SECStatus
ssl_InitSessionCache()
{
/* currently only one function, which is itself idempotent */
return ssl_InitializePRErrorTable();
}

/* This is used to set locking times for the cache. It is not used to set the
* PRTime attributes of sessions, which are driven by ss->now(). */
static PRUint32
Expand Down Expand Up @@ -1165,7 +1176,7 @@ ssl_ConfigServerSessionIDCacheInstanceWithOpt(cacheDesc *cache,
{
SECStatus rv;

rv = ssl_Init();
rv = ssl_InitSessionCache();
if (rv != SECSuccess) {
return rv;
}
Expand Down Expand Up @@ -1341,7 +1352,7 @@ SSL_InheritMPServerSIDCacheInstance(cacheDesc *cache, const char *envString)
int locks_initialized = 0;
int locks_to_initialize = 0;
#endif
SECStatus status = ssl_Init();
SECStatus status = ssl_InitSessionCache();

if (status != SECSuccess) {
return status;
Expand Down
10 changes: 10 additions & 0 deletions tests/ssl/ssl.sh
Expand Up @@ -927,8 +927,18 @@ ssl_policy_selfserv()
# Disallow RSA in key exchange explicitly
setup_policy "disallow=rsa/ssl-key-exchange" ${P_R_SERVERDIR}

SAVE_SERVER_OPTIONS=${SERVER_OPTIONS}
# make sure policy is working in the multiprocess case is working on
# UNIX-like OS's. Other OS's can't properly clean up the child processes
# when our test suite kills the parent, so just use the single process
# self serve for them
if [ "${OS_ARCH}" != "WINNT" -a "${OS_ARCH}" != "WIN95" -a "${OS_ARCH}" != "OS2" ]; then
SERVER_OPTIONS="-M 3 ${SERVER_OPTIONS}"
fi

start_selfserv $CIPHER_SUITES

SERVER_OPTIONS="${SAVE_SERVER_OPTIONS}"
VMIN="ssl3"
VMAX="tls1.2"

Expand Down

0 comments on commit 108aa43

Please sign in to comment.