emersion / go-msgauth

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

dkim.Sign blocks indefinitely on early errors #8

Closed mgnsk closed 4 years ago

mgnsk commented 5 years ago

Issue description

When the goroutine used in dkim.NewSigner returns early due to some error, the done channel never gets closed and thus the defer s.Close() in dkim.Sign blocks undefinitely.

Steps to reproduce the issue

This is a failing test:

package dkim_test

import (
    "bytes"
    "crypto/x509"
    "encoding/pem"
    "testing"

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

func TestSignInvalidMessage(t *testing.T) {

    rawMessage := bytes.NewBuffer([]byte("this is an invalid message"))

    key := []byte(`-----BEGIN RSA PRIVATE KEY-----
MIIEpAIBAAKCAQEA2lasFKCWoAn6nBZ1J4WZP/5T2dfqHPW2iye5vzXyAFLTNr7v
nFYYj8XcUldneSXWzbvh47oZlyA3MEFn0ICrtg2nRXSeh6yjvuicoslNOtB1Z2lp
sjBJODIrhxRs/qSgv3h0AtlBXw8nFBUglaJrdxRo3AeWNIoSo0Ai9V2lTdi6Um4H
FL0eI2E5jg1G8ALt+g8JSp+hZh7y8z4xrW/8pikK71BTVhfoVqTwItS65uNRtzmw
27ymGNTJ62wfXEYilezDsOXamgvAMbGkPCAYMXvO0ChmKQtUdx+IAxba7pfFIobE
Gvt0eABOhlCIuplQwBbEhK1D30goV4KcNy6q3QIDAQABAoIBAQCRezbl96rlsECA
SKZ/UxGuBjSw7qFb8o1TY4Ds23EIrid2TvsxXFy5T8liREL6AjCCnTICnzn17M1Z
JfuafmHryGUwbmhDVtE0n6HfBeqjycqwwRhgVrQy8Zr3QrDta5yAeC40x7Y7NMmB
JCK2EacxjTPhiFyZXXbVuCKTA3blystm76JkXSov6i5wIpF6o5cvDvAkVRZSz0zY
XvkdGgGTjQlDLfiq7EGNipd8V7ikjS+o4egugc8LQuJ3cC/gEuMibU+ULuvkIhTy
6+Ylo122NmQnKau5WVNecXyMj3ltQqNNj2WPYFuiWtNW/reB5jX1exVZ/RzDwYiy
rj3W1kyBAoGBAP2BEBIg6CfJgQjw4ymMTqAhmG89RexKZS/9nipLDjL+MjGx2mJs
msfxboY9aXvGs6nS0f//dd1B83iMWrQNhrFbGdje8dvQsSIW+geFWyNHsKC95pOV
417TrtTEgPSwaljbpyeE5+Zd9L2XQAbAy6ZUTHUEfWx2azWY0kNQuRH1AoGBANx8
+jG152b5NsyWZ7UmDILThsNl76EJNt4XuNRzgWHV1wXLDeVSm7O+FgAM9bAIxc2E
TJOpxxeNurWm5vqzZqqgeb/4BUz5KAHCcp3yoKMQjixrtPeJiRt2z31YW0l6QU4S
kJ1qBtPlPt00dqPJXqfj6rON3fZnrVe9x9E80txJAoGAV9VG7zENnvN3TNTBsFyX
xW2+dhRhzLv+EUGrcnXs5ogidgtsYhvFCS/CnqpaiPNQvq936V3mxZGbPRJMPwRM
vdiVvQmJ/SJyrSAO41o2OKQXM6p4YHxXejyX38px79XMExuP7+ZhvvSg3quwGGbm
aKvejdDPcCwbe0eG2qH2bZ0CgYBstVHF4KHOq2DRTfaj4baZaiEvhbq38wsSRS/j
z28jBYOWX57iSfBqlnXSYJFh0XF0+p2m0DZQ7pf3p+qKAJnF1okwlOBIKzAGbhCE
v3Nj8m2miRQYV785w0JZ0o5vk89O5uhWNEhZgNWVyqAT8NyyejTlgjTFoChe8jrq
dsqfwQKBgQCVptrO3fMd+SaY3KBhw0ebEfJT7Q1mTv0LufIYpKxJ7Hb3+hRwJNoO
UdVdRNFimFJDULU76KuurvPEopZfI94uQB8zgzn/ENYWgWiIo4f1H/BqVzO4vEdN
01OSrsZ2QwyaBffRNc2QxX7ZGALmoaW7sJ+yXTf73+yS0tGQvyjfgQ==
-----END RSA PRIVATE KEY-----
       `)

    block, _ := pem.Decode([]byte(key))
    if block == nil {
        panic("no key")
    }

    dkimKey, err := x509.ParsePKCS1PrivateKey(block.Bytes)
    if err != nil {
        panic("parse err")
    }

    opts := &dkim.SignOptions{
        Domain:                 "example.com",
        Selector:               "selector",
        Signer:                 dkimKey,
        HeaderCanonicalization: dkim.CanonicalizationRelaxed,
        BodyCanonicalization:   dkim.CanonicalizationSimple,
        HeaderKeys:             []string{"From", "To"},
    }

    signedMessage := &bytes.Buffer{}
    if err := dkim.Sign(signedMessage, rawMessage, opts); err != nil {
        panic(err)
    }
}

What's the expected result?

It should return an error.

What's the actual result?

It blocks indefinitely.

Additional details

I would propose moving the close(done) to the beginning of the goroutine as defer close(done) in https://github.com/emersion/go-msgauth/blob/master/dkim/sign.go#L270