Call daemon() before initializing crypto library
PATCH 1/1 Call daemon() before initializing crypto library
But keep the chdir to / at the place where deamon() was before, to preserve the current behaviour wrt relative paths in the config.
This should fix the issue reported in trac #480, without changing the behaviour visible to the end user.
Note that by moving the daemon() call to an earlier stage of the init process, we no longer have to call platform_mlockall() again, or do a pkcs11_forkFixup().
--- src/openvpn/init.c | 32 +++++++++++--------------------- src/openvpn/init.h | 2 ++ src/openvpn/openvpn.c | 4 ++++ src/openvpn/pkcs11.c | 5 ----- src/openvpn/pkcs11.h | 3 --- 5 files changed, 17 insertions(+), 29 deletions(-)
src/openvpn/init.c +11 -21
@@ -916,23 +916,20 @@ do_persist_tuntap (const struct options *options)* Should we become a daemon? * Return true if we did it. */ -static bool +bool possibly_become_daemon (const struct options *options) {bool ret = false;if (options->daemon) {ASSERT (!options->inetd); -if (daemon (options->cd_dir !=NULL , options->log) <0 ) +/* Don't chdir immediately, but the end of the init sequence, if needed */ +if (daemon (1 , options->log) <0 )msg (M_ERR ,"daemon() failed or unsupported" );restore_signal_state ();if (options->log)set_std_files_to_null (true); -#if defined(ENABLE_PKCS11 ) -pkcs11_forkFixup (); -#endif - ret = true; }return ret;
@@ -1809,15 +1806,11 @@ do_deferred_options (struct context *c, const unsigned int found)* Possible hold on initialization */ static bool -do_hold (struct context *c) +do_hold (void ) {#ifdef ENABLE_MANAGEMENT if (management) { -/* if c is defined, daemonize before hold */ -if (c && c->options.daemon &&management_should_daemonize (management)) -do_init_first_time (c); -/* block until management hold is released */ if (management_hold (management))return true;
@@ -1867,7 +1860,7 @@ socket_restart_pause (struct context *c) c->persist.restart_sleep_seconds =0 ;/* do managment hold on context restart, i.e. second, third, fourth, etc. initialization */ -if (do_hold (NULL )) +if (do_hold ()) sec =0 ;if (sec)
@@ -1886,7 +1879,7 @@ do_startup_pause (struct context *c)if (!c->first_time)socket_restart_pause (c);else -do_hold (NULL );/* do management hold on first context initialization */ +do_hold ();/* do management hold on first context initialization */ }/*
@@ -2743,7 +2736,7 @@ do_compute_occ_strings (struct context *c)static void do_init_first_time (struct context *c) { -if (c->first_time && !c->did_we_daemonize && !c->c0) +if (c->first_time && !c->c0) {struct context_0 *c0;
@@ -2758,12 +2751,9 @@ do_init_first_time (struct context *c)/* get --writepid file descriptor */ get_pid_file (c->options.writepid, &c0->pid_state); -/* become a daemon if --daemon */ - c->did_we_daemonize =possibly_become_daemon (&c->options); - -/* should we disable paging? */ -if (c->options.mlock && c->did_we_daemonize) -platform_mlockall (true);/* call again in case we daemonized */ +/* perform postponed chdir if --daemon */ +if (c->did_we_daemonize && c->options.cd_dir ==NULL ) +platform_chdir ("/" );/* save process ID in a file */ write_pid (&c0->pid_state);
@@ -3221,7 +3211,7 @@ open_management (struct context *c) }/* initial management hold, called early, before first context initialization */ -do_hold (c); +do_hold ();if (IS_SIG (c)) {msg (M_WARN ,"Signal received from management interface, exiting" );
src/openvpn/init.h +2 -0
@@ -55,6 +55,8 @@ bool do_genkey (const struct options *options);bool do_persist_tuntap (const struct options *options); +bool possibly_become_daemon (const struct options *options); +void pre_setup (const struct options *options);void init_instance_handle_signals (struct context *c,const struct env_set *env,const unsigned int flags);
src/openvpn/openvpn.c +4 -0
@@ -229,6 +229,10 @@ openvpn_main (int argc, char *argv[])if (do_test_crypto (&c.options))break ; +/* become a daemon if --daemon */ +if (c.first_time) + c.did_we_daemonize =possibly_become_daemon (&c.options); +#ifdef ENABLE_MANAGEMENT /* open management subsystem */ if (!open_management (&c))
src/openvpn/pkcs11.c +0 -5
@@ -336,11 +336,6 @@ pkcs11_terminate () { ); } -void -pkcs11_forkFixup () { -pkcs11h_forkFixup (); -} - bool pkcs11_addProvider ( const char * const provider,
src/openvpn/pkcs11.h +0 -3
@@ -38,9 +38,6 @@ pkcs11_initialize (void pkcs11_terminate (); -void -pkcs11_forkFixup (); - bool pkcs11_addProvider ( const char * const provider,
Signature · 1 line
2.1.4
Hi,
Quoted 9 lines · Steffan Karger
But keep the chdir to / at the place where
deamon()was before, to preserve the current behaviour wrt relative paths in the config.This should fix the issue reported in trac #480, without changing the behaviour visible to the end user.
Note that by moving the
daemon()call to an earlier stage of the init process, we no longer have to callplatform_mlockall()again, or do apkcs11_forkFixup().
I think this is good, so both feature-ACK and code-ACK from me (after staring at the patch and the doxygen generated call graphs for nearly an hour... whee is this a twisty maze of passages, all alike...).
The overamazing thing is that this actually makes the code *more easy* in a number of places :-)
I'd still like to hear feedback from the reporter on #480 if it actually solves their issue (though I'm confident it does).
gert
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany [email protected]...
fax: +49-89-35655025 [email protected]...ACK.
Your patch has been applied to the master and release/2.3 branch.
(I spent quite some thought on whether this is 2.3 material - and decided "yes it is". While it is a complicated patch, I'm fairly sure it has no adverse side effects - and it *does* fix a real and observed problem, has been tested good by the FreeBSD folks affected, and gets rid of some extra convolutions that should have never been there - so, blame me if it breaks anything, I'll be doing the tracs for 2.3.8 anyway...)
commit da9b292733e929a2900dc32d37f0424c3d588366 (master) commit f025de005d719201a69ad0313d545a1ddd244752 (release/2.3)
Author: Steffan Karger Date: Mon Apr 27 16:28:57 2015 +0200
Call daemon() before initializing crypto library
Signed-off-by: Steffan Karger <[email protected]...>
Acked-by: Gert Doering <[email protected]...>
Message-Id: <[email protected]...>
URL: http://article.gmane.org/gmane.network.openvpn.devel/9609
Signed-off-by: Gert Doering <[email protected]...>--
kind regards,
Gert Doering