[pmg-devel] [PATCH pmg-{api, gui, docs} 0/3] fix #2437: Add TLS inbound policy for sender domains

[pmg-devel] [PATCH pmg-{api, gui, docs} 0/3] fix #2437: Add TLS inbound policy for sender domains

[Christoph Heiss]

TL;DR: Implements the approach as laid out by Stoiko in the Bugzilla
ticket [0].

A new API endpoint is added - /api2/json/config/tlsinboundpolicy. This
is used to configure the newly introduced postfix map at
/etc/pmg/tls_inbound_policy, specifying sender domains which get the
`reject_plaintext_session` action [1] set, thus requiring TLS-encrypted
sessions on inbound connections.

On the GUI side, a new panel is added in Configuration -> Mail Proxy ->
TLS, where the domains can be specified for this new policy.

The documentation changes are quite terse, maybe I should expand a bit
more on that topic? (Although the TLS destination policy is only lightly
documented as well, as far as I could see.)

Testing
-------
Tested this to the best of my knowledge, by adding some domains as TLS
inbound policy and using `curl` to send some simple mails:

  echo '' | curl -skv smtp://<host> -T - \
    --mail-from foo@localhost.localdomain \
    --mail-rcpt bar@localhost.localdomain

.. where `localhost.localdomain` is on the new 'TLS Inboud Policy' list.
This will now fail with:

  450 4.7.1 Session encryption is required

When additionally adding the `--ssl-reqd` option to curl (instructing it
to require a TLS-encrypted session), the above command will succeed.

(Also tested it with a domain not on the list, checking that no
regressions are introduced.)

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=2437
[1] http://www.postfix.org/postconf.5.html#reject_plaintext_session

---
pmg-api:

Christoph Heiss (1):
      fix #2437: config: Add inbound TLS policy option

 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(+)

pmg-gui:

Christoph Heiss (1):
      fix #2437: proxy: Add 'TLS Inbound Policy' panel

 js/MailProxyTLSInboundPolicy.js | 93 +++++++++++++++++++++++++++++++++++++++++
 js/MailProxyTLSPanel.js         |  8 +++-
 js/Makefile                     |  1 +
 3 files changed, 101 insertions(+), 1 deletion(-)

pmg-docs:

Christoph Heiss (1):
      pmgconfig: Explain new TLS inbound policy configuration

 pmgconfig.adoc | 8 ++++++++
 1 file changed, 8 insertions(+)

--
2.39.2

[pmg-devel] [PATCH pmg-api 1/3] fix #2437: config: Add inbound TLS policy option

[Christoph Heiss]

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.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 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*$/) {
+	    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

Re: [pmg-devel] [PATCH pmg-api 1/3] fix #2437: config: Add inbound TLS policy option

[Stoiko Ivanov]

On Thu,  9 Mar 2023 11:18:44 +0100
Christoph Heiss <c.heiss@proxmox.com> 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 <c.heiss@proxmox.com>
> ---
>  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
> 
> 

Re: [pmg-devel] [PATCH pmg-api 1/3] fix #2437: config: Add inbound TLS policy option

[Christoph Heiss]

On Thu, Mar 16, 2023 at 01:50:41PM +0100, Stoiko Ivanov wrote:
> On Thu,  9 Mar 2023 11:18:44 +0100
> Christoph Heiss <c.heiss@proxmox.com> 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.
Ack.

>
> One thing that is missing is adding the new file to the cluster sync (`git
> grep tls_policy`).
Thanks, didn't know that - will fix this for the next revision!

>
>
> > [..]
> > +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?
I mostly copied this straight from read_tls_policy(), so that's why ..

>
> 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))
That seems _very_ sensible, especially erroring out on entries with
anything other than `reject_plaintext_session` set to prevent users from
mis-using this file.

I will rework this for v2.

>
> [..]
>

[pmg-devel] [PATCH pmg-gui 2/3] fix #2437: proxy: Add 'TLS Inbound Policy' panel

[Christoph Heiss]

This panel can be used to configure sender domains for which TLS will be
enforced my postfix. As this takes the usual transport domain format,
either a FQDN or .FQDN (for matching subdomains) can be specified.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 js/MailProxyTLSInboundPolicy.js | 93 +++++++++++++++++++++++++++++++++
 js/MailProxyTLSPanel.js         |  8 ++-
 js/Makefile                     |  1 +
 3 files changed, 101 insertions(+), 1 deletion(-)
 create mode 100644 js/MailProxyTLSInboundPolicy.js

diff --git a/js/MailProxyTLSInboundPolicy.js b/js/MailProxyTLSInboundPolicy.js
new file mode 100644
index 0000000..bc45527
--- /dev/null
+++ b/js/MailProxyTLSInboundPolicy.js
@@ -0,0 +1,93 @@
+Ext.define('pmg-tls-inbound-policy', {
+    extend: 'Ext.data.Model',
+    fields: ['domain'],
+    idProperty: 'domain',
+    proxy: {
+	type: 'proxmox',
+	url: '/api2/json/config/tlsinboundpolicy',
+    },
+    sorters: {
+	property: 'domain',
+	direction: 'ASC',
+    },
+});
+
+Ext.define('PMG.TLSInboundPolicyEdit', {
+    extend: 'Proxmox.window.Edit',
+    xtype: 'pmgTLSInboundPolicyEdit',
+    onlineHelp: 'pmgconfig_mailproxy_tls',
+
+    subject: gettext('TLS Inbound Policy'),
+    url: '/api2/extjs/config/tlsinboundpolicy',
+    method: 'POST',
+
+    items: [
+	{
+	    xtype: 'proxmoxtextfield',
+	    name: 'domain',
+	    fieldLabel: gettext('Domain'),
+	},
+    ],
+});
+
+Ext.define('PMG.MailProxyTLSInboundPolicy', {
+    extend: 'Ext.grid.GridPanel',
+    alias: ['widget.pmgMailProxyTLSInboundPolicy'],
+
+    viewConfig: {
+	trackOver: false,
+    },
+
+    columns: [
+	{
+	    header: gettext('Domain'),
+	    flex: 1,
+	    sortable: true,
+	    dataIndex: 'domain',
+	},
+    ],
+
+    initComponent: function() {
+	const me = this;
+
+	const rstore = Ext.create('Proxmox.data.UpdateStore', {
+	    model: 'pmg-tls-inbound-policy',
+	    storeid: 'pmg-mailproxy-tls-inbound-policy-store-' + ++Ext.idSeed,
+	});
+
+	const store = Ext.create('Proxmox.data.DiffStore', { rstore: rstore });
+	const reload = () => rstore.load();
+	me.selModel = Ext.create('Ext.selection.RowModel', {});
+	Proxmox.Utils.monStoreErrors(me, store, true);
+
+	Ext.apply(me, {
+	    store,
+	    tbar: [
+		{
+		    text: gettext('Create'),
+		    handler: () => {
+			Ext.createWidget('pmgTLSInboundPolicyEdit', {
+			    autoShow: true,
+			    listeners: {
+				destroy: reload,
+			    },
+			});
+		    },
+		},
+		{
+		    xtype: 'proxmoxStdRemoveButton',
+		    baseurl: '/config/tlsinboundpolicy',
+		    callback: reload,
+		    waitMsgTarget: me,
+		},
+	    ],
+	    listeners: {
+		activate: rstore.startUpdate,
+		destroy: rstore.stopUpdate,
+		deactivate: rstore.stopUpdate,
+	    },
+	});
+
+	me.callParent();
+    },
+});
diff --git a/js/MailProxyTLSPanel.js b/js/MailProxyTLSPanel.js
index 82dc3f8..5a5837c 100644
--- a/js/MailProxyTLSPanel.js
+++ b/js/MailProxyTLSPanel.js
@@ -26,11 +26,17 @@ Ext.define('PMG.MailProxyTLSPanel', {
 	    flex: 1,
 	});

-	me.items = [tlsSettings, tlsDestinations];
+	const tlsInboundPolicy = Ext.create('PMG.MailProxyTLSInboundPolicy', {
+	    title: gettext('TLS Inbound Policy'),
+	    flex: 1,
+	});
+
+	me.items = [tlsSettings, tlsDestinations, tlsInboundPolicy];

 	me.callParent();

 	tlsSettings.relayEvents(me, ['activate', 'deactivate', 'destroy']);
 	tlsDestinations.relayEvents(me, ['activate', 'deactivate', 'destroy']);
+	tlsInboundPolicy.relayEvents(me, ['activate', 'deactivate', 'destroy']);
     },
 });
diff --git a/js/Makefile b/js/Makefile
index 9a2bcf2..e3b9e78 100644
--- a/js/Makefile
+++ b/js/Makefile
@@ -50,6 +50,7 @@ JSSRC=							\
 	MailProxyTLS.js					\
 	MailProxyTLSPanel.js				\
 	MailProxyTLSDestinations.js			\
+	MailProxyTLSInboundPolicy.js			\
 	Transport.js					\
 	MyNetworks.js					\
 	RelayDomains.js					\
--
2.39.2

Re: [pmg-devel] [PATCH pmg-gui 2/3] fix #2437: proxy: Add 'TLS Inbound Policy' panel

[Stoiko Ivanov]

only a question/hint - since this is yet another list of domains (with w/o
added value) - any chance to merge this with MailProxyTLSDestinations?
(or DKIMDomains, or Domains)?

OTOH the component is small enough and refactoring can very well happen at
another time as well) - so feel free to leave that out :)

On Thu,  9 Mar 2023 11:18:45 +0100
Christoph Heiss <c.heiss@proxmox.com> wrote:

> This panel can be used to configure sender domains for which TLS will be
> enforced my postfix. As this takes the usual transport domain format,
> either a FQDN or .FQDN (for matching subdomains) can be specified.
> 
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>  js/MailProxyTLSInboundPolicy.js | 93 +++++++++++++++++++++++++++++++++
>  js/MailProxyTLSPanel.js         |  8 ++-
>  js/Makefile                     |  1 +
>  3 files changed, 101 insertions(+), 1 deletion(-)
>  create mode 100644 js/MailProxyTLSInboundPolicy.js
> 
> diff --git a/js/MailProxyTLSInboundPolicy.js b/js/MailProxyTLSInboundPolicy.js
> new file mode 100644
> index 0000000..bc45527
> --- /dev/null
> +++ b/js/MailProxyTLSInboundPolicy.js
> @@ -0,0 +1,93 @@
> +Ext.define('pmg-tls-inbound-policy', {
> +    extend: 'Ext.data.Model',
> +    fields: ['domain'],
> +    idProperty: 'domain',
> +    proxy: {
> +	type: 'proxmox',
> +	url: '/api2/json/config/tlsinboundpolicy',
> +    },
> +    sorters: {
> +	property: 'domain',
> +	direction: 'ASC',
> +    },
> +});
> +
> +Ext.define('PMG.TLSInboundPolicyEdit', {
> +    extend: 'Proxmox.window.Edit',
> +    xtype: 'pmgTLSInboundPolicyEdit',
> +    onlineHelp: 'pmgconfig_mailproxy_tls',
> +
> +    subject: gettext('TLS Inbound Policy'),
> +    url: '/api2/extjs/config/tlsinboundpolicy',
> +    method: 'POST',
> +
> +    items: [
> +	{
> +	    xtype: 'proxmoxtextfield',
> +	    name: 'domain',
> +	    fieldLabel: gettext('Domain'),
> +	},
> +    ],
> +});
> +
> +Ext.define('PMG.MailProxyTLSInboundPolicy', {
> +    extend: 'Ext.grid.GridPanel',
> +    alias: ['widget.pmgMailProxyTLSInboundPolicy'],
> +
> +    viewConfig: {
> +	trackOver: false,
> +    },
> +
> +    columns: [
> +	{
> +	    header: gettext('Domain'),
> +	    flex: 1,
> +	    sortable: true,
> +	    dataIndex: 'domain',
> +	},
> +    ],
> +
> +    initComponent: function() {
> +	const me = this;
> +
> +	const rstore = Ext.create('Proxmox.data.UpdateStore', {
> +	    model: 'pmg-tls-inbound-policy',
> +	    storeid: 'pmg-mailproxy-tls-inbound-policy-store-' + ++Ext.idSeed,
> +	});
> +
> +	const store = Ext.create('Proxmox.data.DiffStore', { rstore: rstore });
> +	const reload = () => rstore.load();
> +	me.selModel = Ext.create('Ext.selection.RowModel', {});
> +	Proxmox.Utils.monStoreErrors(me, store, true);
> +
> +	Ext.apply(me, {
> +	    store,
> +	    tbar: [
> +		{
> +		    text: gettext('Create'),
> +		    handler: () => {
> +			Ext.createWidget('pmgTLSInboundPolicyEdit', {
> +			    autoShow: true,
> +			    listeners: {
> +				destroy: reload,
> +			    },
> +			});
> +		    },
> +		},
> +		{
> +		    xtype: 'proxmoxStdRemoveButton',
> +		    baseurl: '/config/tlsinboundpolicy',
> +		    callback: reload,
> +		    waitMsgTarget: me,
> +		},
> +	    ],
> +	    listeners: {
> +		activate: rstore.startUpdate,
> +		destroy: rstore.stopUpdate,
> +		deactivate: rstore.stopUpdate,
> +	    },
> +	});
> +
> +	me.callParent();
> +    },
> +});
> diff --git a/js/MailProxyTLSPanel.js b/js/MailProxyTLSPanel.js
> index 82dc3f8..5a5837c 100644
> --- a/js/MailProxyTLSPanel.js
> +++ b/js/MailProxyTLSPanel.js
> @@ -26,11 +26,17 @@ Ext.define('PMG.MailProxyTLSPanel', {
>  	    flex: 1,
>  	});
> 
> -	me.items = [tlsSettings, tlsDestinations];
> +	const tlsInboundPolicy = Ext.create('PMG.MailProxyTLSInboundPolicy', {
> +	    title: gettext('TLS Inbound Policy'),
> +	    flex: 1,
> +	});
> +
> +	me.items = [tlsSettings, tlsDestinations, tlsInboundPolicy];
> 
>  	me.callParent();
> 
>  	tlsSettings.relayEvents(me, ['activate', 'deactivate', 'destroy']);
>  	tlsDestinations.relayEvents(me, ['activate', 'deactivate', 'destroy']);
> +	tlsInboundPolicy.relayEvents(me, ['activate', 'deactivate', 'destroy']);
>      },
>  });
> diff --git a/js/Makefile b/js/Makefile
> index 9a2bcf2..e3b9e78 100644
> --- a/js/Makefile
> +++ b/js/Makefile
> @@ -50,6 +50,7 @@ JSSRC=							\
>  	MailProxyTLS.js					\
>  	MailProxyTLSPanel.js				\
>  	MailProxyTLSDestinations.js			\
> +	MailProxyTLSInboundPolicy.js			\
>  	Transport.js					\
>  	MyNetworks.js					\
>  	RelayDomains.js					\
> --
> 2.39.2
> 
> 
> 
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
> 
> 

Re: [pmg-devel] [PATCH pmg-gui 2/3] fix #2437: proxy: Add 'TLS Inbound Policy' panel

[Christoph Heiss]

On Thu, Mar 16, 2023 at 01:32:03PM +0100, Stoiko Ivanov wrote:
> only a question/hint - since this is yet another list of domains (with w/o
> added value) - any chance to merge this with MailProxyTLSDestinations?
> (or DKIMDomains, or Domains)?
>
> OTOH the component is small enough and refactoring can very well happen at
> another time as well) - so feel free to leave that out :)
I would do it as a follow-up patch(-series) to this, as to not block
this series on that needlessly :^)

Didn't know there were more such instance other than
MailProxyTLSDestinations, I'll have a look at it and unify them as best
as possible.

>
> On Thu,  9 Mar 2023 11:18:45 +0100
> Christoph Heiss <c.heiss@proxmox.com> wrote:
>
> > This panel can be used to configure sender domains for which TLS will be
> > enforced my postfix. As this takes the usual transport domain format,
> > either a FQDN or .FQDN (for matching subdomains) can be specified.
> >
> > Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> > ---
> >  js/MailProxyTLSInboundPolicy.js | 93 +++++++++++++++++++++++++++++++++
> >  js/MailProxyTLSPanel.js         |  8 ++-
> >  js/Makefile                     |  1 +
> >  3 files changed, 101 insertions(+), 1 deletion(-)
> >  create mode 100644 js/MailProxyTLSInboundPolicy.js
> >
> > diff --git a/js/MailProxyTLSInboundPolicy.js b/js/MailProxyTLSInboundPolicy.js
> > new file mode 100644
> > index 0000000..bc45527
> > --- /dev/null
> > +++ b/js/MailProxyTLSInboundPolicy.js
> > @@ -0,0 +1,93 @@
> > +Ext.define('pmg-tls-inbound-policy', {
> > +    extend: 'Ext.data.Model',
> > +    fields: ['domain'],
> > +    idProperty: 'domain',
> > +    proxy: {
> > +	type: 'proxmox',
> > +	url: '/api2/json/config/tlsinboundpolicy',
> > +    },
> > +    sorters: {
> > +	property: 'domain',
> > +	direction: 'ASC',
> > +    },
> > +});
> > +
> > +Ext.define('PMG.TLSInboundPolicyEdit', {
> > +    extend: 'Proxmox.window.Edit',
> > +    xtype: 'pmgTLSInboundPolicyEdit',
> > +    onlineHelp: 'pmgconfig_mailproxy_tls',
> > +
> > +    subject: gettext('TLS Inbound Policy'),
> > +    url: '/api2/extjs/config/tlsinboundpolicy',
> > +    method: 'POST',
> > +
> > +    items: [
> > +	{
> > +	    xtype: 'proxmoxtextfield',
> > +	    name: 'domain',
> > +	    fieldLabel: gettext('Domain'),
> > +	},
> > +    ],
> > +});
> > +
> > +Ext.define('PMG.MailProxyTLSInboundPolicy', {
> > +    extend: 'Ext.grid.GridPanel',
> > +    alias: ['widget.pmgMailProxyTLSInboundPolicy'],
> > +
> > +    viewConfig: {
> > +	trackOver: false,
> > +    },
> > +
> > +    columns: [
> > +	{
> > +	    header: gettext('Domain'),
> > +	    flex: 1,
> > +	    sortable: true,
> > +	    dataIndex: 'domain',
> > +	},
> > +    ],
> > +
> > +    initComponent: function() {
> > +	const me = this;
> > +
> > +	const rstore = Ext.create('Proxmox.data.UpdateStore', {
> > +	    model: 'pmg-tls-inbound-policy',
> > +	    storeid: 'pmg-mailproxy-tls-inbound-policy-store-' + ++Ext.idSeed,
> > +	});
> > +
> > +	const store = Ext.create('Proxmox.data.DiffStore', { rstore: rstore });
> > +	const reload = () => rstore.load();
> > +	me.selModel = Ext.create('Ext.selection.RowModel', {});
> > +	Proxmox.Utils.monStoreErrors(me, store, true);
> > +
> > +	Ext.apply(me, {
> > +	    store,
> > +	    tbar: [
> > +		{
> > +		    text: gettext('Create'),
> > +		    handler: () => {
> > +			Ext.createWidget('pmgTLSInboundPolicyEdit', {
> > +			    autoShow: true,
> > +			    listeners: {
> > +				destroy: reload,
> > +			    },
> > +			});
> > +		    },
> > +		},
> > +		{
> > +		    xtype: 'proxmoxStdRemoveButton',
> > +		    baseurl: '/config/tlsinboundpolicy',
> > +		    callback: reload,
> > +		    waitMsgTarget: me,
> > +		},
> > +	    ],
> > +	    listeners: {
> > +		activate: rstore.startUpdate,
> > +		destroy: rstore.stopUpdate,
> > +		deactivate: rstore.stopUpdate,
> > +	    },
> > +	});
> > +
> > +	me.callParent();
> > +    },
> > +});
> > diff --git a/js/MailProxyTLSPanel.js b/js/MailProxyTLSPanel.js
> > index 82dc3f8..5a5837c 100644
> > --- a/js/MailProxyTLSPanel.js
> > +++ b/js/MailProxyTLSPanel.js
> > @@ -26,11 +26,17 @@ Ext.define('PMG.MailProxyTLSPanel', {
> >  	    flex: 1,
> >  	});
> >
> > -	me.items = [tlsSettings, tlsDestinations];
> > +	const tlsInboundPolicy = Ext.create('PMG.MailProxyTLSInboundPolicy', {
> > +	    title: gettext('TLS Inbound Policy'),
> > +	    flex: 1,
> > +	});
> > +
> > +	me.items = [tlsSettings, tlsDestinations, tlsInboundPolicy];
> >
> >  	me.callParent();
> >
> >  	tlsSettings.relayEvents(me, ['activate', 'deactivate', 'destroy']);
> >  	tlsDestinations.relayEvents(me, ['activate', 'deactivate', 'destroy']);
> > +	tlsInboundPolicy.relayEvents(me, ['activate', 'deactivate', 'destroy']);
> >      },
> >  });
> > diff --git a/js/Makefile b/js/Makefile
> > index 9a2bcf2..e3b9e78 100644
> > --- a/js/Makefile
> > +++ b/js/Makefile
> > @@ -50,6 +50,7 @@ JSSRC=							\
> >  	MailProxyTLS.js					\
> >  	MailProxyTLSPanel.js				\
> >  	MailProxyTLSDestinations.js			\
> > +	MailProxyTLSInboundPolicy.js			\
> >  	Transport.js					\
> >  	MyNetworks.js					\
> >  	RelayDomains.js					\
> > --
> > 2.39.2
> >
> >
> >
> > _______________________________________________
> > pmg-devel mailing list
> > pmg-devel@lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
> >
> >
>

Re: [pmg-devel] [PATCH pmg-gui 2/3] fix #2437: proxy: Add 'TLS Inbound Policy' panel

[Stoiko Ivanov]

On Mon, 20 Mar 2023 09:36:36 +0100
Christoph Heiss <c.heiss@proxmox.com> wrote:

> On Thu, Mar 16, 2023 at 01:32:03PM +0100, Stoiko Ivanov wrote:
> > only a question/hint - since this is yet another list of domains (with w/o
> > added value) - any chance to merge this with MailProxyTLSDestinations?
> > (or DKIMDomains, or Domains)?
> >
> > OTOH the component is small enough and refactoring can very well happen at
> > another time as well) - so feel free to leave that out :)  
> I would do it as a follow-up patch(-series) to this, as to not block
> this series on that needlessly :^)
Definitely! - it's a separate step after all - if you like to look into it
later - it would be appreciated!

> 
> Didn't know there were more such instance other than
> MailProxyTLSDestinations, I'll have a look at it and unify them as best
> as possible.
did not look into it in detail, so am not sure if it's sensible/better to
unify them - if it makes things more complicated keeping it as is is also
an option.

> 
> >
> > On Thu,  9 Mar 2023 11:18:45 +0100
> > Christoph Heiss <c.heiss@proxmox.com> wrote:
> >  
> > > This panel can be used to configure sender domains for which TLS will be
> > > enforced my postfix. As this takes the usual transport domain format,
> > > either a FQDN or .FQDN (for matching subdomains) can be specified.
> > >
> > > Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> > > ---
> > >  js/MailProxyTLSInboundPolicy.js | 93 +++++++++++++++++++++++++++++++++
> > >  js/MailProxyTLSPanel.js         |  8 ++-
> > >  js/Makefile                     |  1 +
> > >  3 files changed, 101 insertions(+), 1 deletion(-)
> > >  create mode 100644 js/MailProxyTLSInboundPolicy.js
> > >
> > > diff --git a/js/MailProxyTLSInboundPolicy.js b/js/MailProxyTLSInboundPolicy.js
> > > new file mode 100644
> > > index 0000000..bc45527
> > > --- /dev/null
> > > +++ b/js/MailProxyTLSInboundPolicy.js
> > > @@ -0,0 +1,93 @@
> > > +Ext.define('pmg-tls-inbound-policy', {
> > > +    extend: 'Ext.data.Model',
> > > +    fields: ['domain'],
> > > +    idProperty: 'domain',
> > > +    proxy: {
> > > +	type: 'proxmox',
> > > +	url: '/api2/json/config/tlsinboundpolicy',
> > > +    },
> > > +    sorters: {
> > > +	property: 'domain',
> > > +	direction: 'ASC',
> > > +    },
> > > +});
> > > +
> > > +Ext.define('PMG.TLSInboundPolicyEdit', {
> > > +    extend: 'Proxmox.window.Edit',
> > > +    xtype: 'pmgTLSInboundPolicyEdit',
> > > +    onlineHelp: 'pmgconfig_mailproxy_tls',
> > > +
> > > +    subject: gettext('TLS Inbound Policy'),
> > > +    url: '/api2/extjs/config/tlsinboundpolicy',
> > > +    method: 'POST',
> > > +
> > > +    items: [
> > > +	{
> > > +	    xtype: 'proxmoxtextfield',
> > > +	    name: 'domain',
> > > +	    fieldLabel: gettext('Domain'),
> > > +	},
> > > +    ],
> > > +});
> > > +
> > > +Ext.define('PMG.MailProxyTLSInboundPolicy', {
> > > +    extend: 'Ext.grid.GridPanel',
> > > +    alias: ['widget.pmgMailProxyTLSInboundPolicy'],
> > > +
> > > +    viewConfig: {
> > > +	trackOver: false,
> > > +    },
> > > +
> > > +    columns: [
> > > +	{
> > > +	    header: gettext('Domain'),
> > > +	    flex: 1,
> > > +	    sortable: true,
> > > +	    dataIndex: 'domain',
> > > +	},
> > > +    ],
> > > +
> > > +    initComponent: function() {
> > > +	const me = this;
> > > +
> > > +	const rstore = Ext.create('Proxmox.data.UpdateStore', {
> > > +	    model: 'pmg-tls-inbound-policy',
> > > +	    storeid: 'pmg-mailproxy-tls-inbound-policy-store-' + ++Ext.idSeed,
> > > +	});
> > > +
> > > +	const store = Ext.create('Proxmox.data.DiffStore', { rstore: rstore });
> > > +	const reload = () => rstore.load();
> > > +	me.selModel = Ext.create('Ext.selection.RowModel', {});
> > > +	Proxmox.Utils.monStoreErrors(me, store, true);
> > > +
> > > +	Ext.apply(me, {
> > > +	    store,
> > > +	    tbar: [
> > > +		{
> > > +		    text: gettext('Create'),
> > > +		    handler: () => {
> > > +			Ext.createWidget('pmgTLSInboundPolicyEdit', {
> > > +			    autoShow: true,
> > > +			    listeners: {
> > > +				destroy: reload,
> > > +			    },
> > > +			});
> > > +		    },
> > > +		},
> > > +		{
> > > +		    xtype: 'proxmoxStdRemoveButton',
> > > +		    baseurl: '/config/tlsinboundpolicy',
> > > +		    callback: reload,
> > > +		    waitMsgTarget: me,
> > > +		},
> > > +	    ],
> > > +	    listeners: {
> > > +		activate: rstore.startUpdate,
> > > +		destroy: rstore.stopUpdate,
> > > +		deactivate: rstore.stopUpdate,
> > > +	    },
> > > +	});
> > > +
> > > +	me.callParent();
> > > +    },
> > > +});
> > > diff --git a/js/MailProxyTLSPanel.js b/js/MailProxyTLSPanel.js
> > > index 82dc3f8..5a5837c 100644
> > > --- a/js/MailProxyTLSPanel.js
> > > +++ b/js/MailProxyTLSPanel.js
> > > @@ -26,11 +26,17 @@ Ext.define('PMG.MailProxyTLSPanel', {
> > >  	    flex: 1,
> > >  	});
> > >
> > > -	me.items = [tlsSettings, tlsDestinations];
> > > +	const tlsInboundPolicy = Ext.create('PMG.MailProxyTLSInboundPolicy', {
> > > +	    title: gettext('TLS Inbound Policy'),
> > > +	    flex: 1,
> > > +	});
> > > +
> > > +	me.items = [tlsSettings, tlsDestinations, tlsInboundPolicy];
> > >
> > >  	me.callParent();
> > >
> > >  	tlsSettings.relayEvents(me, ['activate', 'deactivate', 'destroy']);
> > >  	tlsDestinations.relayEvents(me, ['activate', 'deactivate', 'destroy']);
> > > +	tlsInboundPolicy.relayEvents(me, ['activate', 'deactivate', 'destroy']);
> > >      },
> > >  });
> > > diff --git a/js/Makefile b/js/Makefile
> > > index 9a2bcf2..e3b9e78 100644
> > > --- a/js/Makefile
> > > +++ b/js/Makefile
> > > @@ -50,6 +50,7 @@ JSSRC=							\
> > >  	MailProxyTLS.js					\
> > >  	MailProxyTLSPanel.js				\
> > >  	MailProxyTLSDestinations.js			\
> > > +	MailProxyTLSInboundPolicy.js			\
> > >  	Transport.js					\
> > >  	MyNetworks.js					\
> > >  	RelayDomains.js					\
> > > --
> > > 2.39.2
> > >
> > >
> > >
> > > _______________________________________________
> > > pmg-devel mailing list
> > > pmg-devel@lists.proxmox.com
> > > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
> > >
> > >  
> >  

[pmg-devel] [PATCH pmg-docs 3/3] pmgconfig: Explain new TLS inbound policy configuration

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 pmgconfig.adoc | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/pmgconfig.adoc b/pmgconfig.adoc
index fea26db..22baef3 100644
--- a/pmgconfig.adoc
+++ b/pmgconfig.adoc
@@ -97,6 +97,10 @@ Stores your subscription key and status.

 TLS policy for outbound connections.

+`/etc/pmg/tls_inbound_domains`::
+
+TLS policy for inbound connections.
+
 `/etc/pmg/transports`::

 Message delivery transport setup.
@@ -495,6 +499,10 @@ This can be used if you need to prevent email delivery without
 encryption, or to work around a broken 'STARTTLS' ESMTP implementation. See
 {postfix_tls_readme} for details on the supported policies.

+Additionally, TLS can also be enforced on incoming connections for specific
+sender domains by creating a TLS inbound policy. Mails with matching domains
+must use encrypted SMTP session, otherwise they are rejected.
+
 Enable TLS logging::

 To get additional information about SMTP TLS activity, you can enable
--
2.39.2

Re: [pmg-devel] [PATCH pmg-{api, gui, docs} 0/3] fix #2437: Add TLS inbound policy for sender domains

[Stoiko Ivanov]

Thanks for tackling this!!

Works as advertised in general - so I think the approach is fine

One thing that seems odd to me (and sadly I don't have a good and short
answer) is that it's not a policy we're setting - it's a list of domains,
for which the singular policy (in this context) is that we accept mails
from them only via tls...

Inbound TLS Domains ? (at least mentions TLS, and explicitly mentioning
domains might be hint enough that you cannot enter an IP (or network)
there)
Reject Plaintext Domains? (probably only appeals to users who know
`postconf(5)` by heart)

some comments inline (some as reply to the individual patches):
On Thu,  9 Mar 2023 11:18:43 +0100
Christoph Heiss <c.heiss@proxmox.com> wrote:

> TL;DR: Implements the approach as laid out by Stoiko in the Bugzilla
> ticket [0].
> 
> A new API endpoint is added - /api2/json/config/tlsinboundpolicy. This
> is used to configure the newly introduced postfix map at
> /etc/pmg/tls_inbound_policy, specifying sender domains which get the
> `reject_plaintext_session` action [1] set, thus requiring TLS-encrypted
> sessions on inbound connections.
> 
> On the GUI side, a new panel is added in Configuration -> Mail Proxy ->
> TLS, where the domains can be specified for this new policy.
> 
> The documentation changes are quite terse, maybe I should expand a bit
> more on that topic? (Although the TLS destination policy is only lightly
> documented as well, as far as I could see.)
I personally am fine with terse documentation - however I always try to
refer to the authoritative source - in this case the relevant postfix
config parameter ([1]) - that way users who want to get more details
actually see what's going on under the hood).
You could rephrase the docs to mention that it sets
reject_plaintext_session for those domains during MAIL FROM)


> 
> Testing
> -------
> Tested this to the best of my knowledge, by adding some domains as TLS
> inbound policy and using `curl` to send some simple mails:
> 
>   echo '' | curl -skv smtp://<host> -T - \
>     --mail-from foo@localhost.localdomain \
>     --mail-rcpt bar@localhost.localdomain
> 
> .. where `localhost.localdomain` is on the new 'TLS Inboud Policy' list.
> This will now fail with:
I use swaks (apt installable) quite extensively for such things - short of
smtputf8 support it should cover most use-cases - but thanks for the tip
with curl being able to speak smtp as well :)

> 
>   450 4.7.1 Session encryption is required
> 
> When additionally adding the `--ssl-reqd` option to curl (instructing it
> to require a TLS-encrypted session), the above command will succeed.
> 
> (Also tested it with a domain not on the list, checking that no
> regressions are introduced.)
> 
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=2437
> [1] http://www.postfix.org/postconf.5.html#reject_plaintext_session
> 
> ---
> pmg-api:
> 
> Christoph Heiss (1):
>       fix #2437: config: Add inbound TLS policy option
> 
>  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(+)
> 
> pmg-gui:
> 
> Christoph Heiss (1):
>       fix #2437: proxy: Add 'TLS Inbound Policy' panel
> 
>  js/MailProxyTLSInboundPolicy.js | 93 +++++++++++++++++++++++++++++++++++++++++
>  js/MailProxyTLSPanel.js         |  8 +++-
>  js/Makefile                     |  1 +
>  3 files changed, 101 insertions(+), 1 deletion(-)
> 
> pmg-docs:
> 
> Christoph Heiss (1):
>       pmgconfig: Explain new TLS inbound policy configuration
> 
>  pmgconfig.adoc | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> --
> 2.39.2
> 
> 
> 
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
> 
> 

Re: [pmg-devel] [PATCH pmg-{api, gui, docs} 0/3] fix #2437: Add TLS inbound policy for sender domains

[Christoph Heiss]

Thanks for the review on the whole series!

On Thu, Mar 16, 2023 at 01:28:42PM +0100, Stoiko Ivanov wrote:
> Thanks for tackling this!!
>
> Works as advertised in general - so I think the approach is fine
>
> One thing that seems odd to me (and sadly I don't have a good and short
> answer) is that it's not a policy we're setting - it's a list of domains,
> for which the singular policy (in this context) is that we accept mails
> from them only via tls...
I'm bad at naming as per usual :^) I agree that 'policy' might be the
wrong word for that ..

>
> Inbound TLS Domains ? (at least mentions TLS, and explicitly mentioning
> domains might be hint enough that you cannot enter an IP (or network)
> there)
> Reject Plaintext Domains? (probably only appeals to users who know
> `postconf(5)` by heart)
From the two suggestions above I'd IMHO go with 'Inbound TLS Domains' -
it mostly says what it does on the tin and is probably one of the better
options in general.
The latter - as you say - leans very much on postfix nomenclature and
might be unclear to (some) users.

If that's okay with you I'll rename it for v2 and than see how it looks.

>
> some comments inline (some as reply to the individual patches):
> On Thu,  9 Mar 2023 11:18:43 +0100
> Christoph Heiss <c.heiss@proxmox.com> wrote:
>
> > [..]
> > The documentation changes are quite terse, maybe I should expand a bit
> > more on that topic? (Although the TLS destination policy is only lightly
> > documented as well, as far as I could see.)
> I personally am fine with terse documentation - however I always try to
> refer to the authoritative source - in this case the relevant postfix
> config parameter ([1]) - that way users who want to get more details
> actually see what's going on under the hood).
> You could rephrase the docs to mention that it sets
> reject_plaintext_session for those domains during MAIL FROM)
Ack, I'll add a link to the postfix config parameter and mention that it
sets `reject_plaintext_session`.

>
>
> >
> > Testing
> > -------
> > [..]
> I use swaks (apt installable) quite extensively for such things - short of
> smtputf8 support it should cover most use-cases - but thanks for the tip
> with curl being able to speak smtp as well :)
Haven't heard of or used swaks before, looks very useful - thanks for the
tip as well!

curl speaks ~every protocol that exists, so it was simply the first tool
that came to my mind :)

>
> >
> >   450 4.7.1 Session encryption is required
> >
> > When additionally adding the `--ssl-reqd` option to curl (instructing it
> > to require a TLS-encrypted session), the above command will succeed.
> >
> > (Also tested it with a domain not on the list, checking that no
> > regressions are introduced.)
> >
> > [0] https://bugzilla.proxmox.com/show_bug.cgi?id=2437
> > [1] http://www.postfix.org/postconf.5.html#reject_plaintext_session
> >
> > ---
> > pmg-api:
> >
> > Christoph Heiss (1):
> >       fix #2437: config: Add inbound TLS policy option
> >
> >  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(+)
> >
> > pmg-gui:
> >
> > Christoph Heiss (1):
> >       fix #2437: proxy: Add 'TLS Inbound Policy' panel
> >
> >  js/MailProxyTLSInboundPolicy.js | 93 +++++++++++++++++++++++++++++++++++++++++
> >  js/MailProxyTLSPanel.js         |  8 +++-
> >  js/Makefile                     |  1 +
> >  3 files changed, 101 insertions(+), 1 deletion(-)
> >
> > pmg-docs:
> >
> > Christoph Heiss (1):
> >       pmgconfig: Explain new TLS inbound policy configuration
> >
> >  pmgconfig.adoc | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > --
> > 2.39.2
> >
> >
> >
> > _______________________________________________
> > pmg-devel mailing list
> > pmg-devel@lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
> >
> >
>

Re: [pmg-devel] [PATCH pmg-{api, gui, docs} 0/3] fix #2437: Add TLS inbound policy for sender domains

[Stoiko Ivanov]

On Mon, 20 Mar 2023 09:14:06 +0100
Christoph Heiss <c.heiss@proxmox.com> wrote:

> Thanks for the review on the whole series!
> 
> On Thu, Mar 16, 2023 at 01:28:42PM +0100, Stoiko Ivanov wrote:
> > Thanks for tackling this!!
> >
> > Works as advertised in general - so I think the approach is fine
> >
> > One thing that seems odd to me (and sadly I don't have a good and short
> > answer) is that it's not a policy we're setting - it's a list of domains,
> > for which the singular policy (in this context) is that we accept mails
> > from them only via tls...  
> I'm bad at naming as per usual :^) I agree that 'policy' might be the
> wrong word for that ..
> 
> >
> > Inbound TLS Domains ? (at least mentions TLS, and explicitly mentioning
> > domains might be hint enough that you cannot enter an IP (or network)
> > there)
> > Reject Plaintext Domains? (probably only appeals to users who know
> > `postconf(5)` by heart)  
> From the two suggestions above I'd IMHO go with 'Inbound TLS Domains' -
> it mostly says what it does on the tin and is probably one of the better
> options in general.
> The latter - as you say - leans very much on postfix nomenclature and
> might be unclear to (some) users.
> 
> If that's okay with you I'll rename it for v2 and than see how it looks.
Sounds good! - Thanks for reworking this!


> 
> >
> > some comments inline (some as reply to the individual patches):
> > On Thu,  9 Mar 2023 11:18:43 +0100
> > Christoph Heiss <c.heiss@proxmox.com> wrote:
> >  
> > > [..]
> > > The documentation changes are quite terse, maybe I should expand a bit
> > > more on that topic? (Although the TLS destination policy is only lightly
> > > documented as well, as far as I could see.)  
> > I personally am fine with terse documentation - however I always try to
> > refer to the authoritative source - in this case the relevant postfix
> > config parameter ([1]) - that way users who want to get more details
> > actually see what's going on under the hood).
> > You could rephrase the docs to mention that it sets
> > reject_plaintext_session for those domains during MAIL FROM)  
> Ack, I'll add a link to the postfix config parameter and mention that it
> sets `reject_plaintext_session`.
> 
> >
> >  
> > >
> > > Testing
> > > -------
> > > [..]  
> > I use swaks (apt installable) quite extensively for such things - short of
> > smtputf8 support it should cover most use-cases - but thanks for the tip
> > with curl being able to speak smtp as well :)  
> Haven't heard of or used swaks before, looks very useful - thanks for the
> tip as well!
> 
> curl speaks ~every protocol that exists, so it was simply the first tool
> that came to my mind :)
> 
> >  
> > >
> > >   450 4.7.1 Session encryption is required
> > >
> > > When additionally adding the `--ssl-reqd` option to curl (instructing it
> > > to require a TLS-encrypted session), the above command will succeed.
> > >
> > > (Also tested it with a domain not on the list, checking that no
> > > regressions are introduced.)
> > >
> > > [0] https://bugzilla.proxmox.com/show_bug.cgi?id=2437
> > > [1] http://www.postfix.org/postconf.5.html#reject_plaintext_session
> > >
> > > ---
> > > pmg-api:
> > >
> > > Christoph Heiss (1):
> > >       fix #2437: config: Add inbound TLS policy option
> > >
> > >  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(+)
> > >
> > > pmg-gui:
> > >
> > > Christoph Heiss (1):
> > >       fix #2437: proxy: Add 'TLS Inbound Policy' panel
> > >
> > >  js/MailProxyTLSInboundPolicy.js | 93 +++++++++++++++++++++++++++++++++++++++++
> > >  js/MailProxyTLSPanel.js         |  8 +++-
> > >  js/Makefile                     |  1 +
> > >  3 files changed, 101 insertions(+), 1 deletion(-)
> > >
> > > pmg-docs:
> > >
> > > Christoph Heiss (1):
> > >       pmgconfig: Explain new TLS inbound policy configuration
> > >
> > >  pmgconfig.adoc | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > --
> > > 2.39.2
> > >
> > >
> > >
> > > _______________________________________________
> > > pmg-devel mailing list
> > > pmg-devel@lists.proxmox.com
> > > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
> > >
> > >  
> >