Right, sorry it's taken a few days to get round to this, it's been a bit
hectic here (and the heat breaks my concentration badly).
On Thu, Jun 25, 2009 at 12:40:34AM +0000, Robert Edmonds wrote:
> On 2009-06-24, George Danchev <danchev@spnet.net> wrote:
> > GVPE looks like a fork of tinc which is already in Debian (or at least shares
> > some code with it), and its source tree carries libev (by the same author)
> > instead of linking with the libev library provided as a separate package and
> > already uploaded in Debian. Unfortunately, code dups, also means (security)
> > bugs dups, like that conditional `devision by zero' in ev_select.c line 105
> > which seems to be windows-specific (NFBITS previously and conditionally defined
> > as 0).
>
> gvpe definitely isn't a fork of tinc, it merely uses tinc's
> platform-specific tun/tap interface code (about 200 lines for
> linux/kfreebsd). the crypto code is completely different and in C++.
Yes, I realised there was some overlap but not a complete copy of tinc.
It's less than desirable, but not avoidable IMO.
> and unfortunately the embedded copy of libev is compiled with
> EV_MULTIPLICITY set to 0 (libev in debian is compiled with the default
> setting of 1), which changes the API/ABI. (it controls whether an
> additional parameter is required by function calls or passed to
> callbacks.)
And likewise, embedding libev isn't great either, but there is a good
reason.
I've emailed upstream to see whether he'd be open to reworking these
problems sometime.
I worked through the diff of your proposed changes and integrated almost
all of them - if I'm right, lots are formatting and style, some cruft
removal, and some technical. The file gvpe.init particularly came mostly
from the debhelper template, which is why it was weaker.
The bits I'm left with are the ones I don't quite follow, here as a
diff with annotations:
init.d:
@@ -31,16 +31,11 @@
# merely create a single tunnel, it creates a real
# network with multiple endpoints.
### END INIT INFO
-PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin
-
-DAEMON=/usr/sbin/gvpe
NAME=gvpe
DESC="GNU Virtual Private Ethernet daemon"
-LOGDIR=/var/log/gvpe
-
PIDFILE=/var/run/$NAME.pid
-test -x $DAEMON || exit 0
+test -x /usr/bin/gvpectrl || exit 0
Ok, I missed the fact that the PATH variable has wrong paths in (oops)
so that can go in. But why remove the declaration of $NAME and move it
further down?
I take it the $LOGDIR variable goes because it doesn't get
used - I assumed that log_daemon_msg and friends used it, probably a bad
thing to assume.
Why test that /usr/bin/gvpectrl is executable but not $DAEMON?
@@ -56,8 +51,6 @@
CIPHER="aes128"
DIGEST="sha256"
-LOGFILE=$LOGDIR/$NAME.log # Server logfile
-
Likewise, I presume $LOGFILE doesn't get used.
@@ -73,8 +66,7 @@
# Use this if you want the user to explicitly set 'RUN' in
# /etc/default/
if [ "x$RUN" != "xyes" ]; then
- log_failure_msg "$NAME disabled, please adjust the configuration to
your needs "
- log_failure_msg "and then set RUN to 'yes' in /etc/default/$NAME to
enable it."
+ log_failure_msg "$NAME disabled in /etc/default/$NAME."
exit 1
Is this change to the output text a preference, or is there a style for
this sort of message that I've missed somewhere?
@@ -153,7 +145,7 @@
log_end_msg 0
exit 0
fi
- if start_server; then
+ if start_server ; then
I think you missed this one ;)
@@ -213,19 +205,11 @@
fi
;;
reload)
- log_daemon_msg "Reloading configuration for $DESC" "$NAME"
- if running; then
- [ -x /usr/sbin/gvpectrl ] && /usr/sbin/gvpectrl -kHUP
- log_end_msg 0
- else
- log_progress_msg "apparently not running"
- log_end_msg 1
- exit 1
- fi
+ log_warning_msg "Reloading $NAME daemon: not implemented (use
restart)."
;;
*)
N=/etc/init.d/$NAME
- echo "Usage: $N
{start|stop|force-stop|restart|reload|force-reload|status}" >&2
+ echo "Usage: $N
{start|stop|force-stop|restart|force-reload|status}" >&2
I only just today noticed that gvpectrl can send a HUP signal to the
daemon, and tested that it reloads the configuration properly, so I've
implemented reloading. Does it look correct?
debian/rules:
@@ -20,7 +20,7 @@
CROSS= --build $(DEB_BUILD_GNU_TYPE)
endif
-CONF=./configure $(CROSS) --prefix=/usr --includedir=$${prefix}/include
--mandir=\$${prefix}/share/man --infodir=\$${prefix}/share/info
--sysconfdir=/etc --localstatedir=/var --libexecdir=$${prefix}/lib/gvpe
--disable-maintainer-mode --disable-dependency-tracking --enable-dns
--enable-icmp --enable-rawip --enable-tcp --enable-udp
CFLAGS="$(CFLAGS)" LDFLAGS="-Wl,-z,defs"
+CONF=./configure $(CROSS) --prefix=/usr --includedir=$${prefix}/include
--mandir=\$${prefix}/share/man --infodir=\$${prefix}/share/info
--sysconfdir=/etc --localstatedir=/var --libexecdir=$${prefix}/lib/gvpe
--disable-maintainer-mode --enable-max-mtu=9100 CFLAGS="$(CFLAGS)"
LDFLAGS="-Wl,-z,defs"
I gathered from the configure script output that extra protocols, like
--enable-icmp and --enable-tcp and so on had to be explicitly enabled.
Certainly using your configure command they don't appear at the end of
the outpt summary; have I mis-interpreted this?
I agree that building multiple binaries, though a bit convoluted, makes
it easier to choose cipher preferences, so that makes sense. I wondered
about debconf questions for them, but that would be new ground for me -
do you think it a good idea? I guess it would be good to learn if
nothing else.
Thanks for the help, and sorry for so many questions!
Cheers,
Jonathan
--
Jonathan Wiltshire
1024D: 0xDB800B52 / 4216 F01F DCA9 21AC F3D3 A903 CA6B EA3E DB80 0B52
4096R: 0xD3524C51 / 0A55 B7C5 1223 3942 86EC 74C3 5394 479D D352 4C51
Attachment:
signature.asc
Description: Digital signature