
Important security vulnerabilities in OpenVPN - guidovranken
https://guidovranken.wordpress.com/2017/06/21/the-openvpn-post-audit-bug-bonanza/
======
simias
Vulnerability #2 is a good example of why OpenSSL is a minefield even for a
competent coder. For such a security-critical library it's pretty insane that
the API is so unfriendly, bordering on hostile.

> The correct way to do this is to call GENERAL_NAMES_free. This is because
> sk_GENERAL_NAME_free frees only the containing structure, whereas
> GENERAL_NAMES_free frees the structure AND its items.

And later:

> Here, the code assumes that a return value that is negative or zero
> indicates failure, and ‘buf’ is not initialized, and needs not to be freed.
> But in fact, this is ONLY the case if ASN1_STRING_to_UTF8 returns a negative
> value. A return value 0 simply means a string of length 0, but memory is
> nonetheless allocated, so there are memory leaks here as well.

It mirrors my experience working with OpenSSL: you have to quadruple check
each function invocation with the docs to make sure you got it right. You're
never sure at a glance what's an input or an output parameter, what needs to
be freed and _how_ you're supposed to free it. What's the return value in case
of error? 0? -1? <0? <=0? And then you have the macro soup with their STACK_OF
"abstraction" (in the leakiest sense of the word) that just serves to make the
code harder to reason about and is a huge pain when you want to create OpenSSL
bindings for an other language.

Let me be clear: I don't blame the OpenSSL devs in any way. It's free, it's
open source, they don't get a ton of money for that. Instead I blame all the
big corporations who use OpenSSL "as-is", directly or indirectly, and don't
invest some money to improve that mess. Maybe after a couple more Heartbleed-
like vulnerabilities they'll take it a little more seriously.

~~~
dom0
> you have to quadruple check each function invocation with the docs to make
> sure you got it right.

If the docs contain that information ¯\\_(ツ)_/¯

~~~
kbenson
There's always a trade-off between how complicated the docs and usage are and
how many people will use it. OpenSSL is problematic because it has very
complicated docs and usage, _but people still used it anyway_. This leads to a
situation where people use it incorrectly, and for a crypto library, that's a
very bad thing.

If OpenSSL wasn't the de facto king of OSS crypto libraries for so long
(partly because it was the only one for so long), then people might have been
able to make more informed choices about what library was better.

~~~
the_why_of_y
> If OpenSSL wasn't the de facto king of OSS crypto libraries for so long
> (partly because it was the only one for so long),

Netscape released the NSS library in 1998, and as far as I can tell it hasn't
even changed its ABI incompatibly since 2000, but perhaps this is too recent
for programmers to take notice.

~~~
kbenson
I have no idea why NSS never got more use, but it didn't, for the most part. I
assume there's a reason, but for the most part major adoption seemed to have
been limited to Mozilla projects, Sun Enterprise Java, and OpenOffice
(possibly because it also originated from a Sun project, and it seems to have
enjoyed some popularity at Sun). If I had to make a wild guess from that info,
it would be that it's more complex to work with, as odd as that sounds given
the current context (although I guess it's possible OpenSSL got more complex
over time, and was much simpler a decade ago).

~~~
ibotty
For one, NSS is not the kitchen sync that is openssl. With openssl you can do
anything with any file you can get from the anywhere.

Secondly, when using openssl-based servers it's usually easier to specify
certificates and keys. You don't have to import them into a database, you only
configure the filesystem path.

------
ProxCoques
So I just installed ovpn on my phone just now. First place I go to test it is
HN. And this story is literally top of the list.

Sigh.

~~~
throwanem
I'm not sure these are so dangerous as all that. I see some server and client
crashes, but nothing that'd allow transparent MITM, RCE, or the like. Perhaps
there's something I have missed, and if so I hope someone more knowledgeable
here will point it out - but right now I don't intend to stop using OpenVPN,
because even if it's possible that a malicious network might crash the VPN
stack on my phone or similar, that's still preferable to sending unprotected
traffic over an untrusted network. If my VPN clients die, at least I know
something's up!

~~~
mkj
A double-free can traditionally allow remote code execution. Maybe modern libc
mitigation​s will protect you, maybe not.

~~~
throwanem
Fair point. But it looks like the scope on that one is pretty limited:

> There are several issues in the extract_x509_extension() function in
> ssl_verify_openssl.c. This function is called if the user has used the
> ‘x509-username-field’ directive in their configuration.

That option, per [1], appears to exist in order to support odd TLS
certificates that don't use the CN field as an identifier, and is also behind
a configure #define that appears not to be enabled by default. So that, if I'm
reading this right, is a pretty narrow attack surface in general - not
inconsiderable, to be sure, but something of a corner case.

[1]
[https://community.openvpn.net/openvpn/attachment/ticket/124/...](https://community.openvpn.net/openvpn/attachment/ticket/124/0001-Documented-x509-username-
field-option.patch)

------
dperfect
Since it's not clear from the blog post or the other HN comments: the
vulnerabilities are fixed in OpenVPN 2.4.3 and 2.3.17.

OpenVPN users are advised to upgrade[1] "as soon as possible."

[1] [https://openvpn.net/index.php/open-
source/downloads.html](https://openvpn.net/index.php/open-
source/downloads.html)

------
akavel
I don't understand why comment out ASSERTs; wouldn't they actually potentially
protect against some of the listed issues?

The article mentions they interfere with libFuzzer; but isn't a fuzzer
expected to detect and handle crashes as part of its core functionality?

~~~
yaantc
You have two kinds of fuzzers:

1) out of process, like AFL: The application is launched for each test, and
there's no problem handling a crash, whatever the cause (asserts are ok). But
for each test there's the application start-up (process creation, etc.)
overhead.

2) in process, like libFuzzer: The application is launched once only. Then
inside the application context the library iterates over the tests. So no
application start-up overhead (nice), but a brutal exit is a problem. That's
where asserts becomes a problem.

I guess that's what's at play here.

~~~
Ded7xSEoPKYNsDd
Why not change asserts (they are macros after all) to tell the fuzzing library
that it found an error? Finding bugs is the whole point of the exercise, isn't
it?

~~~
yaantc
From a quick look at the doc
([http://llvm.org/docs/LibFuzzer.html](http://llvm.org/docs/LibFuzzer.html))
this doesn't seem to be supported.

Aborting from an assert deep in the code has some challenges: unwinding the
stack is ok, but how one would avoid memory leaks for heap data? It would
require trapping all mallocs to track all allocated data and free it. Not
impossible but it adds very significant complexity, and that doesn't seem the
philosophy of libFuzzer. It seems to me that the goal of libFuzzer is to be
very easy to use (just define one function for test harness, compile with
CLANG and fuzzying flag, done) and efficient on an already reasonably well
behaved library.

It makes sense to me: just start with AFL, which has no problems with crashes.
When you reach AFL limits and need more efficiency, only then move to
libFuzzer (both can shared state / test framework). Then the assumption that
the app. is reasonably well behaved makes sense.

Caveat: I haven't used libFuzzer yet. For the reason above, I'm just using AFL
for now. Maybe one day but I'm not there yet ;)

------
LeonM
It's good to see that 2 donations, totaling 0.80260293 BTC (~2000 euro) are
made to Guido's wallet [0]. Probably not worth his time, but since he states
it was "a labor of love" it's still a nice extra!

Edit: corrected BTC amount.

[0]
[https://blockchain.info/address/1D5vYkiLwRptKP1LCnt4V1TPUgk7...](https://blockchain.info/address/1D5vYkiLwRptKP1LCnt4V1TPUgk7cxvVtg)

~~~
bitxbitxbitcoin
Private Internet Access VPN provided $1,000 of that and thanks OP profusely
[0].

[0]
[https://www.privateinternetaccess.com/forum/discussion/24191...](https://www.privateinternetaccess.com/forum/discussion/24191/private-
internet-access-is-not-affected-by-recently-discovered-openvpn-security-
vulnerabilities#latest)

------
rsync
OpenVPN is very complicated and for that reason I use sshuttle[1] which is
very simple.

It does require that you own an sshd running on an endpoint somewhere (like a
VPS or an EC2 instance or your own server somewhere) - but if you can clear
that hurdle, you end up with a very elegant and simple solution.

[1]
[https://github.com/sshuttle/sshuttle](https://github.com/sshuttle/sshuttle)

~~~
curun1r
For Mac users, Sidestep [1] offers a similar VPN over SSH experience. It's the
only VPN product I've ever used that I've never had an issue with...it just
works (no doubt due to the bulletproof nature of SSH).

[1]
[http://chetansurpur.com/projects/sidestep/](http://chetansurpur.com/projects/sidestep/)

~~~
rsync
This interested me however it appears to be abandoned ... last commit was on
2013-10-25 and comments like the one at the end of this comment thread:

[https://github.com/chetan51/sidestep/issues/62](https://github.com/chetan51/sidestep/issues/62)

indicate that it is abandoned ...

------
microcolonel
Man, I'm eager for WireGuard to hit 1.0. It's very elegant, and I could build
a policy layer on it in a couple days which doesn't suck.

~~~
pferde
Indeed, wireguard looks like it's going to be the bee's knees. It still has a
lot of development ahead of it, though.

~~~
microcolonel
Shouldn't take _too_ long, though. It's still about 10kSLoC and it's been
more/less feature frozen for over a year. I get the feeling that it's already
probably safer than most deployed VPN systems. Most of the recent work seems
to be forward porting, reworking the ratelimiter, and tweaks to the treatment
of randomness.

~~~
zx2c4
If by 10kLoC you actually mean less than 4kLoC, then I agree with you. :)

~~~
microcolonel
There's more than 10k lines of C in the repository, 8200 lines of assembly,
and almost three thousand lines of headers. Your kernel module is definitely
smaller than the whole repo. I feel like I've just called you fat. ;- )

------
chrisper
I was thinking of switching to L2TP instead. Would there be any downsides?

I tried out this script once and it worked well:

[https://github.com/hwdsl2/setup-ipsec-vpn](https://github.com/hwdsl2/setup-
ipsec-vpn)

~~~
UnoriginalGuy
L2TP doesn't traverse NATs and other network firewalls very well. Which isn't
relevant if you yourself control all of the hardware between endpoints (e.g.
point2point) but if you plan on using it on public networks (e.g. Starbucks
WiFi) then expect to run into it being blocked sometimes.

OpenVPN was designed for the "Starbucks WiFi" scenario, it can traverse a NAT,
and uses standard TLS. It therefore appears like any other HTTPS traffic and
will be very infrequently blocked.

There's nothing inherently wrong with L2TP/IPSec; it is secure and fast. Just
nicer to blend in with other TLS traffic for reliability reasons.

For a specific example, GoGo Internet (in-flight WiFi), doesn't allow
L2TP/IPsec unless you use UDP encapsulation. OpenVPN works out of the box.

~~~
MichaelGG
Where does this idea that OpenVPN is TLS come from? No, it does not appear
like HTTPS nor does it use TLS for the traffic.

OpenVPN has its completely own encapsulation protocol, and it stuffs a few TLS
packets _inside that_ to do key exchange, nothing more. (And it's key exchange
is odd as they re-implement TLS PRF ... on top of TLS.)

t. Recently implemented an OpenVPN client.

~~~
okasaki
The man page says it supports TLS

~~~
MichaelGG
It supports TLS as an optional key-exchange payload, nothing more. It does not
act in any way like "SSL VPNs" that just open a TLS connection and then tunnel
data inside.

------
yuchi
I’m gonna take the burden (someone would do that eventually anyway) and ask:
how many of those issues would have been completely prevented by using a safer
language such as Rust? How many would have been mitigated?

I’m not a system programmer and the article, while indeed interesting, can be
a little obscure.

~~~
alltakendamned
I really think this is the wrong way to look at issues like this.

Software has bugs, some of which are security bugs. Whenever someone goes
through the effort of looking at software from a security perspective, issues
might get identified and resolved and as such there is an incremental increase
in security.

Switching languages requires a full rewrite, which is often not only
impractical, it will introduce new bugs as the maturity of the software will
plummet because of the rewrite. Then the cycle simply starts again.

Some languages have better features that may prevent some classes of security
bugs, and Rust is just the new kid on the block. As such, new languages are
unproven in the security area and there will be security bugs found there as
well. Java and .NET don't have the same issues either but somehow nobody will
ask about those :)

But then again, Rust introduces other bug types for which the future will tell
whether they have a security impact or not:
[https://gankro.github.io/blah/only-in-
rust/](https://gankro.github.io/blah/only-in-rust/)

My point of this rambling is that yes, C code is really hard to get right from
a security perspective. But software like Apache httpd and OpenSSH
demonstrates clearly it can be done. And switching languages is not the
answer.

~~~
lmm
> Switching languages requires a full rewrite

Not true when those languages are ABI-compatible. E.g. I believe librsvg is
introducing small amounts of Rust that can eventually become a gradual
migration.

> it will introduce new bugs as the maturity of the software will plummet
> because of the rewrite.

Citation needed. I would expect a rewrite guided by an existing implementation
( _not_ the same thing as a rewrite aimed at achieving a from-scratch
_redesign_ ) to result in fewer bugs as it would effectively be equivalent to
a full code review, and that's before we take into account the difference-of-
language effects.

> As such, new languages are unproven in the security area and there will be
> security bugs found there as well.

No. There are specific reasons C is so awful; it's not a question of maturity.

> Java and .NET don't have the same issues either but somehow nobody will ask
> about those :)

People are dumb. There's a popular myth that these languages are inherently
slow, which combined with some genuine licensing issues means the OSS
community doesn't use them, even though they'd be a perfect fit for OpenVPN-
like processes.

> But then again, Rust introduces other bug types for which the future will
> tell whether they have a security impact or not:
> [https://gankro.github.io/blah/only-in-
> rust/](https://gankro.github.io/blah/only-in-rust/)

No, read your link. Those issues happen only in unsafe code, and at absolute
worst mean you're as badly off as you would be in C.

> software like Apache httpd and OpenSSH demonstrates clearly it can be done

Because those programs can go years at a time without a major security
vulnerability? Oh wait, no they can't.

Switching languages is absolutely the answer, and the sooner we get over
ourselves and get on with it the better.

~~~
laumars
> _Citation needed. I would expect a rewrite guided by an existing
> implementation (not the same thing as a rewrite aimed at achieving a from-
> scratch redesign) to result in fewer bugs as it would effectively be
> equivalent to a full code review, and that 's before we take into account
> the difference-of-language effects._

Humans are fallible. Rewriting the code introduces the possibility that the
human coding it might make a mistake. This is true even when using existing
code as a guide.

~~~
lmm
True as far as it goes, but I would still expect the bugs caught by the
rewrite to outweigh the newly-introduced ones most of the time.

~~~
verytrivial
This has not been my experience in the last couple of decades.

~~~
lmm
How many rewrites-that-involved-no-redesign have you done?

------
shmerl
A lot of such stuff can be avoided by simply using Rust.

