[pmg-devel] [PATCH v2 pmg-{api, gui, docs} 0/4] fix #2437: Add TLS enforcment option for inbound domains

[pmg-devel] [PATCH v2 pmg-{api, gui, docs} 0/4] fix #2437: Add TLS enforcment option for inbound 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/tlsinbounddomains. This
is used to configure the newly introduced postfix map at
/etc/pmg/tls_inbound_domains, 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 for which this should be enforced can be specified.

Testing
-------
Tested this to the best of my knowledge, by adding some domains using
the UI 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 Inbound Domains' 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

v1: https://lists.proxmox.com/pipermail/pmg-devel/2023-March/002296.html

---
pmg-api:

Christoph Heiss (2):
      fix #2437: config: Add new tls_inbound_domains postfix map
      fix #2437: api: Add endpoint for managing tls_inbound_domains entries

 src/Makefile                      |   1 +
 src/PMG/API2/Config.pm            |   7 +++
 src/PMG/API2/InboundTLSDomains.pm | 127 ++++++++++++++++++++++++++++++++++++++
 src/PMG/Cluster.pm                |   1 +
 src/PMG/Config.pm                 |  56 +++++++++++++++++
 src/templates/main.cf.in          |   1 +
 6 files changed, 193 insertions(+)

pmg-gui:

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

 js/MailProxyTLSInboundDomains.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 domains configuration

 pmgconfig.adoc | 11 +++++++++++
 1 file changed, 11 insertions(+)
--
2.39.2

[pmg-devel] [PATCH v2 pmg-api 1/4] fix #2437: config: Add new tls_inbound_domains postfix map

[Christoph Heiss]

Add a new configuration file /etc/pmg/tls_inbound_domains, which is a
postfix map containing all domains having `reject_plaintext_session`
action set. This is the only allowed action value and enforced while
parsing.

This map is then used for `smtpd_sender_restriction` in the main.cf
template.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
 * Rename `tls_inbound_policy` to `tls_inbound_domains`
 * Move API endpoint implementation to separate patch
 * Add `tls_inbound_domains` to cluster sync

 src/PMG/Cluster.pm       |  1 +
 src/PMG/Config.pm        | 56 ++++++++++++++++++++++++++++++++++++++++
 src/templates/main.cf.in |  1 +
 3 files changed, 58 insertions(+)

diff --git a/src/PMG/Cluster.pm b/src/PMG/Cluster.pm
index 31384b2..7622a88 100644
--- a/src/PMG/Cluster.pm
+++ b/src/PMG/Cluster.pm
@@ -464,6 +464,7 @@ sub sync_config_from_master {
 	'mynetworks',
 	'transport',
 	'tls_policy',
+	'tls_inbound_domains',
 	'fetchmailrc',
 	];

diff --git a/src/PMG/Config.pm b/src/PMG/Config.pm
index 699a622..08ba1f5 100755
--- a/src/PMG/Config.pm
+++ b/src/PMG/Config.pm
@@ -1160,6 +1160,61 @@ sub postmap_tls_policy {
     PMG::Utils::run_postmap($tls_policy_map_filename);
 }

+sub read_tls_inbound_domains {
+    my ($filename, $fh) = @_;
+
+    return {} if !defined($fh);
+
+    my $domains = {};
+
+    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+) reject_plaintext_session$/) {
+	    my $domain = $1;
+
+	    eval { pmg_verify_transport_domain($domain) };
+	    if (my $err = $@) {
+		$parse_error->($err);
+		next;
+	    }
+
+	    $domains->{$domain} = 1;
+	} else {
+	    $parse_error->('wrong format');
+	}
+    }
+
+    return $domains;
+}
+
+sub write_tls_inbound_domains {
+    my ($filename, $fh, $domains) = @_;
+
+    return if !$domains;
+
+    foreach my $domain (sort keys %$domains) {
+	PVE::Tools::safe_print($filename, $fh, "$domain reject_plaintext_session\n");
+    }
+}
+
+my $tls_inbound_domains_map_filename = "/etc/pmg/tls_inbound_domains";
+PVE::INotify::register_file('tls_inbound_domains', $tls_inbound_domains_map_filename,
+			    \&read_tls_inbound_domains,
+			    \&write_tls_inbound_domains,
+			    undef, always_call_parser => 1);
+
+sub postmap_tls_inbound_domains {
+    PMG::Utils::run_postmap($tls_inbound_domains_map_filename);
+}
+
 my $transport_map_filename = "/etc/pmg/transport";

 sub postmap_pmg_transport {
@@ -1696,6 +1751,7 @@ sub rewrite_config_postfix {
     postmap_pmg_domains();
     postmap_pmg_transport();
     postmap_tls_policy();
+    postmap_tls_inbound_domains();

     rewrite_postfix_whitelist($rulecache) if $rulecache;

diff --git a/src/templates/main.cf.in b/src/templates/main.cf.in
index 190c913..1f4fa91 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_domains
         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] [PATCH v2 pmg-api 2/4] fix #2437: api: Add endpoint for managing tls_inbound_domains entries

[Christoph Heiss]

Add a new API endpoint `/config/tlsinbounddomains` for managing entries
of the `tls_inbound_domains` postfix map. Modelled after the
`DestinationTLSPolicy` implementation.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
 * New patch; split out from patch #1

 src/Makefile                      |   1 +
 src/PMG/API2/Config.pm            |   7 ++
 src/PMG/API2/InboundTLSDomains.pm | 127 ++++++++++++++++++++++++++++++
 3 files changed, 135 insertions(+)
 create mode 100644 src/PMG/API2/InboundTLSDomains.pm

diff --git a/src/Makefile b/src/Makefile
index 0b424e9..32eac57 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -132,6 +132,7 @@ LIBSOURCES =				\
 	PMG/API2/DKIMSignDomains.pm	\
 	PMG/API2/DKIMSign.pm		\
 	PMG/API2/Fetchmail.pm		\
+	PMG/API2/InboundTLSDomains.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..c71432a 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::InboundTLSDomains;
 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::InboundTLSDomains",
+    path => 'tlsinbounddomains',
+});
+
 __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 => 'tlsinbounddomains' };
 	push @$res, { section => 'transport' };
 	push @$res, { section => 'users' };
 	push @$res, { section => 'whitelist' };
diff --git a/src/PMG/API2/InboundTLSDomains.pm b/src/PMG/API2/InboundTLSDomains.pm
new file mode 100644
index 0000000..38bebca
--- /dev/null
+++ b/src/PMG/API2/InboundTLSDomains.pm
@@ -0,0 +1,127 @@
+package PMG::API2::InboundTLSDomains;
+
+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_domains 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 $domains = PVE::INotify::read_file('tls_inbound_domains');
+
+	foreach my $domain (sort keys %$domains) {
+	    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_domains 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 $domains = PVE::INotify::read_file('tls_inbound_domains');
+	    raise_param_exc({ domain => "InboundTLSDomains entry for '$domain' already exists" })
+		if $domains->{$domain};
+
+	    $domains->{$domain} = 1;
+
+	    PVE::INotify::write_file('tls_inbound_domains', $domains);
+	    PMG::Config::postmap_tls_inbound_domains();
+	};
+
+	PMG::Config::lock_config($code, 'adding tls_inbound_domains entry failed');
+
+	return undef;
+    }});
+
+__PACKAGE__->register_method ({
+    name => 'delete',
+    path => '{domain}',
+    method => 'DELETE',
+    description => 'Delete a tls_inbound_domains 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_domains',
+	    },
+	}
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+	my $domain = $param->{domain};
+
+	my $code = sub {
+	    my $domains = PVE::INotify::read_file('tls_inbound_domains');
+
+	    raise_param_exc({ domain => "tls_inbound_domains entry for '$domain' does not exist" })
+		if !$domains->{$domain};
+
+	    delete $domains->{$domain};
+
+	    PVE::INotify::write_file('tls_inbound_domains', $domains);
+	    PMG::Config::postmap_tls_inbound_domains();
+	};
+
+	PMG::Config::lock_config($code, 'deleting tls_inbound_domains entry failed');
+
+	return undef;
+    }});
+
+1;
--
2.39.2

[pmg-devel] [PATCH v2 pmg-gui 3/4] fix #2437: proxy: Add 'TLS Inbound Domains' 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>
---
Changes v1 -> v2:
 * Rename to 'TLS Inbound Domains' from 'TLS Inbound Policy'
 * Change to renamed API endpoint (`tlsinboundpolicy` -> `tlsinbounddomains`)

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

diff --git a/js/MailProxyTLSInboundDomains.js b/js/MailProxyTLSInboundDomains.js
new file mode 100644
index 0000000..27f8fcd
--- /dev/null
+++ b/js/MailProxyTLSInboundDomains.js
@@ -0,0 +1,93 @@
+Ext.define('pmg-tls-inbound-domains', {
+    extend: 'Ext.data.Model',
+    fields: ['domain'],
+    idProperty: 'domain',
+    proxy: {
+	type: 'proxmox',
+	url: '/api2/json/config/tlsinbounddomains',
+    },
+    sorters: {
+	property: 'domain',
+	direction: 'ASC',
+    },
+});
+
+Ext.define('PMG.TLSInboundDomainsEdit', {
+    extend: 'Proxmox.window.Edit',
+    xtype: 'pmgTLSInboundDomainsEdit',
+    onlineHelp: 'pmgconfig_mailproxy_tls',
+
+    subject: gettext('TLS Inbound domains'),
+    url: '/api2/extjs/config/tlsinbounddomains',
+    method: 'POST',
+
+    items: [
+	{
+	    xtype: 'proxmoxtextfield',
+	    name: 'domain',
+	    fieldLabel: gettext('Domain'),
+	},
+    ],
+});
+
+Ext.define('PMG.MailProxyTLSInboundDomains', {
+    extend: 'Ext.grid.GridPanel',
+    alias: ['widget.pmgMailProxyTLSInboundDomains'],
+
+    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-domains',
+	    storeid: 'pmg-mailproxy-tls-inbound-domains-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('pmgTLSInboundDomainsEdit', {
+			    autoShow: true,
+			    listeners: {
+				destroy: reload,
+			    },
+			});
+		    },
+		},
+		{
+		    xtype: 'proxmoxStdRemoveButton',
+		    baseurl: '/config/tlsinbounddomains',
+		    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..96b24de 100644
--- a/js/MailProxyTLSPanel.js
+++ b/js/MailProxyTLSPanel.js
@@ -26,11 +26,17 @@ Ext.define('PMG.MailProxyTLSPanel', {
 	    flex: 1,
 	});

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

 	me.callParent();

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

[pmg-devel] [PATCH v2 pmg-docs 4/4] pmgconfig: Explain new TLS inbound domains configuration

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
 * Rename 'TLS inbound policy' to 'TLS inbound domains'
 * Add link to postconf(5) section for `reject_plaintext_session`

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

diff --git a/pmgconfig.adoc b/pmgconfig.adoc
index fea26db..9a57d06 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`::
+
+Sender domains for which TLS is enforced on inbound connections.
+
 `/etc/pmg/transports`::

 Message delivery transport setup.
@@ -495,6 +499,13 @@ 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 domains entry. Mails with matching
+domains must use a encrypted SMTP session, otherwise they are rejected. All
+domains on this list have the
+https://www.postfix.org/postconf.5.html#reject_plaintext_session[`reject_plaintext_session`]
+postfix parameter set.
+
 Enable TLS logging::

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

[pmg-devel] applied-series: [PATCH v2 pmg-{api, gui, docs} 0/4] fix #2437: Add TLS enforcment option for inbound domains

[Stoiko Ivanov]

Thanks for the rework! 
tested a bit more and applied it with a small rephrase on the docs-commit
and a follow-up for (hopefully) better error-handling on wrong entries in
the API for tls_inbound_domains and tls_policy.



On Mon, 20 Mar 2023 11:35:44 +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/tlsinbounddomains. This
> is used to configure the newly introduced postfix map at
> /etc/pmg/tls_inbound_domains, 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 for which this should be enforced can be specified.
> 
> Testing
> -------
> Tested this to the best of my knowledge, by adding some domains using
> the UI 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 Inbound Domains' 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
> 
> v1: https://lists.proxmox.com/pipermail/pmg-devel/2023-March/002296.html
> 
> ---
> pmg-api:
> 
> Christoph Heiss (2):
>       fix #2437: config: Add new tls_inbound_domains postfix map
>       fix #2437: api: Add endpoint for managing tls_inbound_domains entries
> 
>  src/Makefile                      |   1 +
>  src/PMG/API2/Config.pm            |   7 +++
>  src/PMG/API2/InboundTLSDomains.pm | 127 ++++++++++++++++++++++++++++++++++++++
>  src/PMG/Cluster.pm                |   1 +
>  src/PMG/Config.pm                 |  56 +++++++++++++++++
>  src/templates/main.cf.in          |   1 +
>  6 files changed, 193 insertions(+)
> 
> pmg-gui:
> 
> Christoph Heiss (1):
>       fix #2437: proxy: Add 'TLS Inbound Domains' panel
> 
>  js/MailProxyTLSInboundDomains.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 domains configuration
> 
>  pmgconfig.adoc | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> --
> 2.39.2
> 
> 
> 
> _______________________________________________
> pmg-devel mailing list
> pmg-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
> 
>