From 3100a0fac0845453d6b6e7876e006da00dd11603 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 2 Jan 2025 12:46:31 +0100 Subject: [PATCH 01/47] Move test files them to the testdata directory Moved all dummy certificate and key files along with their licenses to the testdata directory for better organization. --- dummy-chain-cert-ecdsa.pem => testdata/dummy-chain-cert-ecdsa.pem | 0 .../dummy-chain-cert-ecdsa.pem.license | 0 dummy-chain-cert-rsa.pem => testdata/dummy-chain-cert-rsa.pem | 0 .../dummy-chain-cert-rsa.pem.license | 0 dummy-child-key-ecdsa.pem => testdata/dummy-child-key-ecdsa.pem | 0 .../dummy-child-key-ecdsa.pem.license | 0 dummy-child-key-rsa.pem => testdata/dummy-child-key-rsa.pem | 0 .../dummy-child-key-rsa.pem.license | 0 8 files changed, 0 insertions(+), 0 deletions(-) rename dummy-chain-cert-ecdsa.pem => testdata/dummy-chain-cert-ecdsa.pem (100%) rename dummy-chain-cert-ecdsa.pem.license => testdata/dummy-chain-cert-ecdsa.pem.license (100%) rename dummy-chain-cert-rsa.pem => testdata/dummy-chain-cert-rsa.pem (100%) rename dummy-chain-cert-rsa.pem.license => testdata/dummy-chain-cert-rsa.pem.license (100%) rename dummy-child-key-ecdsa.pem => testdata/dummy-child-key-ecdsa.pem (100%) rename dummy-child-key-ecdsa.pem.license => testdata/dummy-child-key-ecdsa.pem.license (100%) rename dummy-child-key-rsa.pem => testdata/dummy-child-key-rsa.pem (100%) rename dummy-child-key-rsa.pem.license => testdata/dummy-child-key-rsa.pem.license (100%) diff --git a/dummy-chain-cert-ecdsa.pem b/testdata/dummy-chain-cert-ecdsa.pem similarity index 100% rename from dummy-chain-cert-ecdsa.pem rename to testdata/dummy-chain-cert-ecdsa.pem diff --git a/dummy-chain-cert-ecdsa.pem.license b/testdata/dummy-chain-cert-ecdsa.pem.license similarity index 100% rename from dummy-chain-cert-ecdsa.pem.license rename to testdata/dummy-chain-cert-ecdsa.pem.license diff --git a/dummy-chain-cert-rsa.pem b/testdata/dummy-chain-cert-rsa.pem similarity index 100% rename from dummy-chain-cert-rsa.pem rename to testdata/dummy-chain-cert-rsa.pem diff --git a/dummy-chain-cert-rsa.pem.license b/testdata/dummy-chain-cert-rsa.pem.license similarity index 100% rename from dummy-chain-cert-rsa.pem.license rename to testdata/dummy-chain-cert-rsa.pem.license diff --git a/dummy-child-key-ecdsa.pem b/testdata/dummy-child-key-ecdsa.pem similarity index 100% rename from dummy-child-key-ecdsa.pem rename to testdata/dummy-child-key-ecdsa.pem diff --git a/dummy-child-key-ecdsa.pem.license b/testdata/dummy-child-key-ecdsa.pem.license similarity index 100% rename from dummy-child-key-ecdsa.pem.license rename to testdata/dummy-child-key-ecdsa.pem.license diff --git a/dummy-child-key-rsa.pem b/testdata/dummy-child-key-rsa.pem similarity index 100% rename from dummy-child-key-rsa.pem rename to testdata/dummy-child-key-rsa.pem diff --git a/dummy-child-key-rsa.pem.license b/testdata/dummy-child-key-rsa.pem.license similarity index 100% rename from dummy-child-key-rsa.pem.license rename to testdata/dummy-child-key-rsa.pem.license From c48795ad91a11de95873eec8d9c25744b4d8c921 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 2 Jan 2025 12:47:45 +0100 Subject: [PATCH 02/47] Fix typo and clarify S/MIME message signing feature Corrected "S/MIME singed messages" to "S/MIME message signing" for accuracy. Also added clarification that this feature is experimental and currently supports only single-part messages. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index eebfc067..5f6429ce 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ Here are some highlights of go-mail's featureset: * [X] Custom error types for delivery errors * [X] Custom dial-context functions for more control over the connection (proxing, DNS hooking, etc.) * [X] Output a go-mail message as EML file and parse EML file into a go-mail message -* [X] S/MIME singed messages +* [X] S/MIME message signing (EXPERIMENTAL - currently only for single-part messages) go-mail works like a programatic email client and provides lots of methods and functionalities you would consider standard in a MUA. From 66a79322e8f203843a847b362968e41d851dd78f Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 2 Jan 2025 12:51:49 +0100 Subject: [PATCH 03/47] Refactor S/MIME constants for consistency and readability. Updated naming of S/MIME-related constants and types to adhere to Go naming conventions, improving clarity and maintaining uniformity across the codebase. Adjusted relevant tests and method calls to reflect these changes. --- encoding.go | 22 ++++++++++++++++------ encoding_test.go | 4 ++-- msg.go | 2 +- msg_test.go | 10 +++++----- msgwriter.go | 2 +- 5 files changed, 25 insertions(+), 15 deletions(-) diff --git a/encoding.go b/encoding.go index 8c38cea8..7e15c4fe 100644 --- a/encoding.go +++ b/encoding.go @@ -172,19 +172,22 @@ const ( // TypeTextPlain represents the MIME type for plain text content. TypeTextPlain ContentType = "text/plain" - // typeSMimeSigned represents the MIME type for S/MIME singed messages. - typeSMimeSigned ContentType = `application/pkcs7-signature; name="smime.p7s"` + // TypeSMIMESigned represents the MIME type for S/MIME singed messages. + TypeSMIMESigned ContentType = `application/pkcs7-signature; name="smime.p7s"` ) const ( // MIMEAlternative MIMEType represents a MIME multipart/alternative type, used for emails with multiple versions. MIMEAlternative MIMEType = "alternative" - // MIMEMixed MIMEType represents a MIME multipart/mixed type used for emails containing different types of content. + + // MIMEMixed MIMEType represents a MIME multipart/mixed type used fork emails containing different types of content. MIMEMixed MIMEType = "mixed" + // MIMERelated MIMEType represents a MIME multipart/related type, used for emails with related content entities. MIMERelated MIMEType = "related" - // MIMESMime MIMEType represents a MIME multipart/signed type, used for siging emails with S/MIME. - MIMESMime MIMEType = `signed; protocol="application/pkcs7-signature"; micalg=sha-256` + + // MIMESMIME MIMEType represents a MIME multipart/signed type, used for siging emails with S/MIME. + MIMESMIME MIMEType = `signed; protocol="application/pkcs7-signature"; micalg=sha-256` ) // String satisfies the fmt.Stringer interface for the Charset type. @@ -223,7 +226,14 @@ func (e Encoding) String() string { return string(e) } -// String is a standard method to convert an MIMEType into a printable format +// String satisfies the fmt.Stringer interface for the MIMEType type. +// It converts an MIMEType into a printable format. +// +// This method returns the string representation of the MIMEType, which can be used +// for displaying or logging purposes. +// +// Returns: +// - A string representation of the MIMEType. func (e MIMEType) String() string { return string(e) } diff --git a/encoding_test.go b/encoding_test.go index 4c7778a8..8a909c5c 100644 --- a/encoding_test.go +++ b/encoding_test.go @@ -63,7 +63,7 @@ func TestContentType_String(t *testing.T) { }, { - "ContentType: pkcs7-signature", typeSMimeSigned, + "ContentType: pkcs7-signature", TypeSMIMESigned, `application/pkcs7-signature; name="smime.p7s"`, }, } @@ -136,7 +136,7 @@ func TestMimeType_String(t *testing.T) { {MIMEAlternative, "alternative"}, {MIMEMixed, "mixed"}, {MIMERelated, "related"}, - {MIMESMime, `signed; protocol="application/pkcs7-signature"; micalg=sha-256`}, + {MIMESMIME, `signed; protocol="application/pkcs7-signature"; micalg=sha-256`}, } for _, tt := range tests { t.Run(tt.mt.String(), func(t *testing.T) { diff --git a/msg.go b/msg.go index d2583f1a..09cc5d01 100644 --- a/msg.go +++ b/msg.go @@ -2291,7 +2291,7 @@ func (m *Msg) createSignaturePart(encoding Encoding, contentType ContentType, ch return nil, err } - signaturePart := m.newPart(typeSMimeSigned, WithPartEncoding(EncodingB64), WithSMimeSinging()) + signaturePart := m.newPart(TypeSMIMESigned, WithPartEncoding(EncodingB64), WithSMimeSinging()) signaturePart.SetContent(*signedMessage) return signaturePart, nil diff --git a/msg_test.go b/msg_test.go index c1bce596..1a8e6680 100644 --- a/msg_test.go +++ b/msg_test.go @@ -6787,8 +6787,8 @@ func TestMsg_createSignaturePart(t *testing.T) { if part.GetEncoding() != EncodingB64 { t.Errorf("createSignaturePart() method failed. Expected encoding: %s, got: %s", EncodingB64, part.GetEncoding()) } - if part.GetContentType() != typeSMimeSigned { - t.Errorf("createSignaturePart() method failed. Expected content type: %s, got: %s", typeSMimeSigned, part.GetContentType()) + if part.GetContentType() != TypeSMIMESigned { + t.Errorf("createSignaturePart() method failed. Expected content type: %s, got: %s", TypeSMIMESigned, part.GetContentType()) } if part.GetCharset() != CharsetUTF8 { t.Errorf("createSignaturePart() method failed. Expected charset: %s, got: %s", CharsetUTF8, part.GetCharset()) @@ -6825,7 +6825,7 @@ func TestMsg_signMessage(t *testing.T) { t.Errorf("createSignaturePart() method failed. Expected encoding: %s, got: %s", EncodingB64, signedPart.GetEncoding()) } if signedPart.GetContentType() != TypeTextPlain { - t.Errorf("createSignaturePart() method failed. Expected content type: %s, got: %s", typeSMimeSigned, signedPart.GetContentType()) + t.Errorf("createSignaturePart() method failed. Expected content type: %s, got: %s", TypeSMIMESigned, signedPart.GetContentType()) } if signedPart.GetCharset() != CharsetUTF8 { t.Errorf("createSignaturePart() method failed. Expected charset: %s, got: %s", CharsetUTF8, signedPart.GetCharset()) @@ -6838,8 +6838,8 @@ func TestMsg_signMessage(t *testing.T) { if signaturePart.GetEncoding() != EncodingB64 { t.Errorf("createSignaturePart() method failed. Expected encoding: %s, got: %s", EncodingB64, signaturePart.GetEncoding()) } - if signaturePart.GetContentType() != typeSMimeSigned { - t.Errorf("createSignaturePart() method failed. Expected content type: %s, got: %s", typeSMimeSigned, signaturePart.GetContentType()) + if signaturePart.GetContentType() != TypeSMIMESigned { + t.Errorf("createSignaturePart() method failed. Expected content type: %s, got: %s", TypeSMIMESigned, signaturePart.GetContentType()) } if signaturePart.GetCharset() != CharsetUTF8 { t.Errorf("createSignaturePart() method failed. Expected charset: %s, got: %s", CharsetUTF8, signaturePart.GetCharset()) diff --git a/msgwriter.go b/msgwriter.go index 9245df4b..337c5475 100644 --- a/msgwriter.go +++ b/msgwriter.go @@ -129,7 +129,7 @@ func (mw *msgWriter) writeMsg(msg *Msg) { } if msg.hasSMime() { - mw.startMP(MIMESMime, msg.boundary) + mw.startMP(MIMESMIME, msg.boundary) mw.writeString(DoubleNewLine) } if msg.hasMixed() { From c2c47bebec8cf71f8e4a6f1d404d9c3b4541cc6d Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 2 Jan 2025 13:50:53 +0100 Subject: [PATCH 04/47] Refactor signature tests and update license headers Revised test code to remove direct stdout usage in favor of buffers for better test isolation. Updated license headers with accurate attributions and documented the project's fork history for clarity. --- pkcs7.go | 15 ++++++++++++--- pkcs7_test.go | 40 +++++++++++++++++++++++++--------------- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/pkcs7.go b/pkcs7.go index 29eb15c6..c41aaaf0 100644 --- a/pkcs7.go +++ b/pkcs7.go @@ -1,4 +1,14 @@ -// SPDX-FileCopyrightText: 2022-2023 The go-mail Authors +// SPDX-FileCopyrightText: Copyright (c) 2015 Andrew Smith +// SPDX-FileCopyrightText: Copyright (c) 2017-2024 The mozilla services project (https://github.com/mozilla-services) +// SPDX-FileCopyrightText: Copyright (c) 2024-2025 The go-mail Authors +// +// Partially forked from https://github.com/mozilla-services/pkcs7, which in turn is also a fork +// of https://github.com/fullsailor/pkcs7. +// Use of the forked source code is, same as go-mail, governed by a MIT license. +// +// go-mail specific modifications by the go-mail Authors. +// Licensed under the MIT License. +// See [PROJECT ROOT]/LICENSES directory for more information. // // SPDX-License-Identifier: MIT @@ -10,6 +20,7 @@ import ( "crypto/ecdsa" "crypto/rand" "crypto/rsa" + _ "crypto/sha256" // for crypto.SHA256 "crypto/x509" "crypto/x509/pkix" "encoding/asn1" @@ -18,8 +29,6 @@ import ( "math/big" "sort" "time" - - _ "crypto/sha256" // for crypto.SHA256 ) var ( diff --git a/pkcs7_test.go b/pkcs7_test.go index e67e7e90..564c4fe4 100644 --- a/pkcs7_test.go +++ b/pkcs7_test.go @@ -1,19 +1,28 @@ -// SPDX-FileCopyrightText: 2022-2023 The go-mail Authors +// SPDX-FileCopyrightText: Copyright (c) 2015 Andrew Smith +// SPDX-FileCopyrightText: Copyright (c) 2017-2024 The mozilla services project (https://github.com/mozilla-services) +// SPDX-FileCopyrightText: Copyright (c) 2024-2025 The go-mail Authors +// +// Partially forked from https://github.com/mozilla-services/pkcs7, which in turn is also a fork +// of https://github.com/fullsailor/pkcs7. +// Use of the forked source code is, same as go-mail, governed by a MIT license. +// +// go-mail specific modifications by the go-mail Authors. +// Licensed under the MIT License. +// See [PROJECT ROOT]/LICENSES directory for more information. // // SPDX-License-Identifier: MIT package mail import ( + "bytes" "crypto" "crypto/rand" "crypto/rsa" "crypto/x509" "crypto/x509/pkix" "encoding/pem" - "fmt" "math/big" - "os" "testing" "time" ) @@ -26,54 +35,55 @@ func TestSign_E2E(t *testing.T) { } content := []byte("Hello World") for _, testDetach := range []bool{false, true} { - toBeSigned, err := newSignedData(content) - if err != nil { + toBeSigned, serr := newSignedData(content) + if serr != nil { t.Fatalf("Cannot initialize signed data: %s", err) } - if err := toBeSigned.addSigner(cert.Certificate, cert.PrivateKey, SignerInfoConfig{}); err != nil { + if serr = toBeSigned.addSigner(cert.Certificate, cert.PrivateKey, SignerInfoConfig{}); serr != nil { t.Fatalf("Cannot add signer: %s", err) } if testDetach { - t.Log("Testing detached signature") toBeSigned.detach() } else { - t.Log("Testing attached signature") } - signed, err := toBeSigned.finish() - if err != nil { + signed, serr := toBeSigned.finish() + if serr != nil { t.Fatalf("Cannot finish signing data: %s", err) } - if err := pem.Encode(os.Stdout, &pem.Block{Type: "PKCS7", Bytes: signed}); err != nil { + buf := bytes.NewBuffer(nil) + if serr = pem.Encode(buf, &pem.Block{Type: "PKCS7", Bytes: signed}); serr != nil { t.Fatalf("Cannot write signed data: %s", err) } } } +// certKeyPair represents a pair of an x509 certificate and its corresponding RSA private key. type certKeyPair struct { Certificate *x509.Certificate PrivateKey *rsa.PrivateKey } +// createTestCertificate generates a test certificate and private key pair. func createTestCertificate() (*certKeyPair, error) { + buf := bytes.NewBuffer(nil) signer, err := createTestCertificateByIssuer("Eddard Stark", nil) if err != nil { return nil, err } - fmt.Println("Created root cert") - if err := pem.Encode(os.Stdout, &pem.Block{Type: "CERTIFICATE", Bytes: signer.Certificate.Raw}); err != nil { + if err = pem.Encode(buf, &pem.Block{Type: "CERTIFICATE", Bytes: signer.Certificate.Raw}); err != nil { return nil, err } pair, err := createTestCertificateByIssuer("Jon Snow", signer) if err != nil { return nil, err } - fmt.Println("Created signer cert") - if err := pem.Encode(os.Stdout, &pem.Block{Type: "CERTIFICATE", Bytes: pair.Certificate.Raw}); err != nil { + if err = pem.Encode(buf, &pem.Block{Type: "CERTIFICATE", Bytes: pair.Certificate.Raw}); err != nil { return nil, err } return pair, nil } +// createTestCertificateByIssuer generates a certificate and private key pair, optionally signed by an issuer. func createTestCertificateByIssuer(name string, issuer *certKeyPair) (*certKeyPair, error) { priv, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { From d95732cdf10b416e551fd8d9888e9128ed1e5c2c Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 2 Jan 2025 13:51:49 +0100 Subject: [PATCH 05/47] Simplify parameter usage in getOIDForEncryptionAlgorithm Replaced the unused OIDDigestAlg parameter with a blank identifier (_) to improve clarity and indicate that it is not utilized. This change avoids misleading readers and aligns the function implementation with standard Go practices. --- pkcs7.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkcs7.go b/pkcs7.go index c41aaaf0..4180e58e 100644 --- a/pkcs7.go +++ b/pkcs7.go @@ -373,7 +373,7 @@ func verifyPartialChain(cert *x509.Certificate, parents []*x509.Certificate) err // getOIDForEncryptionAlgorithm takes the private key type of the signer and // the OID of a digest algorithm to return the appropriate signerInfo.DigestEncryptionAlgorithm -func getOIDForEncryptionAlgorithm(pkey crypto.PrivateKey, OIDDigestAlg asn1.ObjectIdentifier) (asn1.ObjectIdentifier, error) { +func getOIDForEncryptionAlgorithm(pkey crypto.PrivateKey, _ asn1.ObjectIdentifier) (asn1.ObjectIdentifier, error) { switch pkey.(type) { case *rsa.PrivateKey: return OIDEncryptionAlgorithmRSASHA256, nil From c4dca2c54475cb8c36cb4ce26dc8e5ed22fca4ee Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 2 Jan 2025 13:53:28 +0100 Subject: [PATCH 06/47] Refactor S/MIME-related methods and variables for consistency Renamed methods and variables to standardize S/MIME capitalization and naming, including `SignWithSMimeRSA` to `SignWithSMIMERSA` and `sMime` to `sMIME`. Additionally, fixed minor typos in error messages for improved clarity and updated comments for readability. --- msg.go | 45 ++++++++++++++++++++++++++------------------- msg_test.go | 48 ++++++++++++++++++++++++------------------------ 2 files changed, 50 insertions(+), 43 deletions(-) diff --git a/msg.go b/msg.go index 09cc5d01..6d3cbf55 100644 --- a/msg.go +++ b/msg.go @@ -29,7 +29,7 @@ import ( ) var ( - // ErrNoFromAddress should be used when a FROM address is requested but not set + // ErrNoFromAddress indicates that the FROM address is not set, which is required. ErrNoFromAddress = errors.New("no FROM address set") // ErrNoRcptAddresses indicates that no recipient addresses have been set. @@ -150,8 +150,8 @@ type Msg struct { // This can be useful in scenarios where headers are conditionally passed based on receipt - i. e. SMTP proxies. noDefaultUserAgent bool - // SMime represents a middleware used to sign messages with S/MIME - sMime *SMime + // sMIME holds a SMIME type to sign a Msg using S/MIME + sMIME *SMIME } // SendmailPath is the default system path to the sendmail binary - at least on standard Unix-like OS. @@ -341,23 +341,23 @@ func WithNoDefaultUserAgent() MsgOption { } } -// SignWithSMimeRSA configures the Msg to be signed with S/MIME -func (m *Msg) SignWithSMimeRSA(privateKey *rsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) error { - sMime, err := newSMimeWithRSA(privateKey, certificate, intermediateCertificate) +// SignWithSMIMERSA configures the Msg to be signed with S/MIME +func (m *Msg) SignWithSMIMERSA(privateKey *rsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) error { + smime, err := newSMIMEWithRSA(privateKey, certificate, intermediateCertificate) if err != nil { return err } - m.sMime = sMime + m.sMIME = smime return nil } -// SignWithSMimeECDSA configures the Msg to be signed with S/MIME -func (m *Msg) SignWithSMimeECDSA(privateKey *ecdsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) error { - sMime, err := newSMimeWithECDSA(privateKey, certificate, intermediateCertificate) +// SignWithSMIMEECDSA configures the Msg to be signed with S/MIME +func (m *Msg) SignWithSMIMEECDSA(privateKey *ecdsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) error { + smime, err := newSMIMEWithECDSA(privateKey, certificate, intermediateCertificate) if err != nil { return err } - m.sMime = sMime + m.sMIME = smime return nil } @@ -376,15 +376,15 @@ func (m *Msg) SignWithTLSCertificate(keyPairTlS *tls.Certificate) error { switch keyPairTlS.PrivateKey.(type) { case *rsa.PrivateKey: if intermediateCertificate == nil { - return m.SignWithSMimeRSA(keyPairTlS.PrivateKey.(*rsa.PrivateKey), leafCertificate, nil) + return m.SignWithSMIMERSA(keyPairTlS.PrivateKey.(*rsa.PrivateKey), leafCertificate, nil) } - return m.SignWithSMimeRSA(keyPairTlS.PrivateKey.(*rsa.PrivateKey), leafCertificate, intermediateCertificate) + return m.SignWithSMIMERSA(keyPairTlS.PrivateKey.(*rsa.PrivateKey), leafCertificate, intermediateCertificate) case *ecdsa.PrivateKey: if intermediateCertificate == nil { - return m.SignWithSMimeECDSA(keyPairTlS.PrivateKey.(*ecdsa.PrivateKey), leafCertificate, nil) + return m.SignWithSMIMEECDSA(keyPairTlS.PrivateKey.(*ecdsa.PrivateKey), leafCertificate, nil) } - return m.SignWithSMimeECDSA(keyPairTlS.PrivateKey.(*ecdsa.PrivateKey), leafCertificate, intermediateCertificate) + return m.SignWithSMIMEECDSA(keyPairTlS.PrivateKey.(*ecdsa.PrivateKey), leafCertificate, intermediateCertificate) default: return fmt.Errorf("unsupported private key type: %T", keyPairTlS.PrivateKey) } @@ -407,6 +407,8 @@ func getLeafCertificate(keyPairTlS *tls.Certificate) (*x509.Certificate, error) return cert, nil } +// SetCharset sets or overrides the currently set encoding charset of the Msg. +// // This method allows you to specify a character set for the email message. The charset is // important for ensuring that the content of the message is correctly interpreted by // mail clients. Common charset values include UTF-8, ISO-8859-1, and others. If a charset @@ -2282,11 +2284,11 @@ func (m *Msg) signMessage(msg *Msg) (*Msg, error) { // createSignaturePart creates an additional part that be used for storing the S/MIME signature of the given body func (m *Msg) createSignaturePart(encoding Encoding, contentType ContentType, charSet Charset, body []byte) (*Part, error) { - message, err := m.sMime.prepareMessage(encoding, contentType, charSet, body) + message, err := m.sMIME.prepareMessage(encoding, contentType, charSet, body) if err != nil { return nil, err } - signedMessage, err := m.sMime.signMessage(*message) + signedMessage, err := m.sMIME.signMessage(*message) if err != nil { return nil, err } @@ -2707,9 +2709,14 @@ func (m *Msg) hasMixed() bool { return m.pgptype == 0 && ((len(m.parts) > 0 && len(m.attachments) > 0) || len(m.attachments) > 1) } -// hasSMime returns true if the Msg has should be signed with S/MIME +// hasSMime determines if the Msg should be signed with S/MIME. +// +// This function checks whether the Msg has S/MIME type available. +// +// Returns: +// - true if the Msg has S/MIME type assigned and should be signed with S/MIME; otherwise false. func (m *Msg) hasSMime() bool { - return m.sMime != nil + return m.sMIME != nil } // hasRelated returns true if the Msg has related parts. diff --git a/msg_test.go b/msg_test.go index 1a8e6680..08e41e17 100644 --- a/msg_test.go +++ b/msg_test.go @@ -6402,12 +6402,12 @@ func TestMsg_hasAlt(t *testing.T) { func TestMsg_hasAltWithSMime(t *testing.T) { privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() if err != nil { - t.Errorf("failed to laod dummy crypto material. Cause: %v", err) + t.Fatalf("failed to load dummy crypto material: %s", err) } m := NewMsg() m.SetBodyString(TypeTextPlain, "Plain") m.AddAlternativeString(TypeTextHTML, "HTML") - if err := m.SignWithSMimeRSA(privateKey, certificate, intermediateCertificate); err != nil { + if err := m.SignWithSMIMERSA(privateKey, certificate, intermediateCertificate); err != nil { t.Errorf("failed to init smime configuration") } if m.hasAlt() { @@ -6419,10 +6419,10 @@ func TestMsg_hasAltWithSMime(t *testing.T) { func TestMsg_hasSMime(t *testing.T) { privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() if err != nil { - t.Errorf("failed to laod dummy crypto material. Cause: %v", err) + t.Fatalf("failed to load dummy crypto material: %s", err) } m := NewMsg() - if err := m.SignWithSMimeRSA(privateKey, certificate, intermediateCertificate); err != nil { + if err := m.SignWithSMIMERSA(privateKey, certificate, intermediateCertificate); err != nil { t.Errorf("failed to init smime configuration") } m.SetBodyString(TypeTextPlain, "Plain") @@ -6511,13 +6511,13 @@ func TestMsg_hasRelated(t *testing.T) { func TestMsg_WriteToWithSMIME(t *testing.T) { privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() if err != nil { - t.Errorf("failed to laod dummy crypto material. Cause: %v", err) + t.Fatalf("failed to load dummy crypto material: %s", err) } m := NewMsg() m.Subject("This is a test") m.SetBodyString(TypeTextPlain, "Plain") - if err := m.SignWithSMimeRSA(privateKey, certificate, intermediateCertificate); err != nil { + if err := m.SignWithSMIMERSA(privateKey, certificate, intermediateCertificate); err != nil { t.Errorf("failed to init smime configuration") } @@ -6674,13 +6674,13 @@ func TestSignWithSMime_ValidRSAKeyPair(t *testing.T) { t.Errorf("failed to laod dummy crypto material. Cause: %v", err) } m := NewMsg() - if err := m.SignWithSMimeRSA(privateKey, certificate, intermediateCertificate); err != nil { - t.Errorf("failed to set sMime. Cause: %v", err) + if err := m.SignWithSMIMERSA(privateKey, certificate, intermediateCertificate); err != nil { + t.Errorf("failed to set sMIME. Cause: %v", err) } - if m.sMime.privateKey.rsa == nil { + if m.sMIME.privateKey.rsa == nil { t.Errorf("WithSMimeSinging() - no private key is given") } - if m.sMime.certificate == nil { + if m.sMIME.certificate == nil { t.Errorf("WithSMimeSinging() - no certificate is given") } } @@ -6692,13 +6692,13 @@ func TestSignWithSMime_ValidECDSAKeyPair(t *testing.T) { t.Errorf("failed to laod dummy crypto material. Cause: %v", err) } m := NewMsg() - if err := m.SignWithSMimeECDSA(privateKey, certificate, intermediateCertificate); err != nil { - t.Errorf("failed to set sMime. Cause: %v", err) + if err := m.SignWithSMIMEECDSA(privateKey, certificate, intermediateCertificate); err != nil { + t.Errorf("failed to set sMIME. Cause: %v", err) } - if m.sMime.privateKey.ecdsa == nil { + if m.sMIME.privateKey.ecdsa == nil { t.Errorf("WithSMimeSinging() - no private key is given") } - if m.sMime.certificate == nil { + if m.sMIME.certificate == nil { t.Errorf("WithSMimeSinging() - no certificate is given") } } @@ -6711,12 +6711,12 @@ func TestSignWithTLSCertificate(t *testing.T) { } m := NewMsg() if err := m.SignWithTLSCertificate(keyPairTLS); err != nil { - t.Errorf("failed to set sMime. Cause: %v", err) + t.Errorf("failed to set sMIME. Cause: %v", err) } - if m.sMime.privateKey.ecdsa == nil { + if m.sMIME.privateKey.ecdsa == nil { t.Errorf("SignWithTLSCertificate() - no private key is given") } - if m.sMime.certificate == nil { + if m.sMIME.certificate == nil { t.Errorf("SignWithTLSCertificate() - no certificate is given") } } @@ -6734,12 +6734,12 @@ func TestSignWithTLSCertificate_WithKeyPairLeafNil(t *testing.T) { } m := NewMsg() if err := m.SignWithTLSCertificate(keyPairTLS); err != nil { - t.Errorf("failed to set sMime. Cause: %v", err) + t.Errorf("failed to set sMIME. Cause: %v", err) } - if m.sMime.privateKey.ecdsa == nil { + if m.sMIME.privateKey.ecdsa == nil { t.Errorf("SignWithTLSCertificate() - no private key is given") } - if m.sMime.certificate == nil { + if m.sMIME.certificate == nil { t.Errorf("SignWithTLSCertificate() - no certificate is given") } } @@ -6748,7 +6748,7 @@ func TestSignWithTLSCertificate_WithKeyPairLeafNil(t *testing.T) { func TestSignWithSMime_InvalidPrivateKey(t *testing.T) { m := NewMsg() - err := m.SignWithSMimeRSA(nil, nil, nil) + err := m.SignWithSMIMERSA(nil, nil, nil) if !errors.Is(err, ErrInvalidPrivateKey) { t.Errorf("failed to pre-check SignWithSMime method values correctly: %s", err) } @@ -6762,7 +6762,7 @@ func TestSignWithSMime_InvalidCertificate(t *testing.T) { } m := NewMsg() - err = m.SignWithSMimeRSA(privateKey, nil, nil) + err = m.SignWithSMIMERSA(privateKey, nil, nil) if !errors.Is(err, ErrInvalidCertificate) { t.Errorf("failed to pre-check SignWithSMime method values correctly: %s", err) } @@ -6775,7 +6775,7 @@ func TestMsg_createSignaturePart(t *testing.T) { t.Errorf("failed to laod dummy crypto material. Cause: %v", err) } m := NewMsg() - if err := m.SignWithSMimeRSA(privateKey, certificate, intermediateCertificate); err != nil { + if err := m.SignWithSMIMERSA(privateKey, certificate, intermediateCertificate); err != nil { t.Errorf("failed to init smime configuration") } body := []byte("This is the body") @@ -6807,7 +6807,7 @@ func TestMsg_signMessage(t *testing.T) { body := []byte("This is the body") m := NewMsg() m.SetBodyString(TypeTextPlain, string(body)) - if err := m.SignWithSMimeRSA(privateKey, certificate, intermediateCertificate); err != nil { + if err := m.SignWithSMIMERSA(privateKey, certificate, intermediateCertificate); err != nil { t.Errorf("failed to init smime configuration") } msg, err := m.signMessage(m) From dc812ed7ad20ab88c086f79f437a93c0597cfa72 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 2 Jan 2025 13:53:38 +0100 Subject: [PATCH 07/47] Delete obsolete util_test.go file The util_test.go file has been removed as it is no longer required. The file contained test functions for loading cryptographic materials that are not used or relevant in the current codebase. This cleanup reduces unnecessary code and improves maintainability. --- util_test.go | 73 ---------------------------------------------------- 1 file changed, 73 deletions(-) delete mode 100644 util_test.go diff --git a/util_test.go b/util_test.go deleted file mode 100644 index 8a6c6bc8..00000000 --- a/util_test.go +++ /dev/null @@ -1,73 +0,0 @@ -// SPDX-FileCopyrightText: 2022-2023 The go-mail Authors -// -// SPDX-License-Identifier: MIT - -package mail - -import ( - "crypto/ecdsa" - "crypto/rsa" - "crypto/tls" - "crypto/x509" -) - -const ( - certRSAFilePath = "dummy-chain-cert-rsa.pem" - keyRSAFilePath = "dummy-child-key-rsa.pem" - - certECDSAFilePath = "dummy-chain-cert-ecdsa.pem" - keyECDSAFilePath = "dummy-child-key-ecdsa.pem" -) - -// getDummyRSACryptoMaterial loads a certificate (RSA), the associated private key and certificate (RSA) is loaded from local disk for testing purposes -func getDummyRSACryptoMaterial() (*rsa.PrivateKey, *x509.Certificate, *x509.Certificate, error) { - keyPair, err := tls.LoadX509KeyPair(certRSAFilePath, keyRSAFilePath) - if err != nil { - return nil, nil, nil, err - } - - privateKey := keyPair.PrivateKey.(*rsa.PrivateKey) - - certificate, err := x509.ParseCertificate(keyPair.Certificate[0]) - if err != nil { - return nil, nil, nil, err - } - - intermediateCertificate, err := x509.ParseCertificate(keyPair.Certificate[1]) - if err != nil { - return nil, nil, nil, err - } - - return privateKey, certificate, intermediateCertificate, nil -} - -// getDummyECDSACryptoMaterial loads a certificate (ECDSA), the associated private key and certificate (ECDSA) is loaded from local disk for testing purposes -func getDummyECDSACryptoMaterial() (*ecdsa.PrivateKey, *x509.Certificate, *x509.Certificate, error) { - keyPair, err := tls.LoadX509KeyPair(certECDSAFilePath, keyECDSAFilePath) - if err != nil { - return nil, nil, nil, err - } - - privateKey := keyPair.PrivateKey.(*ecdsa.PrivateKey) - - certificate, err := x509.ParseCertificate(keyPair.Certificate[0]) - if err != nil { - return nil, nil, nil, err - } - - intermediateCertificate, err := x509.ParseCertificate(keyPair.Certificate[1]) - if err != nil { - return nil, nil, nil, err - } - - return privateKey, certificate, intermediateCertificate, nil -} - -// getDummyKeyPairTLS loads a certificate (ECDSA) as *tls.Certificate, the associated private key and certificate (ECDSA) is loaded from local disk for testing purposes -func getDummyKeyPairTLS() (*tls.Certificate, error) { - keyPair, err := tls.LoadX509KeyPair(certECDSAFilePath, keyECDSAFilePath) - if err != nil { - return nil, err - } - return &keyPair, err -} From c874c250766c4b3e610df05f1757366b174495ff Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 2 Jan 2025 13:53:57 +0100 Subject: [PATCH 08/47] Rename SMime to SMIME and update related methods/tests Unified the naming convention by renaming `SMime` to `SMIME` for improved consistency and readability. Updated all associated references in methods, comments, and test cases. Added helper functions to load dummy RSA and ECDSA cryptographic materials for testing purposes. --- msgwriter_test.go | 2 +- smime.go | 22 ++++++------ smime_test.go | 92 +++++++++++++++++++++++++++++++++++++++-------- 3 files changed, 90 insertions(+), 26 deletions(-) diff --git a/msgwriter_test.go b/msgwriter_test.go index 124ed3b6..fa21fc0f 100644 --- a/msgwriter_test.go +++ b/msgwriter_test.go @@ -774,7 +774,7 @@ func TestMsgWriter_writeMsg_SMime(t *testing.T) { } m := NewMsg() - if err := m.SignWithSMimeRSA(privateKey, certificate, intermediateCertificate); err != nil { + if err := m.SignWithSMIMERSA(privateKey, certificate, intermediateCertificate); err != nil { t.Errorf("failed to init smime configuration") } _ = m.From(`"Toni Tester" `) diff --git a/smime.go b/smime.go index 6a898cef..fa8ae098 100644 --- a/smime.go +++ b/smime.go @@ -39,18 +39,18 @@ func (p privateKeyHolder) get() crypto.PrivateKey { return p.rsa } -// SMime is used to sign messages with S/MIME -type SMime struct { +// SMIME is used to sign messages with S/MIME +type SMIME struct { privateKey privateKeyHolder certificate *x509.Certificate intermediateCertificate *x509.Certificate } -// newSMimeWithRSA construct a new instance of SMime with provided parameters +// newSMIMEWithRSA construct a new instance of SMIME with provided parameters // privateKey as *rsa.PrivateKey // certificate as *x509.Certificate // intermediateCertificate (optional) as *x509.Certificate -func newSMimeWithRSA(privateKey *rsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) (*SMime, error) { +func newSMIMEWithRSA(privateKey *rsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) (*SMIME, error) { if privateKey == nil { return nil, ErrInvalidPrivateKey } @@ -59,18 +59,18 @@ func newSMimeWithRSA(privateKey *rsa.PrivateKey, certificate *x509.Certificate, return nil, ErrInvalidCertificate } - return &SMime{ + return &SMIME{ privateKey: privateKeyHolder{rsa: privateKey}, certificate: certificate, intermediateCertificate: intermediateCertificate, }, nil } -// newSMimeWithECDSA construct a new instance of SMime with provided parameters +// newSMIMEWithECDSA construct a new instance of SMIME with provided parameters // privateKey as *ecdsa.PrivateKey // certificate as *x509.Certificate // intermediateCertificate (optional) as *x509.Certificate -func newSMimeWithECDSA(privateKey *ecdsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) (*SMime, error) { +func newSMIMEWithECDSA(privateKey *ecdsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) (*SMIME, error) { if privateKey == nil { return nil, ErrInvalidPrivateKey } @@ -79,7 +79,7 @@ func newSMimeWithECDSA(privateKey *ecdsa.PrivateKey, certificate *x509.Certifica return nil, ErrInvalidCertificate } - return &SMime{ + return &SMIME{ privateKey: privateKeyHolder{ecdsa: privateKey}, certificate: certificate, intermediateCertificate: intermediateCertificate, @@ -87,7 +87,7 @@ func newSMimeWithECDSA(privateKey *ecdsa.PrivateKey, certificate *x509.Certifica } // signMessage signs the message with S/MIME -func (sm *SMime) signMessage(message string) (*string, error) { +func (sm *SMIME) signMessage(message string) (*string, error) { lines := parseLines([]byte(message)) toBeSigned := lines.bytesFromLines([]byte("\r\n")) @@ -120,7 +120,7 @@ func (sm *SMime) signMessage(message string) (*string, error) { } // createMessage prepares the message that will be used for the sign method later -func (sm *SMime) prepareMessage(encoding Encoding, contentType ContentType, charset Charset, body []byte) (*string, error) { +func (sm *SMIME) prepareMessage(encoding Encoding, contentType ContentType, charset Charset, body []byte) (*string, error) { encodedMessage, err := sm.encodeMessage(encoding, string(body)) if err != nil { return nil, err @@ -130,7 +130,7 @@ func (sm *SMime) prepareMessage(encoding Encoding, contentType ContentType, char } // encodeMessage encodes the message with the given encoding -func (sm *SMime) encodeMessage(encoding Encoding, message string) (*string, error) { +func (sm *SMIME) encodeMessage(encoding Encoding, message string) (*string, error) { if encoding != EncodingQP { return &message, nil } diff --git a/smime_test.go b/smime_test.go index cb416661..a06125ce 100644 --- a/smime_test.go +++ b/smime_test.go @@ -8,12 +8,21 @@ import ( "bytes" "crypto/ecdsa" "crypto/rsa" + "crypto/tls" + "crypto/x509" "encoding/base64" "fmt" "strings" "testing" ) +const ( + dummyCertRSAPath = "testdata/dummy-chain-cert-rsa.pem" + dummyKeyRSAPath = "testdata/dummy-child-key-rsa.pem" + dummyCertECDSAPath = "testdata/dummy-chain-cert-ecdsa.pem" + dummyKeyECDSAPath = "testdata/dummy-child-key-ecdsa.pem" +) + func TestGet_RSA(t *testing.T) { p := privateKeyHolder{ ecdsa: nil, @@ -43,9 +52,9 @@ func TestNewSMimeWithRSA(t *testing.T) { t.Errorf("Error getting dummy crypto material: %s", err) } - sMime, err := newSMimeWithRSA(privateKey, certificate, intermediateCertificate) + sMime, err := newSMIMEWithRSA(privateKey, certificate, intermediateCertificate) if err != nil { - t.Errorf("Error creating new SMime from keyPair: %s", err) + t.Errorf("Error creating new SMIME from keyPair: %s", err) } if sMime.privateKey.rsa != privateKey { @@ -66,9 +75,9 @@ func TestNewSMimeWithECDSA(t *testing.T) { t.Errorf("Error getting dummy crypto material: %s", err) } - sMime, err := newSMimeWithECDSA(privateKey, certificate, intermediateCertificate) + sMime, err := newSMIMEWithECDSA(privateKey, certificate, intermediateCertificate) if err != nil { - t.Errorf("Error creating new SMime from keyPair: %s", err) + t.Errorf("Error creating new SMIME from keyPair: %s", err) } if sMime.privateKey.ecdsa != privateKey { @@ -89,9 +98,9 @@ func TestSign(t *testing.T) { t.Errorf("Error getting dummy crypto material: %s", err) } - sMime, err := newSMimeWithRSA(privateKey, certificate, intermediateCertificate) + sMime, err := newSMIMEWithRSA(privateKey, certificate, intermediateCertificate) if err != nil { - t.Errorf("Error creating new SMime from keyPair: %s", err) + t.Errorf("Error creating new SMIME from keyPair: %s", err) } message := "This is a test message" @@ -112,9 +121,9 @@ func TestPrepareMessage(t *testing.T) { t.Errorf("Error getting dummy crypto material: %s", err) } - sMime, err := newSMimeWithRSA(privateKey, certificate, intermediateCertificate) + sMime, err := newSMIMEWithRSA(privateKey, certificate, intermediateCertificate) if err != nil { - t.Errorf("Error creating new SMime from keyPair: %s", err) + t.Errorf("Error creating new SMIME from keyPair: %s", err) } encoding := EncodingB64 @@ -147,9 +156,9 @@ func TestPrepareMessage_QuotedPrintable(t *testing.T) { t.Errorf("Error getting dummy crypto material: %s", err) } - sMime, err := newSMimeWithRSA(privateKey, certificate, intermediateCertificate) + sMime, err := newSMIMEWithRSA(privateKey, certificate, intermediateCertificate) if err != nil { - t.Errorf("Error creating new SMime from keyPair: %s", err) + t.Errorf("Error creating new SMIME from keyPair: %s", err) } body := "This is the body with special chars like äöü ÄÖÜ ß!" @@ -186,9 +195,9 @@ func TestEncodeMessage(t *testing.T) { t.Errorf("Error getting dummy crypto material: %s", err) } - sMime, err := newSMimeWithRSA(privateKey, certificate, intermediateCertificate) + sMime, err := newSMIMEWithRSA(privateKey, certificate, intermediateCertificate) if err != nil { - t.Errorf("Error creating new SMime from keyPair: %s", err) + t.Errorf("Error creating new SMIME from keyPair: %s", err) } result, err := sMime.encodeMessage(encoding, body) @@ -212,9 +221,9 @@ func TestEncodeMessage_QuotedPrintable(t *testing.T) { t.Errorf("Error getting dummy crypto material: %s", err) } - sMime, err := newSMimeWithRSA(privateKey, certificate, intermediateCertificate) + sMime, err := newSMIMEWithRSA(privateKey, certificate, intermediateCertificate) if err != nil { - t.Errorf("Error creating new SMime from keyPair: %s", err) + t.Errorf("Error creating new SMIME from keyPair: %s", err) } result, err := sMime.encodeMessage(encoding, body) @@ -332,3 +341,58 @@ func FuzzSplitLine(f *testing.F) { _ = ls.splitLine(sep) }) } + +// getDummyRSACryptoMaterial loads a certificate (RSA), the associated private key and certificate (RSA) is loaded +// from local disk for testing purposes +func getDummyRSACryptoMaterial() (*rsa.PrivateKey, *x509.Certificate, *x509.Certificate, error) { + keyPair, err := tls.LoadX509KeyPair(dummyCertRSAPath, dummyKeyRSAPath) + if err != nil { + return nil, nil, nil, err + } + + privateKey := keyPair.PrivateKey.(*rsa.PrivateKey) + + certificate, err := x509.ParseCertificate(keyPair.Certificate[0]) + if err != nil { + return nil, nil, nil, err + } + + intermediateCertificate, err := x509.ParseCertificate(keyPair.Certificate[1]) + if err != nil { + return nil, nil, nil, err + } + + return privateKey, certificate, intermediateCertificate, nil +} + +// getDummyECDSACryptoMaterial loads a certificate (ECDSA), the associated private key and certificate (ECDSA) is +// loaded from local disk for testing purposes +func getDummyECDSACryptoMaterial() (*ecdsa.PrivateKey, *x509.Certificate, *x509.Certificate, error) { + keyPair, err := tls.LoadX509KeyPair(dummyCertECDSAPath, dummyKeyECDSAPath) + if err != nil { + return nil, nil, nil, err + } + + privateKey := keyPair.PrivateKey.(*ecdsa.PrivateKey) + + certificate, err := x509.ParseCertificate(keyPair.Certificate[0]) + if err != nil { + return nil, nil, nil, err + } + + intermediateCertificate, err := x509.ParseCertificate(keyPair.Certificate[1]) + if err != nil { + return nil, nil, nil, err + } + + return privateKey, certificate, intermediateCertificate, nil +} + +// getDummyKeyPairTLS loads a certificate (ECDSA) as *tls.Certificate, the associated private key and certificate (ECDSA) is loaded from local disk for testing purposes +func getDummyKeyPairTLS() (*tls.Certificate, error) { + keyPair, err := tls.LoadX509KeyPair(dummyCertECDSAPath, dummyKeyECDSAPath) + if err != nil { + return nil, err + } + return &keyPair, err +} From 3745ad10dd4d6e3de768c42d0819cd472b2e2a6e Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 2 Jan 2025 14:01:24 +0100 Subject: [PATCH 09/47] Refactor PKCS7 package and migrate to internal module We don't want to export the PKCS7 code 3rd parties, therefore moved the PKCS7 implementation to an internal module for better encapsulation and renamed functions to use exportable naming conventions. Updated all references to reflect these changes across the codebase, ensuring consistent usability and improved clarity. --- pkcs7.go => internal/pkcs7/pkcs7.go | 22 +++++++++---------- pkcs7_test.go => internal/pkcs7/pkcs7_test.go | 10 ++++----- smime.go | 12 +++++----- 3 files changed, 23 insertions(+), 21 deletions(-) rename pkcs7.go => internal/pkcs7/pkcs7.go (96%) rename pkcs7_test.go => internal/pkcs7/pkcs7_test.go (95%) diff --git a/pkcs7.go b/internal/pkcs7/pkcs7.go similarity index 96% rename from pkcs7.go rename to internal/pkcs7/pkcs7.go index 4180e58e..4c48d5d8 100644 --- a/pkcs7.go +++ b/internal/pkcs7/pkcs7.go @@ -12,7 +12,7 @@ // // SPDX-License-Identifier: MIT -package mail +package pkcs7 import ( "bytes" @@ -227,8 +227,8 @@ type SignedData struct { digestOid asn1.ObjectIdentifier } -// newSignedData initializes a SignedData with content -func newSignedData(data []byte) (*SignedData, error) { +// NewSignedData initializes a SignedData with content +func NewSignedData(data []byte) (*SignedData, error) { content, err := asn1.Marshal(data) if err != nil { return nil, err @@ -244,8 +244,8 @@ func newSignedData(data []byte) (*SignedData, error) { return &SignedData{sd: sd, data: data, digestOid: OIDDigestAlgorithmSHA256}, nil } -// addSigner is a wrapper around AddSignerChain() that adds a signer without any parent. -func (sd *SignedData) addSigner(cert *x509.Certificate, pkey crypto.PrivateKey, config SignerInfoConfig) error { +// AddSigner is a wrapper around AddSignerChain() that adds a signer without any parent. +func (sd *SignedData) AddSigner(cert *x509.Certificate, pkey crypto.PrivateKey, config SignerInfoConfig) error { var parents []*x509.Certificate return sd.addSignerChain(cert, pkey, parents, config) } @@ -327,19 +327,19 @@ func (sd *SignedData) addSignerChain(ee *x509.Certificate, pkey crypto.PrivateKe return nil } -// addCertificate adds the certificate to the payload. Useful for parent certificates -func (sd *SignedData) addCertificate(cert *x509.Certificate) { +// AddCertificate adds the certificate to the payload. Useful for parent certificates +func (sd *SignedData) AddCertificate(cert *x509.Certificate) { sd.certs = append(sd.certs, cert) } -// detach removes content from the signed data struct to make it a detached signature. +// Detach removes content from the signed data struct to make it a detached signature. // This must be called right before Finish() -func (sd *SignedData) detach() { +func (sd *SignedData) Detach() { sd.sd.ContentInfo = contentInfo{ContentType: OIDData} } -// finish marshals the content and its signers -func (sd *SignedData) finish() ([]byte, error) { +// Finish marshals the content and its signers +func (sd *SignedData) Finish() ([]byte, error) { sd.sd.Certificates = marshalCertificates(sd.certs) inner, err := asn1.Marshal(sd.sd) if err != nil { diff --git a/pkcs7_test.go b/internal/pkcs7/pkcs7_test.go similarity index 95% rename from pkcs7_test.go rename to internal/pkcs7/pkcs7_test.go index 564c4fe4..42ed3070 100644 --- a/pkcs7_test.go +++ b/internal/pkcs7/pkcs7_test.go @@ -12,7 +12,7 @@ // // SPDX-License-Identifier: MIT -package mail +package pkcs7 import ( "bytes" @@ -35,18 +35,18 @@ func TestSign_E2E(t *testing.T) { } content := []byte("Hello World") for _, testDetach := range []bool{false, true} { - toBeSigned, serr := newSignedData(content) + toBeSigned, serr := NewSignedData(content) if serr != nil { t.Fatalf("Cannot initialize signed data: %s", err) } - if serr = toBeSigned.addSigner(cert.Certificate, cert.PrivateKey, SignerInfoConfig{}); serr != nil { + if serr = toBeSigned.AddSigner(cert.Certificate, cert.PrivateKey, SignerInfoConfig{}); serr != nil { t.Fatalf("Cannot add signer: %s", err) } if testDetach { - toBeSigned.detach() + toBeSigned.Detach() } else { } - signed, serr := toBeSigned.finish() + signed, serr := toBeSigned.Finish() if serr != nil { t.Fatalf("Cannot finish signing data: %s", err) } diff --git a/smime.go b/smime.go index fa8ae098..4a8ae943 100644 --- a/smime.go +++ b/smime.go @@ -15,6 +15,8 @@ import ( "fmt" "mime/quotedprintable" "strings" + + "github.com/wneessen/go-mail/internal/pkcs7" ) var ( @@ -91,22 +93,22 @@ func (sm *SMIME) signMessage(message string) (*string, error) { lines := parseLines([]byte(message)) toBeSigned := lines.bytesFromLines([]byte("\r\n")) - signedData, err := newSignedData(toBeSigned) + signedData, err := pkcs7.NewSignedData(toBeSigned) if err != nil || signedData == nil { return nil, fmt.Errorf("could not initialize signed data: %w", err) } - if err = signedData.addSigner(sm.certificate, sm.privateKey.get(), SignerInfoConfig{}); err != nil { + if err = signedData.AddSigner(sm.certificate, sm.privateKey.get(), pkcs7.SignerInfoConfig{}); err != nil { return nil, fmt.Errorf("could not add signer message: %w", err) } if sm.intermediateCertificate != nil { - signedData.addCertificate(sm.intermediateCertificate) + signedData.AddCertificate(sm.intermediateCertificate) } - signedData.detach() + signedData.Detach() - signatureDER, err := signedData.finish() + signatureDER, err := signedData.Finish() if err != nil { return nil, fmt.Errorf("could not finish signing: %w", err) } From b5bae7b3a99d44819dd571425d2c4657b9ba56a4 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 2 Jan 2025 14:08:31 +0100 Subject: [PATCH 10/47] "Remove unnecessary else block in pkcs7_test.go" The redundant else block was removed to simplify the code. This change improves readability and adheres to cleaner coding practices. No functional behavior has been altered. --- internal/pkcs7/pkcs7_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/pkcs7/pkcs7_test.go b/internal/pkcs7/pkcs7_test.go index 42ed3070..b8ac9bf7 100644 --- a/internal/pkcs7/pkcs7_test.go +++ b/internal/pkcs7/pkcs7_test.go @@ -44,7 +44,6 @@ func TestSign_E2E(t *testing.T) { } if testDetach { toBeSigned.Detach() - } else { } signed, serr := toBeSigned.Finish() if serr != nil { From aedf065c1d07f65e58fb5ad5f9e6c2d67c75e136 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Thu, 2 Jan 2025 14:08:45 +0100 Subject: [PATCH 11/47] Fix S/MIME signing method naming inconsistencies Renamed methods and related references from "WithSMimeSinging" to "WithSMIMESigning" for correctness and consistency. Updated associated test cases, comments, and documentation to reflect the renamed methods accurately. --- msg.go | 2 +- msg_test.go | 16 ++++++++-------- msgwriter.go | 2 +- part.go | 27 +++++++++++++++++++++------ part_test.go | 18 +++++++++--------- 5 files changed, 40 insertions(+), 25 deletions(-) diff --git a/msg.go b/msg.go index 6d3cbf55..30c1fe21 100644 --- a/msg.go +++ b/msg.go @@ -2293,7 +2293,7 @@ func (m *Msg) createSignaturePart(encoding Encoding, contentType ContentType, ch return nil, err } - signaturePart := m.newPart(TypeSMIMESigned, WithPartEncoding(EncodingB64), WithSMimeSinging()) + signaturePart := m.newPart(TypeSMIMESigned, WithPartEncoding(EncodingB64), WithSMIMESigning()) signaturePart.SetContent(*signedMessage) return signaturePart, nil diff --git a/msg_test.go b/msg_test.go index 08e41e17..54291861 100644 --- a/msg_test.go +++ b/msg_test.go @@ -6667,7 +6667,7 @@ func TestMsg_fileFromIOFS(t *testing.T) { }) } -// TestSignWithSMime_ValidRSAKeyPair tests WithSMimeSinging with given rsa key pair +// TestSignWithSMime_ValidRSAKeyPair tests WithSMIMESigning with given rsa key pair func TestSignWithSMime_ValidRSAKeyPair(t *testing.T) { privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() if err != nil { @@ -6678,14 +6678,14 @@ func TestSignWithSMime_ValidRSAKeyPair(t *testing.T) { t.Errorf("failed to set sMIME. Cause: %v", err) } if m.sMIME.privateKey.rsa == nil { - t.Errorf("WithSMimeSinging() - no private key is given") + t.Errorf("WithSMIMESigning() - no private key is given") } if m.sMIME.certificate == nil { - t.Errorf("WithSMimeSinging() - no certificate is given") + t.Errorf("WithSMIMESigning() - no certificate is given") } } -// TestSignWithSMime_ValidRSAKeyPair tests WithSMimeSinging with given ecdsa key pair +// TestSignWithSMime_ValidRSAKeyPair tests WithSMIMESigning with given ecdsa key pair func TestSignWithSMime_ValidECDSAKeyPair(t *testing.T) { privateKey, certificate, intermediateCertificate, err := getDummyECDSACryptoMaterial() if err != nil { @@ -6696,10 +6696,10 @@ func TestSignWithSMime_ValidECDSAKeyPair(t *testing.T) { t.Errorf("failed to set sMIME. Cause: %v", err) } if m.sMIME.privateKey.ecdsa == nil { - t.Errorf("WithSMimeSinging() - no private key is given") + t.Errorf("WithSMIMESigning() - no private key is given") } if m.sMIME.certificate == nil { - t.Errorf("WithSMimeSinging() - no certificate is given") + t.Errorf("WithSMIMESigning() - no certificate is given") } } @@ -6744,7 +6744,7 @@ func TestSignWithTLSCertificate_WithKeyPairLeafNil(t *testing.T) { } } -// TestSignWithSMime_InvalidPrivateKey tests WithSMimeSinging with given invalid private key +// TestSignWithSMime_InvalidPrivateKey tests WithSMIMESigning with given invalid private key func TestSignWithSMime_InvalidPrivateKey(t *testing.T) { m := NewMsg() @@ -6754,7 +6754,7 @@ func TestSignWithSMime_InvalidPrivateKey(t *testing.T) { } } -// TestSignWithSMime_InvalidCertificate tests WithSMimeSinging with given invalid certificate +// TestSignWithSMime_InvalidCertificate tests WithSMIMESigning with given invalid certificate func TestSignWithSMime_InvalidCertificate(t *testing.T) { privateKey, _, _, err := getDummyRSACryptoMaterial() if err != nil { diff --git a/msgwriter.go b/msgwriter.go index 337c5475..03df20ae 100644 --- a/msgwriter.go +++ b/msgwriter.go @@ -357,7 +357,7 @@ func (mw *msgWriter) writePart(part *Part, charset Charset) { } contentType := part.contentType.String() - if !part.IsSMimeSigned() { + if !part.IsSMIMESigned() { contentType = strings.Join([]string{contentType, "; charset=", partCharset.String()}, "") } diff --git a/part.go b/part.go index 3fab6527..e675f954 100644 --- a/part.go +++ b/part.go @@ -95,8 +95,13 @@ func (p *Part) GetDescription() string { return p.description } -// IsSMimeSigned returns true if the Part should be singed with S/MIME -func (p *Part) IsSMimeSigned() bool { +// IsSMIMESigned determines if the Part should be signed with S/MIME. +// +// This function checks whether the Part has been configured to use S/MIME for signing. +// +// Returns: +// - true if the Part should be signed with S/MIME; otherwise false. +func (p *Part) IsSMIMESigned() bool { return p.smime } @@ -152,8 +157,13 @@ func (p *Part) SetDescription(description string) { p.description = description } -// SetIsSMimeSigned sets the flag for signing the Part with S/MIME -func (p *Part) SetIsSMimeSigned(smime bool) { +// SetIsSMIMESigned sets the flag for signing the Part with S/MIME. +// +// This function updates the S/MIME signing flag for the Part. +// +// Parameters: +// - smime: A boolean indicating whether the Part should be signed with S/MIME. +func (p *Part) SetIsSMIMESigned(smime bool) { p.smime = smime } @@ -225,8 +235,13 @@ func WithPartContentDescription(description string) PartOption { } } -// WithSMimeSinging overrides the flag for signing the Part with S/MIME -func WithSMimeSinging() PartOption { +// WithSMIMESigning enables the S/MIME signing flag for a Part. +// +// This function provides a PartOption that overrides the S/MIME signing flag to enable signing. +// +// Returns: +// - A PartOption that sets the S/MIME signing flag to true. +func WithSMIMESigning() PartOption { return func(p *Part) { p.smime = true } diff --git a/part_test.go b/part_test.go index c1a8ac7e..5f8f3a5b 100644 --- a/part_test.go +++ b/part_test.go @@ -102,21 +102,21 @@ func TestPart_WithPartContentDescription(t *testing.T) { } } -// TestPart_WithSMimeSinging tests the WithSMimeSinging method +// TestPart_WithSMimeSinging tests the WithSMIMESigning method func TestPart_WithSMimeSinging(t *testing.T) { m := NewMsg() - part := m.newPart(TypeTextPlain, WithSMimeSinging()) + part := m.newPart(TypeTextPlain, WithSMIMESigning()) if part == nil { - t.Errorf("newPart() WithSMimeSinging() failed: no part returned") + t.Errorf("newPart() WithSMIMESigning() failed: no part returned") return } if part.smime != true { - t.Errorf("newPart() WithSMimeSinging() failed: expected: %v, got: %v", true, part.smime) + t.Errorf("newPart() WithSMIMESigning() failed: expected: %v, got: %v", true, part.smime) } part.smime = true - part.SetIsSMimeSigned(false) + part.SetIsSMIMESigned(false) if part.smime != false { - t.Errorf("newPart() SetIsSMimeSigned() failed: expected: %v, got: %v", false, part.smime) + t.Errorf("newPart() SetIsSMIMESigned() failed: expected: %v, got: %v", false, part.smime) } } @@ -263,7 +263,7 @@ func TestPart_GetContentBroken(t *testing.T) { } } -// TestPart_IsSMimeSigned tests Part.IsSMimeSigned +// TestPart_IsSMimeSigned tests Part.IsSMIMESigned func TestPart_IsSMimeSigned(t *testing.T) { tests := []struct { name string @@ -281,8 +281,8 @@ func TestPart_IsSMimeSigned(t *testing.T) { t.Errorf("failed: %s", err) return } - pl[0].SetIsSMimeSigned(tt.want) - smime := pl[0].IsSMimeSigned() + pl[0].SetIsSMIMESigned(tt.want) + smime := pl[0].IsSMIMESigned() if smime != tt.want { t.Errorf("SetContentType failed. Got: %v, expected: %v", smime, tt.want) } From 376fee62832f36468c517116c7e296f1fe3b9901 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 4 Jan 2025 12:41:23 +0100 Subject: [PATCH 12/47] Refactor S/MIME signing logic and improve encoding handling. Renamed parameter for clarity and adjusted logic to set `NoEncoding` when S/MIME signing is enabled. Consolidated nested if-statements for the encoding conditions by using the previously used `switch` statement, improving readability and maintainability. --- msgwriter.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/msgwriter.go b/msgwriter.go index 03df20ae..13539448 100644 --- a/msgwriter.go +++ b/msgwriter.go @@ -452,7 +452,7 @@ func (mw *msgWriter) writeHeader(key Header, values ...string) { // - writeFunc: A function that writes the body content to the given io.Writer. // - encoding: The encoding type to use when writing the content (e.g., base64, quoted-printable). // - singingWithSMime: Whether the msg should be signed with S/MIME or not. -func (mw *msgWriter) writeBody(writeFunc func(io.Writer) (int64, error), encoding Encoding, singingWithSMime bool) { +func (mw *msgWriter) writeBody(writeFunc func(io.Writer) (int64, error), encoding Encoding, SMIMEsigned bool) { var writer io.Writer var encodedWriter io.WriteCloser var n int64 @@ -467,11 +467,16 @@ func (mw *msgWriter) writeBody(writeFunc func(io.Writer) (int64, error), encodin lineBreaker := Base64LineBreaker{} lineBreaker.out = &writeBuffer - if encoding == EncodingQP { + if SMIMEsigned { + encoding = NoEncoding + } + + switch encoding { + case EncodingQP: encodedWriter = quotedprintable.NewWriter(&writeBuffer) - } else if encoding == EncodingB64 && !singingWithSMime { + case EncodingB64: encodedWriter = base64.NewEncoder(base64.StdEncoding, &lineBreaker) - } else if encoding == NoEncoding || singingWithSMime { + case NoEncoding: _, err = writeFunc(&writeBuffer) if err != nil { mw.err = fmt.Errorf("bodyWriter function: %w", err) @@ -484,7 +489,7 @@ func (mw *msgWriter) writeBody(writeFunc func(io.Writer) (int64, error), encodin mw.bytesWritten += n } return - } else { + default: encodedWriter = quotedprintable.NewWriter(writer) } From a44e7db12e5b61de816e65f742faf9d536ced73d Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 4 Jan 2025 12:43:23 +0100 Subject: [PATCH 13/47] Rename SMIMEsigned to signSMIME for clarity Updated the function parameter name and associated comments to improve readability and consistency. This change ensures the naming aligns with the parameter's purpose and standard conventions. --- msgwriter.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/msgwriter.go b/msgwriter.go index 13539448..be8dc2a6 100644 --- a/msgwriter.go +++ b/msgwriter.go @@ -451,8 +451,8 @@ func (mw *msgWriter) writeHeader(key Header, values ...string) { // Parameters: // - writeFunc: A function that writes the body content to the given io.Writer. // - encoding: The encoding type to use when writing the content (e.g., base64, quoted-printable). -// - singingWithSMime: Whether the msg should be signed with S/MIME or not. -func (mw *msgWriter) writeBody(writeFunc func(io.Writer) (int64, error), encoding Encoding, SMIMEsigned bool) { +// - signSMIME: Whether the msg should be signed with S/MIME. +func (mw *msgWriter) writeBody(writeFunc func(io.Writer) (int64, error), encoding Encoding, signSMIME bool) { var writer io.Writer var encodedWriter io.WriteCloser var n int64 @@ -467,7 +467,8 @@ func (mw *msgWriter) writeBody(writeFunc func(io.Writer) (int64, error), encodin lineBreaker := Base64LineBreaker{} lineBreaker.out = &writeBuffer - if SMIMEsigned { + // the S/MIME part is already Base64 encoded, we don't want to double-encode + if signSMIME { encoding = NoEncoding } From fb546979fa5a5026e9932b630988054a3f57bbaa Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 4 Jan 2025 12:54:29 +0100 Subject: [PATCH 14/47] Refactor content type generation logic in msgwriter Simplified the logic for building the content type string using `fmt.Sprintf`, enhancing readability and maintainability. Adjusted the condition to handle S/MIME-signed cases as the exception instead of the default. --- msgwriter.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/msgwriter.go b/msgwriter.go index be8dc2a6..60dd5e43 100644 --- a/msgwriter.go +++ b/msgwriter.go @@ -356,9 +356,9 @@ func (mw *msgWriter) writePart(part *Part, charset Charset) { partCharset = charset } - contentType := part.contentType.String() - if !part.IsSMIMESigned() { - contentType = strings.Join([]string{contentType, "; charset=", partCharset.String()}, "") + contentType := fmt.Sprintf("%s; charset=%s", part.contentType, partCharset) + if part.IsSMIMESigned() { + contentType = part.contentType.String() } contentTransferEnc := part.encoding.String() From 893595df64d7d9e5785e14f3cd50ec1f42c14f65 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 4 Jan 2025 13:06:50 +0100 Subject: [PATCH 15/47] Refactor S/MIME methods and rename related functions and types. This commit consolidates and refactors S/MIME-related methods in the `Msg` struct, improving clarity and consistency. Function and type names are updated for proper naming conventions, such as renaming `hasSMime` to `hasSMIME` and `MIMESMIME` to `MIMESMIMESigned`. Redundant code is reorganized or updated for better maintainability. --- encoding.go | 4 +- encoding_test.go | 2 +- msg.go | 279 +++++++++++++++++++++++++++++------------------ msg_test.go | 6 +- msgwriter.go | 6 +- 5 files changed, 181 insertions(+), 116 deletions(-) diff --git a/encoding.go b/encoding.go index 7e15c4fe..5371d56b 100644 --- a/encoding.go +++ b/encoding.go @@ -186,8 +186,8 @@ const ( // MIMERelated MIMEType represents a MIME multipart/related type, used for emails with related content entities. MIMERelated MIMEType = "related" - // MIMESMIME MIMEType represents a MIME multipart/signed type, used for siging emails with S/MIME. - MIMESMIME MIMEType = `signed; protocol="application/pkcs7-signature"; micalg=sha-256` + // MIMESMIMESigned MIMEType represents a MIME multipart/signed type, used for siging emails with S/MIME. + MIMESMIMESigned MIMEType = `signed; protocol="application/pkcs7-signature"; micalg=sha-256` ) // String satisfies the fmt.Stringer interface for the Charset type. diff --git a/encoding_test.go b/encoding_test.go index 8a909c5c..18e12da6 100644 --- a/encoding_test.go +++ b/encoding_test.go @@ -136,7 +136,7 @@ func TestMimeType_String(t *testing.T) { {MIMEAlternative, "alternative"}, {MIMEMixed, "mixed"}, {MIMERelated, "related"}, - {MIMESMIME, `signed; protocol="application/pkcs7-signature"; micalg=sha-256`}, + {MIMESMIMESigned, `signed; protocol="application/pkcs7-signature"; micalg=sha-256`}, } for _, tt := range tests { t.Run(tt.mt.String(), func(t *testing.T) { diff --git a/msg.go b/msg.go index 30c1fe21..f8ac28df 100644 --- a/msg.go +++ b/msg.go @@ -341,72 +341,6 @@ func WithNoDefaultUserAgent() MsgOption { } } -// SignWithSMIMERSA configures the Msg to be signed with S/MIME -func (m *Msg) SignWithSMIMERSA(privateKey *rsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) error { - smime, err := newSMIMEWithRSA(privateKey, certificate, intermediateCertificate) - if err != nil { - return err - } - m.sMIME = smime - return nil -} - -// SignWithSMIMEECDSA configures the Msg to be signed with S/MIME -func (m *Msg) SignWithSMIMEECDSA(privateKey *ecdsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) error { - smime, err := newSMIMEWithECDSA(privateKey, certificate, intermediateCertificate) - if err != nil { - return err - } - m.sMIME = smime - return nil -} - -// SignWithTLSCertificate signs the Msg with the provided *tls.certificate. -func (m *Msg) SignWithTLSCertificate(keyPairTlS *tls.Certificate) error { - intermediateCertificate, err := x509.ParseCertificate(keyPairTlS.Certificate[1]) - if err != nil { - return fmt.Errorf("failed to parse intermediate certificate: %w", err) - } - - leafCertificate, err := getLeafCertificate(keyPairTlS) - if err != nil { - return fmt.Errorf("failed to get leaf certificate: %w", err) - } - - switch keyPairTlS.PrivateKey.(type) { - case *rsa.PrivateKey: - if intermediateCertificate == nil { - return m.SignWithSMIMERSA(keyPairTlS.PrivateKey.(*rsa.PrivateKey), leafCertificate, nil) - } - return m.SignWithSMIMERSA(keyPairTlS.PrivateKey.(*rsa.PrivateKey), leafCertificate, intermediateCertificate) - - case *ecdsa.PrivateKey: - if intermediateCertificate == nil { - return m.SignWithSMIMEECDSA(keyPairTlS.PrivateKey.(*ecdsa.PrivateKey), leafCertificate, nil) - } - return m.SignWithSMIMEECDSA(keyPairTlS.PrivateKey.(*ecdsa.PrivateKey), leafCertificate, intermediateCertificate) - default: - return fmt.Errorf("unsupported private key type: %T", keyPairTlS.PrivateKey) - } -} - -// getLeafCertificate returns the leaf certificate from a tls.Certificate. -// PLEASE NOTE: Before Go 1.23 Certificate.Leaf was left nil, and the parsed certificate was -// discarded. This behavior can be re-enabled by setting "x509keypairleaf=0" -// in the GODEBUG environment variable. -func getLeafCertificate(keyPairTlS *tls.Certificate) (*x509.Certificate, error) { - if keyPairTlS.Leaf != nil { - return keyPairTlS.Leaf, nil - } - - cert, err := x509.ParseCertificate(keyPairTlS.Certificate[0]) - if err != nil { - return nil, err - } - - return cert, nil -} - // SetCharset sets or overrides the currently set encoding charset of the Msg. // // This method allows you to specify a character set for the email message. The charset is @@ -2264,41 +2198,6 @@ func (m *Msg) applyMiddlewares(msg *Msg) *Msg { return msg } -// signMessage sign the Msg with S/MIME -func (m *Msg) signMessage(msg *Msg) (*Msg, error) { - signedPart := msg.GetParts()[0] - body, err := signedPart.GetContent() - if err != nil { - return nil, err - } - - signaturePart, err := m.createSignaturePart(signedPart.GetEncoding(), signedPart.GetContentType(), signedPart.GetCharset(), body) - if err != nil { - return nil, err - } - - m.parts = append(m.parts, signaturePart) - - return m, err -} - -// createSignaturePart creates an additional part that be used for storing the S/MIME signature of the given body -func (m *Msg) createSignaturePart(encoding Encoding, contentType ContentType, charSet Charset, body []byte) (*Part, error) { - message, err := m.sMIME.prepareMessage(encoding, contentType, charSet, body) - if err != nil { - return nil, err - } - signedMessage, err := m.sMIME.signMessage(*message) - if err != nil { - return nil, err - } - - signaturePart := m.newPart(TypeSMIMESigned, WithPartEncoding(EncodingB64), WithSMIMESigning()) - signaturePart.SetContent(*signedMessage) - - return signaturePart, nil -} - // WriteTo writes the formatted Msg into the given io.Writer and satisfies the io.WriterTo interface. // // This method writes the email message, including its headers, body, and attachments, to the provided @@ -2318,7 +2217,7 @@ func (m *Msg) WriteTo(writer io.Writer) (int64, error) { mw := &msgWriter{writer: writer, charset: m.charset, encoder: m.encoder} msg := m.applyMiddlewares(m) - if m.hasSMime() { + if m.hasSMIME() { signedMsg, err := m.signMessage(msg) if err != nil { return 0, err @@ -2621,6 +2520,86 @@ func (m *Msg) addAddr(header AddrHeader, addr string) error { return m.SetAddrHeader(header, addresses...) } +// SignWithSMIMERSA configures the Msg to be signed with S/MIME using RSA. +// +// This function sets up S/MIME signing for the Msg by associating it with the provided private key, +// certificate, and intermediate certificate. +// +// Parameters: +// - privateKey: The RSA private key used for signing. +// - certificate: The x509 certificate associated with the private key. +// - intermediateCertificate: An optional intermediate x509 certificate for chain validation. +// +// Returns: +// - An error if any issue occurred while configuring S/MIME signing; otherwise nil. +func (m *Msg) SignWithSMIMERSA(privateKey *rsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) error { + smime, err := newSMIMEWithRSA(privateKey, certificate, intermediateCertificate) + if err != nil { + return err + } + m.sMIME = smime + return nil +} + +// SignWithSMIMEECDSA configures the Msg to be signed with S/MIME using ECDSA. +// +// This function sets up S/MIME signing for the Msg by associating it with the provided ECDSA private key, +// certificate, and intermediate certificate. +// +// Parameters: +// - privateKey: The ECDSA private key used for signing. +// - certificate: The x509 certificate associated with the private key. +// - intermediateCertificate: An optional intermediate x509 certificate for chain validation. +// +// Returns: +// - An error if any issue occurred while configuring S/MIME signing; otherwise nil. +func (m *Msg) SignWithSMIMEECDSA(privateKey *ecdsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) error { + smime, err := newSMIMEWithECDSA(privateKey, certificate, intermediateCertificate) + if err != nil { + return err + } + m.sMIME = smime + return nil +} + +// SignWithTLSCertificate signs the Msg with the provided *tls.Certificate. +// +// This function configures the Msg for S/MIME signing using the private key and certificates +// from the provided TLS certificate. It supports both RSA and ECDSA private keys. +// +// Parameters: +// - keyPairTlS: The *tls.Certificate containing the private key and associated certificate chain. +// +// Returns: +// - An error if any issue occurred during parsing, signing configuration, or unsupported private key type. +func (m *Msg) SignWithTLSCertificate(keyPairTlS *tls.Certificate) error { + intermediateCertificate, err := x509.ParseCertificate(keyPairTlS.Certificate[1]) + if err != nil { + return fmt.Errorf("failed to parse intermediate certificate: %w", err) + } + + leafCertificate, err := getLeafCertificate(keyPairTlS) + if err != nil { + return fmt.Errorf("failed to get leaf certificate: %w", err) + } + + switch keyPairTlS.PrivateKey.(type) { + case *rsa.PrivateKey: + if intermediateCertificate == nil { + return m.SignWithSMIMERSA(keyPairTlS.PrivateKey.(*rsa.PrivateKey), leafCertificate, nil) + } + return m.SignWithSMIMERSA(keyPairTlS.PrivateKey.(*rsa.PrivateKey), leafCertificate, intermediateCertificate) + + case *ecdsa.PrivateKey: + if intermediateCertificate == nil { + return m.SignWithSMIMEECDSA(keyPairTlS.PrivateKey.(*ecdsa.PrivateKey), leafCertificate, nil) + } + return m.SignWithSMIMEECDSA(keyPairTlS.PrivateKey.(*ecdsa.PrivateKey), leafCertificate, intermediateCertificate) + default: + return fmt.Errorf("unsupported private key type: %T", keyPairTlS.PrivateKey) + } +} + // appendFile adds a File to the Msg, either as an attachment or an embed. // // This method appends a File to the list of files (attachments or embeds) for the message. It applies @@ -2691,7 +2670,7 @@ func (m *Msg) hasAlt() bool { count++ } } - return count > 1 && m.pgptype == 0 && !m.hasSMime() + return count > 1 && m.pgptype == 0 && !m.hasSMIME() } // hasMixed returns true if the Msg has mixed parts. @@ -2709,13 +2688,13 @@ func (m *Msg) hasMixed() bool { return m.pgptype == 0 && ((len(m.parts) > 0 && len(m.attachments) > 0) || len(m.attachments) > 1) } -// hasSMime determines if the Msg should be signed with S/MIME. +// hasSMIME determines if the Msg should be signed with S/MIME. // -// This function checks whether the Msg has S/MIME type available. +// This function checks whether the Msg has SMIME type assigned. // // Returns: -// - true if the Msg has S/MIME type assigned and should be signed with S/MIME; otherwise false. -func (m *Msg) hasSMime() bool { +// - true if the Msg has SMIME type assigned and should be signed with S/MIME; otherwise false. +func (m *Msg) hasSMIME() bool { return m.sMIME != nil } @@ -2814,6 +2793,36 @@ func (m *Msg) checkUserAgent() { } } +// createSignaturePart creates a Part for storing the S/MIME signature of the given body. +// +// This function generates an S/MIME signature for the provided message body and wraps it in a Part +// object that can be included in the email message. +// +// Parameters: +// - encoding: The encoding format to use for the signature. +// - contentType: The content type of the message body being signed. +// - charSet: The character set used in the message body. +// - body: The byte array representing the message body to be signed. +// +// Returns: +// - A Part containing the S/MIME signature. +// - An error if the message preparation or signing process fails. +func (m *Msg) createSignaturePart(encoding Encoding, contentType ContentType, charSet Charset, body []byte) (*Part, error) { + message, err := m.sMIME.prepareMessage(encoding, contentType, charSet, body) + if err != nil { + return nil, err + } + signedMessage, err := m.sMIME.signMessage(*message) + if err != nil { + return nil, err + } + + signaturePart := m.newPart(TypeSMIMESigned, WithPartEncoding(EncodingB64), WithSMIMESigning()) + signaturePart.SetContent(*signedMessage) + + return signaturePart, nil +} + // addDefaultHeader sets default headers if they haven't been set before. // // This method ensures that essential headers such as "Date", "Message-ID", and "MIME-Version" are set @@ -3062,6 +3071,62 @@ func getEncoder(enc Encoding) mime.WordEncoder { } } +// getLeafCertificate retrieves the leaf certificate from a tls.Certificate. +// +// This function returns the parsed leaf certificate from the provided TLS certificate. If the Leaf field +// is nil, it parses and returns the first certificate in the chain. +// +// PLEASE NOTE: In Go versions prior to 1.23, the Certificate.Leaf field was left nil, and the parsed +// certificate was discarded. This behavior can be re-enabled by setting "x509keypairleaf=0" in the +// GODEBUG environment variable. +// +// Parameters: +// - keyPairTlS: The *tls.Certificate containing the certificate chain. +// +// Returns: +// - The parsed leaf x509 certificate. +// - An error if the certificate could not be parsed. +func getLeafCertificate(keyPairTlS *tls.Certificate) (*x509.Certificate, error) { + if keyPairTlS.Leaf != nil { + return keyPairTlS.Leaf, nil + } + + cert, err := x509.ParseCertificate(keyPairTlS.Certificate[0]) + if err != nil { + return nil, err + } + + return cert, nil +} + +// signMessage signs the Msg with S/MIME. +// +// This function creates an S/MIME signature for the first part of the message and appends the +// signature as an additional part to the Msg. +// +// Parameters: +// - msg: The Msg to be signed. +// +// Returns: +// - The Msg with the appended S/MIME signature part. +// - An error if retrieving content, creating the signature part, or signing fails. +func (m *Msg) signMessage(msg *Msg) (*Msg, error) { + signedPart := msg.GetParts()[0] + body, err := signedPart.GetContent() + if err != nil { + return nil, err + } + + signaturePart, err := m.createSignaturePart(signedPart.GetEncoding(), signedPart.GetContentType(), signedPart.GetCharset(), body) + if err != nil { + return nil, err + } + + m.parts = append(m.parts, signaturePart) + + return m, err +} + // writeFuncFromBuffer converts a byte buffer into a writeFunc, which is commonly required by go-mail. // // This function wraps a byte buffer into a write function that can be used to write the buffer's content diff --git a/msg_test.go b/msg_test.go index 54291861..9d3ac1bf 100644 --- a/msg_test.go +++ b/msg_test.go @@ -6415,7 +6415,7 @@ func TestMsg_hasAltWithSMime(t *testing.T) { } } -// TestMsg_hasSMime tests the hasSMime() method of the Msg +// TestMsg_hasSMime tests the hasSMIME() method of the Msg func TestMsg_hasSMime(t *testing.T) { privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() if err != nil { @@ -6426,8 +6426,8 @@ func TestMsg_hasSMime(t *testing.T) { t.Errorf("failed to init smime configuration") } m.SetBodyString(TypeTextPlain, "Plain") - if !m.hasSMime() { - t.Errorf("mail has smime configured but hasSMime() returned true") + if !m.hasSMIME() { + t.Errorf("mail has smime configured but hasSMIME() returned true") } } diff --git a/msgwriter.go b/msgwriter.go index 60dd5e43..4de4067a 100644 --- a/msgwriter.go +++ b/msgwriter.go @@ -128,8 +128,8 @@ func (mw *msgWriter) writeMsg(msg *Msg) { } } - if msg.hasSMime() { - mw.startMP(MIMESMIME, msg.boundary) + if msg.hasSMIME() { + mw.startMP(MIMESMIMESigned, msg.boundary) mw.writeString(DoubleNewLine) } if msg.hasMixed() { @@ -179,7 +179,7 @@ func (mw *msgWriter) writeMsg(msg *Msg) { mw.stopMP() } - if msg.hasSMime() { + if msg.hasSMIME() { mw.stopMP() } } From f2ce3bef684b6f6a828337fa11f53042d52badca Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 4 Jan 2025 13:15:10 +0100 Subject: [PATCH 16/47] Fix and enhance error handling in message signing tests Updated error messages in test cases to provide clearer diagnostics and replaced `t.Errorf` with `t.Fatalf` where test execution must stop. Also, added a nil check for signature parts to prevent potential nil pointer dereferences. --- msg_test.go | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/msg_test.go b/msg_test.go index 9d3ac1bf..c38a3851 100644 --- a/msg_test.go +++ b/msg_test.go @@ -6775,26 +6775,36 @@ func TestMsg_createSignaturePart(t *testing.T) { t.Errorf("failed to laod dummy crypto material. Cause: %v", err) } m := NewMsg() - if err := m.SignWithSMIMERSA(privateKey, certificate, intermediateCertificate); err != nil { + if err = m.SignWithSMIMERSA(privateKey, certificate, intermediateCertificate); err != nil { t.Errorf("failed to init smime configuration") } body := []byte("This is the body") part, err := m.createSignaturePart(EncodingQP, TypeTextPlain, CharsetUTF7, body) if err != nil { - t.Errorf("createSignaturePart() method failed.") + t.Fatalf("failed to create part signatures: %s", err) + } + if part == nil { + t.Fatal("failed to create part signatures, returned part is nil") } - if part.GetEncoding() != EncodingB64 { - t.Errorf("createSignaturePart() method failed. Expected encoding: %s, got: %s", EncodingB64, part.GetEncoding()) + t.Errorf("createSignaturePart() failed. Expected encoding: %s, got: %s", EncodingB64, + part.GetEncoding()) } if part.GetContentType() != TypeSMIMESigned { - t.Errorf("createSignaturePart() method failed. Expected content type: %s, got: %s", TypeSMIMESigned, part.GetContentType()) + t.Errorf("createSignaturePart() failed. Expected content type: %s, got: %s", TypeSMIMESigned, + part.GetContentType()) } if part.GetCharset() != CharsetUTF8 { - t.Errorf("createSignaturePart() method failed. Expected charset: %s, got: %s", CharsetUTF8, part.GetCharset()) + t.Errorf("createSignaturePart() failed. Expected charset: %s, got: %s", CharsetUTF8, + part.GetCharset()) + } + content, err := part.GetContent() + if err != nil { + t.Errorf("failed to get content of part: %s", err) } - if content, err := part.GetContent(); err != nil || len(content) == len(body) { - t.Errorf("createSignaturePart() method failed. Expected content should not be equal: %s, got: %s", body, part.GetEncoding()) + if len(content) == len(body) { + t.Errorf("createSignaturePart() failed. Content length should not be equal to body length. Expected %d, "+ + "got: %d", len(body), len(content)) } } @@ -6812,7 +6822,7 @@ func TestMsg_signMessage(t *testing.T) { } msg, err := m.signMessage(m) if err != nil { - t.Errorf("createSignaturePart() method failed.") + t.Fatalf("failed to sign message: %s", err) } parts := msg.GetParts() From 07f6f7ec380755ae393c3632a3bf9b56c5dcd5d5 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 4 Jan 2025 14:54:32 +0100 Subject: [PATCH 17/47] Fix and enhance TLS certificate signing logic in Msg Correct variable naming for keyPairTLS, handle nil input, and improve intermediate certificate parsing. Avoid duplicate signing in signMessage and track signing state with a new flag. These changes enhance clarity, robustness, and prevent redundant operations. --- msg.go | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/msg.go b/msg.go index f8ac28df..976ad35d 100644 --- a/msg.go +++ b/msg.go @@ -2572,31 +2572,33 @@ func (m *Msg) SignWithSMIMEECDSA(privateKey *ecdsa.PrivateKey, certificate *x509 // // Returns: // - An error if any issue occurred during parsing, signing configuration, or unsupported private key type. -func (m *Msg) SignWithTLSCertificate(keyPairTlS *tls.Certificate) error { - intermediateCertificate, err := x509.ParseCertificate(keyPairTlS.Certificate[1]) - if err != nil { - return fmt.Errorf("failed to parse intermediate certificate: %w", err) +func (m *Msg) SignWithTLSCertificate(keyPairTLS *tls.Certificate) error { + if keyPairTLS == nil { + return fmt.Errorf("keyPairTLS cannot be nil") + } + + var intermediateCert *x509.Certificate + var err error + if len(keyPairTLS.Certificate) > 1 { + intermediateCert, err = x509.ParseCertificate(keyPairTLS.Certificate[1]) + if err != nil { + return fmt.Errorf("failed to parse intermediate certificate: %w", err) + } } - leafCertificate, err := getLeafCertificate(keyPairTlS) + leafCertificate, err := getLeafCertificate(keyPairTLS) if err != nil { return fmt.Errorf("failed to get leaf certificate: %w", err) } - switch keyPairTlS.PrivateKey.(type) { + switch keyPairTLS.PrivateKey.(type) { case *rsa.PrivateKey: - if intermediateCertificate == nil { - return m.SignWithSMIMERSA(keyPairTlS.PrivateKey.(*rsa.PrivateKey), leafCertificate, nil) - } - return m.SignWithSMIMERSA(keyPairTlS.PrivateKey.(*rsa.PrivateKey), leafCertificate, intermediateCertificate) + return m.SignWithSMIMERSA(keyPairTLS.PrivateKey.(*rsa.PrivateKey), leafCertificate, intermediateCert) case *ecdsa.PrivateKey: - if intermediateCertificate == nil { - return m.SignWithSMIMEECDSA(keyPairTlS.PrivateKey.(*ecdsa.PrivateKey), leafCertificate, nil) - } - return m.SignWithSMIMEECDSA(keyPairTlS.PrivateKey.(*ecdsa.PrivateKey), leafCertificate, intermediateCertificate) + return m.SignWithSMIMEECDSA(keyPairTLS.PrivateKey.(*ecdsa.PrivateKey), leafCertificate, intermediateCert) default: - return fmt.Errorf("unsupported private key type: %T", keyPairTlS.PrivateKey) + return fmt.Errorf("unsupported private key type: %T", keyPairTLS.PrivateKey) } } @@ -2818,7 +2820,7 @@ func (m *Msg) createSignaturePart(encoding Encoding, contentType ContentType, ch } signaturePart := m.newPart(TypeSMIMESigned, WithPartEncoding(EncodingB64), WithSMIMESigning()) - signaturePart.SetContent(*signedMessage) + signaturePart.SetContent(signedMessage) return signaturePart, nil } @@ -3111,6 +3113,9 @@ func getLeafCertificate(keyPairTlS *tls.Certificate) (*x509.Certificate, error) // - The Msg with the appended S/MIME signature part. // - An error if retrieving content, creating the signature part, or signing fails. func (m *Msg) signMessage(msg *Msg) (*Msg, error) { + if msg.sMIME.isSigned { + return msg, nil + } signedPart := msg.GetParts()[0] body, err := signedPart.GetContent() if err != nil { @@ -3123,6 +3128,7 @@ func (m *Msg) signMessage(msg *Msg) (*Msg, error) { } m.parts = append(m.parts, signaturePart) + m.sMIME.isSigned = true return m, err } From e6e1e7557860161c455796d0772cb301bce55b79 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 4 Jan 2025 14:55:22 +0100 Subject: [PATCH 18/47] Refactor S/MIME signing and encoding logic Simplified methods in S/MIME by removing redundant line parsing and restructuring encoding logic. Changed pointer-based returns to value-based in key methods for clarity and reliability. Cleaned up test cases to align with the updated code structure. --- smime.go | 104 +++++++++++--------------------------------------- smime_test.go | 104 +++----------------------------------------------- 2 files changed, 29 insertions(+), 179 deletions(-) diff --git a/smime.go b/smime.go index 4a8ae943..42ee4ef0 100644 --- a/smime.go +++ b/smime.go @@ -14,7 +14,6 @@ import ( "errors" "fmt" "mime/quotedprintable" - "strings" "github.com/wneessen/go-mail/internal/pkcs7" ) @@ -46,6 +45,7 @@ type SMIME struct { privateKey privateKeyHolder certificate *x509.Certificate intermediateCertificate *x509.Certificate + isSigned bool } // newSMIMEWithRSA construct a new instance of SMIME with provided parameters @@ -89,17 +89,15 @@ func newSMIMEWithECDSA(privateKey *ecdsa.PrivateKey, certificate *x509.Certifica } // signMessage signs the message with S/MIME -func (sm *SMIME) signMessage(message string) (*string, error) { - lines := parseLines([]byte(message)) - toBeSigned := lines.bytesFromLines([]byte("\r\n")) - - signedData, err := pkcs7.NewSignedData(toBeSigned) +func (sm *SMIME) signMessage(message string) (string, error) { + toBeSigned := bytes.NewBufferString(message) + signedData, err := pkcs7.NewSignedData(toBeSigned.Bytes()) if err != nil || signedData == nil { - return nil, fmt.Errorf("could not initialize signed data: %w", err) + return "", fmt.Errorf("failed to initialize signed data: %w", err) } if err = signedData.AddSigner(sm.certificate, sm.privateKey.get(), pkcs7.SignerInfoConfig{}); err != nil { - return nil, fmt.Errorf("could not add signer message: %w", err) + return "", fmt.Errorf("could not add signer message: %w", err) } if sm.intermediateCertificate != nil { @@ -110,110 +108,54 @@ func (sm *SMIME) signMessage(message string) (*string, error) { signatureDER, err := signedData.Finish() if err != nil { - return nil, fmt.Errorf("could not finish signing: %w", err) + return "", fmt.Errorf("failed to finish signature: %w", err) } pemMsg, err := encodeToPEM(signatureDER) if err != nil { - return nil, fmt.Errorf("could not encode to PEM: %w", err) + return "", fmt.Errorf("could not encode to PEM: %w", err) } return pemMsg, nil } -// createMessage prepares the message that will be used for the sign method later +// prepareMessage prepares the message that will be used for the sign method later func (sm *SMIME) prepareMessage(encoding Encoding, contentType ContentType, charset Charset, body []byte) (*string, error) { encodedMessage, err := sm.encodeMessage(encoding, string(body)) if err != nil { return nil, err } - preparedMessage := fmt.Sprintf("Content-Transfer-Encoding: %v\r\nContent-Type: %v; charset=%v\r\n\r\n%v", encoding, contentType, charset, *encodedMessage) + preparedMessage := fmt.Sprintf("Content-Transfer-Encoding: %s\r\nContent-Type: %s; charset=%s\r\n\r\n%s", + encoding, contentType, charset, encodedMessage) return &preparedMessage, nil } // encodeMessage encodes the message with the given encoding -func (sm *SMIME) encodeMessage(encoding Encoding, message string) (*string, error) { +func (sm *SMIME) encodeMessage(encoding Encoding, message string) (string, error) { if encoding != EncodingQP { - return &message, nil + return message, nil } - buffer := bytes.Buffer{} - writer := quotedprintable.NewWriter(&buffer) + buffer := bytes.NewBuffer(nil) + writer := quotedprintable.NewWriter(buffer) if _, err := writer.Write([]byte(message)); err != nil { - return nil, err + return "", err } if err := writer.Close(); err != nil { - return nil, err + return "", err } encodedMessage := buffer.String() - - return &encodedMessage, nil + return encodedMessage, nil } // encodeToPEM uses the method pem.Encode from the standard library but cuts the typical PEM preamble -func encodeToPEM(msg []byte) (*string, error) { +func encodeToPEM(msg []byte) (string, error) { block := &pem.Block{Bytes: msg} - var arrayBuffer bytes.Buffer - if err := pem.Encode(&arrayBuffer, block); err != nil { - return nil, err - } - - r := arrayBuffer.String() - r = strings.TrimPrefix(r, "-----BEGIN -----") - r = strings.Trim(r, "\n") - r = strings.TrimSuffix(r, "-----END -----") - r = strings.Trim(r, "\n") - - return &r, nil -} - -// line is the representation of one line of the message that will be used for signing purposes -type line struct { - line []byte - endOfLine []byte -} - -// lines is the representation of a message that will be used for signing purposes -type lines []line - -// bytesFromLines creates the line representation with the given endOfLine char -func (ls lines) bytesFromLines(sep []byte) []byte { - var raw []byte - for i := range ls { - raw = append(raw, ls[i].line...) - if len(ls[i].endOfLine) != 0 && sep != nil { - raw = append(raw, sep...) - } else { - raw = append(raw, ls[i].endOfLine...) - } + buf := bytes.NewBuffer(nil) + if err := pem.Encode(buf, block); err != nil { + return "", err } - return raw -} - -// parseLines constructs the lines representation of a given message -func parseLines(raw []byte) lines { - oneLine := line{raw, nil} - lines := lines{oneLine} - lines = lines.splitLine([]byte("\r\n")) - lines = lines.splitLine([]byte("\r")) - lines = lines.splitLine([]byte("\n")) - return lines -} -// splitLine uses the given endOfLine to split the given line -func (ls lines) splitLine(sep []byte) lines { - nl := lines{} - for _, l := range ls { - split := bytes.Split(l.line, sep) - if len(split) > 1 { - for i := 0; i < len(split)-1; i++ { - nl = append(nl, line{split[i], sep}) - } - nl = append(nl, line{split[len(split)-1], l.endOfLine}) - } else { - nl = append(nl, l) - } - } - return nl + return buf.String()[17 : buf.Len()-16], nil } diff --git a/smime_test.go b/smime_test.go index a06125ce..c5c842a7 100644 --- a/smime_test.go +++ b/smime_test.go @@ -5,7 +5,6 @@ package mail import ( - "bytes" "crypto/ecdsa" "crypto/rsa" "crypto/tls" @@ -109,7 +108,7 @@ func TestSign(t *testing.T) { t.Errorf("Error creating singed message: %s", err) } - if *singedMessage == message { + if singedMessage == message { t.Errorf("Sign() did not work") } } @@ -205,8 +204,8 @@ func TestEncodeMessage(t *testing.T) { t.Errorf("Error preparing message: %s", err) } - if *result != body { - t.Errorf("encodeMessage() did not return the correct encoded message: %s", *result) + if result != body { + t.Errorf("encodeMessage() did not return the correct encoded message: %s", result) } } @@ -231,8 +230,8 @@ func TestEncodeMessage_QuotedPrintable(t *testing.T) { t.Errorf("Error preparing message: %s", err) } - if *result != quotedPrintableBody { - t.Errorf("encodeMessage() did not return the correct encoded message: %s", *result) + if result != quotedPrintableBody { + t.Errorf("encodeMessage() did not return the correct encoded message: %s", result) } } @@ -246,102 +245,11 @@ func TestEncodeToPEM(t *testing.T) { } base64Encoded := base64.StdEncoding.EncodeToString(message) - if *pemMessage != base64Encoded { + if pemMessage != base64Encoded { t.Errorf("encodeToPEM() did not work") } } -// TestBytesFromLines tests the bytesFromLines method -func TestBytesFromLines(t *testing.T) { - ls := lines{ - {line: []byte("Hello"), endOfLine: []byte("\n")}, - {line: []byte("World"), endOfLine: []byte("\n")}, - } - expected := []byte("Hello\nWorld\n") - - result := ls.bytesFromLines([]byte("\n")) - if !bytes.Equal(result, expected) { - t.Errorf("Expected %s, but got %s", expected, result) - } -} - -// FuzzBytesFromLines tests the bytesFromLines method with fuzzing -func FuzzBytesFromLines(f *testing.F) { - f.Add([]byte("Hello"), []byte("\n")) - f.Fuzz(func(t *testing.T, lineData, sep []byte) { - ls := lines{ - {line: lineData, endOfLine: sep}, - } - _ = ls.bytesFromLines(sep) - }) -} - -// TestParseLines tests the parseLines method -func TestParseLines(t *testing.T) { - input := []byte("Hello\r\nWorld\nHello\rWorld") - expected := lines{ - {line: []byte("Hello"), endOfLine: []byte("\r\n")}, - {line: []byte("World"), endOfLine: []byte("\n")}, - {line: []byte("Hello"), endOfLine: []byte("\r")}, - {line: []byte("World"), endOfLine: []byte("")}, - } - - result := parseLines(input) - if len(result) != len(expected) { - t.Errorf("Expected %d lines, but got %d", len(expected), len(result)) - } - - for i := range result { - if !bytes.Equal(result[i].line, expected[i].line) || !bytes.Equal(result[i].endOfLine, expected[i].endOfLine) { - t.Errorf("Line %d mismatch. Expected line: %s, endOfLine: %s, got line: %s, endOfLine: %s", - i, expected[i].line, expected[i].endOfLine, result[i].line, result[i].endOfLine) - } - } -} - -// FuzzParseLines tests the parseLines method with fuzzing -func FuzzParseLines(f *testing.F) { - f.Add([]byte("Hello\nWorld\r\nAnother\rLine")) - f.Fuzz(func(t *testing.T, input []byte) { - _ = parseLines(input) - }) -} - -// TestSplitLine tests the splitLine method -func TestSplitLine(t *testing.T) { - ls := lines{ - {line: []byte("Hello\r\nWorld\r\nAnotherLine"), endOfLine: []byte("")}, - } - expected := lines{ - {line: []byte("Hello"), endOfLine: []byte("\r\n")}, - {line: []byte("World"), endOfLine: []byte("\r\n")}, - {line: []byte("AnotherLine"), endOfLine: []byte("")}, - } - - result := ls.splitLine([]byte("\r\n")) - if len(result) != len(expected) { - t.Errorf("Expected %d lines, but got %d", len(expected), len(result)) - } - - for i := range result { - if !bytes.Equal(result[i].line, expected[i].line) || !bytes.Equal(result[i].endOfLine, expected[i].endOfLine) { - t.Errorf("Line %d mismatch. Expected line: %s, endOfLine: %s, got line: %s, endOfLine: %s", - i, expected[i].line, expected[i].endOfLine, result[i].line, result[i].endOfLine) - } - } -} - -// FuzzSplitLine tests the parseLsplitLineines method with fuzzing -func FuzzSplitLine(f *testing.F) { - f.Add([]byte("Hello\r\nWorld"), []byte("\r\n")) - f.Fuzz(func(t *testing.T, input, sep []byte) { - ls := lines{ - {line: input, endOfLine: []byte("")}, - } - _ = ls.splitLine(sep) - }) -} - // getDummyRSACryptoMaterial loads a certificate (RSA), the associated private key and certificate (RSA) is loaded // from local disk for testing purposes func getDummyRSACryptoMaterial() (*rsa.PrivateKey, *x509.Certificate, *x509.Certificate, error) { From c619755c3251125699004a7063cfe9f69fd3e73a Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 4 Jan 2025 15:22:49 +0100 Subject: [PATCH 19/47] Refactor S/MIME signing to unify key handling and improve error clarity Consolidated RSA and ECDSA S/MIME signing into a single method to simplify keypair handling. Replaced custom key holder structure with `crypto.PrivateKey` for flexibility. Enhanced error messages and pre-checks for better diagnostics and maintainability. --- msg.go | 56 +++++++++++++++++------------------------- msg_test.go | 4 +-- smime.go | 71 ++++++++++++++++++++--------------------------------- 3 files changed, 51 insertions(+), 80 deletions(-) diff --git a/msg.go b/msg.go index 976ad35d..ada4f96a 100644 --- a/msg.go +++ b/msg.go @@ -7,6 +7,7 @@ package mail import ( "bytes" "context" + "crypto" "crypto/ecdsa" "crypto/rsa" "crypto/tls" @@ -2520,7 +2521,7 @@ func (m *Msg) addAddr(header AddrHeader, addr string) error { return m.SetAddrHeader(header, addresses...) } -// SignWithSMIMERSA configures the Msg to be signed with S/MIME using RSA. +// SignWithKeypair configures the Msg to be signed with S/MIME using RSA or ECDSA public-/private keypair. // // This function sets up S/MIME signing for the Msg by associating it with the provided private key, // certificate, and intermediate certificate. @@ -2532,29 +2533,8 @@ func (m *Msg) addAddr(header AddrHeader, addr string) error { // // Returns: // - An error if any issue occurred while configuring S/MIME signing; otherwise nil. -func (m *Msg) SignWithSMIMERSA(privateKey *rsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) error { - smime, err := newSMIMEWithRSA(privateKey, certificate, intermediateCertificate) - if err != nil { - return err - } - m.sMIME = smime - return nil -} - -// SignWithSMIMEECDSA configures the Msg to be signed with S/MIME using ECDSA. -// -// This function sets up S/MIME signing for the Msg by associating it with the provided ECDSA private key, -// certificate, and intermediate certificate. -// -// Parameters: -// - privateKey: The ECDSA private key used for signing. -// - certificate: The x509 certificate associated with the private key. -// - intermediateCertificate: An optional intermediate x509 certificate for chain validation. -// -// Returns: -// - An error if any issue occurred while configuring S/MIME signing; otherwise nil. -func (m *Msg) SignWithSMIMEECDSA(privateKey *ecdsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) error { - smime, err := newSMIMEWithECDSA(privateKey, certificate, intermediateCertificate) +func (m *Msg) SignWithKeypair(privateKey crypto.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) error { + smime, err := newSMIME(privateKey, certificate, intermediateCertificate) if err != nil { return err } @@ -2592,11 +2572,8 @@ func (m *Msg) SignWithTLSCertificate(keyPairTLS *tls.Certificate) error { } switch keyPairTLS.PrivateKey.(type) { - case *rsa.PrivateKey: - return m.SignWithSMIMERSA(keyPairTLS.PrivateKey.(*rsa.PrivateKey), leafCertificate, intermediateCert) - - case *ecdsa.PrivateKey: - return m.SignWithSMIMEECDSA(keyPairTLS.PrivateKey.(*ecdsa.PrivateKey), leafCertificate, intermediateCert) + case *rsa.PrivateKey, *ecdsa.PrivateKey: + return m.SignWithKeypair(keyPairTLS.PrivateKey, leafCertificate, intermediateCert) default: return fmt.Errorf("unsupported private key type: %T", keyPairTLS.PrivateKey) } @@ -2814,7 +2791,7 @@ func (m *Msg) createSignaturePart(encoding Encoding, contentType ContentType, ch if err != nil { return nil, err } - signedMessage, err := m.sMIME.signMessage(*message) + signedMessage, err := m.sMIME.signMessage(message) if err != nil { return nil, err } @@ -3089,10 +3066,16 @@ func getEncoder(enc Encoding) mime.WordEncoder { // - The parsed leaf x509 certificate. // - An error if the certificate could not be parsed. func getLeafCertificate(keyPairTlS *tls.Certificate) (*x509.Certificate, error) { + if keyPairTlS == nil { + return nil, errors.New("provided certificate is nil") + } if keyPairTlS.Leaf != nil { return keyPairTlS.Leaf, nil } + if len(keyPairTlS.Certificate) == 0 { + return nil, errors.New("certificate chain is empty") + } cert, err := x509.ParseCertificate(keyPairTlS.Certificate[0]) if err != nil { return nil, err @@ -3116,13 +3099,20 @@ func (m *Msg) signMessage(msg *Msg) (*Msg, error) { if msg.sMIME.isSigned { return msg, nil } - signedPart := msg.GetParts()[0] + + parts := msg.GetParts() + if len(parts) == 0 { + return msg, errors.New("message has no parts") + } + + signedPart := parts[0] body, err := signedPart.GetContent() if err != nil { - return nil, err + return nil, fmt.Errorf("failed to get message body: %w", err) } - signaturePart, err := m.createSignaturePart(signedPart.GetEncoding(), signedPart.GetContentType(), signedPart.GetCharset(), body) + signaturePart, err := m.createSignaturePart(signedPart.GetEncoding(), signedPart.GetContentType(), + signedPart.GetCharset(), body) if err != nil { return nil, err } diff --git a/msg_test.go b/msg_test.go index c38a3851..f946ed85 100644 --- a/msg_test.go +++ b/msg_test.go @@ -6749,7 +6749,7 @@ func TestSignWithSMime_InvalidPrivateKey(t *testing.T) { m := NewMsg() err := m.SignWithSMIMERSA(nil, nil, nil) - if !errors.Is(err, ErrInvalidPrivateKey) { + if !errors.Is(err, ErrPrivateKeyMissing) { t.Errorf("failed to pre-check SignWithSMime method values correctly: %s", err) } } @@ -6763,7 +6763,7 @@ func TestSignWithSMime_InvalidCertificate(t *testing.T) { m := NewMsg() err = m.SignWithSMIMERSA(privateKey, nil, nil) - if !errors.Is(err, ErrInvalidCertificate) { + if !errors.Is(err, ErrCertificateMissing) { t.Errorf("failed to pre-check SignWithSMime method values correctly: %s", err) } } diff --git a/smime.go b/smime.go index 42ee4ef0..ce0d3964 100644 --- a/smime.go +++ b/smime.go @@ -8,7 +8,6 @@ import ( "bytes" "crypto" "crypto/ecdsa" - "crypto/rsa" "crypto/x509" "encoding/pem" "errors" @@ -19,30 +18,16 @@ import ( ) var ( - // ErrInvalidPrivateKey should be used if private key is invalid - ErrInvalidPrivateKey = errors.New("invalid private key") + // ErrPrivateKeyMissing should be used if private key is invalid + ErrPrivateKeyMissing = errors.New("private key is missing") - // ErrInvalidCertificate should be used if the certificate is invalid - ErrInvalidCertificate = errors.New("invalid certificate") + // ErrCertificateMissing should be used if the certificate is invalid + ErrCertificateMissing = errors.New("certificate is missing") ) -// privateKeyHolder is the representation of a private key -type privateKeyHolder struct { - ecdsa *ecdsa.PrivateKey - rsa *rsa.PrivateKey -} - -// get returns the private key of the privateKeyHolder -func (p privateKeyHolder) get() crypto.PrivateKey { - if p.ecdsa != nil { - return p.ecdsa - } - return p.rsa -} - // SMIME is used to sign messages with S/MIME type SMIME struct { - privateKey privateKeyHolder + privateKey crypto.PrivateKey certificate *x509.Certificate intermediateCertificate *x509.Certificate isSigned bool @@ -52,17 +37,16 @@ type SMIME struct { // privateKey as *rsa.PrivateKey // certificate as *x509.Certificate // intermediateCertificate (optional) as *x509.Certificate -func newSMIMEWithRSA(privateKey *rsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) (*SMIME, error) { +func newSMIME(privateKey crypto.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) (*SMIME, error) { if privateKey == nil { - return nil, ErrInvalidPrivateKey + return nil, ErrPrivateKeyMissing } - if certificate == nil { - return nil, ErrInvalidCertificate + return nil, ErrCertificateMissing } return &SMIME{ - privateKey: privateKeyHolder{rsa: privateKey}, + privateKey: privateKey, certificate: certificate, intermediateCertificate: intermediateCertificate, }, nil @@ -74,34 +58,33 @@ func newSMIMEWithRSA(privateKey *rsa.PrivateKey, certificate *x509.Certificate, // intermediateCertificate (optional) as *x509.Certificate func newSMIMEWithECDSA(privateKey *ecdsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) (*SMIME, error) { if privateKey == nil { - return nil, ErrInvalidPrivateKey + return nil, ErrPrivateKeyMissing } - if certificate == nil { - return nil, ErrInvalidCertificate + return nil, ErrCertificateMissing } return &SMIME{ - privateKey: privateKeyHolder{ecdsa: privateKey}, + privateKey: privateKey, certificate: certificate, intermediateCertificate: intermediateCertificate, }, nil } // signMessage signs the message with S/MIME -func (sm *SMIME) signMessage(message string) (string, error) { +func (s *SMIME) signMessage(message string) (string, error) { toBeSigned := bytes.NewBufferString(message) signedData, err := pkcs7.NewSignedData(toBeSigned.Bytes()) if err != nil || signedData == nil { return "", fmt.Errorf("failed to initialize signed data: %w", err) } - if err = signedData.AddSigner(sm.certificate, sm.privateKey.get(), pkcs7.SignerInfoConfig{}); err != nil { + if err = signedData.AddSigner(s.certificate, s.privateKey, pkcs7.SignerInfoConfig{}); err != nil { return "", fmt.Errorf("could not add signer message: %w", err) } - if sm.intermediateCertificate != nil { - signedData.AddCertificate(sm.intermediateCertificate) + if s.intermediateCertificate != nil { + signedData.AddCertificate(s.intermediateCertificate) } signedData.Detach() @@ -120,25 +103,25 @@ func (sm *SMIME) signMessage(message string) (string, error) { } // prepareMessage prepares the message that will be used for the sign method later -func (sm *SMIME) prepareMessage(encoding Encoding, contentType ContentType, charset Charset, body []byte) (*string, error) { - encodedMessage, err := sm.encodeMessage(encoding, string(body)) +func (s *SMIME) prepareMessage(encoding Encoding, contentType ContentType, charset Charset, body []byte) (string, error) { + encodedMessage, err := s.encodeMessage(encoding, body) if err != nil { - return nil, err + return "", err } preparedMessage := fmt.Sprintf("Content-Transfer-Encoding: %s\r\nContent-Type: %s; charset=%s\r\n\r\n%s", encoding, contentType, charset, encodedMessage) - return &preparedMessage, nil + return preparedMessage, nil } // encodeMessage encodes the message with the given encoding -func (sm *SMIME) encodeMessage(encoding Encoding, message string) (string, error) { +func (sm *SMIME) encodeMessage(encoding Encoding, message []byte) (string, error) { if encoding != EncodingQP { - return message, nil + return string(message), nil } buffer := bytes.NewBuffer(nil) writer := quotedprintable.NewWriter(buffer) - if _, err := writer.Write([]byte(message)); err != nil { + if _, err := writer.Write(message); err != nil { return "", err } if err := writer.Close(); err != nil { @@ -151,11 +134,9 @@ func (sm *SMIME) encodeMessage(encoding Encoding, message string) (string, error // encodeToPEM uses the method pem.Encode from the standard library but cuts the typical PEM preamble func encodeToPEM(msg []byte) (string, error) { block := &pem.Block{Bytes: msg} - - buf := bytes.NewBuffer(nil) - if err := pem.Encode(buf, block); err != nil { + buffer := bytes.NewBuffer(nil) + if err := pem.Encode(buffer, block); err != nil { return "", err } - - return buf.String()[17 : buf.Len()-16], nil + return buffer.String()[17 : buffer.Len()-16], nil } From b03a19abb526c68438265e0cbe6ee6208a305a3b Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 4 Jan 2025 15:35:23 +0100 Subject: [PATCH 20/47] Refactor S/MIME signing to unify keypair handling. Updated S/MIME signing functions to replace specific methods (`SignWithSMIMERSA` and `SignWithSMIMEECDSA`) with a unified `SignWithKeypair` method. Simplified test handling and refactored private key checks to make the codebase cleaner and reduce redundancy. Transitioned crypto material loaders to return generic `crypto.PrivateKey` for improved flexibility. --- msg_test.go | 26 +++++----- msgwriter_test.go | 2 +- smime_test.go | 129 +++++++++++++++++----------------------------- 3 files changed, 62 insertions(+), 95 deletions(-) diff --git a/msg_test.go b/msg_test.go index f946ed85..5b9c29bb 100644 --- a/msg_test.go +++ b/msg_test.go @@ -6407,7 +6407,7 @@ func TestMsg_hasAltWithSMime(t *testing.T) { m := NewMsg() m.SetBodyString(TypeTextPlain, "Plain") m.AddAlternativeString(TypeTextHTML, "HTML") - if err := m.SignWithSMIMERSA(privateKey, certificate, intermediateCertificate); err != nil { + if err := m.SignWithKeypair(privateKey, certificate, intermediateCertificate); err != nil { t.Errorf("failed to init smime configuration") } if m.hasAlt() { @@ -6422,7 +6422,7 @@ func TestMsg_hasSMime(t *testing.T) { t.Fatalf("failed to load dummy crypto material: %s", err) } m := NewMsg() - if err := m.SignWithSMIMERSA(privateKey, certificate, intermediateCertificate); err != nil { + if err := m.SignWithKeypair(privateKey, certificate, intermediateCertificate); err != nil { t.Errorf("failed to init smime configuration") } m.SetBodyString(TypeTextPlain, "Plain") @@ -6517,7 +6517,7 @@ func TestMsg_WriteToWithSMIME(t *testing.T) { m := NewMsg() m.Subject("This is a test") m.SetBodyString(TypeTextPlain, "Plain") - if err := m.SignWithSMIMERSA(privateKey, certificate, intermediateCertificate); err != nil { + if err := m.SignWithKeypair(privateKey, certificate, intermediateCertificate); err != nil { t.Errorf("failed to init smime configuration") } @@ -6674,10 +6674,10 @@ func TestSignWithSMime_ValidRSAKeyPair(t *testing.T) { t.Errorf("failed to laod dummy crypto material. Cause: %v", err) } m := NewMsg() - if err := m.SignWithSMIMERSA(privateKey, certificate, intermediateCertificate); err != nil { + if err := m.SignWithKeypair(privateKey, certificate, intermediateCertificate); err != nil { t.Errorf("failed to set sMIME. Cause: %v", err) } - if m.sMIME.privateKey.rsa == nil { + if m.sMIME.privateKey == nil { t.Errorf("WithSMIMESigning() - no private key is given") } if m.sMIME.certificate == nil { @@ -6692,10 +6692,10 @@ func TestSignWithSMime_ValidECDSAKeyPair(t *testing.T) { t.Errorf("failed to laod dummy crypto material. Cause: %v", err) } m := NewMsg() - if err := m.SignWithSMIMEECDSA(privateKey, certificate, intermediateCertificate); err != nil { + if err := m.SignWithKeypair(privateKey, certificate, intermediateCertificate); err != nil { t.Errorf("failed to set sMIME. Cause: %v", err) } - if m.sMIME.privateKey.ecdsa == nil { + if m.sMIME.privateKey == nil { t.Errorf("WithSMIMESigning() - no private key is given") } if m.sMIME.certificate == nil { @@ -6713,7 +6713,7 @@ func TestSignWithTLSCertificate(t *testing.T) { if err := m.SignWithTLSCertificate(keyPairTLS); err != nil { t.Errorf("failed to set sMIME. Cause: %v", err) } - if m.sMIME.privateKey.ecdsa == nil { + if m.sMIME.privateKey == nil { t.Errorf("SignWithTLSCertificate() - no private key is given") } if m.sMIME.certificate == nil { @@ -6736,7 +6736,7 @@ func TestSignWithTLSCertificate_WithKeyPairLeafNil(t *testing.T) { if err := m.SignWithTLSCertificate(keyPairTLS); err != nil { t.Errorf("failed to set sMIME. Cause: %v", err) } - if m.sMIME.privateKey.ecdsa == nil { + if m.sMIME.privateKey == nil { t.Errorf("SignWithTLSCertificate() - no private key is given") } if m.sMIME.certificate == nil { @@ -6748,7 +6748,7 @@ func TestSignWithTLSCertificate_WithKeyPairLeafNil(t *testing.T) { func TestSignWithSMime_InvalidPrivateKey(t *testing.T) { m := NewMsg() - err := m.SignWithSMIMERSA(nil, nil, nil) + err := m.SignWithKeypair(nil, nil, nil) if !errors.Is(err, ErrPrivateKeyMissing) { t.Errorf("failed to pre-check SignWithSMime method values correctly: %s", err) } @@ -6762,7 +6762,7 @@ func TestSignWithSMime_InvalidCertificate(t *testing.T) { } m := NewMsg() - err = m.SignWithSMIMERSA(privateKey, nil, nil) + err = m.SignWithKeypair(privateKey, nil, nil) if !errors.Is(err, ErrCertificateMissing) { t.Errorf("failed to pre-check SignWithSMime method values correctly: %s", err) } @@ -6775,7 +6775,7 @@ func TestMsg_createSignaturePart(t *testing.T) { t.Errorf("failed to laod dummy crypto material. Cause: %v", err) } m := NewMsg() - if err = m.SignWithSMIMERSA(privateKey, certificate, intermediateCertificate); err != nil { + if err = m.SignWithKeypair(privateKey, certificate, intermediateCertificate); err != nil { t.Errorf("failed to init smime configuration") } body := []byte("This is the body") @@ -6817,7 +6817,7 @@ func TestMsg_signMessage(t *testing.T) { body := []byte("This is the body") m := NewMsg() m.SetBodyString(TypeTextPlain, string(body)) - if err := m.SignWithSMIMERSA(privateKey, certificate, intermediateCertificate); err != nil { + if err := m.SignWithKeypair(privateKey, certificate, intermediateCertificate); err != nil { t.Errorf("failed to init smime configuration") } msg, err := m.signMessage(m) diff --git a/msgwriter_test.go b/msgwriter_test.go index fa21fc0f..70cb1693 100644 --- a/msgwriter_test.go +++ b/msgwriter_test.go @@ -774,7 +774,7 @@ func TestMsgWriter_writeMsg_SMime(t *testing.T) { } m := NewMsg() - if err := m.SignWithSMIMERSA(privateKey, certificate, intermediateCertificate); err != nil { + if err := m.SignWithKeypair(privateKey, certificate, intermediateCertificate); err != nil { t.Errorf("failed to init smime configuration") } _ = m.From(`"Toni Tester" `) diff --git a/smime_test.go b/smime_test.go index c5c842a7..c728608e 100644 --- a/smime_test.go +++ b/smime_test.go @@ -5,6 +5,7 @@ package mail import ( + "crypto" "crypto/ecdsa" "crypto/rsa" "crypto/tls" @@ -22,71 +23,37 @@ const ( dummyKeyECDSAPath = "testdata/dummy-child-key-ecdsa.pem" ) -func TestGet_RSA(t *testing.T) { - p := privateKeyHolder{ - ecdsa: nil, - rsa: &rsa.PrivateKey{}, - } - - if p.get() == nil { - t.Errorf("get() did not return the correct private key") - } -} - -func TestGet_ECDSA(t *testing.T) { - p := privateKeyHolder{ - ecdsa: &ecdsa.PrivateKey{}, - rsa: nil, - } - - if p.get() == nil { - t.Errorf("get() did not return the correct private key") - } -} - // TestNewSMimeWithRSA tests the newSMime method with RSA crypto material -func TestNewSMimeWithRSA(t *testing.T) { - privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() - if err != nil { - t.Errorf("Error getting dummy crypto material: %s", err) - } - - sMime, err := newSMIMEWithRSA(privateKey, certificate, intermediateCertificate) - if err != nil { - t.Errorf("Error creating new SMIME from keyPair: %s", err) - } - - if sMime.privateKey.rsa != privateKey { - t.Errorf("NewSMime() did not return the same private key") - } - if sMime.certificate != certificate { - t.Errorf("NewSMime() did not return the same certificate") - } - if sMime.intermediateCertificate != intermediateCertificate { - t.Errorf("NewSMime() did not return the same intermedidate certificate") - } -} - -// TestNewSMimeWithECDSA tests the newSMime method with ECDSA crypto material -func TestNewSMimeWithECDSA(t *testing.T) { - privateKey, certificate, intermediateCertificate, err := getDummyECDSACryptoMaterial() - if err != nil { - t.Errorf("Error getting dummy crypto material: %s", err) - } - - sMime, err := newSMIMEWithECDSA(privateKey, certificate, intermediateCertificate) - if err != nil { - t.Errorf("Error creating new SMIME from keyPair: %s", err) - } - - if sMime.privateKey.ecdsa != privateKey { - t.Errorf("NewSMime() did not return the same private key") - } - if sMime.certificate != certificate { - t.Errorf("NewSMime() did not return the same certificate") - } - if sMime.intermediateCertificate != intermediateCertificate { - t.Errorf("NewSMime() did not return the same intermedidate certificate") +func TestNewSMIME(t *testing.T) { + tests := []struct { + name string + certFunc func() (crypto.PrivateKey, *x509.Certificate, *x509.Certificate, error) + }{ + {"RSA", getDummyRSACryptoMaterial}, + {"ECDSA", getDummyECDSACryptoMaterial}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + privateKey, certificate, intermediateCertificate, err := tt.certFunc() + if err != nil { + t.Errorf("Error getting dummy crypto material: %s", err) + } + + sMime, err := newSMIME(privateKey, certificate, intermediateCertificate) + if err != nil { + t.Errorf("Error creating new SMIME from keyPair: %s", err) + } + + if sMime.privateKey != privateKey { + t.Errorf("NewSMime() did not return the same private key") + } + if sMime.certificate != certificate { + t.Errorf("NewSMime() did not return the same certificate") + } + if sMime.intermediateCertificate != intermediateCertificate { + t.Errorf("NewSMime() did not return the same intermedidate certificate") + } + }) } } @@ -97,7 +64,7 @@ func TestSign(t *testing.T) { t.Errorf("Error getting dummy crypto material: %s", err) } - sMime, err := newSMIMEWithRSA(privateKey, certificate, intermediateCertificate) + sMime, err := newSMIME(privateKey, certificate, intermediateCertificate) if err != nil { t.Errorf("Error creating new SMIME from keyPair: %s", err) } @@ -120,7 +87,7 @@ func TestPrepareMessage(t *testing.T) { t.Errorf("Error getting dummy crypto material: %s", err) } - sMime, err := newSMIMEWithRSA(privateKey, certificate, intermediateCertificate) + sMime, err := newSMIME(privateKey, certificate, intermediateCertificate) if err != nil { t.Errorf("Error creating new SMIME from keyPair: %s", err) } @@ -134,16 +101,16 @@ func TestPrepareMessage(t *testing.T) { t.Errorf("Error preparing message: %s", err) } - if !strings.Contains(*result, encoding.String()) { + if !strings.Contains(result, encoding.String()) { t.Errorf("prepareMessage() did not return the correct encoding") } - if !strings.Contains(*result, contentType.String()) { + if !strings.Contains(result, contentType.String()) { t.Errorf("prepareMessage() did not return the correct contentType") } - if !strings.Contains(*result, string(body)) { + if !strings.Contains(result, string(body)) { t.Errorf("prepareMessage() did not return the correct body") } - if *result != fmt.Sprintf("Content-Transfer-Encoding: %s\r\nContent-Type: %s; charset=%s\r\n\r\n%s", encoding, contentType, charset, string(body)) { + if result != fmt.Sprintf("Content-Transfer-Encoding: %s\r\nContent-Type: %s; charset=%s\r\n\r\n%s", encoding, contentType, charset, string(body)) { t.Errorf("prepareMessage() did not sucessfully create the message") } } @@ -155,7 +122,7 @@ func TestPrepareMessage_QuotedPrintable(t *testing.T) { t.Errorf("Error getting dummy crypto material: %s", err) } - sMime, err := newSMIMEWithRSA(privateKey, certificate, intermediateCertificate) + sMime, err := newSMIME(privateKey, certificate, intermediateCertificate) if err != nil { t.Errorf("Error creating new SMIME from keyPair: %s", err) } @@ -170,16 +137,16 @@ func TestPrepareMessage_QuotedPrintable(t *testing.T) { t.Errorf("Error preparing message: %s", err) } - if !strings.Contains(*result, encoding.String()) { + if !strings.Contains(result, encoding.String()) { t.Errorf("prepareMessage() did not return the correct encoding") } - if !strings.Contains(*result, contentType.String()) { + if !strings.Contains(result, contentType.String()) { t.Errorf("prepareMessage() did not return the correct contentType") } - if !strings.Contains(*result, quotedPrintableBody) { + if !strings.Contains(result, quotedPrintableBody) { t.Errorf("prepareMessage() did not return the correct body") } - if *result != fmt.Sprintf("Content-Transfer-Encoding: %s\r\nContent-Type: %s; charset=%s\r\n\r\n%s", encoding, contentType, charset, quotedPrintableBody) { + if result != fmt.Sprintf("Content-Transfer-Encoding: %s\r\nContent-Type: %s; charset=%s\r\n\r\n%s", encoding, contentType, charset, quotedPrintableBody) { t.Errorf("prepareMessage() did not sucessfully create the message") } } @@ -194,12 +161,12 @@ func TestEncodeMessage(t *testing.T) { t.Errorf("Error getting dummy crypto material: %s", err) } - sMime, err := newSMIMEWithRSA(privateKey, certificate, intermediateCertificate) + sMime, err := newSMIME(privateKey, certificate, intermediateCertificate) if err != nil { t.Errorf("Error creating new SMIME from keyPair: %s", err) } - result, err := sMime.encodeMessage(encoding, body) + result, err := sMime.encodeMessage(encoding, []byte(body)) if err != nil { t.Errorf("Error preparing message: %s", err) } @@ -220,12 +187,12 @@ func TestEncodeMessage_QuotedPrintable(t *testing.T) { t.Errorf("Error getting dummy crypto material: %s", err) } - sMime, err := newSMIMEWithRSA(privateKey, certificate, intermediateCertificate) + sMime, err := newSMIME(privateKey, certificate, intermediateCertificate) if err != nil { t.Errorf("Error creating new SMIME from keyPair: %s", err) } - result, err := sMime.encodeMessage(encoding, body) + result, err := sMime.encodeMessage(encoding, []byte(body)) if err != nil { t.Errorf("Error preparing message: %s", err) } @@ -252,7 +219,7 @@ func TestEncodeToPEM(t *testing.T) { // getDummyRSACryptoMaterial loads a certificate (RSA), the associated private key and certificate (RSA) is loaded // from local disk for testing purposes -func getDummyRSACryptoMaterial() (*rsa.PrivateKey, *x509.Certificate, *x509.Certificate, error) { +func getDummyRSACryptoMaterial() (crypto.PrivateKey, *x509.Certificate, *x509.Certificate, error) { keyPair, err := tls.LoadX509KeyPair(dummyCertRSAPath, dummyKeyRSAPath) if err != nil { return nil, nil, nil, err @@ -275,7 +242,7 @@ func getDummyRSACryptoMaterial() (*rsa.PrivateKey, *x509.Certificate, *x509.Cert // getDummyECDSACryptoMaterial loads a certificate (ECDSA), the associated private key and certificate (ECDSA) is // loaded from local disk for testing purposes -func getDummyECDSACryptoMaterial() (*ecdsa.PrivateKey, *x509.Certificate, *x509.Certificate, error) { +func getDummyECDSACryptoMaterial() (crypto.PrivateKey, *x509.Certificate, *x509.Certificate, error) { keyPair, err := tls.LoadX509KeyPair(dummyCertECDSAPath, dummyKeyECDSAPath) if err != nil { return nil, nil, nil, err From ccb6b05f97cef8a822645c66f0300df138e20973 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 4 Jan 2025 15:36:29 +0100 Subject: [PATCH 21/47] Remove ECDSA-specific S/MIME support from smime.go This change eliminates the `newSMIMEWithECDSA` function and associated imports, streamlining the codebase by removing unused ECDSA-related implementation. The update simplifies maintenance and focuses the library on core S/MIME functionality. --- smime.go | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/smime.go b/smime.go index ce0d3964..f596d19c 100644 --- a/smime.go +++ b/smime.go @@ -7,7 +7,6 @@ package mail import ( "bytes" "crypto" - "crypto/ecdsa" "crypto/x509" "encoding/pem" "errors" @@ -52,25 +51,6 @@ func newSMIME(privateKey crypto.PrivateKey, certificate *x509.Certificate, inter }, nil } -// newSMIMEWithECDSA construct a new instance of SMIME with provided parameters -// privateKey as *ecdsa.PrivateKey -// certificate as *x509.Certificate -// intermediateCertificate (optional) as *x509.Certificate -func newSMIMEWithECDSA(privateKey *ecdsa.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) (*SMIME, error) { - if privateKey == nil { - return nil, ErrPrivateKeyMissing - } - if certificate == nil { - return nil, ErrCertificateMissing - } - - return &SMIME{ - privateKey: privateKey, - certificate: certificate, - intermediateCertificate: intermediateCertificate, - }, nil -} - // signMessage signs the message with S/MIME func (s *SMIME) signMessage(message string) (string, error) { toBeSigned := bytes.NewBufferString(message) @@ -114,7 +94,7 @@ func (s *SMIME) prepareMessage(encoding Encoding, contentType ContentType, chars } // encodeMessage encodes the message with the given encoding -func (sm *SMIME) encodeMessage(encoding Encoding, message []byte) (string, error) { +func (s *SMIME) encodeMessage(encoding Encoding, message []byte) (string, error) { if encoding != EncodingQP { return string(message), nil } From f4537e6d28fff15473e9492baaead727ae18445a Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Mon, 6 Jan 2025 17:18:47 +0100 Subject: [PATCH 22/47] Rename parameter for clarity in S/MIME signing function. Updated the parameter name from `intermediateCertificate` to `intermediateCert` for improved readability and consistency. This change ensures better alignment with naming conventions and enhances code maintainability. --- msg.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/msg.go b/msg.go index ada4f96a..d0bd3018 100644 --- a/msg.go +++ b/msg.go @@ -2529,12 +2529,13 @@ func (m *Msg) addAddr(header AddrHeader, addr string) error { // Parameters: // - privateKey: The RSA private key used for signing. // - certificate: The x509 certificate associated with the private key. -// - intermediateCertificate: An optional intermediate x509 certificate for chain validation. +// - intermediateCert: An optional intermediate x509 certificate for chain validation. // // Returns: // - An error if any issue occurred while configuring S/MIME signing; otherwise nil. -func (m *Msg) SignWithKeypair(privateKey crypto.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) error { - smime, err := newSMIME(privateKey, certificate, intermediateCertificate) +func (m *Msg) SignWithKeypair(privateKey crypto.PrivateKey, certificate *x509.Certificate, + intermediateCert *x509.Certificate) error { + smime, err := newSMIME(privateKey, certificate, intermediateCert) if err != nil { return err } From 210d116fda17cc1f56c36ef7a6cb55c56fcb9907 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Mon, 6 Jan 2025 17:19:08 +0100 Subject: [PATCH 23/47] Refactor S/MIME struct and update related documentation. Refactored field names in the S/MIME struct for consistency and clarity (e.g., `intermediateCertificate` to `intermediateCert`). Updated function comments to improve detail and readability, ensuring better guidance for developers. Adjusted tests to align with renamed fields. --- smime.go | 100 +++++++++++++++++++++++++++++++++++++++++--------- smime_test.go | 2 +- 2 files changed, 83 insertions(+), 19 deletions(-) diff --git a/smime.go b/smime.go index f596d19c..3b81ced8 100644 --- a/smime.go +++ b/smime.go @@ -24,18 +24,36 @@ var ( ErrCertificateMissing = errors.New("certificate is missing") ) -// SMIME is used to sign messages with S/MIME +// SMIME represents the configuration used to sign messages with S/MIME. +// +// This struct encapsulates the private key, certificate, and optional intermediate certificate +// required for S/MIME signing. +// +// Fields: +// - privateKey: The private key used for signing (implements crypto.PrivateKey). +// - certificate: The x509 certificate associated with the private key. +// - intermediateCert: An optional x509 intermediate certificate for chain validation. +// - isSigned: A flag indicating whether the S/MIME signing is enabled. type SMIME struct { - privateKey crypto.PrivateKey - certificate *x509.Certificate - intermediateCertificate *x509.Certificate - isSigned bool + privateKey crypto.PrivateKey + certificate *x509.Certificate + intermediateCert *x509.Certificate + isSigned bool } -// newSMIMEWithRSA construct a new instance of SMIME with provided parameters -// privateKey as *rsa.PrivateKey -// certificate as *x509.Certificate -// intermediateCertificate (optional) as *x509.Certificate +// newSMIME constructs a new instance of SMIME with the provided parameters. +// +// This function initializes an SMIME object with a private key, certificate, and an optional +// intermediate certificate. +// +// Parameters: +// - privateKey: The private key used for signing (must implement crypto.PrivateKey). +// - certificate: The x509 certificate associated with the private key. +// - intermediateCert: An optional x509 intermediate certificate for chain validation. +// +// Returns: +// - An SMIME instance configured with the provided parameters. +// - An error if the private key or certificate is missing. func newSMIME(privateKey crypto.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) (*SMIME, error) { if privateKey == nil { return nil, ErrPrivateKeyMissing @@ -45,13 +63,25 @@ func newSMIME(privateKey crypto.PrivateKey, certificate *x509.Certificate, inter } return &SMIME{ - privateKey: privateKey, - certificate: certificate, - intermediateCertificate: intermediateCertificate, + privateKey: privateKey, + certificate: certificate, + intermediateCert: intermediateCertificate, }, nil } -// signMessage signs the message with S/MIME +// signMessage signs the given message with S/MIME. +// +// This function uses the configured private key and certificate to create an S/MIME signature +// for the provided message. It optionally includes an intermediate certificate for chain validation. +// The resulting signature is returned in PEM format. +// +// Parameters: +// - message: The string content of the message to be signed. +// +// Returns: +// - A string containing the S/MIME signature in PEM format. +// - An error if any step in the signing process fails, such as initializing signed data, adding a signer, +// or encoding the signature. func (s *SMIME) signMessage(message string) (string, error) { toBeSigned := bytes.NewBufferString(message) signedData, err := pkcs7.NewSignedData(toBeSigned.Bytes()) @@ -63,8 +93,8 @@ func (s *SMIME) signMessage(message string) (string, error) { return "", fmt.Errorf("could not add signer message: %w", err) } - if s.intermediateCertificate != nil { - signedData.AddCertificate(s.intermediateCertificate) + if s.intermediateCert != nil { + signedData.AddCertificate(s.intermediateCert) } signedData.Detach() @@ -82,7 +112,20 @@ func (s *SMIME) signMessage(message string) (string, error) { return pemMsg, nil } -// prepareMessage prepares the message that will be used for the sign method later +// prepareMessage prepares the message for signing with S/MIME. +// +// This function formats the message body with the specified encoding, content type, and character set, +// creating a structured message suitable for S/MIME signing. +// +// Parameters: +// - encoding: The encoding format to apply to the message body. +// - contentType: The content type of the message body. +// - charset: The character set used in the message body. +// - body: The byte slice representing the message body to be prepared. +// +// Returns: +// - A string containing the prepared message in the appropriate format. +// - An error if the message encoding process fails. func (s *SMIME) prepareMessage(encoding Encoding, contentType ContentType, charset Charset, body []byte) (string, error) { encodedMessage, err := s.encodeMessage(encoding, body) if err != nil { @@ -93,7 +136,18 @@ func (s *SMIME) prepareMessage(encoding Encoding, contentType ContentType, chars return preparedMessage, nil } -// encodeMessage encodes the message with the given encoding +// encodeMessage encodes the message using the specified encoding. +// +// This function applies the given encoding format to the message. If the encoding is not +// Quoted-Printable (QP), the original message is returned as a string. +// +// Parameters: +// - encoding: The encoding format to apply (e.g., Quoted-Printable). +// - message: The byte slice representing the message to be encoded. +// +// Returns: +// - A string containing the encoded message. +// - An error if the encoding process fails. func (s *SMIME) encodeMessage(encoding Encoding, message []byte) (string, error) { if encoding != EncodingQP { return string(message), nil @@ -111,7 +165,17 @@ func (s *SMIME) encodeMessage(encoding Encoding, message []byte) (string, error) return encodedMessage, nil } -// encodeToPEM uses the method pem.Encode from the standard library but cuts the typical PEM preamble +// encodeToPEM encodes the message to PEM format while removing the typical PEM preamble. +// +// This function uses the standard library's pem.Encode method to convert the message to PEM format. +// It then removes the PEM preamble and footer for a customized output. +// +// Parameters: +// - msg: The byte slice representing the message to be encoded. +// +// Returns: +// - A string containing the encoded message in PEM format without the typical preamble and footer. +// - An error if the encoding process fails. func encodeToPEM(msg []byte) (string, error) { block := &pem.Block{Bytes: msg} buffer := bytes.NewBuffer(nil) diff --git a/smime_test.go b/smime_test.go index c728608e..9f042993 100644 --- a/smime_test.go +++ b/smime_test.go @@ -50,7 +50,7 @@ func TestNewSMIME(t *testing.T) { if sMime.certificate != certificate { t.Errorf("NewSMime() did not return the same certificate") } - if sMime.intermediateCertificate != intermediateCertificate { + if sMime.intermediateCert != intermediateCertificate { t.Errorf("NewSMime() did not return the same intermedidate certificate") } }) From 4f9f9da883e7909e46c5d1da3ae3a075412acd45 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Mon, 6 Jan 2025 17:42:43 +0100 Subject: [PATCH 24/47] Refactor SMIME tests to consolidate and improve coverage Merged redundant SMIME test cases and grouped them into structured subtests. Improved error handling and checks for various configurations, such as missing keys or certificates, while maintaining compatibility with Go 1.23 changes. This enhances readability, reduces duplication, and ensures comprehensive validation. --- msg_test.go | 214 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 127 insertions(+), 87 deletions(-) diff --git a/msg_test.go b/msg_test.go index 5b9c29bb..22a4bec9 100644 --- a/msg_test.go +++ b/msg_test.go @@ -6667,107 +6667,145 @@ func TestMsg_fileFromIOFS(t *testing.T) { }) } -// TestSignWithSMime_ValidRSAKeyPair tests WithSMIMESigning with given rsa key pair -func TestSignWithSMime_ValidRSAKeyPair(t *testing.T) { - privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() +func TestMsg_SignWithKeypair(t *testing.T) { + rsaPrivKey, rsaCert, rsaIntermediate, err := getDummyRSACryptoMaterial() if err != nil { - t.Errorf("failed to laod dummy crypto material. Cause: %v", err) - } - m := NewMsg() - if err := m.SignWithKeypair(privateKey, certificate, intermediateCertificate); err != nil { - t.Errorf("failed to set sMIME. Cause: %v", err) - } - if m.sMIME.privateKey == nil { - t.Errorf("WithSMIMESigning() - no private key is given") - } - if m.sMIME.certificate == nil { - t.Errorf("WithSMIMESigning() - no certificate is given") + t.Fatalf("failed to load dummy crypto material: %s", err) } -} - -// TestSignWithSMime_ValidRSAKeyPair tests WithSMIMESigning with given ecdsa key pair -func TestSignWithSMime_ValidECDSAKeyPair(t *testing.T) { - privateKey, certificate, intermediateCertificate, err := getDummyECDSACryptoMaterial() + ecdsaPrivKey, ecdsaCert, ecdsaIntermediate, err := getDummyRSACryptoMaterial() if err != nil { - t.Errorf("failed to laod dummy crypto material. Cause: %v", err) - } - m := NewMsg() - if err := m.SignWithKeypair(privateKey, certificate, intermediateCertificate); err != nil { - t.Errorf("failed to set sMIME. Cause: %v", err) - } - if m.sMIME.privateKey == nil { - t.Errorf("WithSMIMESigning() - no private key is given") - } - if m.sMIME.certificate == nil { - t.Errorf("WithSMIMESigning() - no certificate is given") + t.Fatalf("failed to load dummy crypto material: %s", err) } -} -// TestSignWithTLSCertificate tests SignWithTLSCertificate with given *tls.Certificate -func TestSignWithTLSCertificate(t *testing.T) { - keyPairTLS, err := getDummyKeyPairTLS() - if err != nil { - t.Errorf("failed to laod dummy crypto material. Cause: %v", err) - } - m := NewMsg() - if err := m.SignWithTLSCertificate(keyPairTLS); err != nil { - t.Errorf("failed to set sMIME. Cause: %v", err) - } - if m.sMIME.privateKey == nil { - t.Errorf("SignWithTLSCertificate() - no private key is given") - } - if m.sMIME.certificate == nil { - t.Errorf("SignWithTLSCertificate() - no certificate is given") - } + t.Run("init signing with valid RSA keypair", func(t *testing.T) { + msg := testMessage(t) + if err = msg.SignWithKeypair(rsaPrivKey, rsaCert, rsaIntermediate); err != nil { + t.Errorf("failed to initialize SMIME configuration: %s", err) + } + if msg.sMIME == nil { + t.Errorf("failed to initialize SMIME configuration, SMIME is nil") + } + if msg.sMIME.privateKey == nil { + t.Errorf("failed to initialize SMIME configuration, private key is nil") + } + if msg.sMIME.certificate == nil { + t.Errorf("failed to initialize SMIME configuration, certificate is nil") + } + if msg.sMIME.intermediateCert == nil { + t.Errorf("failed to initialize SMIME configuration, intermediate certificate is nil") + } + }) + t.Run("init signing with valid ECDSA keypair", func(t *testing.T) { + msg := testMessage(t) + if err = msg.SignWithKeypair(ecdsaPrivKey, ecdsaCert, ecdsaIntermediate); err != nil { + t.Errorf("failed to initialize SMIME configuration: %s", err) + } + if msg.sMIME == nil { + t.Errorf("failed to initialize SMIME configuration, SMIME is nil") + } + if msg.sMIME.privateKey == nil { + t.Errorf("failed to initialize SMIME configuration, private key is nil") + } + if msg.sMIME.certificate == nil { + t.Errorf("failed to initialize SMIME configuration, certificate is nil") + } + if msg.sMIME.intermediateCert == nil { + t.Errorf("failed to initialize SMIME configuration, intermediate certificate is nil") + } + }) + t.Run("init signing with missing private key should fail", func(t *testing.T) { + msg := testMessage(t) + err = msg.SignWithKeypair(nil, rsaCert, rsaIntermediate) + if err == nil { + t.Errorf("SMIME init with missing private key should fail") + } + if !errors.Is(err, ErrPrivateKeyMissing) { + t.Errorf("SMIME init with missing private key should fail with %s, but got: %s", ErrPrivateKeyMissing, + err) + } + if msg.sMIME != nil { + t.Errorf("SMIME init with missing private key should fail, but SMIME is not nil") + } + }) + t.Run("init signing with missing public key should fail", func(t *testing.T) { + msg := testMessage(t) + err = msg.SignWithKeypair(rsaPrivKey, nil, rsaIntermediate) + if err == nil { + t.Errorf("SMIME init with missing public key should fail") + } + if !errors.Is(err, ErrCertificateMissing) { + t.Errorf("SMIME init with missing public key should fail with %s, but got: %s", ErrCertificateMissing, + err) + } + if msg.sMIME != nil { + t.Errorf("SMIME init with missing public key should fail, but SMIME is not nil") + } + }) + t.Run("init signing with missing intermediate cert should succeed", func(t *testing.T) { + msg := testMessage(t) + if err = msg.SignWithKeypair(rsaPrivKey, rsaCert, nil); err != nil { + t.Errorf("failed to initialize SMIME configuration: %s", err) + } + if msg.sMIME == nil { + t.Errorf("failed to initialize SMIME configuration, SMIME is nil") + } + if msg.sMIME.privateKey == nil { + t.Errorf("failed to initialize SMIME configuration, private key is nil") + } + if msg.sMIME.certificate == nil { + t.Errorf("failed to initialize SMIME configuration, certificate is nil") + } + if msg.sMIME.intermediateCert != nil { + t.Errorf("failed to initialize SMIME configuration, intermediate certificate is not nil") + } + }) } -// TestSignWithTLSCertificate tests SignWithTLSCertificate with given *tls.Certificate and nil leaf certificate -// PLEASE NOTE: Before Go 1.23 Certificate.Leaf was left nil, and the parsed certificate was -// discarded. This behavior can be re-enabled by setting "x509keypairleaf=0" -// in the GODEBUG environment variable. -func TestSignWithTLSCertificate_WithKeyPairLeafNil(t *testing.T) { - t.Setenv("GODEBUG", "x509keypairleaf=0") - - keyPairTLS, err := getDummyKeyPairTLS() +func TestMsg_SignWithTLSCertificate(t *testing.T) { + keypair, err := getDummyKeyPairTLS() if err != nil { - t.Errorf("failed to laod dummy crypto material. Cause: %v", err) - } - m := NewMsg() - if err := m.SignWithTLSCertificate(keyPairTLS); err != nil { - t.Errorf("failed to set sMIME. Cause: %v", err) - } - if m.sMIME.privateKey == nil { - t.Errorf("SignWithTLSCertificate() - no private key is given") - } - if m.sMIME.certificate == nil { - t.Errorf("SignWithTLSCertificate() - no certificate is given") + t.Fatalf("failed to load dummy crypto material: %s", err) } -} -// TestSignWithSMime_InvalidPrivateKey tests WithSMIMESigning with given invalid private key -func TestSignWithSMime_InvalidPrivateKey(t *testing.T) { - m := NewMsg() + t.Run("init signing with valid TLS certificate", func(t *testing.T) { + msg := testMessage(t) + if err = msg.SignWithTLSCertificate(keypair); err != nil { + t.Errorf("failed to initialize SMIME configuration: %s", err) + } + if msg.sMIME.privateKey == nil { + t.Errorf("failed to initialize SMIME configuration, private key is nil") + } + if msg.sMIME.certificate == nil { + t.Errorf("failed to initialize SMIME configuration, certificate is nil") + } + if msg.sMIME.intermediateCert == nil { + t.Errorf("failed to initialize SMIME configuration, intermediate certificate is nil") + } + }) - err := m.SignWithKeypair(nil, nil, nil) - if !errors.Is(err, ErrPrivateKeyMissing) { - t.Errorf("failed to pre-check SignWithSMime method values correctly: %s", err) - } -} + // Before Go 1.23 Certificate.Leaf was left nil, and the parsed certificate was discarded. This behavior + // can be re-enabled by setting "x509keypairleaf=0" in the GODEBUG environment variable. + // See: https://go-review.googlesource.com/c/go/+/585856 + t.Run("init signing with valid TLS certificate with nil leaf", func(t *testing.T) { + t.Setenv("GODEBUG", "x509keypairleaf=0") -// TestSignWithSMime_InvalidCertificate tests WithSMIMESigning with given invalid certificate -func TestSignWithSMime_InvalidCertificate(t *testing.T) { - privateKey, _, _, err := getDummyRSACryptoMaterial() - if err != nil { - t.Errorf("failed to laod dummy crypto material. Cause: %v", err) - } - m := NewMsg() - - err = m.SignWithKeypair(privateKey, nil, nil) - if !errors.Is(err, ErrCertificateMissing) { - t.Errorf("failed to pre-check SignWithSMime method values correctly: %s", err) - } + msg := testMessage(t) + if err = msg.SignWithTLSCertificate(keypair); err != nil { + t.Errorf("failed to initialize SMIME configuration: %s", err) + } + if msg.sMIME.privateKey == nil { + t.Errorf("failed to initialize SMIME configuration, private key is nil") + } + if msg.sMIME.certificate == nil { + t.Errorf("failed to initialize SMIME configuration, certificate is nil") + } + if msg.sMIME.intermediateCert == nil { + t.Errorf("failed to initialize SMIME configuration, intermediate certificate is nil") + } + }) } +/* // TestMsg_createSignaturePart tests the Msg.createSignaturePart method func TestMsg_createSignaturePart(t *testing.T) { privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() @@ -6875,6 +6913,8 @@ func TestGetLeafCertificate(t *testing.T) { } } +*/ + // uppercaseMiddleware is a middleware type that transforms the subject to uppercase. type uppercaseMiddleware struct{} From 1a0953a3a093d0195ed329369d8819c5c30da5ee Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Mon, 6 Jan 2025 19:17:05 +0100 Subject: [PATCH 25/47] Refactor `getLeafCertificate` and improve SMIME tests Move `getLeafCertificate` from `msg.go` to `smime.go` for better organization and reusability. Add new test cases to validate SMIME signing edge cases, including nil or malformed TLS certificates and unsupported private keys. --- msg.go | 34 --------------------------- msg_test.go | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++ smime.go | 35 +++++++++++++++++++++++++++ smime_test.go | 11 +++++---- 4 files changed, 106 insertions(+), 39 deletions(-) diff --git a/msg.go b/msg.go index d0bd3018..ceca6088 100644 --- a/msg.go +++ b/msg.go @@ -3051,40 +3051,6 @@ func getEncoder(enc Encoding) mime.WordEncoder { } } -// getLeafCertificate retrieves the leaf certificate from a tls.Certificate. -// -// This function returns the parsed leaf certificate from the provided TLS certificate. If the Leaf field -// is nil, it parses and returns the first certificate in the chain. -// -// PLEASE NOTE: In Go versions prior to 1.23, the Certificate.Leaf field was left nil, and the parsed -// certificate was discarded. This behavior can be re-enabled by setting "x509keypairleaf=0" in the -// GODEBUG environment variable. -// -// Parameters: -// - keyPairTlS: The *tls.Certificate containing the certificate chain. -// -// Returns: -// - The parsed leaf x509 certificate. -// - An error if the certificate could not be parsed. -func getLeafCertificate(keyPairTlS *tls.Certificate) (*x509.Certificate, error) { - if keyPairTlS == nil { - return nil, errors.New("provided certificate is nil") - } - if keyPairTlS.Leaf != nil { - return keyPairTlS.Leaf, nil - } - - if len(keyPairTlS.Certificate) == 0 { - return nil, errors.New("certificate chain is empty") - } - cert, err := x509.ParseCertificate(keyPairTlS.Certificate[0]) - if err != nil { - return nil, err - } - - return cert, nil -} - // signMessage signs the Msg with S/MIME. // // This function creates an S/MIME signature for the first part of the message and appends the diff --git a/msg_test.go b/msg_test.go index 22a4bec9..12031e26 100644 --- a/msg_test.go +++ b/msg_test.go @@ -7,6 +7,8 @@ package mail import ( "bytes" "context" + "crypto" + "crypto/tls" "embed" "errors" "fmt" @@ -6803,6 +6805,56 @@ func TestMsg_SignWithTLSCertificate(t *testing.T) { t.Errorf("failed to initialize SMIME configuration, intermediate certificate is nil") } }) + + t.Run("init signing with nil TLS certificate should fail", func(t *testing.T) { + msg := testMessage(t) + if err = msg.SignWithTLSCertificate(nil); err == nil { + t.Errorf("SMIME initilization with nil TLS certificate should fail") + } + }) + t.Run("init signing with invalid intermediate certificate should fail", func(t *testing.T) { + msg := testMessage(t) + localpair := &tls.Certificate{ + Certificate: keypair.Certificate, + PrivateKey: keypair.PrivateKey, + Leaf: keypair.Leaf, + SignedCertificateTimestamps: keypair.SignedCertificateTimestamps, + SupportedSignatureAlgorithms: keypair.SupportedSignatureAlgorithms, + } + if len(localpair.Certificate) != 2 { + t.Fatal("expected certificate with intermediate certificate") + } + localpair.Certificate[1] = localpair.Certificate[1][:len(localpair.Certificate[1])-16] + if err = msg.SignWithTLSCertificate(localpair); err == nil { + t.Errorf("SMIME initilization with invalid intermediate certificate should fail") + } + }) + t.Run("init signing with empty tls.Certificate should fail on leaf certificate creation", func(t *testing.T) { + msg := testMessage(t) + localpair := &tls.Certificate{} + if err = msg.SignWithTLSCertificate(localpair); err == nil { + t.Errorf("SMIME initilization with invalid intermediate certificate should fail") + } + }) + t.Run("init signing with unsupported crypto.PrivateKey should fail", func(t *testing.T) { + msg := testMessage(t) + localpair := &tls.Certificate{ + Certificate: [][]byte{keypair.Certificate[0]}, + PrivateKey: unsupportedPrivKey{}, + Leaf: keypair.Leaf, + SignedCertificateTimestamps: keypair.SignedCertificateTimestamps, + SupportedSignatureAlgorithms: keypair.SupportedSignatureAlgorithms, + } + if err = msg.SignWithTLSCertificate(localpair); err == nil { + t.Errorf("SMIME initilization with invalid intermediate certificate should fail") + } + expErr := "unsupported private key type: mail.unsupportedPrivKey" + if !strings.EqualFold(err.Error(), expErr) { + t.Errorf("SMIME initilization with invalid intermediate certificate should fail with %s, but got: %s", + expErr, err) + } + }) + } /* @@ -7009,6 +7061,19 @@ func checkAddrHeader(t *testing.T, message *Msg, header AddrHeader, fn string, f } } +// unsupportedPrivKey is a type that satisfies the crypto.PrivateKey interfaces but is not supported for signing +type unsupportedPrivKey struct{} + +// Public implements the Public method of the crypto.PrivateKey interface for the unsupportedPrivKey type +func (u unsupportedPrivKey) Public() crypto.PublicKey { + return nil +} + +// Equal implements the Equal method of the crypto.PrivateKey interface for the unsupportedPrivKey type +func (u unsupportedPrivKey) Equal(_ crypto.PrivateKey) bool { + return true +} + // checkGenHeader validates the generated header in an email message, verifying its presence and expected values. func checkGenHeader(t *testing.T, message *Msg, header Header, fn string, field, wantFields int, wantVal string, diff --git a/smime.go b/smime.go index 3b81ced8..2ffebc8c 100644 --- a/smime.go +++ b/smime.go @@ -7,6 +7,7 @@ package mail import ( "bytes" "crypto" + "crypto/tls" "crypto/x509" "encoding/pem" "errors" @@ -184,3 +185,37 @@ func encodeToPEM(msg []byte) (string, error) { } return buffer.String()[17 : buffer.Len()-16], nil } + +// getLeafCertificate retrieves the leaf certificate from a tls.Certificate. +// +// This function returns the parsed leaf certificate from the provided TLS certificate. If the Leaf field +// is nil, it parses and returns the first certificate in the chain. +// +// PLEASE NOTE: In Go versions prior to 1.23, the Certificate.Leaf field was left nil, and the parsed +// certificate was discarded. This behavior can be re-enabled by setting "x509keypairleaf=0" in the +// GODEBUG environment variable. +// +// Parameters: +// - keyPairTlS: The *tls.Certificate containing the certificate chain. +// +// Returns: +// - The parsed leaf x509 certificate. +// - An error if the certificate could not be parsed. +func getLeafCertificate(keyPairTLS *tls.Certificate) (*x509.Certificate, error) { + if keyPairTLS == nil { + return nil, errors.New("provided certificate is nil") + } + if keyPairTLS.Leaf != nil { + return keyPairTLS.Leaf, nil + } + + if len(keyPairTLS.Certificate) == 0 { + return nil, errors.New("certificate chain is empty") + } + cert, err := x509.ParseCertificate(keyPairTLS.Certificate[0]) + if err != nil { + return nil, err + } + + return cert, nil +} diff --git a/smime_test.go b/smime_test.go index 9f042993..bebf9852 100644 --- a/smime_test.go +++ b/smime_test.go @@ -17,10 +17,11 @@ import ( ) const ( - dummyCertRSAPath = "testdata/dummy-chain-cert-rsa.pem" - dummyKeyRSAPath = "testdata/dummy-child-key-rsa.pem" - dummyCertECDSAPath = "testdata/dummy-chain-cert-ecdsa.pem" - dummyKeyECDSAPath = "testdata/dummy-child-key-ecdsa.pem" + dummyCertRSAPath = "testdata/dummy-chain-cert-rsa.pem" + dummyCertRSABrokenPath = "testdata/dummy-chain-cert-rsa-broken.pem" + dummyKeyRSAPath = "testdata/dummy-child-key-rsa.pem" + dummyCertECDSAPath = "testdata/dummy-chain-cert-ecdsa.pem" + dummyKeyECDSAPath = "testdata/dummy-child-key-ecdsa.pem" ) // TestNewSMimeWithRSA tests the newSMime method with RSA crypto material @@ -265,7 +266,7 @@ func getDummyECDSACryptoMaterial() (crypto.PrivateKey, *x509.Certificate, *x509. // getDummyKeyPairTLS loads a certificate (ECDSA) as *tls.Certificate, the associated private key and certificate (ECDSA) is loaded from local disk for testing purposes func getDummyKeyPairTLS() (*tls.Certificate, error) { - keyPair, err := tls.LoadX509KeyPair(dummyCertECDSAPath, dummyKeyECDSAPath) + keyPair, err := tls.LoadX509KeyPair(dummyCertRSAPath, dummyKeyRSAPath) if err != nil { return nil, err } From 4288a5074f660a0319518b95109d14de40374cf6 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Mon, 6 Jan 2025 19:17:33 +0100 Subject: [PATCH 26/47] `Remove unused test data constants in smime_test.go` Unused constants related to broken RSA certificates were removed to simplify the code. This cleanup enhances code readability and maintains alignment with the test's actual requirements. --- smime_test.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/smime_test.go b/smime_test.go index bebf9852..cd3f7e5d 100644 --- a/smime_test.go +++ b/smime_test.go @@ -17,11 +17,10 @@ import ( ) const ( - dummyCertRSAPath = "testdata/dummy-chain-cert-rsa.pem" - dummyCertRSABrokenPath = "testdata/dummy-chain-cert-rsa-broken.pem" - dummyKeyRSAPath = "testdata/dummy-child-key-rsa.pem" - dummyCertECDSAPath = "testdata/dummy-chain-cert-ecdsa.pem" - dummyKeyECDSAPath = "testdata/dummy-child-key-ecdsa.pem" + dummyCertRSAPath = "testdata/dummy-chain-cert-rsa.pem" + dummyKeyRSAPath = "testdata/dummy-child-key-rsa.pem" + dummyCertECDSAPath = "testdata/dummy-chain-cert-ecdsa.pem" + dummyKeyECDSAPath = "testdata/dummy-child-key-ecdsa.pem" ) // TestNewSMimeWithRSA tests the newSMime method with RSA crypto material From 492325433ce8f282645d79df9dfaa2a9695e3970 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Tue, 7 Jan 2025 11:55:19 +0100 Subject: [PATCH 27/47] Reorder headers in test cases for consistency. Swapped the order of "Content-Type" and "Content-Transfer-Encoding" headers in test cases to align with expected structure. This ensures the tests reflect the accurate header sequence for better reliability. --- quicksend_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/quicksend_test.go b/quicksend_test.go index 497e2a02..bf38c658 100644 --- a/quicksend_test.go +++ b/quicksend_test.go @@ -112,8 +112,8 @@ func TestQuickSend(t *testing.T) { {30, "Subject: " + subject}, {33, "From: "}, {34, "To: "}, - {35, "Content-Type: text/plain; charset=UTF-8"}, - {36, "Content-Transfer-Encoding: quoted-printable"}, + {35, "Content-Transfer-Encoding: quoted-printable"}, + {36, "Content-Type: text/plain; charset=UTF-8"}, {38, "This is a test body"}, {39, "With multiple lines"}, {40, ""}, @@ -171,8 +171,8 @@ func TestQuickSend(t *testing.T) { {34, "Subject: " + subject}, {37, "From: "}, {38, "To: , , "}, - {39, "Content-Type: text/plain; charset=UTF-8"}, - {40, "Content-Transfer-Encoding: quoted-printable"}, + {39, "Content-Transfer-Encoding: quoted-printable"}, + {40, "Content-Type: text/plain; charset=UTF-8"}, {42, "This is a test body"}, {43, "With multiple lines"}, {44, ""}, From 600a793c4152ad4e557cd278a33a7719acd2de8c Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Tue, 7 Jan 2025 11:55:56 +0100 Subject: [PATCH 28/47] Remove unused PKCS7 methods: GetOnlySigner and UnmarshalSignedAttribute This commit removes the `GetOnlySigner` and `UnmarshalSignedAttribute` methods from the PKCS7 struct as they are no longer used. This cleanup simplifies the codebase by eliminating unnecessary functionality. --- internal/pkcs7/pkcs7.go | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/internal/pkcs7/pkcs7.go b/internal/pkcs7/pkcs7.go index 4c48d5d8..ca9c4fc6 100644 --- a/internal/pkcs7/pkcs7.go +++ b/internal/pkcs7/pkcs7.go @@ -56,29 +56,6 @@ type PKCS7 struct { raw interface{} } -// GetOnlySigner returns an x509.Certificate for the first signer of the signed -// data payload. If there are more or less than one signer, nil is returned -func (p7 *PKCS7) GetOnlySigner() *x509.Certificate { - if len(p7.Signers) != 1 { - return nil - } - signer := p7.Signers[0] - return getCertFromCertsByIssuerAndSerial(p7.Certificates, signer.IssuerAndSerialNumber) -} - -// UnmarshalSignedAttribute decodes a single attribute from the signer info -func (p7 *PKCS7) UnmarshalSignedAttribute(attributeType asn1.ObjectIdentifier, out interface{}) error { - sd, ok := p7.raw.(signedData) - if !ok { - return errors.New("pkcs7: payload is not signedData content") - } - if len(sd.SignerInfos) < 1 { - return errors.New("pkcs7: payload has no signers") - } - attributes := sd.SignerInfos[0].AuthenticatedAttributes - return unmarshalAttribute(attributes, attributeType, out) -} - type contentInfo struct { ContentType asn1.ObjectIdentifier Content asn1.RawValue `asn1:"explicit,optional,tag:0"` From 77d27aa940ac77054de6cd49bbdbe1fb4ca99c1d Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Tue, 7 Jan 2025 11:59:37 +0100 Subject: [PATCH 29/47] Refactor and simplify S/MIME signing flow Refactored the S/MIME signing process to streamline logic, improve maintainability, and reduce code duplication. There was a general logic flaw in the whole SMIME signing code. It would only work with quoted-printable and raw encodings, while base64 and others would fail. Instead of forcing to render the message body into QP, we no perform a pre-signing render run of the whole mail which will output all message parts in proper formatting and encoding. The result of this pre-sign run is then used for the signature. It currently doesn't work with multipart messages yet, but we now make use of all available go-mail tools to get the message parts. This makes a bunch of methods in the smime.go obsolete. Removed unused and redundant methods, added safeguards for in-progress signing, and improved boundary generation for multipart messages. Updated existing tests to align with the new implementation. --- msg.go | 81 ++++++++++------------------ msg_test.go | 78 ++++++++++++++++++++------- msgwriter.go | 26 ++++++--- smime.go | 91 ++------------------------------ smime_test.go | 142 +------------------------------------------------- 5 files changed, 113 insertions(+), 305 deletions(-) diff --git a/msg.go b/msg.go index ceca6088..c4852480 100644 --- a/msg.go +++ b/msg.go @@ -2219,15 +2219,17 @@ func (m *Msg) WriteTo(writer io.Writer) (int64, error) { msg := m.applyMiddlewares(m) if m.hasSMIME() { - signedMsg, err := m.signMessage(msg) - if err != nil { + if err := m.signMessage(); err != nil { return 0, err } - msg = signedMsg } mw.writeMsg(msg) + if m.hasSMIME() { + m.sMIME.isSigned = true + } + return mw.bytesWritten, mw.err } @@ -2646,11 +2648,11 @@ func (m *Msg) encodeString(str string) string { func (m *Msg) hasAlt() bool { count := 0 for _, part := range m.parts { - if !part.isDeleted { + if !part.isDeleted && !part.smime { count++ } } - return count > 1 && m.pgptype == 0 && !m.hasSMIME() + return count > 1 && m.pgptype == 0 } // hasMixed returns true if the Msg has mixed parts. @@ -2675,7 +2677,11 @@ func (m *Msg) hasMixed() bool { // Returns: // - true if the Msg has SMIME type assigned and should be signed with S/MIME; otherwise false. func (m *Msg) hasSMIME() bool { - return m.sMIME != nil + return m.sMIME != nil && !m.sMIME.isSigned +} + +func (m *Msg) isSMIMEInProgress() bool { + return m.sMIME != nil && m.sMIME.inProgress } // hasRelated returns true if the Msg has related parts. @@ -2773,36 +2779,6 @@ func (m *Msg) checkUserAgent() { } } -// createSignaturePart creates a Part for storing the S/MIME signature of the given body. -// -// This function generates an S/MIME signature for the provided message body and wraps it in a Part -// object that can be included in the email message. -// -// Parameters: -// - encoding: The encoding format to use for the signature. -// - contentType: The content type of the message body being signed. -// - charSet: The character set used in the message body. -// - body: The byte array representing the message body to be signed. -// -// Returns: -// - A Part containing the S/MIME signature. -// - An error if the message preparation or signing process fails. -func (m *Msg) createSignaturePart(encoding Encoding, contentType ContentType, charSet Charset, body []byte) (*Part, error) { - message, err := m.sMIME.prepareMessage(encoding, contentType, charSet, body) - if err != nil { - return nil, err - } - signedMessage, err := m.sMIME.signMessage(message) - if err != nil { - return nil, err - } - - signaturePart := m.newPart(TypeSMIMESigned, WithPartEncoding(EncodingB64), WithSMIMESigning()) - signaturePart.SetContent(signedMessage) - - return signaturePart, nil -} - // addDefaultHeader sets default headers if they haven't been set before. // // This method ensures that essential headers such as "Date", "Message-ID", and "MIME-Version" are set @@ -3062,32 +3038,31 @@ func getEncoder(enc Encoding) mime.WordEncoder { // Returns: // - The Msg with the appended S/MIME signature part. // - An error if retrieving content, creating the signature part, or signing fails. -func (m *Msg) signMessage(msg *Msg) (*Msg, error) { - if msg.sMIME.isSigned { - return msg, nil +func (m *Msg) signMessage() error { + if m.sMIME.isSigned { + return nil } - parts := msg.GetParts() - if len(parts) == 0 { - return msg, errors.New("message has no parts") - } + buf := bytes.NewBuffer(nil) + mw := &msgWriter{writer: buf, charset: m.charset, encoder: m.encoder} + m.sMIME.inProgress = true + mw.writeMsg(m) - signedPart := parts[0] - body, err := signedPart.GetContent() - if err != nil { - return nil, fmt.Errorf("failed to get message body: %w", err) + index := bytes.Index(buf.Bytes(), []byte("Content-Transfer-Encoding:")) + if index == -1 { + index = 0 } - - signaturePart, err := m.createSignaturePart(signedPart.GetEncoding(), signedPart.GetContentType(), - signedPart.GetCharset(), body) + signedMessage, err := m.sMIME.signMessage(buf.Bytes()[index:]) if err != nil { - return nil, err + return fmt.Errorf("failed to sign message: %w", err) } + signaturePart := m.newPart(TypeSMIMESigned, WithPartEncoding(EncodingB64), WithSMIMESigning()) + signaturePart.SetContent(signedMessage) m.parts = append(m.parts, signaturePart) - m.sMIME.isSigned = true + m.sMIME.inProgress = false - return m, err + return nil } // writeFuncFromBuffer converts a byte buffer into a writeFunc, which is commonly required by go-mail. diff --git a/msg_test.go b/msg_test.go index 12031e26..a5188c54 100644 --- a/msg_test.go +++ b/msg_test.go @@ -6400,23 +6400,6 @@ func TestMsg_hasAlt(t *testing.T) { }) } -// TestMsg_hasAlt tests the hasAlt() method of the Msg with active S/MIME -func TestMsg_hasAltWithSMime(t *testing.T) { - privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() - if err != nil { - t.Fatalf("failed to load dummy crypto material: %s", err) - } - m := NewMsg() - m.SetBodyString(TypeTextPlain, "Plain") - m.AddAlternativeString(TypeTextHTML, "HTML") - if err := m.SignWithKeypair(privateKey, certificate, intermediateCertificate); err != nil { - t.Errorf("failed to init smime configuration") - } - if m.hasAlt() { - t.Errorf("mail has alternative parts and S/MIME is active, but hasAlt() returned true") - } -} - // TestMsg_hasSMime tests the hasSMIME() method of the Msg func TestMsg_hasSMime(t *testing.T) { privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() @@ -6805,7 +6788,6 @@ func TestMsg_SignWithTLSCertificate(t *testing.T) { t.Errorf("failed to initialize SMIME configuration, intermediate certificate is nil") } }) - t.Run("init signing with nil TLS certificate should fail", func(t *testing.T) { msg := testMessage(t) if err = msg.SignWithTLSCertificate(nil); err == nil { @@ -6857,6 +6839,66 @@ func TestMsg_SignWithTLSCertificate(t *testing.T) { } +func TestMsg_createSignaturePart(t *testing.T) { + /* + rsaPrivKey, rsaCert, rsaIntermediate, err := getDummyRSACryptoMaterial() + if err != nil { + t.Fatalf("failed to load dummy crypto material: %s", err) + } + ecdsaPrivKey, ecdsaCert, ecdsaIntermediate, err := getDummyRSACryptoMaterial() + if err != nil { + t.Fatalf("failed to load dummy crypto material: %s", err) + } + + */ + /* + t.Run("create signature with valid RSA certificate", func(t *testing.T) { + msg := testMessage(t) + if err = msg.SignWithKeypair(rsaPrivKey, rsaCert, rsaIntermediate); err != nil { + t.Fatalf("failed to initialize SMIME configuration: %s", err) + } + + parts := msg.GetParts() + if len(parts) != 1 { + t.Fatalf("expected 1 body part, got %d", len(parts)) + } + body, err := parts[0].GetContent() + if err != nil { + t.Fatalf("failed to get body part content: %s", err) + } + + signedPart, err := msg.createSignaturePart(parts[0].GetEncoding(), parts[0].GetContentType(), + parts[0].GetCharset(), body) + if err != nil { + t.Fatalf("failed to create part signatures: %s", err) + } + if signedPart == nil { + t.Fatal("failed to create part signatures, returned part is nil") + } + if signedPart.GetEncoding() != EncodingB64 { + t.Errorf("failed to sign body part, expected encoding: %s, got: %s", EncodingB64, signedPart.GetEncoding()) + } + if signedPart.GetContentType() != TypeSMIMESigned { + t.Errorf("failed to sign body part, expected content type: %s, got: %s", TypeSMIMESigned, + signedPart.GetContentType()) + } + if signedPart.GetCharset() != CharsetUTF8 { + t.Errorf("failed to sign body part, expected charset: %s, got: %s", CharsetUTF8, signedPart.GetCharset()) + } + signedBody, err := signedPart.GetContent() + if err != nil { + t.Fatalf("failed to get signed body part content: %s", err) + } + if len(signedBody) == len(body) { + t.Errorf("failed to sign body part, expected different length, body: %d, signed: %d", len(body), + len(signedBody)) + } + //t.Logf("signed body: %s", signedBody) + }) + + */ +} + /* // TestMsg_createSignaturePart tests the Msg.createSignaturePart method func TestMsg_createSignaturePart(t *testing.T) { diff --git a/msgwriter.go b/msgwriter.go index 4de4067a..68b19bb2 100644 --- a/msgwriter.go +++ b/msgwriter.go @@ -6,6 +6,7 @@ package mail import ( "bytes" + "crypto/rand" "encoding/base64" "fmt" "io" @@ -128,8 +129,8 @@ func (mw *msgWriter) writeMsg(msg *Msg) { } } - if msg.hasSMIME() { - mw.startMP(MIMESMIMESigned, msg.boundary) + if msg.hasSMIME() && !msg.isSMIMEInProgress() { + mw.startMP(MIMESMIMESigned, randomBoundary()) mw.writeString(DoubleNewLine) } if msg.hasMixed() { @@ -179,7 +180,7 @@ func (mw *msgWriter) writeMsg(msg *Msg) { mw.stopMP() } - if msg.hasSMIME() { + if msg.hasSMIME() && !msg.isSMIMEInProgress() { mw.stopMP() } } @@ -196,6 +197,7 @@ func (mw *msgWriter) writeGenHeader(msg *Msg) { for key := range msg.genHeader { keys = append(keys, string(key)) } + sort.Strings(keys) for _, key := range keys { mw.writeHeader(Header(key), msg.genHeader[Header(key)]...) @@ -360,11 +362,11 @@ func (mw *msgWriter) writePart(part *Part, charset Charset) { if part.IsSMIMESigned() { contentType = part.contentType.String() } - contentTransferEnc := part.encoding.String() + if mw.depth == 0 { - mw.writeHeader(HeaderContentType, contentType) mw.writeHeader(HeaderContentTransferEnc, contentTransferEnc) + mw.writeHeader(HeaderContentType, contentType) mw.writeString(SingleNewLine) } if mw.depth > 0 { @@ -372,8 +374,8 @@ func (mw *msgWriter) writePart(part *Part, charset Charset) { if part.description != "" { mimeHeader.Add(string(HeaderContentDescription), part.description) } - mimeHeader.Add(string(HeaderContentType), contentType) mimeHeader.Add(string(HeaderContentTransferEnc), contentTransferEnc) + mimeHeader.Add(string(HeaderContentType), contentType) mw.newPart(mimeHeader) } mw.writeBody(part.writeFunc, part.encoding, part.smime) @@ -469,7 +471,7 @@ func (mw *msgWriter) writeBody(writeFunc func(io.Writer) (int64, error), encodin // the S/MIME part is already Base64 encoded, we don't want to double-encode if signSMIME { - encoding = NoEncoding + //encoding = NoEncoding } switch encoding { @@ -518,6 +520,16 @@ func (mw *msgWriter) writeBody(writeFunc func(io.Writer) (int64, error), encodin } } +// randomBoundary +func randomBoundary() string { + var buf [30]byte + _, err := io.ReadFull(rand.Reader, buf[:]) + if err != nil { + panic(err) + } + return fmt.Sprintf("%x", buf[:]) +} + // sanitizeFilename sanitizes a given filename string by replacing specific unwanted characters with // an underscore ('_'). // diff --git a/smime.go b/smime.go index 2ffebc8c..fead8a91 100644 --- a/smime.go +++ b/smime.go @@ -5,14 +5,11 @@ package mail import ( - "bytes" "crypto" "crypto/tls" "crypto/x509" - "encoding/pem" "errors" "fmt" - "mime/quotedprintable" "github.com/wneessen/go-mail/internal/pkcs7" ) @@ -34,11 +31,12 @@ var ( // - privateKey: The private key used for signing (implements crypto.PrivateKey). // - certificate: The x509 certificate associated with the private key. // - intermediateCert: An optional x509 intermediate certificate for chain validation. -// - isSigned: A flag indicating whether the S/MIME signing is enabled. +// - signatureWritten: A flag indicating whether the S/MIME the signature has already been written type SMIME struct { privateKey crypto.PrivateKey certificate *x509.Certificate intermediateCert *x509.Certificate + inProgress bool isSigned bool } @@ -83,9 +81,8 @@ func newSMIME(privateKey crypto.PrivateKey, certificate *x509.Certificate, inter // - A string containing the S/MIME signature in PEM format. // - An error if any step in the signing process fails, such as initializing signed data, adding a signer, // or encoding the signature. -func (s *SMIME) signMessage(message string) (string, error) { - toBeSigned := bytes.NewBufferString(message) - signedData, err := pkcs7.NewSignedData(toBeSigned.Bytes()) +func (s *SMIME) signMessage(message []byte) (string, error) { + signedData, err := pkcs7.NewSignedData(message) if err != nil || signedData == nil { return "", fmt.Errorf("failed to initialize signed data: %w", err) } @@ -105,85 +102,7 @@ func (s *SMIME) signMessage(message string) (string, error) { return "", fmt.Errorf("failed to finish signature: %w", err) } - pemMsg, err := encodeToPEM(signatureDER) - if err != nil { - return "", fmt.Errorf("could not encode to PEM: %w", err) - } - - return pemMsg, nil -} - -// prepareMessage prepares the message for signing with S/MIME. -// -// This function formats the message body with the specified encoding, content type, and character set, -// creating a structured message suitable for S/MIME signing. -// -// Parameters: -// - encoding: The encoding format to apply to the message body. -// - contentType: The content type of the message body. -// - charset: The character set used in the message body. -// - body: The byte slice representing the message body to be prepared. -// -// Returns: -// - A string containing the prepared message in the appropriate format. -// - An error if the message encoding process fails. -func (s *SMIME) prepareMessage(encoding Encoding, contentType ContentType, charset Charset, body []byte) (string, error) { - encodedMessage, err := s.encodeMessage(encoding, body) - if err != nil { - return "", err - } - preparedMessage := fmt.Sprintf("Content-Transfer-Encoding: %s\r\nContent-Type: %s; charset=%s\r\n\r\n%s", - encoding, contentType, charset, encodedMessage) - return preparedMessage, nil -} - -// encodeMessage encodes the message using the specified encoding. -// -// This function applies the given encoding format to the message. If the encoding is not -// Quoted-Printable (QP), the original message is returned as a string. -// -// Parameters: -// - encoding: The encoding format to apply (e.g., Quoted-Printable). -// - message: The byte slice representing the message to be encoded. -// -// Returns: -// - A string containing the encoded message. -// - An error if the encoding process fails. -func (s *SMIME) encodeMessage(encoding Encoding, message []byte) (string, error) { - if encoding != EncodingQP { - return string(message), nil - } - - buffer := bytes.NewBuffer(nil) - writer := quotedprintable.NewWriter(buffer) - if _, err := writer.Write(message); err != nil { - return "", err - } - if err := writer.Close(); err != nil { - return "", err - } - encodedMessage := buffer.String() - return encodedMessage, nil -} - -// encodeToPEM encodes the message to PEM format while removing the typical PEM preamble. -// -// This function uses the standard library's pem.Encode method to convert the message to PEM format. -// It then removes the PEM preamble and footer for a customized output. -// -// Parameters: -// - msg: The byte slice representing the message to be encoded. -// -// Returns: -// - A string containing the encoded message in PEM format without the typical preamble and footer. -// - An error if the encoding process fails. -func encodeToPEM(msg []byte) (string, error) { - block := &pem.Block{Bytes: msg} - buffer := bytes.NewBuffer(nil) - if err := pem.Encode(buffer, block); err != nil { - return "", err - } - return buffer.String()[17 : buffer.Len()-16], nil + return string(signatureDER), nil } // getLeafCertificate retrieves the leaf certificate from a tls.Certificate. diff --git a/smime_test.go b/smime_test.go index cd3f7e5d..456dca87 100644 --- a/smime_test.go +++ b/smime_test.go @@ -10,9 +10,6 @@ import ( "crypto/rsa" "crypto/tls" "crypto/x509" - "encoding/base64" - "fmt" - "strings" "testing" ) @@ -70,7 +67,7 @@ func TestSign(t *testing.T) { } message := "This is a test message" - singedMessage, err := sMime.signMessage(message) + singedMessage, err := sMime.signMessage([]byte(message)) if err != nil { t.Errorf("Error creating singed message: %s", err) } @@ -80,143 +77,6 @@ func TestSign(t *testing.T) { } } -// TestPrepareMessage tests the createMessage method -func TestPrepareMessage(t *testing.T) { - privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() - if err != nil { - t.Errorf("Error getting dummy crypto material: %s", err) - } - - sMime, err := newSMIME(privateKey, certificate, intermediateCertificate) - if err != nil { - t.Errorf("Error creating new SMIME from keyPair: %s", err) - } - - encoding := EncodingB64 - contentType := TypeTextPlain - charset := CharsetUTF8 - body := []byte("This is the body!") - result, err := sMime.prepareMessage(encoding, contentType, charset, body) - if err != nil { - t.Errorf("Error preparing message: %s", err) - } - - if !strings.Contains(result, encoding.String()) { - t.Errorf("prepareMessage() did not return the correct encoding") - } - if !strings.Contains(result, contentType.String()) { - t.Errorf("prepareMessage() did not return the correct contentType") - } - if !strings.Contains(result, string(body)) { - t.Errorf("prepareMessage() did not return the correct body") - } - if result != fmt.Sprintf("Content-Transfer-Encoding: %s\r\nContent-Type: %s; charset=%s\r\n\r\n%s", encoding, contentType, charset, string(body)) { - t.Errorf("prepareMessage() did not sucessfully create the message") - } -} - -// TestPrepareMessage_QuotedPrintable tests the prepareMessage method with quoted printable encoding -func TestPrepareMessage_QuotedPrintable(t *testing.T) { - privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() - if err != nil { - t.Errorf("Error getting dummy crypto material: %s", err) - } - - sMime, err := newSMIME(privateKey, certificate, intermediateCertificate) - if err != nil { - t.Errorf("Error creating new SMIME from keyPair: %s", err) - } - - body := "This is the body with special chars like äöü ÄÖÜ ß!" - quotedPrintableBody := "This is the body with special chars like =C3=A4=C3=B6=C3=BC =C3=84=C3=96=C3=\r\n=9C =C3=9F!" - encoding := EncodingQP - contentType := TypeTextPlain - charset := CharsetUTF8 - result, err := sMime.prepareMessage(encoding, contentType, charset, []byte(body)) - if err != nil { - t.Errorf("Error preparing message: %s", err) - } - - if !strings.Contains(result, encoding.String()) { - t.Errorf("prepareMessage() did not return the correct encoding") - } - if !strings.Contains(result, contentType.String()) { - t.Errorf("prepareMessage() did not return the correct contentType") - } - if !strings.Contains(result, quotedPrintableBody) { - t.Errorf("prepareMessage() did not return the correct body") - } - if result != fmt.Sprintf("Content-Transfer-Encoding: %s\r\nContent-Type: %s; charset=%s\r\n\r\n%s", encoding, contentType, charset, quotedPrintableBody) { - t.Errorf("prepareMessage() did not sucessfully create the message") - } -} - -// TestEncodeMessage tests the TestEncodeMessage method without any encoding -func TestEncodeMessage(t *testing.T) { - body := "This is the body with special chars like äöü ÄÖÜ ß!" - encoding := EncodingUSASCII - - privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() - if err != nil { - t.Errorf("Error getting dummy crypto material: %s", err) - } - - sMime, err := newSMIME(privateKey, certificate, intermediateCertificate) - if err != nil { - t.Errorf("Error creating new SMIME from keyPair: %s", err) - } - - result, err := sMime.encodeMessage(encoding, []byte(body)) - if err != nil { - t.Errorf("Error preparing message: %s", err) - } - - if result != body { - t.Errorf("encodeMessage() did not return the correct encoded message: %s", result) - } -} - -// TestEncodeMessage_QuotedPrintable tests the TestEncodeMessage method with quoted printable body -func TestEncodeMessage_QuotedPrintable(t *testing.T) { - body := "This is the body with special chars like äöü ÄÖÜ ß!" - quotedPrintableBody := "This is the body with special chars like =C3=A4=C3=B6=C3=BC =C3=84=C3=96=C3=\r\n=9C =C3=9F!" - encoding := EncodingQP - - privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() - if err != nil { - t.Errorf("Error getting dummy crypto material: %s", err) - } - - sMime, err := newSMIME(privateKey, certificate, intermediateCertificate) - if err != nil { - t.Errorf("Error creating new SMIME from keyPair: %s", err) - } - - result, err := sMime.encodeMessage(encoding, []byte(body)) - if err != nil { - t.Errorf("Error preparing message: %s", err) - } - - if result != quotedPrintableBody { - t.Errorf("encodeMessage() did not return the correct encoded message: %s", result) - } -} - -// TestEncodeToPEM tests the encodeToPEM method -func TestEncodeToPEM(t *testing.T) { - message := []byte("This is a test message") - - pemMessage, err := encodeToPEM(message) - if err != nil { - t.Errorf("Error encoding message: %s", err) - } - - base64Encoded := base64.StdEncoding.EncodeToString(message) - if pemMessage != base64Encoded { - t.Errorf("encodeToPEM() did not work") - } -} - // getDummyRSACryptoMaterial loads a certificate (RSA), the associated private key and certificate (RSA) is loaded // from local disk for testing purposes func getDummyRSACryptoMaterial() (crypto.PrivateKey, *x509.Certificate, *x509.Certificate, error) { From 589c81fe9e4eeae79a0092aded664dff276c0b9d Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Tue, 7 Jan 2025 21:26:18 +0100 Subject: [PATCH 30/47] Refactor S/MIME signing to improve header processing Add `headerCount` to track header lines for accurate parsing and signing. Introduced boundary setting and refined message handling to ensure correct header offsets. Included debug logs for unsigned message data to aid troubleshooting. --- msg.go | 21 ++++++++++++++++----- msgwriter.go | 4 ++++ smime.go | 3 ++- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/msg.go b/msg.go index c4852480..6689129c 100644 --- a/msg.go +++ b/msg.go @@ -152,7 +152,8 @@ type Msg struct { noDefaultUserAgent bool // sMIME holds a SMIME type to sign a Msg using S/MIME - sMIME *SMIME + sMIME *SMIME + headerCount uint } // SendmailPath is the default system path to the sendmail binary - at least on standard Unix-like OS. @@ -3046,13 +3047,23 @@ func (m *Msg) signMessage() error { buf := bytes.NewBuffer(nil) mw := &msgWriter{writer: buf, charset: m.charset, encoder: m.encoder} m.sMIME.inProgress = true + m.SetBoundary(randomBoundary()) mw.writeMsg(m) - index := bytes.Index(buf.Bytes(), []byte("Content-Transfer-Encoding:")) - if index == -1 { - index = 0 + fmt.Printf("Header count: %d\n", m.headerCount) + var linecount uint = 0 + pos := 0 + for linecount < m.headerCount { + nextIndex := bytes.Index(buf.Bytes()[pos:], []byte("\r\n")) + if nextIndex == -1 { + return fmt.Errorf("failed to find next index") + } + pos += nextIndex + 2 + linecount++ } - signedMessage, err := m.sMIME.signMessage(buf.Bytes()[index:]) + m.headerCount = 0 + //index := bytes.Index(buf.Bytes(), []byte("\r\n")) + signedMessage, err := m.sMIME.signMessage(buf.Bytes()[pos:]) if err != nil { return fmt.Errorf("failed to sign message: %w", err) } diff --git a/msgwriter.go b/msgwriter.go index 68b19bb2..06c3f49a 100644 --- a/msgwriter.go +++ b/msgwriter.go @@ -116,6 +116,7 @@ func (mw *msgWriter) writeMsg(msg *Msg) { } if hasFrom && (len(from) > 0 && from[0] != nil) { mw.writeHeader(Header(HeaderFrom), from[0].String()) + msg.headerCount++ } // Set the rest of the address headers @@ -126,6 +127,7 @@ func (mw *msgWriter) writeMsg(msg *Msg) { val = append(val, addr.String()) } mw.writeHeader(Header(to), val...) + msg.headerCount++ } } @@ -201,6 +203,7 @@ func (mw *msgWriter) writeGenHeader(msg *Msg) { sort.Strings(keys) for _, key := range keys { mw.writeHeader(Header(key), msg.genHeader[Header(key)]...) + msg.headerCount++ } } @@ -214,6 +217,7 @@ func (mw *msgWriter) writeGenHeader(msg *Msg) { func (mw *msgWriter) writePreformattedGenHeader(msg *Msg) { for key, val := range msg.preformHeader { mw.writeString(fmt.Sprintf("%s: %s%s", key, val, SingleNewLine)) + msg.headerCount++ } } diff --git a/smime.go b/smime.go index fead8a91..79a7f352 100644 --- a/smime.go +++ b/smime.go @@ -82,6 +82,7 @@ func newSMIME(privateKey crypto.PrivateKey, certificate *x509.Certificate, inter // - An error if any step in the signing process fails, such as initializing signed data, adding a signer, // or encoding the signature. func (s *SMIME) signMessage(message []byte) (string, error) { + fmt.Printf("unsigned data: %s\n", string(message)) signedData, err := pkcs7.NewSignedData(message) if err != nil || signedData == nil { return "", fmt.Errorf("failed to initialize signed data: %w", err) @@ -95,7 +96,7 @@ func (s *SMIME) signMessage(message []byte) (string, error) { signedData.AddCertificate(s.intermediateCert) } - signedData.Detach() + //signedData.Detach() signatureDER, err := signedData.Finish() if err != nil { From d3713e25f3447dab5238e0d20f435badf1333ed1 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 11:44:15 +0100 Subject: [PATCH 31/47] Refactor S/MIME signing process in email rendering Removed redundant flags and improved handling of S/MIME state during message signing to streamline the process and avoid duplicate signatures. Added `headerCount` to accurately determine the boundary between headers and body. Updated multipart handling to respect S/MIME signatures and fixed message boundary consistency. We are now able to S/MIME sign all kind of messages, including multipart messages and attachments. --- msg.go | 69 ++++++++++++++++++++++++++++++++-------------------- msgwriter.go | 19 ++++++++++++--- smime.go | 30 ++++++++++------------- 3 files changed, 70 insertions(+), 48 deletions(-) diff --git a/msg.go b/msg.go index 6689129c..5ba3a088 100644 --- a/msg.go +++ b/msg.go @@ -116,6 +116,10 @@ type Msg struct { // representing header values. genHeader map[Header][]string + // headerCount is an indicate for how many headers have been written during the mail rendering process. + // This count can be helpful to identify where the mail header ends and the mail body starts + headerCount uint + // isDelivered indicates wether the Msg has been delivered. isDelivered bool @@ -152,8 +156,7 @@ type Msg struct { noDefaultUserAgent bool // sMIME holds a SMIME type to sign a Msg using S/MIME - sMIME *SMIME - headerCount uint + sMIME *SMIME } // SendmailPath is the default system path to the sendmail binary - at least on standard Unix-like OS. @@ -2226,11 +2229,7 @@ func (m *Msg) WriteTo(writer io.Writer) (int64, error) { } mw.writeMsg(msg) - - if m.hasSMIME() { - m.sMIME.isSigned = true - } - + m.headerCount = 0 return mw.bytesWritten, mw.err } @@ -2678,7 +2677,7 @@ func (m *Msg) hasMixed() bool { // Returns: // - true if the Msg has SMIME type assigned and should be signed with S/MIME; otherwise false. func (m *Msg) hasSMIME() bool { - return m.sMIME != nil && !m.sMIME.isSigned + return m.sMIME != nil } func (m *Msg) isSMIMEInProgress() bool { @@ -3028,50 +3027,66 @@ func getEncoder(enc Encoding) mime.WordEncoder { } } -// signMessage signs the Msg with S/MIME. +// signMessage signs the message with S/MIME and attaches the signature as a new part. // -// This function creates an S/MIME signature for the first part of the message and appends the -// signature as an additional part to the Msg. -// -// Parameters: -// - msg: The Msg to be signed. +// This function removes any existing S/MIME parts to prevent duplicate signatures, renders an +// unsigned version of the message, and then signs it. The resulting signature is appended to the +// message as a new S/MIME signature part. // // Returns: -// - The Msg with the appended S/MIME signature part. -// - An error if retrieving content, creating the signature part, or signing fails. +// - An error if any step in the signing process fails, such as finding the message body position +// or generating the signature. func (m *Msg) signMessage() error { - if m.sMIME.isSigned { - return nil + // To avoid attaching double signatures (i. e. if WriteTo is called multiple times) + // we remove any present smime part before signing the mail + parts := make([]*Part, 0) + for _, part := range m.parts { + if !part.smime { + parts = append(parts, part) + } } + m.parts = parts + // We need to indicate that we are in the signing process + m.sMIME.inProgress = true + defer func() { + m.sMIME.inProgress = false + }() + + // We render an unsigned version of the mail into a buffer so we can use it for + // the S/MIME signature buf := bytes.NewBuffer(nil) mw := &msgWriter{writer: buf, charset: m.charset, encoder: m.encoder} - m.sMIME.inProgress = true - m.SetBoundary(randomBoundary()) + + // If no boundary is set by the user, we set a fixed random boundary, so that + // the boundary of the parts is consistant during the different rendering + // phases + if m.boundary == "" { + m.SetBoundary(randomBoundary()) + } mw.writeMsg(m) - fmt.Printf("Header count: %d\n", m.headerCount) + // Since we only want to sign the message body, we need to find the position within + // the mail body from where we start reading. var linecount uint = 0 - pos := 0 + var pos = 0 for linecount < m.headerCount { nextIndex := bytes.Index(buf.Bytes()[pos:], []byte("\r\n")) if nextIndex == -1 { - return fmt.Errorf("failed to find next index") + return errors.New("unable to find message body starting index within rendered message") } pos += nextIndex + 2 linecount++ } - m.headerCount = 0 - //index := bytes.Index(buf.Bytes(), []byte("\r\n")) + + // Sign the message and attach a new smime signature part to the mail signedMessage, err := m.sMIME.signMessage(buf.Bytes()[pos:]) if err != nil { return fmt.Errorf("failed to sign message: %w", err) } - signaturePart := m.newPart(TypeSMIMESigned, WithPartEncoding(EncodingB64), WithSMIMESigning()) signaturePart.SetContent(signedMessage) m.parts = append(m.parts, signaturePart) - m.sMIME.inProgress = false return nil } diff --git a/msgwriter.go b/msgwriter.go index 06c3f49a..6c0c6d9d 100644 --- a/msgwriter.go +++ b/msgwriter.go @@ -137,15 +137,21 @@ func (mw *msgWriter) writeMsg(msg *Msg) { } if msg.hasMixed() { mw.startMP(MIMEMixed, msg.boundary) - mw.writeString(DoubleNewLine) + if mw.depth == 0 || (msg.hasSMIME() && mw.depth == 1) { + mw.writeString(DoubleNewLine) + } } if msg.hasRelated() { mw.startMP(MIMERelated, msg.boundary) - mw.writeString(DoubleNewLine) + if mw.depth == 0 || (msg.hasSMIME() && mw.depth == 1) { + mw.writeString(DoubleNewLine) + } } if msg.hasAlt() { mw.startMP(MIMEAlternative, msg.boundary) - mw.writeString(DoubleNewLine) + if mw.depth == 0 || (msg.hasSMIME() && mw.depth == 1) { + mw.writeString(DoubleNewLine) + } } if msg.hasPGPType() { switch msg.pgptype { @@ -161,7 +167,7 @@ func (mw *msgWriter) writeMsg(msg *Msg) { } for _, part := range msg.parts { - if !part.isDeleted { + if !part.isDeleted && !part.smime { mw.writePart(part, msg.charset) } } @@ -183,6 +189,11 @@ func (mw *msgWriter) writeMsg(msg *Msg) { } if msg.hasSMIME() && !msg.isSMIMEInProgress() { + for _, part := range msg.parts { + if part.smime { + mw.writePart(part, msg.charset) + } + } mw.stopMP() } } diff --git a/smime.go b/smime.go index 79a7f352..130e968a 100644 --- a/smime.go +++ b/smime.go @@ -22,22 +22,21 @@ var ( ErrCertificateMissing = errors.New("certificate is missing") ) -// SMIME represents the configuration used to sign messages with S/MIME. +// SMIME represents the configuration and state for S/MIME signing. // -// This struct encapsulates the private key, certificate, and optional intermediate certificate -// required for S/MIME signing. +// This struct encapsulates the private key, certificate, optional intermediate certificate, and +// a flag indicating whether a signing process is currently in progress. // // Fields: // - privateKey: The private key used for signing (implements crypto.PrivateKey). // - certificate: The x509 certificate associated with the private key. // - intermediateCert: An optional x509 intermediate certificate for chain validation. -// - signatureWritten: A flag indicating whether the S/MIME the signature has already been written +// - inProgress: A boolean flag indicating if a signing operation is currently active. type SMIME struct { privateKey crypto.PrivateKey certificate *x509.Certificate intermediateCert *x509.Certificate inProgress bool - isSigned bool } // newSMIME constructs a new instance of SMIME with the provided parameters. @@ -53,7 +52,8 @@ type SMIME struct { // Returns: // - An SMIME instance configured with the provided parameters. // - An error if the private key or certificate is missing. -func newSMIME(privateKey crypto.PrivateKey, certificate *x509.Certificate, intermediateCertificate *x509.Certificate) (*SMIME, error) { +func newSMIME(privateKey crypto.PrivateKey, certificate *x509.Certificate, + intermediateCertificate *x509.Certificate) (*SMIME, error) { if privateKey == nil { return nil, ErrPrivateKeyMissing } @@ -68,21 +68,18 @@ func newSMIME(privateKey crypto.PrivateKey, certificate *x509.Certificate, inter }, nil } -// signMessage signs the given message with S/MIME. +// signMessage signs the provided message with S/MIME and returns the signature in DER format. // -// This function uses the configured private key and certificate to create an S/MIME signature -// for the provided message. It optionally includes an intermediate certificate for chain validation. -// The resulting signature is returned in PEM format. +// This function creates S/MIME signed data using the configured private key and certificate. +// It optionally includes an intermediate certificate for chain validation and detaches the signature. // // Parameters: -// - message: The string content of the message to be signed. +// - message: The byte slice representing the message to be signed. // // Returns: -// - A string containing the S/MIME signature in PEM format. -// - An error if any step in the signing process fails, such as initializing signed data, adding a signer, -// or encoding the signature. +// - A string containing the S/MIME signature in DER format. +// - An error if initializing signed data, adding the signer, or finishing the signature fails. func (s *SMIME) signMessage(message []byte) (string, error) { - fmt.Printf("unsigned data: %s\n", string(message)) signedData, err := pkcs7.NewSignedData(message) if err != nil || signedData == nil { return "", fmt.Errorf("failed to initialize signed data: %w", err) @@ -96,8 +93,7 @@ func (s *SMIME) signMessage(message []byte) (string, error) { signedData.AddCertificate(s.intermediateCert) } - //signedData.Detach() - + signedData.Detach() signatureDER, err := signedData.Finish() if err != nil { return "", fmt.Errorf("failed to finish signature: %w", err) From f20b96a59c8b1620031aef76a3d68e90b3bb93ed Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 11:45:23 +0100 Subject: [PATCH 32/47] Remove unused functions from pkcs7.go Deleted `getCertFromCertsByIssuerAndSerial` and `unmarshalAttribute` as they were no longer used in the codebase. This reduces unnecessary code and improves maintainability. --- internal/pkcs7/pkcs7.go | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/internal/pkcs7/pkcs7.go b/internal/pkcs7/pkcs7.go index ca9c4fc6..fbf7f56d 100644 --- a/internal/pkcs7/pkcs7.go +++ b/internal/pkcs7/pkcs7.go @@ -414,25 +414,6 @@ func marshalAttributes(attrs []attribute) ([]byte, error) { return raw.Bytes, nil } -func getCertFromCertsByIssuerAndSerial(certs []*x509.Certificate, ias issuerAndSerial) *x509.Certificate { - for _, cert := range certs { - if isCertMatchForIssuerAndSerial(cert, ias) { - return cert - } - } - return nil -} - func isCertMatchForIssuerAndSerial(cert *x509.Certificate, ias issuerAndSerial) bool { return cert.SerialNumber.Cmp(ias.SerialNumber) == 0 && bytes.Equal(cert.RawIssuer, ias.IssuerName.FullBytes) } - -func unmarshalAttribute(attrs []attribute, attributeType asn1.ObjectIdentifier, out interface{}) error { - for _, attr := range attrs { - if attr.Type.Equal(attributeType) { - _, err := asn1.Unmarshal(attr.Value.Bytes, out) - return err - } - } - return errors.New("pkcs7: attribute type not in attributes") -} From 7f9241a3db0b96bd679c2ecaaf0a1158a1f9f39a Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 11:46:19 +0100 Subject: [PATCH 33/47] Remove unused isCertMatchForIssuerAndSerial function The isCertMatchForIssuerAndSerial function was no longer referenced or used in the codebase. Removing it helps reduce dead code, improving maintainability and clarity. No functional changes were introduced. --- internal/pkcs7/pkcs7.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/pkcs7/pkcs7.go b/internal/pkcs7/pkcs7.go index fbf7f56d..355ec40c 100644 --- a/internal/pkcs7/pkcs7.go +++ b/internal/pkcs7/pkcs7.go @@ -413,7 +413,3 @@ func marshalAttributes(attrs []attribute) ([]byte, error) { } return raw.Bytes, nil } - -func isCertMatchForIssuerAndSerial(cert *x509.Certificate, ias issuerAndSerial) bool { - return cert.SerialNumber.Cmp(ias.SerialNumber) == 0 && bytes.Equal(cert.RawIssuer, ias.IssuerName.FullBytes) -} From 9483835aebb73a3c2df040d60f245620bc113a09 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 11:53:51 +0100 Subject: [PATCH 34/47] Remove redundant S/MIME handling and related code Refactored the S/MIME logic by removing unused `IsSMIMESigned` method, redundant parameters, and unnecessary checks. Updated relevant calls in `msgwriter` and tests to streamline code and improve maintainability. --- internal/pkcs7/pkcs7.go | 1 - msgwriter.go | 14 ++++---------- msgwriter_test.go | 10 +++++----- part.go | 10 ---------- part_test.go | 2 +- 5 files changed, 10 insertions(+), 27 deletions(-) diff --git a/internal/pkcs7/pkcs7.go b/internal/pkcs7/pkcs7.go index 355ec40c..d20343bf 100644 --- a/internal/pkcs7/pkcs7.go +++ b/internal/pkcs7/pkcs7.go @@ -53,7 +53,6 @@ type PKCS7 struct { Certificates []*x509.Certificate CRLs []x509.RevocationList Signers []signerInfo - raw interface{} } type contentInfo struct { diff --git a/msgwriter.go b/msgwriter.go index 6c0c6d9d..64e61f12 100644 --- a/msgwriter.go +++ b/msgwriter.go @@ -339,7 +339,7 @@ func (mw *msgWriter) addFiles(files []*File, isAttachment bool) { } if mw.err == nil { - mw.writeBody(file.Writer, encoding, false) + mw.writeBody(file.Writer, encoding) } } } @@ -374,7 +374,7 @@ func (mw *msgWriter) writePart(part *Part, charset Charset) { } contentType := fmt.Sprintf("%s; charset=%s", part.contentType, partCharset) - if part.IsSMIMESigned() { + if part.smime { contentType = part.contentType.String() } contentTransferEnc := part.encoding.String() @@ -393,7 +393,7 @@ func (mw *msgWriter) writePart(part *Part, charset Charset) { mimeHeader.Add(string(HeaderContentType), contentType) mw.newPart(mimeHeader) } - mw.writeBody(part.writeFunc, part.encoding, part.smime) + mw.writeBody(part.writeFunc, part.encoding) } // writeString writes a string into the msgWriter's io.Writer interface. @@ -468,8 +468,7 @@ func (mw *msgWriter) writeHeader(key Header, values ...string) { // Parameters: // - writeFunc: A function that writes the body content to the given io.Writer. // - encoding: The encoding type to use when writing the content (e.g., base64, quoted-printable). -// - signSMIME: Whether the msg should be signed with S/MIME. -func (mw *msgWriter) writeBody(writeFunc func(io.Writer) (int64, error), encoding Encoding, signSMIME bool) { +func (mw *msgWriter) writeBody(writeFunc func(io.Writer) (int64, error), encoding Encoding) { var writer io.Writer var encodedWriter io.WriteCloser var n int64 @@ -484,11 +483,6 @@ func (mw *msgWriter) writeBody(writeFunc func(io.Writer) (int64, error), encodin lineBreaker := Base64LineBreaker{} lineBreaker.out = &writeBuffer - // the S/MIME part is already Base64 encoded, we don't want to double-encode - if signSMIME { - //encoding = NoEncoding - } - switch encoding { case EncodingQP: encodedWriter = quotedprintable.NewWriter(&writeBuffer) diff --git a/msgwriter_test.go b/msgwriter_test.go index 70cb1693..ba13735b 100644 --- a/msgwriter_test.go +++ b/msgwriter_test.go @@ -679,7 +679,7 @@ func TestMsgWriter_writeBody(t *testing.T) { buffer := bytes.NewBuffer(nil) msgwriter.writer = buffer message := testMessage(t) - msgwriter.writeBody(message.parts[0].writeFunc, NoEncoding, false) + msgwriter.writeBody(message.parts[0].writeFunc, NoEncoding) if msgwriter.err != nil { t.Errorf("writeBody failed to write: %s", msgwriter.err) } @@ -687,7 +687,7 @@ func TestMsgWriter_writeBody(t *testing.T) { t.Run("writeBody on NoEncoding fails on write", func(t *testing.T) { msgwriter.writer = failReadWriteSeekCloser{} message := testMessage(t) - msgwriter.writeBody(message.parts[0].writeFunc, NoEncoding, false) + msgwriter.writeBody(message.parts[0].writeFunc, NoEncoding) if msgwriter.err == nil { t.Errorf("writeBody succeeded, expected error") } @@ -701,7 +701,7 @@ func TestMsgWriter_writeBody(t *testing.T) { writeFunc := func(io.Writer) (int64, error) { return 0, errors.New("intentional write failure") } - msgwriter.writeBody(writeFunc, NoEncoding, false) + msgwriter.writeBody(writeFunc, NoEncoding) if msgwriter.err == nil { t.Errorf("writeBody succeeded, expected error") } @@ -712,7 +712,7 @@ func TestMsgWriter_writeBody(t *testing.T) { t.Run("writeBody Quoted-Printable fails on write", func(t *testing.T) { msgwriter.writer = failReadWriteSeekCloser{} message := testMessage(t) - msgwriter.writeBody(message.parts[0].writeFunc, EncodingQP, false) + msgwriter.writeBody(message.parts[0].writeFunc, EncodingQP) if msgwriter.err == nil { t.Errorf("writeBody succeeded, expected error") } @@ -726,7 +726,7 @@ func TestMsgWriter_writeBody(t *testing.T) { writeFunc := func(io.Writer) (int64, error) { return 0, errors.New("intentional write failure") } - msgwriter.writeBody(writeFunc, EncodingQP, false) + msgwriter.writeBody(writeFunc, EncodingQP) if msgwriter.err == nil { t.Errorf("writeBody succeeded, expected error") } diff --git a/part.go b/part.go index e675f954..8a1320ce 100644 --- a/part.go +++ b/part.go @@ -95,16 +95,6 @@ func (p *Part) GetDescription() string { return p.description } -// IsSMIMESigned determines if the Part should be signed with S/MIME. -// -// This function checks whether the Part has been configured to use S/MIME for signing. -// -// Returns: -// - true if the Part should be signed with S/MIME; otherwise false. -func (p *Part) IsSMIMESigned() bool { - return p.smime -} - // SetContent overrides the content of the Part with the given string. // // This function sets the content of the Part by creating a new writeFunc that writes the diff --git a/part_test.go b/part_test.go index 5f8f3a5b..9f037916 100644 --- a/part_test.go +++ b/part_test.go @@ -282,7 +282,7 @@ func TestPart_IsSMimeSigned(t *testing.T) { return } pl[0].SetIsSMIMESigned(tt.want) - smime := pl[0].IsSMIMESigned() + smime := pl[0].smime if smime != tt.want { t.Errorf("SetContentType failed. Got: %v, expected: %v", smime, tt.want) } From b61fecede1e70c1b70ad1e3d66ceb60e428b6d17 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 11:54:51 +0100 Subject: [PATCH 35/47] Refactor function signature and variable declaration. Updated the SignWithKeypair function signature for improved formatting consistency and refactored the variable declaration in a loop to use shorthand syntax. These changes enhance code readability and adhere to Go's best practices. --- msg.go | 5 +++-- msg_test.go | 1 - smime.go | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/msg.go b/msg.go index 5ba3a088..22bceb33 100644 --- a/msg.go +++ b/msg.go @@ -2536,7 +2536,8 @@ func (m *Msg) addAddr(header AddrHeader, addr string) error { // Returns: // - An error if any issue occurred while configuring S/MIME signing; otherwise nil. func (m *Msg) SignWithKeypair(privateKey crypto.PrivateKey, certificate *x509.Certificate, - intermediateCert *x509.Certificate) error { + intermediateCert *x509.Certificate, +) error { smime, err := newSMIME(privateKey, certificate, intermediateCert) if err != nil { return err @@ -3069,7 +3070,7 @@ func (m *Msg) signMessage() error { // Since we only want to sign the message body, we need to find the position within // the mail body from where we start reading. var linecount uint = 0 - var pos = 0 + pos := 0 for linecount < m.headerCount { nextIndex := bytes.Index(buf.Bytes()[pos:], []byte("\r\n")) if nextIndex == -1 { diff --git a/msg_test.go b/msg_test.go index a5188c54..6c6f473d 100644 --- a/msg_test.go +++ b/msg_test.go @@ -6836,7 +6836,6 @@ func TestMsg_SignWithTLSCertificate(t *testing.T) { expErr, err) } }) - } func TestMsg_createSignaturePart(t *testing.T) { diff --git a/smime.go b/smime.go index 130e968a..dd267364 100644 --- a/smime.go +++ b/smime.go @@ -53,7 +53,8 @@ type SMIME struct { // - An SMIME instance configured with the provided parameters. // - An error if the private key or certificate is missing. func newSMIME(privateKey crypto.PrivateKey, certificate *x509.Certificate, - intermediateCertificate *x509.Certificate) (*SMIME, error) { + intermediateCertificate *x509.Certificate, +) (*SMIME, error) { if privateKey == nil { return nil, ErrPrivateKeyMissing } From 884cb8c3aae217bb5fc2e78e3b365d7102b58354 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 12:56:46 +0100 Subject: [PATCH 36/47] Refactor PKCS7 signing to use crypto.Signer interface Replaced algorithm-specific logic with a more general crypto.Signer implementation for signing. Removed the unused ErrUnsupportedAlgorithm error and simplified the PKCS7 signing process for better maintainability and flexibility. This allows for ECDSA support. --- internal/pkcs7/pkcs7.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/internal/pkcs7/pkcs7.go b/internal/pkcs7/pkcs7.go index d20343bf..8258efe3 100644 --- a/internal/pkcs7/pkcs7.go +++ b/internal/pkcs7/pkcs7.go @@ -44,9 +44,6 @@ var ( OIDEncryptionAlgorithmRSASHA256 = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 1, 11} ) -// ErrUnsupportedAlgorithm tells you when our quick dev assumptions have failed -var ErrUnsupportedAlgorithm = errors.New("pkcs7: cannot decrypt data: only RSA, DES, DES-EDE3, AES-256-CBC and AES-128-GCM supported") - // PKCS7 Represents a PKCS7 structure type PKCS7 struct { Content []byte @@ -368,11 +365,12 @@ func signAttributes(attrs []attribute, pkey crypto.PrivateKey, hash crypto.Hash) h := hash.New() h.Write(attrBytes) hashed := h.Sum(nil) - switch priv := pkey.(type) { - case *rsa.PrivateKey: - return rsa.SignPKCS1v15(rand.Reader, priv, crypto.SHA256, hashed) + + key, ok := pkey.(crypto.Signer) + if !ok { + return nil, errors.New("pkcs7: private key does not implement crypto.Signer") } - return nil, ErrUnsupportedAlgorithm + return key.Sign(rand.Reader, hashed, hash) } // concat and wraps the certificates in the RawValue structure From 30c1f7b289b7ee5b0fd3fec60b7e5a6b7181bec5 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 13:37:50 +0100 Subject: [PATCH 37/47] Add more tests for S/MIME signing support Add new test cases for S/MIME signing validation while removing outdated tests. Improve code structure for handling invalid signing scenarios. --- msg.go | 6 + msg_test.go | 331 +++++++++++++++++++++------------------------------- 2 files changed, 142 insertions(+), 195 deletions(-) diff --git a/msg.go b/msg.go index 22bceb33..45218a5d 100644 --- a/msg.go +++ b/msg.go @@ -2681,6 +2681,12 @@ func (m *Msg) hasSMIME() bool { return m.sMIME != nil } +// isSMIMEInProgress checks whether an S/MIME signing operation is currently in progress. +// +// This function verifies if the S/MIME configuration exists and if the signing process is active. +// +// Returns: +// - true if an S/MIME signing operation is in progress; otherwise false. func (m *Msg) isSMIMEInProgress() bool { return m.sMIME != nil && m.sMIME.inProgress } diff --git a/msg_test.go b/msg_test.go index 6c6f473d..0e2211e1 100644 --- a/msg_test.go +++ b/msg_test.go @@ -9,6 +9,7 @@ import ( "context" "crypto" "crypto/tls" + "crypto/x509" "embed" "errors" "fmt" @@ -5787,6 +5788,48 @@ func TestMsg_WriteTo(t *testing.T) { }) } }) + t.Run("WriteTo with S/MIME signing", func(t *testing.T) { + keypair, err := getDummyKeyPairTLS() + if err != nil { + t.Fatalf("failed to get dummy key material: %s", err) + } + message := testMessage(t) + if err = message.SignWithTLSCertificate(keypair); err != nil { + t.Fatalf("failed to initialize S/MIME signing: %s", err) + } + buffer := bytes.NewBuffer(nil) + if _, err = message.WriteTo(buffer); err != nil { + t.Errorf("failed to write S/MIME signed message to buffer: %s", err) + } + if !strings.Contains(buffer.String(), TypeSMIMESigned.String()) { + t.Error("expected message to be S/MIME signature Content-Type in mail body") + } + if !strings.Contains(buffer.String(), `application/pkcs7-signature; name="smime.p7s"`) { + t.Error("expected message to be S/MIME signature attachment in mail body") + } + }) + t.Run("WriteTo with S/MIME signing fails", func(t *testing.T) { + keypair, err := getDummyKeyPairTLS() + if err != nil { + t.Fatalf("failed to get dummy key material: %s", err) + } + message := testMessage(t) + if err = message.SignWithTLSCertificate(keypair); err != nil { + t.Fatalf("failed to initialize S/MIME signing: %s", err) + } + message.sMIME.privateKey = unsupportedPrivKey{} + + buffer := bytes.NewBuffer(nil) + _, err = message.WriteTo(buffer) + if err == nil { + t.Error("expected WritoTo with invalid S/MIME private key to fail") + } + expErr := "failed to sign message: could not add signer message: pkcs7: cannot convert encryption algorithm " + + "to oid, unknown private key type" + if !strings.Contains(err.Error(), expErr) { + t.Errorf("expected S/MIME signing error to be: %s, got: %s", expErr, err) + } + }) } func TestMsg_WriteToFile(t *testing.T) { @@ -6492,70 +6535,6 @@ func TestMsg_hasRelated(t *testing.T) { }) } -// TestMsg_WriteToWithSMIME tests the WriteTo() method of the Msg -func TestMsg_WriteToWithSMIME(t *testing.T) { - privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() - if err != nil { - t.Fatalf("failed to load dummy crypto material: %s", err) - } - - m := NewMsg() - m.Subject("This is a test") - m.SetBodyString(TypeTextPlain, "Plain") - if err := m.SignWithKeypair(privateKey, certificate, intermediateCertificate); err != nil { - t.Errorf("failed to init smime configuration") - } - - wbuf := bytes.Buffer{} - if _, err = m.WriteTo(&wbuf); err != nil { - t.Errorf("WriteTo() failed: %s", err) - } - - result := wbuf.String() - boundary := result[strings.LastIndex(result, "--")-60 : strings.LastIndex(result, "--")] - if strings.Count(result, boundary) != 4 { - t.Errorf("WriteTo() failed. False number of boundaries found") - } - - parts := strings.Split(result, fmt.Sprintf("--%s", boundary)) - if len(parts) != 4 { - t.Errorf("WriteTo() failed. False number of parts found") - } - - preamble := parts[0] - if !strings.Contains(preamble, "MIME-Version: 1.0") { - t.Errorf("WriteTo() failed. Unable to find MIME-Version") - } - if !strings.Contains(preamble, "Subject: This is a test") { - t.Errorf("WriteTo() failed. Unable to find subject") - } - if !strings.Contains(preamble, fmt.Sprintf("Content-Type: multipart/signed; protocol=\"application/pkcs7-signature\"; micalg=sha-256;\r\n boundary=%s", boundary)) { - t.Errorf("WriteTo() failed. Unable to find Content-Type") - } - - signedData := parts[1] - if !strings.Contains(signedData, "Content-Transfer-Encoding: quoted-printable") { - t.Errorf("WriteTo() failed. Unable to find Content-Transfer-Encoding") - } - if !strings.Contains(signedData, "Content-Type: text/plain; charset=UTF-8") { - t.Errorf("WriteTo() failed. Unable to find Content-Type") - } - if !strings.Contains(signedData, "Plain") { - t.Errorf("WriteTo() failed. Unable to find Content") - } - - signature := parts[2] - if !strings.Contains(signature, "Content-Transfer-Encoding: base64") { - t.Errorf("WriteTo() failed. Unable to find Content-Transfer-Encoding") - } - if !strings.Contains(signature, `application/pkcs7-signature; name="smime.p7s"`) { - t.Errorf("WriteTo() failed. Unable to find Content-Type") - } - if strings.Contains(signature, "Plain") { - t.Errorf("WriteTo() failed. Unable to find signature") - } -} - func TestMsg_hasPGPType(t *testing.T) { t.Run("message has no pgpType", func(t *testing.T) { message := testMessage(t) @@ -6838,156 +6817,120 @@ func TestMsg_SignWithTLSCertificate(t *testing.T) { }) } -func TestMsg_createSignaturePart(t *testing.T) { - /* - rsaPrivKey, rsaCert, rsaIntermediate, err := getDummyRSACryptoMaterial() - if err != nil { - t.Fatalf("failed to load dummy crypto material: %s", err) - } - ecdsaPrivKey, ecdsaCert, ecdsaIntermediate, err := getDummyRSACryptoMaterial() +// TestMsg_signMessage tests the Msg.signMessage method +func TestMsg_signMessage(t *testing.T) { + tests := []struct { + name string + keyMaterialFunc func() (crypto.PrivateKey, *x509.Certificate, *x509.Certificate, error) + asnSig []byte + }{ + {"RSA", getDummyRSACryptoMaterial, []byte{48, 130, 13}}, + {"ECDSA", getDummyECDSACryptoMaterial, []byte{48, 130, 5}}, + } + for _, tt := range tests { + t.Run(fmt.Sprintf("sign message with valid %s keypair", tt.name), func(t *testing.T) { + privKey, cert, intermediate, err := tt.keyMaterialFunc() if err != nil { t.Fatalf("failed to load dummy crypto material: %s", err) } - - */ - /* - t.Run("create signature with valid RSA certificate", func(t *testing.T) { msg := testMessage(t) - if err = msg.SignWithKeypair(rsaPrivKey, rsaCert, rsaIntermediate); err != nil { - t.Fatalf("failed to initialize SMIME configuration: %s", err) + if err = msg.SignWithKeypair(privKey, cert, intermediate); err != nil { + t.Errorf("failed to init smime configuration") + } + if err = msg.signMessage(); err != nil { + t.Fatalf("failed to S/MIME sign message: %s", err) } parts := msg.GetParts() - if len(parts) != 1 { - t.Fatalf("expected 1 body part, got %d", len(parts)) - } - body, err := parts[0].GetContent() - if err != nil { - t.Fatalf("failed to get body part content: %s", err) + if len(parts) != 2 { + t.Fatalf("failed to get message parts, expected 2 parts, got: %d", len(parts)) } - signedPart, err := msg.createSignaturePart(parts[0].GetEncoding(), parts[0].GetContentType(), - parts[0].GetCharset(), body) + bodyPart := parts[0] + if bodyPart.GetCharset() != CharsetUTF8 { + t.Errorf("failed to get signed message bodyPart charset, expected %s, got: %s", CharsetUTF8, + bodyPart.GetCharset()) + } + if bodyPart.GetEncoding() != EncodingQP { + t.Errorf("failed to get signed message bodyPart encoding, expected %s, got: %s", EncodingQP, + bodyPart.GetEncoding()) + } + if bodyPart.smime { + t.Errorf("failed to get signed message bodyPart smime status, expected false, got: %t", bodyPart.smime) + } + body, err := bodyPart.GetContent() if err != nil { - t.Fatalf("failed to create part signatures: %s", err) + t.Fatalf("failed to get signed message body part content: %s", err) + } + if !bytes.Equal(body, []byte("Testmail")) { + t.Errorf("signed body part content does not match, expected %s, got: %s", "Testmail", body) } - if signedPart == nil { - t.Fatal("failed to create part signatures, returned part is nil") + + signaturePart := parts[1] + if signaturePart.GetCharset() != CharsetUTF8 { + t.Errorf("failed to get signed message signaturePart charset, expected %s, got: %s", CharsetUTF8, + signaturePart.GetCharset()) } - if signedPart.GetEncoding() != EncodingB64 { - t.Errorf("failed to sign body part, expected encoding: %s, got: %s", EncodingB64, signedPart.GetEncoding()) + if signaturePart.GetEncoding() != EncodingB64 { + t.Errorf("failed to get signed message signaturePart encoding, expected %s, got: %s", EncodingB64, + signaturePart.GetEncoding()) } - if signedPart.GetContentType() != TypeSMIMESigned { - t.Errorf("failed to sign body part, expected content type: %s, got: %s", TypeSMIMESigned, - signedPart.GetContentType()) + if signaturePart.GetContentType() != TypeSMIMESigned { + t.Errorf("failed to get signed message signaturePart content type, expected %s, got: %s", + TypeSMIMESigned, signaturePart.GetContentType()) } - if signedPart.GetCharset() != CharsetUTF8 { - t.Errorf("failed to sign body part, expected charset: %s, got: %s", CharsetUTF8, signedPart.GetCharset()) + if !signaturePart.smime { + t.Errorf("failed to get signed message signaturePart smime status, expected true, got: %t", + signaturePart.smime) } - signedBody, err := signedPart.GetContent() + signature, err := signaturePart.GetContent() if err != nil { - t.Fatalf("failed to get signed body part content: %s", err) + t.Fatalf("failed to get signature content: %s", err) } - if len(signedBody) == len(body) { - t.Errorf("failed to sign body part, expected different length, body: %d, signed: %d", len(body), - len(signedBody)) + if !bytes.Equal(signature[:3], tt.asnSig) { + t.Errorf("signature ASN.1 signature does not match, expected: %x, got: %x", tt.asnSig, signature[:3]) } - //t.Logf("signed body: %s", signedBody) }) - - */ -} - -/* -// TestMsg_createSignaturePart tests the Msg.createSignaturePart method -func TestMsg_createSignaturePart(t *testing.T) { - privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() - if err != nil { - t.Errorf("failed to laod dummy crypto material. Cause: %v", err) - } - m := NewMsg() - if err = m.SignWithKeypair(privateKey, certificate, intermediateCertificate); err != nil { - t.Errorf("failed to init smime configuration") - } - body := []byte("This is the body") - part, err := m.createSignaturePart(EncodingQP, TypeTextPlain, CharsetUTF7, body) - if err != nil { - t.Fatalf("failed to create part signatures: %s", err) - } - if part == nil { - t.Fatal("failed to create part signatures, returned part is nil") - } - if part.GetEncoding() != EncodingB64 { - t.Errorf("createSignaturePart() failed. Expected encoding: %s, got: %s", EncodingB64, - part.GetEncoding()) - } - if part.GetContentType() != TypeSMIMESigned { - t.Errorf("createSignaturePart() failed. Expected content type: %s, got: %s", TypeSMIMESigned, - part.GetContentType()) - } - if part.GetCharset() != CharsetUTF8 { - t.Errorf("createSignaturePart() failed. Expected charset: %s, got: %s", CharsetUTF8, - part.GetCharset()) - } - content, err := part.GetContent() - if err != nil { - t.Errorf("failed to get content of part: %s", err) - } - if len(content) == len(body) { - t.Errorf("createSignaturePart() failed. Content length should not be equal to body length. Expected %d, "+ - "got: %d", len(body), len(content)) - } -} - -// TestMsg_signMessage tests the Msg.signMessage method -func TestMsg_signMessage(t *testing.T) { - privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() - if err != nil { - t.Errorf("failed to laod dummy crypto material. Cause: %v", err) - } - body := []byte("This is the body") - m := NewMsg() - m.SetBodyString(TypeTextPlain, string(body)) - if err := m.SignWithKeypair(privateKey, certificate, intermediateCertificate); err != nil { - t.Errorf("failed to init smime configuration") - } - msg, err := m.signMessage(m) - if err != nil { - t.Fatalf("failed to sign message: %s", err) } - parts := msg.GetParts() - if len(parts) != 2 { - t.Errorf("createSignaturePart() method failed. Expected 2 parts, got: %d", len(parts)) - } - - signedPart := parts[0] - if signedPart.GetEncoding() != EncodingQP { - t.Errorf("createSignaturePart() method failed. Expected encoding: %s, got: %s", EncodingB64, signedPart.GetEncoding()) - } - if signedPart.GetContentType() != TypeTextPlain { - t.Errorf("createSignaturePart() method failed. Expected content type: %s, got: %s", TypeSMIMESigned, signedPart.GetContentType()) - } - if signedPart.GetCharset() != CharsetUTF8 { - t.Errorf("createSignaturePart() method failed. Expected charset: %s, got: %s", CharsetUTF8, signedPart.GetCharset()) - } - if content, err := signedPart.GetContent(); err != nil || len(content) != len(body) { - t.Errorf("createSignaturePart() method failed. Expected content should be equal: %s, got: %s", body, content) - } - - signaturePart := parts[1] - if signaturePart.GetEncoding() != EncodingB64 { - t.Errorf("createSignaturePart() method failed. Expected encoding: %s, got: %s", EncodingB64, signaturePart.GetEncoding()) - } - if signaturePart.GetContentType() != TypeSMIMESigned { - t.Errorf("createSignaturePart() method failed. Expected content type: %s, got: %s", TypeSMIMESigned, signaturePart.GetContentType()) - } - if signaturePart.GetCharset() != CharsetUTF8 { - t.Errorf("createSignaturePart() method failed. Expected charset: %s, got: %s", CharsetUTF8, signaturePart.GetCharset()) - } - if content, err := signaturePart.GetContent(); err != nil || len(content) == len(body) { - t.Errorf("createSignaturePart() method failed. Expected content should not be equal: %s, got: %s", body, signaturePart.GetEncoding()) - } + t.Run("signing fails with invalid crypto.PrivateKey", func(t *testing.T) { + keypair, err := getDummyKeyPairTLS() + if err != nil { + t.Fatalf("failed to load dummy crypto material: %s", err) + } + msg := testMessage(t) + if err = msg.SignWithTLSCertificate(keypair); err != nil { + t.Fatalf("failed to init SMIME configuration: %s", err) + } + msg.sMIME.privateKey = unsupportedPrivKey{} + err = msg.signMessage() + if err == nil { + t.Error("SMIME signing with invalid private key is expected to fail") + } + expErr := "cannot convert encryption algorithm to oid, unknown private key type" + if !strings.Contains(err.Error(), expErr) { + t.Errorf("SMIME signing with invalid private key is expected to fail with %s, but got: %s", expErr, err) + } + }) + t.Run("signing failed with invalid header count", func(t *testing.T) { + keypair, err := getDummyKeyPairTLS() + if err != nil { + t.Fatalf("failed to load dummy crypto material: %s", err) + } + msg := testMessage(t) + if err = msg.SignWithTLSCertificate(keypair); err != nil { + t.Fatalf("failed to init SMIME configuration: %s", err) + } + msg.headerCount = 1000 + err = msg.signMessage() + if err == nil { + t.Error("SMIME signing with invalid private key is expected to fail") + } + expErr := "unable to find message body starting index within rendered message" + if !strings.EqualFold(err.Error(), expErr) { + t.Errorf("SMIME signing with invalid header count is expected to fail with %s, but got: %s", expErr, err) + } + }) } // TestGetLeafCertificate tests the Msg.getLeafCertificate method @@ -7006,8 +6949,6 @@ func TestGetLeafCertificate(t *testing.T) { } } -*/ - // uppercaseMiddleware is a middleware type that transforms the subject to uppercase. type uppercaseMiddleware struct{} From 742186cfd518b834fad31f500a80039b829e8c49 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 13:46:13 +0100 Subject: [PATCH 38/47] Update README to clarify S/MIME signing support description Refined the wording for S/MIME message signing to make it more concise and aligned with the existing items. This ensures clearer communication while maintaining consistency in the feature list. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5f6429ce..26f17119 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ Here are some highlights of go-mail's featureset: * [X] Custom error types for delivery errors * [X] Custom dial-context functions for more control over the connection (proxing, DNS hooking, etc.) * [X] Output a go-mail message as EML file and parse EML file into a go-mail message -* [X] S/MIME message signing (EXPERIMENTAL - currently only for single-part messages) +* [X] S/MIME message signing support (Experimental) go-mail works like a programatic email client and provides lots of methods and functionalities you would consider standard in a MUA. From ff953ce1718c8965619a76a3103f7cd368a0ce5f Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 14:10:58 +0100 Subject: [PATCH 39/47] Handle errors in random boundary generation and add tests This commit ensures that errors from `randomBoundary()` are handled properly in `msg.go`, preventing potential failures during boundary assignment. A new test case for signing with a broken `rand.Reader` is added in `msg_test.go` to verify the behavior. Minor test descriptions were also updated for clarity. --- msg.go | 6 +++++- msg_test.go | 26 +++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/msg.go b/msg.go index 45218a5d..7fe52d83 100644 --- a/msg.go +++ b/msg.go @@ -3069,7 +3069,11 @@ func (m *Msg) signMessage() error { // the boundary of the parts is consistant during the different rendering // phases if m.boundary == "" { - m.SetBoundary(randomBoundary()) + boundary, err := randomBoundary() + if err != nil { + return fmt.Errorf("failed to generate random boundary: %w", err) + } + m.SetBoundary(boundary) } mw.writeMsg(m) diff --git a/msg_test.go b/msg_test.go index 0e2211e1..09d199e9 100644 --- a/msg_test.go +++ b/msg_test.go @@ -8,6 +8,7 @@ import ( "bytes" "context" "crypto" + "crypto/rand" "crypto/tls" "crypto/x509" "embed" @@ -6912,7 +6913,7 @@ func TestMsg_signMessage(t *testing.T) { t.Errorf("SMIME signing with invalid private key is expected to fail with %s, but got: %s", expErr, err) } }) - t.Run("signing failed with invalid header count", func(t *testing.T) { + t.Run("signing fails with invalid header count", func(t *testing.T) { keypair, err := getDummyKeyPairTLS() if err != nil { t.Fatalf("failed to load dummy crypto material: %s", err) @@ -6931,6 +6932,29 @@ func TestMsg_signMessage(t *testing.T) { t.Errorf("SMIME signing with invalid header count is expected to fail with %s, but got: %s", expErr, err) } }) + t.Run("signing fails with broken rand.Reader", func(t *testing.T) { + defaultRandReader := rand.Reader + t.Cleanup(func() { rand.Reader = defaultRandReader }) + rand.Reader = &randReader{failon: 1} + + keypair, err := getDummyKeyPairTLS() + if err != nil { + t.Fatalf("failed to load dummy crypto material: %s", err) + } + msg := testMessage(t) + if err = msg.SignWithTLSCertificate(keypair); err != nil { + t.Fatalf("failed to init SMIME configuration: %s", err) + } + msg.headerCount = 1000 + err = msg.signMessage() + if err == nil { + t.Error("SMIME signing with broken rand.Reader is expected to fail") + } + expErr := "failed to generate random boundary: failed to read from rand.Reader: broken reader" + if !strings.EqualFold(err.Error(), expErr) { + t.Errorf("SMIME signing with broken rand.Reader is expected to fail with %s, but got: %s", expErr, err) + } + }) } // TestGetLeafCertificate tests the Msg.getLeafCertificate method From a7fb765e663cd5ae017a22cfa5524b16a19dc705 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 14:11:38 +0100 Subject: [PATCH 40/47] Add robust randomBoundary function with error handling Refactors and enhances the `randomBoundary` function to include proper error handling and uses it across the codebase. Adds accompanying tests to validate correct behavior and simulate error scenarios with a broken random reader. --- msgwriter.go | 18 ++++++------------ random.go | 19 +++++++++++++++++++ random_test.go | 20 ++++++++++++++++++++ 3 files changed, 45 insertions(+), 12 deletions(-) diff --git a/msgwriter.go b/msgwriter.go index 64e61f12..f3083aa9 100644 --- a/msgwriter.go +++ b/msgwriter.go @@ -6,7 +6,6 @@ package mail import ( "bytes" - "crypto/rand" "encoding/base64" "fmt" "io" @@ -132,7 +131,12 @@ func (mw *msgWriter) writeMsg(msg *Msg) { } if msg.hasSMIME() && !msg.isSMIMEInProgress() { - mw.startMP(MIMESMIMESigned, randomBoundary()) + boundary, err := randomBoundary() + if err != nil { + mw.err = err + return + } + mw.startMP(MIMESMIMESigned, boundary) mw.writeString(DoubleNewLine) } if msg.hasMixed() { @@ -529,16 +533,6 @@ func (mw *msgWriter) writeBody(writeFunc func(io.Writer) (int64, error), encodin } } -// randomBoundary -func randomBoundary() string { - var buf [30]byte - _, err := io.ReadFull(rand.Reader, buf[:]) - if err != nil { - panic(err) - } - return fmt.Sprintf("%x", buf[:]) -} - // sanitizeFilename sanitizes a given filename string by replacing specific unwanted characters with // an underscore ('_'). // diff --git a/random.go b/random.go index e987f008..2596a721 100644 --- a/random.go +++ b/random.go @@ -7,6 +7,8 @@ package mail import ( "crypto/rand" "encoding/binary" + "fmt" + "io" "strings" ) @@ -70,3 +72,20 @@ func randomStringSecure(length int) (string, error) { return randString.String(), nil } + +// randomBoundary generates a random boundary string for use in MIME messages. +// +// This function creates a 30-byte random value using a cryptographic random number generator +// and formats it as a hexadecimal string. +// +// Returns: +// - A string containing the generated random boundary. +// - An error if reading from the random number generator fails. +func randomBoundary() (string, error) { + var buf [30]byte + _, err := io.ReadFull(rand.Reader, buf[:]) + if err != nil { + return "", fmt.Errorf("failed to read from rand.Reader: %w", err) + } + return fmt.Sprintf("%x", buf[:]), nil +} diff --git a/random_test.go b/random_test.go index b59eba63..27c9da3c 100644 --- a/random_test.go +++ b/random_test.go @@ -58,6 +58,26 @@ func TestRandomStringSecure(t *testing.T) { }) } +func TestRandomBoundary(t *testing.T) { + t.Run("randomBoundary returning valid values", func(t *testing.T) { + boundary, err := randomBoundary() + if err != nil { + t.Errorf("random boundary generation failed: %s", err) + } + if len(boundary) < 30 { + t.Errorf("random boundary length mismatch. got: %d, expected: %d", len(boundary), 30) + } + }) + t.Run("randomBoundary fails on broken rand Reader", func(t *testing.T) { + defaultRandReader := rand.Reader + t.Cleanup(func() { rand.Reader = defaultRandReader }) + rand.Reader = &randReader{failon: 1} + if _, err := randomBoundary(); err == nil { + t.Fatalf("expected random boundary generation to fail on broken rand.Reader") + } + }) +} + func BenchmarkGenerator_RandomStringSecure(b *testing.B) { b.ReportAllocs() for i := 0; i < b.N; i++ { From 23866df544b3052a78b237d87d27a25350a60779 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 14:14:22 +0100 Subject: [PATCH 41/47] Improve HTML attributes in README for better accessibility Replaced `align` attribute with `style` for semantic HTML and added `alt` text to contributor image for improved accessibility. These changes enhance readability and ensure better compliance with modern web standards. --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 26f17119..85abe4b6 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ SPDX-License-Identifier: MIT [![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/wneessen/go-mail/badge)](https://securityscorecards.dev/viewer/?uri=github.com/wneessen/go-mail) buy ma a coffee -

go-mail logo

+

go-mail logo

The main idea of this library was to provide a simple interface for sending mails to my [JS-Mailer](https://github.com/wneessen/js-mailer) project. It quickly evolved into a full-fledged mail library. @@ -113,7 +113,7 @@ contributed ot the project. Big thanks to all of them for improving the go-mail code, reviewing code, writing documenation or helping to translate the website): - + image of contributors A huge thank you also goes to [Maria Letta](https://github.com/MariaLetta) for designing our super cool go-mail logo! From 2d990b721e28c2fb5c27624cc8786d1409bcb052 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 14:21:24 +0100 Subject: [PATCH 42/47] Remove obsolete S/MIME unit test in msgwriter_test.go The TestMsgWriter_writeMsg_SMime function was removed as it is no longer relevant or needed. It tested deprecated S/MIME functionality which is no longer maintained, streamlining the test suite and improving maintainability. --- msgwriter_test.go | 39 --------------------------------------- 1 file changed, 39 deletions(-) diff --git a/msgwriter_test.go b/msgwriter_test.go index ba13735b..2c11406c 100644 --- a/msgwriter_test.go +++ b/msgwriter_test.go @@ -765,42 +765,3 @@ func TestMsgWriter_sanitizeFilename(t *testing.T) { }) } } - -// TestMsgWriter_writeMsg_SMime tests the writeMsg method of the msgWriter with S/MIME types set -func TestMsgWriter_writeMsg_SMime(t *testing.T) { - privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() - if err != nil { - t.Errorf("failed to laod dummy crypto material. Cause: %v", err) - } - - m := NewMsg() - if err := m.SignWithKeypair(privateKey, certificate, intermediateCertificate); err != nil { - t.Errorf("failed to init smime configuration") - } - _ = m.From(`"Toni Tester" `) - _ = m.To(`"Toni Receiver" `) - m.Subject("This is a subject") - m.SetBodyString(TypeTextPlain, "This is the body") - buf := bytes.Buffer{} - mw := &msgWriter{writer: &buf, charset: CharsetUTF8, encoder: mime.QEncoding} - mw.writeMsg(m) - ms := buf.String() - - if !strings.Contains(ms, "MIME-Version: 1.0") { - t.Errorf("writeMsg failed. Unable to find MIME-Version") - } - if !strings.Contains(ms, "Subject: This is a subject") { - t.Errorf("writeMsg failed. Unable to find subject") - } - if !strings.Contains(ms, "From: \"Toni Tester\" ") { - t.Errorf("writeMsg failed. Unable to find transmitter") - } - if !strings.Contains(ms, "To: \"Toni Receiver\" ") { - t.Errorf("writeMsg failed. Unable to find receiver") - } - - boundary := ms[strings.LastIndex(ms, "--")-60 : strings.LastIndex(ms, "--")] - if !strings.Contains(ms, fmt.Sprintf("Content-Type: multipart/signed; protocol=\"application/pkcs7-signature\"; micalg=sha-256;\r\n boundary=%s", boundary)) { - t.Errorf("writeMsg failed. Unable to find Content-Type") - } -} From 1fd658b2c62ab51b6a9cd3e406e83023c333e169 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 14:38:37 +0100 Subject: [PATCH 43/47] Add tests for S/MIME signing with various edge cases Introduces tests to verify S/MIME signing behavior across scenarios such as multipart messages, invalid keys, and a broken rand.Reader. Ensures proper error handling and compatibility with complex message structures like multipart/alternative, multipart/mixed, and multipart/related. --- msg_test.go | 110 +++++++++++++++++++++++++++++++++++++++++++++- msgwriter_test.go | 25 +++++++++++ 2 files changed, 134 insertions(+), 1 deletion(-) diff --git a/msg_test.go b/msg_test.go index 09d199e9..edc0e2a3 100644 --- a/msg_test.go +++ b/msg_test.go @@ -5809,7 +5809,91 @@ func TestMsg_WriteTo(t *testing.T) { t.Error("expected message to be S/MIME signature attachment in mail body") } }) - t.Run("WriteTo with S/MIME signing fails", func(t *testing.T) { + t.Run("WriteTo with S/MIME signing on multipart/alternative message", func(t *testing.T) { + keypair, err := getDummyKeyPairTLS() + if err != nil { + t.Fatalf("failed to get dummy key material: %s", err) + } + message := testMessage(t) + message.AddAlternativeString(TypeTextHTML, "

HTML alternative

") + if err = message.SignWithTLSCertificate(keypair); err != nil { + t.Fatalf("failed to initialize S/MIME signing: %s", err) + } + buffer := bytes.NewBuffer(nil) + if _, err = message.WriteTo(buffer); err != nil { + t.Errorf("failed to write S/MIME signed message to buffer: %s", err) + } + if !strings.Contains(buffer.String(), TypeSMIMESigned.String()) { + t.Error("expected message to be S/MIME signature Content-Type in mail body") + } + if !strings.Contains(buffer.String(), `application/pkcs7-signature; name="smime.p7s"`) { + t.Error("expected message to be S/MIME signature attachment in mail body") + } + if !strings.Contains(buffer.String(), TypeTextHTML.String()) { + t.Error("expected message to have HTML alternative in mail body") + } + if !strings.Contains(buffer.String(), TypeMultipartAlternative.String()) { + t.Error("expected message to have multipart/alternative in mail body") + } + if !strings.Contains(buffer.String(), "

HTML alternative

") { + t.Error("expected message to have HTML alternative in mail body") + } + }) + t.Run("WriteTo with S/MIME signing on multipart/mixed message", func(t *testing.T) { + keypair, err := getDummyKeyPairTLS() + if err != nil { + t.Fatalf("failed to get dummy key material: %s", err) + } + message := testMessage(t) + message.AttachFile("testdata/attachment.txt") + if err = message.SignWithTLSCertificate(keypair); err != nil { + t.Fatalf("failed to initialize S/MIME signing: %s", err) + } + buffer := bytes.NewBuffer(nil) + if _, err = message.WriteTo(buffer); err != nil { + t.Errorf("failed to write S/MIME signed message to buffer: %s", err) + } + if !strings.Contains(buffer.String(), TypeSMIMESigned.String()) { + t.Error("expected message to be S/MIME signature Content-Type in mail body") + } + if !strings.Contains(buffer.String(), `application/pkcs7-signature; name="smime.p7s"`) { + t.Error("expected message to be S/MIME signature attachment in mail body") + } + if !strings.Contains(buffer.String(), TypeMultipartMixed.String()) { + t.Error("expected message to have multipart/mixed in mail body") + } + if !strings.Contains(buffer.String(), "attachment.txt") { + t.Error("expected message to have attachment in mail body") + } + }) + t.Run("WriteTo with S/MIME signing on multipart/related message", func(t *testing.T) { + keypair, err := getDummyKeyPairTLS() + if err != nil { + t.Fatalf("failed to get dummy key material: %s", err) + } + message := testMessage(t) + message.EmbedFile("testdata/embed.txt") + if err = message.SignWithTLSCertificate(keypair); err != nil { + t.Fatalf("failed to initialize S/MIME signing: %s", err) + } + buffer := bytes.NewBuffer(nil) + if _, err = message.WriteTo(buffer); err != nil { + t.Errorf("failed to write S/MIME signed message to buffer: %s", err) + } + if !strings.Contains(buffer.String(), TypeSMIMESigned.String()) { + t.Error("expected message to be S/MIME signature Content-Type in mail body") + } + if !strings.Contains(buffer.String(), `application/pkcs7-signature; name="smime.p7s"`) { + t.Error("expected message to be S/MIME signature attachment in mail body") + } + if !strings.Contains(buffer.String(), TypeMultipartRelated.String()) { + t.Error("expected message to have multipart/related in mail body") + } + if !strings.Contains(buffer.String(), "embed.txt") { + t.Error("expected message to have embeded file in mail body") + } + }) + t.Run("WriteTo with S/MIME signing fails with invalid key", func(t *testing.T) { keypair, err := getDummyKeyPairTLS() if err != nil { t.Fatalf("failed to get dummy key material: %s", err) @@ -5831,6 +5915,30 @@ func TestMsg_WriteTo(t *testing.T) { t.Errorf("expected S/MIME signing error to be: %s, got: %s", expErr, err) } }) + t.Run("WriteTo with S/MIME signing fails with broken rand.Reader", func(t *testing.T) { + defaultRandReader := rand.Reader + t.Cleanup(func() { rand.Reader = defaultRandReader }) + rand.Reader = &randReader{failon: 1} + + keypair, err := getDummyKeyPairTLS() + if err != nil { + t.Fatalf("failed to get dummy key material: %s", err) + } + message := testMessage(t) + if err = message.SignWithTLSCertificate(keypair); err != nil { + t.Fatalf("failed to initialize S/MIME signing: %s", err) + } + + buffer := bytes.NewBuffer(nil) + _, err = message.WriteTo(buffer) + if err == nil { + t.Error("expected WritoTo with invalid S/MIME private key to fail") + } + expErr := "failed to generate random boundary: failed to read from rand.Reader: broken reader" + if !strings.EqualFold(err.Error(), expErr) { + t.Errorf("expected S/MIME signing error to be: %s, got: %s", expErr, err) + } + }) } func TestMsg_WriteToFile(t *testing.T) { diff --git a/msgwriter_test.go b/msgwriter_test.go index 2c11406c..11d856f6 100644 --- a/msgwriter_test.go +++ b/msgwriter_test.go @@ -6,6 +6,7 @@ package mail import ( "bytes" + "crypto/rand" "errors" "fmt" "io" @@ -280,6 +281,30 @@ func TestMsgWriter_writeMsg(t *testing.T) { t.Errorf("expected boundary, got: %s", buffer.String()) } }) + t.Run("msgWriter fails on S/MIME signing with broken rand.Reader", func(t *testing.T) { + defaultRandReader := rand.Reader + t.Cleanup(func() { rand.Reader = defaultRandReader }) + rand.Reader = &randReader{failon: 1} + keypair, err := getDummyKeyPairTLS() + if err != nil { + t.Fatalf("failed to get dummy key material: %s", err) + } + + buffer := bytes.NewBuffer(nil) + msgwriter.writer = buffer + message := testMessage(t) + if err = message.SignWithTLSCertificate(keypair); err != nil { + t.Fatalf("failed to initialize S/MIME signing: %s", err) + } + msgwriter.writeMsg(message) + if msgwriter.err == nil { + t.Errorf("msgWriter should have failed to write with broken rand.Reader") + } + expErr := "failed to read from rand.Reader: broken reader" + if !strings.EqualFold(msgwriter.err.Error(), expErr) { + t.Errorf("expected msgWriter to fail with: %s, got: %s", expErr, msgwriter.err) + } + }) } func TestMsgWriter_writePreformattedGenHeader(t *testing.T) { From 3ef43bbbbca54e83fd150e3d91ec96005de279cb Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 16:31:35 +0100 Subject: [PATCH 44/47] Remove TestGetLeafCertificate from msg_test.go This test was removed as it is redundant or no longer necessary. The cleanup reduces code bloat and improves maintainability of the test suite. --- msg_test.go | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/msg_test.go b/msg_test.go index edc0e2a3..f07d737b 100644 --- a/msg_test.go +++ b/msg_test.go @@ -7065,22 +7065,6 @@ func TestMsg_signMessage(t *testing.T) { }) } -// TestGetLeafCertificate tests the Msg.getLeafCertificate method -func TestGetLeafCertificate(t *testing.T) { - keyPairTLS, err := getDummyKeyPairTLS() - if err != nil { - t.Errorf("failed to laod dummy crypto material. Cause: %v", err) - } - - leafCertificate, err := getLeafCertificate(keyPairTLS) - if err != nil { - t.Errorf("failed to get leaf certificate. Cause: %v", err) - } - if leafCertificate == nil { - t.Errorf("failed to get leaf certificate") - } -} - // uppercaseMiddleware is a middleware type that transforms the subject to uppercase. type uppercaseMiddleware struct{} From a1c1ecb416826221c8bee3ecef9f11db67af4c75 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 16:31:46 +0100 Subject: [PATCH 45/47] Fix nil pointer dereference in certificate validation Previously, the code did not properly handle cases where the Certificate field was nil, leading to potential runtime errors. This update adds a nil check to prevent these issues and ensure robust validation. --- smime.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smime.go b/smime.go index dd267364..9d1dc271 100644 --- a/smime.go +++ b/smime.go @@ -126,7 +126,7 @@ func getLeafCertificate(keyPairTLS *tls.Certificate) (*x509.Certificate, error) return keyPairTLS.Leaf, nil } - if len(keyPairTLS.Certificate) == 0 { + if keyPairTLS.Certificate == nil || len(keyPairTLS.Certificate) == 0 { return nil, errors.New("certificate chain is empty") } cert, err := x509.ParseCertificate(keyPairTLS.Certificate[0]) From f2bb3c78a2d2dc5b569f648d83c992da296ab0a0 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 16:37:00 +0100 Subject: [PATCH 46/47] Simplify certificate nil check in smime.go Replaced redundant `nil` check with a concise length check for `keyPairTLS.Certificate`. This ensures cleaner and more readable code while maintaining the same functionality. --- smime.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smime.go b/smime.go index 9d1dc271..dd267364 100644 --- a/smime.go +++ b/smime.go @@ -126,7 +126,7 @@ func getLeafCertificate(keyPairTLS *tls.Certificate) (*x509.Certificate, error) return keyPairTLS.Leaf, nil } - if keyPairTLS.Certificate == nil || len(keyPairTLS.Certificate) == 0 { + if len(keyPairTLS.Certificate) == 0 { return nil, errors.New("certificate chain is empty") } cert, err := x509.ParseCertificate(keyPairTLS.Certificate[0]) From 671fd2890e881066870f793097b877cc48411ec1 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Wed, 8 Jan 2025 16:37:11 +0100 Subject: [PATCH 47/47] Enhance SMIME tests and add getLeafCertificate validation Refactor and extend SMIME test cases for better readability and coverage, including new edge cases and type validations. Introduce comprehensive tests for `getLeafCertificate` function to ensure proper handling of normal, nil, empty chain, and malformed certificate scenarios. --- smime_test.go | 170 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 138 insertions(+), 32 deletions(-) diff --git a/smime_test.go b/smime_test.go index 456dca87..0ff4df1a 100644 --- a/smime_test.go +++ b/smime_test.go @@ -5,11 +5,14 @@ package mail import ( + "bytes" "crypto" "crypto/ecdsa" "crypto/rsa" "crypto/tls" "crypto/x509" + "errors" + "strings" "testing" ) @@ -31,50 +34,153 @@ func TestNewSMIME(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - privateKey, certificate, intermediateCertificate, err := tt.certFunc() + privkey, certificate, intermediate, err := tt.certFunc() if err != nil { - t.Errorf("Error getting dummy crypto material: %s", err) + t.Errorf("failed getting dummy crypto material: %s", err) } - - sMime, err := newSMIME(privateKey, certificate, intermediateCertificate) + smime, err := newSMIME(privkey, certificate, intermediate) if err != nil { - t.Errorf("Error creating new SMIME from keyPair: %s", err) + t.Errorf("failed to initialize SMIME: %s", err) } - if sMime.privateKey != privateKey { - t.Errorf("NewSMime() did not return the same private key") + switch key := privkey.(type) { + case *rsa.PrivateKey: + if !key.Equal(smime.privateKey) { + t.Error("SMIME initialization failed. Private keys are not equal") + } + case *ecdsa.PrivateKey: + if !key.Equal(smime.privateKey) { + t.Error("SMIME initialization failed. Private keys are not equal") + } + default: + t.Fatal("unsupported private key type") } - if sMime.certificate != certificate { - t.Errorf("NewSMime() did not return the same certificate") + + if !bytes.Equal(certificate.Raw, smime.certificate.Raw) { + t.Errorf("SMIME initialization failed. Expected public key: %x, got: %x", certificate.Raw, + smime.certificate.Raw) } - if sMime.intermediateCert != intermediateCertificate { - t.Errorf("NewSMime() did not return the same intermedidate certificate") + if !bytes.Equal(intermediate.Raw, smime.intermediateCert.Raw) { + t.Errorf("SMIME initialization failed. Expected intermediate certificate: %x, got: %x", + intermediate.Raw, smime.intermediateCert.Raw) } }) } -} -// TestSign tests the sign method -func TestSign(t *testing.T) { - privateKey, certificate, intermediateCertificate, err := getDummyRSACryptoMaterial() - if err != nil { - t.Errorf("Error getting dummy crypto material: %s", err) - } - - sMime, err := newSMIME(privateKey, certificate, intermediateCertificate) - if err != nil { - t.Errorf("Error creating new SMIME from keyPair: %s", err) - } - - message := "This is a test message" - singedMessage, err := sMime.signMessage([]byte(message)) - if err != nil { - t.Errorf("Error creating singed message: %s", err) - } + t.Run("newSMIME fails with nil private key", func(t *testing.T) { + _, certificate, intermediate, err := getDummyRSACryptoMaterial() + if err != nil { + t.Errorf("failed getting dummy crypto material: %s", err) + } + _, err = newSMIME(nil, certificate, intermediate) + if err == nil { + t.Error("newSMIME with nil private key is expected to fail") + } + if !errors.Is(err, ErrPrivateKeyMissing) { + t.Errorf("newSMIME with nil private key is expected to fail with ErrPrivateKeyMissing, got: %s", err) + } + }) + t.Run("newSMIME fails with nil public key", func(t *testing.T) { + privkey, _, intermediate, err := getDummyRSACryptoMaterial() + if err != nil { + t.Errorf("failed getting dummy crypto material: %s", err) + } + _, err = newSMIME(privkey, nil, intermediate) + if err == nil { + t.Error("newSMIME with nil private key is expected to fail") + } + if !errors.Is(err, ErrCertificateMissing) { + t.Errorf("newSMIME with nil public key is expected to fail with ErrCertificateMissing, got: %s", err) + } + }) +} - if singedMessage == message { - t.Errorf("Sign() did not work") - } +func TestGetLeafCertificate(t *testing.T) { + t.Run("getLeafCertificate works normally", func(t *testing.T) { + keypair, err := getDummyKeyPairTLS() + if err != nil { + t.Errorf("failed to load dummy crypto material: %s", err) + } + leaf, err := x509.ParseCertificate(keypair.Certificate[0]) + if err != nil { + t.Fatalf("failed to parse leaf certificate: %s", err) + } + keypair.Leaf = leaf + + leafCert, err := getLeafCertificate(keypair) + if err != nil { + t.Errorf("failed to get leaf certificate: %s", err) + } + if leafCert == nil { + t.Fatal("failed to get leaf certificate, got nil") + } + if !bytes.Equal(leafCert.Raw, keypair.Leaf.Raw) { + t.Errorf("failed to get leaf certificate, expected cert mismatch, expected: %x, got: %x", + keypair.Leaf.Raw, leafCert.Raw) + } + }) + t.Run("getLeafCertificate fails with nil", func(t *testing.T) { + _, err := getLeafCertificate(nil) + if err == nil { + t.Error("getLeafCertificate with nil is expected to fail") + } + }) + t.Run("getLeafCertificate without leaf should return first certificate in chain", func(t *testing.T) { + keypair, err := getDummyKeyPairTLS() + if err != nil { + t.Errorf("failed to load dummy crypto material: %s", err) + } + keypair.Leaf = nil + leafCert, err := getLeafCertificate(keypair) + if err != nil { + t.Errorf("failed to get leaf certificate: %s", err) + } + if leafCert == nil { + t.Fatal("failed to get leaf certificate, got nil") + } + if !bytes.Equal(leafCert.Raw, keypair.Certificate[0]) { + t.Errorf("failed to get leaf certificate, expected cert mismatch, expected: %x, got: %x", + keypair.Leaf.Raw, leafCert.Raw) + } + }) + t.Run("getLeafCertificate with empty chain should fail", func(t *testing.T) { + keypair, err := getDummyKeyPairTLS() + if err != nil { + t.Errorf("failed to load dummy crypto material: %s", err) + } + localpair := &tls.Certificate{ + Certificate: nil, + PrivateKey: keypair.PrivateKey, + Leaf: nil, + SignedCertificateTimestamps: keypair.SignedCertificateTimestamps, + SupportedSignatureAlgorithms: keypair.SupportedSignatureAlgorithms, + } + _, err = getLeafCertificate(localpair) + if err == nil { + t.Errorf("expected getLeafCertificate to fail with empty chain, got: %s", err) + } + expErr := "certificate chain is empty" + if !strings.EqualFold(err.Error(), expErr) { + t.Errorf("getting leaf certificate was supposed to fail with: %s, got: %s", expErr, err.Error()) + } + }) + t.Run("getLeafCertificate fails while parsing broken certificate", func(t *testing.T) { + keypair, err := getDummyKeyPairTLS() + if err != nil { + t.Errorf("failed to load dummy crypto material: %s", err) + } + keypair.Leaf = nil + keypair.Certificate[0] = []byte("broken certificate") + + _, err = getLeafCertificate(keypair) + if err == nil { + t.Errorf("expected getLeafCertificate to fail with broken cert, got: %s", err) + } + expErr := "x509: malformed certificate" + if !strings.EqualFold(err.Error(), expErr) { + t.Errorf("getting leaf certificate was supposed to fail with: %s, got: %s", expErr, err.Error()) + } + }) } // getDummyRSACryptoMaterial loads a certificate (RSA), the associated private key and certificate (RSA) is loaded