Re: New report function for libopensmtpd

2022-10-19 Thread Martin Kjær Jørgensen


I'm not sure how you like patches so here's a crude git diff patch of my
first attempt :-)

diff --git a/Symbols.list b/Symbols.list
index a73487d..360ae2a 100644
--- a/Symbols.list
+++ b/Symbols.list
@@ -14,6 +14,7 @@ osmtpd_register_filter_noop
 osmtpd_register_filter_help
 osmtpd_register_filter_wiz
 osmtpd_register_filter_commit
+osmtpd_register_report_auth
 osmtpd_register_report_connect
 osmtpd_register_report_disconnect
 osmtpd_register_report_identify
diff --git a/opensmtpd.c b/opensmtpd.c
index 71b0691..7953973 100644
--- a/opensmtpd.c
+++ b/opensmtpd.c
@@ -72,6 +72,8 @@ static void osmtpd_connect(struct osmtpd_callback *, struct 
osmtpd_ctx *,
 char *, char *);
 static void osmtpd_identify(struct osmtpd_callback *, struct osmtpd_ctx *,
 char *, char *);
+static void osmtpd_link_auth(struct osmtpd_callback *, struct osmtpd_ctx *,
+char *, char *);
 static void osmtpd_link_connect(struct osmtpd_callback *, struct osmtpd_ctx *,
 char *, char *);
 static void osmtpd_link_disconnect(struct osmtpd_callback *,
@@ -241,6 +243,15 @@ static struct osmtpd_callback osmtpd_callbacks[] = {
0,
0
},
+   {
+   OSMTPD_TYPE_REPORT,
+   OSMTPD_PHASE_LINK_AUTH,
+   1,
+   osmtpd_link_auth,
+   NULL,
+   0,
+   0
+   },
{
OSMTPD_TYPE_REPORT,
OSMTPD_PHASE_LINK_CONNECT,
@@ -829,6 +840,16 @@ osmtpd_register_report_timeout(int incoming, void 
(*cb)(struct osmtpd_ctx *))
incoming, 0, NULL);
 }

+void
+osmtpd_register_report_auth(int incoming, void (*cb)(struct osmtpd_ctx *,
+const char * username, enum osmtpd_auth_result))
+{
+   osmtpd_register(OSMTPD_TYPE_REPORT, OSMTPD_PHASE_LINK_AUTH,
+   incoming, 0, (void *)cb);
+   osmtpd_register(OSMTPD_TYPE_REPORT, OSMTPD_PHASE_LINK_DISCONNECT,
+   incoming, 0, NULL);
+}
+
 void
 osmtpd_local_session(void *(*oncreate)(struct osmtpd_ctx *),
 void (*ondelete)(struct osmtpd_ctx *, void *))
@@ -1267,6 +1288,34 @@ osmtpd_identify(struct osmtpd_callback *cb, struct 
osmtpd_ctx *ctx,
f(ctx, identity);
 }

+static void
+osmtpd_link_auth(struct osmtpd_callback *cb, struct osmtpd_ctx *ctx,
+char *params, char *linedup)
+{
+   enum osmtpd_auth_result auth_res;
+   char * username, *end;
+   void (*f)(struct osmtpd_ctx *, const char *, enum osmtpd_auth_result);
+
+   if ((end = strchr(params, '|')) == NULL)
+   osmtpd_errx(1, "Invalid line received: missing username: %s",
+   linedup);
+   end++[0] = '\0';
+   username = params;
+   params = end;
+   if (strcmp(params, "pass") == 0)
+   auth_res = OSMTPD_AUTH_PASS;
+   else if (strcmp(params, "fail") == 0)
+   auth_res = OSMTPD_AUTH_FAIL;
+   else if (strcmp(params, "error") == 0)
+   auth_res = OSMTPD_AUTH_ERROR;
+   else
+   osmtpd_errx(1, "Invalid line received: invalid result: %s",
+   linedup);
+
+   if ((f = cb->cb) != NULL)
+   f(ctx, username, auth_res);
+}
+
 static void
 osmtpd_link_connect(struct osmtpd_callback *cb, struct osmtpd_ctx *ctx,
 char *params, char *linedup)
@@ -1870,6 +1919,8 @@ osmtpd_strtophase(const char *phase, const char *linedup)
return OSMTPD_PHASE_WIZ;
if (strcmp(phase, "commit") == 0)
return OSMTPD_PHASE_COMMIT;
+   if (strcmp(phase, "link-auth") == 0)
+   return OSMTPD_PHASE_LINK_AUTH;
if (strcmp(phase, "link-connect") == 0)
return OSMTPD_PHASE_LINK_CONNECT;
if (strcmp(phase, "link-disconnect") == 0)
@@ -1951,6 +2002,8 @@ osmtpd_phasetostr(enum osmtpd_phase phase)
return "wiz";
case OSMTPD_PHASE_COMMIT:
return "commit";
+   case OSMTPD_PHASE_LINK_AUTH:
+   return "link-auth";
case OSMTPD_PHASE_LINK_CONNECT:
return "link-connect";
case OSMTPD_PHASE_LINK_DISCONNECT:
diff --git a/opensmtpd.h b/opensmtpd.h
index eef3237..d59d220 100644
--- a/opensmtpd.h
+++ b/opensmtpd.h
@@ -13,6 +13,7 @@
  * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
+#include 
 #include 

 #ifndef __dead
@@ -49,6 +50,7 @@ enum osmtpd_phase {
OSMTPD_PHASE_HELP,
OSMTPD_PHASE_WIZ,
OSMTPD_PHASE_COMMIT,
+   OSMTPD_PHASE_LINK_AUTH,
OSMTPD_PHASE_LINK_CONNECT,
OSMTPD_PHASE_LINK_DISCONNECT,
OSMTPD_PHASE_LINK_GREETING,
@@ -67,6 +69,12 @@ enum osmtpd_phase {
OSMTPD_PHASE_TIMEOUT
 };

+enum osmtpd_auth_result {
+   OSMTPD_AUTH_PASS,
+   OSMTPD_AUTH_FAIL,
+   OSMTPD_AUTH_ERROR
+};
+
 #define OSMTPD_NEED_SRC 1 << 0
 #define OSMTPD_NEED_DST 1 << 1
 #define OSMTPD_NEED_RDNS 1 << 2
@@ -156,6 +164,8 @@ void osmtpd_register_report_server(int, void (*)(struct 

Re: Multiple dkim key with filter-dkimsign

2022-10-19 Thread Archange

Le 19/10/2022 à 09:10, Martijn van Duren a écrit :

On Wed, 2022-10-19 at 00:23 +0400, Archange wrote:

Le 19/10/2022 à 00:07, Martijn van Duren a écrit :

On Wed, 2022-10-19 at 00:02 +0400, Archange wrote:

Hi there,

Due to an issue with the rspamd filter running against rspamd 3.3
(https://github.com/poolpOrg/filter-rspamd/issues/41), I’m looking at
migrating my main server to dkimsign. I’m already using it on several
servers, but they all handle only one domain, and I’m now in need to
handle several domains (all incoming on the same interface).

Those might have different selectors, different keys… Is there a way to
specify those, or the only option is to have the same key and selector
for all domains?

Regards.


It's as you said, only a single key and selector can be used per
dkimsign instance. If I would allow multiple selectors/keys it would
require making the config a lot more complex without any additional
benefit that I can see at the time.

Not necessarily much more complex. I would just ditch -s/-k and make -d
accept domain:selector:keyfile triplets.

Why not take a step further?
domain:selector:keyfile:algorithm:canonicalization:headers

It opens the door to all kind of weirdness for which no real use case
has been presented yet.


You’re right.


This is probably not a good practice (e.g. I should likely have
different selector and dkim keys per server), but currently some domains
are shared with other servers and owners, and so is the dkim key for
those domains. I don’t want that key to be valid for other domains.

Bottom line, dkim is nothing more than "this entity" has seen this mail
in it's current form as long as the signature passes. The only reason
for adding support for multiple domains was because of dmarc, which
combines it with the question if the domain in the from-header matches
with the domain in the signature.

Bottom line is that it's not the domain that manages the key, but the
server and the domain only says that it trusts the server by handling
the public component.


Yeah, that is basically what I came to realize with this issue.


I guess the solution is indeed to switch to different keys per server
instead of per domain.

I'm not setting this in stone, but I need a little more then personal
preferences for adding more complexity to this piece of code which I
already consider quite the monster.

If you really want this you could probably redirect the mail back into
itself but on different ports and let every port have their own
dkimsign instance, similar to how the old dkimproxy setups would work:
https://myconan.net/blog/posts/4567/


I tried something like that in the first place but I was trying to do 
things at the filters level and couldn’t find a way. Thanks for pointing 
me towards the right direction, so for now (as it was easier to fix in 
place rather than switch the whole setup) I ended up using `mail-from 
regex "^.*@domain$"` to redirect on several local ports based on domain, 
and then filtering to dkimsign for each.