Skip to content

Commit

Permalink
Stop calling setenv() from JNI code
Browse files Browse the repository at this point in the history
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 <David.Woodhouse@intel.com>
  • Loading branch information
David Woodhouse authored and David Woodhouse committed Oct 31, 2014
1 parent f30e4ae commit a0de3d1
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 15 deletions.
30 changes: 28 additions & 2 deletions http.c
Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand Down
7 changes: 2 additions & 5 deletions jni.c
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions libopenconnect.map.in
Expand Up @@ -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;


Expand Down
41 changes: 41 additions & 0 deletions library.c
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
3 changes: 2 additions & 1 deletion openconnect-internal.h
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down
14 changes: 12 additions & 2 deletions openconnect.h
Expand Up @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions script.c
Expand Up @@ -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
Expand All @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion tun.c
Expand Up @@ -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);
Expand Down

0 comments on commit a0de3d1

Please sign in to comment.