OpenConnect 7.08 release

[Mike Miller]

On Wed, Dec 14, 2016 at 08:36:56 +0000, David Woodhouse wrote:
> Right. I was saying that we already create the .conf file at configure
> time, so we could create the .module file then too.
> 
> But then again... why? My proposal for discovering where SoftHSM2
> resides was just to look for an existing p11-kit module file in the
> system's configuration. And if we're going to require that the distro's
> packaging of SoftHSM is correct and installs it with a p11-kit .module
> file... then why do we need to bother messing with $HOME and providing
> our own in the first place?
> 
> Let's just drop the whole nonsense and rely on the system packaging
> SoftHSM2 correctly. Does that seem reasonable? I suppose we could also
> add a --with-softhsm2= configure argument, but I'd prefer not to.

Ok, I'll workaround locally for now and work to get the softhsm2 package
to pick up the slack.

> Without a comment explaining why it's done, that *is* going to get
> broken again in future. And I'm confused — what *is* the requirement?
> It can't be that dash doesn't let you set more than one environment
> variable at a time in the command line before the executable, because
> it's still setting both $HOME and $LD_PRELOAD. So what's going on? I'd
> quite like to see an explicit reference here to either the POSIX shell
> specification, or a filed dash bug.

The OPENCONNECT variable contains eval. According to POSIX, variable
assignments before a special built-in utility are made in the shell
environment, and are not necessarily exported to the enviroment of the
command.

When running this script

    #!/bin/sh
    checkenv() {
      env | egrep "^(A|B|C|D|E)="
    }
    A=foo; export A
    A=bar B=bar C=bar eval "D=bar E=bar checkenv"
    echo "In the shell, A=$A, B=$B, C=$C"

under bash, checkenv has A through E in its environment, and only A is
still in the shell's environment at the end with its original value.
This looks correct if you assume that eval is handled like any command.

In mksh, checkenv has A, D, and E in its environment, and the
assignments of A, B, and C persist in the shell (with only A marked as
exported). This looks correct according to the POSIX rules for special
built-in commands.

In dash, checkenv has only A in its environment. That it is missing D
and E may be a dash bug. The assignments of A, B, and C persist in the
shell as with mksh.

It looks like the safest option is to either declare these as bash
scripts, or to avoid any mixing of variable assignments and eval
commands. In fact, every openconnect call already has LD_PRELOAD on its
command line, so the "eval LD_PRELOAD=..." is redundant at this point.

Does this patch and commit message work better for you?


>From 6174d9063979e798d15c28049bf1b472cebec284 Mon Sep 17 00:00:00 2001
From: Mike Miller <mtmiller at debian.org>
Date: Wed, 14 Dec 2016 09:48:44 -0800
Subject: [PATCH] tests: avoid using eval with variable assignments

For shell portability, avoid using eval with variable assignments to set
openconnect's environment. Shell implementations vary on whether
variable assignments in front of eval are marked as environment
variables or just treated as ordinary shell assignments.

Every call to $OPENCONNECT already has LD_PRELOAD=libsocket_wrapper.so
in front of it, so the "eval LD_PRELOAD=libsocket_wrapper.so" was
redundant anyway.

Signed-off-by: Mike Miller <mtmiller at debian.org>
---
 tests/auth-pkcs11 | 2 +-
 tests/common.sh   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/auth-pkcs11 b/tests/auth-pkcs11
index adc40f5..5e9bab7 100755
--- a/tests/auth-pkcs11
+++ b/tests/auth-pkcs11
@@ -37,7 +37,7 @@ for TOKEN in ${pkcs11_tokens}; do
 	echo -n "Connecting to obtain cookie (token ${TOKEN} key ${KEY})... "
 	CERTURI="pkcs11:token=${TOKEN};${KEY};pin-value=1234"
 	( echo "test" | HOME=${srcdir} SOFTHSM2_CONF=softhsm2.conf LD_PRELOAD=libsocket_wrapper.so \
-			    $OPENCONNECT -q $ADDRESS:443 -u test -c \"${CERTURI}\" --key-password 1234 --servercert=d66b507ae074d03b02eafca40d35f87dd81049d3 --cookieonly --passwd-on-stdin ) ||
+			    $OPENCONNECT -q $ADDRESS:443 -u test -c "${CERTURI}" --key-password 1234 --servercert=d66b507ae074d03b02eafca40d35f87dd81049d3 --cookieonly --passwd-on-stdin ) ||
 	    fail $PID "Could not connect with token ${TOKEN} key ${KEY##*/}!"
     done
 done
diff --git a/tests/common.sh b/tests/common.sh
index d89cf83..6d5736a 100644
--- a/tests/common.sh
+++ b/tests/common.sh
@@ -31,7 +31,7 @@ mkdir -p $SOCKDIR
 export SOCKET_WRAPPER_DIR=$SOCKDIR
 export SOCKET_WRAPPER_DEFAULT_IFACE=2
 ADDRESS=127.0.0.$SOCKET_WRAPPER_DEFAULT_IFACE
-OPENCONNECT="eval LD_PRELOAD=libsocket_wrapper.so ${top_builddir}/openconnect"
+OPENCONNECT="${top_builddir}/openconnect"
 
 certdir="${srcdir}/certs"
 confdir="${srcdir}/configs"
-- 
2.10.2

-- 
mike