diff --git a/checker/domain.go b/checker/domain.go index 8203276d..0135329d 100644 --- a/checker/domain.go +++ b/checker/domain.go @@ -127,7 +127,7 @@ func (c *Checker) CheckDomain(domain string, expectedHostnames []string) DomainR } } result.PreferredHostnames = checkedHostnames - result.MTASTSResult = c.checkMTASTS(domain, result.HostnameResults) + result.MTASTSResult = c.CheckMTASTS(domain, result.HostnameResults) // Derive Domain code from Hostname results. if len(checkedHostnames) == 0 { diff --git a/checker/mta_sts.go b/checker/mta_sts.go index cfc0f08b..60872967 100644 --- a/checker/mta_sts.go +++ b/checker/mta_sts.go @@ -184,7 +184,9 @@ func validateMTASTSMXs(policyFileMXs []string, dnsMXs map[string]HostnameResult, } } -func (c Checker) checkMTASTS(domain string, hostnameResults map[string]HostnameResult) *MTASTSResult { +// CheckMTASTS performs all associated checks for a particular domain's +// MTA-STS support. +func (c Checker) CheckMTASTS(domain string, hostnameResults map[string]HostnameResult) *MTASTSResult { if c.checkMTASTSOverride != nil { // Allow the Checker to mock this function. return c.checkMTASTSOverride(domain, hostnameResults) diff --git a/db/sqldb.go b/db/sqldb.go index 2e08e004..8f6a80bc 100644 --- a/db/sqldb.go +++ b/db/sqldb.go @@ -216,6 +216,14 @@ func (db *SQLDatabase) PutDomain(domain models.Domain) error { return err } +// UpdateDomainPolicy allows us to update the internal data about a particular domain. +func (db *SQLDatabase) UpdateDomainPolicy(domain models.Domain) error { + _, err := db.conn.Exec("UPDATE domains SET data=$2, status=$3 WHERE domain=$1 AND mta_sts=TRUE", + domain.Name, strings.Join(domain.MXs[:], ","), domain.State) + return err + +} + // GetDomain retrieves the status and information associated with a particular // mailserver domain. func (db SQLDatabase) GetDomain(domain string, state models.DomainState) (models.Domain, error) { @@ -225,7 +233,7 @@ func (db SQLDatabase) GetDomain(domain string, state models.DomainState) (models // GetDomains retrieves all the domains which match a particular state, // that are not in MTA_STS mode func (db SQLDatabase) GetDomains(state models.DomainState) ([]models.Domain, error) { - return db.queryDomainsWhere("status=$1", state) + return db.queryDomainsWhere("status=$1 AND mta_sts=FALSE", state) } // GetMTASTSDomains retrieves domains which wish their policy to be queued with their MTASTS. @@ -324,7 +332,7 @@ func (db SQLDatabase) queryDomainsWhere(condition string, args ...interface{}) ( return domains, nil } -// DomainsToValidate [interface Validator] retrieves domains from the +// DomainsToValidate [interface DomainPolicyStore] retrieves domains from the // DB whose policies should be validated. func (db SQLDatabase) DomainsToValidate() ([]string, error) { domains := []string{} @@ -332,23 +340,27 @@ func (db SQLDatabase) DomainsToValidate() ([]string, error) { if err != nil { return domains, err } + dataMTASTS, err := db.GetMTASTSDomains() + if err != nil { + return domains, err + } for _, domainInfo := range data { domains = append(domains, domainInfo.Name) } + for _, domainInfo := range dataMTASTS { + domains = append(domains, domainInfo.Name) + } return domains, nil } -// HostnamesForDomain [interface Validator] retrieves the hostname policy for +// GetDomainPolicy [interface DomainPolicyStore] retrieves the domain object for // a particular domain. -func (db SQLDatabase) HostnamesForDomain(domain string) ([]string, error) { +func (db SQLDatabase) GetDomainPolicy(domain string) (models.Domain, error) { data, err := db.GetDomain(domain, models.StateEnforce) if err != nil { data, err = db.GetDomain(domain, models.StateTesting) } - if err != nil { - return []string{}, err - } - return data.MXs, nil + return data, err } // GetHostnameScan retrives most recent scan from database. diff --git a/db/sqldb_test.go b/db/sqldb_test.go index ee59f82a..c8db7ead 100644 --- a/db/sqldb_test.go +++ b/db/sqldb_test.go @@ -270,28 +270,6 @@ func TestDomainsToValidate(t *testing.T) { } } -func TestHostnamesForDomain(t *testing.T) { - database.ClearTables() - database.PutDomain(models.Domain{Name: "x", MXs: []string{"x.com", "y.org"}}) - database.PutDomain(models.Domain{Name: "y"}) - database.SetStatus("x", models.StateTesting) - database.SetStatus("y", models.StateTesting) - result, err := database.HostnamesForDomain("x") - if err != nil { - t.Fatalf("HostnamesForDomain failed: %v\n", err) - } - if len(result) != 2 || result[0] != "x.com" || result[1] != "y.org" { - t.Errorf("Expected two hostnames, x.com and y.org\n") - } - result, err = database.HostnamesForDomain("y") - if err != nil { - t.Fatalf("HostnamesForDomain failed: %v\n", err) - } - if len(result) > 0 { - t.Errorf("Expected no hostnames to be returned, got %s\n", result[0]) - } -} - func TestPutAndIsBlacklistedEmail(t *testing.T) { defer database.ClearTables() @@ -454,3 +432,25 @@ func TestGetMTASTSDomains(t *testing.T) { } } } + +func TestUpdateDomainPolicy(t *testing.T) { + database.ClearTables() + database.PutDomain(models.Domain{Name: "no-mtasts"}) + database.PutDomain(models.Domain{Name: "mtasts", MTASTS: true, Email: "real-email"}) + database.UpdateDomainPolicy(models.Domain{Name: "no-mtasts", State: models.StateEnforce}) + database.UpdateDomainPolicy(models.Domain{Name: "mtasts", State: models.StateEnforce, MXs: []string{"hostname"}, Email: "fake-email"}) + domain, _ := database.GetDomainPolicy("no-mtasts") + if domain.State == models.StateEnforce { + t.Errorf("Expected State to not update since unicorns isn't MTASTS") + } + domain, _ = database.GetDomainPolicy("mtasts") + if domain.State != models.StateEnforce { + t.Errorf("Expected State to update after UpdateDomainPolicy") + } + if len(domain.MXs) != 1 || domain.MXs[0] != "hostname" { + t.Errorf("Expected MXs to update after UpdateDomainPolicy") + } + if domain.Email != "real-email" { + t.Errorf("Did not expect Email to update after UpdateDomainPolicy") + } +} diff --git a/main.go b/main.go index bfab917c..00e95fcb 100644 --- a/main.go +++ b/main.go @@ -119,7 +119,14 @@ func main() { } if os.Getenv("VALIDATE_QUEUED") == "1" { log.Println("[Starting queued validator]") - go validator.ValidateRegularly("Testing domains", db, 24*time.Hour) + v := validator.Validator{ + Name: "Testing and enforced domains", + Store: db, + Interval: 24 * time.Hour, + CheckPerformer: validator.GetDBCheck(db.UpdateDomainPolicy), + } + go v.Run() + // go validator.ValidateRegularly("Testing domains", db, 24*time.Hour) } ServePublicEndpoints(&api, &cfg) } diff --git a/models/domain.go b/models/domain.go index b12e639b..68158a33 100644 --- a/models/domain.go +++ b/models/domain.go @@ -6,6 +6,7 @@ import ( "time" "github.com/EFForg/starttls-backend/checker" + "github.com/EFForg/starttls-backend/util" ) /* Domain represents an email domain's TLS policy. @@ -142,6 +143,17 @@ func (d Domain) AsyncPolicyListCheck(store domainStore, list policyList) <-chan return result } +// SamePolicy checks whether the underlying policy represented by Domain +// and the one picked up by the MTA-STS check represent the same policy. +func (d *Domain) SamePolicy(result *checker.MTASTSResult) bool { + if (result.Mode == "enforce" && d.State != StateEnforce) || + (result.Mode == "testing" && d.State != StateTesting) || + result.Mode == "none" { + return false + } + return util.ListsEqual(d.MXs, result.MXs) +} + // GetDomain retrieves Domain with the most "important" state. // At any given time, there can only be one domain that's either StateEnforce // or StateTesting. If that domain exists in the store, return that one. diff --git a/policy/policy.go b/policy/policy.go index 8eab88ac..c9494748 100644 --- a/policy/policy.go +++ b/policy/policy.go @@ -8,6 +8,8 @@ import ( "net/http" "sync" "time" + + "github.com/EFForg/starttls-backend/models" ) // policyURL is the default URL from which to fetch the policy JSON. @@ -68,7 +70,7 @@ type UpdatedList struct { *List } -// DomainsToValidate [interface Validator] retrieves domains from the +// DomainsToValidate [interface DomainPolicyStore] retrieves domains from the // DB whose policies should be validated. func (l *UpdatedList) DomainsToValidate() ([]string, error) { l.mu.RLock() @@ -80,14 +82,23 @@ func (l *UpdatedList) DomainsToValidate() ([]string, error) { return domains, nil } -// HostnamesForDomain [interface Validator] retrieves the hostname policy for +// GetDomainPolicy [interface DomainPolicyStore] retrieves the domain object for // a particular domain. -func (l *UpdatedList) HostnamesForDomain(domain string) ([]string, error) { +func (l *UpdatedList) GetDomainPolicy(domain string) (models.Domain, error) { policy, err := l.Get(domain) if err != nil { - return []string{}, err + return models.Domain{}, err + } + domainObj := models.Domain{ + Name: domain, + MXs: policy.MXs, + } + if policy.Mode == "enforce" { + domainObj.State = models.StateEnforce + } else if policy.Mode == "testing" { + domainObj.State = models.StateTesting } - return policy.MXs, nil + return domainObj, nil } // Get safely reads from the underlying policy list and returns a TLSPolicy for a domain diff --git a/policy/policy_test.go b/policy/policy_test.go index f79e65bb..5cff375c 100644 --- a/policy/policy_test.go +++ b/policy/policy_test.go @@ -102,12 +102,12 @@ func TestHostnamesForDomain(t *testing.T) { var updatedList = List{Policies: map[string]TLSPolicy{ "eff.org": TLSPolicy{MXs: hostnames}}} list := makeUpdatedList(func() (List, error) { return updatedList, nil }, time.Second) - returned, err := list.HostnamesForDomain("eff.org") + returned, err := list.GetDomainPolicy("eff.org") if err != nil { t.Fatalf("Encountered %v", err) } - if !reflect.DeepEqual(returned, hostnames) { - t.Errorf("Expected %s, got %s", hostnames, returned) + if !reflect.DeepEqual(returned.MXs, hostnames) { + t.Errorf("Expected %s, got %s", hostnames, returned.MXs) } } diff --git a/util/util.go b/util/util.go new file mode 100644 index 00000000..dc057342 --- /dev/null +++ b/util/util.go @@ -0,0 +1,27 @@ +package util + +import ( + "reflect" +) + +// ListsEqual checks that two lists have the same elements, +// regardless of order. +func ListsEqual(x []string, y []string) bool { + // Transform each list into a histogram + xMap := make(map[string]uint) + yMap := make(map[string]uint) + for _, element := range x { + if _, ok := xMap[element]; !ok { + xMap[element] = 0 + } + xMap[element]++ + } + for _, element := range y { + if _, ok := yMap[element]; !ok { + yMap[element] = 0 + } + yMap[element]++ + } + // Compare the histogram maps + return reflect.DeepEqual(xMap, yMap) +} diff --git a/util/util_test.go b/util/util_test.go new file mode 100644 index 00000000..ad2ca66e --- /dev/null +++ b/util/util_test.go @@ -0,0 +1,29 @@ +package util + +import ( + "testing" +) + +func TestListsEqual(t *testing.T) { + testCases := []struct { + x []string + y []string + expected bool + }{ + {[]string{}, []string{}, true}, + {[]string{"a"}, []string{}, false}, + {[]string{"a"}, []string{"a"}, true}, + {[]string{"a", "a"}, []string{"a"}, false}, + {[]string{"a", "b", "c"}, []string{"a", "b", "c"}, true}, + {[]string{"b", "a", "c"}, []string{"a", "b", "c"}, true}, + {[]string{"b", "a", "b", "c"}, []string{"a", "b", "c"}, false}, + {[]string{"b", "a", "b", "c"}, []string{"a", "b", "a", "c"}, false}, + {[]string{"a", "a", "b", "c"}, []string{"a", "b", "a", "c"}, true}, + } + for _, testCase := range testCases { + got := ListsEqual(testCase.x, testCase.y) + if got != testCase.expected { + t.Errorf("Compared %v and %v, expected %v, got %v", testCase.x, testCase.y, testCase.expected, got) + } + } +} diff --git a/validator/validator.go b/validator/validator.go index 87d0b606..fb8e27e9 100644 --- a/validator/validator.go +++ b/validator/validator.go @@ -6,6 +6,7 @@ import ( "time" "github.com/EFForg/starttls-backend/checker" + "github.com/EFForg/starttls-backend/models" "github.com/getsentry/raven-go" ) @@ -14,7 +15,7 @@ import ( // expected hostnames). type DomainPolicyStore interface { DomainsToValidate() ([]string, error) - HostnamesForDomain(string) ([]string, error) + GetDomainPolicy(string) (models.Domain, error) } // Called with failure by defaault. @@ -28,8 +29,10 @@ func reportToSentry(name string, domain string, result checker.DomainResult) { result) } -type checkPerformer func(string, []string) checker.DomainResult -type resultCallback func(string, string, checker.DomainResult) +type resultCallback func(string, models.Domain, checker.DomainResult) + +// CheckPerformer defines a function that performs a security check on a domain. +type CheckPerformer func(models.Domain) checker.DomainResult // Validator runs checks regularly against domain policies. This structure // defines the configurations. @@ -47,18 +50,42 @@ type Validator struct { OnFailure resultCallback // OnSuccess: optional. Called when a particular policy validation succeeds. OnSuccess resultCallback - // checkPerformer: performs the check. - checkPerformer checkPerformer + // CheckPerformer: performs the check. + CheckPerformer CheckPerformer +} + +// UpdatePolicy is a callback we can provide to GetDBCheck in order to perform a policy +// update if we notice a discrepancy between our view and the MTA-STS policy. +type UpdatePolicy func(models.Domain) error + +// GetDBCheck returns a CheckPerformer that performs an MTASTS check and update if +// the policy is updated, or performs a regular security check if MTASTS is not supported. +func GetDBCheck(update UpdatePolicy) CheckPerformer { + c := checker.Checker{Cache: checker.MakeSimpleCache(time.Hour)} + return func(domain models.Domain) checker.DomainResult { + if domain.MTASTS { + result := c.CheckDomain(domain.Name, []string{}) + if !domain.SamePolicy(result.MTASTSResult) { + if update(domain) != nil { + reportToSentry("Couldn't update policy in DB", domain.Name, result) + } + } + return result + } + return c.CheckDomain(domain.Name, domain.MXs) + } } -func (v *Validator) checkPolicy(domain string, hostnames []string) checker.DomainResult { - if v.checkPerformer == nil { +func (v *Validator) checkPolicy(domain models.Domain) checker.DomainResult { + if v.CheckPerformer == nil { c := checker.Checker{ Cache: checker.MakeSimpleCache(time.Hour), } - v.checkPerformer = c.CheckDomain + v.CheckPerformer = func(domain models.Domain) checker.DomainResult { + return c.CheckDomain(domain.Name, domain.MXs) + } } - return v.checkPerformer(domain, hostnames) + return v.CheckPerformer(domain) } func (v *Validator) interval() time.Duration { @@ -68,14 +95,14 @@ func (v *Validator) interval() time.Duration { return time.Hour * 24 } -func (v *Validator) policyFailed(name string, domain string, result checker.DomainResult) { +func (v *Validator) policyFailed(name string, domain models.Domain, result checker.DomainResult) { if v.OnFailure != nil { v.OnFailure(name, domain, result) } - reportToSentry(name, domain, result) + reportToSentry(name, domain.Name, result) } -func (v *Validator) policyPassed(name string, domain string, result checker.DomainResult) { +func (v *Validator) policyPassed(name string, domain models.Domain, result checker.DomainResult) { if v.OnSuccess != nil { v.OnSuccess(name, domain, result) } @@ -93,17 +120,17 @@ func (v *Validator) Run() { continue } for _, domain := range domains { - hostnames, err := v.Store.HostnamesForDomain(domain) + domainData, err := v.Store.GetDomainPolicy(domain) if err != nil { log.Printf("[%s validator] Could not retrieve policy for domain %s: %v", v.Name, domain, err) continue } - result := v.checkPolicy(domain, hostnames) + result := v.checkPolicy(domainData) if result.Status != 0 { log.Printf("[%s validator] %s failed; sending report", v.Name, domain) - v.policyFailed(v.Name, domain, result) + v.policyFailed(v.Name, domainData, result) } else { - v.policyPassed(v.Name, domain, result) + v.policyPassed(v.Name, domainData, result) } } } diff --git a/validator/validator_test.go b/validator/validator_test.go index eec55a9f..3457faed 100644 --- a/validator/validator_test.go +++ b/validator/validator_test.go @@ -5,6 +5,7 @@ import ( "time" "github.com/EFForg/starttls-backend/checker" + "github.com/EFForg/starttls-backend/models" ) type mockDomainPolicyStore struct { @@ -19,21 +20,21 @@ func (m mockDomainPolicyStore) DomainsToValidate() ([]string, error) { return domains, nil } -func (m mockDomainPolicyStore) HostnamesForDomain(domain string) ([]string, error) { - return m.hostnames[domain], nil +func (m mockDomainPolicyStore) GetDomainPolicy(domain string) (models.Domain, error) { + return models.Domain{Name: domain, MXs: m.hostnames[domain]}, nil } -func noop(_ string, _ string, _ checker.DomainResult) {} +func noop(_ string, _ models.Domain, _ checker.DomainResult) {} func TestRegularValidationValidates(t *testing.T) { called := make(chan bool) - fakeChecker := func(domain string, hostnames []string) checker.DomainResult { + fakeChecker := func(_ models.Domain) checker.DomainResult { called <- true return checker.DomainResult{} } mock := mockDomainPolicyStore{ hostnames: map[string][]string{"a": []string{"hostname"}}} - v := Validator{Store: mock, Interval: 100 * time.Millisecond, checkPerformer: fakeChecker, OnFailure: noop} + v := Validator{Store: mock, Interval: 100 * time.Millisecond, CheckPerformer: fakeChecker, OnFailure: noop} go v.Run() select { @@ -46,25 +47,25 @@ func TestRegularValidationValidates(t *testing.T) { func TestRegularValidationReportsErrors(t *testing.T) { reports := make(chan string) - fakeChecker := func(domain string, hostnames []string) checker.DomainResult { - if domain == "fail" || domain == "error" { + fakeChecker := func(domain models.Domain) checker.DomainResult { + if domain.Name == "fail" || domain.Name == "error" { return checker.DomainResult{Status: 5} } return checker.DomainResult{Status: 0} } - fakeReporter := func(name string, domain string, result checker.DomainResult) { - reports <- domain + fakeReporter := func(name string, domain models.Domain, result checker.DomainResult) { + reports <- domain.Name } successReports := make(chan string) - fakeSuccessReporter := func(name string, domain string, result checker.DomainResult) { - successReports <- domain + fakeSuccessReporter := func(name string, domain models.Domain, result checker.DomainResult) { + successReports <- domain.Name } mock := mockDomainPolicyStore{ hostnames: map[string][]string{ "fail": []string{"hostname"}, "error": []string{"hostname"}, "normal": []string{"hostname"}}} - v := Validator{Store: mock, Interval: 100 * time.Millisecond, checkPerformer: fakeChecker, + v := Validator{Store: mock, Interval: 100 * time.Millisecond, CheckPerformer: fakeChecker, OnFailure: fakeReporter, OnSuccess: fakeSuccessReporter, } go v.Run()