Mailing List Archive

[PATCH 3/5] delete tun address on disconnect
This enables persist tun device to be reused in future
connections.

ipv6 is not tested.

Maybe it would be cleaner to define functions for address
manipulation, not sure.

To be simple, this patch only handles iproute2.

Signed-off-by: Alon Bar-Lev <alon.barlev@gmail.com>
---
vpnc-script | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/vpnc-script b/vpnc-script
index c8483ee..1af6f4d 100755
--- a/vpnc-script
+++ b/vpnc-script
@@ -726,6 +726,20 @@ do_disconnect() {
if [ -n "$INTERNAL_IP4_DNS" ]; then
$RESTORERESOLVCONF
fi
+
+ if [ -n "$IPROUTE" ]; then
+ if [ -n "$INTERNAL_IP4_ADDRESS" ]; then
+ $IPROUTE addr del "$INTERNAL_IP4_ADDRESS/255.255.255.255" peer "$INTERNAL_IP4_ADDRESS" dev "$TUNDEV"
+ fi
+ # If the netmask is provided, it contains the address _and_ netmask
+ if [ -n "$INTERNAL_IP6_ADDRESS" ] && [ -z "$INTERNAL_IP6_NETMASK" ]; then
+ INTERNAL_IP6_NETMASK="$INTERNAL_IP6_ADDRESS/128"
+ fi
+ if [ -n "$INTERNAL_IP6_NETMASK" ]; then
+ $IPROUTE -6 addr del $INTERNAL_IP6_NETMASK dev $TUNDEV
+ fi
+ fi
+
destroy_tun_device
}

--
1.8.3.2

_______________________________________________
vpnc-devel mailing list
vpnc-devel@unix-ag.uni-kl.de
https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel
http://www.unix-ag.uni-kl.de/~massar/vpnc/
Re: [PATCH 3/5] delete tun address on disconnect [ In reply to ]
On Sun, 2013-12-15 at 19:23 +0200, Alon Bar-Lev wrote:
> This enables persist tun device to be reused in future
> connections.
>
> ipv6 is not tested.
>
> Maybe it would be cleaner to define functions for address
> manipulation, not sure.
>
> To be simple, this patch only handles iproute2.

It should be simple enough to "break" the detection of iproute2 and thus
force it to take the ifconfig path; please could we implement and test
that too?

If we're dealing with persistent interfaces, I also wonder if we should
have a method which deletes *all* IP addresses from the interface?

We could call that before the 'connect' method too, to make sure that we
are using an interface in a sane state and it doesn't already have stale
IP addresses.

Thanks for implementing the IPv6 code, although I appreciate it's hard
to test with vpnc. I can test that with OpenConnect as soon as we
finalise the disconnect/destroy thing.

--
dwmw2
Re: [PATCH 3/5] delete tun address on disconnect [ In reply to ]
On Thu, Feb 20, 2014 at 7:06 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Sun, 2013-12-15 at 19:23 +0200, Alon Bar-Lev wrote:
>> This enables persist tun device to be reused in future
>> connections.
>>
>> ipv6 is not tested.
>>
>> Maybe it would be cleaner to define functions for address
>> manipulation, not sure.
>>
>> To be simple, this patch only handles iproute2.
>
> It should be simple enough to "break" the detection of iproute2 and thus
> force it to take the ifconfig path; please could we implement and test
> that too?

Please be more specific, what is broken?

>
> If we're dealing with persistent interfaces, I also wonder if we should
> have a method which deletes *all* IP addresses from the interface?
>
> We could call that before the 'connect' method too, to make sure that we
> are using an interface in a sane state and it doesn't already have stale
> IP addresses.
>
> Thanks for implementing the IPv6 code, although I appreciate it's hard
> to test with vpnc. I can test that with OpenConnect as soon as we
> finalise the disconnect/destroy thing.
>
> --
> dwmw2
_______________________________________________
vpnc-devel mailing list
vpnc-devel@unix-ag.uni-kl.de
https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel
http://www.unix-ag.uni-kl.de/~massar/vpnc/
Re: [PATCH 3/5] delete tun address on disconnect [ In reply to ]
> On Thu, Feb 20, 2014 at 7:06 PM, David Woodhouse <dwmw2@infradead.org>
> wrote:
>> On Sun, 2013-12-15 at 19:23 +0200, Alon Bar-Lev wrote:
>>> This enables persist tun device to be reused in future
>>> connections.
>>>
>>> ipv6 is not tested.
>>>
>>> Maybe it would be cleaner to define functions for address
>>> manipulation, not sure.
>>>
>>> To be simple, this patch only handles iproute2.
>>
>> It should be simple enough to "break" the detection of iproute2 and thus
>> force it to take the ifconfig path; please could we implement and test
>> that too?
>
> Please be more specific, what is broken?

Nothing is broken. I was suggesting a way to fix the problem you mentioned
-- the fact that it's only implemented for iproute2 -- by *artificially*
breaking the detection of iproute2 in the script and thus testing the
ifconfig code paths.


--
dwmw2

_______________________________________________
vpnc-devel mailing list
vpnc-devel@unix-ag.uni-kl.de
https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel
http://www.unix-ag.uni-kl.de/~massar/vpnc/
Re: [PATCH 3/5] delete tun address on disconnect [ In reply to ]
On Thu, Feb 20, 2014 at 9:56 PM, David Woodhouse <dwmw2@infradead.org> wrote:
>
>
> > On Thu, Feb 20, 2014 at 7:06 PM, David Woodhouse <dwmw2@infradead.org>
> > wrote:
> >> On Sun, 2013-12-15 at 19:23 +0200, Alon Bar-Lev wrote:
> >>> This enables persist tun device to be reused in future
> >>> connections.
> >>>
> >>> ipv6 is not tested.
> >>>
> >>> Maybe it would be cleaner to define functions for address
> >>> manipulation, not sure.
> >>>
> >>> To be simple, this patch only handles iproute2.
> >>
> >> It should be simple enough to "break" the detection of iproute2 and thus
> >> force it to take the ifconfig path; please could we implement and test
> >> that too?
> >
> > Please be more specific, what is broken?
>
> Nothing is broken. I was suggesting a way to fixIf you have claims, please show the exact location of breakage and I will fix. the problem you mentioned
> -- the fact that it's only implemented for iproute2 -- by *artificially*
> breaking the detection of iproute2 in the script and thus testing the
> ifconfig code paths.

Once again, ifconfig is not broken, iproute2 detection was not modified.

The only thing that I claim is that if one wish to use non privileged
mode, he must have iproute2 available on his system.

Using ifconfig/route and friends these days to achieve the task has no
benefit once we have the clear syntax provided by iproute2.

So for the general public nothing is changed.

For whoever interested in security (unprivileged), iproute2 should be
available, as single command (ip) with clear syntax is much easier to
wrap.

Regards,
Alon
_______________________________________________
vpnc-devel mailing list
vpnc-devel@unix-ag.uni-kl.de
https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel
http://www.unix-ag.uni-kl.de/~massar/vpnc/
Re: [PATCH 3/5] delete tun address on disconnect [ In reply to ]
On Thu, 2014-02-20 at 22:01 +0200, Alon Bar-Lev wrote:
>
> The only thing that I claim is that if one wish to use non privileged
> mode, he must have iproute2 available on his system.

So now it's gratuitously Linux-only, and the vpnc-script has different
capabilities if you run it on different operating systems.

After I went to the trouble of installing a farm of random *BSD and
Solaris systems and making sure it worked, consistently and coherently.

No thanks. Do the job properly please, or your patch is unacceptable.

--
dwmw2
Re: [PATCH 3/5] delete tun address on disconnect [ In reply to ]
On Thu, Feb 20, 2014 at 10:09 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Thu, 2014-02-20 at 22:01 +0200, Alon Bar-Lev wrote:
>>
>> The only thing that I claim is that if one wish to use non privileged
>> mode, he must have iproute2 available on his system.
>
> So now it's gratuitously Linux-only, and the vpnc-script has different
> capabilities if you run it on different operating systems.
>
> After I went to the trouble of installing a farm of random *BSD and
> Solaris systems and making sure it worked, consistently and coherently.
>
> No thanks. Do the job properly please, or your patch is unacceptable.

Nothing was changed for *BSD, nothing gained, nothing lost.

If you want to have unprivileged mode for *BSD, please checkout the
entire solution of using unprivileged mode, the utilities are the
minor task (replace ifconfig with "${IFCONFIG}").

I am unsure we can provide the same for solaris anyway... so do you
claim that because solaris cannot host unprivileged vpnc, we should
limit other environment?

>
> --
> dwmw2
_______________________________________________
vpnc-devel mailing list
vpnc-devel@unix-ag.uni-kl.de
https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel
http://www.unix-ag.uni-kl.de/~massar/vpnc/
Re: [PATCH 3/5] delete tun address on disconnect [ In reply to ]
On Thu, 2014-02-20 at 22:13 +0200, Alon Bar-Lev wrote:
> On Thu, Feb 20, 2014 at 10:09 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> > No thanks. Do the job properly please, or your patch is unacceptable.
>
> Nothing was changed for *BSD, nothing gained, nothing lost.

There are space-constrained Linux systems that lack iproute2, too. As
far as I know, busybox still only provides ifconfig tools.

I'm content with having a difference in the feature set where iproute2
genuinely *can* do things that the older tools cannot. But unless I'm
missing something, that's not the case here; it's just that you can't be
bothered to do the job properly.

That isn't the way things work in open source. You're expected to keep
your eyes open and be aware of how your changes work in the wider
context, not remain entirely blinkered to your *own* use case to the
exclusion of all others.

It is not acceptable to introduce gratuitous inconsistencies in the way
the script behaves.

Just as it is not acceptable to wilfully ignore the NetworkManager use
case for unprivileged operation, and refuse to even consider whether
your design is doing the *wrong* thing for NM and will subsequently need
to be fixed.

--
dwmw2
Re: [PATCH 3/5] delete tun address on disconnect [ In reply to ]
On Thu, Feb 20, 2014 at 10:30 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Thu, 2014-02-20 at 22:13 +0200, Alon Bar-Lev wrote:
>> On Thu, Feb 20, 2014 at 10:09 PM, David Woodhouse <dwmw2@infradead.org> wrote:
>> > No thanks. Do the job properly please, or your patch is unacceptable.
>>
>> Nothing was changed for *BSD, nothing gained, nothing lost.
>
> There are space-constrained Linux systems that lack iproute2, too. As
> far as I know, busybox still only provides ifconfig tools.
>
> I'm content with having a difference in the feature set where iproute2
> genuinely *can* do things that the older tools cannot. But unless I'm
> missing something, that's not the case here; it's just that you can't be
> bothered to do the job properly.
>
> That isn't the way things work in open source. You're expected to keep
> your eyes open and be aware of how your changes work in the wider
> context, not remain entirely blinkered to your *own* use case to the
> exclusion of all others.
>
> It is not acceptable to introduce gratuitous inconsistencies in the way
> the script behaves.
>
> Just as it is not acceptable to wilfully ignore the NetworkManager use
> case for unprivileged operation, and refuse to even consider whether
> your design is doing the *wrong* thing for NM and will subsequently need
> to be fixed.

I fail to understand... really fail to understand...

1. busybox has iproute2 no problem to set it up, but let's leave that.

2. wrapper of iproute2 can be used to delegate commands to
ifconfig/route, it is easier to use the iproute2 interface than to
support several interfaces, especially these of ifconfig/route that
are harder to parse. Instead of inventing yet another interface use
well defined one.

3. you can use iproute2 wrapper also in environments that lack
iproute2, see (2).

4. so you can see that I am looking at much wider context than you,
please do not underestimate people you do not know.

5. Using unprivileged mode of vpnc will not break the way network
manager currently use vpnc, as there is no change to current use
cases... so please do not add noise to discussion.

I am unsure what you are trying to do, you reject patches you do not
understand, and you want to divert the discussion to something else
you need.

These patches are introduced to one reason, to enable OPTIONAL
privileged escalation wrapper, for this I chose the iproute2 interface
which is the best candidate among the alternatives, nothing forces you
to use this feature, nothing is changed if you do not use it.

Alon
_______________________________________________
vpnc-devel mailing list
vpnc-devel@unix-ag.uni-kl.de
https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel
http://www.unix-ag.uni-kl.de/~massar/vpnc/
Re: [PATCH 3/5] delete tun address on disconnect [ In reply to ]
On Thu, 2014-02-20 at 22:43 +0200, Alon Bar-Lev wrote:
> I fail to understand... really fail to understand...

Alon, the patch we're discussing (see $SUBJECT) isn't specifically about
unprivileged operation. It's about persistent tunnel devices, where we
want to set up and tear down the IP configuration on a device without
actually creating and destroying said device dynamically.

This *is* something we can do on *BSD and Solaris; the devices *are*
persistent there (which is why we have to have the 'destroy' bits in the
script in the first place).

Surely you could have fixed the 'ifconfig' code paths to do this
correctly in far *less* than the amount of time you've been arguing
about it?

> 5. Using unprivileged mode of vpnc will not break the way network
> manager currently use vpnc, as there is no change to current use
> cases... so please do not add noise to discussion.

I'm disappointed that you don't see the relevance of that.

If you're going to implement support for unprivileged operation, it is
appropriate to at least make sure that your design is not *wrong* for
the other users who will want to make use of it, and that we won't have
to revert and/or fix parts of it later — especially if the later changes
have compatibility implications for your use case. Is that so hard to
comprehend?

--
dwmw2
Re: [PATCH 3/5] delete tun address on disconnect [ In reply to ]
On Thu, Feb 20, 2014 at 11:09 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Thu, 2014-02-20 at 22:43 +0200, Alon Bar-Lev wrote:
>> I fail to understand... really fail to understand...
>
> Alon, the patch we're discussing (see $SUBJECT) isn't specifically about
> unprivileged operation. It's about persistent tunnel devices, where we
> want to set up and tear down the IP configuration on a device without
> actually creating and destroying said device dynamically.
>
> This *is* something we can do on *BSD and Solaris; the devices *are*
> persistent there (which is why we have to have the 'destroy' bits in the
> script in the first place).
>
> Surely you could have fixed the 'ifconfig' code paths to do this
> correctly in far *less* than the amount of time you've been arguing
> about it?
>
>> 5. Using unprivileged mode of vpnc will not break the way network
>> manager currently use vpnc, as there is no change to current use
>> cases... so please do not add noise to discussion.
>
> I'm disappointed that you don't see the relevance of that.
>
> If you're going to implement support for unprivileged operation, it is
> appropriate to at least make sure that your design is not *wrong* for
> the other users who will want to make use of it, and that we won't have
> to revert and/or fix parts of it later — especially if the later changes
> have compatibility implications for your use case. Is that so hard to
> comprehend?

Once again, it is not wrong... you do not get the idea of wrapper for
privileged network commands. You assume it is wrong and tries to prove
it with no major success, you took one note from the commit message
and start argument.

On Solaris and on *BSD the interface of iproute2 is usable for network
configuration as privileged utility via suid or sudo, this will enable
checking if the configuration is permitted and delegate actual
settings into system utilities. It is better than inventing 3rd party
interface and better than having ifconfig/route interfaces. We used it
successfully in OpenVPN (also in windows) and here in vpnc.

If you think you can do this better, show an example.

If you require to have a complete solution for Linux, *BSD, Solaris at
the same time, please suggest one, as I do not have access to these
environments.

My proposed change does not break *BSD, Solaris, deferring merge of it
only because of a principal game is something I would like not to see.

Of course if someone in future cares enough for unprivileged *BSD,
Solaris some changes might be required for native part, not sure the
wrapper will be any different.

For some reason I needed to use vpnc... so I added these features I
added to OpenVPN to vpnc as well to reach same level of security so I
can use it. I thought that you guys will love to get some help in
achieving more secure setup, but it taking too long and too much
energy, and already has alternative to use OpenVPN, so I continue this
only for fun.

Regards,
Alon

_______________________________________________
vpnc-devel mailing list
vpnc-devel@unix-ag.uni-kl.de
https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel
http://www.unix-ag.uni-kl.de/~massar/vpnc/
Re: [PATCH 3/5] delete tun address on disconnect [ In reply to ]
On Thu, 2014-02-20 at 23:25 +0200, Alon Bar-Lev wrote:
> If you think you can do this better, show an example.

Since you appear to have conceptually difficulty understanding the
*basic* request that you make this patch work for *both* iproute2 and
ifconfig code paths, here's a completely untested example...

diff --git a/vpnc-script b/vpnc-script
index 047e94a..79cf6e2 100755
--- a/vpnc-script
+++ b/vpnc-script
@@ -722,6 +722,31 @@ do_disconnect() {
if [ -n "$INTERNAL_IP4_DNS" ]; then
$RESTORERESOLVCONF
fi
+
+
+ if [ -n "$IPROUTE" ]; then
+ if [ -n "$INTERNAL_IP4_ADDRESS" ]; then
+ $IPROUTE addr del "$INTERNAL_IP4_ADDRESS/255.255.255.255" peer "$INTERNAL_IP4_ADDRESS" dev "$TUNDEV"
+ fi
+ # If the netmask is provided, it contains the address _and_ netmask
+ if [ -n "$INTERNAL_IP6_ADDRESS" ] && [ -z "$INTERNAL_IP6_NETMASK" ]; then
+ INTERNAL_IP6_NETMASK="$INTERNAL_IP6_ADDRESS/128"
+ fi
+ if [ -n "$INTERNAL_IP6_NETMASK" ]; then
+ $IPROUTE -6 addr del $INTERNAL_IP6_NETMASK dev $TUNDEV
+ fi
+ else
+ if [ -n "$INTERNAL_IP4_ADDRESS" ]; then
+ ifconfig "$TUNDEV" 0.0.0.0
+ fi
+ if [ -n "$INTERNAL_IP6_ADDRESS" ] && [ -z "$INTERNAL_IP6_NETMASK" ]; then
+ INTERNAL_IP6_NETMASK="$INTERNAL_IP6_ADDRESS/128"
+ fi
+ if [ -n "$INTERNAL_IP6_NETMASK" ]; then
+ ifconfig "$TUNDEV" inet6 del $INTERNAL_IP6_NETMASK
+ fi
+ fi
+
destroy_tun_device
}


There, was that really so hard?

--
dwmw2
Re: [PATCH 3/5] delete tun address on disconnect [ In reply to ]
On Thu, Feb 20, 2014 at 11:46 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Thu, 2014-02-20 at 23:25 +0200, Alon Bar-Lev wrote:
>> If you think you can do this better, show an example.
>
> Since you appear to have conceptually difficulty understanding the
> *basic* request that you make this patch work for *both* iproute2 and
> ifconfig code paths, here's a completely untested example...
>
> diff --git a/vpnc-script b/vpnc-script
> index 047e94a..79cf6e2 100755
> --- a/vpnc-script
> +++ b/vpnc-script
> @@ -722,6 +722,31 @@ do_disconnect() {
> if [ -n "$INTERNAL_IP4_DNS" ]; then
> $RESTORERESOLVCONF
> fi
> +
> +
> + if [ -n "$IPROUTE" ]; then
> + if [ -n "$INTERNAL_IP4_ADDRESS" ]; then
> + $IPROUTE addr del "$INTERNAL_IP4_ADDRESS/255.255.255.255" peer "$INTERNAL_IP4_ADDRESS" dev "$TUNDEV"
> + fi
> + # If the netmask is provided, it contains the address _and_ netmask
> + if [ -n "$INTERNAL_IP6_ADDRESS" ] && [ -z "$INTERNAL_IP6_NETMASK" ]; then
> + INTERNAL_IP6_NETMASK="$INTERNAL_IP6_ADDRESS/128"
> + fi
> + if [ -n "$INTERNAL_IP6_NETMASK" ]; then
> + $IPROUTE -6 addr del $INTERNAL_IP6_NETMASK dev $TUNDEV
> + fi
> + else
> + if [ -n "$INTERNAL_IP4_ADDRESS" ]; then
> + ifconfig "$TUNDEV" 0.0.0.0
> + fi
> + if [ -n "$INTERNAL_IP6_ADDRESS" ] && [ -z "$INTERNAL_IP6_NETMASK" ]; then
> + INTERNAL_IP6_NETMASK="$INTERNAL_IP6_ADDRESS/128"
> + fi
> + if [ -n "$INTERNAL_IP6_NETMASK" ]; then
> + ifconfig "$TUNDEV" inet6 del $INTERNAL_IP6_NETMASK
> + fi
> + fi
> +
> destroy_tun_device
> }
>
>
> There, was that really so hard?
>

If it was so simple, why wasn't it your initial response?

As I do not have access to Solaris and *BSD, I do not know if
re-execute of vpnc at this state of interface will succeed, I will not
submit patches I cannot verify.

Glad to see progress.

Regards,
Alon
_______________________________________________
vpnc-devel mailing list
vpnc-devel@unix-ag.uni-kl.de
https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel
http://www.unix-ag.uni-kl.de/~massar/vpnc/
Re: [PATCH 3/5] delete tun address on disconnect [ In reply to ]
On Thu, 2014-02-20 at 23:52 +0200, Alon Bar-Lev wrote:
>
> If it was so simple, why wasn't it your initial response?

If someone sends a patch which is incomplete, the first response is to
ask them to fix it, rather than immediately jumping in to do it myself.

This is *especially* true of the patch comes from someone who really
ought to know better.

I spend enough of my life cleaning up after people, without also doing
it for someone who *is* perfectly capable and just seems intent on being
lazy and obtuse.

> As I do not have access to Solaris and *BSD, I do not know if
> re-execute of vpnc at this state of interface will succeed, I will not
> submit patches I cannot verify.

Nonsense. You have as much access to Solaris and *BSD as I do. I only
installed them in virtual machines for the purpose of doing a competent
cross-platform job on VPN stuff. I don't use them for anything *else*.

And I also explained to you how you could test the ifconfig code paths
on Linux too.

You're not some kid in college who's trying to submit a patch to an open
source project for the first time. As I said, you really ought to know
better. You are an embarrassment to your employer.

--
dwmw2
Re: [PATCH 3/5] delete tun address on disconnect [ In reply to ]
On Fri, Feb 21, 2014 at 12:14 AM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Thu, 2014-02-20 at 23:52 +0200, Alon Bar-Lev wrote:
>>
>> If it was so simple, why wasn't it your initial response?
>
> If someone sends a patch which is incomplete, the first response is to
> ask them to fix it, rather than immediately jumping in to do it myself.
>
> This is *especially* true of the patch comes from someone who really
> ought to know better.
>
> I spend enough of my life cleaning up after people, without also doing
> it for someone who *is* perfectly capable and just seems intent on being
> lazy and obtuse.
>
>> As I do not have access to Solaris and *BSD, I do not know if
>> re-execute of vpnc at this state of interface will succeed, I will not
>> submit patches I cannot verify.
>
> Nonsense. You have as much access to Solaris and *BSD as I do. I only
> installed them in virtual machines for the purpose of doing a competent
> cross-platform job on VPN stuff. I don't use them for anything *else*.
>
> And I also explained to you how you could test the ifconfig code paths
> on Linux too.
>
> You're not some kid in college who's trying to submit a patch to an open
> source project for the first time. As I said, you really ought to know
> better. You are an embarrassment to your employer.

Many thanks for the public personal feedback. Not sure how this
progresses the vpnc project.
_______________________________________________
vpnc-devel mailing list
vpnc-devel@unix-ag.uni-kl.de
https://lists.unix-ag.uni-kl.de/mailman/listinfo/vpnc-devel
http://www.unix-ag.uni-kl.de/~massar/vpnc/