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=-8.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL 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 6B3FDC2D0CD for ; Wed, 18 Dec 2019 11:37:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 40BC824650 for ; Wed, 18 Dec 2019 11:37:45 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="aRf4sMeW" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726940AbfLRLho (ORCPT ); Wed, 18 Dec 2019 06:37:44 -0500 Received: from mail-qk1-f194.google.com ([209.85.222.194]:37541 "EHLO mail-qk1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726701AbfLRLhn (ORCPT ); Wed, 18 Dec 2019 06:37:43 -0500 Received: by mail-qk1-f194.google.com with SMTP id 21so1271881qky.4 for ; Wed, 18 Dec 2019 03:37:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=S+M8i39/jB6Cqau0xv7tzu7ie7N03vkjz45ADcmxSd0=; b=aRf4sMeWzpadW5YJrmob5NdYi06W4xs5sug9fKJBjCXVsXaWQTyeKCMHdYNRW4gUir o2k89xJ7NsawicDftFZF0m4Ydmc7VvW+wymYJJFcE2SA+28K7kUDG+s8NjIwJv8cA0PV RA3WKgbFfPQGjv62mEKfn16vDFaNdFOjC5VzPQh82/dIAsYQSupfL/Z437nK6PumzOPF +h+x6ROL/EONo82GvI6fAxLNWsZb0cEEmfUi3Kg1+v5RoNJbszmIjfgQUTQBMvmPpRU5 mZ1Opk9FCvaWvglpbvXKaK4Qh6GF5guLxxq3DCKPIsFn5/BA0HwFiBtolnHtqM0xEge4 h2uw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=S+M8i39/jB6Cqau0xv7tzu7ie7N03vkjz45ADcmxSd0=; b=plv1N/GK8SDVi/WxIy/2JsAQRCd20I+0i4ho8INCBijiYjPtoJO8UU7SWicoSA6u/y NfUNr0xUbxy4hG8lYyv31w+R3OYpQQcwX8K79xEqDL1+lLILu75hj00+m+mOOI4L0B17 xkZtpbC1/ubMxh4jX4iu40fF6rQBxo+i+gMw9gPia5ZaK3sr6El+SteagxJ88QFsR0qG mkEplcBz9T7UuAa9FvS8QsydcrUhX7vZrdNDgPxTV0NwMD7w0UngQjq45xO29ZGPU4q8 bdiuWjnuYnguSWlTN6zuUChsrBcIRqsPt4hZDLWG/UOBVx5asMdBqiB0WEiW/5VgT4Oo S6Ig== X-Gm-Message-State: APjAAAWP8qCP+6JWLvU0KqopHeGrqDtOeugyP9FzRO+XWHmz8rYezS/K 37m5oPVrYpCoOrOS+wDrnO2FHbXb9FbWjPScOsFD1Q== X-Google-Smtp-Source: APXvYqyKK+FDzDrpsx/z+doLr55we/uUU5vCPbMr4o16ebJXWlTeSoCgL+oyXSSiJUasHPhtkAICWMChBnTXKRzBoGQ= X-Received: by 2002:a05:620a:1136:: with SMTP id p22mr1987714qkk.8.1576669062422; Wed, 18 Dec 2019 03:37:42 -0800 (PST) MIME-Version: 1.0 References: <20191208232734.225161-1-Jason@zx2c4.com> In-Reply-To: From: Dmitry Vyukov Date: Wed, 18 Dec 2019 12:37:30 +0100 Message-ID: Subject: Re: [PATCH net-next v2] net: WireGuard secure network tunnel To: "Jason A. Donenfeld" Cc: netdev , LKML , David Miller , Greg KH , Linus Torvalds , Herbert Xu , "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Dec 18, 2019 at 11:57 AM Jason A. Donenfeld wrote: > > Hi Dmitry, > > On Wed, Dec 18, 2019 at 11:13 AM Dmitry Vyukov wrote: > > Does it really do "verbose debug log"? I only see it is used for > > self-tests and debug checks: > > Yes, it does, via net_dbg and co. People with the Linux "dynamic > debugging" feature turned also get the log by twiddling with some file > at runtime. > > > In different contexts one may enable different sets of these. > > In particular in fuzzing context one absolutely wants additional debug > > checks, but not self tests and definitely no verbose logging. CI and > > various manual scenarios will require different sets as well. > > If this does verbose logging, we won't get debug checks as well during > > fuzzing, which is unfortunate. > > Can make sense splitting CONFIG_WIREGUARD_DEBUG into 2 or 3 separate > > configs (that's what I see frequently). Unfortunately there is no > > standard conventions for anything of this, so CIs will never find your > > boot tests and fuzzing won't find the additional checks... > > I agree that it might make sense to split this up at some point, but > for now I think it might be a bit overkill. Both the self-tests and > debug tests are *very* light at the moment. Down the road if these > become heavier, I think it'd probably be a good idea, but for the time > being it'd mostly be more complexity for nothing. > > Another more interesting point, though, you wrote > > and definitely no verbose logging. > > Actually with WireGuard, I think that's not the case. The WireGuard > logging has been written with DoS in mind. You /should/ be able to > safely run it on a production system exposed to the wild Internet, and > while there will be some additional things in your dmesg, an attacker > isn't supposed to be able to totally flood it without ratelimiting or > inject malicious strings into it (such as ANSI escape sequence). In > other words, I consider the logging to be fair game attack surface. If > your fuzzer manages to craft some nasty sequence of packets that > tricks some rate limiting logic and lets you litter all over dmesg > totally unbounded, I'd consider that a real security bug worth > stressing out about. So from the perspective of letting your fuzzers > loose on WireGuard, I'd actually like to see this option kept on. This is the case even with CONFIG_WIREGUARD_DEBUG turned on, right? Or without? Well, it may be able to trigger unbounded printing, but that won't be detected as a bug and won't be reported. To be reported it needs to fall into a set of predefined bug cases (e.g. "BUG:" or "WARNING:" on console). Unless of course it triggers total stall/hang. But I'm afraid it will just dirty dmesg, make reading crashes harder and slow down everything without benefit. O(1) output per test is generally not OK in heavy stressing scenario, even if it's overall bounded and rate limited.