dkim: generated header not properly formatted

ludusrusso commented 4 years ago

Hi, I'm playing with this library and I'm incurring in a strange issue.

In some case, when I try to generate a DKIM signature, the generated header get an extra "\r\n" that makes the mail invalid.

For istance, if I try to sign this email:

From: test@test.com
To: test@test.com
Subject: Send Test
Message-ID: <xxxxx@xxxxx>
Return-Path: <test@test>
Date: Mon, 09 Mar 2020 17:04:25 +0100

Message Body

I got this signed message

DKIM-Signature: a=rsa-sha256; bh=opmXtA9gMR670DN8CmqNEL8Q4RBdtB/ULgaN/SkjFn
 0=; c=simple/simple; d=mail.ludusrusso.space; h=From:To:Subject:Message-ID:
 Return-Path:Date; s=brisbane; t=1583769865; v=1; b=JHLkTc5vtTRA4udN9sAMcPWx
 TLLfqQ6ry++3GLbcWjhR769N4hK7kaL2P1iseXaoVbQpxVGzZYz9x5P8q9brfYZEuuQciWPvk1n
 k99E685amWpMsSqUnsHWifpeFtZpNHwPSVWJp03Kmeq2wE85+Fauq/P+7bVfRa/N1hUIFUpk=;

From: test@test.com
To: test@test.com
Subject: Send Test
Message-ID: <xxxxx@xxxxx>
Return-Path: <test@test>
Date: Mon, 09 Mar 2020 17:04:25 +0100

Message Body

notice that the extra black line added before From header makes the message to be worngly interpreded by a receiving SMTP.

The issues seems related to the length of the header, in fact if I try to add (or remove) headers from the message it works properly:

DKIM-Signature: a=rsa-sha256; bh=bvW0aiEWdP0ie2rawBb+IiTxlHI9KgEIYjEYTGzMRa
 0=; c=simple/simple; d=mail.ludusrusso.space; h=X-Extra-Header:From:To:Subj
 ect:Message-ID:Return-Path:Date; s=brisbane; t=1583770035; v=1; b=Rer+wvDGH
 tVS7SwYC5kdouGz6Su0B0iEvegWcwQe4GcMERi5QfJWV8hDnjappa8KX1fst2YmhFjUST+Ai3zM
 dyH1iKJiIm78Yt0n3c1f/95bP9Ey6xc1t1zYcJm3t/zQ8Sho/XjPRqdilOJYXPZB18y5JILnV67
 69eKFcsnzmic=;
X-Extra-Header: extra header
From: test@test.com
To: test@test.com
Subject: Send Test
Message-ID: <xxxxx@xxxxx>
Return-Path: <test@test>
Date: Mon, 09 Mar 2020 17:07:15 +0100

Longer message body

Note the extra header here.

If you give me some tips, I could try to solve the issue and submit a PR! It seems to me that the issue is due to the 76 char limitation of the message line.

emersion commented 4 years ago

Hmm, interesting. Is len equal to zero in https://github.com/emersion/go-msgauth/blob/master/dkim/header.go#L57?

ludusrusso commented 4 years ago

No, in the last iteration the len is 1 and the character is "\n".

I wrote a simple test program to reproduce the error. It seems due to the lenght of the DKIM header string.

package main

import (
    "bytes"
    "crypto/rand"
    "crypto/rsa"
    "fmt"
    "log"
    "strings"
    "time"

    "github.com/emersion/go-msgauth/dkim"
)

func main() {
    header := make(map[string]string)
    header["Return-Path"] = fmt.Sprintf("xxx+111@xxx.xxxx.xxxx")
    header["From"] = "xxxx@xxxx.co"
    header["To"] = "xxxx@xxxx.co"
    header["Subject"] = "Send Test"
    header["Message-ID"] = "<xxxx@xxxx>"
    header["Date"] = time.Now().Format(time.RFC1123Z)

    message := ""
    for k, v := range header {
        message += fmt.Sprintf("%s: %s\r\n", k, v)
    }

    message += "\r\n" + "Longer message body"

    fmt.Printf("%v\n", message)

    reader := rand.Reader
    bitSize := 1024
    key, _ := rsa.GenerateKey(reader, bitSize)

    r := strings.NewReader(message)
    options := &dkim.SignOptions{
        Domain:   "test.xxxxxxxxxx.xxxxx",
        Selector: "brisbane",
        Signer:   key,
    }

    var b bytes.Buffer
    if err := dkim.Sign(&b, r, options); err != nil {
        log.Fatal(err)
    }

    fmt.Printf("Message:\n%v\n\n", b.String())
}

Output:

Return-Path: xxx+111@xxx.xxxx.xxxx
From: xxxx@xxxx.co
To: xxxx@xxxx.co
Subject: Send Test
Message-ID: <xxxx@xxxx>
Date: Tue, 10 Mar 2020 18:02:53 +0100

Longer message body
Message:
DKIM-Signature: a=rsa-sha256; bh=bvW0aiEWdP0ie2rawBb+IiTxlHI9KgEIYjEYTGzMRa
 0=; c=simple/simple; d=test.xxxxxxxxxx.xxxxx; h=Return-Path:From:To:Subject
 :Message-ID:Date; s=brisbane; t=1583859773; v=1; b=IIlaZ79pLDdypX2YzJuy9Evx
 sbrqF360CLhkQqMqVC/GIa1K/X8p8POny7WuVuFTMHLvA1zH9YcXgEgIH+X0GgGsqqzq8ETL+QG
 Z9g1EKCmIg+ZEYV5JnH25Ar6bJpGra3jg6sIxKi6CthpK50kd8T0Q3K/oZGot4BkLaPqogpo=;

Return-Path: xxx+111@xxx.xxxx.xxxx
From: xxxx@xxxx.co
To: xxxx@xxxx.co
Subject: Send Test
Message-ID: <xxxx@xxxx>
Date: Tue, 10 Mar 2020 18:02:53 +0100

Longer message body

emersion commented 4 years ago

Does it work properly if we remove crlf from https://github.com/emersion/go-msgauth/blob/master/dkim/header.go#L52 and add it back on line 66?

ludusrusso commented 4 years ago

Yes it does! I've submitted a pull request that adds a test that fails due to this issue and solves it!

emersion / go-msgauth

dkim: generated header not properly formatted #23