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