RE: proposed API breaks with async DNS

From: Murray S. Kucherawy <msk_at_cloudmark.com>
Date: Tue, 3 Nov 2009 00:15:55 -0800

> -----Original Message-----
> From: opendkim-dev-bounce_at_lists.opendkim.org [mailto:opendkim-dev-
> bounce_at_lists.opendkim.org] On Behalf Of Daniel Black
> Sent: Friday, October 30, 2009 10:02 PM
> To: opendkim-dev_at_lists.opendkim.org; opendkim-users_at_lists.opendkim.org
> Subject: proposed API breaks with async DNS
>
> Some are aware that I'm developing some libopendkim code to consolidate DNS
> querying and make it asynchronous. The plan is for queries for ADSP, DKIM-
> Signatures and reputation to occur as soon as the relevant header is
> passed in for processing.

One thing to consider is that an author signature that validates removes the need for an ADSP query, so starting one as soon as From: arrives might generate work that's not actually needed. It's impossible to say yet how often this might occur, so I've no idea whether to say it's a good idea or not-so-much.

> #1.
>
> As this will make policy and signature information available earlier
> would you like to set call back functions for these?
>
> e.g.
>
> DKIM_STAT dkim_set_signature_callback
> DKIM_LIB *libopendkim,
> DKIM_CBSTAT (*func)(DKIM *dkim, DKIM_SIGINFO *sigs,
> DKIM_SIGERROR
> status)
> );
>
> will do a calllback as soon as something invalid is known about the
> signature or DKIM_SIGERROR_OK - if all is finished and the signature validates.

Is the idea that the resolver, upon receiving a reply, would process the signature validation itself rather than just returning a DNS reply and letting libopendkim do the parsing? I'm wondering if we're blurring the demarcation between the resolver and libopendkim if so.

If not, the resolver would need a callback to perform reply processing once it's available, and it would have to handle timeouts as well (call the callback with a specific result code, maybe?), and a condition variable of some kind so that the requesting thread can block and be awakened when the reply arrives.

> #3.
>
> The result of this is that DKIM-Signature header fields need to be
> parsed
> earlier and, than unlike previous version, allocation of a continuous
> block of
> signatures is not practical.

I'm not sure I see the connection between earlier parsing and the necessity for linked lists. Can you describe what you have in mind there? I'm a little sensitive about changing the API there because there are some large sites that use this code as-is, so minimal changes to the existing public data structures and the API would be preferable.

> #4
>
> The current FFR_DKIM_REPUTATION uses the API:
>
> DKIM_STAT dkim_get_reputation __P((DKIM *dkim, DKIM_SIGINFO *sig,
> char *qroot, int *rep));
>
>
> Passing of the qroot value here make async DNS here impossible so here
> are
> some options:
> 1. make qroot a DKIM option
> 2. make qroot a list of DKIM options (more that one reputation service
> supported)

These are OK with me. If (2) then "rep" would need to become an array or list whose size matches the number of services being queried.

> 3. remove it and let the application fetch the domain with
> dkim_sig_getdomain
> and do its own reputation service.

I'm undecided about what seems best here. On one hand I'd like to support DKIM reputation by including it in the library to promote its use, but then again so far I've also sought to restrict the library to functions that are part of DKIM or ADSP and their extensions.

So maybe it should go in its own reputation library or, as you said, be moved directly to the application. As the currently supported reputation mechanism is only one experiment with many more to come, maybe library-izing it on its own is the right idea.

> #5 Namespace consistency:
>
> rfc2822_mailbox_split is the only function that doesn't have a dkim_
> prefix.
> So that we don't conflict with functions elsewhere that happen to be
> called
> the same thing lets use dkim_rfc2822_mailbox_split instead.

This is more for historical reasons (it was borrowed from another library). At this point I'm fine with renaming it on the trunk (i.e. in 1.2.0 and later).

> #6 ASYNC DNS API exported
>
> The async DNS may provide API for doing your own async DNS for things
> like
> VBR, SPF, CSV (insert other three letter email acronym).
>
> DKIM_STAT dkim_dns_callback(const char *query, int type, void (*f)(char
> *result, DKIM_DNSSEC, DKIM_DNSERROR), unsigned timeoutms);
>
> here you setup the query and receive a callback when its done. it
> follows
> CNAMES etc.

You'll also need a mechanism to see if a query has been completed yet so if the requesting code wakes up, it knows if the query is pending, has timed out, arrived but was corrupt, is being parsed, or has completed. As I suggested, it also needs a mechanism for the caller to go synchronous if it has nothing at all to do until the reply arrives or a timeout occurs.
Received on Tue Nov 03 2009 - 08:16:06 PST