On Fri, Dec 01, 2023 at 08:30:47PM +0100, Omar Polo wrote:
> [moving to tech@, +CC Gilles and Eric]
> 
> On 2023/11/13 14:04:37 -0300, Crystal Kolipe <kolip...@exoticsilicon.com> 
> wrote:
> > The smtpd.conf manual says:
> 
> table(5) actually ;-)

Indeed :-).

> > >     When using a `file' table, a list will be written with each value on a
> > >     line by itself.  Comments can be put anywhere in the file using a hash
> > >     mark (`#'), and extend to the end of the current line.
> > 
> > Unfortunately this is not true :-).
> > 
> > Worse still, it causes silent configuration breakage in some cases, for
> > example consider if you have a list of IPs that you want to block stored in 
> > a
> > table:
> > 
> > $ cat /etc/mail/blocked_ips
> > 10.0.0.1
> > 10.0.0.50
> > 10.1.2.3 # Sends loads of rubbish since last month
> > 10.4.5.6
> > 
> > This would be loaded in smtpd.conf via something like:
> > 
> > table ip_reject_list file:/etc/mail/blocked_ips
> > 
> > No error would be reported, and smtpd would start just fine.
> > 
> > But the IP address with the comment would _not_ be included in the list!
> 
> My guess is that the intention was to allow only comments at the start
> of the line, and the text in table(5) was just a side-effect of
> copy-pasting.
> 
> In fact, I don't think it should support comments "anywhere in the
> file", as they can have unwanted side effects.  With your diff, what
> would happen if I have a '#' character in one of my passwords?

True, that would only work with escaping or quoting.

> Instead of adding means of escaping (and escaping the escaping), that
> would equally breaking existing confuration, I think we should just
> 'fix' the manpage.

Sure, that's fine by me too.

The key point here was that the existing situation caused _silent_ breakage
- in the example I gave the line with the commented IP address was just parsed
as an invalid IP address and dropped, so unless the user actually looked at the
contents of the loaded table afterwards they would never see it.

> While here, I noticed that the parser accepts the magic "@list" comment
> to force the file to be considered a list table instead of a mapping.
> It was introduced by eric@ in rev. 1.17 back in 2017 but never
> documented.  Should we document it too?

Seems like a good idea.

Reply via email to