emersion / go-msgauth

🔏 A Go library and tools for DKIM, DMARC and Authentication-Results
MIT License
168 stars 52 forks source link

dkim: Provide a low-level Verify function for more control #10

Closed foxcpp closed 4 years ago

foxcpp commented 4 years ago

Use-case

I'm thinking of ways to make maddys DKIM support work well and it needs quite a lot of control over verification process for that. Here is my experience report so far:

Non-cancelable network I/O done by dkim.Verify makes latter less useful. Also, it is done not using custom DNS resolver implementation provided by maddy.

Currently I have to serialize header to bytes.Buffer and then pass it to dkim.Verify. Allowing to pass already parsed header would be nice.

It is not that important, as I can keep non-parsed header blob around and pass it to dkim.Verify, thus eliminating one part of the problem and hoping other is not that significant.

Proposed solution(s)

  1. Expose low-level functionality Expose internal structures and separate operations such as: Parse DKIM key record, parse DKIM signature header, verify the signature using provided header subset and body reader.

With that I would simply pass DKIM-Signature fields to dkim.ParseField, then fetch keys using replaceable DNS resolver with context.Context support, let dkim.ParseKey parse them and then finally pass everything to the dkim.VerifySig to do the actual verification.

Even more: I can take the subset of fields required for signature and pass it to dkim.VerifySig instead of resorting to serialize-then-parse hack. That could be complicated though, so I guess being able to simply pass go-message/textproto.Header is better..

This way I even will be able to distinguish different failure types and act accordingly (e.g. increase the "quarantine score" for messages with broken signatures, immediately quarantine messages with a signature without a corresponding key in DNS, but only if this is not caused by a temporary resolution error).

... or ...

  1. Add support for custom resolver (#4), context.Context, possibly address "already parsed header" case or leave it as is.

Tell me what you think. I am willing to help with the implementation of whatever solution you think is better.

foxcpp commented 4 years ago
type VerifierOptions struct {
  // if nil - net.LookupTXT is used, AD flag is used to indicate DNSSEC status which is then included into Verification struct.
  LookupTXT func(domain string) (ad bool, txt []string, err error)
}
foxcpp commented 4 years ago

Closing, not worth the effort.