All of lore.kernel.org
 help / color / mirror / Atom feed
* passtos patch
@ 2018-01-18 11:30 Vadim Zotov
  2018-01-18 11:56 ` Kalin KOZHUHAROV
  2018-01-18 16:11 ` Jason A. Donenfeld
  0 siblings, 2 replies; 7+ messages in thread
From: Vadim Zotov @ 2018-01-18 11:30 UTC (permalink / raw)
  To: WireGuard mailing list


[-- Attachment #1.1: Type: text/plain, Size: 1274 bytes --]

Hello,

in some circumstances it is important to set the TOS field in tunnel
packet equivalent to payload packet TOS.

for example, our provider supports three different SLAs, depending on
packet TOS field, with different jitter,

packet loss and service availability. In current release wireguard
always set tos to 0.

This patch solves that problem.


--- send.c.orig 2017-10-17 20:26:29.000000000 +0300
+++ send.c      2018-01-08 15:10:25.364428109 +0300
@@ -302,7 +302,7 @@
         * all of the packets in the queue. If we can't assign nonces
for all of them,
         * we just consider it a failure and wait for the next handshake. */
        skb_queue_walk (&packets, skb) {
-               PACKET_CB(skb)->ds = ip_tunnel_ecn_encap(0 /* No outer
TOS: no leak. TODO: should we use flowi->tos as outer? */, ip_hdr(skb),
skb);
+               PACKET_CB(skb)->ds =
ip_tunnel_ecn_encap(ipv4_get_dsfield(ip_hdr(skb)) & ~INET_ECN_MASK,
ip_hdr(skb), skb);
                PACKET_CB(skb)->nonce =
atomic64_inc_return(&key->counter.counter) - 1;
                if (unlikely(PACKET_CB(skb)->nonce >=
REJECT_AFTER_MESSAGES))
                        goto out_invalid;


[-- Attachment #1.2: Type: text/html, Size: 1846 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: passtos.patch --]
[-- Type: text/x-patch; name="passtos.patch", Size: 726 bytes --]

--- send.c.orig	2017-10-17 20:26:29.000000000 +0300
+++ send.c	2018-01-08 15:10:25.364428109 +0300
@@ -302,7 +302,7 @@
 	 * all of the packets in the queue. If we can't assign nonces for all of them,
 	 * we just consider it a failure and wait for the next handshake. */
 	skb_queue_walk (&packets, skb) {
-		PACKET_CB(skb)->ds = ip_tunnel_ecn_encap(0 /* No outer TOS: no leak. TODO: should we use flowi->tos as outer? */, ip_hdr(skb), skb);
+		PACKET_CB(skb)->ds = ip_tunnel_ecn_encap(ipv4_get_dsfield(ip_hdr(skb)) & ~INET_ECN_MASK, ip_hdr(skb), skb);
 		PACKET_CB(skb)->nonce = atomic64_inc_return(&key->counter.counter) - 1;
 		if (unlikely(PACKET_CB(skb)->nonce >= REJECT_AFTER_MESSAGES))
 			goto out_invalid;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: passtos patch
  2018-01-18 11:30 passtos patch Vadim Zotov
@ 2018-01-18 11:56 ` Kalin KOZHUHAROV
  2018-01-18 13:03   ` Matthias Urlichs
  2018-01-18 20:57   ` Ivan Labáth
  2018-01-18 16:11 ` Jason A. Donenfeld
  1 sibling, 2 replies; 7+ messages in thread
From: Kalin KOZHUHAROV @ 2018-01-18 11:56 UTC (permalink / raw)
  To: Vadim Zotov; +Cc: WireGuard mailing list

On Thu, Jan 18, 2018 at 12:30 PM, Vadim Zotov <z@zenit.ru> wrote:
> in some circumstances it is important to set the TOS field in tunnel packet equivalent to payload packet TOS.
> for example, our provider supports three different SLAs, depending on packet TOS field, with different jitter,
> packet loss and service availability. In current release wireguard always set tos to 0.
>
Yep, I completely agree to "in some circumstances" :-D

I am not sure how copying the TOS field can be exploited, I guess it
is one of those things "potential hazard, use KISS, avoid till needed
AND assessment can be made"...
Copying the field straight (in plain-text in a way), will provide some
see-through-the-tunnel, which WG is designed NOT to allow, AFAIK.

There are no optional parameters/options in wireguard (well, except
fwmark), but I guess that one should be implemented as an option, at
least at first.
But again, that contradict KISS principle...

Since this is not a common scenario, IMHO, and there are only a
handful TOS worth doing something, a workaround would be to bunch a
few wg tunnels (even bridge them at both ends?), use fwmark and mangle
the TOS with iptables/ift...
Just a suggestion, not tried obviously.

Regards,
Kalin.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: passtos patch
  2018-01-18 11:56 ` Kalin KOZHUHAROV
@ 2018-01-18 13:03   ` Matthias Urlichs
  2018-01-18 20:57   ` Ivan Labáth
  1 sibling, 0 replies; 7+ messages in thread
From: Matthias Urlichs @ 2018-01-18 13:03 UTC (permalink / raw)
  To: wireguard

On 18.01.2018 12:56, Kalin KOZHUHAROV wrote:
> a workaround would be to bunch a
> few wg tunnels (even bridge them at both ends?), use fwmark and mangle
> the TOS with iptables/ift...

So instead of outside information being visible by way of the TOS field
it's now visible by way of different UDP ports we're talking to.

I don't see any advantage here.

In fact I don't see much advantage of passing TOS out in the first
place. Either you have a reliable transit network with short queues, or
you don't. In the former case TOS is useless. In the latter case you
have other problems which a TOS field cannot fix anyway. (OK, this is a
bit more black+white than the Real World, but …)

-- 
-- Matthias Urlichs

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: passtos patch
  2018-01-18 11:30 passtos patch Vadim Zotov
  2018-01-18 11:56 ` Kalin KOZHUHAROV
@ 2018-01-18 16:11 ` Jason A. Donenfeld
  2018-01-18 21:10   ` Eric Light
  2018-01-19  4:04   ` Daniel Kahn Gillmor
  1 sibling, 2 replies; 7+ messages in thread
From: Jason A. Donenfeld @ 2018-01-18 16:11 UTC (permalink / raw)
  To: Vadim Zotov; +Cc: WireGuard mailing list

Not sure the infoleak is worth it.

List: thoughts?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: passtos patch
  2018-01-18 11:56 ` Kalin KOZHUHAROV
  2018-01-18 13:03   ` Matthias Urlichs
@ 2018-01-18 20:57   ` Ivan Labáth
  1 sibling, 0 replies; 7+ messages in thread
From: Ivan Labáth @ 2018-01-18 20:57 UTC (permalink / raw)
  To: Kalin KOZHUHAROV; +Cc: WireGuard mailing list

On Thu, Jan 18, 2018 at 12:56:55PM +0100, Kalin KOZHUHAROV wrote:
> 
> Since this is not a common scenario, IMHO, and there are only a
> handful TOS worth doing something, a workaround would be to bunch a
> few wg tunnels (even bridge them at both ends?), use fwmark and mangle
> the TOS with iptables/ift...
> Just a suggestion, not tried obviously.
> 

Perhaps it would be possible to use one of the **tables packet markings
to store the TOS before it enters the tunnel and restore it on the
encrypted packet.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: passtos patch
  2018-01-18 16:11 ` Jason A. Donenfeld
@ 2018-01-18 21:10   ` Eric Light
  2018-01-19  4:04   ` Daniel Kahn Gillmor
  1 sibling, 0 replies; 7+ messages in thread
From: Eric Light @ 2018-01-18 21:10 UTC (permalink / raw)
  To: wireguard

Hi all,

Wearing my 'Wireguard enthusiast who doesn't know *that* much about crypto, and only uses WG as an end-user'  hat:

It sounds to me like additional complexity, additional code, and additional information leakage, for what seems to be a relatively uncommon scenario, which by the sounds of things could be addressed with fwmark.

Are any of those observations incorrect?

E

--------------------------------------------
Q: Why is this email five sentences or less?
A: http://five.sentenc.es

On Fri, 19 Jan 2018, at 05:11, Jason A. Donenfeld wrote:
> Not sure the infoleak is worth it.
> 
> List: thoughts?
> _______________________________________________
> WireGuard mailing list
> WireGuard@lists.zx2c4.com
> https://lists.zx2c4.com/mailman/listinfo/wireguard

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: passtos patch
  2018-01-18 16:11 ` Jason A. Donenfeld
  2018-01-18 21:10   ` Eric Light
@ 2018-01-19  4:04   ` Daniel Kahn Gillmor
  1 sibling, 0 replies; 7+ messages in thread
From: Daniel Kahn Gillmor @ 2018-01-19  4:04 UTC (permalink / raw)
  To: Jason A. Donenfeld, Vadim Zotov; +Cc: WireGuard mailing list

[-- Attachment #1: Type: text/plain, Size: 1015 bytes --]

On Thu 2018-01-18 17:11:16 +0100, Jason A. Donenfeld wrote:
> Not sure the infoleak is worth it.
>
> List: thoughts?

I don't think the infoleak is worth it.  Certainly not by default.

and i know wg doesn't want to have a lot of fiddly knobs, so if it's not
by default, please don't add a fiddly knob here.

As just one scenario where it's harmful, consider the case where your
ISP wants to sell you VoIP service.  They have a concrete financial
incentive to delay or add jitter to packets coming from you marked with
common VoIP ToS markings if your VoIP connections are not made through
their competing service.  If your VoIP traffic goes out via wireguard,
your ISP will damage it to try to convince you that their service is
what you should be using :/

The goal of wireguard-style tunnelling is to avoid leaking information
about what the user is actively doing.  Let's not introduce exceptions
where we actively try to export otherwise-confidential information
outside the encrypted envelope.

        --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-01-19  4:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-18 11:30 passtos patch Vadim Zotov
2018-01-18 11:56 ` Kalin KOZHUHAROV
2018-01-18 13:03   ` Matthias Urlichs
2018-01-18 20:57   ` Ivan Labáth
2018-01-18 16:11 ` Jason A. Donenfeld
2018-01-18 21:10   ` Eric Light
2018-01-19  4:04   ` Daniel Kahn Gillmor

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.