From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <s.ivanov@proxmox.com>
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 <pmg-devel@lists.proxmox.com>; 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 <pmg-devel@lists.proxmox.com>; 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 <pmg-devel@lists.proxmox.com>; 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 <s.ivanov@proxmox.com>
To: Daniel Berteaud <daniel@firewall-services.com>
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
<pmg-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pmg-devel>,
<mailto:pmg-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pmg-devel/>
List-Post: <mailto:pmg-devel@lists.proxmox.com>
List-Help: <mailto:pmg-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel>,
<mailto:pmg-devel-request@lists.proxmox.com?subject=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 <daniel@firewall-services.com> 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 <daniel@firewall-services.com>
> ---
> 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);