OpenARC icon indicating copy to clipboard operation
OpenARC copied to clipboard

[patch] Fix a bug preventing load of InternalHosts network/host list

Open oleg-nauman opened this issue 6 years ago • 4 comments

There is inverted logic in arcf_config_setlib that prevents load of InternalHosts network/host list. Below is the patch that fixes wrong behavior.

*** openarc.c.orig 2018-07-23 14:23:32.541068573 +0000 --- openarc.c 2018-07-23 14:24:21.097334563 +0000


*** 1575,1593 **** str = NULL; if (data != NULL) (void) config_get(data, "InternalHosts", &str, sizeof str); if (str != NULL) { int status; char *dberr = NULL;

            status = arcf_list_load(&conf->conf_internal, str, &dberr);

! if (status != 0) { snprintf(err, errlen, "%s: arcf_list_load(): %s", str, dberr); return -1; } } else if (!testmode) { _Bool status; --- 1575,1594 ---- str = NULL; if (data != NULL) (void) config_get(data, "InternalHosts", &str, sizeof str); if (str != NULL) { int status; char *dberr = NULL;

            status = arcf_list_load(&conf->conf_internal, str, &dberr);

! /* if (status != 0) */ ! if (!status) { snprintf(err, errlen, "%s: arcf_list_load(): %s", str, dberr); return -1; } } else if (!testmode) { _Bool status;

oleg-nauman avatar Jul 23 '18 14:07 oleg-nauman

I do not believe that this is the correct patch. It seems that the openarc.conf refers to a "dataset" type akin to opendkim.conf, but the implementation always treats the argument for PeerList and InternalHosts as a file. openarc.conf.sample confirms this.

Why OpenARC treats fields differently from OpenDKIM is beyond me.

jeredfloyd avatar Jul 26 '18 21:07 jeredfloyd

@jeredfloyd it's probably a mistake, that's why ;-)

It should be handled the same.

@oleg-nauman if you want to submit a PR that fixes this appropriately, that would be greatly appreciated.

AntiFreeze avatar Aug 15 '18 17:08 AntiFreeze

OpenARC does not (yet) support the suite of database systems that OpenDKIM does. It's entirely possible we don't need all of them, so I'd rather not create a dependency on multiple external libraries if we can avoid it.

For the moment we only accept plain text files, and that works as far as I can tell.

The return from arcf_load_list() is a Boolean, so the current test looks right to me.

mskucherawy avatar Sep 17 '18 21:09 mskucherawy

Also, "if (!status)" is the current check and has been since 1ef92e6797535ea76efc10fcd561f4b83d489f4d (a year ago). Is this being built against master or develop?

mskucherawy avatar Sep 17 '18 21:09 mskucherawy