From afc9048f7191b93da416bede0a4a67ba466a434a Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Fri, 4 Jan 2019 20:18:02 -0500 Subject: [PATCH 01/17] Move descriptive check strings to backend --- checker/check_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 checker/check_test.go diff --git a/checker/check_test.go b/checker/check_test.go new file mode 100644 index 00000000..de32c12f --- /dev/null +++ b/checker/check_test.go @@ -0,0 +1,41 @@ +package checker + +import ( + "bytes" + "encoding/json" + "testing" +) + +func TestMarshalResultJSON(t *testing.T) { + // Should set description and status_text for CheckResult w/ recognized keys + result := CheckResult{ + Name: "starttls", + Status: Success, + } + marshalled, err := json.Marshal(result) + if err != nil { + t.Fatal(err) + } + if !bytes.Contains(marshalled, []byte("\"status_text\":\"Supports")) { + t.Errorf(string(marshalled)) + } + if !bytes.Contains(marshalled, []byte("\"description\":\"")) { + t.Errorf(string(marshalled)) + } + + // Should survive unrecognized keys + result = CheckResult{ + Name: "foo", + Status: 100, + } + marshalled, _ = json.Marshal(result) + if err != nil { + t.Fatal(err) + } + if bytes.Contains(marshalled, []byte("\"status_text\":\"")) { + t.Errorf(string(marshalled)) + } + if bytes.Contains(marshalled, []byte("\"description\":\"")) { + t.Errorf(string(marshalled)) + } +} From 3cc15c79dad26157898227386a573e90b071a253 Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Mon, 7 Jan 2019 18:32:25 -0500 Subject: [PATCH 02/17] Respond to html requests with html --- api.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/api.go b/api.go index 4e208233..25e3d37a 100644 --- a/api.go +++ b/api.go @@ -79,7 +79,11 @@ func apiWrapper(api apiHandler) func(w http.ResponseWriter, r *http.Request) { packet := raven.NewPacket(response.Message, raven.NewHttp(r)) raven.Capture(packet, nil) } - writeJSON(w, response) + if strings.Contains(r.Header.Get("accept"), "text/html") { + writeHTML(w, response) + } else { + writeJSON(w, response) + } } } @@ -359,3 +363,14 @@ func writeJSON(w http.ResponseWriter, v interface{}) { } fmt.Fprintf(w, "%s\n", b) } + +func writeHTML(w http.ResponseWriter, v interface{}) { + type htmlResponder interface { + WriteHTML(w http.ResponseWriter) error + } + if responder, ok := v.(htmlResponder); ok { + responder.WriteHTML(w) + } else { + http.Error(w, "HTML is not supported for this action.", http.StatusUnsupportedMediaType) + } +} From e08eb2ea1479dfdef603ae078a78f01e4ba7b778 Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Wed, 9 Jan 2019 16:19:19 -0800 Subject: [PATCH 03/17] Render scan html --- api.go | 4 ++-- scan_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/api.go b/api.go index 25e3d37a..10a247a5 100644 --- a/api.go +++ b/api.go @@ -364,11 +364,11 @@ func writeJSON(w http.ResponseWriter, v interface{}) { fmt.Fprintf(w, "%s\n", b) } -func writeHTML(w http.ResponseWriter, v interface{}) { +func writeHTML(w http.ResponseWriter, apiResponse APIResponse) { type htmlResponder interface { WriteHTML(w http.ResponseWriter) error } - if responder, ok := v.(htmlResponder); ok { + if responder, ok := apiResponse.Response.(htmlResponder); ok { responder.WriteHTML(w) } else { http.Error(w, "HTML is not supported for this action.", http.StatusUnsupportedMediaType) diff --git a/scan_test.go b/scan_test.go index 2d1ef7b9..b6e48d39 100644 --- a/scan_test.go +++ b/scan_test.go @@ -11,6 +11,30 @@ import ( "github.com/EFForg/starttls-backend/models" ) +func TestScanHTML(t *testing.T) { + defer teardown() + + // Request a scan! + data := url.Values{} + data.Set("domain", "eff.org") + + req, err := http.NewRequest("POST", server.URL+"/api/scan", strings.NewReader(data.Encode())) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("accept", "text/html") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + // if resp.StatusCode != http.StatusOK { + // t.Fatalf("POST to api/scan failed with error %d", resp.StatusCode) + // } + scanBody, _ := ioutil.ReadAll(resp.Body) + t.Fatal(string(scanBody)) +} + // Tests basic scanning workflow. // Requests a scan for a particular domain, and // makes sure that the scan is persisted correctly in DB across requests. From d72128c6078d37fecfd8633396ff64287276bb61 Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Wed, 9 Jan 2019 16:28:32 -0800 Subject: [PATCH 04/17] Add views --- api.go | 4 ++-- views/scan.html.tmpl | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 views/scan.html.tmpl diff --git a/api.go b/api.go index 10a247a5..bf2f3717 100644 --- a/api.go +++ b/api.go @@ -158,13 +158,13 @@ func (api API) Scan(r *http.Request) APIResponse { return APIResponse{StatusCode: http.StatusOK, Response: scan} } // 1. Conduct scan via starttls-checker - rawScandata, err := api.CheckDomain(api, domain) + scanData, err := api.CheckDomain(api, domain) if err != nil { return APIResponse{StatusCode: http.StatusInternalServerError, Message: err.Error()} } scan = models.Scan{ Domain: domain, - Data: rawScandata, + Data: scanData, Timestamp: time.Now(), } // 2. Put scan into DB diff --git a/views/scan.html.tmpl b/views/scan.html.tmpl new file mode 100644 index 00000000..794b4861 --- /dev/null +++ b/views/scan.html.tmpl @@ -0,0 +1,20 @@ + + +

Scan results for {{ .Domain }}

+

You're viewing unstyled results. You can enable Javascript to view styled content.

+

@todo Domain-level scan results

+ +

Mailboxes

+ {{ range $hostname, $result := .Data.HostnameResults }} +

{{ $hostname }}

+
    + {{ range $check, $checkResult := $result.Checks }} +
  • +

    {{ $checkResult.StatusText }}

    +

    {{ $checkResult.Description }} +

    • + {{ end }} +
+ {{ end }} + + From e04530164ebff452e1382bbbed3ec0903bc007f9 Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Thu, 17 Jan 2019 18:46:47 -0800 Subject: [PATCH 05/17] Render full scan to HTML --- api.go | 10 ++++++++-- models/scan.go | 19 +++++++++++++++++++ models/scan_test.go | 21 +++++++++++++++++++++ scan_test.go | 29 +++++++++++++++++------------ views/scan.html.tmpl | 26 +++++++++++++++++++++++++- 5 files changed, 90 insertions(+), 15 deletions(-) create mode 100644 models/scan_test.go diff --git a/api.go b/api.go index bf2f3717..f6fd7d3d 100644 --- a/api.go +++ b/api.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "fmt" + "io" "log" "net/http" "strings" @@ -366,10 +367,15 @@ func writeJSON(w http.ResponseWriter, v interface{}) { func writeHTML(w http.ResponseWriter, apiResponse APIResponse) { type htmlResponder interface { - WriteHTML(w http.ResponseWriter) error + WriteHTML(io.Writer) error } if responder, ok := apiResponse.Response.(htmlResponder); ok { - responder.WriteHTML(w) + err := responder.WriteHTML(w) + if err != nil { + log.Fatal(err) + } + } else if apiResponse.Message != "" { + http.Error(w, apiResponse.Message, apiResponse.StatusCode) } else { http.Error(w, "HTML is not supported for this action.", http.StatusUnsupportedMediaType) } diff --git a/models/scan.go b/models/scan.go index 6e46544a..07f3c6e0 100644 --- a/models/scan.go +++ b/models/scan.go @@ -1,6 +1,9 @@ package models import ( + "html/template" + "io" + "os" "time" "github.com/EFForg/starttls-backend/checker" @@ -12,3 +15,19 @@ type Scan struct { Data checker.DomainResult `json:"scandata"` // Scan results from starttls-checker Timestamp time.Time `json:"timestamp"` // Time at which this scan was conducted } + +// WriteHTML renders a scan result as HTML. +func (s Scan) WriteHTML(w io.Writer) error { + data := struct { + Scan + BaseURL string + }{ + Scan: s, + BaseURL: os.Getenv("FRONTEND_WEBSITE_LINK"), + } + tmpl, err := template.ParseFiles("views/scan.html.tmpl") + if err != nil { + return err + } + return tmpl.Execute(w, data) +} diff --git a/models/scan_test.go b/models/scan_test.go new file mode 100644 index 00000000..147e93fd --- /dev/null +++ b/models/scan_test.go @@ -0,0 +1,21 @@ +package models + +import ( + "bytes" + "fmt" + "testing" + + "github.com/EFForg/starttls-backend/checker" +) + +func TestWriteScanHTML(t *testing.T) { + scan := Scan{ + Data: checker.DomainResult{ + Domain: "eff.org", + }, + Domain: "eff.org", + } + var html bytes.Buffer + scan.WriteHTML(&html) + fmt.Println(html.String()) +} diff --git a/scan_test.go b/scan_test.go index b6e48d39..251ebd5c 100644 --- a/scan_test.go +++ b/scan_test.go @@ -11,28 +11,33 @@ import ( "github.com/EFForg/starttls-backend/models" ) +func htmlPost(path string, data url.Values) (*http.Response, error) { + req, err := http.NewRequest("POST", server.URL+path, strings.NewReader(data.Encode())) + if err != nil { + return &http.Response{}, err + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("accept", "text/html") + return http.DefaultClient.Do(req) +} + func TestScanHTML(t *testing.T) { defer teardown() // Request a scan! data := url.Values{} data.Set("domain", "eff.org") - - req, err := http.NewRequest("POST", server.URL+"/api/scan", strings.NewReader(data.Encode())) + resp, err := htmlPost("/api/scan", data) if err != nil { t.Fatal(err) } - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("accept", "text/html") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatal(err) + if resp.StatusCode != http.StatusOK { + t.Errorf("HTML POST to api/scan failed with error %d", resp.StatusCode) + } + body, _ := ioutil.ReadAll(resp.Body) + if !strings.Contains(string(body), "eff.org") { + t.Errorf("Response should contain scan domain, got %s", string(body)) } - // if resp.StatusCode != http.StatusOK { - // t.Fatalf("POST to api/scan failed with error %d", resp.StatusCode) - // } - scanBody, _ := ioutil.ReadAll(resp.Body) - t.Fatal(string(scanBody)) } // Tests basic scanning workflow. diff --git a/views/scan.html.tmpl b/views/scan.html.tmpl index 794b4861..77b1ac97 100644 --- a/views/scan.html.tmpl +++ b/views/scan.html.tmpl @@ -2,7 +2,31 @@

Scan results for {{ .Domain }}

You're viewing unstyled results. You can enable Javascript to view styled content.

-

@todo Domain-level scan results

+

Summary

+ {{ if eq .Data.Status 0 }} +

Congratulations, your domain passed all checks.

+ {{ else if eq .Data.Status 1 }} +

Your domain passed all checks with some warnings. See below for details.

+ {{ else }} +

There were some problems with your domain. See below for details.

+ {{ end }} + + {{ if ne .Data.Message "" }} +

{{ .Data.Message }}

+ {{ end }} + +

STARTTLS Everywhere Policy List

+ {{ if eq .Data.Status 0 }} + requiredVars[varName] = os.Getenv(varName) +

Your mail domain is on the STARTTLS Everywhere policy list. See {{ printf "%s/faq#what-list" .BaseURL }} for details

+ {{ else }} +

Your mail domain is not on the STARTTLS Everywhere policy list. See {{ printf "%s/faq#what-list" .BaseURL }} for details

+ {{ end }} + + {{ $policyListResult := index .Data.ExtraResults "policy-list" }} + {{ range $message := $policyListResult.Messages }} +

{{ $message }}

+ {{ end}}

Mailboxes

{{ range $hostname, $result := .Data.HostnameResults }} From 88ada4e24c2c36c2b2b2280eae4e19c57f9f468a Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Thu, 17 Jan 2019 18:46:02 -0800 Subject: [PATCH 06/17] Define check types as global vars --- api.go | 4 +-- checker/check_test.go | 41 ------------------------------ checker/domain_test.go | 24 +++++++++--------- checker/hostname.go | 54 +++++++++++++++++++++++++++++++++++----- checker/hostname_test.go | 38 ++++++++++++++-------------- checker/mta_sts.go | 24 ++++++++++++++++-- checker/mta_sts_test.go | 8 +++--- 7 files changed, 107 insertions(+), 86 deletions(-) delete mode 100644 checker/check_test.go diff --git a/api.go b/api.go index f6fd7d3d..8b3254e3 100644 --- a/api.go +++ b/api.go @@ -89,8 +89,8 @@ func apiWrapper(api apiHandler) func(w http.ResponseWriter, r *http.Request) { } // Checks the policy status of this domain. -func (api API) policyCheck(domain string) *checker.Result { - result := checker.Result{Name: "policylist"} +func (api API) policyCheck(domain string) checker.CheckResult { + result := checker.CheckResult{CheckType: checker.CheckType{Name: "policylist"}} if _, err := api.List.Get(domain); err == nil { return result.Success() } diff --git a/checker/check_test.go b/checker/check_test.go deleted file mode 100644 index de32c12f..00000000 --- a/checker/check_test.go +++ /dev/null @@ -1,41 +0,0 @@ -package checker - -import ( - "bytes" - "encoding/json" - "testing" -) - -func TestMarshalResultJSON(t *testing.T) { - // Should set description and status_text for CheckResult w/ recognized keys - result := CheckResult{ - Name: "starttls", - Status: Success, - } - marshalled, err := json.Marshal(result) - if err != nil { - t.Fatal(err) - } - if !bytes.Contains(marshalled, []byte("\"status_text\":\"Supports")) { - t.Errorf(string(marshalled)) - } - if !bytes.Contains(marshalled, []byte("\"description\":\"")) { - t.Errorf(string(marshalled)) - } - - // Should survive unrecognized keys - result = CheckResult{ - Name: "foo", - Status: 100, - } - marshalled, _ = json.Marshal(result) - if err != nil { - t.Fatal(err) - } - if bytes.Contains(marshalled, []byte("\"status_text\":\"")) { - t.Errorf(string(marshalled)) - } - if bytes.Contains(marshalled, []byte("\"description\":\"")) { - t.Errorf(string(marshalled)) - } -} diff --git a/checker/domain_test.go b/checker/domain_test.go index 172937f7..679f0c24 100644 --- a/checker/domain_test.go +++ b/checker/domain_test.go @@ -21,23 +21,23 @@ var mxLookup = map[string][]string{ // Fake hostname checks :) var hostnameResults = map[string]Result{ "noconnection": Result{ - Status: Error, + Status: 3, Checks: map[string]*Result{ - "connectivity": {"connectivity", Error, nil, nil}, + Connectivity: {Connectivity, 3, nil, nil}, }, }, "nostarttls": Result{ - Status: Failure, + Status: 2, Checks: map[string]*Result{ - "connectivity": {"connectivity", 0, nil, nil}, - "starttls": {"starttls", Failure, nil, nil}, + Connectivity: {Connectivity, 0, nil, nil}, + STARTTLS: {STARTTLS, 2, nil, nil}, }, }, "nostarttlsconnect": Result{ - Status: Error, + Status: 3, Checks: map[string]*Result{ - "connectivity": {"connectivity", 0, nil, nil}, - "starttls": {"starttls", Error, nil, nil}, + Connectivity: {Connectivity, 0, nil, nil}, + STARTTLS: {STARTTLS, 3, nil, nil}, }, }, } @@ -69,10 +69,10 @@ func mockCheckHostname(domain string, hostname string) HostnameResult { Result: &Result{ Status: 0, Checks: map[string]*Result{ - "connectivity": {"connectivity", 0, nil, nil}, - "starttls": {"starttls", 0, nil, nil}, - "certificate": {"certificate", 0, nil, nil}, - "version": {"version", 0, nil, nil}, + Connectivity: {Connectivity, 0, nil, nil}, + STARTTLS: {STARTTLS, 0, nil, nil}, + Certificate: {Certificate, 0, nil, nil}, + Version: {Version, 0, nil, nil}, }, }, Timestamp: time.Now(), diff --git a/checker/hostname.go b/checker/hostname.go index 44bdb74b..1bbc9ea7 100644 --- a/checker/hostname.go +++ b/checker/hostname.go @@ -95,7 +95,7 @@ func smtpDialWithTimeout(hostname string, timeout time.Duration) (*smtp.Client, // Simply tries to StartTLS with the server. func checkStartTLS(client *smtp.Client) *Result { - result := MakeResult("starttls") + result := MakeResult(STARTTLS) ok, _ := client.Extension("StartTLS") if !ok { return result.Failure("Server does not advertise support for STARTTLS.") @@ -141,7 +141,7 @@ var certRoots *x509.CertPool // Checks that the certificate presented is valid for a particular hostname, unexpired, // and chains to a trusted root. func checkCert(client *smtp.Client, domain, hostname string) *Result { - result := MakeResult("certificate") + result := MakeResult(Certificate) state, ok := client.TLSConnectionState() if !ok { return result.Error("TLS not initiated properly.") @@ -168,8 +168,8 @@ func tlsConfigForCipher(ciphers []uint16) tls.Config { } // Checks to see that insecure ciphers are disabled. -func checkTLSCipher(hostname string, timeout time.Duration) *Result { - result := MakeResult("cipher") +func checkTLSCipher(hostname string, timeout time.Duration) CheckResult { + result := CheckResult{CheckType: CheckType{Name: "cipher"}} badCiphers := []uint16{ tls.TLS_RSA_WITH_RC4_128_SHA, tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, @@ -188,7 +188,7 @@ func checkTLSCipher(hostname string, timeout time.Duration) *Result { } func checkTLSVersion(client *smtp.Client, hostname string, timeout time.Duration) *Result { - result := MakeResult("version") + result := MakeResult(Version) // Check the TLS version of the existing connection. tlsConnectionState, ok := client.TLSConnectionState() @@ -235,7 +235,7 @@ func (c *Checker) CheckHostname(domain string, hostname string) HostnameResult { } // Connect to the SMTP server and use that connection to perform as many checks as possible. - connectivityResult := MakeResult("connectivity") + connectivityResult := MakeResult(Connectivity) client, err := smtpDialWithTimeout(hostname, c.timeout()) if err != nil { result.addCheck(connectivityResult.Error("Could not establish connection: %v", err)) @@ -256,3 +256,45 @@ func (c *Checker) CheckHostname(domain string, hostname string) HostnameResult { return result } + +// Supported checks against hostnames +var ( + STARTTLS CheckType = CheckType{ + Name: "starttls", + StatusText: StatusText{ + Success: "Supports STTARTTLS", + Failure: "Does not support STARTTLS", + }, + Description: `“STARTTLS” is the command an email server sends if it wants to encrypt communications (using Transport Layer Security or “TLS”) with another email server. If your server supports STARTTLS, that means any other server that supports STARTTLS can communicate securely with it. + +This checks that your email server sends the STARTTLS command correctly, as well as accepting the STARTTLS command from other servers.`, + } + Version CheckType = CheckType{ + Name: "version", + StatusText: StatusText{ + Success: "Uses a secure version of TLS", + Failure: "Does not use a secure TLS version", + }, + Description: `TLS has changed many times over the years. Researchers have discovered security flaws in some older versions, named “SSLv2” and “SSLv3”, so technologists across the internet are working to deprecate SSLv2/3. + +This checks that your email server does not allow establishing a valid TLS connection over SSLv2/3.`, + } + Certificate CheckType = CheckType{ + Name: "certificate", + StatusText: StatusText{ + Success: "Presents a valid certificate", + Failure: "Does not present a valid certificate", + }, + Description: `On the internet, even if you *think* you’re talking to a service named “eff.org”, it could be an impersonator pretending to be “eff.org”. Checking a mail server’s certificate helps ensure that you really are talking to the actual service. + +In order for your certificate to be valid for your email domain, it should be unexpired, chain to a valid root, and one of the names on the certificate should either match the domain (the part of an email address after the @) or the server’s hostname (the name of the server, as indicated by an MX record).`, + } + Connectivity CheckType = CheckType{ + Name: "connectivity", + StatusText: StatusText{ + Success: "Server is up and running", + Failure: "Could not establish connection", + }, + Description: `We couldn't successfully connect to this mailserver to scan it. This could be an error on our side, too. If you're having trouble getting the scanner to work, shoot us an email at starttls-policy@eff.org.`, + } +) diff --git a/checker/hostname_test.go b/checker/hostname_test.go index a6fe9447..272d18bb 100644 --- a/checker/hostname_test.go +++ b/checker/hostname_test.go @@ -107,7 +107,7 @@ func TestNoConnection(t *testing.T) { expected := Result{ Status: 3, Checks: map[string]*Result{ - "connectivity": {"connectivity", 3, nil, nil}, + "connectivity": {Connectivity, 3, nil, nil}, }, } compareStatuses(t, expected, result) @@ -122,8 +122,8 @@ func TestNoTLS(t *testing.T) { expected := Result{ Status: 2, Checks: map[string]*Result{ - "connectivity": {"connectivity", 0, nil, nil}, - "starttls": {"starttls", 2, nil, nil}, + Connectivity: {Connectivity, 0, nil, nil}, + STARTTLS: {STARTTLS, 2, nil, nil}, }, } compareStatuses(t, expected, result) @@ -142,10 +142,10 @@ func TestSelfSigned(t *testing.T) { expected := Result{ Status: 2, Checks: map[string]*Result{ - "connectivity": {"connectivity", 0, nil, nil}, - "starttls": {"starttls", 0, nil, nil}, - "certificate": {"certificate", 2, nil, nil}, - "version": {"version", 0, nil, nil}, + Connectivity: {Connectivity, 0, nil, nil}, + STARTTLS: {STARTTLS, 0, nil, nil}, + Certificate: {Certificate, 2, nil, nil}, + Version: {Version, 0, nil, nil}, }, } compareStatuses(t, expected, result) @@ -168,10 +168,10 @@ func TestNoTLS12(t *testing.T) { expected := Result{ Status: 2, Checks: map[string]*Result{ - "connectivity": {"connectivity", 0, nil, nil}, - "starttls": {"starttls", 0, nil, nil}, - "certificate": {"certificate", 2, nil, nil}, - "version": {"version", 1, nil, nil}, + Connectivity: {Connectivity, 0, nil, nil}, + STARTTLS: {STARTTLS, 0, nil, nil}, + Certificate: {Certificate, 2, nil, nil}, + Version: {Version, 1, nil, nil}, }, } compareStatuses(t, expected, result) @@ -200,10 +200,10 @@ func TestSuccessWithFakeCA(t *testing.T) { expected := Result{ Status: 0, Checks: map[string]*Result{ - "connectivity": {"connectivity", 0, nil, nil}, - "starttls": {"starttls", 0, nil, nil}, - "certificate": {"certificate", 0, nil, nil}, - "version": {"version", 0, nil, nil}, + Connectivity: {Connectivity, 0, nil, nil}, + STARTTLS: {STARTTLS, 0, nil, nil}, + Certificate: {Certificate, 0, nil, nil}, + Version: {Version, 0, nil, nil}, }, } compareStatuses(t, expected, result) @@ -275,10 +275,10 @@ func TestFailureWithBadHostname(t *testing.T) { expected := Result{ Status: 2, Checks: map[string]*Result{ - "connectivity": {"connectivity", 0, nil, nil}, - "starttls": {"starttls", 0, nil, nil}, - "certificate": {"certificate", 2, nil, nil}, - "version": {"version", 0, nil, nil}, + Connectivity: {Connectivity, 0, nil, nil}, + STARTTLS: {STARTTLS, 0, nil, nil}, + Certificate: {Certificate, 2, nil, nil}, + Version: {Version, 0, nil, nil}, }, } compareStatuses(t, expected, result) diff --git a/checker/mta_sts.go b/checker/mta_sts.go index e516974b..4145b368 100644 --- a/checker/mta_sts.go +++ b/checker/mta_sts.go @@ -40,7 +40,7 @@ func getKeyValuePairs(record string, lineDelimiter string, } func checkMTASTSRecord(domain string) *Result { - result := MakeResult("mta-sts-txt") + result := MakeResult(MTASTSText) records, err := net.LookupTXT(fmt.Sprintf("_mta-sts.%s", domain)) if err != nil { return result.Failure("Couldn't find MTA-STS TXT record: %v", err) @@ -63,7 +63,7 @@ func validateMTASTSRecord(records []string, result *Result) *Result { } func checkMTASTSPolicyFile(domain string, hostnameResults map[string]HostnameResult) *Result { - result := MakeResult("policy-file") + result := MakeResult(MTASTSPolicyFile) client := &http.Client{ // Don't follow redirects. CheckRedirect: func(req *http.Request, via []*http.Request) error { @@ -144,3 +144,23 @@ func checkMTASTS(domain string, hostnameResults map[string]HostnameResult) *Resu result.addCheck(checkMTASTSPolicyFile(domain, hostnameResults)) return result } + +// Types of MTA-STS checks supported +var ( + MTASTSText CheckType = CheckType{ + Name: "mta-sts-text", + StatusText: StatusText{ + Success: "", + Failure: "", + }, + Description: ``, + } + MTASTSPolicyFile CheckType = CheckType{ + Name: "mta-sts-policy-file", + StatusText: StatusText{ + Success: "", + Failure: "", + }, + Description: ``, + } +) diff --git a/checker/mta_sts_test.go b/checker/mta_sts_test.go index 41e033e1..a717f095 100644 --- a/checker/mta_sts_test.go +++ b/checker/mta_sts_test.go @@ -79,8 +79,8 @@ func TestValidateMTASTSMXs(t *testing.T) { Result: &Result{ Status: 3, Checks: map[string]*Result{ - "connectivity": {"connectivity", 0, nil, nil}, - "starttls": {"starttls", 0, nil, nil}, + "connectivity": {Connectivity, 0, nil, nil}, + "starttls": {STARTTLS, 0, nil, nil}, }, }, } @@ -88,8 +88,8 @@ func TestValidateMTASTSMXs(t *testing.T) { Result: &Result{ Status: 3, Checks: map[string]*Result{ - "connectivity": {"connectivity", 0, nil, nil}, - "starttls": {"starttls", 3, nil, nil}, + "connectivity": {Connectivity, 0, nil, nil}, + "starttls": {STARTTLS, 3, nil, nil}, }, }, } From 55cb75afd1d057c1ad5b2608ae404f4b93c5e70a Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Tue, 22 Jan 2019 17:58:39 -0800 Subject: [PATCH 07/17] Let controller choose template --- api.go | 54 +++++++++++++++++++++++++++++--------------- models/scan.go | 19 ---------------- models/scan_test.go | 21 ----------------- scan_test.go | 3 +++ views/scan.html.tmpl | 16 ++++++------- 5 files changed, 47 insertions(+), 66 deletions(-) delete mode 100644 models/scan_test.go diff --git a/api.go b/api.go index 8b3254e3..ea81abc3 100644 --- a/api.go +++ b/api.go @@ -3,9 +3,10 @@ package main import ( "encoding/json" "fmt" - "io" + "html/template" "log" "net/http" + "os" "strings" "time" @@ -63,9 +64,10 @@ type EmailSender interface { // APIResponse wraps all the responses from this API. type APIResponse struct { - StatusCode int `json:"status_code"` - Message string `json:"message"` - Response interface{} `json:"response"` + StatusCode int `json:"status_code"` + Message string `json:"message"` + Response interface{} `json:"response"` + TemplatePath string `json:"omit"` } type apiHandler func(r *http.Request) APIResponse @@ -80,7 +82,8 @@ func apiWrapper(api apiHandler) func(w http.ResponseWriter, r *http.Request) { packet := raven.NewPacket(response.Message, raven.NewHttp(r)) raven.Capture(packet, nil) } - if strings.Contains(r.Header.Get("accept"), "text/html") { + if strings.Contains(r.Header.Get("accept"), "text/html") && + response.TemplatePath != "" { writeHTML(w, response) } else { writeJSON(w, response) @@ -156,7 +159,11 @@ func (api API) Scan(r *http.Request) APIResponse { // 0. If last scan was recent, return cached scan. scan, err := api.Database.GetLatestScan(domain) if err == nil && time.Now().Before(scan.Timestamp.Add(cacheScanTime)) { - return APIResponse{StatusCode: http.StatusOK, Response: scan} + return APIResponse{ + StatusCode: http.StatusOK, + Response: scan, + TemplatePath: "views/scan.html.tmpl", + } } // 1. Conduct scan via starttls-checker scanData, err := api.CheckDomain(api, domain) @@ -173,7 +180,11 @@ func (api API) Scan(r *http.Request) APIResponse { if err != nil { return APIResponse{StatusCode: http.StatusInternalServerError, Message: err.Error()} } - return APIResponse{StatusCode: http.StatusOK, Response: scan} + return APIResponse{ + StatusCode: http.StatusOK, + Response: scan, + TemplatePath: "views/scan.html.tmpl", + } // GET: Just fetch the most recent scan } else if r.Method == http.MethodGet { scan, err := api.Database.GetLatestScan(domain) @@ -366,17 +377,24 @@ func writeJSON(w http.ResponseWriter, v interface{}) { } func writeHTML(w http.ResponseWriter, apiResponse APIResponse) { - type htmlResponder interface { - WriteHTML(io.Writer) error + // Add some additional useful fields for use in templates. + data := struct { + APIResponse + BaseURL string + }{ + APIResponse: apiResponse, + BaseURL: os.Getenv("FRONTEND_WEBSITE_LINK"), } - if responder, ok := apiResponse.Response.(htmlResponder); ok { - err := responder.WriteHTML(w) - if err != nil { - log.Fatal(err) - } - } else if apiResponse.Message != "" { - http.Error(w, apiResponse.Message, apiResponse.StatusCode) - } else { - http.Error(w, "HTML is not supported for this action.", http.StatusUnsupportedMediaType) + tmpl, err := template.ParseFiles(apiResponse.TemplatePath) + if err != nil { + http.Error(w, "Internal error: could not parse template.", http.StatusInternalServerError) + return + } + err = tmpl.Execute(w, data) + // We can't write a 500 status header if tmpl.Execute fails because Execute + // may have already written to the status and body of w. + // @TODO make sure this logs to Raven + if err != nil { + log.Fatal(err) } } diff --git a/models/scan.go b/models/scan.go index 07f3c6e0..6e46544a 100644 --- a/models/scan.go +++ b/models/scan.go @@ -1,9 +1,6 @@ package models import ( - "html/template" - "io" - "os" "time" "github.com/EFForg/starttls-backend/checker" @@ -15,19 +12,3 @@ type Scan struct { Data checker.DomainResult `json:"scandata"` // Scan results from starttls-checker Timestamp time.Time `json:"timestamp"` // Time at which this scan was conducted } - -// WriteHTML renders a scan result as HTML. -func (s Scan) WriteHTML(w io.Writer) error { - data := struct { - Scan - BaseURL string - }{ - Scan: s, - BaseURL: os.Getenv("FRONTEND_WEBSITE_LINK"), - } - tmpl, err := template.ParseFiles("views/scan.html.tmpl") - if err != nil { - return err - } - return tmpl.Execute(w, data) -} diff --git a/models/scan_test.go b/models/scan_test.go deleted file mode 100644 index 147e93fd..00000000 --- a/models/scan_test.go +++ /dev/null @@ -1,21 +0,0 @@ -package models - -import ( - "bytes" - "fmt" - "testing" - - "github.com/EFForg/starttls-backend/checker" -) - -func TestWriteScanHTML(t *testing.T) { - scan := Scan{ - Data: checker.DomainResult{ - Domain: "eff.org", - }, - Domain: "eff.org", - } - var html bytes.Buffer - scan.WriteHTML(&html) - fmt.Println(html.String()) -} diff --git a/scan_test.go b/scan_test.go index 251ebd5c..3268b737 100644 --- a/scan_test.go +++ b/scan_test.go @@ -35,6 +35,9 @@ func TestScanHTML(t *testing.T) { t.Errorf("HTML POST to api/scan failed with error %d", resp.StatusCode) } body, _ := ioutil.ReadAll(resp.Body) + if !strings.Contains(strings.ToLower(string(body)), " -

Scan results for {{ .Domain }}

+

Scan results for {{ .Response.Domain }}

You're viewing unstyled results. You can enable Javascript to view styled content.

Summary

- {{ if eq .Data.Status 0 }} + {{ if eq .Response.Data.Status 0 }}

Congratulations, your domain passed all checks.

- {{ else if eq .Data.Status 1 }} + {{ else if eq .Response.Data.Status 1 }}

Your domain passed all checks with some warnings. See below for details.

{{ else }}

There were some problems with your domain. See below for details.

{{ end }} - {{ if ne .Data.Message "" }} -

{{ .Data.Message }}

+ {{ if ne .Response.Data.Message "" }} +

{{ .Response.Data.Message }}

{{ end }}

STARTTLS Everywhere Policy List

- {{ if eq .Data.Status 0 }} + {{ if eq .Response.Data.Status 0 }} requiredVars[varName] = os.Getenv(varName)

Your mail domain is on the STARTTLS Everywhere policy list. See {{ printf "%s/faq#what-list" .BaseURL }} for details

{{ else }}

Your mail domain is not on the STARTTLS Everywhere policy list. See {{ printf "%s/faq#what-list" .BaseURL }} for details

{{ end }} - {{ $policyListResult := index .Data.ExtraResults "policy-list" }} + {{ $policyListResult := index .Response.Data.ExtraResults "policy-list" }} {{ range $message := $policyListResult.Messages }}

{{ $message }}

{{ end}}

Mailboxes

- {{ range $hostname, $result := .Data.HostnameResults }} + {{ range $hostname, $result := .Response.Data.HostnameResults }}

{{ $hostname }}

    {{ range $check, $checkResult := $result.Checks }} From b13179d13cebdee06fe3a1e0184ba80c66f6b41a Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Wed, 23 Jan 2019 11:42:35 -0800 Subject: [PATCH 08/17] Render domain status to HTML --- api.go | 19 ++++++++++++++++--- api_test.go | 22 ++++++++++++++++++++++ queue_test.go | 13 +++++++++++++ scan_test.go | 23 +++-------------------- views/domain.html.tmpl | 19 +++++++++++++++++++ 5 files changed, 73 insertions(+), 23 deletions(-) create mode 100644 views/domain.html.tmpl diff --git a/api.go b/api.go index ea81abc3..3d637ea3 100644 --- a/api.go +++ b/api.go @@ -281,7 +281,11 @@ func (api API) Queue(r *http.Request) APIResponse { // 2. Create token for domain token, err := api.Database.PutToken(domain) if err != nil { - return APIResponse{StatusCode: http.StatusInternalServerError, Message: err.Error()} + return APIResponse{ + StatusCode: http.StatusInternalServerError, + Message: err.Error(), + TemplatePath: "views/domain.html.tmpl", + } } // 3. Send email @@ -291,14 +295,22 @@ func (api API) Queue(r *http.Request) APIResponse { return APIResponse{StatusCode: http.StatusInternalServerError, Message: "Unable to send validation e-mail"} } - return APIResponse{StatusCode: http.StatusOK, Response: domainData} + return APIResponse{ + StatusCode: http.StatusOK, + Response: domainData, + TemplatePath: "views/domain.html.tmpl", + } // GET: Retrieve domain status from queue } else if r.Method == http.MethodGet { status, err := api.Database.GetDomain(domain) if err != nil { return APIResponse{StatusCode: http.StatusNotFound, Message: err.Error()} } - return APIResponse{StatusCode: http.StatusOK, Response: status} + return APIResponse{ + StatusCode: http.StatusOK, + Response: status, + TemplatePath: "views/domain.html.tmpl", + } } else { return APIResponse{StatusCode: http.StatusMethodNotAllowed, Message: "/api/queue only accepts POST and GET requests"} @@ -387,6 +399,7 @@ func writeHTML(w http.ResponseWriter, apiResponse APIResponse) { } tmpl, err := template.ParseFiles(apiResponse.TemplatePath) if err != nil { + log.Fatal(err) http.Error(w, "Internal error: could not parse template.", http.StatusInternalServerError) return } diff --git a/api_test.go b/api_test.go index 923d4b4b..51b585e4 100644 --- a/api_test.go +++ b/api_test.go @@ -1,6 +1,10 @@ package main import ( + "io/ioutil" + "net/http" + "net/url" + "strings" "testing" "github.com/EFForg/starttls-backend/checker" @@ -40,3 +44,21 @@ func TestPolicyCheckWithQueuedDomain(t *testing.T) { t.Errorf("Check should have warned.") } } + +func testHTMLPost(path string, data url.Values, t *testing.T) ([]byte, int) { + req, err := http.NewRequest("POST", server.URL+path, strings.NewReader(data.Encode())) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("accept", "text/html") + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + body, _ := ioutil.ReadAll(resp.Body) + if !strings.Contains(strings.ToLower(string(body)), " + + {{ if or (eq .Response.State "unknown") (eq .Response.State "") }} + Something went wrong - your domain was not submitted to the STARTTLS policy list. + {{ end }} + {{ if eq .Response.State "unvalidated" }} + Thank you for submitting your domain. Please check postmaster@{{ .Response.Name }} to validate that you control the domain. + {{ end }} + {{ if eq .Response.State "queued" }} + Thank you for submitting your domain. Your domain is queued to be reviewed and added to the STARTTLS policy list. + {{ end }} + {{ if eq .Response.State "failed" }} + Your domain does not qualify for addition to the STARTTLS policy list. Please visit {{ .BaseURL }} to review your domain's status and correct any issues. + {{ end }} + {{ if eq .Response.State "added" }} + Your domain is already on the STARTTLS policy list. Thanks for helping to make e-mail more secure. + {{ end }} + + From 092fc15524fb4f381d9c7f1ff5115adccb26ea4c Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Wed, 23 Jan 2019 17:24:55 -0800 Subject: [PATCH 09/17] Log template errors to sentry --- api.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/api.go b/api.go index 3d637ea3..bf93478a 100644 --- a/api.go +++ b/api.go @@ -399,15 +399,14 @@ func writeHTML(w http.ResponseWriter, apiResponse APIResponse) { } tmpl, err := template.ParseFiles(apiResponse.TemplatePath) if err != nil { - log.Fatal(err) + raven.CaptureError(err, nil) http.Error(w, "Internal error: could not parse template.", http.StatusInternalServerError) return } err = tmpl.Execute(w, data) // We can't write a 500 status header if tmpl.Execute fails because Execute // may have already written to the status and body of w. - // @TODO make sure this logs to Raven if err != nil { - log.Fatal(err) + raven.CaptureError(err, nil) } } From f87c30cb8c67953b86b19a2731930a46acff7e39 Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Fri, 25 Jan 2019 13:18:42 -0800 Subject: [PATCH 10/17] Keep long descriptive text on frontend --- api.go | 6 +++-- checker/hostname.go | 50 ++++----------------------------------- checker/mta_sts.go | 22 +---------------- checker/result.go | 56 +++++++++++++++++++++++++++++++++++++++++++- views/scan.html.tmpl | 49 +++++++++++++++++++------------------- 5 files changed, 88 insertions(+), 95 deletions(-) diff --git a/api.go b/api.go index bf93478a..b1c682f0 100644 --- a/api.go +++ b/api.go @@ -92,8 +92,8 @@ func apiWrapper(api apiHandler) func(w http.ResponseWriter, r *http.Request) { } // Checks the policy status of this domain. -func (api API) policyCheck(domain string) checker.CheckResult { - result := checker.CheckResult{CheckType: checker.CheckType{Name: "policylist"}} +func (api API) policyCheck(domain string) *checker.Result { + result := checker.Result{Name: checker.PolicyList} if _, err := api.List.Get(domain); err == nil { return result.Success() } @@ -399,6 +399,7 @@ func writeHTML(w http.ResponseWriter, apiResponse APIResponse) { } tmpl, err := template.ParseFiles(apiResponse.TemplatePath) if err != nil { + log.Println(err) raven.CaptureError(err, nil) http.Error(w, "Internal error: could not parse template.", http.StatusInternalServerError) return @@ -407,6 +408,7 @@ func writeHTML(w http.ResponseWriter, apiResponse APIResponse) { // We can't write a 500 status header if tmpl.Execute fails because Execute // may have already written to the status and body of w. if err != nil { + log.Println(err) raven.CaptureError(err, nil) } } diff --git a/checker/hostname.go b/checker/hostname.go index 1bbc9ea7..05934b29 100644 --- a/checker/hostname.go +++ b/checker/hostname.go @@ -28,11 +28,11 @@ func (h HostnameResult) checkSucceeded(checkName string) bool { } func (h HostnameResult) couldConnect() bool { - return h.checkSucceeded("connectivity") + return h.checkSucceeded(Connectivity) } func (h HostnameResult) couldSTARTTLS() bool { - return h.checkSucceeded("starttls") + return h.checkSucceeded(STARTTLS) } // Modelled after policyMatches in Appendix B of the MTA-STS RFC 8641. @@ -168,8 +168,8 @@ func tlsConfigForCipher(ciphers []uint16) tls.Config { } // Checks to see that insecure ciphers are disabled. -func checkTLSCipher(hostname string, timeout time.Duration) CheckResult { - result := CheckResult{CheckType: CheckType{Name: "cipher"}} +func checkTLSCipher(hostname string, timeout time.Duration) *Result { + result := MakeResult("cipher") badCiphers := []uint16{ tls.TLS_RSA_WITH_RC4_128_SHA, tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, @@ -256,45 +256,3 @@ func (c *Checker) CheckHostname(domain string, hostname string) HostnameResult { return result } - -// Supported checks against hostnames -var ( - STARTTLS CheckType = CheckType{ - Name: "starttls", - StatusText: StatusText{ - Success: "Supports STTARTTLS", - Failure: "Does not support STARTTLS", - }, - Description: `“STARTTLS” is the command an email server sends if it wants to encrypt communications (using Transport Layer Security or “TLS”) with another email server. If your server supports STARTTLS, that means any other server that supports STARTTLS can communicate securely with it. - -This checks that your email server sends the STARTTLS command correctly, as well as accepting the STARTTLS command from other servers.`, - } - Version CheckType = CheckType{ - Name: "version", - StatusText: StatusText{ - Success: "Uses a secure version of TLS", - Failure: "Does not use a secure TLS version", - }, - Description: `TLS has changed many times over the years. Researchers have discovered security flaws in some older versions, named “SSLv2” and “SSLv3”, so technologists across the internet are working to deprecate SSLv2/3. - -This checks that your email server does not allow establishing a valid TLS connection over SSLv2/3.`, - } - Certificate CheckType = CheckType{ - Name: "certificate", - StatusText: StatusText{ - Success: "Presents a valid certificate", - Failure: "Does not present a valid certificate", - }, - Description: `On the internet, even if you *think* you’re talking to a service named “eff.org”, it could be an impersonator pretending to be “eff.org”. Checking a mail server’s certificate helps ensure that you really are talking to the actual service. - -In order for your certificate to be valid for your email domain, it should be unexpired, chain to a valid root, and one of the names on the certificate should either match the domain (the part of an email address after the @) or the server’s hostname (the name of the server, as indicated by an MX record).`, - } - Connectivity CheckType = CheckType{ - Name: "connectivity", - StatusText: StatusText{ - Success: "Server is up and running", - Failure: "Could not establish connection", - }, - Description: `We couldn't successfully connect to this mailserver to scan it. This could be an error on our side, too. If you're having trouble getting the scanner to work, shoot us an email at starttls-policy@eff.org.`, - } -) diff --git a/checker/mta_sts.go b/checker/mta_sts.go index 4145b368..ee509c17 100644 --- a/checker/mta_sts.go +++ b/checker/mta_sts.go @@ -139,28 +139,8 @@ func validateMTASTSMXs(policyFileMXs []string, dnsMXs map[string]HostnameResult, } func checkMTASTS(domain string, hostnameResults map[string]HostnameResult) *Result { - result := MakeResult("mta-sts") + result := MakeResult(MTASTS) result.addCheck(checkMTASTSRecord(domain)) result.addCheck(checkMTASTSPolicyFile(domain, hostnameResults)) return result } - -// Types of MTA-STS checks supported -var ( - MTASTSText CheckType = CheckType{ - Name: "mta-sts-text", - StatusText: StatusText{ - Success: "", - Failure: "", - }, - Description: ``, - } - MTASTSPolicyFile CheckType = CheckType{ - Name: "mta-sts-policy-file", - StatusText: StatusText{ - Success: "", - Failure: "", - }, - Description: ``, - } -) diff --git a/checker/result.go b/checker/result.go index e588b0da..1ebd1b05 100644 --- a/checker/result.go +++ b/checker/result.go @@ -1,6 +1,9 @@ package checker -import "fmt" +import ( + "encoding/json" + "fmt" +) // CheckStatus is an enum encoding the status of the overall check. type CheckStatus int32 @@ -13,6 +16,17 @@ const ( Error CheckStatus = 3 ) +var statusText = map[CheckStatus]string{ + Success: "Success", + Warning: "Warning", + Failure: "Failure", + Error: "Error", +} + +func (r Result) StatusText() string { + return statusText[r.Status] +} + // SetStatus the resulting status of combining old & new. The order of priority // for CheckStatus goes: Error > Failure > Warning > Success func SetStatus(oldStatus CheckStatus, newStatus CheckStatus) CheckStatus { @@ -91,3 +105,43 @@ func (r *Result) addCheck(checkResult *Result) { // SetStatus sets Result's status to the most severe of any individual check r.Status = SetStatus(r.Status, checkResult.Status) } + +const ( + Connectivity = "connectivity" + STARTTLS = "starttls" + Version = "version" + Certificate = "certificate" + MTASTS = "mta-sts" + MTASTSText = "mta-sts-text" + MTASTSPolicyFile = "mta-sts-policy-file" + PolicyList = "policylist" +) + +var checkNames = map[string]string{ + Connectivity: "Server connectivity", + STARTTLS: "Support for STARTTLS", + Version: "Secure version of TLS", + Certificate: "Valid certificate", + MTASTS: "Implementation of MTA-STS", + MTASTSText: "Correct MTA-STS DNS record", + MTASTSPolicyFile: "Correct MTA-STS policy file", + PolicyList: "Status on EFF's STARTTLS Everywhere policy list", +} + +func (r Result) Description() string { + return checkNames[r.Name] +} + +func (r Result) MarshalJSON() ([]byte, error) { + // FakeResult lets us access the default json.Marshall result for Result. + type FakeResult Result + return json.Marshal(struct { + FakeResult + StatusText string `json:"status_text,omitempty"` + Description string `json:"description,omitempty"` + }{ + FakeResult: FakeResult(r), + StatusText: r.StatusText(), + Description: r.Description(), + }) +} diff --git a/views/scan.html.tmpl b/views/scan.html.tmpl index 489d001e..960e240f 100644 --- a/views/scan.html.tmpl +++ b/views/scan.html.tmpl @@ -1,41 +1,40 @@

    Scan results for {{ .Response.Domain }}

    -

    You're viewing unstyled results. You can enable Javascript to view styled content.

    + You're viewing unstyled results. You can enable Javascript to view styled content. +

    Summary

    - {{ if eq .Response.Data.Status 0 }} -

    Congratulations, your domain passed all checks.

    - {{ else if eq .Response.Data.Status 1 }} -

    Your domain passed all checks with some warnings. See below for details.

    - {{ else }} -

    There were some problems with your domain. See below for details.

    - {{ end }} + {{ if eq .Response.Data.Status 0 }} +

    Congratulations, your domain passed all checks.

    + {{ else if eq .Response.Data.Status 1 }} +

    Your domain passed all checks with some warnings. See below for details.

    + {{ else }} +

    There were some problems with your domain. See below for details.

    + {{ end }} - {{ if ne .Response.Data.Message "" }} -

    {{ .Response.Data.Message }}

    - {{ end }} +

    {{ .Response.Data.Message }}

    STARTTLS Everywhere Policy List

    - {{ if eq .Response.Data.Status 0 }} - requiredVars[varName] = os.Getenv(varName) -

    Your mail domain is on the STARTTLS Everywhere policy list. See {{ printf "%s/faq#what-list" .BaseURL }} for details

    - {{ else }} -

    Your mail domain is not on the STARTTLS Everywhere policy list. See {{ printf "%s/faq#what-list" .BaseURL }} for details

    + {{ $r := index .Response.Data.ExtraResults "policylist" }} + {{ $r.Description }}: {{ $r.StatusText }} +
      + {{ range $_, $message := $r.Messages }} +
    • {{ $message }}
    • {{ end }} - - {{ $policyListResult := index .Response.Data.ExtraResults "policy-list" }} - {{ range $message := $policyListResult.Messages }} -

      {{ $message }}

      - {{ end}} +

    Mailboxes

    - {{ range $hostname, $result := .Response.Data.HostnameResults }} + {{ range $hostname, $hostnameResult := .Response.Data.HostnameResults }}

    {{ $hostname }}

      - {{ range $check, $checkResult := $result.Checks }} + {{ range $_, $r := $hostnameResult.Checks }}
    • -

      {{ $checkResult.StatusText }}

      -

      {{ $checkResult.Description }} + {{ $r.Description }}: {{ $r.StatusText }} +

        + {{ range $_, $message := $r.Messages }} +
      • {{ $message }}
      • + {{ end }} +
      • {{ end }}
    From e8fa4a8dd759a40371a355ba8275da4f35663686 Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Fri, 25 Jan 2019 13:46:14 -0800 Subject: [PATCH 11/17] CheckStatus -> Status --- checker/domain.go | 2 +- checker/mta_sts_test.go | 6 +++--- checker/result.go | 18 +++++++++--------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/checker/domain.go b/checker/domain.go index cac7aa2c..2e7e3e91 100644 --- a/checker/domain.go +++ b/checker/domain.go @@ -57,7 +57,7 @@ func (d DomainResult) Class() string { } func (d DomainResult) setStatus(status DomainStatus) DomainResult { - d.Status = DomainStatus(SetStatus(CheckStatus(d.Status), CheckStatus(status))) + d.Status = DomainStatus(SetStatus(Status(d.Status), Status(status))) return d } diff --git a/checker/mta_sts_test.go b/checker/mta_sts_test.go index a717f095..1499b95b 100644 --- a/checker/mta_sts_test.go +++ b/checker/mta_sts_test.go @@ -38,7 +38,7 @@ func TestGetKeyValuePairs(t *testing.T) { func TestValidateMTASTSRecord(t *testing.T) { tests := []struct { txt []string - status CheckStatus + status Status }{ {[]string{"v=STSv1; id=1234", "v=STSv1; id=5678"}, Failure}, {[]string{"v=STSv1; id=20171114T070707;"}, Success}, @@ -57,7 +57,7 @@ func TestValidateMTASTSRecord(t *testing.T) { func TestValidateMTASTSPolicyFile(t *testing.T) { tests := []struct { txt string - status CheckStatus + status Status }{ {"version: STSv1\nmode: enforce\nmax_age:100000\nmx: foo.example.com\nmx: bar.example.com\n", Success}, // Support UTF-8 @@ -96,7 +96,7 @@ func TestValidateMTASTSMXs(t *testing.T) { tests := []struct { policyFileMXs []string dnsMXs map[string]HostnameResult - status CheckStatus + status Status }{ { []string{"mail.example.com"}, diff --git a/checker/result.go b/checker/result.go index 1ebd1b05..e362752d 100644 --- a/checker/result.go +++ b/checker/result.go @@ -5,18 +5,18 @@ import ( "fmt" ) -// CheckStatus is an enum encoding the status of the overall check. -type CheckStatus int32 +// Status is an enum encoding the status of the overall check. +type Status int32 // Values for CheckStatus const ( - Success CheckStatus = 0 - Warning CheckStatus = 1 - Failure CheckStatus = 2 - Error CheckStatus = 3 + Success Status = 0 + Warning Status = 1 + Failure Status = 2 + Error Status = 3 ) -var statusText = map[CheckStatus]string{ +var statusText = map[Status]string{ Success: "Success", Warning: "Warning", Failure: "Failure", @@ -29,7 +29,7 @@ func (r Result) StatusText() string { // SetStatus the resulting status of combining old & new. The order of priority // for CheckStatus goes: Error > Failure > Warning > Success -func SetStatus(oldStatus CheckStatus, newStatus CheckStatus) CheckStatus { +func SetStatus(oldStatus Status, newStatus Status) Status { if newStatus > oldStatus { return newStatus } @@ -42,7 +42,7 @@ func SetStatus(oldStatus CheckStatus, newStatus CheckStatus) CheckStatus { // warning messages associated. type Result struct { Name string `json:"name"` - Status CheckStatus `json:"status"` + Status Status `json:"status"` Messages []string `json:"messages,omitempty"` Checks map[string]*Result `json:"checks,omitempty"` } From c449feecbea98b604ee93f9b3cb43fe8a3af2aa6 Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Mon, 28 Jan 2019 14:31:00 -0800 Subject: [PATCH 12/17] Lint --- checker/result.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/checker/result.go b/checker/result.go index e362752d..a13c20f4 100644 --- a/checker/result.go +++ b/checker/result.go @@ -8,7 +8,7 @@ import ( // Status is an enum encoding the status of the overall check. type Status int32 -// Values for CheckStatus +// Values for Result Status const ( Success Status = 0 Warning Status = 1 @@ -23,6 +23,7 @@ var statusText = map[Status]string{ Error: "Error", } +// StatusText returns the text version of the Result Status func (r Result) StatusText() string { return statusText[r.Status] } @@ -106,6 +107,7 @@ func (r *Result) addCheck(checkResult *Result) { r.Status = SetStatus(r.Status, checkResult.Status) } +// IDs for checks that can be run const ( Connectivity = "connectivity" STARTTLS = "starttls" @@ -117,6 +119,7 @@ const ( PolicyList = "policylist" ) +// Text descriptions of checks that can be run var checkNames = map[string]string{ Connectivity: "Server connectivity", STARTTLS: "Support for STARTTLS", @@ -128,10 +131,13 @@ var checkNames = map[string]string{ PolicyList: "Status on EFF's STARTTLS Everywhere policy list", } +// Description returns the full-text name of a check. func (r Result) Description() string { return checkNames[r.Name] } +// MarshalJSON writes Result to JSON. It adds status_text and description to +// the output. func (r Result) MarshalJSON() ([]byte, error) { // FakeResult lets us access the default json.Marshall result for Result. type FakeResult Result From cd2a68223258c4037c54daa2fcf214bd510103b9 Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Mon, 28 Jan 2019 18:55:00 -0800 Subject: [PATCH 13/17] Test marshal result JSON --- checker/result_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 checker/result_test.go diff --git a/checker/result_test.go b/checker/result_test.go new file mode 100644 index 00000000..04a7554a --- /dev/null +++ b/checker/result_test.go @@ -0,0 +1,41 @@ +package checker + +import ( + "bytes" + "encoding/json" + "testing" +) + +func TestMarshalResultJSON(t *testing.T) { + // Should set description and status_text for CheckResult w/ recognized keys + result := Result{ + Name: "starttls", + Status: Success, + } + marshalled, err := json.Marshal(result) + if err != nil { + t.Fatal(err) + } + if !bytes.Contains(marshalled, []byte("\"status_text\":\"Success\"")) { + t.Errorf("Marshalled result should contain status_text, got %s", string(marshalled)) + } + if !bytes.Contains(marshalled, []byte("\"description\":\"")) { + t.Errorf("Marshalled result should contain description, got %s", string(marshalled)) + } + + // Should survive unrecognized keys + result = Result{ + Name: "foo", + Status: 100, + } + marshalled, _ = json.Marshal(result) + if err != nil { + t.Fatal(err) + } + if bytes.Contains(marshalled, []byte("\"status_text\":\"")) { + t.Errorf("Result with unrecognized keys shouldn't output status_text, got %s", string(marshalled)) + } + if bytes.Contains(marshalled, []byte("\"description\":\"")) { + t.Errorf("Result with unrecognized keys shouldn't output status_text, got %s", string(marshalled)) + } +} From 2ae7c134a13e67e305070c33edde6230d83352d1 Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Tue, 29 Jan 2019 16:25:49 -0800 Subject: [PATCH 14/17] Tweaks for Sydney PR review --- api.go | 2 +- checker/result.go | 4 ++-- views/scan.html.tmpl | 5 ++++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/api.go b/api.go index 50d0ea3c..7ec1de43 100644 --- a/api.go +++ b/api.go @@ -67,7 +67,7 @@ type APIResponse struct { StatusCode int `json:"status_code"` Message string `json:"message"` Response interface{} `json:"response"` - TemplatePath string `json:"omit"` + TemplatePath string `json:"-"` } type apiHandler func(r *http.Request) APIResponse diff --git a/checker/result.go b/checker/result.go index a13c20f4..02856acc 100644 --- a/checker/result.go +++ b/checker/result.go @@ -122,10 +122,10 @@ const ( // Text descriptions of checks that can be run var checkNames = map[string]string{ Connectivity: "Server connectivity", - STARTTLS: "Support for STARTTLS", + STARTTLS: "Support for inbound STARTTLS", Version: "Secure version of TLS", Certificate: "Valid certificate", - MTASTS: "Implementation of MTA-STS", + MTASTS: "Inbound MTA-STS support", MTASTSText: "Correct MTA-STS DNS record", MTASTSPolicyFile: "Correct MTA-STS policy file", PolicyList: "Status on EFF's STARTTLS Everywhere policy list", diff --git a/views/scan.html.tmpl b/views/scan.html.tmpl index 960e240f..d63e8e26 100644 --- a/views/scan.html.tmpl +++ b/views/scan.html.tmpl @@ -19,9 +19,12 @@ {{ $r.Description }}: {{ $r.StatusText }}
      {{ range $_, $message := $r.Messages }} -
    • {{ $message }}
    • +
    • {{ $message }}
      • {{ end }}
    + {{ if eq $r.Status 1 }} + Add your email domain the STARTTLS Everywhere Policy List + {{ end }}

    Mailboxes

    {{ range $hostname, $hostnameResult := .Response.Data.HostnameResults }} From aaeb9437e545f6fa2da0e19d610d65a4806b6862 Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Wed, 30 Jan 2019 11:22:05 -0800 Subject: [PATCH 15/17] Don't respond with plain text --- api.go | 36 ++++++++++++++++-------------------- views/default.html.tmpl | 8 ++++++++ views/domain.html.tmpl | 19 ------------------- views/scan.html.tmpl | 17 +++++++++-------- 4 files changed, 33 insertions(+), 47 deletions(-) create mode 100644 views/default.html.tmpl delete mode 100644 views/domain.html.tmpl diff --git a/api.go b/api.go index 7ec1de43..a08281b8 100644 --- a/api.go +++ b/api.go @@ -75,15 +75,11 @@ type apiHandler func(r *http.Request) APIResponse func apiWrapper(api apiHandler) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { response := api(r) - if response.StatusCode != http.StatusOK { - http.Error(w, response.Message, response.StatusCode) - } if response.StatusCode == http.StatusInternalServerError { packet := raven.NewPacket(response.Message, raven.NewHttp(r)) raven.Capture(packet, nil) } - if strings.Contains(r.Header.Get("accept"), "text/html") && - response.TemplatePath != "" { + if strings.Contains(r.Header.Get("accept"), "text/html") { writeHTML(w, response) } else { writeJSON(w, response) @@ -284,11 +280,7 @@ func (api API) Queue(r *http.Request) APIResponse { // 2. Create token for domain token, err := api.Database.PutToken(domain) if err != nil { - return APIResponse{ - StatusCode: http.StatusInternalServerError, - Message: err.Error(), - TemplatePath: "views/domain.html.tmpl", - } + return APIResponse{StatusCode: http.StatusInternalServerError, Message: err.Error()} } // 3. Send email @@ -298,21 +290,22 @@ func (api API) Queue(r *http.Request) APIResponse { return APIResponse{StatusCode: http.StatusInternalServerError, Message: "Unable to send validation e-mail"} } + // domainData.State = Unvalidated + // or queued? return APIResponse{ - StatusCode: http.StatusOK, - Response: domainData, - TemplatePath: "views/domain.html.tmpl", + StatusCode: http.StatusOK, + Response: fmt.Sprintf("Thank you for submitting your domain. Please check postmaster@%s to validate that you control the domain.", domain), } // GET: Retrieve domain status from queue + // JSON only } else if r.Method == http.MethodGet { status, err := api.Database.GetDomain(domain) if err != nil { return APIResponse{StatusCode: http.StatusNotFound, Message: err.Error()} } return APIResponse{ - StatusCode: http.StatusOK, - Response: status, - TemplatePath: "views/domain.html.tmpl", + StatusCode: http.StatusOK, + Response: status, } } else { return APIResponse{StatusCode: http.StatusMethodNotAllowed, @@ -380,9 +373,10 @@ func getParam(param string, r *http.Request) (string, error) { // Writes `v` as a JSON object to http.ResponseWriter `w`. If an error // occurs, writes `http.StatusInternalServerError` to `w`. -func writeJSON(w http.ResponseWriter, v interface{}) { +func writeJSON(w http.ResponseWriter, apiResponse APIResponse) { w.Header().Set("Content-Type", "application/json; charset=utf-8") - b, err := json.MarshalIndent(v, "", " ") + w.WriteHeader(apiResponse.StatusCode) + b, err := json.MarshalIndent(apiResponse, "", " ") if err != nil { msg := fmt.Sprintf("Internal error: could not format JSON. (%s)\n", err) http.Error(w, msg, http.StatusInternalServerError) @@ -393,6 +387,9 @@ func writeJSON(w http.ResponseWriter, v interface{}) { func writeHTML(w http.ResponseWriter, apiResponse APIResponse) { // Add some additional useful fields for use in templates. + if apiResponse.TemplatePath == "" { + apiResponse.TemplatePath = "views/default.html.tmpl" + } data := struct { APIResponse BaseURL string @@ -407,9 +404,8 @@ func writeHTML(w http.ResponseWriter, apiResponse APIResponse) { http.Error(w, "Internal error: could not parse template.", http.StatusInternalServerError) return } + w.WriteHeader(apiResponse.StatusCode) err = tmpl.Execute(w, data) - // We can't write a 500 status header if tmpl.Execute fails because Execute - // may have already written to the status and body of w. if err != nil { log.Println(err) raven.CaptureError(err, nil) diff --git a/views/default.html.tmpl b/views/default.html.tmpl new file mode 100644 index 00000000..3b411c23 --- /dev/null +++ b/views/default.html.tmpl @@ -0,0 +1,8 @@ + + + STARTTLS Everywhere + + + {{ .Response }} + + diff --git a/views/domain.html.tmpl b/views/domain.html.tmpl deleted file mode 100644 index f9f47417..00000000 --- a/views/domain.html.tmpl +++ /dev/null @@ -1,19 +0,0 @@ - - - {{ if or (eq .Response.State "unknown") (eq .Response.State "") }} - Something went wrong - your domain was not submitted to the STARTTLS policy list. - {{ end }} - {{ if eq .Response.State "unvalidated" }} - Thank you for submitting your domain. Please check postmaster@{{ .Response.Name }} to validate that you control the domain. - {{ end }} - {{ if eq .Response.State "queued" }} - Thank you for submitting your domain. Your domain is queued to be reviewed and added to the STARTTLS policy list. - {{ end }} - {{ if eq .Response.State "failed" }} - Your domain does not qualify for addition to the STARTTLS policy list. Please visit {{ .BaseURL }} to review your domain's status and correct any issues. - {{ end }} - {{ if eq .Response.State "added" }} - Your domain is already on the STARTTLS policy list. Thanks for helping to make e-mail more secure. - {{ end }} - - diff --git a/views/scan.html.tmpl b/views/scan.html.tmpl index d63e8e26..8767f71c 100644 --- a/views/scan.html.tmpl +++ b/views/scan.html.tmpl @@ -15,15 +15,16 @@

    {{ .Response.Data.Message }}

    STARTTLS Everywhere Policy List

    - {{ $r := index .Response.Data.ExtraResults "policylist" }} - {{ $r.Description }}: {{ $r.StatusText }} -
      - {{ range $_, $message := $r.Messages }} -
    • {{ $message }}
      • + {{ with $r := index .Response.Data.ExtraResults "policylist" }} + {{ $r.Description }}: {{ $r.StatusText }} +
        + {{ range $_, $message := $r.Messages }} +
      • {{ $message }}
        • + {{ end }} +
      + {{ if eq $r.Status 1 }} + Add your email domain the STARTTLS Everywhere Policy List {{ end }} -
    - {{ if eq $r.Status 1 }} - Add your email domain the STARTTLS Everywhere Policy List {{ end }}

    Mailboxes

    From 0fd0a88ab91ca0e579f7430a82d2b2df6d1f1415 Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Wed, 30 Jan 2019 11:44:51 -0800 Subject: [PATCH 16/17] Render errors as HTML --- api.go | 4 +++- queue_test.go | 12 ++++++++++++ views/default.html.tmpl | 10 +++++++++- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/api.go b/api.go index a08281b8..edfc68e1 100644 --- a/api.go +++ b/api.go @@ -392,10 +392,12 @@ func writeHTML(w http.ResponseWriter, apiResponse APIResponse) { } data := struct { APIResponse - BaseURL string + BaseURL string + StatusText string }{ APIResponse: apiResponse, BaseURL: os.Getenv("FRONTEND_WEBSITE_LINK"), + StatusText: http.StatusText(apiResponse.StatusCode), } tmpl, err := template.ParseFiles(apiResponse.TemplatePath) if err != nil { diff --git a/queue_test.go b/queue_test.go index eb34939f..d9f7a8d5 100644 --- a/queue_test.go +++ b/queue_test.go @@ -36,6 +36,18 @@ func TestQueueHTML(t *testing.T) { } } +func TestQueueErrorHTML(t *testing.T) { + defer teardown() + + body, status := testHTMLPost("/api/queue", url.Values{}, t) + if status != http.StatusBadRequest { + t.Errorf("HTML POST status should be %d, got %d", http.StatusBadRequest, status) + } + if !strings.Contains(string(body), "Bad Request") { + t.Errorf("Response should contain failed status text, got %s", string(body)) + } +} + func TestGetDomainHidesEmail(t *testing.T) { defer teardown() diff --git a/views/default.html.tmpl b/views/default.html.tmpl index 3b411c23..234679a1 100644 --- a/views/default.html.tmpl +++ b/views/default.html.tmpl @@ -3,6 +3,14 @@ STARTTLS Everywhere - {{ .Response }} + {{ if ne .StatusCode 200 }} +

    {{ .StatusText }}

    + {{ end }} + + {{ if ne .Message "" }} +

    {{ .Message }}

    + {{ end }} + +

    {{ .Response }}

    From 3d6ff0f6bfba8493141e2d4073f01a2fbd1ef4aa Mon Sep 17 00:00:00 2001 From: Vivian Brown Date: Thu, 31 Jan 2019 11:50:17 -0800 Subject: [PATCH 17/17] Fix scan html template --- api_test.go | 2 +- models/scan.go | 10 ++++++++ scan_test.go | 54 +++++++++++++++++++++++++++++++++++++++++++- views/scan.html.tmpl | 10 ++++---- 4 files changed, 69 insertions(+), 7 deletions(-) diff --git a/api_test.go b/api_test.go index 51b585e4..f3879736 100644 --- a/api_test.go +++ b/api_test.go @@ -57,7 +57,7 @@ func testHTMLPost(path string, data url.Values, t *testing.T) ([]byte, int) { t.Fatal(err) } body, _ := ioutil.ReadAll(resp.Body) - if !strings.Contains(strings.ToLower(string(body)), "{{ .Response.Data.Message }}

    STARTTLS Everywhere Policy List

    - {{ with $r := index .Response.Data.ExtraResults "policylist" }} - {{ $r.Description }}: {{ $r.StatusText }} + {{ with index .Response.Data.ExtraResults "policylist" }} + {{ .Description }}: {{ .StatusText }}
      - {{ range $_, $message := $r.Messages }} + {{ range $_, $message := .Messages }}
    • {{ $message }}
      • {{ end }}
    - {{ if eq $r.Status 1 }} + {{ end }} + {{ if .Response.CanAddToPolicyList }} Add your email domain the STARTTLS Everywhere Policy List - {{ end }} {{ end }}

    Mailboxes