Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve mail quality #87

Open
stoecker opened this issue May 14, 2024 · 3 comments
Open

Improve mail quality #87

stoecker opened this issue May 14, 2024 · 3 comments
Labels
refinement An improvement, but not a new feature

Comments

@stoecker
Copy link

The email sent by certspotter is so low quality, that it's hard to get it through a spam filter.

Please add:

  • A configurable "From" (i.e. sender address).
  • Use this from in the SMTP hello message (-f option of sendmail)
  • Use the header line "Precendence: bulk" to indicate it's not human generated.
  • If possible add some short text like "This is an automated message of certspotter running on {machinename}. A new certificate for domain {domain} was detected in the list of {ctprovidername}. Following text describes the certificate:" ... or something similar to make it look like a real human would write this. Make the email also better readable.

Especially the sender from address and the "From:" mail header are important. They also enable to properly setup DKIM/DMARC.

First 3 points I fixed hardcoded in notify.go on my system:
fmt.Fprintf(stdin, "From: myname@mytld\n")
fmt.Fprintf(stdin, "To: %s\n", strings.Join(to, ", "))
fmt.Fprintf(stdin, "Subject: [certspotter] %s\n", notif.Summary())
fmt.Fprintf(stdin, "Date: %s\n", time.Now().Format(mailDateFormat))
fmt.Fprintf(stdin, "Message-ID: <%s>\n", generateMessageID())
fmt.Fprintf(stdin, "Mime-Version: 1.0\n")
fmt.Fprintf(stdin, "Content-Type: text/plain; charset=US-ASCII\n")
fmt.Fprintf(stdin, "X-Mailer: certspotter\n")
fmt.Fprintf(stdin, "Precedence: bulk\n")
fmt.Fprintf(stdin, "\n")
fmt.Fprint(stdin, notif.Text())

args := []string{"-i", "-f", "myname@mytld", "--"}

@AGWA AGWA added the refinement An improvement, but not a new feature label May 14, 2024
@stoecker
Copy link
Author

stoecker commented May 21, 2024

Patch against 0.18:

diff -ur pkg/mod/software.sslmate.com/src/[email protected]_orig/cmd/certspotter/main.go pkg/mod/software.sslmate.com/src/[email protected]/cmd/certspotter/main.go
--- pkg/mod/software.sslmate.com/src/[email protected]_orig/cmd/certspotter/main.go	2024-05-21 15:53:27.388850751 +0200
+++ pkg/mod/software.sslmate.com/src/[email protected]/cmd/certspotter/main.go	2024-05-21 15:14:09.636850751 +0200
@@ -169,6 +169,7 @@
 		verbose     bool
 		version     bool
 		watchlist   string
+		mailSender  string
 	}
 	flag.IntVar(&flags.batchSize, "batch_size", 1000, "Max number of entries to request per call to get-entries (advanced)")
 	flag.Func("email", "Email address to contact when matching certificate is discovered (repeatable)", appendFunc(&flags.email))
@@ -182,6 +183,7 @@
 	flag.BoolVar(&flags.verbose, "verbose", false, "Be verbose")
 	flag.BoolVar(&flags.version, "version", false, "Print version and exit")
 	flag.StringVar(&flags.watchlist, "watchlist", defaultWatchListPathIfExists(), "File containing domain names to watch")
+	flag.StringVar(&flags.mailSender, "mailsender", "", "Sender mail address")
 	flag.Parse()
 
 	if flags.version {
@@ -204,6 +206,7 @@
 		Email:               flags.email,
 		Stdout:              flags.stdout,
 		HealthCheckInterval: flags.healthcheck,
+		MailSender:          flags.mailSender,
 	}
 
 	emailFileExists := false
diff -ur pkg/mod/software.sslmate.com/src/[email protected]_orig/man/certspotter.md pkg/mod/software.sslmate.com/src/[email protected]/man/certspotter.md
--- pkg/mod/software.sslmate.com/src/[email protected]_orig/man/certspotter.md	2024-05-21 15:53:27.392850751 +0200
+++ pkg/mod/software.sslmate.com/src/[email protected]/man/certspotter.md	2024-05-21 15:13:05.480850751 +0200
@@ -113,6 +113,10 @@
     certspotter reads the watch list only when starting up, so you must restart
     certspotter if you change it.
 
+mailsender *ADDRESS*
+
+:   Sender address for mail notification.
+
 # NOTIFICATIONS
 
 When certspotter detects a certificate matching your watchlist, or encounters
diff -ur pkg/mod/software.sslmate.com/src/[email protected]_orig/monitor/config.go pkg/mod/software.sslmate.com/src/[email protected]/monitor/config.go
--- pkg/mod/software.sslmate.com/src/[email protected]_orig/monitor/config.go	2024-05-21 15:53:27.392850751 +0200
+++ pkg/mod/software.sslmate.com/src/[email protected]/monitor/config.go	2024-05-21 15:13:54.920850751 +0200
@@ -25,4 +25,5 @@
 	Email               []string
 	Stdout              bool
 	HealthCheckInterval time.Duration
+	MailSender          string
 }
diff -ur pkg/mod/software.sslmate.com/src/[email protected]_orig/monitor/notify.go pkg/mod/software.sslmate.com/src/[email protected]/monitor/notify.go
--- pkg/mod/software.sslmate.com/src/[email protected]_orig/monitor/notify.go	2024-05-21 15:53:27.396850751 +0200
+++ pkg/mod/software.sslmate.com/src/[email protected]/monitor/notify.go	2024-05-21 15:14:24.456850751 +0200
@@ -37,7 +37,7 @@
 	}
 
 	if len(config.Email) > 0 {
-		if err := sendEmail(ctx, config.Email, notif); err != nil {
+		if err := sendEmail(ctx, config.Email, config.MailSender, notif); err != nil {
 			return err
 		}
 	}
@@ -63,10 +63,13 @@
 	os.Stdout.WriteString(notif.Text() + "\n")
 }
 
-func sendEmail(ctx context.Context, to []string, notif notification) error {
+func sendEmail(ctx context.Context, to []string, mailSender string, notif notification) error {
 	stdin := new(bytes.Buffer)
 	stderr := new(bytes.Buffer)
 
+        if mailSender != "" {
+            fmt.Fprintf(stdin, "From: %s\n", mailSender)
+        }
 	fmt.Fprintf(stdin, "To: %s\n", strings.Join(to, ", "))
 	fmt.Fprintf(stdin, "Subject: [certspotter] %s\n", notif.Summary())
 	fmt.Fprintf(stdin, "Date: %s\n", time.Now().Format(mailDateFormat))
@@ -74,10 +77,20 @@
 	fmt.Fprintf(stdin, "Mime-Version: 1.0\n")
 	fmt.Fprintf(stdin, "Content-Type: text/plain; charset=US-ASCII\n")
 	fmt.Fprintf(stdin, "X-Mailer: certspotter\n")
+	fmt.Fprintf(stdin, "Precedence: bulk\n")
 	fmt.Fprintf(stdin, "\n")
+	host, err := os.Hostname()
+	if err != nil {
+	  host = "unknown host"
+	}
+	fmt.Fprintf(stdin, "This is an automated message of certspotter running on %s. A new event was detected (%s). Following text describes the certificate:", host, notif.Summary())
 	fmt.Fprint(stdin, notif.Text())
 
-	args := []string{"-i", "--"}
+        args := []string{"-i"}
+	if mailSender != "" {
+	    args = append(args, "-f", mailSender)
+	}
+	args = append(args, "--")
 	args = append(args, to...)
 
 	sendmail := exec.CommandContext(ctx, sendmailPath(), args...)

AGWA added a commit that referenced this issue May 21, 2024
Envelope sender and RFC5322.From address are set to $EMAIL if it's non-empty.

Requested in #87
@AGWA
Copy link
Member

AGWA commented May 21, 2024

certspotter will now set the envelope sender and RFC5322.From address to the value of $EMAIL if set. $EMAIL is a semi-standard environment variable for specifying a user's email address.

Please provide citations to justify using Precedence: bulk and adding text to the beginning of the message. In particular, "indicate it's not human generated" and "make it look like a real human would write this" are contradictory.

@stoecker
Copy link
Author

stoecker commented May 21, 2024

Precedence header: https://stackoverflow.com/questions/6148865/what-is-the-effect-of-a-precedence-bulk-header-on-e-mail-messages

Was suggested by Google guidelines. It helped to get mail to Google. Seems they no longer request it.

SpamAssassin and other mail filters expect mails to be readable text. Everything which deviates from that usually get's downvoted. Also these systems often have Bayes filters for training. And the text can contain additional information and it simply looks better when an e-mail states what it is.

fritterhoff pushed a commit to hm-edu/certspotter that referenced this issue Jun 9, 2024
Envelope sender and RFC5322.From address are set to $EMAIL if it's non-empty.

Requested in SSLMate#87
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refinement An improvement, but not a new feature
Projects
None yet
Development

No branches or pull requests

2 participants