From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 602A16290F for ; Tue, 27 Oct 2020 12:39:34 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5182A17650 for ; Tue, 27 Oct 2020 12:39:04 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 309AF17641 for ; Tue, 27 Oct 2020 12:39:03 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id E4ECE45F50; Tue, 27 Oct 2020 12:39:02 +0100 (CET) Date: Tue, 27 Oct 2020 12:39:01 +0100 From: Stoiko Ivanov To: Daniel Berteaud Cc: pmg-devel@lists.proxmox.com Message-ID: <20201027123901.0117e0a5@rosa.proxmox.com> In-Reply-To: <20201026105046.424454-2-daniel@firewall-services.com> References: <20201026105046.424454-1-daniel@firewall-services.com> <20201026105046.424454-2-daniel@firewall-services.com> X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.082 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [dkimsign.pm] Subject: Re: [pmg-devel] [PATCH pmg-api 1/1] [pmg-api]: fix #3098 : first check for exact domain match X-BeenThere: pmg-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Mail Gateway development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Oct 2020 11:39:34 -0000 Hi, Thank you very much for catching this, opening a report and directly providing a patch! The patch works as advertised (gave it a quick spin on my test-setup). However I wonder if it wouldn't be better to address this by sorting properly - instead of differentiating between exact match and first matching subdomain we could match for the longest matching subdomain (suggestion inline). This should cover your issue as well as far as I can tell, and provide fewer suprises regarding the chosen signing domain. What do you think? On Mon, 26 Oct 2020 11:50:46 +0100 Daniel Berteaud wrote: > When selecting the sending domain for the DKIM signature, we should first check for an exact match. If none is found, look for parent domains. This fixes the case where wrong signing domain can be added if sign_all is disabled and we sign both a parent and a child domain. > > Signed-off-by: Daniel Berteaud > --- > src/PMG/DKIMSign.pm | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/src/PMG/DKIMSign.pm b/src/PMG/DKIMSign.pm > index 7cb06a6..8fd9eed 100644 > --- a/src/PMG/DKIMSign.pm > +++ b/src/PMG/DKIMSign.pm > @@ -69,6 +69,14 @@ sub signing_domain { > my $dkimdomains = PVE::INotify::read_file('dkimdomains'); > $dkimdomains = PVE::INotify::read_file('domains') if !scalar(%$dkimdomains); > > + # First check for an exact match in the domain list > + foreach my $domain (sort keys %$dkimdomains) { > + if ( $input_domain eq $domain ) { > + $self->domain($domain); > + return 1; > + } > + } > + # If no exact match is found, check for parent/child domains > foreach my $domain (sort keys %$dkimdomains) { by replacing `sort keys %$dkimdomains` with: `sort { length($b) <=> length($a) || $a cmp $b} keys %$dkimdomains` (from the top of my head - without thorough tests) - sort longest domains first and if the length is equal sort lexically > if ( $input_domain =~ /\Q$domain\E$/i ) { > $self->domain($domain);