flashmob / go-guerrilla

Mini SMTP server written in golang
MIT License
2.75k stars 359 forks source link

Feature Request: Ability to return custom responses in validation process #151

Open landons opened 5 years ago

landons commented 5 years ago

Hey there,

I'd like to create an SPF validation processor. This works as part of the save process, due to allowing custom responses (i.e. #78), but there's a lot of unnecessary overhead in getting all the way through the DATA command before doing the check, so it'd be better to do it when validating the recipient (that's Postfix's default approach as well).

However, the return value for ValidateRcpt is just an error, which is assumed to semantically mean the user doesn't exist.

// server.go
// ...
client.PushRcpt(to)
rcptError := s.backend().ValidateRcpt(client.Envelope)
if rcptError != nil {
    client.PopRcpt()
    client.sendResponse(r.FailRcptCmd, " ", rcptError.Error())
} else {
    client.sendResponse(r.SuccessRcptCmd)
}

Are you open to a similar change to what you did for #78? Being able to return RFC 7208-compatible responses to the RCPT TO command would be awesome.

I'm happy to take a stab at it too if you're open to it.

landons commented 5 years ago

Actually, now I'll play devil's advocate against my own request for this specific case. It's rare for modern servers to use SPF by itself--DMARC constitutes the policy of what to do when SPF/DKIM checks fail, but the message body does need to be parsed to do DKIM or DMARC verification. I don't think I actually need this feature after all.

flashmob commented 5 years ago

Howdy @landons

You've raised a good point!

It would be a good feature to add, so do you mind if this could be re-opened?

It would be better to be able to analyze the message while it is getting saved. The current model doesn't allow for this, as you have noticed, but a new "processing the message as a stream" feature is being worked on PR #135

What does Postfix do once the spf or dkim checks fail? Does it allow the connection to continue, or does it terminate it?

landons commented 5 years ago

You got it.

I think you’re right that streaming would be a better approach than buffering the whole body in each processor! However, DKIM signatures include a hash of the body contents, so there might be no way around keeping the whole thing in memory for that step.

Here are the libraries I’m trying out: https://godoc.org/github.com/emersion/go-msgauth https://godoc.org/blitiri.com.ar/go/spf

The DKIM package does take a reader, but I’m pretty sure it uses a byte slice in the body canonicalization step. DKIM allows for declaring the number of bytes of the body to consider in the hash, but I think it’s still post-canonicalization, which implies buffering for anything but the “simple” algorithm.

As for failures, I’m not certain how postfix approaches those, but this is generally the logic prescribed in the RFCs:

If the domain publishes DMARC DNS entries, those entries specify what to do in failure cases for both SPF and DKIM and supersede the default handling prescribed for each. ESPs and MTAs have slightly different roles for how to handle.

A) None (let the mail through)

B) Quarantine (don’t go to inbox; up for discretion whether to move to spam, hold for future processing, reject, etc.; most ESP’s recommend the MTA pass it through so they can handle this one)

C) Reject (give back a failure response)

SPF and DKIM generally follow those same classifications, with clear “pass” and “failure” states (failures can be transient or permanent). SPF also has a “softfail” case, which is similar to DMARC’s “quarantine.”

jackwellsxyz commented 4 years ago

Hello @flashmob and @landons, I'd love to have DKIM/SPF/DMARC added to go-guerrilla as well. How's the pull request going? Need any assistance? Thanks!

flashmob commented 4 years ago

Hi, it's coming along nicely, had some great progress last few weeks.

Not sure if there's a stream-based DKIM library out there for verifying. The following seems like the entire message needs to be passed as a string to verify: https://godoc.org/github.com/emersion/go-msgauth/dkim#example-Verify

Now that a stream based mime-parser has been implemented in PR #135, it should be possible to implement a stream based DKIM verifier (since we can now parse the headers-on-the-fly). See the s_chunksaver.go file for examples for how the result from the mime parser is used.

As for SPF, perhaps it's something that should be verified on the connection level, ie, as soon as the client sends a MAIL FROM command? In that case - what do you think about adding an event and using api.go to allow you to set an event handler on each MAIL FROM (in this case, trigger an SPF check)

On Thu, 19 Sep 2019 at 00:11, jackwellsxyz notifications@github.com wrote:

Hello @flashmob https://github.com/flashmob and @landons https://github.com/landons, I'd love to have DKIM/SPF/DMARC added to go-guerrilla as well. How's the pull request going? Need any assistance? Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/flashmob/go-guerrilla/issues/151?email_source=notifications&email_token=AAE6MPYTUKZBXN6IA53BUXTQKIZJTA5CNFSM4HLD3GF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7AGM7I#issuecomment-532702845, or mute the thread https://github.com/notifications/unsubscribe-auth/AAE6MP3VWNPCG6LFD3VUFWDQKIZJTANCNFSM4HLD3GFQ .

landons commented 4 years ago

It’s easy to do SPF checks without reading the entire message body, but not with DKIM because the body contents are used in the signature. I think there’s a header in the spec to signal that only the first X bytes of the body are used in the signature, but its optional, and makes the verification less secure. Accordingly, a streaming approach might not be possible.

I abandoned the project I was working on with this (Apple all but obsoleted the idea), but I’m happy to weigh in if it’s helpful.

On Sep 18, 2019, at 7:59 AM, Flashmob notifications@github.com wrote:

Hi, it's coming along nicely, had some great progress last few weeks.

Not sure if there's a stream-based DKIM library out there for verifying. The following seems like the entire message needs to be passed as a string to verify: https://godoc.org/github.com/emersion/go-msgauth/dkim#example-Verify

Now that a stream based mime-parser has been implemented in PR #135, it should be possible to implement a stream based DKIM verifier (since we can now parse the headers-on-the-fly). See the s_chunksaver.go file for examples for how the result from the mime parser is used.

As for SPF, perhaps it's something that should be verified on the connection level, ie, as soon as the client sends a MAIL FROM command? In that case - what do you think about adding an event and using api.go to allow you to set an event handler on each MAIL FROM (in this case, trigger an SPF check)

On Thu, 19 Sep 2019 at 00:11, jackwellsxyz notifications@github.com wrote:

Hello @flashmob https://github.com/flashmob and @landons https://github.com/landons, I'd love to have DKIM/SPF/DMARC added to go-guerrilla as well. How's the pull request going? Need any assistance? Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/flashmob/go-guerrilla/issues/151?email_source=notifications&email_token=AAE6MPYTUKZBXN6IA53BUXTQKIZJTA5CNFSM4HLD3GF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7AGM7I#issuecomment-532702845, or mute the thread https://github.com/notifications/unsubscribe-auth/AAE6MP3VWNPCG6LFD3VUFWDQKIZJTANCNFSM4HLD3GFQ .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.