emersion / go-msgauth

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

DKIM verification assumes message receipt time is *now* #63

Open oogali opened 5 months ago

oogali commented 5 months ago

I use go-msgauth/dkim to verify signatures in my internal MDA.

In my development environment, I had an issue where mail was backed up for a few days. Once I fixed the issue and resumed queue processing at the upstream MTA, I began receiving a number of DKIM signature expiration failures.

It appears that the timestamp for verification is always the time at which the message was presented to the library. https://github.com/emersion/go-msgauth/blob/master/dkim/verify.go#L268

I think it would be beneficial for both testing and delayed delivery if the caller could optionally present a timestamp to perform verification against rather than the current time.

oogali commented 5 months ago

I understand it's a bit of a quandary since the published DNS records that are used for verification may or may not be different from the time that the message was sent, versus the time go-msgauth performs the query.

But I don't think failing (or in an extreme instance, skipping DKIM verification altogether) on MTA-delayed emails is the right behavior.

emersion commented 5 months ago

In general when signing it's advisable to set the expiration to at least a few days to accommodate for cases like these.

I'm a bit wary about exposing security-sensitive details like this, it could easily be misused e.g. by passing the Date header field.

oogali commented 5 months ago

I understand that position.

If it helps shed more light on the situation, the email in question came from a newsletter delivered by Mailgun.

My guess is they're rotating keys on a daily basis because the DKIM verification result showed a 24-hour expiry.

In my MDA, I receive the message via LMTP, then queue it for processing with an out-of-band timestamp recorded at the moment of LMTP ingest.

I'm not requesting that you parse the Date header, but perhaps allow me as a library caller to perform some action along the lines of this crude example:

verifications, err := dkim.Verify(data, &dkim.VerifyOpts{
    Timestamp: receivedAt,
})
emersion commented 5 months ago

Yeah, in a similar way that the stdlib does it: https://pkg.go.dev/crypto/x509#VerifyOptions

I'm worried about library users misunderstanding how to set this option, and passing a user-controlled field (Date) instead of the time when the message hit a trusted SMTP server. I suppose some good docs could help here.

Good to know that Mailgun rotates keys aggressively! I thought you were generating the signature.

oogali commented 5 months ago

I agree that this could be a potential footgun and I'm not sure you can completely avoid it, but only hope to mitigate it with a combination of documentation and a strongly worded struct member name?

TimestampEmailWasActuallyReceivedAt or something equally descriptive but less absurd?

emersion commented 4 months ago

What about this?

// Time that the message was first received at the administrative domain of the
// verifier. Note, this must not be set to a user-controlled value. If zero,
// the current time is used.
VerifiedReceiptTime time.Time
AGWA commented 4 months ago

// Time that the message was first received at the administrative domain of the // verifier. Note, this must not be set to a user-controlled value. If zero, // the current time is used.

I think "user" is ambiguous as it could also refer to the user of the library. How about:

// Time that the message was first received at the administrative domain of the // verifier. Note, this value must not come from an untrusted source, such as // the sender of the email. If zero, the current time is used.

emersion commented 4 months ago

Good point!