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 E98D2906FE for ; Thu, 16 Mar 2023 13:50:45 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C8B4646AE for ; Thu, 16 Mar 2023 13:50:45 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (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 for ; Thu, 16 Mar 2023 13:50:44 +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 112F1437FA for ; Thu, 16 Mar 2023 13:50:44 +0100 (CET) Date: Thu, 16 Mar 2023 13:50:41 +0100 From: Stoiko Ivanov To: Christoph Heiss Cc: pmg-devel@lists.proxmox.com Message-ID: <20230316135041.4de4316e@rosa.proxmox.com> In-Reply-To: <20230309101846.192177-2-c.heiss@proxmox.com> References: <20230309101846.192177-1-c.heiss@proxmox.com> <20230309101846.192177-2-c.heiss@proxmox.com> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.24; 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.143 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pmg-devel] [PATCH pmg-api 1/3] fix #2437: config: Add inbound TLS policy option 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: Thu, 16 Mar 2023 12:50:46 -0000 On Thu, 9 Mar 2023 11:18:44 +0100 Christoph Heiss wrote: > Add a new configuration file /etc/pmg/tls_inbound_policy, which is a > postfix map containing all domains having `reject_plaintext_session` > action set, which is then used in smtpd_sender_restriction in the > main.cf template. > > Also add the accompanying API endpoint for modifying it. I usually split this out into a patch of its own. One thing that is missing is adding the new file to the cluster sync (`git grep tls_policy`). > > Signed-off-by: Christoph Heiss > --- > src/Makefile | 1 + > src/PMG/API2/Config.pm | 7 ++ > src/PMG/API2/InboundTLSPolicy.pm | 127 +++++++++++++++++++++++++++++++ > src/PMG/Config.pm | 56 ++++++++++++++ > src/templates/main.cf.in | 1 + > 5 files changed, 192 insertions(+) > create mode 100644 src/PMG/API2/InboundTLSPolicy.pm > > diff --git a/src/Makefile b/src/Makefile > index 49c7974..139a4b5 100644 > --- a/src/Makefile > +++ b/src/Makefile > @@ -130,6 +130,7 @@ LIBSOURCES = \ > PMG/API2/DKIMSignDomains.pm \ > PMG/API2/DKIMSign.pm \ > PMG/API2/Fetchmail.pm \ > + PMG/API2/InboundTLSPolicy.pm \ > PMG/API2/Users.pm \ > PMG/API2/Transport.pm \ > PMG/API2/MyNetworks.pm \ > diff --git a/src/PMG/API2/Config.pm b/src/PMG/API2/Config.pm > index 37da096..8a8a469 100644 > --- a/src/PMG/API2/Config.pm > +++ b/src/PMG/API2/Config.pm > @@ -23,6 +23,7 @@ use PMG::API2::SMTPWhitelist; > use PMG::API2::MimeTypes; > use PMG::API2::Fetchmail; > use PMG::API2::DestinationTLSPolicy; > +use PMG::API2::InboundTLSPolicy; > use PMG::API2::DKIMSign; > use PMG::API2::SACustom; > use PMG::API2::PBS::Remote; > @@ -86,6 +87,11 @@ __PACKAGE__->register_method ({ > path => 'tlspolicy', > }); > > +__PACKAGE__->register_method ({ > + subclass => "PMG::API2::InboundTLSPolicy", > + path => 'tlsinboundpolicy', > +}); > + > __PACKAGE__->register_method({ > subclass => "PMG::API2::DKIMSign", > path => 'dkim', > @@ -146,6 +152,7 @@ __PACKAGE__->register_method ({ > push @$res, { section => 'ruledb' }; > push @$res, { section => 'tfa' }; > push @$res, { section => 'tlspolicy' }; > + push @$res, { section => 'tlsinboundpolicy' }; > push @$res, { section => 'transport' }; > push @$res, { section => 'users' }; > push @$res, { section => 'whitelist' }; > diff --git a/src/PMG/API2/InboundTLSPolicy.pm b/src/PMG/API2/InboundTLSPolicy.pm > new file mode 100644 > index 0000000..74fb16a > --- /dev/null > +++ b/src/PMG/API2/InboundTLSPolicy.pm > @@ -0,0 +1,127 @@ > +package PMG::API2::InboundTLSPolicy; > + > +use strict; > +use warnings; > + > +use PVE::RESTHandler; > +use PVE::INotify; > +use PVE::Exception qw(raise_param_exc); > + > +use PMG::Config; > + > +use base qw(PVE::RESTHandler); > + > +__PACKAGE__->register_method ({ > + name => 'index', > + path => '', > + method => 'GET', > + description => 'List tls_inbound_policy entries.', > + proxyto => 'master', > + permissions => { check => [ 'admin', 'audit' ] }, > + parameters => { > + additionalProperties => 0, > + properties => {}, > + }, > + returns => { > + type => 'array', > + items => { > + type => 'string', > + format => 'transport-domain', > + }, > + description => 'List of domains for which TLS will be enforced on incoming connections', > + links => [ { rel => 'child', href => '{domain}' } ], > + }, > + code => sub { > + my ($param) = @_; > + > + my $res = []; > + > + my $policies = PVE::INotify::read_file('tls_inbound_policy'); > + > + foreach my $domain (sort keys %$policies) { > + push @$res, { domain => $domain }; > + } > + > + return $res; > + }}); > + > +__PACKAGE__->register_method ({ > + name => 'create', > + path => '', > + method => 'POST', > + proxyto => 'master', > + protected => 1, > + permissions => { check => [ 'admin' ] }, > + description => 'Add new tls_inbound_policy entry.', > + parameters => { > + additionalProperties => 0, > + properties => { > + domain => { > + type => 'string', > + format => 'transport-domain', > + description => 'Domain for which TLS should be enforced on incoming connections', > + }, > + }, > + }, > + returns => { type => 'null' }, > + code => sub { > + my ($param) = @_; > + my $domain = $param->{domain}; > + > + my $code = sub { > + my $policies = PVE::INotify::read_file('tls_inbound_policy'); > + raise_param_exc({ domain => "InboundTLSPolicy entry for '$domain' already exists" }) > + if $policies->{$domain}; > + > + $policies->{$domain} = 1; > + > + PVE::INotify::write_file('tls_inbound_policy', $policies); > + PMG::Config::postmap_tls_inbound_policy(); > + }; > + > + PMG::Config::lock_config($code, 'adding tls_inbound_policy entry failed'); > + > + return undef; > + }}); > + > +__PACKAGE__->register_method ({ > + name => 'delete', > + path => '{domain}', > + method => 'DELETE', > + description => 'Delete a tls_inbound_policy entry', > + protected => 1, > + permissions => { check => [ 'admin' ] }, > + proxyto => 'master', > + parameters => { > + additionalProperties => 0, > + properties => { > + domain => { > + type => 'string', > + format => 'transport-domain', > + description => 'Domain which should be removed from tls_inbound_policy', > + }, > + } > + }, > + returns => { type => 'null' }, > + code => sub { > + my ($param) = @_; > + my $domain = $param->{domain}; > + > + my $code = sub { > + my $policies = PVE::INotify::read_file('tls_inbound_policy'); > + > + raise_param_exc({ domain => "tls_inbound_policy entry for '$domain' does not exist" }) > + if !$policies->{$domain}; > + > + delete $policies->{$domain}; > + > + PVE::INotify::write_file('tls_inbound_policy', $policies); > + PMG::Config::postmap_tls_inbound_policy(); > + }; > + > + PMG::Config::lock_config($code, 'deleting tls_inbound_policy entry failed'); > + > + return undef; > + }}); > + > +1; > diff --git a/src/PMG/Config.pm b/src/PMG/Config.pm > index a0b1866..45a4b3a 100755 > --- a/src/PMG/Config.pm > +++ b/src/PMG/Config.pm > @@ -1154,6 +1154,61 @@ sub postmap_tls_policy { > PMG::Utils::run_postmap($tls_policy_map_filename); > } > > +sub read_tls_inbound_policy { > + my ($filename, $fh) = @_; > + > + return {} if !defined($fh); > + > + my $tls_policy = {}; > + > + while (defined(my $line = <$fh>)) { > + chomp $line; > + next if $line =~ m/^\s*$/; > + next if $line =~ m/^#(.*)\s*$/; > + > + my $parse_error = sub { > + my ($err) = @_; > + die "parse error in '$filename': $line - $err"; > + }; > + > + if ($line =~ m/^(\S+)\s+.+\s*$/) { The matching seems odd - IIRC + is greedy so '.+' above would match everything anyways - making \s* superfluous? Why not explicitly match for 'reject_plain_text_session'? - since we write this literally into the file it should be there. (erroring out on unexpected content is better than to clobber it and replace what the users wrote there with 'reject_plaintext_session' upon any next update (and hopefully motivates the users to not use this particular file for other unrelated ACL entries)) > + my $domain = $1; > + > + eval { pmg_verify_transport_domain($domain) }; > + if (my $err = $@) { > + $parse_error->($err); > + next; > + } > + > + $tls_policy->{$domain} = 1; > + } else { > + $parse_error->('wrong format'); > + } > + } > + > + return $tls_policy; > +} > + > +sub write_tls_inbound_policy { > + my ($filename, $fh, $tls_policy) = @_; > + > + return if !$tls_policy; > + > + foreach my $domain (sort keys %$tls_policy) { > + PVE::Tools::safe_print($filename, $fh, "$domain reject_plaintext_session\n"); > + } > +} > + > +my $tls_inbound_policy_map_filename = "/etc/pmg/tls_inbound_policy"; > +PVE::INotify::register_file('tls_inbound_policy', $tls_inbound_policy_map_filename, > + \&read_tls_inbound_policy, > + \&write_tls_inbound_policy, > + undef, always_call_parser => 1); > + > +sub postmap_tls_inbound_policy { > + PMG::Utils::run_postmap($tls_inbound_policy_map_filename); > +} > + > my $transport_map_filename = "/etc/pmg/transport"; > > sub postmap_pmg_transport { > @@ -1684,6 +1739,7 @@ sub rewrite_config_postfix { > postmap_pmg_domains(); > postmap_pmg_transport(); > postmap_tls_policy(); > + postmap_tls_inbound_policy(); > > rewrite_postfix_whitelist($rulecache) if $rulecache; > > diff --git a/src/templates/main.cf.in b/src/templates/main.cf.in > index 190c913..4905eeb 100644 > --- a/src/templates/main.cf.in > +++ b/src/templates/main.cf.in > @@ -79,6 +79,7 @@ smtpd_sender_restrictions = > reject_non_fqdn_sender > check_client_access cidr:/etc/postfix/clientaccess > check_sender_access regexp:/etc/postfix/senderaccess > + check_sender_access hash:/etc/pmg/tls_inbound_policy > check_recipient_access regexp:/etc/postfix/rcptaccess > [%- IF pmg.mail.rejectunknown %] reject_unknown_client_hostname[% END %] > [%- IF pmg.mail.rejectunknownsender %] reject_unknown_sender_domain[% END %] > -- > 2.39.2 > > > > _______________________________________________ > pmg-devel mailing list > pmg-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel > >