From 021e003e784f046dfaf90953512228bd5577c0de Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 26 Oct 2022 11:59:03 +0200 Subject: [PATCH 1/4] Closes #74 This fix makes sure that generated message IDs via SetMessageID() are truly random and unique --- msg.go | 8 ++++---- msg_test.go | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/msg.go b/msg.go index d644d0a..5ef84eb 100644 --- a/msg.go +++ b/msg.go @@ -357,12 +357,12 @@ func (m *Msg) SetMessageID() { if err != nil { hn = "localhost.localdomain" } - ct := time.Now().Unix() + ct := time.Now().UnixNano() r := rand.New(rand.NewSource(ct)) - rn := r.Int() + rn := r.Int63() pid := os.Getpid() - - mid := fmt.Sprintf("%d.%d.%d@%s", pid, rn, ct, hn) + cts := fmt.Sprintf("%d", ct) + mid := fmt.Sprintf("%d.%d.%s@%s", pid, rn, cts[:15], hn) m.SetMessageIDWithValue(mid) } diff --git a/msg_test.go b/msg_test.go index 7fa7a05..2f27e39 100644 --- a/msg_test.go +++ b/msg_test.go @@ -695,6 +695,26 @@ func TestMsg_SetMessageIDWithValue(t *testing.T) { } } +// TestMsg_SetMessageIDRandomness tests the randomness of Msg.SetMessageID methods +func TestMsg_SetMessageIDRandomness(t *testing.T) { + var mids []string + for i := 0; i < 100; i++ { + m := NewMsg() + m.SetMessageID() + mid := m.GetGenHeader(HeaderMessageID) + mids = append(mids, mid[0]) + } + c := make(map[string]int) + for i := range mids { + c[mids[i]]++ + } + for k, v := range c { + if v > 1 { + t.Errorf("MessageID randomness not give. MessageID %q was generated %d times", k, v) + } + } +} + // TestMsg_FromFormat tests the FromFormat and EnvelopeFrom methods for the Msg object func TestMsg_FromFormat(t *testing.T) { tests := []struct { From 5bf0c10525f98b161a6c0156519e934f7a21e39f Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 26 Oct 2022 13:43:51 +0200 Subject: [PATCH 2/4] Randomness apparently not good enough for windows. Tests were failing. This fix improves it --- msg.go | 4 ++-- msg_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/msg.go b/msg.go index 5ef84eb..0e1aca3 100644 --- a/msg.go +++ b/msg.go @@ -360,9 +360,9 @@ func (m *Msg) SetMessageID() { ct := time.Now().UnixNano() r := rand.New(rand.NewSource(ct)) rn := r.Int63() - pid := os.Getpid() + pid := os.Getpid() * (r.Intn(10000) + 1) cts := fmt.Sprintf("%d", ct) - mid := fmt.Sprintf("%d.%d.%s@%s", pid, rn, cts[:15], hn) + mid := fmt.Sprintf("%d.%d.%s@%s", pid, rn, cts[:16], hn) m.SetMessageIDWithValue(mid) } diff --git a/msg_test.go b/msg_test.go index 2f27e39..dc55eca 100644 --- a/msg_test.go +++ b/msg_test.go @@ -710,7 +710,7 @@ func TestMsg_SetMessageIDRandomness(t *testing.T) { } for k, v := range c { if v > 1 { - t.Errorf("MessageID randomness not give. MessageID %q was generated %d times", k, v) + t.Errorf("MessageID randomness not given. MessageID %q was generated %d times", k, v) } } } From 8484e557d0be9a0004077b1c6c84bde9386c87ac Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 26 Oct 2022 14:05:25 +0200 Subject: [PATCH 3/4] Replace math/rand with crypto/rand Since the Windows tests were still failing, we are replacing the random number generation with math/rand with random string generation via crypto/rand --- msg.go | 12 ++++----- random.go | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 random.go diff --git a/msg.go b/msg.go index 0e1aca3..762dbd2 100644 --- a/msg.go +++ b/msg.go @@ -12,7 +12,6 @@ import ( "fmt" ht "html/template" "io" - "math/rand" "mime" "net/mail" "os" @@ -357,12 +356,11 @@ func (m *Msg) SetMessageID() { if err != nil { hn = "localhost.localdomain" } - ct := time.Now().UnixNano() - r := rand.New(rand.NewSource(ct)) - rn := r.Int63() - pid := os.Getpid() * (r.Intn(10000) + 1) - cts := fmt.Sprintf("%d", ct) - mid := fmt.Sprintf("%d.%d.%s@%s", pid, rn, cts[:16], hn) + rn, _ := randNum(100000000) + rm, _ := randNum(10000) + rs, _ := randomStringSecure(25) + pid := os.Getpid() * rm + mid := fmt.Sprintf("%d.%d%d.%s@%s", pid, rn, rm, rs, hn) m.SetMessageIDWithValue(mid) } diff --git a/random.go b/random.go new file mode 100644 index 0000000..5c23972 --- /dev/null +++ b/random.go @@ -0,0 +1,75 @@ +// SPDX-FileCopyrightText: 2022 Winni Neessen +// +// SPDX-License-Identifier: MIT + +package mail + +import ( + "crypto/rand" + "encoding/binary" + "fmt" + "math/big" + "strings" +) + +// Range of characters for the secure string generation +const cr = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890" + +// Bitmask sizes for the string generators (based on 93 chars total) +const ( + letterIdxBits = 7 // 7 bits to represent a letter index + letterIdxMask = 1<= 0; { + if r == 0 { + _, err := rand.Read(rp) + if err != nil { + return rs.String(), err + } + c, r = binary.BigEndian.Uint64(rp), letterIdxMax + } + if idx := int(c & letterIdxMask); idx < crl { + rs.WriteByte(cr[idx]) + i-- + } + c >>= letterIdxBits + r-- + } + + return rs.String(), nil +} + +// randNum returns a random number with a maximum value of n +func randNum(n int) (int, error) { + if n <= 0 { + return 0, fmt.Errorf("provided number is <= 0: %d", n) + } + mbi := big.NewInt(int64(n)) + if !mbi.IsUint64() { + return 0, fmt.Errorf("big.NewInt() generation returned negative value: %d", mbi) + } + rn64, err := rand.Int(rand.Reader, mbi) + if err != nil { + return 0, err + } + rn := int(rn64.Int64()) + if rn < 0 { + return 0, fmt.Errorf("generated random number does not fit as int64: %d", rn64) + } + return rn, nil +} From 38627631cf13b88ee4f43f7e9d93fc8a19aa48e4 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 26 Oct 2022 14:21:00 +0200 Subject: [PATCH 4/4] Test for random.go added --- random_test.go | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 random_test.go diff --git a/random_test.go b/random_test.go new file mode 100644 index 0000000..095e321 --- /dev/null +++ b/random_test.go @@ -0,0 +1,70 @@ +// SPDX-FileCopyrightText: 2022 Winni Neessen +// +// SPDX-License-Identifier: MIT + +package mail + +import ( + "strings" + "testing" +) + +// TestRandomStringSecure tests the randomStringSecure method +func TestRandomStringSecure(t *testing.T) { + tt := []struct { + testName string + length int + mustNotMatch string + }{ + {"20 chars", 20, "'"}, + {"100 chars", 100, "'"}, + {"1000 chars", 1000, "'"}, + } + + for _, tc := range tt { + t.Run(tc.testName, func(t *testing.T) { + rs, err := randomStringSecure(tc.length) + if err != nil { + t.Errorf("random string generation failed: %s", err) + } + if strings.Contains(rs, tc.mustNotMatch) { + t.Errorf("random string contains unexpected character. got: %s, not-expected: %s", + rs, tc.mustNotMatch) + } + if len(rs) != tc.length { + t.Errorf("random string length does not match. expected: %d, got: %d", tc.length, len(rs)) + } + }) + } +} + +// TestRandomNum tests the randomNum method +func TestRandomNum(t *testing.T) { + tt := []struct { + testName string + max int + }{ + {"Max: 1", 1}, + {"Max: 20", 20}, + {"Max: 50", 50}, + {"Max: 100", 100}, + {"Max: 1000", 1000}, + {"Max: 10000", 10000}, + {"Max: 100000000", 100000000}, + } + + for _, tc := range tt { + t.Run(tc.testName, func(t *testing.T) { + rn, err := randNum(tc.max) + if err != nil { + t.Errorf("random number generation failed: %s", err) + } + if rn < 0 { + t.Errorf("random number generation failed: %d is smaller than zero", rn) + } + if rn > tc.max { + t.Errorf("random number generation failed: %d is bigger than given value %d", rn, tc.max) + } + }) + } +}