Yahoo Groups archive

Milter-greylist

Index last updated: 2026-04-28 23:32 UTC

Message

Config/reconfig behavior

2006-01-08 by Ranko Zivojnovic

Hi,

I believe that current reconfiguration method has flaws and can even
cause the loss of collected triplets. Here's why/how:

conf_load() resets the configuration to program defaults + command line
switches ("defconf") before it proceeds to opening of the configuration
file and parsing.
Those defaults may not be what my system is meant to run with. In the
multithreaded environment such as here, no protection is currently
offered against for example running expiration procedure on pending and
autowhitelisted entries between the time the config was reset to the
defaults and loading of the new configuration. Access lists are the only
ones that are protected currently (well - sort of, read on).

Things can get even worse in case the config file has a syntax error in
it. The configuration file will be only partially reloaded and the
program could continue running without the rest of the configuration
that follows the line that aborted the parsing procedure. You may even
end up without any access list defined since ALL of the access lists are
cleared just before the parsing commences - or your aw/pending timers
can stay reset to "defconf" and entries will get lost during the queue
expiration procedures (given that you want to keep them for longer than
program defaults).

Further, all of the acl_add_*() functions called from the config_parse()
exit() upon failure. Maybe acceptable for the program startup, but I
believe - bad for the automated reconfiguration.

Next (minor), conf_line variable does not get reset to 1 upon reload of
the file - causing parsing error messages to be produced with the wrong
line numbers - this needs fixing too.

So now, in order to solve these issues the following is what I believe
it should be done:

      * All of the acl_add_*() functions should be changed to return
        succes/failure rather than exit() and use the result to accept
        or reject the line in config_parse()
      * acl_head should be part of the "struct conf" structure
      * a pointer to the running configuration should be used rather
        than statically allocated structure
      * config_parse should work on a "new" config structure which at
        the end of the parsing can be accepted or rejected. If accepted,
        the running configuration pointer should be "safely" swapped via
        write lock and the old configuration and access lists should be
        released.
      * acl_register_entry_{first,last} should be working against
        acl_head in the "new" structure.
      * acl_clear() should receive as the parameter the structure to
        work on.
      * The same logic applied to acl_* functions should be applied to
        related peer_* functions.
      * config_parse should go through the whole config file and report
        all the errors found and not just the first one
      * A macro called something like "CONF_GET" should be defined and
        used to safely retrieve the value from the running
        configuration, by read locking it.

Since the amount of changes needed are quite large and intrusive, I was
hoping to get some possible suggestions here before I go ahead and try
to do it.

Best regards,

Ranko

Attachments

Move to quarantaine

This moves the raw source file on disk only. The archive index is not changed automatically, so you still need to run a manual refresh afterward.