Opened 4 years ago

Closed 4 years ago

#682 closed Feature Wish (fixed)

Protect the client from accepting arbitrary options pushed by the server

Reported by: chris.laif Owned by:
Priority: major Milestone:
Component: Generic / unclassified Version: OpenVPN git master branch (Community Ed)
Severity: Not set (select this one, unless your'e a OpenVPN developer) Keywords:
Cc:

Description

I wonder if there is an easy way to protect the client from executing ifconfig/route-statements sent by an (untrusted) server. I think of some config options like

ifconfig-limit 10.123.0.0/24
route-limit 10.111.0.0/16
route-limit 10.222.0.0/24

Any statements sent by the server not matching those networks would be ignored.

I know the 'ifconfig-noexec' and 'route-nopull' options. That's what I'm using right now.

In case of the 'route' statements it's kind of working, because in most of the cases I know the destination networks in advance.

In case of 'ifconfig' it's not working, because in many cases the other VPN endpoint dynamically allocates my endpoint IP address from a class C pool. And I can not pick a fixed IP address, which might be allocated by some other user of the VPN now or later.

My problem is, that the remote administrator might 'inject' arbitrary IP addresses/networks conflicting with my existing (local) network. He might do this by malicious intent or even by accident (renumbering his network, overlapping with other networks on our side). In our current setup, we have a 'VPN hub' connecting to 10+ partner networks. If any of the partners changes his network topology, this might clash with other partner networks. And no, unfortunately we can not negotiate fixed IP addresses with all of the partners. This is why I vote for implementing 'ifconfig-limit' and 'route-limit' statements or some other mechanism to protect our setup.

-- Comment by Gert Doering (https://sourceforge.net/p/openvpn/mailman/message/35086118/):

OK, I can see that in your setup (with multiple VPNs connecting to a
hub, not "just the client") more tight control is desireable.

I'm not promising anything - this is a fairly special-case request, and
we already have sooo many special-case options that tend to get broken
if we change other bits of the code - it should be able to implement
these (route, ifconfig, ipv4 and ipv6) in a way that is not touched
much by other code bits - and maybe we can even come up with a more
general "--pull-option-filter <script>" thing where options get run
through an external script that implements local policy, and returns
only those options that are acceptable, or throws an error if things
cannot go on...

I'd actually prefer the latter (a generic script) because once we have
these four options for your requirements, the next one will show up
and ask for a DHCP filter, and then we'll see something else again.

-- Comment end

Change History (9)

comment:1 Changed 4 years ago by Gert Döring

thanks - it might still take long to get implemented, but at least the idea won't get lost.

comment:2 Changed 4 years ago by debbie10t

--ifconfig-noexec and --route-noexec provide the exact hooks required to filter, via scripting, what is and what is not acceptable to use for a specific client.

comment:3 in reply to:  2 Changed 4 years ago by Gert Döring

Replying to debbie10t:

--ifconfig-noexec and --route-noexec provide the exact hooks required to filter, via scripting, what is and what is not acceptable to use for a specific client.

Well, they do, but only if you're willing to duplicate all the "... and now go out and configure ifconfig and route!" logic in the script called - which is error-prone, quite clumsy, and not general enough either... (and, btw, the original poster acknowledged the existance of these options)

comment:4 Changed 4 years ago by debbie10t

There is fine grained control and then there is [xpd] ..

I would prefer to see that developers of such high esteem focus on what is important and not this obvious corner case

However .. you are welcome to make your mark any way you want to ..

Last edited 4 years ago by debbie10t (previous) (diff)

comment:5 in reply to:  4 Changed 4 years ago by Gert Döring

Replying to debbie10t:

I would prefer to see that developers of such high esteem focus on what is important and not this obvious corner case

This being open source, the people you are not paying for their work decide on their own what they find important and what not. You are not the one to decide that.

comment:6 Changed 4 years ago by debbie10t

This is a feature wish .. and in four years of helping this project, this is the first time such a request has been made .. it is a corner case at best.

I am not deciding, I am merely stating the obvious .. you are free to chase what ever rainbow you choose .. but this being openVPN and not openswiss-army-network-knife the effort would be better spent maintaining a working project not polluting it with useless code to fix one problem for one person who clearly does not have the skills to manage theirs and their partner's networks sensibly.

One side comment: how about --pull-reset (the client-side equivalent to --push-reset) .. where by no pulled options are imposed and it is left to the client to implement whatever options they choose.

Not wanting to start a flame war, I will make no further comment.

Last edited 4 years ago by debbie10t (previous) (diff)

comment:7 in reply to:  6 Changed 4 years ago by Gert Döring

Replying to debbie10t:

This is a feature wish .. and in four years of helping this project, this is the first time such a request has been made .. it is a corner case at best.

It helps deploy OpenVPN in corporate context, where we are not very strong today. This is not your typical end user deployment, so requirements are different. The original poster has opened this trac request because I have asked him to, because for someone who understands corporate networks, this is a very reasonable thing to ask for.

I am not deciding, I am merely stating the obvious .. you are free to chase what ever rainbow you choose .. but this being openVPN and not openswiss-army-network-knife the effort would be better spent maintaining a working project not polluting it with useless code to fix one problem for one person

Since you're not the one to maintain this (and OpenVPN is the swiss army knife of VPNs, just btw), it would be prudent to leave this decision to those who actually do* write and maintain the code.

who clearly does not have the skills to manage theirs and their partner's networks sensibly.

Please re-read the thread on openvpn-users and try to understand the scenario. There is nothing to maintain "in the partner's network" (because that's what the partner does), and to protect your own network from accidential or malicious(!) push configs coming in from a partner network is a very sensible thing to do - and actually that is "manage their network sensibly". OpenVPN today provides hooks, but they are clumsy and error prone.

Maybe we'll never implement something better, because we discover that it's very hard to do in the current push-parsing framework, or nobody has time - but that does not invalidate the feature request.

comment:8 Changed 4 years ago by Gert Döring

Ask, and your wish shall be granted :-)

commit 7f74c27e105a365d278181d00708c55a299398a0
Author: Selva Nair <selva.nair@…>
Date: Sun Jun 5 17:41:23 2016 -0400

Add an option to filter options received from server

...

Example:

pull-filter accept "route 192.168."
pull-filter ignore "route "
pull-filter accept "ifconfig 10.9.0."
pull-filter reject "ifconfig "

...

I have just commited this to git master, and pushed to sf/github. Thanks to @selvanair for finding this interesting and useful enough to hack on it :-)

As per our release guidelines, we won't have this in release/2.3 - it's "new feature", and only bugs / long-term compatibility issues are supposed to appear there. The patch *should* apply to release/2.3 as well, though, if you do not want to risk git master (even if I claim that git master has seen fairly rigorous testing now, so it normally works as well as 2.3.x)

comment:9 Changed 4 years ago by Gert Döring

Resolution: fixed
Status: newclosed

closing, because the feature wish has been granted :-)

A bit more discussion about feature semantics is ongoing on the openvpn-users list (whether or not it should be possible to do an exact match) but the basic feature is here to stay. Thanks, Selva.

Note: See TracTickets for help on using tickets.