Here's the summary of the IRC meeting.



Place: #openvpn-meeting on irc.freenode.net
Date: Wednesday 28th November 2018
Time: 11:30 CET (10:30 UTC)

Planned meeting topics for this meeting were here:


The next meeting has not been scheduled yet.

Your local meeting time is easy to check from services such as



cron2, dazo, ordex, plaisthos and rozmansi participated in this meeting.


Talked about tap-windows6 HLK testing. Stephen Stair is working on it,
and it looks promising, though progress is a bit slow due to other
projects getting in the way.


Discussed OSTIF's proposal to security audit the changes between OpenVPN
2.4.0 and 2.5.0. Agreed that auditing the new code paths would be
beneficial. In particular these paths:

- tls-crypt v2
- vlan patches
- Refactored ifconfig handling

Agreed that the audit should target OpenVPN 2.5.0, i.e. the already
stabilized codebase.


Had a discussion about --fragment. Agreed that if we can fix internal
fragmentation without needing a change in frame format then we can
definitely deprecate --fragment in the long-term. Also noted that lack
of tun-mtu support on Windows might be for historic reasons and might
not apply to tap-windows6. Also questioned whether the default MTU of
1500 is the best possible choice.


Full chatlog attached.

Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock

<rozmansi> hello
<cron2> good morning
<cron2> so, meeting time!
<dazo> hey!
<lev__> hello
<rozmansi> hi
<cron2> looking at the agenda, we need mattock and ordex for most of it...
<cron2> I can do the tap-windows6 HLK testing update - "Stephen Stair is 
working on it, and it looks promising, though progress is a bit slow due to 
other projects getting in the way"
<cron2> tls-crypt-v2 is done (all patches related to this have been ACKed and 
<plaisthos> hello
<cron2> moin :)
<dazo> Actually, we have a couple things which needs to be discussed too ... 
but don't want to "steal" the time from other topics on the list, if they can 
be taken first
<dazo> (keywords: --fragment and --mssfix )
<cron2> well, we can discuss 1. and mattock can pick up from IRC log... 
(security-auditing, OSTIF)
<dazo> sounds good
<cron2> there will be quite a bit of new code - tls-crypt v2, vlan patches, 
async client-connect, sitnl, plus refactoring changes
<cron2> so I think an audit on those parts would be good
<cron2> focus on "new code", obviously :)
<dazo> in regards to sec audit of tls-crypt-v2 would be mostly appropriate, 
perhaps also vlan patches as well ... but not sure it makes much sense to have 
a security audit on async client-connect, sitnl and non-crypto related stuff
<dazo> it basically depends on what kind of sec audit we expect
<cron2> async client-connect maybe not so, but if you can trick a server-pushed 
ifconfig string into a bug in the sitnl stuff, that might be an attack angle 
against a client
<dazo> also considering that cov-scans have not revealed anything in the 
general code quality
<cron2> so everything that touches "network packets" or "coming from PUSH_REPLY"
<cron2> yes
<dazo> good point ... but iirc, the netlink API is fairly strict here, and that 
would more be a kernel issue than a sitnl issue
<dazo> ordex: ^^^ 
<cron2> for the netlink bits, yes, but since it's refactoring the whole 
ifconfig stuff, and we're still calling "sudo ifconfig $user_supplied_string" 
on other platforms...
<cron2> (I do not think there is an attack angle since we only pass 32bit IP 
addresses, not "strings")
<dazo> ahh, yeah, my scope was limited to netlink only ... but yeah, you're 
right on the the places we do execve() stuff
<plaisthos> tls crypt is a different beast from the rest
<ordex> sorry overlooked the meeting email
<plaisthos> the rest a "normal" auditor can do
<plaisthos> for tls-crypt you need someone you can understand well enough to 
also look if tls-crypt as crypto implementation is solid 
<plaisthos> "This xor encryption does not have any buffer overflows and also 
does not allow remote code execution" is pretty pointless :D
<dazo> :-P
<ordex> so we got a audit request from OSTIF (sorry I have missed that)
<dazo> iiuc, more like OSTIF asks us if it makes sense to do so
<ordex> oh ok
<ordex> we also have sitnl coming - I am wondering if it makes sense to wait 
for that ?
<ordex> and vlan patches too
<cron2> yes
<ordex> but I thinkit would make sense to also wait for some tetsing to be 
done, so maybe we can wait for when a full 2.5 release is out ?
<dazo> we know that 2.4.0 is good ... we don't believe we've done it (much) 
worse in the releases since that one .... so I think we can wait until 2.5.0 is 
out and then fix things as they appear
<cron2> +1
<ordex> +1
<ordex> also because by releaseing beta/rc versions we might have bug report 
from users, which we can fix beforehand and avoid wasting ostif time on that. 
i.e. OSTIF should join after the "stabilization" phase. so what dazo says :)
<ordex> *reportS
<rozmansi> have to catch another meeting... take care, bye.
<ordex> bye!
<ordex> I think this meeting is somewhat finishing anyway .. ? :D
<dazo> so anything else on the sec-audit/OSTIF front?
<cron2> not from me
<cron2> dazo: have you seen the uncrustify-git-commit-thingie that plaisthos 
<dazo> I've seen the discussion ... and it makes sense on conceptual level ... 
but haven't looked at that approach in detail
<dazo> but for that to work, we anyhow need to have a reasonable uncrust config 
we can use
<cron2> where's the 2.4.0 config we had?
<dazo> it's in dev-tools/uncrustify.conf ... but we might want to adjust it 
* rozmansi has quit (Ping timeout: 256 seconds)
<dazo> so that it doesn't change anything in our current code base ..... or 
that we do another round of uncrusting our current code
<cron2> this is one of our biggest issues right now - large patch sets from 
plaisthos and ordex that get sent back after 2+ weeks for style issues
<dazo> yupp
<dazo> for new code/files, this the current config can be used ... but if it 
starts modifying lines not being modified by the patch, then we have real 
<cron2> right
<ordex> yeah
<plaisthos> for the client-connect patch I did a "make uncrustify clean" commit 
to be able to use uncrustify during my repase
<cron2> which can be a bit problematic if it changes context in a way that "git 
am" fails later on
<plaisthos> cron2: was only in unrelated code but yes
<cron2> (I do not think this very likely)
<cron2> indeed
<dazo> these are the changes our current uncrustify config wants on the latest 
git master .... http://termbin.com/y9yt
<cron2> huh
<dazo> (I only took .c and .h files)
<cron2> did we leave out sample-plugins last time?
<dazo> it seems so :/
<cron2> because these changes are all unambiguously (sp?) agreed...
<dazo> and lz4 stuff too
<cron2> well, the lz4 has the "we do as little changes as possible on import" 
<cron2> (so it can be diff'ed to upstream releases)
<dazo> agreed, this was just a simple test .... for f in $(git ls-files | grep 
-E \\.[hc]\$ | grep -v "vendor/"); do echo "=========> $f"; uncrustify -c 
dev-tools/uncrustify.conf  --no-backup -l C -p debug.uncr $f; echo; echo; done;
<cron2> huh
<cron2> we left in changes to crypto.c which do not have {}
<dazo> but we're really sloppy about curly braces :-P
<cron2> -        ret = false;;
<cron2> huh
<cron2> so, looking at these diffs (excluding compat-lz4*) I would say "we 
should apply them, via patch-on-the-list, and apply the pre-commit-hook ASAP!!"
<cron2> -                fprintf(fp, "%"PRIi64".%06ld %x %s%s%s%s",
<cron2> +                fprintf(fp, "%" PRIi64 ".%06ld %x %s%s%s%s",
<cron2> these ones I can live with either way...
<cron2> but it's only a few
<dazo> most of this is really fair changes ... I don't disagree at all
<cron2> adding extra blank lines before #endif if that wraps only a single line 
(forward.h) looks a bit silly, so that option might need tweaking
<cron2> -#endif
<cron2> +#endif /* if P2MP */
<cron2> haha, there we go :)
<dazo> ;-)
<dazo> just looking at these ones:  
<dazo>      bool error; /* if true, fatal TCP error has occurred,
<dazo> -                *  requiring that connection be restarted */
<dazo> +                 *  requiring that connection be restarted */
<dazo> I think that we should have the closing */ on a separate line in the 
multi-line approach ... then all the * would be aligned too
<dazo> oh, this one actually aligns it ... anyhow I think I saw one which did 
the opposite
<cron2> this might have been tab/space related
<dazo> yeah, could be
<dazo> +    |MBEDTLS_X509_ID_FLAG( MBEDTLS_MD_SHA384 )
<dazo> this looks wrong ... shouldn't it be: | 
<cron2> the PRIi64 change is new - this is all "old code", so I think you have 
a newer uncrustify version :-)
<dazo> uncrustify-0.64-1.el7.x86_64
<dazo> I don't recall what we used last time, but I don't think it's that much 
newer :)
<cron2> but it does change new things :-)
<dazo> yeah, it does
<cron2> some of the code is just new and wasn't uncrustified (so, shame on us) 
but other stuff has been there longer
<plaisthos> cron2: yeah there is also the oddity that if you add lines to a 
short #ifdef section the #endif gets further away and then uncrusity adds the 
<dazo> oh the config has been updated since the run we had
<dazo> commit 4cd4899e8
<dazo> that shouldn't change that much though ... 
<cron2> right, but that commit also includes the changes effected by this
<dazo> I wonder if we've just done lots of changes since jan 2017 and not been 
picky enough on the coding style :)
<plaisthos> probably the latter
<cron2> half of the brackets is "new code".  But we forgot the samples in the 
first run.
<cron2> plus, new uncrustify :-) 
<plaisthos> someone needed to point out to me that I could just run uncrustify 
on my commits before commiting because for some reason that never occured to me
<dazo> :)
<dazo> plaisthos: trust me, I'll enforce this on openvpn3-linux too :-P
<dazo> (just need to wrap up a reasonable config for it there as well)
<plaisthos> dazo: openvpn3 linux is different mess to begin with, since you are 
constantly switching from GNU style+linux style mix to OpenVPN C style with 
random C++ style 
<dazo> plaisthos: no no no ... *my* code is everything outside the core library 
... the core library we can't do much about now :-P
<plaisthos> dazo: yes but when working on the code I am switched from core to 
openvpn3-linux back and forth
<dazo> we need to trick james to use a more reasonable coding style as well ... 
and enforce uncrustify runs on *commit* time for him :-P
<dazo> yeah, that's a real challenge, I see that
<cron2> can you make a video of that?  I want to see this ;-)
<dazo> :-D
<plaisthos> dazo: the more modern code of James more closely follows Linux 
coding+what emacs automatically does
<plaisthos> so it is like writing linux style but with GNU identation rules
<plaisthos> and tab is 8 instead of 4
<dazo> unfortunately ... I've experienced slight changes styles in emacs on 
upgrades as well :/
<dazo> so .... okay ... that's coding style
<dazo> tls-crypt-v2 update?  isn't that done?
<dazo> ordex ?
<cron2> the update is "the open patches have been merged", done
<dazo> cool!
<dazo> sitnl patches is being reviewed by plaisthos, I believe  (and I'll 
follow up on the struct argv patches too)
<dazo> vlan patches?
<cron2> stuck somewhere between ordex and plaisthos, I assume, and neither 
wants to talk about it?
<plaisthos> cron2: my last information was that you and ordex somehow magically 
produce some patchset that I can review
<plaisthos> and I would do the same with client-connect for one of you two to 
<ordex> dazo: yes tls-crypt-v2 is done 
<ordex> yes, for sitnl I have to provide a new patchset - it is WIP
<dazo> goodie :)
<dazo> So can we take a quick discussion about --fragment and --mssfix now? ... 
primarily to just get the ball starting to roll
<cron2> I've seen the patches, but I admit I have no answers.  So I really need 
to read into this (and for the --fragment frame stuff, I need syzzer)
<dazo> right, we're actually going a bit further ... we want to deprecate 
--fragment in a longer term
<cron2> if we can fix internal fragmentation without needing a change in frame 
format, I'm all for it
<dazo> we've looked into what to do in OpenVPN 3 .... and the complexity of 
--fragment is not really wanted  (packet re-assembly)
<dazo> so what we're doing is to make --mssfix better
<cron2> you can't avoid fragmentation if you need to carry big UDP packets
<dazo> which covers TCP packets inside the tunnel
<cron2> mssfix will only help TCP, nothing else...
<cron2> exactly
<dazo> and then send ICMP packet too big responses otherwise
<cron2> this will not work for real-world traffic in the general case
<dazo> the ICMP responses would be inside the tunnel though
<plaisthos> To get cron2 up to speed
<cron2> it will still not work.  If the UDP-generating application isn't 
interested in the ICMP packet too bigs, it will still send out too-big UDP 
<dazo> yes please :)
<plaisthos> we had an internal discussion. And the main goal is to make the 
packets inside the tunnel small enough or use --fragment. --fragment is a 
horrible option that we try to elimiate as much possible
<cron2> if we reduce tun-mtu today, it would achieve the same thing (make the 
linux stack generate ICMP ptb)
<plaisthos> so for makeing the packets small you can either lower the tun-mtu 
or do workaround like mssfix
<plaisthos> James decided a few years ago that *always* tun-mtu 1500 and mssfix 
is the best solution 
<cron2> "small enough packets" is best, but it will not be possible to do in 
the general case...  unfortunately
<plaisthos> the windows tap driver does always 1500 unless you do some manual 
configuration, tun-mtu is ignored on windows
<dazo> (the windows tun-mtu stuff might be for historic reasons, where it was 
needed to recreate the tun device to change the MTU ... not sure if that has 
improved since then)
<cron2> as a workaround for this, generating the ICMP ptb locally might be 
better than "silently lose packets", I'm with you there :-)
<plaisthos> and current result is that we do mssfix to vpn udp packets lower 
than outer mtu and hope that for bigger packets udp fragmentation does not break
<cron2> understood (and it does, unfortunately)
<plaisthos> yes on my network udp fragemnts break, have no dig into what breaks 
<dazo> and we also consider that TCP is mostly used inside the tunnels .... and 
modern based applications UDP often will account for MTU issues and "fix" this 
on their own
<cron2> we can assume that in a non-trivial subset of use cases, external UDP 
fragments will not work.  So that's something to avoid.
<cron2> dazo: the key word is "often", but I've seen enough cases where it 
didn't work, so --fragment was indeed very useful to fix that.  Which is 
annyoing as it needs state for reassembly, there is complex code and many bugs 
<dazo> but that does not mean it will help UDP cases where the senders just 
assume it will work and not bother checking if it really works
<cron2> like, DNS with DNSSEC, unfortunately...
<dazo> yeah, I'm fully with you on that
<dazo> yikes
<cron2> a typical DNS response can be close to 1500 bytes these days, if you 
have a few records and all the signatures...
<plaisthos> cron2: but doesn't have that a 1280 limit?
<dazo> right ... but I've seen a growth of DNS queries moved over to TCP too
<cron2> plaisthos: that's the minimum IPv6 packet size that a network must 
<cron2> dazo: which is one way to cope if UDP doesn't give a reply, yes
<plaisthos> oh right it just depends on udp fragmentation
<plaisthos> dazo: fallback to tcp 
<dazo> but I'm not saying we're happy about this ... and we do want to make 
this "just work" ... but implementing --fragment is just too intrusive as it is 
in openvpn 2 today
<cron2> for DNS it is complicated (EDNS0 maximum packet size, truncation, tcp 
fallback, broken firewalls, ...)
<plaisthos> you mean in openvpn3
<dazo> so we just want to start the discussion with the community to come up 
with something which can work reasonably well or better
<cron2> dazo: I understand, and accept the reasoning
<dazo> plaisthos: I meant implementing --fragment in openvpn 3 as it is in 
openvpn 2 today, is too intrusive
<cron2> I wonder how we can get this tested in a meaningful way... it needs 
exposure to lots of funny stuff inside the tunnel to see what works and what 
breaks, and why
<cron2> like, does QUIC cope if you have an openvpn tunnel with a small inside 
MTU, because it is tunneld through an OpenVPN tunnel through an IPSEC tunnel, 
or so
<plaisthos> but based on our discussion here and if do not implement something 
like --fragment in oepnvpn3 we only have the option to lower tun-mtu to 
something that fits and declare networks that support 1280+vpn overhead as 
broken and openvpn(3) ccannot work on them
<cron2> "that do not"
<plaisthos> yes not support 1280+overhead
<dazo> lev__ is the one digging into the --mssfix and ICMP ptb responses for 
openvpn3 ... this will appear in openvpn3-linux plus there is an ugly reference 
client in the openvpn3 project as well (test/ovpncli/cli.cpp) .... so this 
would be a way to test things as it evolves
<cron2> yep
<dazo> another aspect with tun-mtu ... we don't want to reduce it unless 
strictly needed ... so we'd like to have 1500 as the default where it works, 
but if it doesn't - ideally OpenVPN should detect this itself and potentially 
reduce it dynamically
<plaisthos> I never got the reason why we want 1500 as default
<plaisthos> it is a stupid choice for non tap
<dazo> That I don't know either ... just following James in this case
<plaisthos> it makes perfect sense on tap devices but for tun I am really not 
convinced. And the windows tap driver is not up to the task is really a lame 
<plaisthos> James argument for 1500 only works in a world where mssfix actually 
works for nearly all traffic
<dazo> for the win-tap driver, we need to see if things have changed there .... 
with the old win-tap driver, I have no doubt this was a real issue .... with 
tap-windows6, I'm not so sure any more
<cron2> I'm fairly sure you can set the MTU on windows as well. It's just that 
nobody bothered yet to figure out how (do you need an ioctl to the driver, is 
there an API call to windows, do you call netsh?)
<cron2> anyway.  Thanks for the update, I actually need to make the next 
meeting now...
<dazo> sure, lets call it a meeting ... and we'll send mattock the scrollback :)

Attachment: signature.asc
Description: OpenPGP digital signature

Openvpn-devel mailing list

Reply via email to