From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.9 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 61DA1C83004 for ; Tue, 28 Apr 2020 09:11:20 +0000 (UTC) Received: from krantz.zx2c4.com (krantz.zx2c4.com [192.95.5.69]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0EC65206E2 for ; Tue, 28 Apr 2020 09:11:19 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="hnUHL4Ze" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0EC65206E2 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=wireguard-bounces@lists.zx2c4.com Received: by krantz.zx2c4.com (ZX2C4 Mail Server) with ESMTP id 35942d22; Tue, 28 Apr 2020 08:59:16 +0000 (UTC) Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.61]) by krantz.zx2c4.com (ZX2C4 Mail Server) with ESMTP id 03f3524e for ; Tue, 28 Apr 2020 08:59:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1588065050; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=u3xuxjNfVBNEZttbPvmOjVEPPldZq5/VBaGsumlIgP8=; b=hnUHL4Ze43DOpGvU2B6KJMd0SAo3L82Zd+48irijoRqC8g+ZyBREOeyWuHQChZp547EXma 5C/8zEgVzdq/tn8bxsuB1TrDwKNLS4O1s0GXjLoQgmo1dyAL4GW/9hIq6BaG3RaUhzNgWR qF5wN18JTsW2r/1ix4Emk2hrbyFXW0s= Received: from mail-lf1-f71.google.com (mail-lf1-f71.google.com [209.85.167.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-380-meEamhbnOhiPAWVIg_aS1Q-1; Tue, 28 Apr 2020 05:10:48 -0400 X-MC-Unique: meEamhbnOhiPAWVIg_aS1Q-1 Received: by mail-lf1-f71.google.com with SMTP id g5so8678078lfh.9 for ; Tue, 28 Apr 2020 02:10:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version; bh=mOdBWJKQxFlNabMwjODinAvYOR6ZDK7O0dodfKx0hcc=; b=kMm62oZM++reBnwDg0OXQZhtTIWJjuUbOcGs8Eo2E5Z7uyqIlfmCJ3Ea4Ed8IZ43e2 FssKN5Zjk2NF39EHzOYm4MNx/KwnTAJYYc5OxSyPf24qdxT0FDyAO/8ogjINiiqzHdFr BnSvNf3Dk0y3SCQhTbFipoHjx957FwuZhE0yFFpeATFZH4U5Ch7XiLmFRYhKLAcNGbDi bWPHjxHtL71Oil0M1chx79jnGedZPxj0q4vnksOuDo9oVfNUP5Fwoj5xSVdSZFRMH3Iv JsgthuTd26AtVJaCXkhRTqlnHfdztHo9kNYsuEZP97n83KYIBTeD736phmOD8og/NONO y7ZA== X-Gm-Message-State: AGi0PuZRBPaqEYLU4VxKQ+xV+B+n7iz5CE8MPnYU+CSQQ8sO9+woPW8V arK2H6wkjBK6jC9MCIdL3hTEOSz1V93cRR9hgeirNFZ200iDfTTGNJxl16t0hNttyL/dzk2pUPG xKKWOHOxws92UMuZHxT8+ X-Received: by 2002:ac2:57cb:: with SMTP id k11mr18503632lfo.19.1588065046851; Tue, 28 Apr 2020 02:10:46 -0700 (PDT) X-Google-Smtp-Source: APiQypIH/hZQ/rx7Ol7j9CLUYi6IHDh/x4Luka4wg/zRVzQni3QvyQRdLbuPtHuphYfxyjJMAsZnBg== X-Received: by 2002:ac2:57cb:: with SMTP id k11mr18503620lfo.19.1588065046546; Tue, 28 Apr 2020 02:10:46 -0700 (PDT) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id b28sm13669623lfo.46.2020.04.28.02.10.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Apr 2020 02:10:45 -0700 (PDT) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id DE4981814FF; Tue, 28 Apr 2020 11:10:43 +0200 (CEST) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= To: "Rodney W. Grimes" Cc: "Jason A. Donenfeld" , David Miller , Netdev , WireGuard mailing list , Olivier Tilmans , Dave Taht , "Rodney W . Grimes" Subject: Re: [PATCH net] wireguard: Use tunnel helpers for decapsulating ECN markings In-Reply-To: <202004280109.03S19SCY001751@gndrsh.dnsmgr.net> References: <202004280109.03S19SCY001751@gndrsh.dnsmgr.net> X-Clacks-Overhead: GNU Terry Pratchett Date: Tue, 28 Apr 2020 11:10:43 +0200 Message-ID: <87zhawvuuk.fsf@toke.dk> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain Content-Transfer-Encoding: quoted-printable X-BeenThere: wireguard@lists.zx2c4.com X-Mailman-Version: 2.1.30rc1 Precedence: list List-Id: Development discussion of WireGuard List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: wireguard-bounces@lists.zx2c4.com Sender: "WireGuard" "Rodney W. Grimes" writes: > Replying to a single issue I am reading, and really hope I > am miss understanding. I am neither a wireguard or linux > user so I may be miss understanding what is said. > > Inline at {RWG} > >> "Jason A. Donenfeld" writes: >>=20 >> > Hey Toke, >> > >> > Thanks for fixing this. I wasn't aware there was a newer ECN RFC. A >> > few comments below: >> > >> > On Mon, Apr 27, 2020 at 8:47 AM Toke H?iland-J?rgensen wrote: >> >> RFC6040 also recommends dropping packets on certain combinations of >> >> erroneous code points on the inner and outer packet headers which sho= uldn't >> >> appear in normal operation. The helper signals this by a return value= > 1, >> >> so also add a handler for this case. >> > >> > This worries me. In the old implementation, we propagate some outer >> > header data to the inner header, which is technically an authenticity >> > violation, but minor enough that we let it slide. This patch here >> > seems to make that violation a bit worse: namely, we're now changing >> > the behavior based on a combination of outer header + inner header. An >> > attacker can manipulate the outer header (set it to CE) in order to >> > learn whether the inner header was CE or not, based on whether or not >> > the packet gets dropped, which is often observable. That's some form > > Why is anyone dropping on decap over the CE bit? It should be passed > on, not lead to a packet drop. If the outer header is CE on an inner > header of CE it should just continue to be a CE, dropping it is actually > breaking the purpose of the CE codepoint, to signal congestion before > having to cause a packet loss. > >> > of an oracle, which I'm not too keen on having in wireguard. On the >> > other hand, we pretty much already _explicitly leak this bit_ on tx >> > side -- in send.c: >> > >> > PACKET_CB(skb)->ds =3D ip_tunnel_ecn_encap(0, ip_hdr(skb), skb); // in= ner packet >> > ... >> > wg_socket_send_skb_to_peer(peer, skb, PACKET_CB(skb)->ds); // outer pa= cket >> > >> > We considered that leak a-okay. But a decryption oracle seems slightly >> > worse than an explicit and intentional leak. But maybe not that much >> > worse. >>=20 >> Well, seeing as those two bits on the outer header are already copied >> from the inner header, there's no additional leak added by this change, >> is there? An in-path observer could set CE and observe that the packet >> gets dropped, but all they would learn is that the bits were zero > > Again why is CE leading to anyone dropping? > >> (non-ECT). Which they already knew because they could just read the bits >> directly from the header. >>=20 >> Also note, BTW, that another difference between RFC 3168 and 6040 is the >> propagation of ECT(1) from outer to inner header. That's not actually >> done correctly in Linux ATM, but I sent a separate patch to fix this[0], >> which Wireguard will also benefit from with this patch. > > Thanks for this. > >>=20 >> > I wanted to check with you: is the analysis above correct? And can you >> > somehow imagine the =3D=3D2 case leading to different behavior, in whi= ch >> > the packet isn't dropped? Or would that ruin the "[de]congestion" part >> > of ECN? I just want to make sure I understand the full picture before >> > moving in one direction or another. >>=20 >> So I think the logic here is supposed to be that if there are CE marks >> on the outer header, then an AQM somewhere along the path has marked the >> packet, which is supposed to be a congestion signal, which we want to >> propagate all the way to the receiver (who will then echo it back to the >> receiver). However, if the inner packet is non-ECT then we can't >> actually propagate the ECN signal; and a drop is thus the only >> alternative congestion signal available to us. > > You cannot get a CE mark on the outer packet if the inner packet is > not ECT, as the outer packet would also be not ECT and thus not > eligible for CE mark. If you get the above sited condition something > has gone *wrong*. Yup, you're quite right. If everything is working correctly, this should never happen. This being the internet, though, there are bound to be cases where it will go wrong :) >> This case shouldn't >> actually happen that often, a middlebox has to be misconfigured to >> CE-mark a non-ECT packet in the first place. But, well, misconfigured >> middleboxes do exist as you're no doubt aware :) > > That is true, though I believe the be liberal in what you accept > concept would say ok, someone messed up, just propogate it and > let the end nodes deal with it, otherwise your creating a blackhole > that could be very hard to find. But that would lead you to ignore a congestion signal. And someone has to go through an awful lot of trouble to set this signal; if they're just randomly mangling bits the packet checksum will likely be wrong and the packet would be dropped anyway. So on balance I'd tend to agree with the RFC that the right thing to do is to propagate the congestion signal; which in the case of a non-ECT packet means dropping it, otherwise we'd just be contributing to the RFC-violating behaviour... I do believe the advice in the RFC to log these cases is exactly because of the risk of blackholes you're referring to. I discussed this a bit with Jason and we ended up agreeing that just marking it as a framing error should be enough for Wireguard, though... -Toke