Review fixes

cmd/policy-loader/base-rules.json

@@ -1,3 +1,13 @@
 {
-  "Blacklist": ["in-addr.arpa", "example", "example.com", "example.net", "example.org", "invalid", "local", "localhost", "test"]
+  "Blacklist": [
+    "in-addr.arpa",
+    "example",
+    "example.com",
+    "example.net",
+    "example.org",
+    "invalid",
+    "local",
+    "localhost",
+    "test"
+  ]
 }

cmd/policy-loader/main.go

@@ -48,16 +48,16 @@ func main() {
 			Usage: "Write out whitelist and blacklist from database to a rule file",
 			Action: func(c *cli.Context) {
 				padb, ruleFile := setupFromContext(c)
-				bList, wList, err := padb.DumpRules()
+				ruleSet, err := padb.DumpRules()
 				cmd.FailOnError(err, "Couldn't retrieve whitelist rules")
 				var rules struct {
 					Blacklist []string
 					Whitelist []string
 				}
-				for _, r := range bList {
+				for _, r := range ruleSet.Blacklist {
 					rules.Blacklist = append(rules.Blacklist, r.Host)
 				}
-				for _, r := range wList {
+				for _, r := range ruleSet.Whitelist {
 					rules.Whitelist = append(rules.Whitelist, r.Host)
 				}
 				rulesJSON, err := json.Marshal(rules)
@@ -75,22 +75,18 @@ func main() {
 
 				rulesJSON, err := ioutil.ReadFile(ruleFile)
 				cmd.FailOnError(err, "Couldn't read configuration file")
-				var rules struct {
-					Blacklist []string
-					Whitelist []string
-				}
-				bList := []policy.BlacklistRule{}
-				for _, r := range rules.Blacklist {
-					bList = append(bList, policy.BlacklistRule{Host: r})
-				}
-				wList := []policy.WhitelistRule{}
-				for _, r := range rules.Blacklist {
-					wList = append(wList, policy.WhitelistRule{Host: r})
-				}
+				rules := policy.RawRuleSet{}
 				err = json.Unmarshal(rulesJSON, &rules)
 				cmd.FailOnError(err, "Couldn't unmarshal rules list")
+				rs := policy.RuleSet{}
+				for _, r := range rules.Blacklist {
+					rs.Blacklist = append(rs.Blacklist, policy.BlacklistRule{Host: r})
+				}
+				for _, r := range rules.Whitelist {
+					rs.Whitelist = append(rs.Whitelist, policy.WhitelistRule{Host: r})
+				}
 
-				err = padb.LoadRules(bList, wList)
+				err = padb.LoadRules(rs)
 				cmd.FailOnError(err, "Couldn't load rules")
 
 				fmt.Println("# Loaded whitelist and blacklist into database")

policy/policy-authority-data.go

@@ -21,9 +21,24 @@ type domainRule struct {
 	Host string `db:"host"`
 }
 
+// BlacklistRule is used to hold rules blacklisting a DNS name
 type BlacklistRule domainRule
+
+// WhitelistRule is used to hold rules whitelisting a DNS name
 type WhitelistRule domainRule
 
+// RawRuleSet describes the rule set file format
+type RawRuleSet struct {
+	Blacklist []string
+	Whitelist []string
+}
+
+// RuleSet describes the rules to load into the policy database
+type RuleSet struct {
+	Blacklist []BlacklistRule
+	Whitelist []WhitelistRule
+}
+
 func reverseName(domain string) string {
 	labels := strings.Split(domain, ".")
 	for i, j := 0, len(labels)-1; i < j; i, j = i+1, j-1 {
@@ -57,7 +72,7 @@ func NewPolicyAuthorityDatabaseImpl(dbMap *gorp.DbMap) (padb *PolicyAuthorityDat
 
 // LoadRules loads the whitelist and blacklist into the database in a transaction
 // deleting any previous content
-func (padb *PolicyAuthorityDatabaseImpl) LoadRules(bRules []BlacklistRule, wRules []WhitelistRule) error {
+func (padb *PolicyAuthorityDatabaseImpl) LoadRules(rs RuleSet) error {
 	tx, err := padb.dbMap.Begin()
 	if err != nil {
 		tx.Rollback()
@@ -68,7 +83,7 @@ func (padb *PolicyAuthorityDatabaseImpl) LoadRules(bRules []BlacklistRule, wRule
 		tx.Rollback()
 		return err
 	}
-	for _, r := range bRules {
+	for _, r := range rs.Blacklist {
 		r.Host = reverseName(r.Host)
 		tx.Insert(&r)
 	}
@@ -77,7 +92,7 @@ func (padb *PolicyAuthorityDatabaseImpl) LoadRules(bRules []BlacklistRule, wRule
 		tx.Rollback()
 		return err
 	}
-	for _, r := range wRules {
+	for _, r := range rs.Whitelist {
 		tx.Insert(&r)
 	}
 
@@ -87,7 +102,8 @@ func (padb *PolicyAuthorityDatabaseImpl) LoadRules(bRules []BlacklistRule, wRule
 
 // DumpRules retrieves all domainRules in the database so they can be written to
 // disk
-func (padb *PolicyAuthorityDatabaseImpl) DumpRules() (bList []BlacklistRule, wList []WhitelistRule, err error) {
+func (padb *PolicyAuthorityDatabaseImpl) DumpRules() (rs RuleSet, err error) {
+	var bList []BlacklistRule
 	_, err = padb.dbMap.Select(&bList, "SELECT * FROM blacklist")
 	if err != nil {
 		return
@@ -95,8 +111,14 @@ func (padb *PolicyAuthorityDatabaseImpl) DumpRules() (bList []BlacklistRule, wLi
 	for _, r := range bList {
 		r.Host = reverseName(r.Host)
 	}
+	rs.Blacklist = bList
+	var wList []WhitelistRule
 	_, err = padb.dbMap.Select(&wList, "SELECT * FROM whitelist")
-	return bList, wList, err
+	if err != nil {
+		return
+	}
+	rs.Whitelist = wList
+	return rs, err
 }
 
 func (padb *PolicyAuthorityDatabaseImpl) allowedByBlacklist(host string) bool {
@@ -141,7 +163,7 @@ func (padb *PolicyAuthorityDatabaseImpl) CheckHostLists(host string, requireWhit
 	if requireWhitelisted {
 		if !padb.allowedByWhitelist(host) {
 			// return fmt.Errorf("Domain is not whitelisted for issuance")
-			return WhitelistedError{}
+			return NotWhitelistedError{}
 		}
 	}
 	// Overrides the whitelist if a blacklist rule is found

policy/policy-authority-data_test.go

@@ -28,13 +28,16 @@ func TestBlacklist(t *testing.T) {
 	p, cleanup := padbImpl(t)
 	defer cleanup()
 
-	err := p.LoadRules([]BlacklistRule{
-		BlacklistRule{
-			Host: "bad.com",
+	err := p.LoadRules(RuleSet{
+		Blacklist: []BlacklistRule{
+			BlacklistRule{
+				Host: "bad.com",
+			},
 		},
-	}, []WhitelistRule{
-		WhitelistRule{
-			Host: "good.bad.com",
+		Whitelist: []WhitelistRule{
+			WhitelistRule{
+				Host: "good.bad.com",
+			},
 		},
 	})
 	test.AssertNotError(t, err, "Couldn't load rules")
@@ -57,16 +60,19 @@ func TestWhitelist(t *testing.T) {
 	p, cleanup := padbImpl(t)
 	defer cleanup()
 
-	err := p.LoadRules([]BlacklistRule{
-		BlacklistRule{
-			Host: "bad.com",
+	err := p.LoadRules(RuleSet{
+		Blacklist: []BlacklistRule{
+			BlacklistRule{
+				Host: "bad.com",
+			},
 		},
-	}, []WhitelistRule{
-		WhitelistRule{
-			Host: "good.bad.com",
-		},
-		WhitelistRule{
-			Host: "good.com",
+		Whitelist: []WhitelistRule{
+			WhitelistRule{
+				Host: "good.bad.com",
+			},
+			WhitelistRule{
+				Host: "good.com",
+			},
 		},
 	})
 	test.AssertNotError(t, err, "Couldn't load rules")

policy/policy-authority.go

@@ -85,14 +85,14 @@ type NonPublicError struct{}
 // BlacklistedError indicates we have blacklisted one or more of these identifiers.
 type BlacklistedError struct{}
 
-// WhitelistedError indicates we have not whitelisted one or more of these identifiers.
-type WhitelistedError struct{}
+// NotWhitelistedError indicates we have not whitelisted one or more of these identifiers.
+type NotWhitelistedError struct{}
 
 func (e InvalidIdentifierError) Error() string { return "Invalid identifier type" }
 func (e SyntaxError) Error() string            { return "Syntax error" }
 func (e NonPublicError) Error() string         { return "Name does not end in a public suffix" }
 func (e BlacklistedError) Error() string       { return "Name is blacklisted" }
-func (e WhitelistedError) Error() string       { return "Name is not whitelisted" }
+func (e NotWhitelistedError) Error() string    { return "Name is not whitelisted" }
 
 // WillingToIssue determines whether the CA is willing to issue for the provided
 // identifier.

policy/policy-authority_test.go

@@ -108,11 +108,11 @@ func TestWillingToIssue(t *testing.T) {
 	pa, cleanup := paImpl(t)
 	defer cleanup()
 
-	rules := []BlacklistRule{}
+	rules := RuleSet{}
 	for _, b := range shouldBeBlacklisted {
-		rules = append(rules, BlacklistRule{Host: b})
+		rules.Blacklist = append(rules.Blacklist, BlacklistRule{Host: b})
 	}
-	err := pa.DB.LoadRules(rules, nil)
+	err := pa.DB.LoadRules(rules)
 	test.AssertNotError(t, err, "Couldn't load rules")
 
 	// Test for invalid identifier type