Re: [PATCH net-next v2] net: WireGuard secure network tunnel

[Jason A. Donenfeld]

Hi Dmitry,

On Wed, Dec 18, 2019 at 12:37 PM Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote:
> > 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?

Turned on.

> 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.

Bummer. Well, at least the stall case is interesting.

> But I'm
> afraid it will just dirty dmesg, make reading crashes harder and slow
> down everything without benefit.

Actually the point of the logging is usually to make it more obvious
why a crash has come about, to provide some trail about the sequence
of events. This was especially helpful in fixing old race conditions
where subtle packet timing caused WireGuard's timer-based state
machine to go haywire. Is syzkaller able to backtrack from crashes to
the packets and packet timing that caused them, in order to make a
test case to replay the crash? Is this precise enough for race
condition bugs? If so, then when debugging the crashes I could always
replay it later with logging turned on, in which case it might make
sense to split out the debug logging into CONFIG_WIREGUARD_VERBOSE_LOG
or similar (unless the logging itself changes the timing constraints
and I can't repro that way). If this isn't possible, then it seems
like logging might be something we would benefit from having in the
crash reports, right? Or am I missing some other detail of how the
system works?

Jason