emersion / go-msgauth

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

fix body hash issue in relaxe mode #38 #37

Closed mschneider82 closed 3 years ago

mschneider82 commented 3 years ago

I have added a test which failed before. With this fix i can sucessfully verify my testmail which was valid but the libray said body hash missmatch.

verify issue described in #38

emersion commented 3 years ago

So, it seems like you're doing essentially this:

b = bytes.TrimRight(b, "\r")

ie. removing trailing \r if any. Is this correct?

This should likely be part of the relaxedBodyCanonicalizer.Write loop, because it's specific to the relaxed mode.


This seems to be yet another quirk for relaxed mode. The RFC doesn't seem to mention it. Or maybe it's part of "Ignore all whitespace at the end of lines"? Or maybe "Ignore all empty lines at the end of the message body"?

mschneider82 commented 3 years ago

@emersion yes its basicly bytes.TrimRight, but i just remove exact one \r at the end. not more (TrimRight could trim more than one).

mschneider82 commented 3 years ago

are you thinking about?

b = fixCRLF(b)
b = bytes.TrimRight(b, "\r")

this seems like fixCRLF doesnt fix correct, thats why i included it into fixCRLF()

emersion commented 3 years ago

Hmm, wait. Your bug only happens when a slice passed to Write ends with a \r. That likely happens because a CRLF sequence is split between two Write calls:

Write([]byte("line1\r"))
Write([]byte("\nline2"))

I've written a failing test:

func TestRelaxedCanonicalizer_CanonicalBody_splitCRLF(t *testing.T) {
    want := "line 1\r\nline 2\r\n"
    writes := [][]byte{
        []byte("line 1\r"),
        []byte("\nline 2"),
    }

    var b bytes.Buffer
    wc := new(relaxedCanonicalizer).CanonicalizeBody(&b)
    for _, b := range writes {
        if _, err := wc.Write(b); err != nil {
            t.Errorf("Expected no error while writing to relaxed body canonicalizer, got: %v", err)
        }
    }
    if err := wc.Close(); err != nil {
        t.Errorf("Expected no error while closing relaxed body canonicalizer, got: %v", err)
    } else if s := b.String(); s != want {
        t.Errorf("Expected canonical body to be %q, but got %q", want, s)
    }
}
emersion commented 3 years ago

Superseded by https://github.com/emersion/go-msgauth/pull/39