From a0de3d158c033679146bc1de13091a42526c4f36 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Fri, 31 Oct 2014 09:41:36 +0000 Subject: [PATCH] Stop calling setenv() from JNI code If there's ever an environment that's likely to be multi-threaded, it's Java. So we really can't use setenv() directly. Instead, add a new openconnect_set_csd_environ() function and set the environment variables only after fork(). With a hack to make it look for a $TMPDIR setting because that gets used *before* forking, for the download location for the script. Signed-off-by: David Woodhouse --- http.c | 30 ++++++++++++++++++++++++++++-- jni.c | 7 ++----- libopenconnect.map.in | 1 + library.c | 41 +++++++++++++++++++++++++++++++++++++++++ openconnect-internal.h | 3 ++- openconnect.h | 14 ++++++++++++-- script.c | 8 ++++---- tun.c | 2 +- 8 files changed, 91 insertions(+), 15 deletions(-) diff --git a/http.c b/http.c index 2e7ce09f..4f6eed10 100644 --- a/http.c +++ b/http.c @@ -714,8 +714,32 @@ static int run_csd_script(struct openconnect_info *vpninfo, char *buf, int bufle fname[0] = 0; if (buflen) { - char *tmpdir = getenv("TMPDIR"); - snprintf(fname, 64, "%s/csdXXXXXX", tmpdir ? tmpdir : "/tmp"); + struct oc_vpn_option *opt; + const char *tmpdir = NULL; + + /* If the caller wanted $TMPDIR set for the CSD script, that + means for us too; look through the csd_env for a TMPDIR + override. */ + for (opt = vpninfo->csd_env; opt; opt = opt->next) { + if (!strcmp(opt->option, "TMPDIR")) { + tmpdir = opt->value; + break; + } + } + if (!opt) + tmpdir = getenv("TMPDIR"); + + if (!tmpdir && !access("/var/tmp", W_OK)) + tmpdir = "/var/tmp"; + if (!tmpdir) + tmpdir = "/tmp"; + + if (access(tmpdir, W_OK)) + vpn_progress(vpninfo, PRG_ERR, + _("Temporary directory '%s' is not writable: %s\n"), + tmpdir, strerror(errno)); + + snprintf(fname, 64, "%s/csdXXXXXX", tmpdir); fd = mkstemp(fname); if (fd < 0) { int err = -errno; @@ -803,6 +827,8 @@ static int run_csd_script(struct openconnect_info *vpninfo, char *buf, int bufle if (setenv("CSD_HOSTNAME", vpninfo->hostname, 1)) goto out; + apply_script_env(vpninfo->csd_env); + execv(csd_argv[0], csd_argv); out: diff --git a/jni.c b/jni.c index 90b8836c..0698f854 100644 --- a/jni.c +++ b/jni.c @@ -804,11 +804,8 @@ JNIEXPORT void JNICALL Java_org_infradead_libopenconnect_LibOpenConnect_setCSDWr !get_cstring(ctx->jenv, jarg2, &arg2)) { openconnect_setup_csd(ctx->vpninfo, getuid(), 1, arg0); - if (arg1) - setenv("TMPDIR", arg1, 1); - - if (arg2) - setenv("PATH", arg2, 1); + openconnect_set_csd_environ(ctx->vpninfo, "TMPDIR", arg1); + openconnect_set_csd_environ(ctx->vpninfo, "PATH", arg2); } release_cstring(ctx->jenv, jarg0, arg0); diff --git a/libopenconnect.map.in b/libopenconnect.map.in index 87e6f9e4..b241fd8b 100644 --- a/libopenconnect.map.in +++ b/libopenconnect.map.in @@ -61,6 +61,7 @@ OPENCONNECT_4.1 { openconnect_set_system_trust; openconnect_get_dtls_cipher; openconnect_get_cstp_cipher; + openconnect_set_csd_environ; } OPENCONNECT_4.0; diff --git a/library.c b/library.c index c0f2a1ce..24f513f3 100644 --- a/library.c +++ b/library.c @@ -178,6 +178,7 @@ void openconnect_vpninfo_free(struct openconnect_info *vpninfo) CloseHandle(vpninfo->dtls_event); #endif free(vpninfo->peer_addr); + free_optlist(vpninfo->csd_env); free_optlist(vpninfo->script_env); free_optlist(vpninfo->cookies); free_optlist(vpninfo->cstp_options); @@ -215,6 +216,7 @@ void openconnect_vpninfo_free(struct openconnect_info *vpninfo) free(vpninfo->csd_starturl); free(vpninfo->csd_waiturl); free(vpninfo->csd_preurl); + if (vpninfo->opaque_srvdata) xmlFreeNode(vpninfo->opaque_srvdata); free(vpninfo->profile_url); @@ -686,3 +688,42 @@ const char *openconnect_get_dtls_cipher(struct openconnect_info *vpninfo) return vpninfo->dtls_cipher; #endif } + +int openconnect_set_csd_environ(struct openconnect_info *vpninfo, + const char *name, const char *value) +{ + struct oc_vpn_option *p; + + if (!name) { + free_optlist(vpninfo->csd_env); + vpninfo->csd_env = NULL; + return 0; + } + for (p = vpninfo->csd_env; p; p = p->next) { + if (!strcmp(name, p->option)) { + char *valdup = strdup(value); + if (!valdup) + return -ENOMEM; + free(p->value); + p->value = valdup; + return 0; + } + } + p = malloc(sizeof(*p)); + if (!p) + return -ENOMEM; + p->option = strdup(name); + if (!p->option) { + free(p); + return -ENOMEM; + } + p->value = strdup(value); + if (!p->value) { + free(p->option); + free(p); + return -ENOMEM; + } + p->next = vpninfo->csd_env; + vpninfo->csd_env = p; + return 0; +} diff --git a/openconnect-internal.h b/openconnect-internal.h index 4acbd53c..9bc86dc5 100644 --- a/openconnect-internal.h +++ b/openconnect-internal.h @@ -290,6 +290,7 @@ struct openconnect_info { struct oc_vpn_option *dtls_options; struct oc_vpn_option *script_env; + struct oc_vpn_option *csd_env; unsigned pfs; #if defined(OPENCONNECT_OPENSSL) @@ -566,7 +567,7 @@ int script_setenv(struct openconnect_info *vpninfo, const char *opt, const char int script_setenv_int(struct openconnect_info *vpninfo, const char *opt, int value); void prepare_script_env(struct openconnect_info *vpninfo); int script_config_tun(struct openconnect_info *vpninfo, const char *reason); -int apply_script_env(struct openconnect_info *vpninfo); +int apply_script_env(struct oc_vpn_option *envs); /* tun.c / tun-win32.c */ void os_shutdown_tun(struct openconnect_info *vpninfo); diff --git a/openconnect.h b/openconnect.h index d611103b..df407186 100644 --- a/openconnect.h +++ b/openconnect.h @@ -34,7 +34,7 @@ /* * API version 4.1: * - Add openconnect_get_cstp_cipher(), openconnect_get_dtls_cipher(), - * openconnect_set_system_trust(). + * openconnect_set_system_trust(), openconnect_set_csd_environ(). * - Change openconnect_init_ssl() to return int. * * API version 4.0: @@ -60,7 +60,7 @@ * openconnect_get_ifname(), openconnect_set_reqmtu(), * openconnect_get_ip_info(), openconnect_set_protect_socket_handler(), * openconnect_set_mobile_info(), openconnect_set_xmlpost(), - * openconnect_set_stats_handler() +7 * openconnect_set_stats_handler() * * API version 3.0: * - Change oc_form_opt_select->choices to an array of pointers @@ -283,6 +283,16 @@ typedef enum { of the provided strings. */ +/* Provide environment variables to be set in the CSD trojan environment + before spawning it. Some callers may need to set $TMPDIR, $PATH and + other such things if not running from a standard UNIX-like environment. + To ensure that a variable is unset, pass its name with value==NULL. + To clear all settings and allow the CSD trojan to inherit an unmodified + environment, call with name==NULL. */ + +int openconnect_set_csd_environ(struct openconnect_info *vpninfo, + const char *name, const char *value); + /* The buffer 'buf' must be at least 41 bytes. It will receive a hex string with trailing NUL, representing the SHA1 fingerprint of the certificate. */ int openconnect_get_cert_sha1(struct openconnect_info *vpninfo, diff --git a/script.c b/script.c index e022893f..dda79245 100644 --- a/script.c +++ b/script.c @@ -474,11 +474,11 @@ int script_config_tun(struct openconnect_info *vpninfo, const char *reason) } #else /* Must only be run after fork(). */ -int apply_script_env(struct openconnect_info *vpninfo) +int apply_script_env(struct oc_vpn_option *envs) { - struct oc_vpn_option *p = vpninfo->script_env; + struct oc_vpn_option *p; - for (p = vpninfo->script_env; p; p = p->next) { + for (p = envs; p; p = p->next) { if (p->value) setenv(p->option, p->value, 1); else @@ -500,7 +500,7 @@ int script_config_tun(struct openconnect_info *vpninfo, const char *reason) /* Child */ char *script = openconnect_utf8_to_legacy(vpninfo, vpninfo->vpnc_script); - apply_script_env(vpninfo); + apply_script_env(vpninfo->script_env); setenv("reason", reason, 1); diff --git a/tun.c b/tun.c index d65d23bc..9d1db420 100644 --- a/tun.c +++ b/tun.c @@ -475,7 +475,7 @@ int openconnect_setup_tun_script(struct openconnect_info *vpninfo, vpn_perror(vpninfo, _("setpgid")); close(fds[0]); script_setenv_int(vpninfo, "VPNFD", fds[1]); - apply_script_env(vpninfo); + apply_script_env(vpninfo->script_env); execl("/bin/sh", "/bin/sh", "-c", vpninfo->vpnc_script, NULL); vpn_perror(vpninfo, _("execl")); exit(1);