From patchwork Sun Oct 27 13:30:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arkadiusz Kozdra X-Patchwork-Id: 2002951 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=ai9lW4zv; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.openwrt.org (client-ip=2607:7c80:54:3::133; helo=bombadil.infradead.org; envelope-from=openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org; receiver=patchwork.ozlabs.org) Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4XbyBf541xz1xw2 for ; Mon, 28 Oct 2024 00:32:56 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type:List-Help: Reply-To:List-Archive:List-Unsubscribe:List-Subscribe:From:List-Post:List-Id: Message-ID:MIME-Version:Date:Subject:To:Cc:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Owner; bh=cO4y+pCsttuQ7+loBzOIU2p+F69Lk+TvWTpyO2TnBbo=; b=ai9lW4zvFxp9/qqEfjPl68Gzay WtIJyqCVxxsOXoXGiSb6hsrcnJa/MH7hnX41WR4bBNA1V2TFL60wE8c5dw0ybP0XNGtWzBnXp4X2T 1rYx1oAXDHmGypA4vTtudv2dy3wbz+/f5raS2mwAvK5K0JyWE/CWdDmvZNsG83EWw8Tu2237SGM1l 7evkG4XPt3Xik2lVWyvK6NSRJvy4RWOiHVKb41gMtVCdyqfAswUiIktivMnS9Tf9GRlRKaK/PHxVe bAohxlvkij/Kq+dFFhKb++a4D8aTUgcVBqPTQ/KTeCE0j1HSmMhvBCgOQNGKtTO5oCxQChmLH6U5O usCnJ1Jw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1t53MW-00000008GPO-0HC3; Sun, 27 Oct 2024 13:31:20 +0000 To: openwrt-devel@lists.openwrt.org Subject: [PATCH] rules: Prevent empty rule rejecting everything Date: Sun, 27 Oct 2024 14:30:39 +0100 MIME-Version: 1.0 Message-ID: List-Id: OpenWrt Development List List-Post: X-Patchwork-Original-From: Arkadiusz Kozdra via openwrt-devel From: Arkadiusz Kozdra Precedence: list X-Mailman-Version: 2.1.34 X-BeenThere: openwrt-devel@lists.openwrt.org List-Subscribe: , List-Unsubscribe: , List-Archive: Reply-To: Arkadiusz Kozdra List-Help: Sender: "openwrt-devel" Errors-To: openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org The sender domain has a DMARC Reject/Quarantine policy which disallows sending mailing list messages using the original "From" header. To mitigate this problem, the original message has been wrapped automatically by the mailing list software. Previously if the config file contained any empty rule (like a duplicate 'config rule' line), the firewall understood it as rejecting all output traffic (TCP+UDP). The router was therefore functioning properly, but was unable to answer DHCP requests (because they are not ESTABLISHED nor RELATED) nor send any upstream packets, which was hard to diagnose. The change now requires every rule to contain at least one of zone, protocol or target. Signed-off-by: Arkadiusz Kozdra --- rules.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/rules.c b/rules.c index d506a96..bbd9c37 100644 --- a/rules.c +++ b/rules.c @@ -104,6 +104,8 @@ alloc_rule(struct fw3_state *state) static bool check_rule(struct fw3_state *state, struct fw3_rule *r, struct uci_element *e) { + int guessed = 0; + if (!r->enabled) return false; @@ -194,23 +196,35 @@ check_rule(struct fw3_state *state, struct fw3_rule *r, struct uci_element *e) { warn_section("rule", r, e, "has neither a source nor a destination zone assigned " "- assuming an output rule"); + guessed++; } if (list_empty(&r->proto)) { warn_section("rule", r, e, "does not specify a protocol, assuming TCP+UDP"); fw3_parse_protocol(&r->proto, "tcpudp", true); + guessed++; } if (r->target == FW3_FLAG_UNSPEC) { warn_section("rule", r, e, "has no target specified, defaulting to REJECT"); r->target = FW3_FLAG_REJECT; + guessed++; } else if (r->target > FW3_FLAG_DSCP) { warn_section("rule", r, e, "has invalid target specified, defaulting to REJECT"); r->target = FW3_FLAG_REJECT; + guessed++; + } + + if (guessed > 2) + { + /* empty config rule would reject all output TCP+UDP */ + warn_section("rule", r, e, "must specify at least one valid value of " + "source/destination zone, protocol and target"); + return false; } /* NB: r family... */