mistralmail / mistralmail Goto Github PK

Tests are wrong here since they do not expect a 250 reply after DATA.

We need tests with commands in upper/lower/mixed case.

RFC 5321 section 2.4.

Verbs and argument values (e.g., "TO:" or "to:" in the RCPT command
and extension name keywords) are not case sensitive, with the sole
exception in this specification of a mailbox local-part (SMTP
Extensions may explicitly specify case-sensitive elements). That is,
a command verb, an argument value other than a mailbox local-part,
and free form text MAY be encoded in upper case, lower case, or any
mixture of upper and lower case with no impact on its meaning. The
local-part of a mailbox MUST BE treated as case sensitive.
Therefore, SMTP implementations MUST take care to preserve the case
of mailbox local-parts. In particular, for some hosts, the user
"smith" is different from the user "Smith". However, exploiting the
case sensitivity of mailbox local-parts impedes interoperability and
is discouraged. Mailbox domains follow normal DNS rules and are
hence not case sensitive.

Stel da ge deze cmds binnen krijgt:

helo
data
test mail
.

dan moet onze server na die data cmd al nen error geven dat er eerst een mail cmd moet zijn geweest.
Probleem is da de parser het geen dat erachter komt eerst parsed en dan pas een DataCmd returned.

We hebben zoiets nodig:

type DataCmd struct {
    data DeferedContent
}

type DeferedContent struct {
    br *bufio.Reader
}

func (c *DeferedContent) GetContents() string {
    // lees en return hetgeen dat achter het data cmd komt.
}

cmd.data.getContents()

als er geen email achter die TO komt returned de parser een RcptCmd. Zou eigenlijk een InvalidCmd moete returnen. InvalidCmd stuurt dan een SyntaxError naar de client. Denk eigenlijk dat het een SyntaxErrorParam moet zijn, maar daar hebbe wij geen cmd voor.

Edit: Er zijn wss nog zo'n gevallen

#31 (comment)

We should make handlers cleaner and make objects of them, so they can call different functions on the same members and can store config data and a state for easier implementation.

Because of a7130e1 the assertions will run in a different goroutine, which crashes goconvey.

Sending helo or ehlo without an address crashes the server.

// Uses args[0] without checking if there is an arg.
command = HeloCmd{Domain: args[0]}

Telenet gives this error when sending helo without an address:
501 HELO requires valid address

I've implemented a Blacklist struct/interface, but it isn't yet actually being used by the MTA.

https://github.com/gopistolet/gopistolet/blob/master/helpers/blacklist.go

Blacklist could panic on this:

func (ns *Nixspam) CheckIp(ip string) bool {
    index := sort.SearchStrings(ns.IpList, ip)
    return ns.IpList[index] == ip
}

index could equal len(ns.IpList).

SearchStrings searches for x in a sorted slice of strings and returns the index as specified by Search. The return value is the index to insert x if x is not present (it could be len(a)). The slice must be sorted in ascending order.

We need to work on how we store config in main.go to make it available for other component like e.g. the handlers.

When sending whitspace/newlines the parser creates smtp.UnknownCmd's.
The parser should just ignore whitespace.

Send \r\n after commands. We currently send \n but the spec says \r\n. And microsoft doesn't like \n...
We should also strip the \r\n from the commands we receive instead of only a \n.

Authentication-Results: receiver.example.org;
 spf=pass smtp.mailfrom=example.com;
 dkim=pass [email protected]

receiver.example.org should be id of authentication server (in this case GoPistolet) and not sender

https://github.com/gopistolet/gopistolet/blob/master/smtp/parser_test.go#L74
You use the underlying reader directly bypassing the datareader. Is there a reason for that?

In the Authentication-Results header we need access to the hostname, to specify the authserv-id as written in RFC 5451 section 2.2.

Authentication-Results: receiver.example.org; spf=pass smtp.mailfrom=example.com;

ping @trtstm

Check for error, otherwise we can get a deadlock. (Already fixed but not yet committed)

for line == "" {
    line, __ = br.ReadString('\n')
}

This command keeps the server waiting for a command. Even if I type a newline manually.

echo -ne 'helo' | nc localhost 2525

Don't know if it's netcat or our server's problem.

        state.reset()

        proto.Send(smtp.Answer{
            Status:  smtp.StartData,
            Message: "Start mail input; end with <CRLF>.<CRLF>",
        })

        data := []byte{}

Shouldn't be there since reset clears the from and rcpt fields.
Also use state.data instead of creating a new variable.

RFC 6409

About ports (abstract)

Abstract

This memo splits message submission from message relay, allowing each
service to operate according to its own rules (for security, policy,
etc.), and specifies what actions are to be taken by a submission
server.

Message relay is unaffected, and continues to use SMTP over port 25.

When conforming to this document, message submission uses the
protocol specified here, normally over port 587.

This separation of function offers a number of benefits, including
the ability to apply specific security or policy requirements.

About MTA and MSA (section 2.1)

Message Submission Agent (MSA)

A process that conforms to this specification. An MSA acts as a
submission server to accept messages from MUAs, and it either
delivers them or acts as an SMTP client to relay them to an MTA.

Message Transfer Agent (MTA)

A process that conforms to [SMTP-MTA]. An MTA acts as an SMTP server
to accept messages from an MSA or another MTA, and it either delivers
them or acts as an SMTP client to relay them to another MTA.

Message User Agent (MUA)

A process that acts (often on behalf of a user and with a user
interface) to compose and submit new messages, and to process
delivered messages.

For delivered messages, the receiving MUA may obtain and process the
message according to local conventions or, in what is commonly
referred to as a split-MUA model, Post Office Protocol [POP3] or IMAP
[IMAP4] is used to access delivered messages, whereas the protocol
defined here (or SMTP) is used to submit messages.

@trtstm

Next to Authentication-Results there is also the Received-SPF header to implement:

Received-SPF: fail (google.com: domain of [email protected] does not designate 137.191.225.35 as permitted sender) client-ip=137.191.225.35

RFC 4408 7. The Received-SPF Header Field

For example sending this command:

UNKN some unknown command

The parser creates this Command from it (note the \n): UnknownCmd{Cmd: "UNKN some unknown command\n"}.
I think it should not include the newline in the Cmd member. If this is changed we just have to change some expected values from the parser's tests.