go-msgauth icon indicating copy to clipboard operation
go-msgauth copied to clipboard

DKIM verification assumes message receipt time is *now*

Open oogali opened this issue 1 year ago • 8 comments

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 avatar May 08 '24 16:05 oogali

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.

oogali avatar May 08 '24 16:05 oogali

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.

emersion avatar May 08 '24 16:05 emersion

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,
})

oogali avatar May 08 '24 16:05 oogali

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.

emersion avatar May 08 '24 16:05 emersion

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?

oogali avatar May 08 '24 17:05 oogali

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

emersion avatar May 09 '24 06:05 emersion

// 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.

AGWA avatar May 09 '24 11:05 AGWA

Good point!

emersion avatar May 09 '24 11:05 emersion