Hi,
Here's the summary of today's IRC meeting.
---
COMMUNITY MEETING
Place: #openvpn-devel on irc.freenode.net
List-Post: [email protected]
Date: Monday 27th Jul 2015
Time: 20:00 CEST (18:00 UTC)
Planned meeting topics for this meeting were here:
<https://community.openvpn.net/openvpn/wiki/Topics-2015-07-27>
The next meeting is scheduled to two weeks from this meeting:
<https://community.openvpn.net/openvpn/wiki/Topics-2015-08-10>
Your local meeting time is easy to check from services such as
<http://www.timeanddate.com/worldclock>
SUMMARY
cron2, mattock, plaisthos and syzzer participated in this meeting
---
Discussed integrating cppcheck into our Buildbot. All were in favor.
---
Discussed the possible man-page issue reported by Jan Just Keijser:
<http://sourceforge.net/p/openvpn/mailman/message/34249691/>
Meeting participants believe there's no error on the man-page and thus
no need for action.
---
Discussed the OpenVPN 2.3.8 release. The goal is to release it next
Monday. Any tickets with milestone 2.3.8 not fixed by that time wlil be
moved to milestone 2.3.9.
---
Discussed the "Provide OpenVPN version information to plug-ins" patchset:
<http://thread.gmane.org/gmane.network.openvpn.devel/9906>
The patchset was ACKed and merged. However, a nasty bug was found in
more thorough build testing. This bug was fixed and merged immediately
in the meeting.
It was noted that the value of PRODUCT_VERSION_PATCH is not optimal
right now. For example, for 2.4.0 it would translate to ".0" and for Git
"master" it would become "_git". However, it was also agreed that this
issue is not related to this patchset and can be fixed later.
---
Discussed the "Fix overflow check in openvpn_decrypt()" patch:
<http://article.gmane.org/gmane.network.openvpn.devel/9842>
This patch has not received an ACK yet. We asked jamesyonan and
plaisthos to review it.
--
Full chatlog is included as an attachment.
--
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc
irc freenode net: mattock
(21:04:08) syzzer: mattock: I think there's enough on the agenda :)
(21:04:31) mattock: probably :)
(21:04:37) mattock: cron2: you here?
(21:04:41) cron2: no
(21:05:36) cron2: first bullet: yes, all for it. The catch is (as with
coverity) - "someone needs to actually look at the results"
(21:05:47) ***cron2 sort of volunteers
(21:06:28) mattock: ok, topic #1 covered
(21:06:43) syzzer: nice
(21:06:45) mattock: we've managed to look at buildbot warnings, so these
cppcheck warnings/errors would not be any different
(21:06:55) syzzer: I'll look into coverity in the mean while
(21:06:59) mattock: coverity, on the other hand, is an external service that
needs pampering
(21:07:00) syzzer: looks very promising too
(21:07:34) mattock: yeah, if it can (now) track a Git repo then it would be
quite useful
(21:07:41) mattock: it produces lots of noise though
(21:07:46) mattock: topic #2?
(21:08:07) mattock: http://sourceforge.net/p/openvpn/mailman/message/34249691/
(21:09:28) mattock: I don't think that email from Jan got any response
(21:09:42) mattock: unless it was discussed here
(21:09:42) cron2: no, but I think I understand what the manpage is trying to say
(21:09:58) cron2: --verify-x509-name is subject to different --compat-names
rules than --tls-remote
(21:10:03) cron2: but they server similar purposes
(21:11:08) mattock: ok, so nothing to fix then... I also think the man-page is
pretty clear
(21:11:33) mattock: move on to topic #3: 2.3.8 release (when, what else to
include)?
(21:11:47) cron2: --tls-remote actually says "this is deprecated, go use
--verify-x509-name"
(21:13:08) cron2: on 2.3.8 - we have a number of important bugfixes in there,
and should release it "soonish". Of course there's tons of bugs with milestone
2.3.8, but there seems to be little traction right now in working on them, so
I'd do 2.3.8 soon, and bump the bugs to 2.3.9 then
(21:13:31) mattock: +1
(21:13:40) syzzer: agreed
(21:13:41) cron2: so, when could you do a release?
(21:14:10) mattock: I'm traveling and I had to turn my server off, but next
Monday would work
(21:14:37) cron2: that's not soonish :-) - that is "NEXT WEEK!"
(21:14:41) mattock: :D
(21:14:52) cron2: ok, so I'll tag and push as soon as you tell me to - I'm
around
(21:15:12) cron2: maybe someone feels like fixing a bug in the meantime :-)
(21:15:20) syzzer: I think plaisthos patch of today should go in
(21:15:24) mattock: (I need the server to produce Debian packages)
(21:15:37) cron2: what did plaisthos send?
(21:15:39) ***cron2 looks
(21:15:42) syzzer: 2.3.8 will be a 'fix stuff syzzer broke' release :')
(21:16:17) mattock: syzzer: well, you're being doing important work in a
codebase that is, as James says, "entangled"
(21:16:18) cron2: shall we introduce nicknames? :)
(21:16:41) mattock: Breaky Brunch?
(21:17:04) mattock: uh, I'm not so sure :P
(21:17:18) cron2: so what plaisthos found is a corner case with a certificate
that is *just* the right lenght in bytes and then breaks sanitation later on?
(21:17:37) cron2: besides, looks reasonable
(21:17:39) syzzer: it will break the parsing
(21:18:03) syzzer: so a certificate with *just* the right size won't work
(21:18:11) cron2: yeah, that's what I meant
(21:18:14) syzzer: no security issue at all, but still a bit nasty
(21:18:19) cron2: is that an ACK?
(21:18:30) cron2: (I'll remove the extra blank line...)
(21:18:49) syzzer: yes, that mail was meant as an ACK :)
(21:19:24) cron2: haven't seen "that mail" yet :) - my mail server was down due
to power outage and mail seems to be slightl slow with spamassassin fighting
lack of RAM, or such
(21:19:53) syzzer: hmm
(21:19:57) syzzer: might be my fault
(21:20:13) syzzer: yep, sent it so plaisthos only
(21:20:17) syzzer: I'll resend
(21:20:19) cron2: lol
(21:22:16) mattock: btw. tickets for milestone 2.3.8:
https://community.openvpn.net/openvpn/query?status=accepted&status=assigned&status=new&status=reopened&milestone=release+2.3.8&col=id&col=summary&col=status&col=owner&col=type&col=priority&col=milestone&order=priority
(21:22:23) vpnHelper: Title: Custom Query – OpenVPN Community (at
community.openvpn.net)
(21:23:03) vpnHelper: RSS Update - gitrepo: reintroduce md5_digest wrapper
struct to fix gcc warnings
<https://github.com/OpenVPN/openvpn/commit/2dd6501e3d679046a1ed488f22d62defdf737cf3>
(21:24:41) mattock: has anyone seen dazo or is he completely on vacation?
(21:25:08) mattock: he promised to review Tim's patches at some point, and many
of the to-be-reviewed patches are from dazo
(21:27:01) syzzer: I think Tim mentioned dazo didn't manage to do the review
before his vacantion
(21:27:06) syzzer: so it will take a bit longer
(21:27:24) syzzer: but we might be able to review some of dazo's patches
(21:28:39) mattock: shall we move on to those, or is there something about
2.3.8 we'd need to cover?
(21:28:51) cron2: 2.3.8 is covered "next monday"
(21:29:51) mattock: good
(21:29:55) mattock: so dazo's patches
(21:30:02) syzzer: looking at
http://thread.gmane.org/gmane.network.openvpn.devel/9906 now
(21:30:04) vpnHelper: Title: Gmane Loom (at thread.gmane.org)
(21:30:09) mattock: http://thread.gmane.org/gmane.network.openvpn.devel/9906
(21:30:16) syzzer: we should really bump master to 2.4_git I think
(21:30:18) mattock: lol :)
(21:30:25) mattock: +1
(21:30:29) syzzer: the 2.3_git is quite confusing
(21:31:08) cron2: +1
(21:31:27) TimSmall: mattock1: I got the impression that dazo was fully away on
family holiday for 1+ week.
(21:31:40) cron2: ecrist: could you give vpn-helper a slap, when time permits?
it's refusing to show git commits...
(21:32:06) mattock: hi TimSmall!
(21:32:33) mattock: TimSmall: I see... hopefully he'll hav some time after that
(21:33:48) vpnHelper: RSS Update - gitrepo: Fix commit e473b7c if an inline
file happens to have a line break exactly at buffer limit
<https://github.com/OpenVPN/openvpn/commit/d40cbf0e2601b35bfb1c0551c6f3907b5c5178ff>
(21:34:15) cron2: oh? that is... interesting
(21:34:27) cron2: and what branch is it monitoring now?
(21:34:52) cron2: this is a "master" commit, but it missed the one before
that...
(21:36:18) syzzer: ok, so I think 9906 looks fine, but I don't really like that
the patch level needs a dot in the variable...
(21:36:33) syzzer: (or an underscore)
(21:37:54) syzzer: but I guess we can change that in a follow-up commit if
needed. dazo's version sticks to what we have now nicely
(21:38:50) cron2: where's the dot?
(21:39:24) syzzer: he now does +define([PRODUCT_VERSION_PATCH], [_git])
(21:39:48) syzzer: which would become define([PRODUCT_VERSION_PATCH], [.0]) for
a x.y.0 release
(21:40:30) cron2: ah, ic
(21:40:55) mattock: there's probably no easy way around that
(21:41:06) mattock: even though I agree it's a bit nasty
(21:41:23) mattock: syzzer: so ACK to both of dazo's patches?
(21:41:33) cron2: well, if we insist on keep "2.3_git", no. If we make that
"2.4.git"...
(21:42:29) syzzer: exactly, but I'll treat that as a separate issue from dazo's
patches
(21:42:41) cron2: (but it would still be somewhat unclear what "2.4.git"
acutally is - "release/2.4"?
(21:42:45) syzzer: mattock1: for now I just looked at 1/2
(21:42:51) mattock: syzzer: ok
(21:42:55) syzzer: I'll send an ACK for that to the list
(21:43:39) cron2: PRODUCT_VERSION_PATCH actually gets replaced by a commit id
somewhere in the sausage machinery
(21:44:31) cron2: good enough for me
(21:48:10) mattock: how does patch 2/2 look?
(21:49:30) syzzer: looks good to me
(21:49:56) syzzer: though I don't fully grasp the plugin philosophy, but I
guess that's dazo's job
(21:50:27) cron2: interesting
(21:50:47) cron2: the patch has a file rename in it, which git is happy to do,
but when I do "git diff HEAD~1" I get the full "this file gone, this file new!"
output
(21:50:49) s7r [~s7r@openvpn/user/s7r] è entrato nella stanza.
(21:51:22) syzzer: yeah, not all tools seem to handle that equally well
(21:51:35) syzzer: gitk usually does get it
(21:52:14) cron2: ah, diff needs "-M" or a global switch. Fun
(21:54:37) syzzer: in 2/2, I don't get why the 'AC_DEFINE's follow the
'AC_SUBST's, instead of replacing them
(21:55:22) cron2: dazo commented on that in IRC
(21:55:43) cron2: AC_DEFINE is for one sort of thing, AC_SUBST for something
else (like, .h and "other text processing")
(21:56:24) syzzer: ah, okay
(21:57:00) cron2: I won't claim to be an autoconf guru, but that sort of makes
sense - one ends up in config.h, the other can be used for .h.in->.h
processing, or so
(21:57:23) cron2: (config.h is always generated, openvpn-plugin.h is now
text-edited from openvpn-plugin.h.in)
(21:58:38) syzzer: ok, sounds reasonable (and like you're in a better position
to ACK it than I am)
(22:02:12) mattock: dazo's patches covered then?
(22:02:24) cron2: first half
(22:02:58) syzzer: I am getting build errors now
(22:03:00) mattock: so no ACK yet for 2/2
(22:03:07) cron2: syzzer: what?
(22:03:09) syzzer: missing openvpn-plugin.h
(22:03:22) cron2: you'll need to autoreconf
(22:03:35) syzzer: looking into it, but this configure stuff is so sloooow
(22:03:48) cron2: I always thought this is my ancient machine :)
(22:04:13) syzzer: I did an "autoreconf -vi"
(22:04:42) syzzer: but I'm doing out-of-source builds, maybe that causes it
(22:05:56) vpnHelper: RSS Update - gitrepo: Provide OpenVPN runtime version
information to plug-ins
<https://github.com/OpenVPN/openvpn/commit/6a40276c7500c2d0a2fe44b1a450ffe9cb2f37cd>
|| Provide compile time OpenVPN version information to plug-ins
<https://github.com/OpenVPN/openvpn/commit/9de35d4633ce3035b048957b2e20b81e31a40cd6>
(22:06:25) cron2: running -vif now... sloowwww...
(22:06:53) cron2: (and I always build out of tree)
(22:09:43) cron2: config.status: creating include/openvpn-plugin.h
(22:11:10) syzzer: yes, but it does that in the build dir, and that include is
not in the -I list (at least for me)
(22:11:38) syzzer: don't you still have an openvpn-plugin.h in you source tree/
(22:12:07) cron2: ah
(22:12:32) cron2: no
(22:12:46) cron2: the version in the source tree got renamed to .h.in
(22:13:02) cron2: ./openvpn/include/openvpn-plugin.h.in
(22:13:02) cron2: ./test-build-master/include/openvpn-plugin.h
(22:14:41) syzzer: hmm, okay, and that does still work for you? must be
something in my setup than
(22:14:53) cron2: I do get a warning about src/openvpn/plugin.c, though
(22:14:58) cron2: ../../../openvpn/src/openvpn/plugin.c: In function
'plugin_open_item':
(22:14:58) cron2: ../../../openvpn/src/openvpn/plugin.c:393:53: warning: excess
elements in struct initializer [enabled by default] PACKAGE_VERSION, ^
(22:15:11) cron2: ... which hints at "it finds something, but not what it is
looking for"
(22:15:21) cron2: /usr/include/openvpn-plugin.h
(22:15:23) cron2: yeah
(22:15:24) cron2: not
(22:15:26) cron2: grrr
(22:15:50) syzzer: ah, okay, so it is the missing include
(22:16:02) cron2: ../../../openvpn/src/openvpn/plugin.h:38:28: fatal error:
openvpn-plugin.h: No such file or directory
(22:16:05) cron2: bam
(22:17:06) cron2: *grumble*... once for a change I just merge and push, and
then I get to revert...
(22:17:56) syzzer: or we'll just seek for the fix en commit that :)
(22:19:52) mattock: that might be a better idea
(22:20:19) mattock: at least if the fix is fairly trivial
(22:20:40) syzzer: well, it *is* autoconf...
(22:23:04) mattock: so autoconf -trivial
(22:26:34) cron2: some magic addition to a magic file to teach it "use
-I$(topdir)/include as well"... while that would be ugly (yet another
-I../../xxx on the command line) it would at least work...
(22:28:19) mattock: need we discuss this further today?
(22:28:40) cron2: well, master does not compile, so it would good to commit a
fix to that
(22:28:49) mattock: syzzer: thoughts?
(22:29:37) mattock: ah, lovely, buildslaves failing
(22:29:45) cron2:
http://stackoverflow.com/questions/20230827/how-to-set-include-paths-with-autotools
(22:29:46) vpnHelper: Title: autoconf - how to set include paths with autotools
- Stack Overflow (at stackoverflow.com)
(22:29:46) mattock: I agree this needs a quick fix or revert
(22:30:00) cron2: "I've read about 3 hours worth of documents and can't figure
it out"...
(22:30:02) cron2: yay :)
(22:30:32) syzzer: moving openvpn-plugin.h out of 'include' may work
(22:30:49) syzzer: it seems to work for config.h and config-version.h
(22:32:48) cron2: which implies moving + changing Makefile.am and
include/Makefile.am
(22:34:18) syzzer: aargh, this mess...
(22:34:44) mattock: (I'll be back in ~10 minutes, but I doubt I would be of
much help anyways)
(22:39:50) cron2: well, it's either "move openvpn-plugin.h.in upwards, adapt
Makefile.am, configure.ac, include/Makefile.am" or "add -I../../include to
AM_CPPFLAGS in Makefile.am"...
(22:39:59) cron2: both stink
(22:40:28) syzzer: the -I needs some reference to the build dir, right?
(22:41:04) cron2: well, you need to know on which level you are relativ to the
build top dir
(22:41:07) syzzer: ah, no, since pwd is in the build dir
(22:41:12) syzzer: right
(22:41:15) cron2: so in src/openvpn/Makefile, you'd add "../../include"
(22:41:21) syzzer: probably the best option
(22:41:33) cron2: and in the plugins directories, -I../../../include (if I'm
counting right)
(22:41:57) cron2: but the plugins actually built (but maybe that was due to the
old /usr/include file)
(22:43:16) syzzer: I think so, plugins bork for me too
(22:46:16) cron2: ok, my version seems to compile as well now. So what is "the
way to go"?
(22:47:16) syzzer: what is 'your version'/
(22:47:33) cron2: mv include/openvpn-plugin.h.in .
(22:47:45) cron2: adapt configure.ac, Makefile.am, include/Makefile.am to find
it there
(22:47:56) cron2: not add -I statements
(22:48:07) syzzer: fine by me
(22:48:54) syzzer: I can test and ACK as soon as it's on the list
(22:50:24) mattock: great!
(22:50:36) cron2: let me remove include/Makefile.am cmpletely and try rebuilding
(22:52:52) syzzer: ok... so I got yet another fix
(22:53:20) syzzer: move openvpn-plugin.h from AC_CONFIG_FILES to
AC_CONFIG_HEADERS
(22:53:26) syzzer: (in configure.ac)
(22:53:33) cron2: what?
(22:53:57) syzzer: it will automatically add the required -I../../../../../../
stuff
(22:54:09) syzzer: still clutters the command like, but not the Makefile.am
files
(22:54:24) cron2: so it's a one-liner, adding "include/openvpn-plugin.h" there?
(22:54:48) syzzer: two-liner: also removing it from AC_CONFIG_FILES
(22:54:49) cron2: well, two
(22:54:50) cron2: yeah
(22:54:59) syzzer: yep
(22:55:37) cron2: if that works, I think it's nicer... you send mail, I ack and
merge (and test :) )
(22:55:58) syzzer: http://pastebin.com/ze6ZQYSe
(22:56:08) syzzer: I'll send the mail
(22:56:51) ***cron2 waits for autoconf...
(22:59:13) BuddyButterfly ha abbandonato la stanza (quit: Quit: Leaving.).
(23:00:11) syzzer: on the list
(23:01:45) cron2: I really need a faster machine
(23:04:06) syzzer: throw in an ssd, it really helps
(23:04:11) mattock: +1
(23:04:30) mattock: I would not go back to a spinning disk anymore
(23:04:51) cron2: repeated configure/autoconf runs should all come from buffer
cache anyway...
(23:05:02) cron2: but not having a 5-year-old atom CPU would help :)
(23:05:15) cron2: (laptop has a SSD, configure is still slow...)
(23:05:20) mattock: definitely not :)
(23:07:05) cron2: so. Next topic!
(23:07:11) syzzer: whee
(23:07:11) mattock: yeah!
(23:07:17) mattock: did janjust resend his patches?
(23:07:32) mattock: new versions of them I mean
(23:07:34) syzzer: I would like to pitch the overflow patch
(23:08:00) syzzer: we really need to do something with Sebastians findings
(23:09:00) mattock: what's the status of the overflow patch exactly?
(23:09:16) mattock: this one?
http://article.gmane.org/gmane.network.openvpn.devel/9842
(23:09:18) vpnHelper: RSS Update - gitrepo: Fix out-of-tree builds;
openvpn-plugin.h should be in AC_CONFIG_HEADERS
<https://github.com/OpenVPN/openvpn/commit/0a51c4f152d0f695a6fb8b605dbfefca65e63838>
(23:09:18) syzzer: sent to the list for reveiw
(23:09:19) vpnHelper: Title: Gmane -- PATCH Fix overflow check in openvpn
decrypt (at article.gmane.org)
(23:09:26) syzzer: yes
(23:09:59) cron2: it would be nice to get an ACK from plaisthos here, he
understands the crypto subtleties better than I do
(23:10:18) mattock: maybe jamesyonan?
(23:10:31) cron2: yeah, but james seems to be as busy as usual
(23:11:15) mattock: I saw him pop in the company chat earlier today
(23:11:19) mattock: but he's not there anymore
(23:11:29) mattock: I can forward the email to him nevertheless
(23:11:39) mattock: he just might review it, as it's probably interesting to him
(23:11:50) syzzer: I do have a question regarding this patch. On second
thought, I think the decrypt code should have more ASSERT()s instead of
CRYPT_ERROR()s, at least if we are actually writing outside buffers (which, for
clarity, we currently do *not* do, it is just hardening)
(23:11:56) plaisthos: cron2: ack for what?
(23:12:01) plaisthos: I just arrived from sport
(23:12:05) mattock: ah, plaisthos, hi!
(23:12:17) cron2: ah, cool :)
(23:12:32) cron2: what does a CRYPT_ERROR() do?
(23:12:47) syzzer: 'return error, continue with next packet'
(23:13:11) plaisthos: cron2: my users did find the obscure cornercase and
provided me with a config that breaks
(23:13:12) cron2: well, it would be safe (since we check before overflowing),
right? But it might go unnoticed
(23:13:25) cron2: plaisthos: your users are great :-) - already ACKed and merged
(23:13:54) syzzer: there is the primary overflow check, which should be
CRYPT_ERROR() for decrypt to avoid DoS
(23:14:39) syzzer: but the 'secondary' checks I added as a hardening measure
should probably be ASSERT()s
(23:15:18) syzzer: those really are for the case we (I) screwed up the primary
check
(23:16:08) cron2: sounds reasonable
(23:17:58) mattock: I sent a mail to James
(23:17:58) syzzer: ok, I'll resend tomorrow. Don't have my work laptop with me.
(23:23:19) mattock: ok, anything else? it's getting quite late
(23:23:31) ***cron2 is game over
(23:23:36) ***syzzer too
(23:24:04) syzzer: well at least we finally got some of dazo's patches in :)
(23:24:34) cron2: yeah :)
(23:25:14) mattock: syzzer: what exactly will you resend tomorrow? (for the
summary)
(23:25:42) syzzer: the openvpn_decrypt() overflow check patch
(23:26:00) syzzer: http://article.gmane.org/gmane.network.openvpn.devel/9842
(23:26:01) vpnHelper: Title: Gmane -- PATCH Fix overflow check in openvpn
decrypt (at article.gmane.org)
(23:26:21) mattock: ic
(23:26:45) mattock: good enough for me, so good night guys!
(23:26:55) syzzer: good night!
(23:27:01) cron2: good night!
(23:27:07) cron2: (now why are my opensolaris builds fialing?)
(23:27:14) mattock: oh, next meeting in two weeks?
(23:27:19) cron2: yep
(23:27:31) mattock: great! mentioning that in the summary...
