Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove config live reloader package #7112

Merged
merged 11 commits into from
Oct 26, 2023
50 changes: 12 additions & 38 deletions ca/ca_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ var (
)

const arbitraryRegID int64 = 1001
const yamlLoadErrMsg = "Error loading YAML bytes for ECDSA allow list:"

// Useful key and certificate files.
const caKeyFile = "../test/test-ca.key"
Expand Down Expand Up @@ -328,7 +327,7 @@ func TestIssuePrecertificate(t *testing.T) {
// the precertificates previously generated from the preceding
// "precertificate" test.
for _, mode := range []string{"precertificate", "certificate-for-precertificate"} {
ca, sa := issueCertificateSubTestSetup(t)
ca, sa := issueCertificateSubTestSetup(t, nil)

t.Run(fmt.Sprintf("%s - %s", mode, testCase.name), func(t *testing.T) {
req, err := x509.ParseCertificateRequest(testCase.csr)
Expand Down Expand Up @@ -366,21 +365,18 @@ func TestIssuePrecertificate(t *testing.T) {
}
}

func makeECDSAAllowListBytes(regID int64) []byte {
regIDBytes := []byte(fmt.Sprintf("%d", regID))
contents := []byte(`
- `)
return append(contents, regIDBytes...)
}

func issueCertificateSubTestSetup(t *testing.T) (*certificateAuthorityImpl, *mockSA) {
func issueCertificateSubTestSetup(t *testing.T, e *ECDSAAllowList) (*certificateAuthorityImpl, *mockSA) {
testCtx := setup(t)
ecdsaAllowList := &ECDSAAllowList{}
if e == nil {
e = ecdsaAllowList
}
sa := &mockSA{}
ca, err := NewCertificateAuthorityImpl(
sa,
testCtx.pa,
testCtx.boulderIssuers,
&ECDSAAllowList{},
e,
testCtx.certExpiry,
testCtx.certBackdate,
testCtx.serialPrefix,
Expand Down Expand Up @@ -482,47 +478,25 @@ func TestECDSAAllowList(t *testing.T) {
req := &capb.IssueCertificateRequest{Csr: ECDSACSR, RegistrationID: arbitraryRegID}

// With allowlist containing arbitraryRegID, issuance should come from ECDSA issuer.
ca, _ := issueCertificateSubTestSetup(t)
contents := makeECDSAAllowListBytes(arbitraryRegID)
err := ca.ecdsaAllowList.Update(contents)
if err != nil {
t.Errorf("%s %s", yamlLoadErrMsg, err)
t.FailNow()
}
regIDMap := makeRegIDsMap([]int64{arbitraryRegID})
ca, _ := issueCertificateSubTestSetup(t, &ECDSAAllowList{regIDMap})
result, err := ca.IssuePrecertificate(ctx, req)
test.AssertNotError(t, err, "Failed to issue certificate")
cert, err := x509.ParseCertificate(result.DER)
test.AssertNotError(t, err, "Certificate failed to parse")
test.AssertByteEquals(t, cert.RawIssuer, caCert2.RawSubject)

// Attempts to update the allow list with malformed YAML should
// fail, but the allowlist should still contain arbitraryRegID, so
// issuance should come from ECDSA issuer
malformed_yaml := []byte(`
)(\/=`)
err = ca.ecdsaAllowList.Update(malformed_yaml)
test.AssertError(t, err, "Update method accepted malformed YAML")
result, err = ca.IssuePrecertificate(ctx, req)
test.AssertNotError(t, err, "Failed to issue certificate after Update was called with malformed YAML")
cert, err = x509.ParseCertificate(result.DER)
test.AssertNotError(t, err, "Certificate failed to parse")
test.AssertByteEquals(t, cert.RawIssuer, caCert2.RawSubject)

// With allowlist not containing arbitraryRegID, issuance should fall back to RSA issuer.
contents = makeECDSAAllowListBytes(int64(2002))
err = ca.ecdsaAllowList.Update(contents)
if err != nil {
t.Errorf("%s %s", yamlLoadErrMsg, err)
t.FailNow()
}
regIDMap = makeRegIDsMap([]int64{2002})
ca, _ = issueCertificateSubTestSetup(t, &ECDSAAllowList{regIDMap})
result, err = ca.IssuePrecertificate(ctx, req)
test.AssertNotError(t, err, "Failed to issue certificate")
cert, err = x509.ParseCertificate(result.DER)
test.AssertNotError(t, err, "Certificate failed to parse")
test.AssertByteEquals(t, cert.RawIssuer, caCert.RawSubject)

// With empty allowlist but ECDSAForAll enabled, issuance should come from ECDSA issuer.
ca, _ = issueCertificateSubTestSetup(t)
ca, _ = issueCertificateSubTestSetup(t, nil)
_ = features.Set(map[string]bool{"ECDSAForAll": true})
defer features.Reset()
result, err = ca.IssuePrecertificate(ctx, req)
Expand Down
71 changes: 17 additions & 54 deletions ca/ecdsa_allow_list.go
Original file line number Diff line number Diff line change
@@ -1,64 +1,22 @@
package ca

import (
"sync"
"os"

"github.com/letsencrypt/boulder/log"
"github.com/letsencrypt/boulder/reloader"
"github.com/letsencrypt/boulder/strictyaml"
"github.com/prometheus/client_golang/prometheus"
)

// ECDSAAllowList acts as a container for a map of Registration IDs, a
// mutex, and a file reloader. This allows the map of IDs to be updated
// safely if changes to the allow list are detected.
// ECDSAAllowList acts as a container for a map of Registration IDs.
type ECDSAAllowList struct {
sync.RWMutex
regIDsMap map[int64]bool
reloader *reloader.Reloader
logger log.Logger
}

// Update is an exported method (typically specified as a callback to a
// file reloader) that replaces the inner `regIDsMap` with the contents
// of a YAML list (as bytes).
func (e *ECDSAAllowList) Update(contents []byte) error {
var regIDs []int64
err := strictyaml.Unmarshal(contents, &regIDs)
if err != nil {
return err
}
e.Lock()
defer e.Unlock()
e.regIDsMap = makeRegIDsMap(regIDs)
return nil
}

// permitted checks if ECDSA issuance is permitted for the specified
// Registration ID.
func (e *ECDSAAllowList) permitted(regID int64) bool {
e.RLock()
defer e.RUnlock()
return e.regIDsMap[regID]
}

// length returns the number of entries currently in the allow list.
func (e *ECDSAAllowList) length() int {
e.RLock()
defer e.RUnlock()
return len(e.regIDsMap)
}

// Stop stops an active allow list reloader. Typically called during
// boulder-ca shutdown.
func (e *ECDSAAllowList) Stop() {
e.Lock()
defer e.Unlock()
if e.reloader != nil {
e.reloader.Stop()
}
}

func makeRegIDsMap(regIDs []int64) map[int64]bool {
regIDsMap := make(map[int64]bool)
for _, regID := range regIDs {
Expand All @@ -67,17 +25,22 @@ func makeRegIDsMap(regIDs []int64) map[int64]bool {
return regIDsMap
}

// NewECDSAAllowListFromFile is exported to allow `boulder-ca` to
// construct a new `ECDSAAllowList` object. An initial entry count is
// returned to `boulder-ca` for logging purposes.
func NewECDSAAllowListFromFile(filename string, logger log.Logger, metric *prometheus.GaugeVec) (*ECDSAAllowList, int, error) {
allowList := &ECDSAAllowList{logger: logger}
// Create an allow list reloader. This also populates the inner
// allowList regIDsMap.
reloader, err := reloader.New(filename, allowList.Update, logger)
// NewECDSAAllowListFromFile is exported to allow `boulder-ca` to construct a
// new `ECDSAAllowList` object. It returns the ECDSAAllowList, the size of allow
// list after attempting to load it (for CA logging purposes so inner fields don't need to be exported), or an error.
func NewECDSAAllowListFromFile(filename string) (*ECDSAAllowList, int, error) {
configBytes, err := os.ReadFile(filename)
if err != nil {
return nil, 0, err
}

var regIDs []int64
err = strictyaml.Unmarshal(configBytes, &regIDs)
if err != nil {
return nil, 0, err
}
allowList.reloader = reloader
return allowList, allowList.length(), nil
allowList := &ECDSAAllowList{}
allowList.regIDsMap = makeRegIDsMap(regIDs)

pgporada marked this conversation as resolved.
Show resolved Hide resolved
return allowList, len(allowList.regIDsMap), nil
}
27 changes: 10 additions & 17 deletions ca/ecdsa_allow_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,11 @@ package ca

import (
"testing"

"github.com/letsencrypt/boulder/log"
"github.com/letsencrypt/boulder/reloader"
"github.com/prometheus/client_golang/prometheus"
)

func TestNewECDSAAllowListFromFile(t *testing.T) {
type args struct {
filename string
reloader *reloader.Reloader
logger log.Logger
metric *prometheus.GaugeVec
}
tests := []struct {
name string
Expand All @@ -24,28 +17,28 @@ func TestNewECDSAAllowListFromFile(t *testing.T) {
}{
{
name: "one entry",
args: args{"testdata/ecdsa_allow_list.yml", nil, nil, nil},
args: args{"testdata/ecdsa_allow_list.yml"},
want1337Permitted: true,
wantEntries: 1,
wantErrBool: false,
},
{
name: "one entry but it's not 1337",
args: args{"testdata/ecdsa_allow_list2.yml", nil, nil, nil},
args: args{"testdata/ecdsa_allow_list2.yml"},
want1337Permitted: false,
wantEntries: 1,
wantErrBool: false,
},
{
name: "should error due to no file",
args: args{"testdata/ecdsa_allow_list_no_exist.yml", nil, nil, nil},
args: args{"testdata/ecdsa_allow_list_no_exist.yml"},
want1337Permitted: false,
wantEntries: 0,
wantErrBool: true,
},
{
name: "should error due to malformed YAML",
args: args{"testdata/ecdsa_allow_list_malformed.yml", nil, nil, nil},
args: args{"testdata/ecdsa_allow_list_malformed.yml"},
want1337Permitted: false,
wantEntries: 0,
wantErrBool: true,
Expand All @@ -54,18 +47,18 @@ func TestNewECDSAAllowListFromFile(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, got1, err := NewECDSAAllowListFromFile(tt.args.filename, tt.args.logger, tt.args.metric)
allowList, gotEntries, err := NewECDSAAllowListFromFile(tt.args.filename)

if (err != nil) != tt.wantErrBool {
t.Errorf("NewECDSAAllowListFromFile() error = %v, wantErr %v", err, tt.wantErrBool)
t.Error(got, got1, err)
t.Error(allowList, gotEntries, err)
return
}
if got != nil && got.permitted(1337) != tt.want1337Permitted {
t.Errorf("NewECDSAAllowListFromFile() got = %v, want %v", got, tt.want1337Permitted)
if allowList != nil && allowList.permitted(1337) != tt.want1337Permitted {
t.Errorf("NewECDSAAllowListFromFile() allowList = %v, want %v", allowList, tt.want1337Permitted)
}
if got1 != tt.wantEntries {
t.Errorf("NewECDSAAllowListFromFile() got1 = %v, want %v", got1, tt.wantEntries)
if gotEntries != tt.wantEntries {
t.Errorf("NewECDSAAllowListFromFile() gotEntries = %v, want %v", gotEntries, tt.wantEntries)
}
})
}
Expand Down
16 changes: 4 additions & 12 deletions cmd/boulder-ca/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,20 +221,12 @@ func main() {
cmd.FailOnError(err, "Unable to create key policy")

var ecdsaAllowList *ca.ECDSAAllowList
var entries int
if c.CA.ECDSAAllowListFilename != "" {
// Create a gauge vector to track allow list reloads.
allowListGauge := prometheus.NewGaugeVec(prometheus.GaugeOpts{
Name: "ecdsa_allow_list_status",
Help: "Number of ECDSA allow list entries and status of most recent update attempt",
}, []string{"result"})
scope.MustRegister(allowListGauge)

// Create a reloadable allow list object.
var entries int
ecdsaAllowList, entries, err = ca.NewECDSAAllowListFromFile(c.CA.ECDSAAllowListFilename, logger, allowListGauge)
// Create an allow list object.
ecdsaAllowList, entries, err = ca.NewECDSAAllowListFromFile(c.CA.ECDSAAllowListFilename)
cmd.FailOnError(err, "Unable to load ECDSA allow list from YAML file")
defer ecdsaAllowList.Stop()
logger.Infof("Created a reloadable allow list, it was initialized with %d entries", entries)
logger.Infof("Loaded an ECDSA allow list with %d entries", entries)
}

srv := bgrpc.NewServer(c.CA.GRPCCA, logger)
Expand Down
19 changes: 7 additions & 12 deletions policy/pa.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"math/rand"
"net"
"net/mail"
"os"
"regexp"
"slices"
"strings"
Expand All @@ -21,7 +22,6 @@ import (
"github.com/letsencrypt/boulder/iana"
"github.com/letsencrypt/boulder/identifier"
blog "github.com/letsencrypt/boulder/log"
"github.com/letsencrypt/boulder/reloader"
"github.com/letsencrypt/boulder/strictyaml"
)

Expand Down Expand Up @@ -74,22 +74,17 @@ type blockedNamesPolicy struct {
AdminBlockedNames []string `yaml:"AdminBlockedNames"`
}

// SetHostnamePolicyFile will load the given policy file, returning error if it
// fails. It will also start a reloader in case the file changes
// SetHostnamePolicyFile will load the given policy file, returning an error if
// it fails.
func (pa *AuthorityImpl) SetHostnamePolicyFile(f string) error {
pgporada marked this conversation as resolved.
Show resolved Hide resolved
if _, err := reloader.New(f, pa.loadHostnamePolicy, pa.log); err != nil {
configBytes, err := os.ReadFile(f)
if err != nil {
return err
}
return nil
}

// 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)
hash := sha256.Sum256(configBytes)
pa.log.Infof("loading hostname policy, sha256: %s", hex.EncodeToString(hash[:]))
var policy blockedNamesPolicy
err := strictyaml.Unmarshal(contents, &policy)
err = strictyaml.Unmarshal(configBytes, &policy)
if err != nil {
return err
}
Expand Down
8 changes: 6 additions & 2 deletions ra/ra.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"math/big"
"net"
"net/url"
"os"
"slices"
"sort"
"strconv"
Expand Down Expand Up @@ -48,7 +49,6 @@ import (
rapb "github.com/letsencrypt/boulder/ra/proto"
"github.com/letsencrypt/boulder/ratelimit"
"github.com/letsencrypt/boulder/ratelimits"
"github.com/letsencrypt/boulder/reloader"
"github.com/letsencrypt/boulder/revocation"
sapb "github.com/letsencrypt/boulder/sa/proto"
vapb "github.com/letsencrypt/boulder/va/proto"
Expand Down Expand Up @@ -275,7 +275,11 @@ func NewRegistrationAuthorityImpl(
}

func (ra *RegistrationAuthorityImpl) SetRateLimitPoliciesFile(filename string) error {
pgporada marked this conversation as resolved.
Show resolved Hide resolved
_, err := reloader.New(filename, ra.rlPolicies.LoadPolicies, ra.log)
configBytes, err := os.ReadFile(filename)
if err != nil {
return err
}
err = ra.rlPolicies.LoadPolicies(configBytes)
if err != nil {
return err
}
Expand Down
Loading