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

[Christoph Heiss <c.heiss@proxmox.com>]

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.

>
> [..]
>