Skip to content

Commit

Permalink
Ensure certificates retrieved through the cache get persisted with au…
Browse files Browse the repository at this point in the history
…to-config (#8409)
  • Loading branch information
mkeeler committed Jul 30, 2020
1 parent 4f98af0 commit c9b6615
Show file tree
Hide file tree
Showing 7 changed files with 198 additions and 30 deletions.
13 changes: 12 additions & 1 deletion agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,8 @@ func New(options ...AgentOption) (*Agent, error) {
WithNodeName(a.config.NodeName).
WithFallback(a.autoConfigFallbackTLS).
WithLogger(a.logger.Named(logging.AutoConfig)).
WithTokens(a.tokens)
WithTokens(a.tokens).
WithPersistence(a.autoConfigPersist)
acCertMon, err := certmon.New(cmConf)
if err != nil {
return nil, err
Expand Down Expand Up @@ -889,9 +890,19 @@ func (a *Agent) autoEncryptInitialCertificate(ctx context.Context) (*structs.Sig
}

func (a *Agent) autoConfigFallbackTLS(ctx context.Context) (*structs.SignedResponse, error) {
if a.autoConf == nil {
return nil, fmt.Errorf("AutoConfig manager has not been created yet")
}
return a.autoConf.FallbackTLS(ctx)
}

func (a *Agent) autoConfigPersist(resp *structs.SignedResponse) error {
if a.autoConf == nil {
return fmt.Errorf("AutoConfig manager has not been created yet")
}
return a.autoConf.RecordUpdatedCerts(resp)
}

func (a *Agent) listenAndServeGRPC() error {
if len(a.config.GRPCAddrs) < 1 {
return nil
Expand Down
54 changes: 39 additions & 15 deletions agent/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"
"time"

"github.com/golang/protobuf/jsonpb"
"github.com/google/tcpproxy"
"github.com/hashicorp/consul/agent/cache"
cachetype "github.com/hashicorp/consul/agent/cache-types"
Expand All @@ -31,6 +32,7 @@ import (
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/internal/go-sso/oidcauth/oidcauthtest"
"github.com/hashicorp/consul/ipaddr"
"github.com/hashicorp/consul/proto/pbautoconf"
"github.com/hashicorp/consul/sdk/freeport"
"github.com/hashicorp/consul/sdk/testutil"
"github.com/hashicorp/consul/sdk/testutil/retry"
Expand Down Expand Up @@ -4728,21 +4730,28 @@ func TestAutoConfig_Integration(t *testing.T) {
})
require.NoError(t, err)

client := StartTestAgent(t, TestAgent{Name: "test-client", HCL: `
bootstrap = false
server = false
ca_file = "` + caFile + `"
verify_outgoing = true
verify_server_hostname = true
node_name = "test-client"
ports {
server = ` + strconv.Itoa(srv.Config.RPCBindAddr.Port) + `
}
auto_config {
enabled = true
intro_token = "` + token + `"
server_addresses = ["` + srv.Config.RPCBindAddr.String() + `"]
}`})
client := StartTestAgent(t, TestAgent{Name: "test-client",
Overrides: `
connect {
test_ca_leaf_root_change_spread = "1ns"
}
`,
HCL: `
bootstrap = false
server = false
ca_file = "` + caFile + `"
verify_outgoing = true
verify_server_hostname = true
node_name = "test-client"
ports {
server = ` + strconv.Itoa(srv.Config.RPCBindAddr.Port) + `
}
auto_config {
enabled = true
intro_token = "` + token + `"
server_addresses = ["` + srv.Config.RPCBindAddr.String() + `"]
}`,
})

defer client.Shutdown()

Expand Down Expand Up @@ -4782,6 +4791,21 @@ func TestAutoConfig_Integration(t *testing.T) {
// ensure that a new cert gets generated and pushed into the TLS configurator
retry.Run(t, func(r *retry.R) {
require.NotEqual(r, cert1, client.Agent.tlsConfigurator.Cert())

// check that the on disk certs match expectations
data, err := ioutil.ReadFile(filepath.Join(client.DataDir, "auto-config.json"))
require.NoError(r, err)
rdr := strings.NewReader(string(data))

var resp pbautoconf.AutoConfigResponse
pbUnmarshaler := &jsonpb.Unmarshaler{
AllowUnknownFields: false,
}
require.NoError(r, pbUnmarshaler.Unmarshal(rdr, &resp), "data: %s", data)

actual, err := tls.X509KeyPair([]byte(resp.Certificate.CertPEM), []byte(resp.Certificate.PrivateKeyPEM))
require.NoError(r, err)
require.Equal(r, client.Agent.tlsConfigurator.Cert(), &actual)
})

// spot check that we now have an ACL token
Expand Down
36 changes: 27 additions & 9 deletions agent/auto-config/auto_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,16 @@ var (
// then we will need to add some locking here. I am deferring that for now
// to help ease the review of this already large PR.
type AutoConfig struct {
builderOpts config.BuilderOpts
logger hclog.Logger
directRPC DirectRPC
waiter *lib.RetryWaiter
overrides []config.Source
certMonitor CertMonitor
config *config.RuntimeConfig
autoConfigData string
cancel context.CancelFunc
builderOpts config.BuilderOpts
logger hclog.Logger
directRPC DirectRPC
waiter *lib.RetryWaiter
overrides []config.Source
certMonitor CertMonitor
config *config.RuntimeConfig
autoConfigResponse *pbautoconf.AutoConfigResponse
autoConfigData string
cancel context.CancelFunc
}

// New creates a new AutoConfig object for providing automatic
Expand Down Expand Up @@ -493,6 +494,8 @@ func (ac *AutoConfig) generateCSR() (csr string, key string, err error) {
// config data to be used during a call to ReadConfig, updating the
// tls Configurator and prepopulating the cache.
func (ac *AutoConfig) update(resp *pbautoconf.AutoConfigResponse) error {
ac.autoConfigResponse = resp

if err := ac.updateConfigFromResponse(resp); err != nil {
return err
}
Expand Down Expand Up @@ -591,3 +594,18 @@ func (ac *AutoConfig) FallbackTLS(ctx context.Context) (*structs.SignedResponse,

return extractSignedResponse(resp)
}

func (ac *AutoConfig) RecordUpdatedCerts(resp *structs.SignedResponse) error {
var err error
ac.autoConfigResponse.ExtraCACertificates = resp.ManualCARoots
ac.autoConfigResponse.CARoots, err = translateCARootsToProtobuf(&resp.ConnectCARoots)
if err != nil {
return err
}
ac.autoConfigResponse.Certificate, err = translateIssuedCertToProtobuf(&resp.IssuedCert)
if err != nil {
return err
}

return ac.recordResponse(ac.autoConfigResponse)
}
31 changes: 31 additions & 0 deletions agent/auto-config/config_translate.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,3 +226,34 @@ func mapstructureTranslateToStructs(in interface{}, out interface{}) error {

return decoder.Decode(in)
}

func translateCARootsToProtobuf(in *structs.IndexedCARoots) (*pbconnect.CARoots, error) {
var out pbconnect.CARoots
if err := mapstructureTranslateToProtobuf(in, &out); err != nil {
return nil, fmt.Errorf("Failed to re-encode CA Roots: %w", err)
}

return &out, nil
}

func translateIssuedCertToProtobuf(in *structs.IssuedCert) (*pbconnect.IssuedCert, error) {
var out pbconnect.IssuedCert
if err := mapstructureTranslateToProtobuf(in, &out); err != nil {
return nil, fmt.Errorf("Failed to re-encode CA Roots: %w", err)
}

return &out, nil
}

func mapstructureTranslateToProtobuf(in interface{}, out interface{}) error {
decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
DecodeHook: proto.HookTimeToPBTimestamp,
Result: out,
})

if err != nil {
return err
}

return decoder.Decode(in)
}
33 changes: 33 additions & 0 deletions agent/cert-monitor/cert_monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ type CertMonitor struct {
tokens *token.Store
leafReq cachetype.ConnectCALeafRequest
rootsReq structs.DCSpecificRequest
persist PersistFunc
fallback FallbackFunc
fallbackLeeway time.Duration
fallbackRetry time.Duration
Expand All @@ -66,6 +67,11 @@ type CertMonitor struct {
// events from the token store when the Agent
// token is updated.
tokenUpdates token.Notifier

// this is used to keep a local copy of the certs
// keys and ca certs. It will be used to persist
// all of the local state at once.
certs structs.SignedResponse
}

// New creates a new CertMonitor for automatically rotating
Expand Down Expand Up @@ -115,6 +121,7 @@ func New(config *Config) (*CertMonitor, error) {
cache: config.Cache,
tokens: config.Tokens,
tlsConfigurator: config.TLSConfigurator,
persist: config.Persist,
fallback: config.Fallback,
fallbackLeeway: config.FallbackLeeway,
fallbackRetry: config.FallbackRetry,
Expand All @@ -135,6 +142,8 @@ func (m *CertMonitor) Update(certs *structs.SignedResponse) error {
return nil
}

m.certs = *certs

if err := m.populateCache(certs); err != nil {
return fmt.Errorf("error populating cache with certificates: %w", err)
}
Expand Down Expand Up @@ -306,6 +315,8 @@ func (m *CertMonitor) handleCacheEvent(u cache.UpdateEvent) error {
return fmt.Errorf("invalid type for roots watch response: %T", u.Result)
}

m.certs.ConnectCARoots = *roots

var pems []string
for _, root := range roots.Roots {
pems = append(pems, root.RootCert)
Expand All @@ -314,6 +325,13 @@ func (m *CertMonitor) handleCacheEvent(u cache.UpdateEvent) error {
if err := m.tlsConfigurator.UpdateAutoTLSCA(pems); err != nil {
return fmt.Errorf("failed to update Connect CA certificates: %w", err)
}

if m.persist != nil {
copy := m.certs
if err := m.persist(&copy); err != nil {
return fmt.Errorf("failed to persist certificate package: %w", err)
}
}
case leafWatchID:
m.logger.Debug("leaf certificate watch fired - updating TLS certificate")
if u.Err != nil {
Expand All @@ -324,9 +342,19 @@ func (m *CertMonitor) handleCacheEvent(u cache.UpdateEvent) error {
if !ok {
return fmt.Errorf("invalid type for agent leaf cert watch response: %T", u.Result)
}

m.certs.IssuedCert = *leaf

if err := m.tlsConfigurator.UpdateAutoTLSCert(leaf.CertPEM, leaf.PrivateKeyPEM); err != nil {
return fmt.Errorf("failed to update the agent leaf cert: %w", err)
}

if m.persist != nil {
copy := m.certs
if err := m.persist(&copy); err != nil {
return fmt.Errorf("failed to persist certificate package: %w", err)
}
}
}

return nil
Expand Down Expand Up @@ -380,6 +408,11 @@ func (m *CertMonitor) handleFallback(ctx context.Context) error {
return fmt.Errorf("error when getting new agent certificate: %w", err)
}

if m.persist != nil {
if err := m.persist(reply); err != nil {
return fmt.Errorf("failed to persist certificate package: %w", err)
}
}
return m.Update(reply)
}

Expand Down
Loading

0 comments on commit c9b6615

Please sign in to comment.