Supply advice despite non-fatal errors

Return a 400 error for single invalid domain API requests
Clarify why DKIM may be missing
Add `s1` and `s2` to our known DKIM selectors
Don't return an error for NXDOMAIN DNS requests, as these are expected from some requests (e.g. trying to find the correct DKIM selector)

Author: wolveix

README.md

@@ -1,8 +1,14 @@
 # Domain Security Scanner
-The Domain Security Scanner can be used to perform scans against domains for DKIM, DMARC, and SPF DNS records. You can also serve this functionality via an API, or a dedicated mailbox. A web application is also available if organizations would like to perform a single domain scan for DKIM, DMARC or SPF at [https://dmarcguide.globalcyberalliance.org](https://dmarcguide.globalcyberalliance.org).
+
+The Domain Security Scanner can be used to perform scans against domains for DKIM, DMARC, and SPF DNS records. You can
+also serve this functionality via an API, or a dedicated mailbox. A web application is also available if organizations
+would like to perform a single domain scan for DKIM, DMARC or SPF
+at [https://dmarcguide.globalcyberalliance.org](https://dmarcguide.globalcyberalliance.org).
 
 ## Download
-You can download pre-compiled binaries for macOS, Linux and Windows from the [releases](https://github.com/GlobalCyberAlliance/domain-security-scanner/releases) page.
+
+You can download pre-compiled binaries for macOS, Linux and Windows from
+the [releases](https://github.com/GlobalCyberAlliance/domain-security-scanner/releases) page.
 
 Alternatively, you can run the binary from within our pre-built Docker image:
 
@@ -23,6 +29,7 @@ make
 This will output a binary called `dss`. You can then move it or use it by running `./bin/dss` (on Unix devices).
 
 ## Find a Specific Record From a Single Domain
+
 To scan a domain for a specific type of record (A, AAAA, CNAME, DKIM, DMARC, MX, SPF, TXT), run:
 
 `dss scan [domain] --type dmarc`
@@ -35,7 +42,8 @@ Example:
 
 ## Bulk Scan Domains
 
-Scan any number of domains' DNS records. By default, this listens on `STDIN`, meaning you run the command via `dss scan` and then enter each domain one-by-one.
+Scan any number of domains' DNS records. By default, this listens on `STDIN`, meaning you run the command via `dss scan`
+and then enter each domain one-by-one.
 
 Alternatively, you can specify multiple domains at runtime:
 
@@ -49,13 +57,17 @@ See the [zonefile.example](zonefile.example) file in this repo.
 
 ## Serve REST API
 
-You can also expose the domain scanning functionality via a REST API. By default, this is rate limited to 3 requests per 3 second interval from a single IP address. Serve the API by running the following:
+You can also expose the domain scanning functionality via a REST API. By default, this is rate limited to 3 requests per
+3 second interval from a single IP address. Serve the API by running the following:
 
 `dss serve api --port 80`
 
-You can reach the API docs by visiting `http://server-ip:port/api/v1/docs` and the OpenAPI schema at `http://server-ip:port/api/v1/docs.json` or `http://server-ip:port/api/v1/docs.yaml`. You can also test requests through this interface thanks to [Scalar](https://github.com/scalar/scalar).
+You can reach the API docs by visiting `http://server-ip:port/api/v1/docs` and the OpenAPI schema
+at `http://server-ip:port/api/v1/docs.json` or `http://server-ip:port/api/v1/docs.yaml`. You can also test requests
+through this interface thanks to [Scalar](https://github.com/scalar/scalar).
 
-You can then get a single domain's results by submitting a GET request like this `http://server-ip:port/api/v1/scan/globalcyberalliance.org`, which will return a JSON response similar to this:
+You can then get a single domain's results by submitting a GET request like
+this `http://server-ip:port/api/v1/scan/globalcyberalliance.org`, which will return a JSON response similar to this:
 
 ```json
 {
@@ -98,7 +110,8 @@ You can then get a single domain's results by submitting a GET request like this
 }
 ```
 
-Alternatively, you can scan multiple domains by POSTing them to `http://server-ip:port/api/v1/scan` with a request body like this:
+Alternatively, you can scan multiple domains by POSTing them to `http://server-ip:port/api/v1/scan` with a request body
+like this:
 
 ```json
 {
@@ -199,6 +212,7 @@ dss serve mail --inboundHost "imap.gmail.com:993" --inboundPass "SomePassword" -
 You can then email this inbox from any address, and you'll receive an email back with your scan results.
 
 ### Global Flags
+
 | Flag             | Short | Description                                                                                                     |
 |------------------|-------|-----------------------------------------------------------------------------------------------------------------|
 | `--advise`       | `-a`  | Provide suggestions for incorrect/missing mail security features                                                |

cmd/dss/main.go

@@ -26,7 +26,7 @@ var (
 		Use:     "dss",
 		Short:   "Scan a domain's DNS records.",
 		Long:    "Scan a domain's DNS records.\nhttps://github.com/GlobalCyberAlliance/domain-security-scanner",
-		Version: "3.0.5",
+		Version: "3.0.6",
 		PersistentPreRun: func(cmd *cobra.Command, args []string) {
 			var logWriter io.Writer
 

cmd/dss/scan.go

@@ -2,12 +2,11 @@ package main
 
 import (
 	"bufio"
-	"os"
-
 	"github.com/GlobalCyberAlliance/domain-security-scanner/pkg/advisor"
 	"github.com/GlobalCyberAlliance/domain-security-scanner/pkg/model"
 	"github.com/GlobalCyberAlliance/domain-security-scanner/pkg/scanner"
 	"github.com/spf13/cobra"
+	"os"
 )
 
 func init() {
@@ -98,7 +97,7 @@ func printResult(result *scanner.Result, domainAdvisor *advisor.Advisor) {
 		ScanResult: result,
 	}
 
-	if result.Error == "" && advise {
+	if advise && result.Error != scanner.ErrInvalidDomain {
 		resultWithAdvice.Advice = domainAdvisor.CheckAll(result.Domain, result.BIMI, result.DKIM, result.DMARC, result.MX, result.SPF)
 	}
 

pkg/advisor/advisor.go

@@ -189,7 +189,7 @@ func (a *Advisor) CheckBIMI(bimi string) (advice []string) {
 
 func (a *Advisor) CheckDKIM(dkim string) (advice []string) {
 	if dkim == "" {
-		return []string{"We couldn't detect any active DKIM record for your domain. Please visit https://dmarcguide.globalcyberalliance.org to fix this."}
+		return []string{"We couldn't detect any active DKIM record for your domain. Due to how DKIM works, we only lookup common/known DKIM selectors (such as x, selector1, google). Visit https://dmarcguide.globalcyberalliance.org for more info on how to configure DKIM for your domain."}
 	}
 
 	if strings.Contains(dkim, ";") {

pkg/http/scan.go

@@ -3,11 +3,10 @@ package http
 import (
 	"context"
 	"fmt"
-	"net/http"
-
 	"github.com/GlobalCyberAlliance/domain-security-scanner/pkg/model"
 	"github.com/GlobalCyberAlliance/domain-security-scanner/pkg/scanner"
 	"github.com/danielgtaylor/huma/v2"
+	"net/http"
 )
 
 func (s *Server) registerScanRoutes() {
@@ -44,11 +43,15 @@ func (s *Server) registerScanRoutes() {
 			return nil, huma.Error500InternalServerError(fmt.Errorf("expected 1 result, got %d", len(results)).Error())
 		}
 
+		if results[0].Error == scanner.ErrInvalidDomain {
+			return nil, huma.Error400BadRequest(scanner.ErrInvalidDomain)
+		}
+
 		result := model.ScanResultWithAdvice{
 			ScanResult: results[0],
 		}
 
-		if s.Advisor != nil && result.ScanResult.Error == "" {
+		if s.Advisor != nil {
 			result.Advice = s.Advisor.CheckAll(result.ScanResult.Domain, result.ScanResult.BIMI, result.ScanResult.DKIM, result.ScanResult.DMARC, result.ScanResult.MX, result.ScanResult.SPF)
 		}
 
@@ -93,7 +96,7 @@ func (s *Server) registerScanRoutes() {
 				ScanResult: result,
 			}
 
-			if s.Advisor != nil && result.Error == "" {
+			if s.Advisor != nil && result.Error != scanner.ErrInvalidDomain {
 				res.Advice = s.Advisor.CheckAll(result.Domain, result.BIMI, result.DKIM, result.DMARC, result.MX, result.SPF)
 			}
 

pkg/mail/server.go

@@ -107,7 +107,7 @@ func (s *Server) handler() error {
 					ScanResult: result,
 				}
 
-				if s.advisor != nil || result.Error == "" {
+				if s.advisor != nil || result.Error != scanner.ErrInvalidDomain {
 					resultWithAdvice.Advice = s.advisor.CheckAll(result.Domain, result.BIMI, result.DKIM, result.DMARC, result.MX, result.SPF)
 				}
 

pkg/scanner/requests.go

@@ -26,6 +26,8 @@ var (
 		"google",        // Google
 		"selector1",     // Microsoft
 		"selector2",     // Microsoft
+		"s1",            // Generic
+		"s2",            // Generic
 		"k1",            // MailChimp
 		"mandrill",      // Mandrill
 		"everlytickey1", // Everlytic
@@ -91,6 +93,11 @@ func (s *Scanner) getDNSAnswers(domain string, recordType uint16) ([]dns.RR, err
 	}
 
 	if in.Rcode != dns.RcodeSuccess {
+		// disregard NXDOMAIN errors
+		if in.Rcode == dns.RcodeNameError {
+			return nil, nil
+		}
+
 		return nil, fmt.Errorf("DNS query failed with rcode %v", in.Rcode)
 	}
 

pkg/scanner/scanner.go

@@ -17,6 +17,10 @@ import (
 	"github.com/spf13/cast"
 )
 
+const (
+	ErrInvalidDomain = "invalid domain name"
+)
+
 type (
 	Scanner struct {
 		// cache is a simple in-memory cache to reduce external requests from the scanner.
@@ -79,8 +83,8 @@ func New(logger zerolog.Logger, timeout time.Duration, opts ...Option) (*Scanner
 	dnsClient.Timeout = timeout
 
 	scanner := &Scanner{
-		dnsClient:   dnsClient, // Initialize a new dns.Client
-		dnsBuffer:   4096,      // Set the dnsBuffer size to 1024 bytes
+		dnsClient:   dnsClient,
+		dnsBuffer:   4096,
 		logger:      logger,
 		nameservers: []string{"8.8.8.8:53", "8.8.4.4:53", "1.1.1.1:53"}, // Set the default nameservers to Google and Cloudflare
 		poolSize:    uint16(runtime.NumCPU()),
@@ -167,7 +171,7 @@ func (s *Scanner) Scan(domains ...string) ([]*Result, error) {
 					// fill variable to satisfy deferred cache fill
 					result = &Result{
 						Domain: domainToScan,
-						Error:  "invalid domain name",
+						Error:  ErrInvalidDomain,
 					}
 
 					mutex.Lock()