tidy: cleanup JSON hostname policy support. (#4214)

We transitioned this data to YAML to have support for comments and can
remove the legacy JSON support/test data.
This commit is contained in:
Daniel McCarney 2019-05-14 17:06:36 -04:00 committed by GitHub
parent 6e06f36309
commit 443c949180
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 87 additions and 163 deletions

View File

@ -196,7 +196,7 @@ func setup(t *testing.T) *testCtx {
pa, err := policy.New(nil)
test.AssertNotError(t, err, "Couldn't create PA")
err = pa.SetHostnamePolicyFile("../test/hostname-policy.json")
err = pa.SetHostnamePolicyFile("../test/hostname-policy.yaml")
test.AssertNotError(t, err, "Couldn't set hostname policy")
allowedExtensions := []cfsslConfig.OID{

View File

@ -35,7 +35,7 @@ func init() {
if err != nil {
log.Fatal(err)
}
err = pa.SetHostnamePolicyFile("../../test/hostname-policy.json")
err = pa.SetHostnamePolicyFile("../../test/hostname-policy.yaml")
if err != nil {
log.Fatal(err)
}

View File

@ -3,7 +3,6 @@ package policy
import (
"crypto/sha256"
"encoding/hex"
"encoding/json"
"fmt"
"math/rand"
"net"
@ -57,17 +56,13 @@ type blockedNamesPolicy struct {
// matching an entry in the list will be forbidden. (e.g. `ExactBlockedNames`
// containing `www.example.com` will not block `example.com` or
// `mail.example.com`).
//
// TODO(@cpu): Remove the JSON tag when data is updated to use the new field names
ExactBlockedNames []string `json:"exactBlacklist" yaml:"ExactBlockedNames"`
ExactBlockedNames []string `yaml:"ExactBlockedNames"`
// HighRiskBlockedNames is like ExactBlockedNames except that issuance is
// blocked for subdomains as well. (e.g. BlockedNames containing `example.com`
// will block `www.example.com`).
//
// This list typically doesn't change with much regularity.
//
// TODO(@cpu): Remove the JSON tag when data is updated to use the new field names
HighRiskBlockedNames []string `json:"blacklist" yaml:"HighRiskBlockedNames"`
HighRiskBlockedNames []string `yaml:"HighRiskBlockedNames"`
// AdminBlockedNames operates the same as BlockedNames but is changed with more
// frequency based on administrative blocks/revocations that are added over
@ -77,21 +72,9 @@ type blockedNamesPolicy struct {
}
// SetHostnamePolicyFile will load the given policy file, returning error if it
// fails. It will also start a reloader in case the file changes. It supports
// YAML and JSON serialization formats and chooses the correct unserialization
// method based on the file extension which must be ".yaml", ".yml", or ".json".
// fails. It will also start a reloader in case the file changes
func (pa *AuthorityImpl) SetHostnamePolicyFile(f string) error {
var loadHandler func([]byte) error
if strings.HasSuffix(f, ".json") {
loadHandler = pa.loadHostnamePolicy(json.Unmarshal)
} else if strings.HasSuffix(f, ".yml") || strings.HasSuffix(f, ".yaml") {
loadHandler = pa.loadHostnamePolicy(yaml.Unmarshal)
} else {
return fmt.Errorf(
"Hostname policy file %q has unknown extension. Supported: .yml,.yaml,.json",
f)
}
if _, err := reloader.New(f, loadHandler, pa.hostnamePolicyLoadError); err != nil {
if _, err := reloader.New(f, pa.loadHostnamePolicy, pa.hostnamePolicyLoadError); err != nil {
return err
}
return nil
@ -101,32 +84,23 @@ func (pa *AuthorityImpl) hostnamePolicyLoadError(err error) {
pa.log.AuditErrf("error loading hostname policy: %s", err)
}
// unmarshalHandler is a function type that abstracts away a choice between
// json.Unmarshal and yaml.Unmarshal, both of which take a byte slice, an
// interface to unmarshal to, and return an error.
type unmarshalHandler func([]byte, interface{}) error
// loadHostnamePolicy returns a reloader dataCallback function that uses the
// unmarshalHandler to load a hostname policy. The returned callback is suitable
// for use with reloader.New()
func (pa *AuthorityImpl) loadHostnamePolicy(unmarshal unmarshalHandler) func([]byte) error {
// Return a reloader dataCallback that uses the provided unmarshalHandler.
return func(contents []byte) error {
hash := sha256.Sum256(contents)
pa.log.Infof("loading hostname policy, sha256: %s", hex.EncodeToString(hash[:]))
var policy blockedNamesPolicy
err := unmarshal(contents, &policy)
if err != nil {
return err
}
if len(policy.HighRiskBlockedNames) == 0 {
return fmt.Errorf("No entries in HighRiskBlockedNames.")
}
if len(policy.ExactBlockedNames) == 0 {
return fmt.Errorf("No entries in ExactBlockedNames.")
}
return pa.processHostnamePolicy(policy)
// loadHostnamePolicy is a callback suitable for use with reloader.New() that
// will unmarshal a YAML hostname policy.
func (pa *AuthorityImpl) loadHostnamePolicy(contents []byte) error {
hash := sha256.Sum256(contents)
pa.log.Infof("loading hostname policy, sha256: %s", hex.EncodeToString(hash[:]))
var policy blockedNamesPolicy
err := yaml.Unmarshal(contents, &policy)
if err != nil {
return err
}
if len(policy.HighRiskBlockedNames) == 0 {
return fmt.Errorf("No entries in HighRiskBlockedNames.")
}
if len(policy.ExactBlockedNames) == 0 {
return fmt.Errorf("No entries in ExactBlockedNames.")
}
return pa.processHostnamePolicy(policy)
}
// processHostnamePolicy handles loading a new blockedNamesPolicy into the PA.

View File

@ -1,8 +1,6 @@
package policy
import (
"encoding/json"
"fmt"
"io/ioutil"
"os"
"testing"
@ -149,13 +147,6 @@ func TestWillingToIssue(t *testing.T) {
AdminBlockedNames: adminBlockedContents,
}
jsonPolicyBytes, err := json.Marshal(policy)
test.AssertNotError(t, err, "Couldn't JSON serialize blocklist")
jsonPolicyFile, _ := ioutil.TempFile("", "test-blocklist.*.json")
defer os.Remove(jsonPolicyFile.Name())
err = ioutil.WriteFile(jsonPolicyFile.Name(), jsonPolicyBytes, 0640)
test.AssertNotError(t, err, "Couldn't write JSON blocklist")
yamlPolicyBytes, err := yaml.Marshal(policy)
test.AssertNotError(t, err, "Couldn't YAML serialize blocklist")
yamlPolicyFile, _ := ioutil.TempFile("", "test-blocklist.*.yaml")
@ -163,73 +154,62 @@ func TestWillingToIssue(t *testing.T) {
err = ioutil.WriteFile(yamlPolicyFile.Name(), yamlPolicyBytes, 0640)
test.AssertNotError(t, err, "Couldn't write YAML blocklist")
testPolicyFile := func(f string) {
pa := paImpl(t)
pa := paImpl(t)
err = pa.SetHostnamePolicyFile(f)
test.AssertNotError(t, err, "Couldn't load rules")
err = pa.SetHostnamePolicyFile(yamlPolicyFile.Name())
test.AssertNotError(t, err, "Couldn't load rules")
// Test for invalid identifier type
identifier := core.AcmeIdentifier{Type: "ip", Value: "example.com"}
err = pa.WillingToIssue(identifier)
if err != errInvalidIdentifier {
t.Error("Identifier was not correctly forbidden: ", identifier)
}
// Test for invalid identifier type
identifier := core.AcmeIdentifier{Type: "ip", Value: "example.com"}
err = pa.WillingToIssue(identifier)
if err != errInvalidIdentifier {
t.Error("Identifier was not correctly forbidden: ", identifier)
}
// Test syntax errors
for _, tc := range testCases {
identifier := core.AcmeIdentifier{Type: core.IdentifierDNS, Value: tc.domain}
err := pa.WillingToIssue(identifier)
if err != tc.err {
t.Errorf("WillingToIssue(%q) = %q, expected %q", tc.domain, err, tc.err)
}
}
// Invalid encoding
err = pa.WillingToIssue(core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "www.xn--m.com"})
test.AssertError(t, err, "WillingToIssue didn't fail on a malformed IDN")
// Valid encoding
err = pa.WillingToIssue(core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "www.xn--mnich-kva.com"})
test.AssertNotError(t, err, "WillingToIssue failed on a properly formed IDN")
// IDN TLD
err = pa.WillingToIssue(core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "xn--example--3bhk5a.xn--p1ai"})
test.AssertNotError(t, err, "WillingToIssue failed on a properly formed domain with IDN TLD")
features.Reset()
// Test domains that are equal to public suffixes
for _, domain := range shouldBeTLDError {
identifier := core.AcmeIdentifier{Type: core.IdentifierDNS, Value: domain}
err := pa.WillingToIssue(identifier)
if err != errICANNTLD {
t.Error("Identifier was not correctly forbidden: ", identifier, err)
}
}
// Test expected blocked domains
for _, domain := range shouldBeBlocked {
identifier := core.AcmeIdentifier{Type: core.IdentifierDNS, Value: domain}
err := pa.WillingToIssue(identifier)
if err != errPolicyForbidden {
t.Error("Identifier was not correctly forbidden: ", identifier, err)
}
}
// Test acceptance of good names
for _, domain := range shouldBeAccepted {
identifier := core.AcmeIdentifier{Type: core.IdentifierDNS, Value: domain}
if err := pa.WillingToIssue(identifier); err != nil {
t.Error("Identifier was incorrectly forbidden: ", identifier, err)
}
// Test syntax errors
for _, tc := range testCases {
identifier := core.AcmeIdentifier{Type: core.IdentifierDNS, Value: tc.domain}
err := pa.WillingToIssue(identifier)
if err != tc.err {
t.Errorf("WillingToIssue(%q) = %q, expected %q", tc.domain, err, tc.err)
}
}
// Both the JSON and the YAML policy files should behave the exact same way
// when tested.
for _, f := range []string{
jsonPolicyFile.Name(),
yamlPolicyFile.Name(),
} {
testPolicyFile(f)
// Invalid encoding
err = pa.WillingToIssue(core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "www.xn--m.com"})
test.AssertError(t, err, "WillingToIssue didn't fail on a malformed IDN")
// Valid encoding
err = pa.WillingToIssue(core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "www.xn--mnich-kva.com"})
test.AssertNotError(t, err, "WillingToIssue failed on a properly formed IDN")
// IDN TLD
err = pa.WillingToIssue(core.AcmeIdentifier{Type: core.IdentifierDNS, Value: "xn--example--3bhk5a.xn--p1ai"})
test.AssertNotError(t, err, "WillingToIssue failed on a properly formed domain with IDN TLD")
features.Reset()
// Test domains that are equal to public suffixes
for _, domain := range shouldBeTLDError {
identifier := core.AcmeIdentifier{Type: core.IdentifierDNS, Value: domain}
err := pa.WillingToIssue(identifier)
if err != errICANNTLD {
t.Error("Identifier was not correctly forbidden: ", identifier, err)
}
}
// Test expected blocked domains
for _, domain := range shouldBeBlocked {
identifier := core.AcmeIdentifier{Type: core.IdentifierDNS, Value: domain}
err := pa.WillingToIssue(identifier)
if err != errPolicyForbidden {
t.Error("Identifier was not correctly forbidden: ", identifier, err)
}
}
// Test acceptance of good names
for _, domain := range shouldBeAccepted {
identifier := core.AcmeIdentifier{Type: core.IdentifierDNS, Value: domain}
if err := pa.WillingToIssue(identifier); err != nil {
t.Error("Identifier was incorrectly forbidden: ", identifier, err)
}
}
}
@ -242,12 +222,12 @@ func TestWillingToIssueWildcard(t *testing.T) {
}
pa := paImpl(t)
bannedBytes, err := json.Marshal(blockedNamesPolicy{
bannedBytes, err := yaml.Marshal(blockedNamesPolicy{
HighRiskBlockedNames: bannedDomains,
ExactBlockedNames: exactBannedDomains,
})
test.AssertNotError(t, err, "Couldn't serialize banned list")
f, _ := ioutil.TempFile("", "test-wildcard-banlist.*.json")
f, _ := ioutil.TempFile("", "test-wildcard-banlist.*.yaml")
defer os.Remove(f.Name())
err = ioutil.WriteFile(f.Name(), bannedBytes, 0640)
test.AssertNotError(t, err, "Couldn't write serialized banned list to file")
@ -394,7 +374,7 @@ func TestChallengesForWildcard(t *testing.T) {
test.AssertEquals(t, challenges[0].Type, core.ChallengeTypeDNS01)
}
// TestMalformedExactBlocklist tests that loading a JSON policy file with an
// TestMalformedExactBlocklist tests that loading a YAML policy file with an
// invalid exact blocklist entry will fail as expected.
func TestMalformedExactBlocklist(t *testing.T) {
pa := paImpl(t)
@ -407,37 +387,23 @@ func TestMalformedExactBlocklist(t *testing.T) {
"placeholder.domain.not.important.for.this.test.com",
}
// Create JSON for the exactBannedDomains
bannedBytes, err := json.Marshal(blockedNamesPolicy{
// Create YAML for the exactBannedDomains
bannedBytes, err := yaml.Marshal(blockedNamesPolicy{
HighRiskBlockedNames: bannedDomains,
ExactBlockedNames: exactBannedDomains,
})
test.AssertNotError(t, err, "Couldn't serialize banned list")
// Create a temp file for the JSON contents
f, _ := ioutil.TempFile("", "test-invalid-exactblocklist.*.json")
// Create a temp file for the YAML contents
f, _ := ioutil.TempFile("", "test-invalid-exactblocklist.*.yaml")
defer os.Remove(f.Name())
// Write the JSON to the temp file
// Write the YAML to the temp file
err = ioutil.WriteFile(f.Name(), bannedBytes, 0640)
test.AssertNotError(t, err, "Couldn't write serialized banned list to file")
// Try to use the JSON tempfile as the hostname policy. It should produce an
// Try to use the YAML tempfile as the hostname policy. It should produce an
// error since the exact blocklist contents are malformed.
err = pa.SetHostnamePolicyFile(f.Name())
test.AssertError(t, err, "Loaded invalid exact blocklist content without error")
test.AssertEquals(t, err.Error(), "Malformed ExactBlockedNames entry, only one label: \"com\"")
}
func TestSetHostnamePolicyFileExtension(t *testing.T) {
filename := "hostname.policy.json.j2"
expectedErrMsg := fmt.Sprintf(
`Hostname policy file %q has unknown extension. Supported: .yml,.yaml,.json`,
filename)
pa := paImpl(t)
if err := pa.SetHostnamePolicyFile(filename); err != nil && err.Error() != expectedErrMsg {
t.Errorf("expected SetHostnamePolicyFile error %q got %q", expectedErrMsg, err.Error())
} else if err == nil {
t.Errorf("expected SetHostnamePolicyFile error %q got nil", expectedErrMsg)
}
}

View File

@ -295,7 +295,7 @@ func initAuthorities(t *testing.T) (*DummyValidationAuthority, *sa.SQLStorageAut
})
test.AssertNotError(t, err, "Couldn't create PA")
err = pa.SetHostnamePolicyFile("../test/hostname-policy.json")
err = pa.SetHostnamePolicyFile("../test/hostname-policy.yaml")
test.AssertNotError(t, err, "Couldn't set hostname policy")
stats := metrics.NewNoopScope()

View File

@ -41,7 +41,7 @@
"lifespanOCSP": "96h",
"maxNames": 100,
"enableMustStaple": true,
"hostnamePolicyFile": "test/hostname-policy.json",
"hostnamePolicyFile": "test/hostname-policy.yaml",
"cfssl": {
"signing": {
"profiles": {

View File

@ -41,7 +41,7 @@
"lifespanOCSP": "96h",
"maxNames": 100,
"enableMustStaple": true,
"hostnamePolicyFile": "test/hostname-policy.json",
"hostnamePolicyFile": "test/hostname-policy.yaml",
"cfssl": {
"signing": {
"profiles": {

View File

@ -2,7 +2,7 @@
"certChecker": {
"dbConnectFile": "test/secrets/cert_checker_dburl",
"maxDBConns": 10,
"hostnamePolicyFile": "test/hostname-policy.json"
"hostnamePolicyFile": "test/hostname-policy.yaml"
},
"pa": {

View File

@ -4,7 +4,7 @@
"maxConcurrentRPCServerRequests": 100000,
"maxContactsPerRegistration": 100,
"debugAddr": ":8002",
"hostnamePolicyFile": "test/hostname-policy.json",
"hostnamePolicyFile": "test/hostname-policy.yaml",
"maxNames": 100,
"reuseValidAuthz": true,
"authorizationLifetimeDays": 30,

View File

@ -1,16 +0,0 @@
{
"ExactBlacklist": [
"highrisk.le-test.hoffman-andrews.com",
"exactblacklist.letsencrypt.org"
],
"Blacklist": [
"in-addr.arpa",
"example",
"example.net",
"example.org",
"invalid",
"local",
"localhost",
"test"
]
}

View File

@ -367,7 +367,7 @@ def test_wildcard_exactblacklist():
should fail with a policy error.
"""
# We include "highrisk.le-test.hoffman-andrews.com" in `test/hostname-policy.json`
# We include "highrisk.le-test.hoffman-andrews.com" in `test/hostname-policy.yaml`
# Issuing for "*.le-test.hoffman-andrews.com" should be blocked
domain = "*.le-test.hoffman-andrews.com"
# We expect this to produce a policy problem