[Openswan dev] init script feature request

[Harald Jenny]

On Fri, Nov 05, 2010 at 11:04:22AM +1000, David McCullough wrote:
> 
> Jivin Harald Jenny lays it down ...
> > Hi all,
> > 
> > I have had an interesting discussion on a Debian mailing list about a good way
> > to make a smooth transition from openswan 2.4 to 2.6 and somehow the issue was
> > raised to not allow pluto to start/restart/reload without proper configuration.
> > As looking into the current code shows that such a protection is already in
> > place for start but it's currently missing from restart, reload/force-reload
> > and condrestart/try-restart. What is the lists opinion on this matter? I
> > personally think it would be benefical for the userbase (for example someone
> > using the VPN to access the system itself).
> 
> Sounds reasonable to me,  at least it would be consistent then :-)

Seems like I was a bit too enthusiastic - the current code does use ipsec
addconn to parse the config variables but does not use it's return value:

programs/setup/setup.in:90
    variables=`ipsec addconn $IPSEC_CONFS/ipsec.conf --varprefix IPSEC --configsetup`
    eval $variables
    RETVAL=$?
    if [ $RETVAL != 0 ]
    then
         echo "Failed to parse config setup portion of ipsec.conf"
         exit $RETVAL
    fi

With the eval $variables statement the value of $? is set to the eval return
value not the ipsec addconn one. My suggestions would therefore be something
like this:

if test " $IPSEC_DIR" = " "     # if we were not called by the ipsec command
then
    # we must establish a suitable PATH ourselves
    PATH="${IPSEC_SBINDIR}":/sbin:/usr/sbin:/usr/local/bin:/bin:/usr/bin
    export PATH

    IPSEC_DIR="$IPSEC_LIBDIR"
    export IPSEC_DIR IPSEC_CONFS IPSEC_LIBDIR IPSEC_EXECDIR
fi

# Does not make any sense at all to continue without the main binary
test -x $IPSEC_SBINDIR/ipsec || exit 5
.
.
.
RETVAL=0

verify_config() {
    test -f $IPSEC_CONFS/ipsec.conf || exit 6

    config_error=`ipsec addconn --configsetup 2>&1 >/dev/null`
    RETVAL=$?
    if [ $RETVAL != 0 ]
    then
        echo "failed to start openswan IKE daemon - the following error occured:"
        echo $config_error
        exit $RETVAL
    fi
}


used like this:

start() {
    verify_config

    # Pick up IPsec configuration (until we have done this, successfully, we
    # do not know where errors should go, hence the explicit "daemon.error"s.)
    # Note the "--export", which exports the variables created.
    variables=`ipsec addconn $IPSEC_CONFS/ipsec.conf --varprefix IPSEC --configsetup`
    eval $variables

    IPSEC_confreadsection=${IPSEC_confreadsection:-setup}
    export IPSEC_confreadsection
.
.
.
restart() {
    verify_config
    stop
.
.
.
condrestart() {
    verify_config
    ipsec _realsetup status || exit 0
.
.
.
status() {
    ipsec _realsetup status


This would also have the side effect of making all functions resistant to ipsec
binary unavailability and unifying the code. Any objections against this?

> 
> -- 
> David McCullough,      david_mccullough at mcafee.com,  Ph:+61 734352815
> McAfee - SnapGear      http://www.mcafee.com         http://www.uCdot.org