public inbox for pmg-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Stoiko Ivanov <s.ivanov@proxmox.com>
To: Christoph Heiss <c.heiss@proxmox.com>
Cc: pmg-devel@lists.proxmox.com
Subject: Re: [pmg-devel] [PATCH pmg-api 1/3] fix #2437: config: Add inbound TLS policy option
Date: Thu, 16 Mar 2023 13:50:41 +0100	[thread overview]
Message-ID: <20230316135041.4de4316e@rosa.proxmox.com> (raw)
In-Reply-To: <20230309101846.192177-2-c.heiss@proxmox.com>

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
> 
> 





  reply	other threads:[~2023-03-16 12:50 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-09 10:18 [pmg-devel] [PATCH pmg-{api, gui, docs} 0/3] fix #2437: Add TLS inbound policy for sender domains Christoph Heiss
2023-03-09 10:18 ` [pmg-devel] [PATCH pmg-api 1/3] fix #2437: config: Add inbound TLS policy option Christoph Heiss
2023-03-16 12:50   ` Stoiko Ivanov [this message]
2023-03-20  8:21     ` Christoph Heiss
2023-03-09 10:18 ` [pmg-devel] [PATCH pmg-gui 2/3] fix #2437: proxy: Add 'TLS Inbound Policy' panel Christoph Heiss
2023-03-16 12:32   ` Stoiko Ivanov
2023-03-20  8:36     ` Christoph Heiss
2023-03-20  8:42       ` Stoiko Ivanov
2023-03-09 10:18 ` [pmg-devel] [PATCH pmg-docs 3/3] pmgconfig: Explain new TLS inbound policy configuration Christoph Heiss
2023-03-16 12:28 ` [pmg-devel] [PATCH pmg-{api, gui, docs} 0/3] fix #2437: Add TLS inbound policy for sender domains Stoiko Ivanov
2023-03-20  8:14   ` Christoph Heiss
2023-03-20  8:36     ` Stoiko Ivanov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230316135041.4de4316e@rosa.proxmox.com \
    --to=s.ivanov@proxmox.com \
    --cc=c.heiss@proxmox.com \
    --cc=pmg-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal